| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-16 | |||
| 11:50:13 | lyarwood | bauzas: https://review.opendev.org/#/c/702021/ & https://review.opendev.org/#/c/711679/ for example | |
| 11:52:46 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: Avoid spurious error logging in _get_compute_nodes_in_db https://review.opendev.org/702902 | |
| 11:53:03 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: Reject boot request for unsupported images https://review.opendev.org/708577 | |
| 12:46:32 | bauzas | lyarwood: sorry, was at lunch, but indeed, everything can be accepted, I'm not the kind of person who tell you at the door 'sorry, but we can't accept you because of your shoes' | |
| 12:47:03 | stephenfin | lyarwood: those are dodgy shoes though | |
| 13:09:01 | lyarwood | stephenfin: white trainers ftw | |
| 13:11:21 | bauzas | lyarwood: get off my lawn ! | |
| 13:11:32 | bauzas | :p | |
| 13:12:44 | bauzas | heh, s/barf/bark of course | |
| 13:12:57 | bauzas | (pardon my French (c) ) | |
| 13:21:09 | kashyap | lyarwood: Zuul (grenade-py3) failure on this: https://review.opendev.org/#/c/702021/ | |
| 13:21:23 | kashyap | If you've already seen it, disregard me. | |
| 13:24:58 | lyarwood | kashyap: yeah unrelated | |
| 13:25:07 | lyarwood | thanks for looking | |
| 13:26:57 | kashyap | bauzas: FWIW, this is straightforward to just merge this: https://review.opendev.org/#/c/702021/ (libvirt: Remove VIR_DOMAIN [...]) | |
| 13:28:43 | kashyap | (And the 'qemu-img' one, too.) | |
| 14:47:49 | gibi | lyarwood, bauzas, stephenfin: Am I missing something here https://review.opendev.org/#/c/711679/5/nova/virt/images.py@42 ? | |
| 14:49:31 | stephenfin | Oh, it looks like it | |
| 14:50:02 | lyarwood | gibi: nope it's unused | |
| 14:50:25 | sean-k-mooney | well its used here https://review.opendev.org/#/c/711679/5/nova/virt/images.py@48 | |
| 14:50:37 | sean-k-mooney | but i dont know if its everset to anything other then json | |
| 14:50:54 | bauzas | gibi: lyarwood: wait, it can be | |
| 14:51:01 | bauzas | sec, finding the github link | |
| 14:51:49 | bauzas | gibi: lyarwood: stephenfin: https://github.com/openstack/nova/blob/e1359567e4985e9a671359d4c0d53404a8ba64ab/nova/virt/libvirt/utils.py#L224 | |
| 14:52:14 | stephenfin | bauzas: that's format, not output_format | |
| 14:53:35 | sean-k-mooney | looking at the reference on github the only place output_formate is ever set today | |
| 14:53:47 | sean-k-mooney | is https://github.com/openstack/nova/blob/e1359567e4985e9a671359d4c0d53404a8ba64ab/nova/tests/unit/virt/libvirt/test_utils.py#L167 | |
| 14:53:52 | sean-k-mooney | where it is currently set to json | |
| 14:54:04 | gibi | sean-k-mooney: at https://review.opendev.org/#/c/711679/5/nova/virt/images.py@48 we can hardcode the output_format to 'json' | |
| 14:54:13 | sean-k-mooney | gibi: yes | |
| 14:54:23 | sean-k-mooney | and remove the parmanter form our method | |
| 14:54:25 | stephenfin | burn it with fire | |
| 14:54:31 | lyarwood | there's actually a place in the driver as well | |
| 14:54:34 | lyarwood | but we can drop it from there | |
| 14:54:36 | lyarwood | 1 sec | |
| 14:55:58 | sean-k-mooney | so useing codesearch http://codesearch.openstack.org/?q=qemu_img_info&i=nope&files=&repos=openstack/nova | |
| 14:57:30 | sean-k-mooney | its either not set or set to json our side of the funtion definitions | |
| 14:58:11 | sean-k-mooney | since you change the defualt to json in the patch any fucntion that did not set it already works with json | |
| 14:58:23 | sean-k-mooney | so ya i think we can just hardcode it and drop it | |
| 14:58:45 | sean-k-mooney | or at least push a patch to do that and see what breaks | |
| 14:59:19 | stephenfin | bauzas: comments left on https://review.opendev.org/#/c/712741/ | |
| 14:59:31 | stephenfin | Looks like they're mostly the same as sean-k-mooney's (de-duping stuff) | |
| 14:59:36 | bauzas | stephenfin: ack, looking | |
| 15:00:30 | stephenfin | dansmith, gibi, melwitt: Thoughts on merging https://review.opendev.org/#/c/589085/ ? | |
| 15:01:32 | stephenfin | given that it touches the driver API. Is it still okay to land after feature freeze? | |
| 15:06:16 | sean-k-mooney | ya i had that question too but the notifcation was also already sent to the mailing list of the change about a month ago | |
| 15:07:50 | gibi | we need to figure out how risky is this change. what can we break? | |
| 15:08:20 | dansmith | what's the point of merging that, if not just to enable a feature? | |
| 15:08:48 | dansmith | IMHO, unless it gets us something by landing it in U to set up for fewer migration problems in V, it's really not worth doing at this point | |
| 15:09:24 | dansmith | meaning, if this got us a cycle earlier on data migration or something, but I don't see that unless I'm missing something | |
| 15:13:32 | bauzas | gibi: dansmith: sean-k-mooney: stephenfin: I wrote a ML thread 1 month ago about this and honestly it's an internal API | |
| 15:13:47 | sean-k-mooney | this is required for resize its not related to the multi gpu types so it has no impact on upgrde or data migrations | |
| 15:13:53 | bauzas | and there is no object impact or RPC call involved in any matter | |
| 15:14:03 | dansmith | sean-k-mooney: right | |
| 15:14:40 | dansmith | sean-k-mooney: that's my point.. if we're not going to merge the feature it enables, and it doesn't set us up for something early, then merging it now vs. as soon as V opens doesn't matter, except for risk and change yeah? | |
| 15:15:36 | bauzas | there is no feature behind it | |
| 15:15:52 | bauzas | we already support instances resizes | |
| 15:15:53 | dansmith | bauzas: oh is it a bug fix? | |
| 15:15:58 | bauzas | yup | |
| 15:16:14 | bauzas | I wouldn't beg for a FFE | |
| 15:16:16 | bauzas | if so | |
| 15:16:18 | openstack | Launchpad bug 1778563 in OpenStack Compute (nova) "Resize/Cold-migrate doesn't recreate vGPUs" [High,In progress] - Assigned to Sylvain Bauza (sylvain-bauza) | |
| 15:16:18 | bauzas | https://bugs.launchpad.net/nova/+bug/1778563 | |
| 15:16:37 | bauzas | we just blindly resize the instance with forgetting the vgpus | |
| 15:16:52 | dansmith | ah, sorry I figured this was for new stuff | |
| 15:16:53 | bauzas | so, 'whoops, sorry' basically | |
| 15:17:15 | bauzas | adding a new call to _allocate_mdevs() will just return if no VGPUs involvedf | |
| 15:17:20 | bauzas | that's basically a noop | |
| 15:17:27 | bauzas | (for instances with no VGPUs in there) | |
| 15:17:31 | dansmith | is this new in U or latent since T? | |
| 15:17:38 | bauzas | latent since Queens :) | |
| 15:18:00 | bauzas | https://review.opendev.org/#/c/720250/1/doc/source/admin/virtual-gpu.rst | |
| 15:18:07 | dansmith | ah, then probably not worth rushing into U then | |
| 15:18:21 | dansmith | I haven't reviewed the stack so I'm not sure how much risk, granted | |
| 15:18:34 | dansmith | but if no object, rpc, or db changes then it could be backported even right? | |
| 15:19:27 | stephenfin | I don't think so. It has virt driver API changes, which means a backport would break out-of-tree drivers | |
| 15:19:43 | bauzas | that's the issue i see ^ | |
| 15:19:43 | stephenfin | I'd have approved this already were it not for that | |
| 15:20:00 | bauzas | I warned the out-of-tree driver maintainers that we were changing the resize signature | |
| 15:20:13 | stephenfin | this hunk, basically https://review.opendev.org/#/c/589085/8/nova/virt/driver.py | |
| 15:20:30 | dansmith | I'm not aware of ever blocking a backport on those grounds | |
| 15:20:39 | dansmith | not sure why that would matter at all, when it doesn't for master | |
| 15:20:44 | stephenfin | o rly? | |
| 15:20:47 | bauzas | I dunno, honestly | |
| 15:21:03 | bauzas | either way, if folks prefer to punt it to V, fair enough | |
| 15:21:12 | stephenfin | I figured people had notice on master and would expect to have to do some work with a new major version | |
| 15:21:15 | stephenfin | not so a new minor one | |
| 15:21:19 | bauzas | I just thought it wouldn't harm (c) :p | |
| 15:22:10 | artom | dansmith, not sure we ever blocked a backport, but there was a case where we made a point of *not* changing the driver interface to enable the backport | |
| 15:22:19 | artom | And then in a FUP re-did the thing the "correct" way | |
| 15:22:55 | artom | https://review.opendev.org/#/c/670645/2/nova/virt/libvirt/driver.py@9045 | |
| 15:23:04 | dansmith | artom: because it was doable without I guess? either way, I bet I wasn't the one worried about it :) | |
| 15:23:05 | bauzas | dansmith: stephenfin: gibi: shouldn't we just discuss this as a 'Stuck review' during the nova meeting in 48 mins ? | |
| 15:23:17 | bauzas | and find a consensus there so we move on | |
| 15:23:19 | artom | dansmith, it may have been mriedem, don't remember | |
| 15:23:28 | dansmith | sounds like him :) | |
| 15:23:35 | stephenfin | bauzas: um, sure, why not | |
| 15:24:01 | bauzas | again, that doesn't really change my business | |
| 15:24:15 | bauzas | if we go accepting this change, that's cool | |
| 15:24:18 | artom | (And here's the "correct" fup: https://review.opendev.org/#/c/668631/5 ) | |