| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-16 | |||
| 13:59:12 | mriedem | what i don't get is it looks like we hit the "no target device" error from libvirt, which we handle and reraise as DeviceNotFound, but then it looks like we try to detach again and get "libvirtError: End of file while reading data: Input/output error" which we don't handle | |
| 14:00:58 | mriedem | ok so the device name is different also because when attaching a volume to a shelved offloaded instance, we don't set the device on the bdm https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3678 | |
| 14:01:14 | mriedem | the tempest test should probably not even be specifying the device name | |
| 14:02:08 | lyarwood | yeah sorry I was just looking for the device xml during the detach to confirm what the target device was | |
| 14:12:09 | mriedem | so in the attach volume to shelved offloaded server case, we actually call os-attach in the cinder api to mark the volume as in-use, since we can't do it from the compute (since the instance isn't on a host), | |
| 14:12:10 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.volume_detach notification https://review.openstack.org/408676 | |
| 14:12:40 | mriedem | i'm wondering how that might mess up the new create attachment flows, i guess we can just update the volume attachment later when we're unshelving on the host and have the connection info, we update the vol attachment in cinder | |
| 14:12:55 | alexey_weyl | gibi: so the nova AZ has no aggregated host that the nova AZ is part of. | |
| 14:13:23 | alexey_weyl | gibi: is it correct to devstack only? or also to openstack in production? | |
| 14:13:24 | lyarwood | mriedem: right, update with the connector and get the connection_info back during unshelve | |
| 14:14:27 | gibi | alexey_weyl: the nova AZ contains by default every host that does not belong to any other AZ. | |
| 14:15:27 | gibi | alexey_weyl: it is not devstack specific but the name of the default AZ is configurable in the nova.conf with default_availability_zone option | |
| 14:16:05 | lyarwood | mriedem: do we have a bug for the detach from unshelve issue btw? I don't think it's an issue with the target dev, we look this up correctly and call for the detach using the correct xml AFAICT | |
| 14:16:16 | mriedem | lyarwood: we don't yet no | |
| 14:16:27 | lyarwood | mriedem: kk, I'll write one up if I can reproduce | |
| 14:16:33 | alexey_weyl | gibi: I see. Thanks a lot | |
| 14:16:50 | mriedem | i think we've been seeing "libvirtError: End of file while reading data: Input/output error" in the logs so much when the libvirt connection temporarily drops, that we just haven't considered this a separate issue | |
| 14:17:01 | mriedem | but it clearly is related to detaching the device | |
| 14:17:01 | mriedem | if ret == -1: raise libvirtError ('virDomainDetachDeviceFlags() failed', dom=self) | |
| 14:19:05 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transform instance.reboot notifications https://review.openstack.org/382959 | |
| 14:19:10 | sdague | sfinucan: fixed unit tests on - https://review.openstack.org/#/c/446452/ | |
| 14:19:20 | mriedem | lyarwood: i think i see an issue | |
| 14:19:52 | mriedem | lyarwood: in the normal attach volume flow, we pass do_driver_attach=True here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4799 | |
| 14:20:02 | mriedem | which is used here https://github.com/openstack/nova/blob/master/nova/virt/block_device.py#L265 | |
| 14:20:10 | mriedem | to actually attach the block device to the guest | |
| 14:20:19 | gibi | sdague: Hi! I left a detailed use case description in https://review.openstack.org/#/c/440580/5/specs/pike/approved/scheduler-hints-in-server-details.rst | |
| 14:20:21 | dansmith | gibi: replied on https://review.openstack.org/#/c/445697/2 | |
| 14:20:29 | gibi | dansmith: checking... | |
| 14:20:34 | mriedem | lyarwood: in the case of unshelve, we don't pass that https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4445 | |
| 14:20:37 | mriedem | and it defaults to False | |
| 14:20:50 | mriedem | so i don't think we ever actually attach the block device to the guest during unshelve | |
| 14:21:12 | lyarwood | mriedem: isn't part of the test to start the instance? | |
| 14:21:25 | mriedem | lyarwood: the test doesn't ssh into the guest to see that the device is there | |
| 14:21:42 | mriedem | the test just hits the compute api asking if the volume is associated with the instance, which it is via the bdms table | |
| 14:21:51 | mriedem | that doesn't mean it's actually attached in the guest though :) | |
| 14:22:17 | mriedem | i'm wondering if it's always been this way and the test is new, or if this was regressed when we removed check_attach internally, | |
| 14:22:25 | lyarwood | mriedem: right, I might be mixing up the ordering here, there's debug XML listing the device when we bring the instance up | |
| 14:22:34 | mriedem | https://github.com/openstack/nova/commit/63805735c25a54ad1b9b97e05080c1a6153d8e22 | |
| 14:23:24 | mriedem | nvm, i guess we just never pass do_driver_attach for unshelve | |
| 14:23:51 | sfinucan | sdague: Cool. Done | |
| 14:23:56 | mriedem | oh, probably because on unshelve we are spawning the instance | |
| 14:25:28 | sfinucan | jaypipes: I'd figured os-vif provided the modelling while nova would just do the wiring up, e.g. altering various aspects of the libvirt XML | |
| 14:25:54 | sfinucan | that change seemed to be more of the former, so that line is blurry :) | |
| 14:26:07 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.reboot.error notification https://review.openstack.org/411791 | |
| 14:26:34 | jaypipes | sfinucan: ya, I added a review comment on it. | |
| 14:26:54 | sfinucan | jaypipes: Gotcha. I'll check that out now | |
| 14:27:06 | gibi | dansmith: your propsal OK for me, thanks for looking into this problem | |
| 14:27:18 | dansmith | gibi: okay cool, I'll push it up.. thanks | |
| 14:28:07 | sdague | gibi: ok, further response here, I think we need to at least narrow this | |
| 14:29:23 | gibi | sdague: thanks, looking... | |
| 14:30:26 | openstackgerrit | Ed Leafe proposed openstack/nova master: WIP - add some functional tests for placement https://review.openstack.org/446123 | |
| 14:30:27 | openstackgerrit | Ed Leafe proposed openstack/nova master: Refactor placement fixtures https://review.openstack.org/446122 | |
| 14:31:07 | mriedem | lyarwood: ooo i think i've got it | |
| 14:31:17 | mriedem | we're using save_and_reraise_exception incorrectly | |
| 14:31:23 | mriedem | in detach_device_with_retry | |
| 14:31:24 | lbragstad | aunnam did you have a patch that commented out the policy during sample generation? | |
| 14:31:58 | mriedem | lyarwood: looking at the logs, we get here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/guest.py#L408 | |
| 14:32:10 | mriedem | File "/opt/stack/new/nova/nova/virt/libvirt/guest.py", line 408, in _try_detach_device | |
| 14:32:14 | bauzas | mriedem: dansmith: so I looked this CET morning around the archive_deleted_rows method | |
| 14:32:16 | aunnam | lbragstad, i am still fixing the tests that are failing | |
| 14:32:37 | lbragstad | aunnam i can take a stab at porting that to oslo.policy if you want | |
| 14:32:39 | mriedem | lyarwood: but because we aren't telling " with excutils.save_and_reraise_exception():" that we want to raise something new, it re-raises the original libvirtError | |
| 14:32:43 | bauzas | mriedem: dansmith: it seems to me that it could be a bit difficult to use it directly | |
| 14:32:53 | lbragstad | aunnam s/if you want/if you want to keep focusing on the nova tests/ | |
| 14:32:56 | mriedem | lyarwood: which i think then gets us into the retry loop https://github.com/openstack/nova/blob/master/nova/virt/libvirt/guest.py#L420 | |
| 14:33:05 | mriedem | and at that point we hit the i/o error | |
| 14:33:34 | aunnam | lbragstad, its no the nova tests that are failing, its the tests from oslo.policy | |
| 14:33:35 | openstackgerrit | Jim Rollenhagen proposed openstack/nova-specs master: Add spec for custom resource classes in flavors https://review.openstack.org/446570 | |
| 14:33:47 | lbragstad | aunnam oh - i misunderstood that then | |
| 14:33:47 | jroll | edleafe: jaypipes: cdent: ^ tell me where I'm wrong :) | |
| 14:33:52 | aunnam | lbragstad, s/no/not | |
| 14:33:53 | mriedem | hmm, "If another exception occurs, the saved exception is logged and the new exception is re-raised." | |
| 14:34:03 | mriedem | i wonder if something regressed in oslo.utils | |
| 14:34:26 | cdent | jroll: will you accept "everywhere" as an answer? | |
| 14:34:37 | lbragstad | aunnam for some reason i thought you were working on nova specific tests, i'll give the patch you have a look | |
| 14:34:41 | jroll | cdent: yes, then I'll give up :) | |
| 14:35:00 | lbragstad | aunnam is it up for review? | |
| 14:35:26 | aunnam | lbragstad, i'll post it now | |
| 14:35:46 | lyarwood | mriedem: right that's weird, we shouldn't be hitting the no target device error in the first place | |
| 14:35:52 | mriedem | lyarwood: nvm that can't be right, test_detach_device_with_retry_invalid_argument tests for that case | |
| 14:35:52 | aunnam | lbragstad, ya can discuss with you the problem i am facing | |
| 14:36:05 | lbragstad | aunnam cool | |
| 14:36:27 | lbragstad | aunnam fwiw - https://github.com/openstack/oslo.policy/commit/a95606c1dfd7368a247c79d5f65a54c629ce29b2 landed yesterday | |
| 14:36:39 | mriedem | lyarwood: could this be related to the patch you have about how we always pass persistent=True to detach_device_with_retry ? | |
| 14:36:58 | mriedem | https://review.openstack.org/#/c/441204/ | |
| 14:37:21 | aunnam | lbragstad, cool | |
| 14:37:33 | lyarwood | mriedem: hmm yeah if the domain from an unshelve isn't persistent | |
| 14:37:45 | mriedem | lyarwood: can you explain what a persistent domain even is? | |
| 14:37:47 | mriedem | i've never actually know | |
| 14:38:38 | lyarwood | mriedem: https://wiki.libvirt.org/page/VM_lifecycle#Transient_guest_domains_vs_Persistent_guest_domains | |
| 14:38:42 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.reboot notifications https://review.openstack.org/382959 | |
| 14:38:51 | aunnam | lbragstad, https://review.openstack.org/#/c/443332/1/oslo_policy/generator.py I commented out the rule at line 100 in this patch | |
| 14:39:14 | aunnam | lbragstad, https://github.com/openstack/oslo.policy/blob/master/oslo_policy/tests/test_generator.py#L278 this test is failing | |
| 14:39:20 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.reboot.error notification https://review.openstack.org/411791 | |
| 14:39:42 | lyarwood | mriedem: but it's just a normal driver.spawn() in the compute layer so I can't see how that could happen tbh | |
| 14:40:28 | aunnam | lbragstad, because in this test it is getting rules from sample policy file and merging those with the modified rules | |
| 14:40:41 | mriedem | lyarwood: yeah that page doesn't help me out here, | |
| 14:40:57 | mriedem | because that implies nova is making a decision when creating the domain as to whether or not it's persistent, right? | |
| 14:41:27 | mriedem | maybe we are, but can you point out where we decide that? | |
| 14:41:50 | lyarwood | mriedem: just checking but for spawn() I think they are always persistent, live mighration is one of the few cases I know where we start a transient domain on the destination | |