Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-08-02
12:59:38 gibi stephenfin: _IntegratedTestBase base class already set up the generic NeutronFixture which means that nova.network.neutronv2.api.get_client is already stubbed by the NeutronFixture
12:59:40 sean-k-mooney maciejjozefczyk: not nessicarialy. but in this case it is not that invasive a cchange
13:00:02 gibi stephenfin: then L350 sets up another fixture NUMAAffinityNeutronFixture that will also stub nova.network.neutronv2.api.get_client
13:00:20 gibi stephenfin: this is OK as the second stub overrides what the first stub did
13:00:23 stephenfin gibi: ahhh, of course. I missed that we were using the other fixture
13:00:33 stephenfin Not like I wrote that code or anything :)
13:00:53 gibi stephenfin: I guess you had a good vacation at properly reset your brain :)
13:01:01 gibi s/at/that/
13:01:06 sean-k-mooney maciejjozefczyk: for there binary direver it may or may not be complient with there EULA. for the linux opensource Nouveau driver its probaly fine
13:01:12 stephenfin gibi: Currently trying to remember what "Python" is
13:01:22 stephenfin :)
13:01:24 gibi :)
13:03:46 stephenfin gibi: One other comment (the second one here) https://review.openstack.org/#/c/587412/3/nova/tests/functional/libvirt/test_numa_servers.py
13:04:12 stephenfin sean-k-mooney: Off the top of your head, would calling os_vif.initialize() twice have any bad side effects?
13:04:33 sean-k-mooney stephenfin: no we specificaly check for that
13:04:57 gibi stephenfin: I can remove that os_vif.initialize() as that is already in the fake libivirt now
13:04:58 sean-k-mooney call it a 1000 times in a loop if you like it will only initalise once
13:05:25 stephenfin gibi: Meh, unless you want to, I'm happy to just +W as is. It's a nit
13:05:26 sean-k-mooney stephenfin: unless you pass reset=true
13:05:43 stephenfin sean-k-mooney: Excellent. It's just a clean up so
13:05:47 gibi stephenfin: I will respin it quickly
13:05:54 stephenfin gibi: ack
13:06:39 sean-k-mooney stephenfin: by the way if your remove os_vif.initialize from setup and dont call it at all your test will fail
13:08:21 sean-k-mooney stephenfin: i added it becuse your tests were failing because nova assumes (correctly) that os-vif is initalised when its using it and the code is written in such a way that it fails if its not
13:09:17 sean-k-mooney stephenfin: i put it in setup beacuse i dont know what order the test will be run in
13:09:37 stephenfin sean-k-mooney: Yup, I think is was you that pointed that out to me. It should be good now though because of https://review.openstack.org/#/c/587412/3/nova/tests/unit/virt/libvirt/fakelibvirt.py
13:09:50 stephenfin So it'll get initialized the same time the fake nova-compute service starts
13:10:11 sean-k-mooney oh sweet then ya you can remove it from setup
13:11:20 openstack Launchpad bug 1784353 in OpenStack Compute (nova) "Rescheduled boot from volume instances fail due to the premature removal of their attachments" [Medium,In progress] - Assigned to Lee Yarwood (lyarwood)
13:11:20 mriedem lyarwood: before i get too far into https://bugs.launchpad.net/nova/+bug/1784353 - we don't reschedule on a boot from volume failure
13:11:21 sean-k-mooney or not its a nit as you said. its not needed anymore but not enough for a respin on its own
13:11:59 mriedem lyarwood: or is this non-volume backed, so not really boot from volume,
13:12:06 mriedem just boot with volumes attached, but the root disk is on local storage
13:15:19 openstackgerrit Balazs Gibizer proposed openstack/nova master: Improve NeutronFixture and remove unncessary stubbing https://review.openstack.org/587412
13:15:23 gibi stephenfin: ^^
13:17:32 stephenfin gibi: and done
13:17:40 gibi stephenfin: thanks
13:17:42 mriedem lyarwood: do you have some extra changes that make us reschedule on volume attach failures during boot? because we should abort here on any failure to attach volumes https://github.com/openstack/nova/blob/7125dcb9cb821faf3c68526ac34365a28141e480/nova/compute/manager.py#L2320
13:18:23 lyarwood mriedem: I've used BFV instances in the regression tests and just mocked out spawn to fail, not the volume attachments etc
13:18:39 mriedem lyarwood: that's not your bug though
13:18:46 mriedem your bug is not that spawn fails, but volume attach fails
13:19:06 mriedem oh wait,
13:19:11 mriedem i think i get it,
13:19:13 lyarwood mriedem: that's for the following attempt
13:19:14 mriedem first boot spawn() fails,
13:19:16 mriedem reschedule,
13:19:17 lyarwood mriedem: yeah
13:19:22 mriedem 2nd host fails b/c volume attachments are wrong
13:19:23 mriedem ok
13:19:28 openstack bug 1488111 in OpenStack Compute (nova) "Boot from volumes that fail in initialize_connection are not rescheduled" [Wishlist,Confirmed] https://launchpad.net/bugs/1488111
13:19:28 mriedem b/c bug 1488111
13:19:39 mriedem i was like, whatchutalkinboutyarwood
13:19:47 lyarwood ^_^
13:30:09 sean-k-mooney mriedem: lyarwood stephenfin. Quick Question can you think of any reason why i shoulds not replace calls to suspend in cold shapshot case with calles to pause given suspend does not actully suspend the instance regardless of the name or comemnt and its break sriov/pcipasshtough in some cases?
13:31:30 sean-k-mooney im going to try it in any case and see what happens but do ye know why the current bevhaior is to detach all pci devices then save guest ram and not suspend
13:32:23 mriedem don't ask me
13:32:45 mriedem i always have to lookup the difference in libvirt between pause and suspend
13:33:17 lyarwood sean-k-mooney: hmmm iirc we need to detach PCI devices as we can't save or restore their state
13:33:46 sean-k-mooney lyarwood: to stop dma transfer or somthing?
13:34:54 sean-k-mooney in any case the current state seams broken.
13:35:00 lyarwood sahid: ^ do you know?
13:35:09 lyarwood sean-k-mooney: I'm not sure tbh
13:35:53 lyarwood sean-k-mooney: and yeah suspend just saves the domain state to disk right? So that wouldn't help during a cold snapshot.
13:37:21 lyarwood wait that isn't right, it does pause the domain
13:37:50 sean-k-mooney lyarwood: no it doesnt
13:38:00 sean-k-mooney https://github.com/openstack/nova/blame/77ece76a70aa251e6f06e073b28c2ca978caf8f8/nova/virt/libvirt/driver.py#L2639-L2646
13:39:16 mriedem lyarwood: your bug says we call _shutdown_instance before reschedule but i don't think that's the case if driver.spawn() fails
13:39:25 sean-k-mooney we call it in _prepare_domain_for_snapshot https://github.com/openstack/nova/blob/77ece76a70aa251e6f06e073b28c2ca978caf8f8/nova/virt/libvirt/driver.py#L1740
13:40:18 mriedem lyarwood: wouldn't we only call _shutdown_instance here prior to rescheduling https://github.com/openstack/nova/blob/7125dcb9cb821faf3c68526ac34365a28141e480/nova/compute/manager.py#L2364 but only if network or bdm setup failed?
13:41:13 lyarwood sean-k-mooney: ack, so ManagedSave doesn't pause the domain, fun.
13:41:29 mriedem oh nvm,
13:41:38 mriedem i guess that's the exception handling from the yield on the context manager,
13:41:48 mriedem here https://github.com/openstack/nova/blob/7125dcb9cb821faf3c68526ac34365a28141e480/nova/compute/manager.py#L2092
13:41:54 mriedem which is what calls driver.spawn()
13:41:57 mriedem god this flow is confusing
13:42:26 lyarwood mriedem: yeah indeed, it's awkward.
13:43:34 mriedem ok and the bdm.attachment_id in this flow is created in the API right?
13:43:45 lyarwood mriedem: yes
13:43:49 sean-k-mooney lyarwood: i have to go into the libvirt python binding to check but no code in nova does. anyway ill keep digging and see what i find
13:43:53 mriedem so api creates the attachment record, we hit hostA, fail to spawn, delete the attachment, reshcedule to hostB, try to attach and the attachment is gone
13:44:07 lyarwood mriedem: yup that's it
13:57:13 mdbooth mriedem: spent a bunch of time thinking about your rollback failed evacuated guest patch. TL;DR I think we need to clean that up in the driver, and if we get out of the driver with a running instance we shouldn't be talking about rollbacks any more.
13:57:51 mdbooth mriedem: Detail in review.
14:00:03 mriedem mdbooth: thanks; i asked in the bug report if they had details on what the actual failure was after the guest was spawned on the dest - like if it was a db error updating the instance status or something
14:00:12 mriedem i'm not in love with this patch as noted in the commit message
14:00:41 mdbooth mriedem: ack. I got that.
14:00:48 openstackgerrit Merged openstack/nova stable/queens: Call generate_image_url only for legacy notification https://review.openstack.org/584969
14:06:25 mriedem the bug reporter replied with what they failed on the first time they hit the dest host,
14:06:29 mriedem and apparently it was driver.spawn()
14:06:59 mriedem message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken."
14:06:59 mriedem "libvirtError: Did not receive a reply. Possible causes include: the remote application did not send a reply, the
14:13:28 stephenfin gibi: If I run 'tox -e api-samples', I see three new files in doc/api_samples/. Is that expected?
14:13:52 stephenfin gibi: http://paste.openstack.org/show/727146/
14:15:37 gibi stephenfin: actually I don't know
14:15:58 gibi the notification sample handling is pretty different from the api sample handling
14:16:16 gibi for example the test run never generates notificaton samples
14:16:22 gibi on the file system
14:16:30 stephenfin gibi: Hmm, I wonder who would know? gmann?
14:16:51 gibi sdauge was the mastermind but alex_xu or gmann could know
14:18:15 mdbooth mriedem: That sounds like a bug in the libvirt driver and/or libvirt to me.

Earlier   Later