| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-30 | |||
| 16:19:11 | openstackgerrit | Stephen Finucane proposed openstack/nova master: nova-net: Add TODOs for remaining nova-network functional tests https://review.opendev.org/684345 | |
| 16:21:18 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Migrate old style volume attachments on nova-compute startup https://review.opendev.org/549130 | |
| 16:21:19 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Extract some helper functions from DriverVolumeBlockDevice https://review.opendev.org/685752 | |
| 16:24:11 | openstackgerrit | Stephen Finucane proposed openstack/nova master: nova-net: Migrate 'test_floating_ips' functional tests https://review.opendev.org/684344 | |
| 16:24:12 | openstackgerrit | Stephen Finucane proposed openstack/nova master: nova-net: Add TODOs for remaining nova-network functional tests https://review.opendev.org/684345 | |
| 16:26:52 | openstackgerrit | Merged openstack/nova-specs master: Fix invalid link index https://review.opendev.org/685664 | |
| 16:45:50 | openstackgerrit | Adam Spiers proposed openstack/nova master: Also enable iommu for virtio controllers in libvirt https://review.opendev.org/684825 | |
| 16:47:44 | openstackgerrit | Adam Spiers proposed openstack/nova stable/train: Also enable iommu for virtio controllers in libvirt https://review.opendev.org/685756 | |
| 17:05:40 | efried | aspiers: RC2? ^ | |
| 17:06:07 | efried | not clear from the bug report what actually breaks | |
| 17:11:46 | sean-k-mooney | efried: im guessing it breaks sev guest when you set the disk bus to scsi and select virt-scsi as the model | |
| 17:14:08 | efried | sean-k-mooney: is that something that happens frequently? | |
| 17:14:13 | sean-k-mooney | yes | |
| 17:14:41 | efried | and by "breaks" -- the guest won't boot? or won't be SEV'd? or...? | |
| 17:14:52 | sean-k-mooney | hw_disk_mode=scsi and hw_scsi_model=virtio-scsi is the recommend mode for ceph | |
| 17:15:38 | sean-k-mooney | im not sure what aspiers is chaning is enabling the iommu integration so i would guest it unencyrped but i dont think tha tis allowd for sev guest so it might just not boot | |
| 17:17:00 | aspiers | efried: I think it probably causes a kernel panic | |
| 17:17:12 | sean-k-mooney | in the guest right | |
| 17:17:15 | aspiers | from the spec: "The iommu attribute is on for all virtio devices. Despite the name, this does not require the guest or host to have an IOMMU device, but merely enables the virtio flag which indicates that virtualized DMA should be used. This ties into the SEV code to handle memory encryption/decryption, and prevents IO buffers being shared between host and guest." | |
| 17:17:43 | aspiers | I've definitely seen kernel panics from incorrectly configured guests, I can't remember if I explicitly tested virtio-scsi without iommu | |
| 17:17:48 | aspiers | but it's required for sure | |
| 17:17:54 | efried | k, would be good to understand all of that to know whether this should be considered critical for RC2. It sounds like it probably is. <== dansmith mriedem | |
| 17:18:28 | sean-k-mooney | well you have to opt into enabling the scsi disk bus | |
| 17:18:30 | efried | aspiers: imo [unencrypted when encrypted was expected] would be worse than [kernel panic] (assuming the latter is only affecting the guest, not the whole host) | |
| 17:18:34 | sean-k-mooney | since that is not the default | |
| 17:18:43 | aspiers | sean-k-mooney: not if config drives or cdroms are used | |
| 17:18:52 | sean-k-mooney | but it would be very common to do so if you are using ceph | |
| 17:19:02 | efried | but either way it's a bug in a new feature and therefore RC potential IIUC | |
| 17:19:10 | sean-k-mooney | aspiers: config drive should now defualt to sata | |
| 17:19:10 | aspiers | efried: [unencrypted when encrypted was expected] will not happen | |
| 17:19:18 | sean-k-mooney | and it used to default to ide | |
| 17:19:21 | efried | well that's good anyway :) | |
| 17:19:36 | aspiers | efried: the <launchSecurity> element will be there regardless of any iommu stuff | |
| 17:22:47 | dansmith | efried: in a sec | |
| 17:23:12 | dansmith | mriedem: do you know if another db_sync --all-cells patch was floated somewhere? The one I was thinking of never merged and was abandoned silently in july | |
| 17:23:21 | dansmith | mriedem: https://review.opendev.org/#/c/519275/ | |
| 17:23:30 | dansmith | not sure why, but that probably took it off anyone's radar | |
| 17:24:10 | dansmith | or hmm, maybe we made it hit all cells by default... gosh, all this fell out of my head | |
| 17:25:19 | dansmith | ah, no --local-cell is just skipping cell0, so it still doesn't fan like I thought | |
| 17:29:30 | dansmith | efried: probably depends on what the fix looks like and what impacts it might have, and what the impact of not having it is (like you say expecting safe, but not safe) | |
| 17:30:55 | sean-k-mooney | thats the main part of the fix https://review.opendev.org/#/c/684825/4/nova/virt/libvirt/designer.py | |
| 17:33:37 | sean-k-mooney | aspiers: by the way the only think that stikes me about the fix is that we are testing with a fake xml we cannot generate | |
| 17:33:38 | mriedem | dansmith: i don't remember another, though i thought i had reported a bug at some point about making db sync support all cells but i might just be thinking of the archive command | |
| 17:34:05 | sean-k-mooney | e.g. its not posibel to have two scsi contolers with a different model based on how nova generates the xml | |
| 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, | |