Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-02-01
15:22:25 mriedem so https://review.openstack.org/#/c/538003/2/nova/virt/libvirt/driver.py@5036
15:23:08 hrw mriedem: and that's what I did in 5th patch
15:24:21 Spazmotic Hehe my commit went from working on someone elses with large overhaul.. to my possibility of a XCP version check.. to now just a single line to eat an exception.
15:24:29 Spazmotic The power of code review.
15:27:10 Spazmotic Speaking of code reduction, I resubmitted my commit with the changes you suggested bauzas, if you get a chance sometime this week to take a look. You're still on the review so no rush :)
15:27:21 Spazmotic Have a good night novaers.. time for me to hit the hay
15:28:49 hrw I do wonder how much time it will take when I submit patch which adds usb-keyboard ;D
15:29:42 mriedem mgoddard: johnthetubaguy: how much of this stuff is required for queens? https://review.openstack.org/#/q/topic:bug/1746209+(status:open+OR+status:merged)
15:29:48 mriedem i see the ironic driver traits patches are merged
15:29:51 hrw hm.
15:30:16 hrw mriedem: I think that I will have to rewrite that patch again anyway.
15:31:31 hrw mriedem: I just realized that as I need a keyboard in VM (usb one to make sure it works on all archs) I may need to add a check "if video requested and not x86 then add usb host controller and usb keyboard"
15:32:10 hrw mriedem: and then it does not really matter is it usbtablet or ps2mouse or other mambojambo
15:33:55 mgoddard_ mriedem: none of that is required for queens. https://review.openstack.org/539506 is a speculative bugfix, so might be nice to get that one in
15:36:05 jroll +1 to that
15:38:28 gibi edleafe: see my suggestion inline https://review.openstack.org/#/c/539323/1/nova/cmd/manage.py@115
15:39:13 jroll mriedem: agree with mgoddard_ there, though they all look fairly safe to me, I'd just land them in queens personally
15:41:20 kashyap mriedem: stephenfin: Back here. About the bumping mi libvirt / QEMU, we can work it out. Should also send out an email to operators list as a heads-up
15:42:11 stephenfin kashyap: I'm assuming we use min(version in Ubuntu 16.04, version in RHEL 7.4, etc.) ?
15:42:24 stephenfin Or is it little more nuanced than that? :)
15:43:11 hrw stephenfin: ubuntu 16.04 or canonical cloud archive for 16.04?
15:43:22 kashyap stephenfin: Not that straightforward :-)
15:43:46 kashyap stephenfin: But yes, the lowest common denominator across distros upstream cares about
15:43:57 hrw for aarch64 sensible version is 3.6.0 and nothing lower
15:44:08 mgoddard_ mriedem, jroll: while we're on ironic virt driver bugs, I think https://bugs.launchpad.net/nova/+bug/1710141 has crept back in. Should we target this for queens?
15:44:09 openstack Launchpad bug 1710141 in OpenStack Compute (nova) ocata "Continual warnings in n-cpu logs about being unable to delete inventory for an ironic node with an instance on it" [Undecided,New]
15:44:38 mriedem alex_xu: gibi: replied in https://review.openstack.org/#/c/539658/ - see what you think, it's nuanced for sure
15:45:11 edleafe gibi: sure, that would work. I considered doing a shift of 8 instead of 1 for the same reason, but I thought an explicit shift-unshift difference would be less likely to be confusing. IOW, a real entry is shifted for creating a marker, and unshifted to return to the original value
15:45:20 mriedem kashyap: stephenfin: there has already been a "next version" in the driver since pike, so we'd use that,
15:45:29 mriedem the question is what the next version would be after that
15:45:34 jroll mgoddard_: that looks complicated :|
15:45:44 mriedem which is likely whatever our min is for supported distros today
15:45:55 stephenfin mriedem: Yup, that's the version I'm referring to
15:45:57 kashyap mriedem: Ah, right. We're looking for the one _after_
15:46:23 mgoddard_ jroll: I think we just need to report inventory for nodes with instances
15:47:00 jroll mgoddard_: we should be already, is the thing
15:47:15 mgoddard_ jroll: nope https://github.com/openstack/nova/blob/d25feca/nova/virt/ironic/driver.py#L758
15:47:33 jroll mgoddard_: right, tracking it down, because that's the real bug like you say
15:47:58 mgoddard_ jroll: I think we didn't then briefly we did, then we didn't again :)
15:48:34 jroll mgoddard_: yeah we broke that. sigh.
15:48:51 mgoddard_ I think this fixed it briefly: https://github.com/openstack/nova/commit/c92337bdf80fea4c0a8ebb433bacec4cc07f7a94
15:49:12 mgoddard_ jroll: then this broke it again: https://github.com/openstack/nova/commit/d25feca90ec4bad6ec9ececedced63b9f00b4c87
15:49:27 gibi edleafe: I don't know why shifting by 1 is more explict that shifting by 8. In the other hand you can keep shift and un_shift function so that the client code will be explict but simply calls shift from un_shift to make the implementation simpler
15:49:59 mgoddard_ jroll: there's also the matter of this TODO: https://github.com/openstack/nova/blob/d25feca/nova/virt/ironic/driver.py#L753
15:53:28 edleafe gibi: the explicitness is that as written, shift(shift(uuid)) will not return the original value. IOW, you have to be explicit that you are encoding/decoding
15:54:48 jroll mgoddard_: hm, I'm not sure the best way to handle this, though completing that todo may just solve it forever :)
15:55:20 mgoddard_ jroll: +1, but seems late in the cycle for that change
15:56:11 jroll mgoddard_: agree, though if it's just spurious logs, I think it's fine to wait
15:57:05 gibi mriedem: see my answer in https://review.openstack.org/#/c/539658/1/doc/source/user/placement.rst@279
15:58:42 gibi edleafe: this is why I suggest to keep shift and un_shift for the caller. So on the caller side it is explicitly encode/decode the marker. But the implementation can be like un_shift(uuid): return shift(uuid) and shift(uuid) can also be written in a single line
15:59:52 gibi mriedem: I would go for less options to avoid some later confusions
16:00:06 openstackgerrit Stephen Finucane proposed openstack/nova-specs master: trivial: Resolve Python 3 issues https://review.openstack.org/539907
16:00:46 edleafe gibi: ok, that's fair. Not sure why a one-line function is desireable, though
16:00:55 openstackgerrit Matt Riedemann proposed openstack/nova master: Pass limit to /allocation_requests https://review.openstack.org/531517
16:02:18 mgoddard_ jroll: yeah I guess so
16:05:16 gibi edleafe: it is not the one linenes that really matters, but the simple reverse has a smaller cyclomatic complexity as well
16:07:30 openstackgerrit Eric Fried proposed openstack/nova master: Ensure resource classes correctly https://review.openstack.org/539738
16:07:42 melwitt mriedem: ack, will review
16:08:01 efried jaypipes: The above fixes a bug I *do* think we need resolved in Q.
16:08:18 efried edleafe, cdent: may also interest y'all.
16:09:00 edleafe efried: ack. In meeting hell today.
16:09:09 efried no worries
16:12:55 openstackgerrit Eric Fried proposed openstack/nova master: Avoid inventory DELETE API (no conflict detection) https://review.openstack.org/539712
16:19:18 mriedem stephenfin: you know zuulv3 things, care to take a look at the nova-multiattach job? https://review.openstack.org/#/c/532689/
16:19:24 mriedem dependent patches are approved
16:19:45 stephenfin Sure thing
16:19:56 mriedem thanks; will be good to not regress that stuff
16:36:53 mriedem ameeda: easy docs bug to fix in cinder https://bugs.launchpad.net/cinder/+bug/1711267
16:36:55 openstack Launchpad bug 1711267 in Cinder "Boot from volume in cinder" [Undecided,New]
16:36:58 mriedem has some broken links
16:51:41 stephenfin mriedem: Comments left
16:59:09 mriedem stephenfin: replied; i'm not sure i follow your confusion though
17:00:22 openstackgerrit Marcin Juszkiewicz proposed openstack/nova master: Make sure that we have usable input for graphical console https://review.openstack.org/538003
17:01:25 hrw mriedem, stephenfin: rewroted. now it adds usb host controller if there is none and adds usb keyboard if there is no keyboard. and does it on !(x86(-64), ppc64, s390x) architectures
17:02:49 jaypipes efried: IBM PowerKVM CI failure. I'll wait until that is resolved.
17:03:25 efried jaypipes: PowerKVM? Non-voting, right? I've been steadfastly ignoring that guy.
17:03:40 jaypipes efried: I was joking with you.
17:03:50 efried jaypipes: PowerVM failures are due to the fact that esberglu is rebuilding some CI systems, I think.
17:03:53 jaypipes efried: clearly, you do not share my brand of humour.
17:04:25 hrw jaypipes: https://review.openstack.org/538003 ^^ ;D
17:04:33 efried jaypipes: Sorry, the gate has me grumpy about CI holding up patches.
17:05:03 efried jaypipes: Just for that, I'm going to put you down as the approver for this blueprint I'm writing up.
17:05:22 mmedvede PowerKVM is voting, but it is not blocking, i.e. it can not prevent a patch with +2+w from merging
17:05:43 esberglu efried: Huh? I'm not doing anything with prod CI, just staging
17:05:55 esberglu Oh this is PowerKVM we're talking
17:06:32 efried esberglu: Okay, so I should be able to recheck powervm failures?
17:06:55 efried esberglu: Seeing OOT failures pretty frequently.
17:07:04 openstackgerrit Stephen Finucane proposed openstack/nova-specs master: trivial: Resolve Python 3 issues https://review.openstack.org/539907
17:07:23 jaypipes hrw: I thought mriedem was a no go on that getting in Queens?
17:07:34 jaypipes efried: :P
17:08:23 hrw jaypipes: how that holds you from reviewing?
17:08:46 esberglu efried: Yeah recheck. Only 1 issue hitting OOT, was gonna have someone from REST take a look this afternoon
17:08:55 efried ight
17:09:25 hrw jaypipes: for me it may land in Rocky cycle as well as in Queens. I just have spare time now as what I wanted to have in nova/Queens got merged.
17:09:44 mriedem the usbhost controller for aarch64 is a bug
17:09:53 mriedem working around a limitation in libvirt for non-x86
17:09:54 hrw jaypipes: just prefer to have it reviewed when I still remember what is it all about
17:09:54 mriedem so that's fine
17:09:56 efried stephenfin: I think if we get up to PS4, you have to remove "trivial".
17:10:11 stephenfin Hahaha

Earlier   Later