| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-02 | |||
| 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 | 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: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: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 | mriedem | b/c bug 1488111 | |
| 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: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 | "libvirtError: Did not receive a reply. Possible causes include: the remote application did not send a reply, the | |
| 14:06:59 | mriedem | message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken." | |
| 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. | |
| 14:21:05 | mriedem | mdbooth: agree | |
| 14:21:11 | mriedem | this was also mitaka so shrug | |
| 14:23:35 | mriedem | lyarwood: hmm, am i just not seeing it, but with the old style attach flow during bfv, if we failed to attach the volume, i don't see that we ever unreserve the volume from the instance | |
| 14:23:51 | mriedem | the api reserves the volume, but compute never unreserves it on failure | |
| 14:23:55 | mriedem | unlike if attach_volume fails | |
| 14:24:38 | mriedem | maybe that's just always been the way it is and expected b/c you can delete the instance in ERROR state which should unreserve the volume then | |
| 14:25:30 | mriedem | i guess _shutdown_instance calls the os-detach api in cinder but i don't know if that rolls back the reserved status | |
| 14:26:23 | lyarwood | mriedem: yeah I don't think we did unreserve in the old flow when we hit this error | |
| 14:26:44 | lyarwood | mriedem: I thought we had talked about updating attachments in the new flow with a None connector in this case | |
| 14:27:15 | lyarwood | mriedem: so the new compute only has to come along and update again with the correct connector | |
| 14:27:20 | mriedem | i think the os-detach call to cinder for the old flow in _shutdown_instance will make the volume available again | |
| 14:27:46 | lyarwood | kk then our removal of the attachment is fine | |
| 14:28:16 | mriedem | here is i think we're we'd get in the old flow prior to reschedule https://github.com/openstack/cinder/blob/b0e9ee1d501fb83b7ebbc59584aa6255dbaec086/cinder/volume/manager.py#L1314 | |
| 14:28:21 | mriedem | *where we'd | |
| 14:31:05 | openstackgerrit | Jay Pipes proposed openstack/nova master: DNM - example https://review.openstack.org/588295 | |
| 14:32:23 | openstackgerrit | Konstantinos Samaras-Tsakiris proposed openstack/nova master: Hide hypervisor id on windows guests https://review.openstack.org/579897 | |
| 14:32:25 | mriedem | lyarwood: ok comments in your series | |
| 14:32:35 | mriedem | lyarwood: lots of internal debate on this one, but i think what you're doing is likely the best | |
| 14:33:00 | lyarwood | mriedem: ack thanks | |
| 14:34:55 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP/PoC: safe_connect shouldn't hide failures https://review.openstack.org/584593 | |
| 14:41:39 | gmann | stephenfin: gibi api-samples tox run sample file with GENERATE_SAMPLES=True | |
| 14:42:03 | gmann | stephenfin: there can be chance that few file missing in doc/api_samples let me check those | |