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@xxxxxxxxxx> > --- > 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@xxxxxxxxxx> John [...] -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
