| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-01-11 | |||
| 14:48:37 | bauzas | pasting it | |
| 14:48:53 | bauzas | hmpf, paste.o.o is super slow | |
| 14:50:08 | sean-k-mooney | ya it is weird | |
| 14:50:18 | bauzas | sean-k-mooney: https://pastebin.com/x0g0anRS | |
| 14:50:41 | bauzas | agreed with the logic ? | |
| 14:50:51 | bauzas | if so, adding the new neutron knob | |
| 14:51:18 | bauzas | (nevermind the pep8 indenting issue, fwiw :) ) | |
| 14:51:53 | sean-k-mooney | ya that works | |
| 14:52:10 | sean-k-mooney | although i kind of feel like lines 17-22 should be under the if on 13 | |
| 14:52:18 | sean-k-mooney | well if/else | |
| 14:52:51 | sean-k-mooney | i mean you dont need to do the if network_id as that will always be true | |
| 14:53:32 | sean-k-mooney | you could keep the if subnet_id but that will only be set if we go to line 14 in which case we know it wont be empty | |
| 14:54:07 | bauzas | yeah, I can improve this | |
| 14:54:14 | sean-k-mooney | so you can put lines 21 and 22 on line 15 and lines 18 and 19 on line 17 | |
| 14:54:29 | sean-k-mooney | but ya the logic looks correct to me | |
| 15:23:22 | openstackgerrit | Balazs Gibizer proposed openstack/nova stable/victoria: Reproduce bug 1907522 in functional test https://review.opendev.org/c/openstack/nova/+/769736 | |
| 15:23:24 | openstack | bug 1907522 in OpenStack Compute (nova) "test_*_with_qos_min_bw_allocation fails in the nova-multi-cell job with: nova.exception.MigrationPreCheckError: Migration pre-check error: Failed to create port bindings for host |
|
| 15:24:46 | openstackgerrit | Balazs Gibizer proposed openstack/nova stable/victoria: Fallback to same-cell resize with qos ports https://review.opendev.org/c/openstack/nova/+/769727 | |
| 16:10:37 | lyarwood | dansmith: https://review.opendev.org/c/openstack/nova-specs/+/769547 - would you mind hitting PS3 today? Should be good to go now. | |
| 16:11:03 | dansmith | roger | |
| 16:11:30 | lyarwood | cheers | |
| 16:15:07 | openstackgerrit | Dan Smith proposed openstack/nova-specs master: libvirt: Update instance machine type stash spec https://review.opendev.org/c/openstack/nova-specs/+/769547 | |
| 16:38:05 | openstackgerrit | Merged openstack/nova-specs master: libvirt: Update instance machine type stash spec https://review.opendev.org/c/openstack/nova-specs/+/769547 | |
| 16:39:57 | gibi | lyarwood: I'm looking at detach_device_with_retry() codepath detach_device_with_retry | |
| 16:40:26 | gibi | lyarwood: and wondering why we have the code set up in a way that we always call an intial detach then after it a detach in a loop | |
| 16:41:41 | gibi | lyarwood: also, I'm wondering what is the way _do_wait_and_retry_detach() normally exit the tool | |
| 16:41:44 | gibi | loop | |
| 16:42:20 | gibi | is it raises DeviceDetachFailed to signal that the detach is done? | |
| 16:42:49 | lyarwood | gibi: just in a call | |
| 16:43:12 | gibi | no worries, I will drop soon, so we can talk about it tomorrow morning | |
| 16:43:27 | lyarwood | gibi: ack if we could that would be great | |
| 16:46:38 | gibi | OK | |
| 16:48:33 | melwitt | gibi: IIRC it's because the initial detach "should" detach it from persistent + live config but the guest can refuse the live detach as it's an ACPI request. so we retry in case that happens (guest refusal by it's busy). under nominal conditions I think the loop detects it's detached in the first iteration and moves on | |
| 16:48:48 | melwitt | *bc it's busy | |
| 16:51:44 | gibi | melwitt: thanks. | |
| 16:52:06 | gibi | melwitt: both the initial and the retry loop calls _try_detach_device() _try_detach_device | |
| 16:52:12 | gibi | https://github.com/openstack/nova/blob/745f835e9d8931261501747130165824334abb4b/nova/virt/libvirt/guest.py#L404 | |
| 16:53:51 | gibi | melwitt: so if the first all to _try_detach_device() finishes without exception then when we call the retry loop it calls _try_detach_device() again that will detach again | |
| 16:54:02 | gibi | * first call | |
| 16:55:02 | melwitt | yeah, IIRC the second call will be a no-op if the guest detached the live config the first call | |
| 16:55:33 | melwitt | if the first call did not result in detach of the live config, it will loop N times to keep trying | |
| 16:55:57 | gibi | melwitt: ohh, is it because we have the condition in the loop config = get_device_conf_func(device) | |
| 16:56:00 | gibi | if config is not None: | |
| 16:56:05 | gibi | if config is not None: | |
| 16:56:05 | gibi | config = get_device_conf_func(device) | |
| 16:56:07 | gibi | sorry | |
| 16:56:10 | gibi | back copy paste | |
| 16:56:19 | gibi | here https://github.com/openstack/nova/blob/745f835e9d8931261501747130165824334abb4b/nova/virt/libvirt/guest.py#L476 | |
| 16:57:10 | melwitt | mmmaybe. I think that only checks whether the persistent config got detached, not the live | |
| 17:00:06 | melwitt | I think that only checks whether the persistent detach succeeded and so it will go ahead and try the live detach and get DeviceNotFound | |
| 17:00:56 | lyarwood | melwitt++ | |
| 17:03:10 | melwitt | and DeviceNotFound gets ignored as a no-op | |
| 17:03:48 | gibi | melwitt: what is the exit condition of _do_wait_and_retry_detach() ? the retry decorator retries on DeviceDetachFailed exception and if _try_detach_device() succeed without exception then _do_wait_and_retry_detach() raises a DeviceNotFound so that also leads to retry | |
| 17:05:38 | gibi | so the exit condition must be that the config become None | |
| 17:08:34 | melwitt | hmm.. yeah I see what you're saying. I hadn't thought the detach from live would be reflected in that get_disk but maybe it is | |
| 17:09:13 | melwitt | gibi: (it seems like it has to be, like you said) | |
| 17:10:42 | gibi | ohh we have two different exceptions in the control flow DeviceDetachFailed and DeviceNotFound, I think I mixed the two | |
| 17:11:12 | gibi | for the initial detach DeviceNotFound means we failed the detaching | |
| 17:11:22 | gibi | in one of the domains | |
| 17:12:13 | gibi | and hence we move to retry | |
| 17:12:17 | gibi | but in the retry loop we only retry on DeviceDetachFailed not on DeviceNotFound | |
| 17:12:56 | melwitt | gibi: ahhhh yes you got it | |
| 17:13:33 | melwitt | that sneaky RetryDecorator | |
| 17:13:33 | gibi | so the retry loop does not care about DeviceNotFound (partial detach failure) but only cares about the config being not None | |
| 17:13:56 | gibi | this code is hard | |
| 17:14:58 | melwitt | yes. this is exactly why lyarwood has plans to refactor it :) | |
| 17:15:08 | gibi | but then if the retry loop sees any exception (other than the self inflicted DeviceDetachFailed) e.g. DeviceNotFound then it just let the exception buble up and not retry. So I don't know how this really retries a partial detach failure in libvirt | |
| 17:15:23 | gibi | melwitt: yeah, and I try to help him, hence my questions :) | |
| 17:16:07 | gibi | (no the exceptions are not inherinting from each other) | |
| 17:16:17 | lyarwood | FWIW with an event based approach we would just ask libvirt to detach once and then wait | |
| 17:16:34 | gibi | lyarwood: and if if sends a failed event, we call detach again? | |
| 17:17:12 | lyarwood | true yeah we could but it should be easier to write and maintain than this mess | |
| 17:18:00 | gibi | lyarwood: or we consider the DeviceRemovalFailedEvent as a permanent failure that we buble up? | |
| 17:18:08 | gibi | s/than/and/ | |
| 17:18:16 | gibi | nvm | |
| 17:18:31 | gibi | that is end of my day. I will continue looking into this tomorrow | |
| 17:18:38 | gibi | melwitt, lyarwood: thanks for the help | |
| 17:31:53 | melwitt | it seems like get_device_conf_func has to be reflecting the live config too, else this doesn't make sense | |
| 18:39:09 | gmann | gibi: melwitt this should fix the multiple-network issue in nova-next job occurring in tempest scenario manager.https://review.opendev.org/c/openstack/tempest/+/770169 | |
| 18:39:51 | gmann | Previously i thought it cannot happen in scenario manager but if 'CONF.network.port_vnic_type' and 'CONF.network.port_profile' are not configured then it can occur which is what we saw in nova-next job | |
| 18:40:22 | melwitt | woot, thanks gmann | |
| 18:41:07 | gmann | this is where it falling on tenant default network which are note present https://zuul.opendev.org/t/openstack/build/ce402610f88a45928ff808bbb4a6303f/log/controller/logs/tempest_log.txt#53439 | |
| 18:41:15 | gmann | not | |
| 18:43:32 | melwitt | I vaguely remember trying to fix it with https://review.opendev.org/c/openstack/tempest/+/716809 a long time ago | |
| 18:45:25 | gmann | melwitt: ah i remember now that discussion. base class also need to setup the resources. | |
| 18:46:02 | gmann | in my patch I am creating default network resource by default for scenario tests and provide flag for subclass to stop that if needed | |
| 18:46:23 | gmann | creating by default should be right thing for scenario testing | |
| 18:46:38 | melwitt | yeah, I see. cool | |
| 21:32:40 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for resource providers https://review.opendev.org/c/openstack/placement/+/760240 | |
| 21:32:41 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for aggregates https://review.opendev.org/c/openstack/placement/+/760235 | |
| 21:32:42 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for allocations https://review.opendev.org/c/openstack/placement/+/760236 | |
| 21:32:43 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for allocation candidates https://review.opendev.org/c/openstack/placement/+/760237 | |
| 21:32:44 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for inventories https://review.opendev.org/c/openstack/placement/+/760238 | |
| 21:32:45 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for resource classes https://review.opendev.org/c/openstack/placement/+/760239 | |
| 21:32:47 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for traits https://review.opendev.org/c/openstack/placement/+/760241 | |
| 21:32:51 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for usage https://review.opendev.org/c/openstack/placement/+/760242 | |
| 22:19:21 | openstackgerrit | Merged openstack/nova master: api: Drop statistics-style fields from os-hypervisors https://review.opendev.org/c/openstack/nova/+/764040 | |
| #openstack-nova - 2021-01-12 | |||
| 02:54:36 | alex_xu | gibi: bauzas could you help me confirm this https://review.opendev.org/c/openstack/nova/+/729563/26/nova/conductor/manager.py#1023, it looks like a bug we left before or I missed something. also not sure if we can move forware with fix later or not. just trying help brinzhang move forward his patch. | |
| 08:07:01 | openstackgerrit | Wenping Song proposed openstack/nova master: Nova supports password encrypted VNC https://review.opendev.org/c/openstack/nova/+/622336 | |
| 08:31:19 | gibi | alex_xu: ack, I will check | |