| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-09-18 | |||
| 10:35:21 | sean-k-mooney | i think that should work | |
| 10:35:32 | sean-k-mooney | can you add a patch to try it | |
| 10:35:47 | gibi | stephenfin: boot, attach, stop, start: guest keeps the attached device | |
| 10:35:59 | sean-k-mooney | as i said i know we have had some issues where the live and persistent domains get out of sync | |
| 10:36:15 | stephenfin | weeeeeird | |
| 10:36:23 | stephenfin | might have to ask danpb about this one | |
| 10:36:30 | sean-k-mooney | gibi: start recreated the xml form start so its going to work in the nova cases | |
| 10:36:39 | sean-k-mooney | even if we dont update the persistent domain | |
| 10:37:07 | sean-k-mooney | i kindof wish we did not have a persistnet domain in nova | |
| 10:37:13 | sean-k-mooney | just use teh traisiant one | |
| 10:37:38 | sean-k-mooney | it would remove a subset of bugs | |
| 10:37:43 | stephenfin | sean-k-mooney: ah, that's a fair point | |
| 10:37:58 | lyarwood | sean-k-mooney: I thought that was the plan in W? | |
| 10:38:07 | lyarwood | sean-k-mooney: to look into removing the persistent domain that is | |
| 10:38:13 | stephenfin | I was thinking of reboot in the libvirt sense, where you have to have a persistent domain | |
| 10:38:14 | sean-k-mooney | lyarwood: add it to the ptg/spec list | |
| 10:38:26 | sean-k-mooney | lyarwood: that is what we talked about after that CVE | |
| 10:38:33 | lyarwood | yup doing it now | |
| 10:38:34 | sean-k-mooney | did you bring it up upstream yet | |
| 10:39:20 | sean-k-mooney | lyarwood: stephenfin is looking at the device attach/detach flag we use and tryign to figure out why we pass live/persited true/false in places | |
| 10:39:26 | lyarwood | I thought we had talked about it here but obviously not sorry | |
| 10:39:46 | lyarwood | ah | |
| 10:39:48 | lyarwood | so | |
| 10:39:53 | lyarwood | that's because the initial attempt to detach | |
| 10:40:04 | lyarwood | might only detach from the inactive config | |
| 10:40:24 | lyarwood | after that we only want to try the live config | |
| 10:40:33 | stephenfin | lyarwood: not for PCI devices | |
| 10:40:36 | lyarwood | that's async and can fail if the guestos doesn't work with us | |
| 10:40:53 | stephenfin | we're not using the detach_device_with_retry helper for those | |
| 10:40:54 | stephenfin | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4396 | |
| 10:41:04 | stephenfin | vs https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L2306-L2309 | |
| 10:41:15 | stephenfin | also not for mdev devices https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7512 | |
| 10:41:48 | lyarwood | what's the reason for not using the helper with PCI devices? | |
| 10:41:50 | lyarwood | or mdevs? | |
| 10:42:03 | lyarwood | shouldn't the semantics be the same | |
| 10:42:20 | stephenfin | not a clue :) that's what I was thinking but I can't find proof | |
| 10:42:36 | stephenfin | nor can I find proof that what we do for the other cases is even correct | |
| 10:42:43 | sean-k-mooney | so pci device detach was really only done in suspend until recently | |
| 10:42:44 | lyarwood | yeah I would think they should be the same tbh, virtio-blk devices are PCI devices after all | |
| 10:42:52 | lyarwood | oh right | |
| 10:42:57 | stephenfin | for example, we don't pass the live or persistent flags to the attach_device call for PCI devices | |
| 10:42:58 | stephenfin | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4425 | |
| 10:43:02 | lyarwood | hotunplug wasn't always a thing with PCI passthrough | |
| 10:43:17 | stephenfin | yet we do for other devices like network interfaces https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L2247 | |
| 10:43:23 | stephenfin | why? I don't know | |
| 10:43:38 | sean-k-mooney | for pci passthough its still not a thing for neutron sriov port it partly worked btu it never freed the devices | |
| 10:43:43 | sean-k-mooney | in the pci tracker | |
| 10:43:51 | lyarwood | kk then that might be why | |
| 10:44:02 | lyarwood | a single call would be enough if the domain was paused | |
| 10:44:08 | lyarwood | and iirc live == running or paused right | |
| 10:44:21 | stephenfin | yeah | |
| 10:44:27 | sean-k-mooney | a paused domian is runnign yes | |
| 10:44:33 | sean-k-mooney | well live | |
| 10:44:34 | stephenfin | I want to replace that with a call to isActive | |
| 10:44:38 | stephenfin | same thing, fwict | |
| 10:44:48 | sean-k-mooney | its still defiend and qemu is running but the cpus are stopped | |
| 10:45:14 | lyarwood | is this all from the dom.xml(PAUSED) patch btw stephenfin ? | |
| 10:45:30 | stephenfin | yup :( I'm way down this rabbit hole now | |
| 10:45:37 | lyarwood | kk | |
| 10:45:48 | lyarwood | well just to confuse you even more | |
| 10:46:03 | lyarwood | https://review.opendev.org/#/c/749929/ is how we should be doing this | |
| 10:46:06 | lyarwood | eventually | |
| 10:46:30 | lyarwood | that's incomplete but events are apparently a better way of checking when things are actually detached | |
| 10:46:48 | stephenfin | oh, very nice | |
| 10:49:53 | lyarwood | urgh I want to rewrite detach_device_with_retry so much in W | |
| 10:51:42 | lyarwood | stephenfin: so sorry I lost track of what the issue is here now | |
| 10:52:31 | stephenfin | The question is whether we should be setting the live parameter here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4425 | |
| 10:53:19 | stephenfin | gibi's testing suggests it's not necessary, in which case I guess the question is do we need to be setting it here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L2247 | |
| 10:54:21 | sean-k-mooney | stephenfin: if we just remove the persistnt domain then this is a moot point | |
| 10:54:29 | stephenfin | true | |
| 10:55:22 | sean-k-mooney | well maybe not | |
| 10:55:27 | sean-k-mooney | we might need to backport something | |
| 10:55:37 | sean-k-mooney | for issue on older branches | |
| 10:55:43 | lyarwood | stephenfin: yeah odd, I wonder if it's something in https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainAttachDeviceFlags that defaults to attaching it to all configs? | |
| 10:56:17 | stephenfin | VIR_DOMAIN_AFFECT_CURRENT sounds reasonable | |
| 10:56:47 | lyarwood | https://github.com/openstack/nova/blob/c49cd3b9669da3f8da377369442bbe749fcb3c13/nova/virt/libvirt/driver.py#L1921 we also do that for disks FWIW | |
| 10:56:52 | lyarwood | provide persistent and live that is | |
| 10:57:22 | sean-k-mooney | lyarwood: ya i was wondering if currently we shoudl jsut set both to true | |
| 10:57:35 | sean-k-mooney | but was not sure if that would fail in some cases | |
| 10:57:56 | stephenfin | Or drop both flags and use VIR_DOMAIN_AFFECT_CURRENT (modify whatever state the instance is currently in) | |
| 11:01:40 | lyarwood | stephenfin: right but we would need to ensure that would attach or detach from both configs at the moment | |
| 11:01:54 | stephenfin | can the two get out of sync? | |
| 11:01:56 | lyarwood | stephenfin: if we dropped the inactive persistent config then yeah we could just use that | |
| 11:01:58 | lyarwood | stephenfin: yup | |
| 11:02:11 | stephenfin | I thought an instance was either persistent or transient | |
| 11:02:24 | stephenfin | *domain | |
| 11:03:45 | openstack | bugzilla.redhat.com bug 1861142 in openstack-nova "[OSP 13.0.z] Unable to detach volume from instance when previously removed from the inactive config" [High,Verified] - Assigned to lyarwood | |
| 11:03:45 | lyarwood | stephenfin: https://bugzilla.redhat.com/show_bug.cgi?id=1861142 for an example btw | |
| 11:05:23 | stephenfin | fun | |
| 11:05:27 | lyarwood | so much | |
| 11:13:58 | sean-k-mooney | how would people feel about backporting droping the persistent domain? is that doable or no? | |
| 11:14:12 | sean-k-mooney | im not sure how that and rolling upgrades would interact | |
| 11:14:15 | stephenfin | let's get it in master before we even think about | |
| 11:14:16 | stephenfin | that | |
| 11:15:06 | sean-k-mooney | ya im just wondering for some of the downstream bugs. | |
| 11:15:20 | sean-k-mooney | although lyarwood you are fixign that bug via the events right | |
| 11:19:51 | lyarwood | We could for new domains | |
| 11:20:03 | lyarwood | we would just need to retain support for persisted domains for a while | |
| 11:20:07 | lyarwood | but that's the same in master | |
| 11:20:22 | lyarwood | actually I don't know if we could ever drop that support really | |