Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-01-11
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 config = get_device_conf_func(device)
16:56:05 gibi if config is not None:
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 gibi so the retry loop does not care about DeviceNotFound (partial detach failure) but only cares about the config being not None
17:13:33 melwitt that sneaky RetryDecorator
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
08:32:03 gibi lyarwood: I have to be away for keyboard for couple of hours this morning but I made some progress with the device detach code I will push it soon then at some point we can try to discuss it
08:36:27 openstackgerrit Balazs Gibizer proposed openstack/nova master: DNM try to replace retry with libvirt event in detach https://review.opendev.org/c/openstack/nova/+/770246
08:37:17 gibi lyarwood: ^^
09:41:23 lyarwood gibi: ack cool np, I'll take a look
10:55:10 bauzas sean-k-mooney: around ?
10:55:25 bauzas I'm facing a problem with the neutron API
10:55:37 bauzas I can't ask for getting the segments for a specific subnet
10:55:54 bauzas https://docs.openstack.org/api-ref/network/v2/index.html?expanded=list-segments-detail#segments
10:59:24 bauzas actually, https://docs.openstack.org/api-ref/network/v2/index.html?#segment-extension
11:00:29 bauzas we could get the segment ID from the subnet
11:43:16 sean-k-mooney bauzas: you don tneed too
11:43:22 sean-k-mooney the segment id is in the subnet
11:43:42 sean-k-mooney get the subnet id from the port then look up the subnet to get the id
11:44:21 sean-k-mooney if the subnet does not have a segemnt id then its not a routed network
11:52:45 bauzas right, that's what I found
12:47:34 alex_xu gibi: thanks
13:17:20 openstackgerrit Brin Zhang proposed openstack/nova master: Replaces tenant_id with project_id from List/Update Servers APIs https://review.opendev.org/c/openstack/nova/+/764292
13:17:20 openstackgerrit Brin Zhang proposed openstack/nova master: Replace all_tenants with all_projects in List Server APIs https://review.opendev.org/c/openstack/nova/+/765311
13:43:45 stephenfin gibi: So who's got the 2.89 microversion?
13:43:55 stephenfin Just started looking at brinzhang's series above ^
13:44:01 gibi stephenfin: did you get 2.88?
13:44:02 stephenfin (assuming you know)

Earlier   Later