| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-16 | |||
| 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 | if ret == -1: raise libvirtError ('virDomainDetachDeviceFlags() failed', dom=self) | |
| 14:17:01 | mriedem | but it clearly is related to detaching the device | |
| 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 | jroll | edleafe: jaypipes: cdent: ^ tell me where I'm wrong :) | |
| 14:33:47 | lbragstad | aunnam oh - i misunderstood that then | |
| 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 | aunnam | lbragstad, ya can discuss with you the problem i am facing | |
| 14:35:52 | mriedem | lyarwood: nvm that can't be right, test_detach_device_with_retry_invalid_argument tests for that case | |
| 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 | |
| 14:41:53 | lbragstad | aunnam so what do you have locally? | |
| 14:42:21 | aunnam | lbragstad, so since I commented out the rule the parser is not getting the policy rules and its failing | |
| 14:42:48 | lbragstad | aunnam can you paste a `git diff`? | |
| 14:43:49 | openstackgerrit | Sujitha proposed openstack/nova master: Adding tags field to InstancePayload https://review.openstack.org/407228 | |
| 14:44:04 | aunnam | lbragstad, so was thinking to write to policy-sample.yaml file without calling _generate_sample https://github.com/openstack/oslo.policy/blob/master/oslo_policy/tests/test_generator.py#L252 | |
| 14:44:24 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.reboot.error notification https://review.openstack.org/411791 | |