| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-28 | |||
| 18:49:34 | melwitt | zigo: yup, have at it. ping me whenever you post it and I'll review it | |
| 18:49:44 | zigo | Cheers. | |
| 18:56:57 | openstackgerrit | Merged openstack/nova master: Implement query param schema for sec group APIs https://review.openstack.org/521353 | |
| 18:57:11 | openstackgerrit | Merged openstack/nova master: Add instance action record for lock/unlock instances https://review.openstack.org/523353 | |
| 18:57:33 | openstackgerrit | Merged openstack/nova master: Add regression test for rebuild with new image doubling allocations https://review.openstack.org/521153 | |
| 18:57:41 | mriedem | melwitt: if the config option isn't used until later in the series, why isn't it just introduced later in the series, when it's used? | |
| 18:59:50 | melwitt | mriedem: I think it's because in the series, patch 1 adds a base class (that uses the config option to iterate through choices), patch 2 adds one auth implementation, patch 3 adds another (and the final) auth implementation, each of which is one of the config option choices | |
| 19:00:41 | zigo | melwitt: How can I import nova.virt.libvirt.driver.LibvirtDriver.disk_cachemode() ? It's not ok to just import it as it's a driver, right? | |
| 19:00:56 | mriedem | sdague: you might have input here https://bugs.launchpad.net/nova/+bug/1734698 | |
| 19:00:56 | openstack | Launchpad bug 1734698 in OpenStack Compute (nova) "Squash database patches" [Undecided,Invalid] | |
| 19:01:33 | melwitt | mriedem: er, sorry, patch 2 adds an impl, patch 3 does something else. let me look deeper into this first | |
| 19:01:38 | openstackgerrit | Merged openstack/nova master: [placement] POST /allocations to set allocations for >1 consumers https://review.openstack.org/500073 | |
| 19:01:45 | openstackgerrit | Merged openstack/nova master: [placement] Fix GET PUT /allocations nits https://review.openstack.org/523401 | |
| 19:03:17 | melwitt | zigo: where do you want to import it? just thinking ahead on whether it might need to be moved to utils to do what you want to do | |
| 19:05:00 | melwitt | because we don't want to import any driver stuff into imagebackend or images | |
| 19:05:04 | efried | jaypipes Here's a scenario: One or more sharing RPs (MISC_SHARES_VIA_AGGREGATE) in an aggregate. But that aggregate does *not* contain any non-sharing RPs. Would it be correct to say that we should *never* get allocation candidates including any RP in that aggregate? | |
| 19:08:20 | zigo | melwitt: In nova/virt/images.py there's def _convert_image(source, dest, in_format, out_format, run_as_root):. There, there is cmd = ('qemu-img', 'convert', '-t', 'none', '-O', out_format). | |
| 19:08:34 | zigo | 'none' needs to be replaced by a call to that function. | |
| 19:09:08 | openstackgerrit | Merged openstack/nova master: [placement] Clean up TODOs in allocations.yaml gabbit https://review.openstack.org/513057 | |
| 19:09:17 | openstackgerrit | Merged openstack/nova master: Update the documentation links https://review.openstack.org/523288 | |
| 19:10:19 | jaypipes | efried: wouldn't it depend on the request? I mean, if the request is only for resources that are being shared by those providers, then why wouldn't those providers be returned in allocation candidates? (note: the scheduler would throw those providers away since they wouldn't match a compute node UUID, but that's not the point, right?) | |
| 19:11:03 | efried | jaypipes That's indeed not the point. Hum, I guess it's legit... | |
| 19:12:37 | melwitt | zigo: ah, gotcha. hmm ... yeah, so virt/images.py is not *supposed* to be libvirt-specific but it obviously is. and as you can tell from the layout, we've got separation between the driver and image related code. so we wouldn't want to import from the libvirt driver there. so I'm thinking what's the least ugly way we could do this ... | |
| 19:12:45 | efried | jaypipes Is _get_all_with_shared supposed to return only non-sharing RPs? | |
| 19:13:09 | jaypipes | efried: it was supposed to, yeah. but alex_xu (and you, right?) pointed out that wasn't actually the case. | |
| 19:13:33 | jaypipes | efried: and upon further thought, decided to leave it as it was, returning both sharing and non-sharing. | |
| 19:13:55 | jaypipes | efried: and leave it up to callers to choose to ignore sharing-only allocation requests. | |
| 19:15:24 | efried | jaypipes Hell, I don't know anymore. | |
| 19:15:57 | mriedem | dansmith: this is a fun bug https://bugs.launchpad.net/nova/+bug/1734504 | |
| 19:15:57 | openstack | Launchpad bug 1734504 in OpenStack Compute (nova) "User can't know which flavor used for resize by the result of "nova migration-list"" [Low,Triaged] | |
| 19:16:07 | dansmith | hmm, doesn't look fun | |
| 19:16:07 | mriedem | apparently we leak internal flavor primary keys out of the os-migrations REST API | |
| 19:16:26 | mriedem | which, whatever, we leak them out, big whoop, but they have no meaning at all to an end user since they aren't the flavorid | |
| 19:16:42 | mriedem | good times | |
| 19:17:03 | mriedem | and the columns on the migrations table that stores those ids is an integer column so we can't store string flavorid in there anyway :) | |
| 19:17:11 | efried | jaypipes Is there any meaning to MISC_SHARES_VIA_AGGREGATE if that's the case? | |
| 19:17:32 | efried | jaypipes What semantic does it provide beyond just being associated with a given aggregate? | |
| 19:18:36 | jaypipes | efried: it says "I share my inventory with any provider in any aggregate I'm associated with" | |
| 19:18:59 | mriedem | as far as i can tell, there is no reason that we even store the flavor.id on the migration record *except* to return it out of the API | |
| 19:19:03 | mriedem | nothing else in the code depends on it | |
| 19:19:22 | dansmith | "no reason to store it other than to return the wrong thing out of the API" <-- FTFY | |
| 19:19:29 | mriedem | yes correct | |
| 19:19:33 | dansmith | \o/ | |
| 19:20:17 | melwitt | zigo: okay, after looking through the code, what I'll guess is a decent way to try first is, move the static _supports_direct_io function into nova/utils.py and make it public, then call that in both the libvirt driver and virt/images.py to find out how to set cachemode | |
| 19:20:45 | efried | jaypipes Are we allowed to have more than one non-MISC_SHARES_VIA_AGGREGATE in a given aggregate? I mean, nothing stops us from doing that; but what does it mean? | |
| 19:20:46 | melwitt | because AFAICT, the test for direct io support is just a linux thing, not specific to libvirt | |
| 19:22:32 | jaypipes | efried: aggregates don't have traits. only providers have traits. | |
| 19:22:42 | jaypipes | efried: aggregates are simply groups of providers, nothing more. | |
| 19:23:10 | efried | jaypipes I understand that. Rephrase: Are we allowed to have more than one RP without the MISC_SHARES_VIA_AGGREGATE trait in a given aggregate? I mean, nothing stops us from doing that; but what does it mean? | |
| 19:23:13 | jaypipes | efried: are you asking whether it's allowed to have >1 provider sharing the same resource class to other providers in its aggregates? | |
| 19:23:20 | efried | no | |
| 19:23:43 | efried | jaypipes In today's terms: Am I allowed to have more than one "compute node" in the same aggregate? | |
| 19:23:46 | jaypipes | efried: *most* providers in an aggregate will *not* have the MISC_SHARES_VIA_AGGREGATE trait. | |
| 19:24:01 | jaypipes | efried: for instance, compute node providers won't (typically) have that trait. | |
| 19:24:10 | mriedem | how bad would an alter table be on the migrations table to change old/new_instance_type from an integer to a varchar(64)? | |
| 19:24:18 | jaypipes | efried: yes, it's totally expected to have >1 compute node in an aggregate | |
| 19:24:22 | mriedem | with say 100k entries | |
| 19:24:31 | jaypipes | mriedem: less than a couple seconds. | |
| 19:24:46 | mriedem | and, | |
| 19:24:48 | dansmith | we should just add the column, not change it | |
| 19:24:56 | dansmith | per normal and then backfill the data | |
| 19:25:02 | mriedem | yeah that was the other option i was thinking of | |
| 19:25:23 | mriedem | i don't know that we could even reliably backfill it | |
| 19:25:26 | dansmith | there's no reason to change it unless we're going to rewrite the contents and that would be ungoodly | |
| 19:25:39 | dansmith | likely not, and it'd require details from the api database | |
| 19:25:48 | mriedem | yup, and, flavor 1 might no longer exist | |
| 19:26:09 | dansmith | is it majorly problematic that the user can't see what the resize actually was? | |
| 19:26:19 | dansmith | I mean, it's the migrations api, mostly about moves anyway right? | |
| 19:26:39 | mriedem | it's all about moves | |
| 19:26:42 | mriedem | admin-only by default | |
| 19:27:51 | mriedem | if we added a new column, the api could just check that first, and if not set (old record), we fallback to the existing broken field | |
| 19:28:12 | dansmith | and that's just empty for non-resize moves? | |
| 19:28:53 | mriedem | no, | |
| 19:28:57 | mriedem | it's just equal for everything else | |
| 19:28:57 | mriedem | :) | |
| 19:29:14 | openstackgerrit | Ian Wienand proposed openstack/nova stable/newton: [DNM] Testing d-g automatic -eol tag detection https://review.openstack.org/523509 | |
| 19:29:15 | mriedem | https://github.com/openstack/nova/blob/5b5b5c8df316e4c26b853c80ae8f8ea91f9c05c0/nova/conductor/tasks/migrate.py#L180-L181 | |
| 19:29:25 | dansmith | I just don't see the point I guess | |
| 19:29:51 | mriedem | sure, but, | |
| 19:29:58 | mriedem | it's also dumb | |
| 19:30:02 | dansmith | flavorid doesn't really get you the info you need either | |
| 19:30:08 | mriedem | not if the flavor is deleted | |
| 19:30:11 | mriedem | i agree | |
| 19:30:13 | dansmith | so if you want to do this, we should go all overkill and store the actual flavors | |
| 19:30:18 | dansmith | really fatten out our database | |
| 19:30:19 | mriedem | so we could microversion the fields out of the response | |
| 19:30:38 | mriedem | database, singular? | |
| 19:30:43 | dansmith | yuup | |
| 19:30:44 | dansmith | and/or always return zero there | |
| 19:30:51 | dansmith | heh | |
| 19:34:31 | efried | jaypipes Am I missing something or are these identical? https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L1074-L1091 | |
| 19:34:42 | efried | (other than the name of the alias) | |
| 19:35:05 | zigo | melwitt: Ok, thanks. | |
| 19:35:27 | melwitt | zigo: let me know if you have any questions or if something wasn't clear | |
| 19:35:30 | zigo | I'll do that later on (when my kids sleep...:) | |
| 19:35:39 | melwitt | heh, k | |
| 19:37:15 | jaypipes | efried: nope, you're not missing anything. for explanation of why that's needed, see the comments above about "butterfly join" | |
| 19:37:40 | efried | jaypipes Yeah, totally confused by all of that at the moment. | |
| 19:37:54 | jaypipes | efried: https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L890-L949 | |
| 19:38:47 | efried | jaypipes Is there some reason we need two copies of that dict? | |