[libvirt] [PATCH 00/12] qemu: input device hotplug
https://bugzilla.redhat.com/show_bug.cgi?id=1379603 Ján Tomko (12): conf: audit passthrough input devices at domain startup qemuDomainAttachControllerDevice: remove dead code qemuDomainAttachRNGDevice: do not access source.file randomly Move qemuCheckCCWS390AddressSupport to qemu_domain Split out qemuDomainEnsureVirtioAddress Use qemuDomainEnsureVirtioAddress where possible qemu: allow coldplugging input devices qemu: allow cold unplugging of input devices split out qemuAssignDeviceInputAlias Introduce qemuBuildInputDevStr qemu: implement input device hotplug qemu: implement input device hotunplug src/conf/domain_audit.c | 44 ++++++++ src/conf/domain_audit.h | 5 + src/conf/domain_conf.c | 37 +++++++ src/conf/domain_conf.h | 3 + src/libvirt_private.syms | 3 + src/qemu/qemu_alias.c | 24 ++++- src/qemu/qemu_alias.h | 3 + src/qemu/qemu_command.c | 82 +++++--------- src/qemu/qemu_command.h | 12 ++- src/qemu/qemu_domain.c | 40 +++++++ src/qemu/qemu_domain.h | 5 + src/qemu/qemu_domain_address.c | 45 ++++++++ src/qemu/qemu_domain_address.h | 4 + src/qemu/qemu_driver.c | 25 ++++- src/qemu/qemu_hotplug.c | 239 +++++++++++++++++++++++++++-------------- src/qemu/qemu_hotplug.h | 8 ++ 16 files changed, 436 insertions(+), 143 deletions(-) -- 2.13.0
Introduce virDomainAuditInput and use it to log the evdev passed to the guest. --- src/conf/domain_audit.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_audit.h | 5 +++++ 2 files changed, 49 insertions(+) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 4afc22019..723c73736 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -868,6 +868,9 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) for (i = 0; i < vm->def->nshmems; i++) virDomainAuditShmem(vm, vm->def->shmems[i], "start", true); + for (i = 0; i < vm->def->ninputs; i++) + virDomainAuditInput(vm, vm->def->inputs[i], "start", true); + virDomainAuditMemory(vm, 0, virDomainDefGetMemoryTotal(vm->def), "start", true); virDomainAuditVcpu(vm, 0, virDomainDefGetVcpus(vm->def), "start", true); @@ -983,3 +986,44 @@ virDomainAuditShmem(virDomainObjPtr vm, VIR_FREE(shmpath); return; } + + +void +virDomainAuditInput(virDomainObjPtr vm, + virDomainInputDefPtr input, + const char *reason, + bool success) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *vmname; + const char *virt = virDomainVirtTypeToString(vm->def->virtType); + + virUUIDFormat(vm->def->uuid, uuidstr); + + if (!(vmname = virAuditEncode("vm", vm->def->name))) + goto no_memory; + + switch ((virDomainInputType) input->type) { + case VIR_DOMAIN_INPUT_TYPE_MOUSE: + case VIR_DOMAIN_INPUT_TYPE_TABLET: + case VIR_DOMAIN_INPUT_TYPE_KBD: + break; + + case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: + VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, + "virt=%s resrc=evdev reason=%s %s uuid=%s path=%s", + virt, reason, vmname, uuidstr, VIR_AUDIT_STR(input->source.evdev)); + break; + + case VIR_DOMAIN_INPUT_TYPE_LAST: + break; + } + + cleanup: + VIR_FREE(vmname); + return; + + no_memory: + VIR_WARN("OOM while encoding audit message"); + goto cleanup; +} diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 8cb585dc7..474ccb6b8 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -133,6 +133,11 @@ void virDomainAuditShmem(virDomainObjPtr vm, virDomainShmemDefPtr def, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +void virDomainAuditInput(virDomainObjPtr vm, + virDomainInputDefPtr input, + const char *reason, + bool success) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); #endif /* __VIR_DOMAIN_AUDIT_H__ */ -- 2.13.0
After a successful attach, the device address has already been set. Remove the pointless assignment. --- src/qemu/qemu_hotplug.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0288986d8..2d13c2891 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -590,11 +590,8 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, goto cleanup; } - if (ret == 0) { - if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) - controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + if (ret == 0) virDomainControllerInsertPreAlloced(vm->def, controller); - } cleanup: if (ret != 0 && releaseaddr) -- 2.13.0
We pass the source.file to qemuCheckCCWS390AddressSupport for the purpose of reporting an error message without actually checking that the rng device is of type VIR_DOMAIN_RNG_BACKEND_RANDOM. Change it to a hardcoded "rng" string, which also avoids referring to the device by a host-side attribute. --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2d13c2891..1e7931a84 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2159,7 +2159,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, } } else { if (!qemuCheckCCWS390AddressSupport(vm->def, rng->info, priv->qemuCaps, - rng->source.file)) + "rng")) goto cleanup; } -- 2.13.0
Let it be reused in qemu_domain_address. --- src/qemu/qemu_command.c | 40 ---------------------------------------- src/qemu/qemu_command.h | 5 ----- src/qemu/qemu_domain.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 5 +++++ 4 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f68b82d08..138bbdf1a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1263,46 +1263,6 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk) } -/* Check whether the device address is using either 'ccw' or default s390 - * address format and whether that's "legal" for the current qemu and/or - * guest os.machine type. This is the corollary to the code which doesn't - * find the address type set using an emulator that supports either 'ccw' - * or s390 and sets the address type based on the capabilities. - * - * If the address is using 'ccw' or s390 and it's not supported, generate - * an error and return false; otherwise, return true. - */ -bool -qemuCheckCCWS390AddressSupport(const virDomainDef *def, - virDomainDeviceInfo info, - virQEMUCapsPtr qemuCaps, - const char *devicename) -{ - if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { - if (!qemuDomainIsS390CCW(def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot use CCW address type for device " - "'%s' using machine type '%s'"), - devicename, def->os.machine); - return false; - } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("CCW address type is not supported by " - "this QEMU")); - return false; - } - } else if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio S390 address type is not supported by " - "this QEMU")); - return false; - } - } - return true; -} - - /* QEMU 1.2 and later have a binary flag -enable-fips that must be * used for VNC auth to obey FIPS settings; but the flag only * exists on Linux, and with no way to probe for it via QMP. Our diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 94e4592cc..1254ad4df 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -189,11 +189,6 @@ int qemuCheckDiskConfig(virDomainDiskDefPtr disk); bool qemuCheckFips(void); -bool qemuCheckCCWS390AddressSupport(const virDomainDef *def, - virDomainDeviceInfo info, - virQEMUCapsPtr qemuCaps, - const char *devicename); - virJSONValuePtr qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 05e8b96aa..f6eb4277a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10122,3 +10122,43 @@ qemuDomainGetMachineName(virDomainObjPtr vm) return ret; } + + +/* Check whether the device address is using either 'ccw' or default s390 + * address format and whether that's "legal" for the current qemu and/or + * guest os.machine type. This is the corollary to the code which doesn't + * find the address type set using an emulator that supports either 'ccw' + * or s390 and sets the address type based on the capabilities. + * + * If the address is using 'ccw' or s390 and it's not supported, generate + * an error and return false; otherwise, return true. + */ +bool +qemuCheckCCWS390AddressSupport(const virDomainDef *def, + virDomainDeviceInfo info, + virQEMUCapsPtr qemuCaps, + const char *devicename) +{ + if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + if (!qemuDomainIsS390CCW(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot use CCW address type for device " + "'%s' using machine type '%s'"), + devicename, def->os.machine); + return false; + } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("CCW address type is not supported by " + "this QEMU")); + return false; + } + } else if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio S390 address type is not supported by " + "this QEMU")); + return false; + } + } + return true; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 5201c6a0a..6abefc929 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -978,4 +978,9 @@ qemuDomainFixupCPUs(virDomainObjPtr vm, char * qemuDomainGetMachineName(virDomainObjPtr vm); +bool qemuCheckCCWS390AddressSupport(const virDomainDef *def, + virDomainDeviceInfo info, + virQEMUCapsPtr qemuCaps, + const char *devicename); + #endif /* __QEMU_DOMAIN_H__ */ -- 2.13.0
On 10/17/2017 11:04 AM, Ján Tomko wrote:
Let it be reused in qemu_domain_address.
Alternatively you could have added "#include qemu_command.h" to qemu_domain_address.c, right? IDC about moving, but if you do....
--- src/qemu/qemu_command.c | 40 ---------------------------------------- src/qemu/qemu_command.h | 5 ----- src/qemu/qemu_domain.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 5 +++++ 4 files changed, 45 insertions(+), 45 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f68b82d08..138bbdf1a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1263,46 +1263,6 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk) }
-/* Check whether the device address is using either 'ccw' or default s390 - * address format and whether that's "legal" for the current qemu and/or - * guest os.machine type. This is the corollary to the code which doesn't - * find the address type set using an emulator that supports either 'ccw' - * or s390 and sets the address type based on the capabilities. - * - * If the address is using 'ccw' or s390 and it's not supported, generate - * an error and return false; otherwise, return true. - */ -bool -qemuCheckCCWS390AddressSupport(const virDomainDef *def, - virDomainDeviceInfo info, - virQEMUCapsPtr qemuCaps, - const char *devicename) -{ - if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { - if (!qemuDomainIsS390CCW(def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot use CCW address type for device " - "'%s' using machine type '%s'"), - devicename, def->os.machine); - return false; - } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("CCW address type is not supported by " - "this QEMU")); - return false; - } - } else if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio S390 address type is not supported by " - "this QEMU")); - return false; - } - } - return true; -} - - /* QEMU 1.2 and later have a binary flag -enable-fips that must be * used for VNC auth to obey FIPS settings; but the flag only * exists on Linux, and with no way to probe for it via QMP. Our diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 94e4592cc..1254ad4df 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -189,11 +189,6 @@ int qemuCheckDiskConfig(virDomainDiskDefPtr disk); bool qemuCheckFips(void);
-bool qemuCheckCCWS390AddressSupport(const virDomainDef *def, - virDomainDeviceInfo info, - virQEMUCapsPtr qemuCaps, - const char *devicename); - virJSONValuePtr qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu) ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 05e8b96aa..f6eb4277a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10122,3 +10122,43 @@ qemuDomainGetMachineName(virDomainObjPtr vm)
return ret; } + + +/* Check whether the device address is using either 'ccw' or default s390 + * address format and whether that's "legal" for the current qemu and/or + * guest os.machine type. This is the corollary to the code which doesn't + * find the address type set using an emulator that supports either 'ccw' + * or s390 and sets the address type based on the capabilities. + * + * If the address is using 'ccw' or s390 and it's not supported, generate + * an error and return false; otherwise, return true. + */ +bool +qemuCheckCCWS390AddressSupport(const virDomainDef *def, + virDomainDeviceInfo info, + virQEMUCapsPtr qemuCaps, + const char *devicename)
... This should be renamed to qemuDomainCheckCCWS390AddressSupport since it's in qemu_domain.c
+{ + if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + if (!qemuDomainIsS390CCW(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot use CCW address type for device " + "'%s' using machine type '%s'"), + devicename, def->os.machine); + return false; + } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("CCW address type is not supported by " + "this QEMU")); + return false; + } + } else if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio S390 address type is not supported by " + "this QEMU")); + return false; + } + } + return true; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 5201c6a0a..6abefc929 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -978,4 +978,9 @@ qemuDomainFixupCPUs(virDomainObjPtr vm, char * qemuDomainGetMachineName(virDomainObjPtr vm);
+bool qemuCheckCCWS390AddressSupport(const virDomainDef *def, + virDomainDeviceInfo info, + virQEMUCapsPtr qemuCaps, + const char *devicename);
More recently I've been encouraged by some reviewers to use: bool qemuDomainCheck...(); type format to follow the .c file format when adding functions. John
+ #endif /* __QEMU_DOMAIN_H__ */
On Thu, Oct 19, 2017 at 08:08:34AM -0400, John Ferlan wrote:
On 10/17/2017 11:04 AM, Ján Tomko wrote:
Let it be reused in qemu_domain_address.
Alternatively you could have added "#include qemu_command.h" to qemu_domain_address.c, right?
The function does not deal directly with the command line and including qemu_command.h would feel wrong.
IDC about moving, but if you do....
[...]
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 5201c6a0a..6abefc929 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -978,4 +978,9 @@ qemuDomainFixupCPUs(virDomainObjPtr vm, char * qemuDomainGetMachineName(virDomainObjPtr vm);
+bool qemuCheckCCWS390AddressSupport(const virDomainDef *def, + virDomainDeviceInfo info, + virQEMUCapsPtr qemuCaps, + const char *devicename);
More recently I've been encouraged by some reviewers to use:
bool qemuDomainCheck...();
type format to follow the .c file format when adding functions.
I purposefully formatted it this way to match most of the prototypes in the file. Jan
John
+ #endif /* __QEMU_DOMAIN_H__ */
Split out the common code responsible for reserving/assigning PCI/CCW addresses for virtio disks into a helper function for reuse by other virtio devices. --- src/qemu/qemu_domain_address.c | 45 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain_address.h | 4 ++++ src/qemu/qemu_hotplug.c | 29 +++------------------------ 3 files changed, 52 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index c4485d4ab..ebe9eb861 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2904,3 +2904,48 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, VIR_WARN("Unable to release USB address on %s", NULLSTR(devstr)); } + + +int +qemuDomainEnsureVirtioAddress(bool *releaseAddr, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + const char *devicename) +{ + virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev); + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainCCWAddressSetPtr ccwaddrs = NULL; + virQEMUDriverPtr driver = priv->driver; + int ret = -1; + + if (!info->type) { + if (qemuDomainIsS390CCW(vm->def) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) + info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; + else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) + info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390; + } else { + if (!qemuCheckCCWS390AddressSupport(vm->def, *info, priv->qemuCaps, + devicename)) + return -1; + } + + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) + goto cleanup; + if (virDomainCCWAddressAssign(info, ccwaddrs, + !info->addr.ccw.assigned) < 0) + goto cleanup; + } else if (!info->type || + info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (qemuDomainEnsurePCIAddress(vm, dev, driver) < 0) + goto cleanup; + *releaseAddr = true; + } + + ret = 0; + + cleanup: + virDomainCCWAddressSetFree(ccwaddrs); + return ret; +} diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h index b5644fa9c..e951a4c88 100644 --- a/src/qemu/qemu_domain_address.h +++ b/src/qemu/qemu_domain_address.h @@ -59,6 +59,10 @@ qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def) int qemuDomainAssignMemoryDeviceSlot(virDomainDefPtr def, virDomainMemoryDefPtr mem); +int qemuDomainEnsureVirtioAddress(bool *releaseAddr, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + const char *devicename); # define __QEMU_DOMAIN_ADDRESS_H__ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1e7931a84..177c8a01d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -363,7 +363,6 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, bool driveAdded = false; bool secobjAdded = false; bool encobjAdded = false; - virDomainCCWAddressSetPtr ccwaddrs = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); virJSONValuePtr secobjProps = NULL; @@ -372,33 +371,12 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, qemuDomainSecretInfoPtr secinfo; qemuDomainSecretInfoPtr encinfo; - if (!disk->info.type) { - if (qemuDomainIsS390CCW(vm->def) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) - disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; - else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) - disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390; - } else { - if (!qemuCheckCCWS390AddressSupport(vm->def, disk->info, priv->qemuCaps, - disk->dst)) - goto cleanup; - } - if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0) goto cleanup; - if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { - if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) - goto error; - if (virDomainCCWAddressAssign(&disk->info, ccwaddrs, - !disk->info.addr.ccw.assigned) < 0) - goto error; - } else if (!disk->info.type || - disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) - goto error; - } - releaseaddr = true; + if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev, disk->dst) < 0) + goto error; + if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error; @@ -477,7 +455,6 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, virJSONValueFree(secobjProps); virJSONValueFree(encobjProps); qemuDomainSecretDiskDestroy(disk); - virDomainCCWAddressSetFree(ccwaddrs); VIR_FREE(devstr); VIR_FREE(drivestr); VIR_FREE(drivealias); -- 2.13.0
On 10/17/2017 11:04 AM, Ján Tomko wrote:
Split out the common code responsible for reserving/assigning PCI/CCW addresses for virtio disks into a helper function for reuse by other virtio devices. --- src/qemu/qemu_domain_address.c | 45 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain_address.h | 4 ++++ src/qemu/qemu_hotplug.c | 29 +++------------------------ 3 files changed, 52 insertions(+), 26 deletions(-)
Not an issue - just a note from review...
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index c4485d4ab..ebe9eb861 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2904,3 +2904,48 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, VIR_WARN("Unable to release USB address on %s", NULLSTR(devstr)); } + + +int +qemuDomainEnsureVirtioAddress(bool *releaseAddr, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + const char *devicename) +{ + virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev); + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainCCWAddressSetPtr ccwaddrs = NULL; + virQEMUDriverPtr driver = priv->driver; + int ret = -1; + + if (!info->type) { + if (qemuDomainIsS390CCW(vm->def) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) + info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; + else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) + info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390; + } else { + if (!qemuCheckCCWS390AddressSupport(vm->def, *info, priv->qemuCaps, + devicename)) + return -1; + } + + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) + goto cleanup; + if (virDomainCCWAddressAssign(info, ccwaddrs, + !info->addr.ccw.assigned) < 0) + goto cleanup; + } else if (!info->type || + info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (qemuDomainEnsurePCIAddress(vm, dev, driver) < 0) + goto cleanup; + *releaseAddr = true;
This is only setting *releaseAddr in this instance; whereas, the previous code would also set it for info->type CCW [1] Still looking at how @releaseaddr is used, we see that it's used to call qemuDomainReleaseDeviceAddress which only cares about PCI and USB, so I suppose this is fine - just different.
+ } + + ret = 0; + + cleanup: + virDomainCCWAddressSetFree(ccwaddrs); + return ret; +} diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h index b5644fa9c..e951a4c88 100644 --- a/src/qemu/qemu_domain_address.h +++ b/src/qemu/qemu_domain_address.h @@ -59,6 +59,10 @@ qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def) int qemuDomainAssignMemoryDeviceSlot(virDomainDefPtr def, virDomainMemoryDefPtr mem);
+int qemuDomainEnsureVirtioAddress(bool *releaseAddr, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + const char *devicename);
# define __QEMU_DOMAIN_ADDRESS_H__
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1e7931a84..177c8a01d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -363,7 +363,6 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, bool driveAdded = false; bool secobjAdded = false; bool encobjAdded = false; - virDomainCCWAddressSetPtr ccwaddrs = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); virJSONValuePtr secobjProps = NULL; @@ -372,33 +371,12 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, qemuDomainSecretInfoPtr secinfo; qemuDomainSecretInfoPtr encinfo;
- if (!disk->info.type) { - if (qemuDomainIsS390CCW(vm->def) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) - disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; - else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) - disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390; - } else { - if (!qemuCheckCCWS390AddressSupport(vm->def, disk->info, priv->qemuCaps, - disk->dst)) - goto cleanup; - } - if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0) goto cleanup;
- if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { - if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) - goto error; - if (virDomainCCWAddressAssign(&disk->info, ccwaddrs, - !disk->info.addr.ccw.assigned) < 0) - goto error; - } else if (!disk->info.type || - disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) - goto error; - } - releaseaddr = true;
[1] @releaseaddr is set for both CCW and PCI here.
+ if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev, disk->dst) < 0) + goto error; + if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error;
@@ -477,7 +455,6 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, virJSONValueFree(secobjProps); virJSONValueFree(encobjProps); qemuDomainSecretDiskDestroy(disk); - virDomainCCWAddressSetFree(ccwaddrs); VIR_FREE(devstr); VIR_FREE(drivestr); VIR_FREE(drivealias);
There are two more cases where we set an S390/CCW/PCI address type based on the machine type. Reuse qemuDomainEnsureVirtioAddress to reduce repetition. --- src/qemu/qemu_hotplug.c | 56 ++++--------------------------------------------- 1 file changed, 4 insertions(+), 52 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 177c8a01d..c65e7e500 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -498,7 +498,6 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_CONTROLLER, { .controller = controller } }; - virDomainCCWAddressSetPtr ccwaddrs = NULL; bool releaseaddr = false; if (controller->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { @@ -523,30 +522,9 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, return -1; } - if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (qemuDomainIsS390CCW(vm->def) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) - controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; - else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) - controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390; - } else { - if (!qemuCheckCCWS390AddressSupport(vm->def, controller->info, - priv->qemuCaps, "controller")) - goto cleanup; - } + if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev, "controller") < 0) + return -1; - if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE || - controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) - goto cleanup; - } else if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { - if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) - goto cleanup; - if (virDomainCCWAddressAssign(&controller->info, ccwaddrs, - !controller->info.addr.ccw.assigned) < 0) - goto cleanup; - } - releaseaddr = true; if (qemuAssignDeviceControllerAlias(vm->def, priv->qemuCaps, controller) < 0) goto cleanup; @@ -575,7 +553,6 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, qemuDomainReleaseDeviceAddress(vm, &controller->info, NULL); VIR_FREE(devstr); - virDomainCCWAddressSetFree(ccwaddrs); return ret; } @@ -2115,7 +2092,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, bool chardevAdded = false; bool objAdded = false; virJSONValuePtr props = NULL; - virDomainCCWAddressSetPtr ccwaddrs = NULL; const char *type; int ret = -1; int rv; @@ -2127,31 +2103,8 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, if (VIR_REALLOC_N(vm->def->rngs, vm->def->nrngs + 1) < 0) goto cleanup; - if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (qemuDomainIsS390CCW(vm->def) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { - rng->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; - } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) { - rng->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390; - } - } else { - if (!qemuCheckCCWS390AddressSupport(vm->def, rng->info, priv->qemuCaps, - "rng")) - goto cleanup; - } - - if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE || - rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) - goto cleanup; - } else if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { - if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) - goto cleanup; - if (virDomainCCWAddressAssign(&rng->info, ccwaddrs, - !rng->info.addr.ccw.assigned) < 0) - goto cleanup; - } - releaseaddr = true; + if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev, "rng") < 0) + return -1; if (qemuDomainNamespaceSetupRNG(driver, vm, rng) < 0) goto cleanup; @@ -2226,7 +2179,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, VIR_FREE(charAlias); VIR_FREE(objAlias); VIR_FREE(devstr); - virDomainCCWAddressSetFree(ccwaddrs); return ret; exit_monitor: -- 2.13.0
https://bugzilla.redhat.com/show_bug.cgi?id=1379603 --- src/qemu/qemu_driver.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fb4d72236..050a41524 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8060,6 +8060,10 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, break; case VIR_DOMAIN_DEVICE_INPUT: + if (VIR_APPEND_ELEMENT(vmdef->inputs, vmdef->ninputs, dev->data.input) < 0) + return -1; + break; + case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: -- 2.13.0
https://bugzilla.redhat.com/show_bug.cgi?id=1379603 --- src/conf/domain_conf.c | 37 +++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 8 ++++++++ 4 files changed, 49 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 516f9fa06..495d85f90 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16556,6 +16556,43 @@ virDomainShmemDefRemove(virDomainDefPtr def, } +static bool +virDomainInputDefEquals(const virDomainInputDef *a, + const virDomainInputDef *b) +{ + if (a->type != b->type) + return false; + + if (a->bus != b->bus) + return false; + + if (a->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH && + STRNEQ_NULLABLE(a->source.evdev, b->source.evdev)) + return false; + + if (a->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&a->info, &b->info)) + return false; + + return true; +} + + +ssize_t +virDomainInputDefFind(const virDomainDef *def, + const virDomainInputDef *input) +{ + size_t i; + + for (i = 0; i < def->ninputs; i++) { + if (virDomainInputDefEquals(input, def->inputs[i])) + return i; + } + + return -1; +} + + char * virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a42efcfa6..c210bd418 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3204,6 +3204,9 @@ ssize_t virDomainShmemDefFind(virDomainDefPtr def, virDomainShmemDefPtr shmem) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; virDomainShmemDefPtr virDomainShmemDefRemove(virDomainDefPtr def, size_t idx) ATTRIBUTE_NONNULL(1); +ssize_t virDomainInputDefFind(const virDomainDef *def, + const virDomainInputDef *input) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; VIR_ENUM_DECL(virDomainTaint) VIR_ENUM_DECL(virDomainVirt) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7bd21ae23..7713ecc01 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -385,6 +385,7 @@ virDomainHubTypeFromString; virDomainHubTypeToString; virDomainHypervTypeFromString; virDomainHypervTypeToString; +virDomainInputDefFind; virDomainInputDefFree; virDomainIOMMUModelTypeFromString; virDomainIOMMUModelTypeToString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 050a41524..092df673c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8239,6 +8239,14 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, break; case VIR_DOMAIN_DEVICE_INPUT: + if ((idx = virDomainInputDefFind(vmdef, dev->data.input)) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("matching input device not found")); + return -1; + } + VIR_DELETE_ELEMENT(vmdef->inputs, idx, vmdef->ninputs); + break; + case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: -- 2.13.0
Move assignment of input device alias into a separate function, for reuse on hotplug. --- src/qemu/qemu_alias.c | 24 +++++++++++++++++++++++- src/qemu/qemu_alias.h | 3 +++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 72df1083f..737fc2fda 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -412,6 +412,28 @@ qemuAssignDeviceWatchdogAlias(virDomainWatchdogDefPtr watchdog) if (VIR_STRDUP(watchdog->info.alias, "watchdog0") < 0) return -1; + + return 0; +} + +int +qemuAssignDeviceInputAlias(virDomainDefPtr def, + virDomainInputDefPtr input, + int idx) +{ + if (idx == -1) { + int thisidx; + size_t i; + + for (i = 0; i < def->ninputs; i++) { + if ((thisidx = qemuDomainDeviceAliasIndex(&def->inputs[i]->info, "input")) >= idx) + idx = thisidx + 1; + } + } + + if (virAsprintf(&input->info.alias, "input%d", idx) < 0) + return -1; + return 0; } @@ -461,7 +483,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) return -1; } for (i = 0; i < def->ninputs; i++) { - if (virAsprintf(&def->inputs[i]->info.alias, "input%zu", i) < 0) + if (qemuAssignDeviceInputAlias(def, def->inputs[i], i) < 0) return -1; } for (i = 0; i < def->nparallels; i++) { diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index 652ffea0c..35424829f 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -66,6 +66,9 @@ int qemuAssignDeviceShmemAlias(virDomainDefPtr def, int idx); int qemuAssignDeviceWatchdogAlias(virDomainWatchdogDefPtr watchdog); +int qemuAssignDeviceInputAlias(virDomainDefPtr def, + virDomainInputDefPtr input, + int idx); int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps); -- 2.13.0
On 10/17/2017 11:04 AM, Ján Tomko wrote:
Move assignment of input device alias into a separate function, for reuse on hotplug. --- src/qemu/qemu_alias.c | 24 +++++++++++++++++++++++- src/qemu/qemu_alias.h | 3 +++ 2 files changed, 26 insertions(+), 1 deletion(-)
[...]
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index 652ffea0c..35424829f 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -66,6 +66,9 @@ int qemuAssignDeviceShmemAlias(virDomainDefPtr def, int idx);
int qemuAssignDeviceWatchdogAlias(virDomainWatchdogDefPtr watchdog);
Add an empty line here for readability. John
+int qemuAssignDeviceInputAlias(virDomainDefPtr def, + virDomainInputDefPtr input, + int idx);
int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps);
A function that builds the -device string for input devices. --- src/qemu/qemu_command.c | 42 +++++++++++++++++++++++++++++------------- src/qemu/qemu_command.h | 7 +++++++ 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 138bbdf1a..7bdff85fc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4303,6 +4303,27 @@ qemuBuildUSBInputDevStr(const virDomainDef *def, } +int +qemuBuildInputDevStr(char **devstr, + const virDomainDef *def, + virDomainInputDefPtr input, + virQEMUCapsPtr qemuCaps) +{ + switch (input->bus) { + case VIR_DOMAIN_INPUT_BUS_USB: + if (!(*devstr = qemuBuildUSBInputDevStr(def, input, qemuCaps))) + return -1; + break; + + case VIR_DOMAIN_INPUT_BUS_VIRTIO: + if (!(*devstr = qemuBuildVirtioInputDevStr(def, input, qemuCaps))) + return -1; + break; + } + return 0; +} + + static int qemuBuildInputCommandLine(virCommandPtr cmd, const virDomainDef *def, @@ -4312,22 +4333,17 @@ qemuBuildInputCommandLine(virCommandPtr cmd, for (i = 0; i < def->ninputs; i++) { virDomainInputDefPtr input = def->inputs[i]; + char *devstr = NULL; - if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { - char *optstr; - virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildUSBInputDevStr(def, input, qemuCaps))) - return -1; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } else if (input->bus == VIR_DOMAIN_INPUT_BUS_VIRTIO) { - char *optstr; + if (qemuBuildInputDevStr(&devstr, def, input, qemuCaps) < 0) + return -1; + + if (devstr) { virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildVirtioInputDevStr(def, input, qemuCaps))) - return -1; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); + virCommandAddArg(cmd, devstr); } + + VIR_FREE(devstr); } return 0; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 1254ad4df..0961ec8cb 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -204,4 +204,11 @@ char *qemuBuildWatchdogDevStr(const virDomainDef *def, virDomainWatchdogDefPtr dev, virQEMUCapsPtr qemuCaps); +int qemuBuildInputDevStr(char **devstr, + const virDomainDef *def, + virDomainInputDefPtr input, + virQEMUCapsPtr qemuCaps) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4); + #endif /* __QEMU_COMMAND_H__*/ -- 2.13.0
On 10/17/2017 11:04 AM, Ján Tomko wrote:
A function that builds the -device string for input devices. --- src/qemu/qemu_command.c | 42 +++++++++++++++++++++++++++++------------- src/qemu/qemu_command.h | 7 +++++++ 2 files changed, 36 insertions(+), 13 deletions(-)
[...]
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 1254ad4df..0961ec8cb 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -204,4 +204,11 @@ char *qemuBuildWatchdogDevStr(const virDomainDef *def, virDomainWatchdogDefPtr dev, virQEMUCapsPtr qemuCaps);
+int qemuBuildInputDevStr(char **devstr, + const virDomainDef *def, + virDomainInputDefPtr input, + virQEMUCapsPtr qemuCaps) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4);
Not sure it matters if qemuCaps is NULL or not since virQEMUCapsGet checks anyway. John
+ #endif /* __QEMU_COMMAND_H__*/
For both virtio input devices and USB input devices. https://bugzilla.redhat.com/show_bug.cgi?id=1379603 --- src/libvirt_private.syms | 2 ++ src/qemu/qemu_driver.c | 9 +++++- src/qemu/qemu_hotplug.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 5 ++++ 4 files changed, 88 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7713ecc01..f5cdbeb66 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -157,6 +157,7 @@ virDomainAuditDisk; virDomainAuditFS; virDomainAuditHostdev; virDomainAuditInit; +virDomainAuditInput; virDomainAuditIOThread; virDomainAuditMemory; virDomainAuditNet; @@ -385,6 +386,7 @@ virDomainHubTypeFromString; virDomainHubTypeToString; virDomainHypervTypeFromString; virDomainHypervTypeToString; +virDomainInputBusTypeToString; virDomainInputDefFind; virDomainInputDefFree; virDomainIOMMUModelTypeFromString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 092df673c..75a0e42aa 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7664,9 +7664,16 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, } break; + case VIR_DOMAIN_DEVICE_INPUT: + ret = qemuDomainAttachInputDevice(driver, vm, dev->data.input); + if (ret == 0) { + alias = dev->data.input->info.alias; + dev->data.input = NULL; + } + break; + case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_FS: - case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c65e7e500..b32acb71e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2905,6 +2905,79 @@ qemuDomainAttachWatchdog(virQEMUDriverPtr driver, } +int +qemuDomainAttachInputDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainInputDefPtr input) +{ + int ret = -1; + char *devstr = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_INPUT, + { .input = input } }; + bool releaseaddr = false; + + if (input->bus != VIR_DOMAIN_INPUT_BUS_USB && + input->bus != VIR_DOMAIN_INPUT_BUS_VIRTIO) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("input device on bus '%s' cannot be hot plugged."), + virDomainInputBusTypeToString(input->bus)); + return -1; + } + + if (input->bus == VIR_DOMAIN_INPUT_BUS_VIRTIO) { + if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev, "input") < 0) + return -1; + } else if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { + if (priv->usbaddrs) { + if (virDomainUSBAddressEnsure(priv->usbaddrs, &input->info) < 0) + goto cleanup; + releaseaddr = true; + } + } + + if (qemuAssignDeviceInputAlias(vm->def, input, -1) < 0) + goto cleanup; + + if (qemuBuildInputDevStr(&devstr, vm->def, input, priv->qemuCaps) < 0) + goto cleanup; + + if (VIR_REALLOC_N(vm->def->inputs, vm->def->ninputs + 1) < 0) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + if (qemuMonitorAddDevice(priv->mon, devstr) < 0) + goto exit_monitor; + + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + releaseaddr = false; + goto cleanup; + } + + VIR_APPEND_ELEMENT_COPY_INPLACE(vm->def->inputs, vm->def->ninputs, input); + + ret = 0; + releaseaddr = false; + + audit: + virDomainAuditInput(vm, input, "attach", ret == 0); + + cleanup: + if (releaseaddr) + qemuDomainReleaseDeviceAddress(vm, &input->info, NULL); + + VIR_FREE(devstr); + return ret; + + exit_monitor: + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + releaseaddr = false; + goto cleanup; + } + goto audit; +} + + static int qemuDomainChangeNetBridge(virDomainObjPtr vm, virDomainNetDefPtr olddev, diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 3455832d6..985b7495b 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -125,6 +125,11 @@ int qemuDomainDetachShmemDevice(virQEMUDriverPtr driver, int qemuDomainDetachWatchdog(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainWatchdogDefPtr watchdog); + +int qemuDomainAttachInputDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainInputDefPtr input); + int qemuDomainAttachLease(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainLeaseDefPtr lease); -- 2.13.0
Allow unplugging USB and virtio USB devices. https://bugzilla.redhat.com/show_bug.cgi?id=1379603 --- src/qemu/qemu_driver.c | 4 ++- src/qemu/qemu_hotplug.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 3 ++ 3 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 75a0e42aa..a9d3ba778 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7764,9 +7764,11 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_WATCHDOG: ret = qemuDomainDetachWatchdog(driver, vm, dev->data.watchdog); break; + case VIR_DOMAIN_DEVICE_INPUT: + ret = qemuDomainDetachInputDevice(vm, dev->data.input); + break; case VIR_DOMAIN_DEVICE_FS: - case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b32acb71e..85faa2a46 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4430,6 +4430,31 @@ qemuDomainRemoveWatchdog(virQEMUDriverPtr driver, } +static int +qemuDomainRemoveInputDevice(virDomainObjPtr vm, + virDomainInputDefPtr dev) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + virObjectEventPtr event = NULL; + size_t i; + + VIR_DEBUG("Removing input device %s from domain %p %s", + dev->info.alias, vm, vm->def->name); + + event = virDomainEventDeviceRemovedNewFromObj(vm, dev->info.alias); + qemuDomainEventQueue(driver, event); + for (i = 0; i < vm->def->ninputs; i++) { + if (vm->def->inputs[i] == dev) + break; + } + qemuDomainReleaseDeviceAddress(vm, &dev->info, NULL); + virDomainInputDefFree(vm->def->inputs[i]); + VIR_DELETE_ELEMENT(vm->def->inputs, i, vm->def->ninputs); + return 0; +} + + int qemuDomainRemoveDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -6184,3 +6209,54 @@ qemuDomainSetVcpuInternal(virQEMUDriverPtr driver, virObjectUnref(cfg); return ret; } + + +int +qemuDomainDetachInputDevice(virDomainObjPtr vm, + virDomainInputDefPtr def) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + virDomainInputDefPtr input; + int ret = -1; + int idx; + + if ((idx = virDomainInputDefFind(vm->def, def)) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("matching input device not found")); + return -1; + } + input = vm->def->inputs[idx]; + + switch ((virDomainInputBus) input->bus) { + case VIR_DOMAIN_INPUT_BUS_PS2: + case VIR_DOMAIN_INPUT_BUS_XEN: + case VIR_DOMAIN_INPUT_BUS_PARALLELS: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("input device on bus '%s' cannot be detached"), + virDomainInputBusTypeToString(input->bus)); + return -1; + + case VIR_DOMAIN_INPUT_BUS_LAST: + case VIR_DOMAIN_INPUT_BUS_USB: + case VIR_DOMAIN_INPUT_BUS_VIRTIO: + break; + } + + qemuDomainMarkDeviceForRemoval(vm, &input->info); + + qemuDomainObjEnterMonitor(driver, vm); + if (qemuMonitorDelDevice(priv->mon, input->info.alias)) { + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + goto cleanup; + } + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + + if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) + ret = qemuDomainRemoveInputDevice(vm, input); + + cleanup: + qemuDomainResetDeviceRemoval(vm); + return ret; +} diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 985b7495b..3e0d618e0 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -184,4 +184,7 @@ int qemuDomainSetVcpuInternal(virQEMUDriverPtr driver, virBitmapPtr vcpus, bool state); +int qemuDomainDetachInputDevice(virDomainObjPtr vm, + virDomainInputDefPtr def); + #endif /* __QEMU_HOTPLUG_H__ */ -- 2.13.0
On 10/17/2017 11:04 AM, Ján Tomko wrote:
Allow unplugging USB and virtio USB devices.
https://bugzilla.redhat.com/show_bug.cgi?id=1379603 --- src/qemu/qemu_driver.c | 4 ++- src/qemu/qemu_hotplug.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 3 ++ 3 files changed, 82 insertions(+), 1 deletion(-)
Another aha moment ;-)... No issue here...
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 75a0e42aa..a9d3ba778 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7764,9 +7764,11 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_WATCHDOG: ret = qemuDomainDetachWatchdog(driver, vm, dev->data.watchdog); break; + case VIR_DOMAIN_DEVICE_INPUT: + ret = qemuDomainDetachInputDevice(vm, dev->data.input); + break;
case VIR_DOMAIN_DEVICE_FS: - case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b32acb71e..85faa2a46 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4430,6 +4430,31 @@ qemuDomainRemoveWatchdog(virQEMUDriverPtr driver, }
+static int +qemuDomainRemoveInputDevice(virDomainObjPtr vm, + virDomainInputDefPtr dev) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver;
So none of the Remove* API's need to pass @driver... doh! Never even considered that - the first non-lemming ;-)! John [...]
On Thu, Oct 19, 2017 at 08:11:39AM -0400, John Ferlan wrote:
On 10/17/2017 11:04 AM, Ján Tomko wrote:
[...]
@@ -4430,6 +4430,31 @@ qemuDomainRemoveWatchdog(virQEMUDriverPtr driver, }
+static int +qemuDomainRemoveInputDevice(virDomainObjPtr vm, + virDomainInputDefPtr dev) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver;
So none of the Remove* API's need to pass @driver... doh! Never even considered that - the first non-lemming ;-)!
Not since commit 2e6ecba, but the cleanup is TBD. Jan
John
[...]
On 10/17/2017 11:04 AM, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1379603
Ján Tomko (12): conf: audit passthrough input devices at domain startup qemuDomainAttachControllerDevice: remove dead code qemuDomainAttachRNGDevice: do not access source.file randomly Move qemuCheckCCWS390AddressSupport to qemu_domain Split out qemuDomainEnsureVirtioAddress Use qemuDomainEnsureVirtioAddress where possible qemu: allow coldplugging input devices qemu: allow cold unplugging of input devices split out qemuAssignDeviceInputAlias Introduce qemuBuildInputDevStr qemu: implement input device hotplug qemu: implement input device hotunplug
src/conf/domain_audit.c | 44 ++++++++ src/conf/domain_audit.h | 5 + src/conf/domain_conf.c | 37 +++++++ src/conf/domain_conf.h | 3 + src/libvirt_private.syms | 3 + src/qemu/qemu_alias.c | 24 ++++- src/qemu/qemu_alias.h | 3 + src/qemu/qemu_command.c | 82 +++++--------- src/qemu/qemu_command.h | 12 ++- src/qemu/qemu_domain.c | 40 +++++++ src/qemu/qemu_domain.h | 5 + src/qemu/qemu_domain_address.c | 45 ++++++++ src/qemu/qemu_domain_address.h | 4 + src/qemu/qemu_driver.c | 25 ++++- src/qemu/qemu_hotplug.c | 239 +++++++++++++++++++++++++++-------------- src/qemu/qemu_hotplug.h | 8 ++ 16 files changed, 436 insertions(+), 143 deletions(-)
Noted a couple of issues along the way to clean up for a... Reviewed-by: John Ferlan <jferlan@redhat.com> (series) John
participants (2)
-
John Ferlan -
Ján Tomko