Earlier  
Posted Nick Remark
#openstack-nova - 2020-04-16
11:38:46 bauzas nothing really urgent afaics
11:39:06 bauzas any other reviews needed ? should I say
11:39:55 bauzas sean-k-mooney: hopefully, only a very few server actions miss allocations I think
11:40:04 bauzas and that being said...
11:40:07 gibi sean-k-mooney: support for move operations landed in Ussuri (tempest test is open)
11:40:20 bauzas gibi: sean-k-mooney: stephenfin: I'd indeed appreciate reviews of https://review.opendev.org/#/q/topic:bug/1778563+(status:open+OR+status:merged)
11:40:32 gibi sean-k-mooney: I don't think we need any extra support from the virt dirver side for qos
11:40:50 gibi bauzas: thanks for the triage.
11:41:00 gibi bauzas: and yes, I still have your patch series open
11:41:05 bauzas gibi: I'll continue tomorrow morning
11:41:09 gibi bauzas: thanks
11:41:32 bauzas but I think I'll open my review dashboard for bugs and see what to chime in
11:41:51 bauzas folks, a nova-core is looking for bugfixes to review, please hassle him <=
11:42:00 bauzas err,
11:42:06 bauzas <= please hassle him
11:43:02 gibi :)
11:50:01 lyarwood bauzas: do you also take trivial cleanups and fups?
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 bauzas https://bugs.launchpad.net/nova/+bug/1778563
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: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 stephenfin I'd have approved this already were it not for that
15:19:43 bauzas that's the issue i see ^
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?

Earlier   Later