On a Tuesday in 2022, Michal Privoznik wrote:
A <hostdev/> can have <address type='unassigned'/> which means libvirt manages the device detach from/reattach to the host but the device is never exposed to the guest. This means that we have to take a shortcut during hotunplug (e.g. never ask QEMU on the monitor to detach the device, or never wait for DEVICE_DELETED event). Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- src/qemu/qemu_hotplug.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b998b51f5a..bde5f683d8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -6043,6 +6043,7 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, { virDomainDeviceDef detach = { .type = match->type }; virDomainDeviceInfo *info = NULL; + bool unassigned = false; int ret = -1; switch ((virDomainDeviceType)match->type) { @@ -6181,6 +6182,8 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, return -1; } + unassigned = info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED; + if (qemuIsMultiFunctionDevice(vm->def, info)) { virReportError(VIR_ERR_OPERATION_FAILED, _("cannot hot unplug %s device with multifunction PCI guest address: " @@ -6225,8 +6228,10 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, * Issue the qemu monitor command to delete the device (based on * its alias), and optionally wait a short time in case the * DEVICE_DELETED event arrives from qemu right away. + * Unassigned devices are not exposed to QEMU, so mimic + * !async case. */ - if (!async) + if (!async || unassigned) qemuDomainMarkDeviceForRemoval(vm, info);
This does not seem right - we won't be waiting for an event from QEMU so we don't need to mark the alias for unassigned devices.
if (qemuDomainDeleteDevice(vm, info->alias) < 0) {
qemuDomainDeleteDevice is the one that calls 'device_del' on the monitor, it should not be called for unassigned devices.
@@ -6235,11 +6240,13 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, goto cleanup; } - if (async) { + if (async && !unassigned) { ret = 0; } else { - if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) + if (unassigned || + (ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) { ret = qemuDomainRemoveDevice(driver, vm, &detach);
RemoveDevice is the only interesting function. Instead of prefixing each line of code with if (unassigned) or if (!unassigned) depending on whether it makes sense for unassigned hostdevs, it would be more readable to call qemuDomainRemoveDevice earlier and return early. Jano
+ } } cleanup: -- 2.34.1
Attachment:
signature.asc
Description: PGP signature