| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-28 | |||
| 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? | |
| 19:39:00 | jaypipes | efried: I'd be happy to walk you through it in a hangout if you need. | |
| 19:39:14 | jaypipes | efried: it's two separate sets of aliased tables. not the same dict. | |
| 19:39:21 | efried | jaypipes Hum, yeah. | |
| 19:39:45 | efried | jaypipes A walkthrough would be great, though I don't think my brain can handle it now. I'll take you up on it at some point when I'm feeling smarter. | |
| 19:39:58 | jaypipes | efried: np, happy to do that. | |
| 19:40:03 | efried | thanks | |
| 19:40:58 | jaypipes | efried: sharing providers are the bane of my existence. | |
| 19:41:20 | efried | Surely not the *only* bane. | |
| 19:41:33 | jaypipes | efried: heh | |
| 19:43:03 | jaypipes | efried: just completed the last of the test runs on n-r-p series. just in time to need to rebase and bump to 1.14 instead of 1.13... | |
| 19:43:20 | efried | jaypipes Fun day. | |
| 19:43:49 | edleafe | efried: if you ever need a sanity check on differences, try https://www.diffchecker.com | |
| 19:44:09 | edleafe | Really helpful when comparing expected vs. actual in tests | |
| 19:44:23 | efried | edleafe Nice, thanks! | |
| 19:44:42 | openstackgerrit | Merged openstack/nova master: Regenerate and pass configdrive when rebuild Ironic nodes https://review.openstack.org/503088 | |
| 19:49:31 | jaypipes | efried: actually... if you are looking for something to do... | |
| 19:49:55 | efried | jaypipes Tell me. I think I could just about handle a deep rebase right now. | |
| 19:50:01 | jaypipes | efried: if you wouldn't mind fixing those little issues you found on https://review.openstack.org/#/c/523192/, that would be super useful since I'm currently rebasing again the n-r-p series. | |
| 19:50:14 | efried | ight | |
| 19:50:20 | jaypipes | efried: since that's an important bug. | |
| 19:50:26 | efried | lemme propose this ksa release and I'll hit that... | |
| 19:50:27 | jaypipes | efried: and will make mriedem happy. | |
| 19:50:30 | jaypipes | ++ | |
| 19:50:33 | jaypipes | thanks much | |