Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-09-17
14:44:41 stephenfin elod, lyarwood, melwitt: Obviously the top few of these are still blocked by their Ussuri equivalents, but could you add https://review.opendev.org/#/q/topic:bug/1879878+status:open+branch:stable/train to your review queues?
14:46:23 lyarwood ack
14:51:47 elod stephenfin: sure, will do
14:53:56 stephenfin thanks
14:57:23 elod stephenfin: btw, can those heavy test refactors be avoided somehow?
14:58:15 stephenfin elod: Do you mean the first three patches I brought into the series or the changes I made to later patches?
14:59:52 elod stephenfin: for example this one: https://review.opendev.org/#/q/I33d8ac0a1cae0b2d275a21287d5e44c008a68122
15:00:17 openstack bug 1895696 in OpenStack Compute (nova) "unable to boot instance from encrypted volume created from a glance image of an encrypted volume" [High,In progress] https://launchpad.net/bugs/1895696 - Assigned to Lee Yarwood (lyarwood)
15:00:17 openstackgerrit Lee Yarwood proposed openstack/nova stable/ussuri: Add regression test for bug #1895696 https://review.opendev.org/752485
15:00:18 openstackgerrit Lee Yarwood proposed openstack/nova stable/ussuri: compute: Skip cinder_encryption_key_id check when booting from volume https://review.opendev.org/752486
15:01:02 openstackgerrit Lee Yarwood proposed openstack/nova stable/train: Add regression test for bug #1895696 https://review.opendev.org/752487
15:01:04 openstackgerrit Lee Yarwood proposed openstack/nova stable/train: compute: Skip cinder_encryption_key_id check when booting from volume https://review.opendev.org/752489
15:01:04 openstackgerrit Lee Yarwood proposed openstack/nova stable/train: Create volume attachment during boot from volume in compute https://review.opendev.org/752488
15:01:09 stephenfin elod: So the former. I could, but it would involve even more significant rewriting of the tests I'm bringing back plus significant rewrite of any future tests that depend on this feature
15:01:15 lyarwood stephenfin: ^ as requested
15:01:19 stephenfin lyarwood: thanks
15:02:06 stephenfin elod: Given they're test-only (low risk) and avoid merge conflicts for this series and potentially many more tests in the future, I'm personally inclined to think they belong here.
15:06:52 elod stephenfin: the test-only part is good, but still it would mean larger and larger extra refactor (due to conflicts) in older branches which could cause more problems later on maybe
15:08:16 stephenfin elod: Fair point but tbh I'm not sure how much further back we should go. Those tests and the changes themselves are invasive enough that I think they may have to be rewritten if I want to go significantly further back
15:10:36 stephenfin elod: In fact, I don't think we need to go further back. The main issue here was introduced by https://review.opendev.org/#/c/641806/ which was never backported
15:10:46 stephenfin and was merged in train
15:11:39 elod stephenfin: yes, that is another question, how far should a backport go. in general, whether a bugfix worth the backporting effort if it involves so much refactor, hence risks
15:12:23 elod stephenfin: but i understand you are saying this won't go further than train
15:13:19 stephenfin given the nasty side-effects of this race (worst case scenario: doubly pinned instances) I think bringing it back to train makes sense
15:24:09 elod stephenfin: ok, i accept it. (I won't say i'm happy with it, but i understand your point o:))
15:28:13 openstackgerrit Lee Yarwood proposed openstack/nova stable/train: Create volume attachment during boot from volume in compute https://review.opendev.org/752488
15:28:14 openstackgerrit Lee Yarwood proposed openstack/nova stable/train: compute: Skip cinder_encryption_key_id check when booting from volume https://review.opendev.org/752489
15:37:01 sean-k-mooney gibi: have you see error in the fucntional test that look like ValueError: Malformed PCI address 0000:00:03
15:37:28 sean-k-mooney gibi: your new sriov attach detach test are failing for me when i rebase one of my patches
15:37:43 sean-k-mooney but the code im chanign i dont think should cause that
15:38:40 stephenfin sean-k-mooney: I haven't, but I know those fixtures well enough that I could debug. What test?
15:38:45 stephenfin *what patch?
15:38:54 sean-k-mooney http://paste.openstack.org/show/798018/
15:39:21 sean-k-mooney nova.tests.functional.libvirt.test_pci_sriov_servers.SRIOVAttachDetachTest.test_detach_macvtap
15:39:36 sean-k-mooney and nova.tests.functional.libvirt.test_pci_sriov_servers.SRIOVAttachDetachTest.test_detach_attach_direct
15:40:35 sean-k-mooney im cherry picking https://review.opendev.org/#/c/602432/16 onto another patch but after the rebase on master those new test are failing for me
15:40:59 sean-k-mooney i could just push what i have it might be simpler to look at
15:41:48 gibi sean-k-mooney: yes please ^^
15:41:52 openstackgerrit Balazs Gibizer proposed openstack/nova master: Reject HW_CPU_HYPERTHREADING trait in named groups https://review.opendev.org/735947
15:41:58 gibi sean-k-mooney: I did not remember seeing such
15:42:00 gibi err
15:43:27 stephenfin sean-k-mooney: this is a guess but https://review.opendev.org/#/c/602432/16/nova/tests/unit/virt/libvirt/fakelibvirt.py
15:46:53 sean-k-mooney ya i guess that could be it ill take a look
15:47:02 sean-k-mooney i made fucn be octal
15:47:13 sean-k-mooney func = 0o0
15:47:23 sean-k-mooney but ya it could be the formating
15:48:10 stephenfin '1'
15:48:10 stephenfin >>> '%s' % x
15:48:10 stephenfin >>> x = 0o1
15:48:35 sean-k-mooney ok ya so that is proably it
15:49:21 sean-k-mooney cool now i need to look up hex/octal format codes :)
15:50:17 sean-k-mooney so it shoudl be o not s
15:50:36 sean-k-mooney or i guess x would work too for hex
15:52:10 gibi the nova weekly meeting starts in 8 minutes in #openstack-meetings-3
15:52:25 gibi I mean #openstack-meeting-3
15:59:44 sean-k-mooney stephenfin: actully the adress is 0000:00:03. its missing the function e.g. 0000:00:03.1 vs 0000:00:03.
16:00:21 sean-k-mooney so its proably not the formating but its likely related to that change in my patch
16:24:29 stephenfin sean-k-mooney, bauzas: Either of you know why we pass 'live=True' here instead of actually checking the power state of the instance as we do elsewhere? https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4396
16:24:51 stephenfin for both PCI (link above) and MDEV-type devices https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7512
16:25:46 bauzas looking
16:25:54 stephenfin Is it because we pass 'persistent=True' when attaching them, e.g. https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4425 ?
16:27:04 sean-k-mooney i would expect use to be consistent
16:27:28 sean-k-mooney git blame on github does not work on that file because its too big
16:27:49 sean-k-mooney we want to remove it form both the live and persistt doamin that said we dont use the persitent domain
16:28:04 bauzas stephenfin: "–live indicates that the operation is dynamic and will reflect in the running guest."
16:28:31 sean-k-mooney bauzas: right but that should be true of attach too
16:28:39 stephenfin So if the guest was paused or stopped, that would fail right?
16:28:43 sean-k-mooney https://github.com/openstack/nova/blob/f47bde539c89d1ac1fa90ea6f0d72a7e2de19532/nova/virt/libvirt/guest.py#L488
16:28:54 sean-k-mooney yes
16:29:01 stephenfin so...bug?
16:29:09 sean-k-mooney although you cant detach from a paused guest
16:29:13 sean-k-mooney it will fail
16:29:24 stephenfin PCI devices or anything?
16:29:47 sean-k-mooney so detach will send a acpi interupt to the guest
16:29:48 stephenfin I guess PCI devices are different because of the whole "managed" aspect
16:29:59 sean-k-mooney but when its paused nothing will repond to that
16:30:03 sean-k-mooney so the detach shoudl fail
16:30:15 sean-k-mooney the same shoudl be true fo mdevs
16:30:39 sean-k-mooney in the pasued state the vm domain is still defined but the cpus are stopped
16:30:50 sean-k-mooney qemu is still running
16:30:58 sean-k-mooney unlike suspend
16:31:04 stephenfin does that also apply for e.g. block devices?
16:31:10 sean-k-mooney yes
16:31:25 bauzas you lost me, folks
16:31:37 sean-k-mooney we use virtio-blk which uses a pci per volume by default
16:31:50 sean-k-mooney so volume detaches do a pci hot unplug
16:31:58 sean-k-mooney so it wont work on a paused guest
16:32:20 sean-k-mooney im not sure about virtio-scisi but i suspect that wont work either
16:32:29 stephenfin bauzas: we're questioning whether the live parameter makes sense here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L2247
16:32:50 stephenfin or if we should be outright rejecting a request to attach/detach a device from a paused guest
16:32:54 sean-k-mooney stephenfin: i think we shoudl be passing both live and persisted in almsot every case
16:33:18 sean-k-mooney stephenfin: i dont know of any reason we would not want to update both
16:33:23 sean-k-mooney always
16:33:53 sean-k-mooney well i guess if the vm was off
16:34:03 sean-k-mooney but in htat case we dont need to update the domain
16:34:10 sean-k-mooney we will just recreated it when we start it
16:36:28 stephenfin okay, let me see if I can untangle this /o\
16:43:48 sean-k-mooney this could explain why we are getting out of sync
16:44:08 sean-k-mooney there is at least one bug where the live and perstient domains get out of sync
16:44:21 sean-k-mooney if we are attaching with persistent and detaching with live that would do it

Earlier   Later