| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-10 | |||
| 08:44:21 | openstackgerrit | Boxiang Zhu proposed openstack/nova-specs master: Add host and hypervisor_hostname flag to create server https://review.openstack.org/645458 | |
| 08:58:07 | alex_xu | sean-k-mooney: thanks! | |
| 08:58:26 | alex_xu | sean-k-mooney: I mean all of them, they are good comments :) | |
| 09:02:16 | alex_xu | sean-k-mooney: I saw you like the extra spec defined in https://review.openstack.org/#/c/651024/5/specs/train/approved/rmd-plugin-energy-efficiency-core-p-states-control.rst@310, but I'm not quite understand, it will be great you have an example for it, if it is good, I'm happy to change also | |
| 09:02:29 | alex_xu | for now, I need to offline for catching my daughter | |
| 09:02:44 | sean-k-mooney | ok o/ | |
| 09:02:51 | alex_xu | thanks! | |
| 09:03:02 | sean-k-mooney | and ya i like it more as its a finite set | |
| 09:03:18 | sean-k-mooney | using arbiarty traits makes me a little uneasy | |
| 09:03:26 | alex_xu | ah, i see now | |
| 09:03:38 | sean-k-mooney | it could be fine just need to get used to the idea | |
| 09:04:13 | alex_xu | but I don't want to assume there are only two priority(high and low), the user may have their own configuration, they can have some custom tratis | |
| 09:04:43 | alex_xu | anyway, I have to go, catch up you later | |
| 09:05:05 | sean-k-mooney | they may but that is an interoperablity headache between clouds. sure dont be late for dad taxi service :) | |
| 09:46:55 | kashyap | alex_xu: sfinucan: After some more thinking last night, I'm updating that `cpu_model_list` spec a bit more | |
| 09:47:03 | kashyap | Err, stephenfin ^ :-) | |
| 09:47:29 | kashyap | stephenfin: On the name, I'm responding on the change...but quickly here: I didn't even think of the "Hungarian Notation" until you mentioned :-0 | |
| 09:47:35 | kashyap | But I see your point, though. | |
| 09:48:04 | kashyap | IMHO, it's just unambiguously clear to go with the _list; instead of playing with alises and such. | |
| 09:48:13 | stephenfin | kashyap: Yeah, I really don't think it's necessary. oslo.config is powerful enough to let us handle the migration issues, as I noted | |
| 09:48:33 | kashyap | I've seen far too many such subtle and confusing mistakes in config attributes in the past. :-( | |
| 09:48:45 | stephenfin | Perhaps, but you're going to have to use aliases anyway, assuming you want to remove the current value at some point in the future | |
| 09:49:00 | sean-k-mooney | kashyap: useing _list may be clear but it breaks convention | |
| 09:49:07 | kashyap | I see. It's not merely the technical. But I'm still not convinced from the human / operator "visual clarity" | |
| 09:49:24 | kashyap | sean-k-mooney: What convention? We should break "conventions" when it makes sense. | |
| 09:49:40 | stephenfin | I think that matters less than you think. People don't set nova.conf files, tools do | |
| 09:49:52 | sean-k-mooney | the conventions of we just use the plural form to denote lists in the config | |
| 09:49:59 | stephenfin | Be that TripleO, Kolla, or something else | |
| 09:50:24 | kashyap | Yeah, realize it as much. I want to wait a bit more to convince myself | |
| 09:50:41 | sean-k-mooney | kashyap: im not arguing _list is wronge im just saying i dont think its that important if its _list or plural | |
| 09:51:10 | kashyap | (I'm not convinced. I'll think a bit more and write on the review.) But there's more important bits in that spec to tackle, though | |
| 09:51:24 | stephenfin | sean-k-mooney: If I resize an instance, does that instance move host? | |
| 09:51:25 | kashyap | (The more I think about this topic, the more subtle questions I can think of...) | |
| 09:51:35 | stephenfin | s/does/can/ | |
| 09:51:43 | sean-k-mooney | not in all cases but it is allowed too move | |
| 09:52:20 | sean-k-mooney | rebuilds are always to the same host but resize default to migration but you can enable resize to same host in the config in which case we dont add the current host to the ingroed host list | |
| 09:58:16 | sean-k-mooney | stephenfin: does that make sesnse ^ | |
| 09:58:22 | stephenfin | sean-k-mooney: It sure does | |
| 11:00:24 | yaawang | bauzas: stephenfin Thanks for your review. I agree with your comments about not expose driver's feature to the REST api. So how about putting these into the flavor extra flags and image metadata? Use these properties to datermine whether to use 'auto converge' or 'post copy' during live migration. | |
| 11:22:54 | trident | Are there any specific reasons why block migration is not implemented with the LVM backend? Any specific issues with getting it to work or just that nobody has implemented support for it? | |
| 11:47:18 | kashyap | stephenfin: Idea: Is this any palatable: `acceptable_cpu_models`? (It closely captures the intended meaning _and_ we can avoid "Hungarian") | |
| 11:47:51 | kashyap | (I am wary of bike-shedding, while knowing that "naming is hard", so want to avoid mistakes when I can. :-)) | |
| 11:48:52 | kashyap | (Context to others: By "Hungarian", /me was referring to the "Hungarian Notation" -- https://en.wikipedia.org/wiki/Hungarian_notation) | |
| 11:58:54 | rabel | hi there. if a hypervisor has a negative disk_available_least value, but free space on the disk (local storage, qcow2 images). and now it does not allow live migrations to this hypervisor. is it sufficient to increase disk_allocation_ratio to make this hv available for live migration again? | |
| 11:59:15 | kashyap | Maybe `acceptable_cpu_models` is not acceptable; it breaks the "symmetry" of existing config, options which all start with: cpu_{mode, model|models|model_list, extra_flags} | |
| 12:16:54 | sean-k-mooney | kashyap: enabled_cpu_models would be similar to enabled_filters for the schduler | |
| 12:17:53 | sean-k-mooney | but also i think both are verbose and we can use something shorter | |
| 12:18:42 | kashyap | A little verbose is OK, so long as it conveys intended meaning. I'll stick to `cpu_model_list` for now. It is just unambiguous. | |
| 12:22:30 | sean-k-mooney | kashyap: its also a chose that several people dislike but as i said it dotn care enough to rathole on it | |
| 12:36:16 | rabel | nvm my question. it seems we were hitting https://bugs.launchpad.net/nova/+bug/1708572 | |
| 12:36:17 | openstack | Launchpad bug 1708572 in OpenStack Compute (nova) "Unable to live-migrate : Disk of instance is too large" [High,Fix released] - Assigned to Matthew Booth (mbooth-9) | |
| 12:50:24 | alex_xu | kashyap: in the initial spec discussion, we said we can't done the check without hypervisour-literate api https://review.openstack.org/#/c/620959/5..9/specs/stein/approved/cpu-model-selection.rst@63 | |
| 12:50:42 | alex_xu | let me what we changed now :) | |
| 13:05:05 | kashyap | alex_xu: Being dragged into a meeting; will respond there | |
| 13:11:22 | alex_xu | kashyap: no hurry | |
| 13:28:22 | sean-k-mooney | stephenfin: i think https://review.openstack.org/#/c/648123/4 looks good now | |
| 13:28:57 | sean-k-mooney | alex_xu: efried care to take a look ^ i think this is a fairly simple fix | |
| 13:30:09 | efried | sean-k-mooney: Wow, that has more words in the title that I don't understand than that I do. | |
| 13:30:51 | efried | sean-k-mooney: If you get desperate I'll take a look, but you should find someone more familiar with the subject matter. | |
| 13:31:34 | sean-k-mooney | efried: no worries | |
| 13:31:45 | alex_xu | sean-k-mooney: I can do tomorrow, I don't think I have clear mind now | |
| 13:31:59 | sean-k-mooney | alex_xu: specs have that effect | |
| 13:32:11 | alex_xu | hah :) | |
| 13:38:48 | alex_xu | that is really bad to see jaypipes's strong -1 just after decide end the day :) | |
| 13:40:18 | alex_xu | anyway, I will leave it to tomorrow | |
| 13:51:15 | artom | sean-k-mooney, you OK with the tests refactor being in the same patch as the actual fix? | |
| 13:51:26 | artom | I'm kinda leaning towards splitting that in two patches? | |
| 13:57:54 | sean-k-mooney | artom: are you refering to https://review.openstack.org/#/c/648123/4 | |
| 13:58:05 | artom | sean-k-mooney, yeah. Just left a revirw | |
| 13:58:07 | artom | *review | |
| 13:58:27 | sean-k-mooney | artom: i did ask myself that question and the anser was "meh" ill see if anyone else cares | |
| 13:58:51 | sean-k-mooney | for backporting would likely be simpler to have them as seperate patches i gues | |
| 13:59:53 | sean-k-mooney | because the test refactor was related to the change i was ok with it but normally i would have prefered them to be split | |
| 14:01:06 | artom | sean-k-mooney, well I have a -1 for other small nits, we'll let others make that call for us :D | |
| 14:01:17 | artom | Since we both appear to be "meh" :) | |
| 14:02:42 | sean-k-mooney | artom: on the api question there are times where this works today and it ends up in the api with the mac but not stats | |
| 14:03:00 | sean-k-mooney | artom: so i dont think its an api change | |
| 14:03:26 | artom | sean-k-mooney, that's what I had understood as well | |
| 14:03:38 | artom | And the test does assert one of the fields is None, so we're probably covered | |
| 14:04:36 | sean-k-mooney | oh i didnt put https://bugzilla.redhat.com/show_bug.cgi?id=1649688#c8 into the upstream bug | |
| 14:04:37 | openstack | bugzilla.redhat.com bug 1649688 in openstack-nova "[osp13] nova diagnostics command is not working with all instances" [High,Assigned] - Assigned to fpalin | |
| 14:05:38 | sean-k-mooney | actully there might be a slight api change in that before nic_details was empty and no i has a mac | |
| 14:06:00 | sean-k-mooney | but i dont think that is versioned but good question | |
| 14:07:43 | artom | I don't think it'd have been empty before, rather an error 50 | |
| 14:07:45 | artom | 500 | |
| 14:08:13 | sean-k-mooney | no it was empty when i tried to reporduce the bug on my local systlem | |
| 14:08:58 | sean-k-mooney | if the vf has a netdev before its plug libvirt added the target element and we did not raise an error but you had no data at all | |
| 14:09:00 | artom | Hrmm, in any case, None is still allowed in fields | |
| 14:09:32 | artom | So nic_details = None is valid, as is nic_detail = {mac: blah, rest of fields: None} | |
| 14:13:14 | sean-k-mooney | well we dont have any tests assering the api behavior so i think there is some flexibity here. we can either say its fine as it is and this was a bug that was fixed and there does not need a microverion or there is no testing therfor it was broke by default and this fixed it :) | |
| 14:13:52 | sean-k-mooney | but i dont think its being to checky to say it does not need a microverions | |
| 14:13:52 | artom | Well we have an api-ref and a when-to-microversion flow chart | |
| 14:14:09 | artom | And fixing 500 errors is NOT a new microversion | |
| 14:14:14 | sean-k-mooney | we do | |
| 14:14:18 | sean-k-mooney | correct | |
| 14:14:22 | artom | I was just concerned the fields would be *missing* outright | |
| 14:14:57 | artom | Which is moot, because it doesn't appear to be the case, but would have required a new microversion, or more likely an updated patch that didn't have that problem | |
| 14:15:38 | sean-k-mooney | yep well this is why we have code review is a valid concern i think we are ok however. | |
| 14:24:27 | mriedem | boxiang: just a few small things to address in your spec here https://review.openstack.org/#/c/645458/ | |
| 14:24:51 | mriedem | but i know it's late so feel free to address those comments tomorrow | |