Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-03-04
15:41:38 lyarwood stephenfin: https://review.opendev.org/c/openstack/nova/+/673790 - any plans to respond to this -1 btw, I'm ready to go through the rest of the series once we've sorted this out.
15:42:07 stephenfin lyarwood: yup, I was going to do a separate FUP
15:42:12 stephenfin to avoid rebasing the whole series
15:43:27 lyarwood stephenfin: yup fair, I'll let you update the change before I continue on in the series
15:43:39 lyarwood and by that I mean comment, not rebase or anything
15:45:41 stephenfin kashyap: comments left on https://review.opendev.org/c/openstack/nova/+/774240. If you can respin I'll re-review today
15:47:49 kashyap stephenfin: Thanks for the review. Let me look ...
15:49:15 kashyap stephenfin: That mock of _register_instance_machine_type is required after Lee's change
15:49:27 stephenfin but you didn't touch that function?
15:49:36 kashyap stephenfin: Especially as the test is calling init_host() directly
15:50:10 stephenfin kashyap: Ah, whoops
15:50:15 lyarwood the diff has moved it around
15:50:20 stephenfin yup
15:50:24 stephenfin apologies
15:50:36 kashyap No problem
15:57:42 kashyap stephenfin: Is it palatable to you if I don't address the style nit here: https://review.opendev.org/c/openstack/nova/+/774240/11/nova/tests/unit/virt/libvirt/test_driver.py#1579
15:57:45 gibi nova meeting starts in 2 minutes in #openstack-meeting-3
15:57:48 kashyap stephenfin: I'm addressing the rest of all your comments
15:59:46 stephenfin sure
16:00:03 stephenfin tbc though, I'm only suggesting doing that for the new functions, not the old ones of course
16:00:19 openstackgerrit Stephen Finucane proposed openstack/nova master: trivial: Clarify purpose of 'Host.supports_*' properties https://review.opendev.org/c/openstack/nova/+/778739
16:00:33 stephenfin lyarwood: ^
16:00:38 stephenfin Lemme know if that's not clear
16:01:10 lyarwood stephenfin: thanks
16:06:30 kashyap stephenfin: Ah, okay; yes, just for the newly-added ones might as well address that
16:10:11 openstackgerrit Claudiu Belu proposed openstack/nova master: POC: tests: Adds test checking unbalanced NUMA node association https://review.opendev.org/c/openstack/nova/+/778740
16:13:20 lyarwood dansmith: sorry joined the meeting late, re the cinder failures, anything like https://bugs.launchpad.net/cinder/+bug/1917750 ?
16:13:23 openstack Launchpad bug 1917750 in Cinder "Running parallel iSCSI/LVM c-vol backends is causing random failures in CI" [Undecided,New]
16:14:12 dansmith lyarwood: depends on how that manifests, but can't say I've seen that specifically
16:14:38 dansmith the two major symptoms I see are a complaint about state conflict, and "unable to delete volume"
16:14:45 lyarwood right sorry, that basically leads to two instances looking at the same volume even when it isn't multiattached
16:15:04 dansmith ack
16:15:23 dansmith (and also.. ouch)
16:15:33 lyarwood that could be related if we are trying to detach the volume in Nova but it's still attached to another instance
16:16:15 dansmith well, I haven't seen detach fails, so much as failure to delete, but I guess it's possible it's just how we report it
16:16:45 lyarwood oh if it's the actual delete on the cinder side then it's likely something else
16:17:03 lyarwood we've already nuked the connections to the computes at that point
16:17:03 dansmith yeah I think it's like "delete volume, poll until it's gone...timeout"
16:17:34 lyarwood kk, melwitt had a bug for lvcreate being slow, assuming it's waiting on lvdelete it could be related
16:17:52 lyarwood lvchange*
16:17:56 lyarwood there's no lvdelete
16:18:06 dansmith ack
16:30:48 kashyap stephenfin: Isn't your "while" spurious, here, on line-8? (The "it. If niether..." bit makes sense, though.) -- https://review.opendev.org/c/openstack/nova/+/774240/11/releasenotes/notes/allow-disabling-cpu-flags-cc861a3bdfffadf8.yaml#8
16:31:52 kashyap It looks like so. I'll disregard it.
16:33:51 stephenfin kashyap: I think it's relevant
16:33:54 stephenfin This is possible via a '+' / '-' notation, where if you specify a CPU flag prefixed with a '+' sign (without quotes), it will be enabled for the guest, a prefix of '-' will disable it
16:34:11 stephenfin This is possible via a '+' / '-' notation, where if you specify a CPU flag prefixed with a '+' sign (without quotes) then it will be enabled for the guest while a prefix of '-' will disable it.
16:34:32 stephenfin The latter reads better to me
16:35:18 kashyap stephenfin: Ah, there; yes. But you're missing a comma after "guest" :)
16:35:44 kashyap Either that, or I've gone comma-wild (I was accused of this once, in a friendly way, on qemu-devel list before :D)
16:36:09 stephenfin Correct, missing comma
16:36:13 stephenfin since it's a comparison
16:36:27 stephenfin *Correct. Missing comma ;)
16:37:04 kashyap stephenfin: I.e. the missing comma in the second version is correct? Yeah?
16:37:18 stephenfin yeah, add the comma
16:37:35 kashyap Ah, nod.
16:38:21 lyarwood stephenfin: https://review.opendev.org/c/openstack/nova/+/769548 - just a reminder if you didn't have this on your list, that would then move the series into the gate.
16:38:38 claudiub Hello! I've noticed that in the NUMA-related docs (https://docs.openstack.org/nova/latest/admin/cpu-topologies.html#customizing-instance-numa-placement-policies) it says that "The NUMA node(s) used are normally chosen at random",
16:38:40 lyarwood oh and https://review.opendev.org/c/openstack/nova/+/778462/2 that gibi++ added in
16:38:42 claudiub but numa_fit_instance_to_host (https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L2235) says that it will return a new InstanceNUMATopology with its cell ids set to host cell ids of the first successful permutation, or None.
16:39:26 claudiub so, there's a mismatch there. From what I've seen, all the instances end up in the 1st NUMA node.
16:39:52 stephenfin lyarwood: was in the middle of reviewing. +2 on the whole series now
16:39:57 claudiub that could be problematic. Basically, I can have nodes with 50% consumed resources, but 1 NUMA node completely empty.
16:40:10 stephenfin claudiub: Yup, that's a known bug :(
16:40:21 claudiub so, I'm wondering which should be corrected: The docs, or the code.
16:40:29 stephenfin I think sean-k-mooney filed a bug for same
16:40:34 stephenfin claudiub: the code
16:40:40 claudiub ah, gotcha. :)
16:41:05 claudiub Wondering what the backport potential for this would be, if a fix would be added. :)
16:41:07 stephenfin we should shuffle the nodes or sort by least-allocated node
16:41:23 stephenfin depends on the implementation of course but definitely backportable IMO
16:41:30 stephenfin *should definitely be
16:42:08 claudiub is someone working on this? I could look into it if no one is
16:42:52 stephenfin Not right now. If you could look, I'd be happy to review
16:42:56 openstackgerrit Artom Lifshitz proposed openstack/nova master: pci: track host NUMA topology in stats https://review.opendev.org/c/openstack/nova/+/774149
16:42:58 openstackgerrit Artom Lifshitz proposed openstack/nova master: pci: implement the 'socket' NUMA affinity policy https://review.opendev.org/c/openstack/nova/+/772779
16:43:00 openstackgerrit Artom Lifshitz proposed openstack/nova master: pci: always pass node_id to manager https://review.opendev.org/c/openstack/nova/+/778747
16:43:08 lyarwood stephenfin: awesome thanks for that
16:43:25 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: libvirt: Allow disabling CPU flags via `cpu_model_extra_flags` https://review.opendev.org/c/openstack/nova/+/774240
16:43:34 claudiub great, will let you know how it goes. :)
16:44:05 kashyap stephenfin: gibi (lost your +2, if you'd like to add the stamp again) --^ Addressed the doc nits from Stephen
16:44:13 gibi sure
16:45:25 bauzas sean-k-mooney: could you please confirm that the os-vif SHA1 for the next release looks good to you ?
16:45:30 bauzas sean-k-mooney: https://review.opendev.org/c/openstack/releases/+/777955/1/deliverables/wallaby/os-vif.yaml
16:45:56 bauzas from what I see, yup
16:46:38 sean-k-mooney looking
16:49:19 sean-k-mooney yep its the current head of master and the main delta is just fixing the lower-constratists-job and a deprecation wraning
16:49:46 sean-k-mooney normlaly i would say that it could be a bugfix release but we always do feature version bump for the end of a cycle
16:49:49 sean-k-mooney so this looks correct
16:50:37 claudiub Also, speaking of NUMA, I was wondering if you know if an instance placed in a single NUMA node can be live-migrated to another node in a different NUMA node, or it has to be in the same NUMA node? Trying to gauge the severity of that bug that places all the instances in the same NUMA node.
16:51:00 sean-k-mooney claudiub: that can be done but only from train
16:51:23 sean-k-mooney claudiub: artom added numa live migration in the train release whre we can regenerate teh xml as part of the migration
16:51:31 claudiub live-migrating instances with numa topologies, right? I've seen that bit in code
16:52:04 artom claudiub, except the user can't specify which NUMA node - which I think is what you're getting at
16:52:04 sean-k-mooney what bug are you triaging?
16:52:10 artom Nova just finds a "free" one
16:52:52 claudiub artom: that's perfect then. :)
16:53:09 sean-k-mooney claudiub: please do not file a new bug for numa blanacing
16:53:41 claudiub sean-k-mooney: I was hitting an issue where all the created instances were created on the same numa node, so I was asking about that. :)
16:53:58 sean-k-mooney claudiub: yep that is by design

Earlier   Later