| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-30 | |||
| 17:34:06 | dansmith | mriedem: yeah, so code in tree does not fan out (except to cell0) and that other patch got abandoned in July for some reason | |
| 17:34:16 | sean-k-mooney | that said it is vaild in libvirt to do that | |
| 17:34:19 | mriedem | i also remember bringing it up at some summit, i.e. should the nova-manage commands hit all cells? and it was a low priority response - that might have been sydney... | |
| 17:34:36 | mriedem | dansmith: my guess is they abandoned it b/c it sat since july with no replies | |
| 17:34:47 | dansmith | could be | |
| 17:34:59 | mriedem | lincanwei is still around though, he's the watcher ptl | |
| 17:39:20 | dansmith | oh, nm, this was abandoned in Jan, last patch was in july 2018, I see | |
| 17:39:27 | dansmith | hmm, I thought there was a more recent attempt at this t hen | |
| 17:40:32 | mriedem | i see a duplicate of the archive all-cells patch | |
| 17:40:56 | mriedem | https://review.opendev.org/#/c/599050/ ? | |
| 17:41:08 | mriedem | oh no that's different | |
| 17:41:13 | dansmith | no | |
| 17:41:14 | dansmith | yeah | |
| 17:41:22 | dansmith | anyway, not a big deal | |
| 17:41:51 | gmann | mriedem: dansmith : seems like sec groups are added for down cell response for detail GET API case only (it is not included in Show API case) | |
| 17:42:06 | mriedem | dansmith: heh https://review.opendev.org/#/c/420973/ | |
| 17:42:08 | mriedem | it was me! | |
| 17:42:14 | gmann | and that is because sec grps are added explicitly for detail case- https://github.com/openstack/nova/blob/961c2945491ebcea3cf1cb175a06d057155aa5a5/nova/api/openstack/compute/views/servers.py#L410 | |
| 17:42:32 | dansmith | mriedem: that's not the one I was thinking of, but..funny | |
| 17:42:34 | gmann | is it missed in 2.69 microversion ? | |
| 17:44:12 | gmann | with nova-net it was not added as there were no sec grp in DB nut with stephenfin patch to run sample tests with neutron fail - https://review.opendev.org/#/c/684335/5/doc/api_samples/servers/v2.69/servers-details-resp.json@12 | |
| 17:45:06 | mriedem | it was probably an oversight | |
| 17:45:36 | mriedem | i'm also not sure how much we need to care if the cell that the instance is in is down | |
| 17:45:40 | gmann | and same for host_status ? | |
| 17:46:12 | mriedem | if the cell that the instance is in is down, the host_status likely doesn't matter | |
| 17:46:16 | mriedem | you can't do anything with that instance | |
| 17:46:23 | mriedem | except maybe delete it | |
| 17:47:13 | mriedem | the whole point with 2.69 is return a minimal set of stuff based on what's in the API DB for the instance | |
| 17:47:24 | mriedem | not to return everything we possibly can | |
| 17:47:57 | mriedem | e.g. we could also proxy to cinder to get attached volume info but we're not going to do that (like the neutron api proxy call to get security groups) | |
| 17:48:49 | mriedem | gmann: for host_status this method won't work really https://github.com/openstack/nova/blob/961c2945491ebcea3cf1cb175a06d057155aa5a5/nova/compute/api.py#L4881 | |
| 17:48:57 | gmann | yeah, it would not hurt to return. only difference will be GET and GET details response. Not sure is it worth to fix though. | |
| 17:49:11 | mriedem | instance.host wouldn't be set so at best we'd be returning NONE which is potentially not accurate - UNKNOWN would be more appropriate | |
| 17:49:29 | mriedem | if you're using 2.69+ and getting a repsonse from a down cell, there are already going to be a lot of differences | |
| 17:49:37 | mriedem | for which you need to account client-side | |
| 17:50:02 | mriedem | https://docs.openstack.org/api-guide/compute/down_cells.html gives the details on the fields that are returned | |
| 17:52:34 | sean-k-mooney | aspiers: commented on https://review.opendev.org/#/c/684825/4 over all this address the reported bug but it misses another edgecase where qemu virtio channeles are not handeled | |
| 17:53:09 | gmann | i see. then let's include in sample files as it is returned. | |
| 17:54:50 | mriedem | gmann: sorry, what is returned? | |
| 17:55:13 | mriedem | host_status nor security_groups are returned for GET /servers/{server_id} when the cell is down | |
| 17:55:51 | mriedem | show and detail are different in the down cell case b/c in the detail case we won't even get to https://github.com/openstack/nova/blob/961c2945491ebcea3cf1cb175a06d057155aa5a5/nova/api/openstack/compute/views/servers.py#L146 because we will have already filtered out the instances from the down cells | |
| 17:55:57 | gmann | mriedem: yeah but 'security_groups' are returned in GET /servers/details case | |
| 17:56:10 | mriedem | not if the instance is in a down cell | |
| 17:56:16 | gmann | mriedem: due to this - https://github.com/openstack/nova/blob/961c2945491ebcea3cf1cb175a06d057155aa5a5/nova/api/openstack/compute/views/servers.py#L410 | |
| 17:56:30 | mriedem | gmann: you will not get there with an instance from a down cell | |
| 17:56:37 | mriedem | the multi-cell instance lister code will filter out those results | |
| 17:56:45 | gmann | because for detail case it is added after show method return | |
| 17:57:11 | mriedem | ... | |
| 17:57:17 | mriedem | again, | |
| 17:57:28 | mriedem | GET /servers/detail will filter out instances from down cells | |
| 17:57:35 | mriedem | so we will not get to https://github.com/openstack/nova/blob/961c2945491ebcea3cf1cb175a06d057155aa5a5/nova/api/openstack/compute/views/servers.py#L410 for instances from down cells | |
| 17:57:38 | mriedem | so i don't see the problem | |
| 17:58:14 | mriedem | oh i think i see | |
| 17:58:24 | mriedem | https://github.com/openstack/nova/blob/961c2945491ebcea3cf1cb175a06d057155aa5a5/nova/api/openstack/compute/views/servers.py#L396 | |
| 17:58:27 | mriedem | self._list_view(self.show | |
| 17:58:29 | mriedem | but still, | |
| 17:58:39 | mriedem | instances from a down cell won't get that far | |
| 17:58:47 | mriedem | gmann: are you just basing this on code inspection? | |
| 17:59:30 | gmann | mriedem: this is from tests which I am not sure does actual down cell handling - https://review.opendev.org/#/c/684335/5/doc/api_samples/servers/v2.69/servers-details-resp.json | |
| 17:59:56 | gmann | so instance from down-cell will be there in GET detail case or not | |
| 18:00:08 | mriedem | will not | |
| 18:00:11 | gmann | *so not sure if instance | |
| 18:00:13 | gmann | ok | |
| 18:00:15 | mriedem | compute API get_all will filter those out | |
| 18:00:49 | mriedem | you would need to follow this thread https://github.com/openstack/nova/blob/961c2945491ebcea3cf1cb175a06d057155aa5a5/nova/compute/api.py#L2760 | |
| 18:02:54 | gmann | but cell-down-support will be true for > 2.69 - https://github.com/openstack/nova/blob/961c2945491ebcea3cf1cb175a06d057155aa5a5/nova/api/openstack/compute/servers.py#L329 | |
| 18:03:24 | gmann | then it will include the instances form down cell also | |
| 18:03:29 | mriedem | no | |
| 18:03:31 | mriedem | that's not what that means | |
| 18:03:39 | mriedem | you need to look at the InstanceLister code | |
| 18:03:45 | aspiers | sean-k-mooney: I've replied and emailed the SEV gurus to ask them | |
| 18:03:53 | aspiers | sean-k-mooney: I would prefer to deal with that in a follow-up though | |
| 18:04:14 | aspiers | especially considering we don't actually know for sure yet it's an issue | |
| 18:04:32 | sean-k-mooney | yep we could | |
| 18:04:56 | sean-k-mooney | the code as written should be correct and adress the current bug | |
| 18:05:09 | mriedem | gmann: in here https://github.com/openstack/nova/blob/961c2945491ebcea3cf1cb175a06d057155aa5a5/nova/compute/multi_cell_list.py#L414 | |
| 18:05:17 | sean-k-mooney | but i think we should file a bug for the qemu-channel too | |
| 18:05:42 | sean-k-mooney | aspiers: if ye file a bug for it im +1 i just dont want to loose track of it | |
| 18:05:57 | sean-k-mooney | aspiers: we can always close it as invalid if its not required | |
| 18:06:17 | sean-k-mooney | aspiers: did ye try to boot a vm with a spice console and sev? | |
| 18:06:19 | aspiers | sean-k-mooney: I'll wait until the gurus respond, they're normally very fast and that can potentially save me the effort of filing | |
| 18:06:26 | aspiers | sean-k-mooney: yes I booted with spice fine | |
| 18:06:37 | aspiers | or at least I'm pretty sure | |
| 18:06:46 | aspiers | I tried a lot of different combos, can't remember all the details | |
| 18:07:07 | aspiers | definitely saw one minor virtio device without iommu | |
| 18:07:10 | aspiers | and it didn't crash | |
| 18:07:39 | aspiers | in fact I think I checked about that with our guru and he said don't worry about it... think it was mouse or USB input of some kind | |
| 18:09:17 | aspiers | heading off now o/ | |
| 18:09:26 | sean-k-mooney | aspiers: its used for spices usb fowradign support that allows usb devices to be hotpluged at runtime | |
| 18:09:41 | sean-k-mooney | without requireing libvirt to modify the xml | |
| 18:09:57 | sean-k-mooney | i belive it also allows spcie to do remote hotpug | |
| 18:10:22 | sean-k-mooney | e.g. a local usb to a remove vm like RDP does with drives and printers | |
| 18:15:50 | gmann | mriedem: thanks. that param in multi cell control only exception or skip to prepare the minimum response in APi view. | |
| 18:18:44 | gmann | we need to mock the same in sample tests also. | |
| 18:21:28 | mriedem | gmann_afk: ok. there are samples for 2.69 which use a down cell fixture so maybe something is just missing from that, idk. | |
| 18:56:41 | mriedem | dansmith: maybe we should finally fix this :) https://bugs.launchpad.net/nova/+bug/1781286 | |
| 18:56:41 | openstack | Launchpad bug 1781286 in OpenStack Compute (nova) "CantStartEngineError in cell conductor during reschedule - get_host_availability_zone up-call" [Medium,Triaged] | |
| 18:57:03 | mriedem | i think it's just a matter of shoving the AZ into the Selection object that we pass around in conductor | |
| 18:58:05 | mriedem | tricky thing is functional testing since we don't have a great way to poison conductor on a reschedule to not hit the API DB | |
| 18:58:39 | dansmith | ack | |