On 05/30/2018 10:57 AM, Ján Tomko wrote:
Allow hotplugging the vsock device.
https://bugzilla.redhat.com/show_bug.cgi?id=1291851
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 9 ++++++- src/qemu/qemu_hotplug.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 4 +++ 3 files changed, 82 insertions(+), 1 deletion(-)
Why is QEMU_CAPS_DEVICE_VHOST_VSOCK never checked in the command line startup code? Was that forgotten or just felt not needed.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3aa694de12..fa94ae9e38 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7690,6 +7690,14 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, } break;
+ case VIR_DOMAIN_DEVICE_VSOCK: + ret = qemuDomainAttachVsockDevice(driver, vm, dev->data.vsock); + if (ret == 0) { + alias = dev->data.vsock->info.alias; + dev->data.vsock = NULL; + } + break; + case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_SOUND: @@ -7702,7 +7710,6 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_TPM: case VIR_DOMAIN_DEVICE_PANIC: case VIR_DOMAIN_DEVICE_IOMMU: - case VIR_DOMAIN_DEVICE_VSOCK: case VIR_DOMAIN_DEVICE_LAST: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("live attach of device '%s' is not supported"), diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b4bbe62c75..ada120bcfe 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3015,6 +3015,76 @@ qemuDomainAttachInputDevice(virQEMUDriverPtr driver, }
+int +qemuDomainAttachVsockDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainVsockDefPtr vsock) +{ + qemuDomainVsockPrivatePtr vsockPriv = (qemuDomainVsockPrivatePtr)vsock->privateData; + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_VSOCK, + { .vsock = vsock } }; + virErrorPtr originalError = NULL; + const char *fdprefix = "vsockfd"; + bool releaseaddr = false; + char *fdname = NULL; + char *devstr = NULL; + int ret = -1; + + if (vm->def->vsock) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("the domain already has a vsock device")); + return -1; + }
Theoretically, shouldn't this code check if QEMU_CAPS_DEVICE_VHOST_VSOCK is available for the domain? qemuDomainAttachSCSIVHostDevice and qemuDomainAttachHostPCIDevice (other consumers of qemuMonitorAddDeviceWithFd have capabilities checks...)
+ + if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev, "vsock") < 0) + return -1; + + if (qemuAssignDeviceVsockAlias(vsock) < 0) + goto cleanup; + + if (qemuProcessOpenVhostVsock(vsock) < 0) + goto cleanup; + + if (virAsprintf(&fdname, "%s%u", fdprefix, vsockPriv->vhostfd) < 0)
Should this be similar to others using "fd-%d" and "vhost-%d"... IDC, but this is why I made the comment in patch 2 review. With at least the capability check added or an explanation why it doesn't need to be, Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]