| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-16 | |||
| 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 | |
| 14:44:40 | aunnam | lbragstad, i am not sure if it is the right way so waiting on that | |
| 14:44:52 | mhenkel | hello jaypipes | |
| 14:44:54 | lyarwood | mriedem: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/host.py#L833 - defineXML == creates a persistent domain | |
| 14:45:08 | mriedem | lyarwood: ok. another odd thing is in the stacktrace and error, if we were re-raising DeviceNotFound from _try_detach_device, we should see oslo.utils log the original libvirtError before reraising the new exception http://git.openstack.org/cgit/openstack/oslo.utils/tree/oslo_utils/excutils.py#n212 | |
| 14:45:12 | mriedem | lyarwood: but i don't see that happen | |
| 14:45:12 | aunnam | lbragstad, thats what found when I look into code | |
| 14:45:24 | mriedem | instead we get here http://git.openstack.org/cgit/openstack/oslo.utils/tree/oslo_utils/excutils.py#n220 | |
| 14:45:25 | lbragstad | aunnam i might have missed it, but what changes have you made locally? | |
| 14:45:30 | jaypipes | mhenkel: hi! currently on calls for another hour or so... gimme a bit? :) | |
| 14:45:37 | mriedem | i wonder if we're using a newer eventlet that's causing some context switching bugs | |
| 14:45:57 | aunnam | lbragstad, just commented out the rule, that all rule, haven't changed the tests yet | |
| 14:46:03 | mhenkel | jaypipes: sure thing, ping me when you have a minute or two | |
| 14:46:30 | openstackgerrit | Sujitha proposed openstack/nova master: Change tags to default field in Instance object. https://review.openstack.org/415298 | |
| 14:46:33 | aunnam | lbragstad, commented the rule in here https://github.com/openstack/oslo.policy/blob/master/oslo_policy/generator.py#L109 | |
| 14:46:44 | openstackgerrit | Sujitha proposed openstack/nova master: Reduce calls to load_tags() to 0 https://review.openstack.org/435146 | |
| 14:47:15 | mriedem | no that's not it, last time we bumped eventlet up was last may | |
| 14:48:16 | lbragstad | aunnam alright - this is what i have locally http://cdn.pasteraw.com/3u2pj6gigswl55mkeqdbojeifp6gzf4 | |
| 14:48:30 | lbragstad | which is showing me 6 failures | |
| 14:48:39 | lyarwood | mriedem: do you have a logstash query for this already btw? | |
| 14:49:08 | mriedem | lyarwood: just this: http://logstash.openstack.org/#dashboard/file/logstash.json?query=message%3A%5C%22AttachVolumeShelveTestJSON%5C%22%20AND%20message%3A%5C%22Failed%20to%20detach%20volume%5C%22%20AND%20message%3A%5C%22libvirtError%3A%20End%20of%20file%20while%20reading%20data%3A%20Input%2Foutput%20error%5C%22%20AND%20tags%3A%5C%22screen-n-cpu.txt%5C%22&from=7d | |
| 14:49:57 | aunnam | lbragstad, ya that's the same think that i have | |
| 14:49:59 | mriedem | lyarwood: did you create a bug? i think we need some more logging in detach_device_with_retry because i can't make out the stacktrace from that method given all of the context managers | |
| 14:50:05 | lbragstad | aunnam cool | |
| 14:50:59 | lyarwood | mriedem: thanks, not yet just waiting on devstack finishing locally before I try to reproduce | |
| 14:51:33 | gibi | sdague: responded on https://review.openstack.org/#/c/440580/5/specs/pike/approved/scheduler-hints-in-server-details.rst I can bargain on what hints the API will return | |
| 14:52:34 | openstackgerrit | Sujitha proposed openstack/nova master: Adding auto_disk_config field to InstancePayload https://review.openstack.org/419185 | |
| 14:53:13 | openstackgerrit | Béla Vancsics proposed openstack/nova master: Reduce code complexity - libvirt/config.py https://review.openstack.org/359879 | |
| 14:53:46 | mriedem | lyarwood: since we dont see "Original exception being dropped" i'm wondering if we're hitting _try_detach_device the first time, it's ok, and then when we try the 2nd time, it fails with that i/o error | |
| 14:54:07 | mriedem | that's why i want more debug logging in there to know if we're calling _try_detach_device before we get into the retry loop | |
| 14:54:27 | mriedem | because i'm guessing that the i/o error thing on an already detached device is newish in libvirt 1.3.1 | |
| 14:54:47 | mdbooth | lyarwood mriedem: Just finished reading scrollback. Sounds fun. | |