[libvirt] [PATCH 0/8] vsock hotplug
Followup for: https://bugzilla.redhat.com/show_bug.cgi?id=1291851 Ján Tomko (8): qemu: split out qemuBuildVsockDevStr qemuBuildVsockDevStr: allow passing a fdprefix export virDomainVsockDefFree qemu: export vsock-related functions qemu: implement vsock hotplug conf: introduce virDomainVsockDefEquals qemu: implement vsock hotunplug qemu: implement vsock coldplug/coldunplug src/conf/domain_conf.c | 18 +++++++ src/conf/domain_conf.h | 3 ++ src/libvirt_private.syms | 2 + src/qemu/qemu_alias.c | 2 +- src/qemu/qemu_alias.h | 2 + src/qemu/qemu_command.c | 39 ++++++++++---- src/qemu/qemu_command.h | 8 +++ src/qemu/qemu_driver.c | 36 +++++++++++-- src/qemu/qemu_hotplug.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_hotplug.h | 7 +++ src/qemu/qemu_process.c | 2 +- src/qemu/qemu_process.h | 2 + 12 files changed, 241 insertions(+), 18 deletions(-) -- 2.16.1
Split out the device string building to allow reusal in hotplug. https://bugzilla.redhat.com/show_bug.cgi?id=1291851 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0b5ec4f2ba..3269c08806 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9912,17 +9912,15 @@ qemuBuildSeccompSandboxCommandLine(virCommandPtr cmd, } -static int -qemuBuildVsockCommandLine(virCommandPtr cmd, - virDomainDefPtr def, - virDomainVsockDefPtr vsock, - virQEMUCapsPtr qemuCaps) +static char * +qemuBuildVsockDevStr(virDomainDefPtr def, + virDomainVsockDefPtr vsock, + virQEMUCapsPtr qemuCaps) { qemuDomainVsockPrivatePtr priv = (qemuDomainVsockPrivatePtr)vsock->privateData; - const char *device = "vhost-vsock-pci"; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *devstr = NULL; - int ret = -1; + const char *device = "vhost-vsock-pci"; + char *ret = NULL; virBufferAsprintf(&buf, "%s", device); virBufferAsprintf(&buf, ",id=%s", vsock->info.alias); @@ -9934,7 +9932,26 @@ qemuBuildVsockCommandLine(virCommandPtr cmd, if (virBufferCheckError(&buf) < 0) goto cleanup; - devstr = virBufferContentAndReset(&buf); + ret = virBufferContentAndReset(&buf); + + cleanup: + virBufferFreeAndReset(&buf); + return ret; +} + + +static int +qemuBuildVsockCommandLine(virCommandPtr cmd, + virDomainDefPtr def, + virDomainVsockDefPtr vsock, + virQEMUCapsPtr qemuCaps) +{ + qemuDomainVsockPrivatePtr priv = (qemuDomainVsockPrivatePtr)vsock->privateData; + char *devstr = NULL; + int ret = -1; + + if (!(devstr = qemuBuildVsockDevStr(def, vsock, qemuCaps))) + goto cleanup; virCommandPassFD(cmd, priv->vhostfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); priv->vhostfd = -1; @@ -9942,7 +9959,6 @@ qemuBuildVsockCommandLine(virCommandPtr cmd, ret = 0; cleanup: - virBufferFreeAndReset(&buf); VIR_FREE(devstr); return ret; } -- 2.16.1
On 05/30/2018 10:57 AM, Ján Tomko wrote:
Split out the device string building to allow reusal in hotplug.
s/reusal in hotplug/reuse for hotplug/
https://bugzilla.redhat.com/show_bug.cgi?id=1291851
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
A string prefix for the file descriptor name. Domain startup uses the numeric value of fd without a prefix, but hotplug will need to use a prefix because file descriptor names passed via add-fd cannot start with a number. https://bugzilla.redhat.com/show_bug.cgi?id=1291851 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3269c08806..89cd931de6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9915,7 +9915,8 @@ qemuBuildSeccompSandboxCommandLine(virCommandPtr cmd, static char * qemuBuildVsockDevStr(virDomainDefPtr def, virDomainVsockDefPtr vsock, - virQEMUCapsPtr qemuCaps) + virQEMUCapsPtr qemuCaps, + const char *fdprefix) { qemuDomainVsockPrivatePtr priv = (qemuDomainVsockPrivatePtr)vsock->privateData; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -9925,7 +9926,7 @@ qemuBuildVsockDevStr(virDomainDefPtr def, virBufferAsprintf(&buf, "%s", device); virBufferAsprintf(&buf, ",id=%s", vsock->info.alias); virBufferAsprintf(&buf, ",guest-cid=%u", vsock->guest_cid); - virBufferAsprintf(&buf, ",vhostfd=%u", priv->vhostfd); + virBufferAsprintf(&buf, ",vhostfd=%s%u", fdprefix, priv->vhostfd); if (qemuBuildDeviceAddressStr(&buf, def, &vsock->info, qemuCaps) < 0) goto cleanup; @@ -9950,7 +9951,7 @@ qemuBuildVsockCommandLine(virCommandPtr cmd, char *devstr = NULL; int ret = -1; - if (!(devstr = qemuBuildVsockDevStr(def, vsock, qemuCaps))) + if (!(devstr = qemuBuildVsockDevStr(def, vsock, qemuCaps, ""))) goto cleanup; virCommandPassFD(cmd, priv->vhostfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); -- 2.16.1
$subj: qemu: Add prefix for vsock vhostfd On 05/30/2018 10:57 AM, Ján Tomko wrote:
A string prefix for the file descriptor name. Domain startup
consider: Alter qemuBuildVsockDevStr to allow passing a prefix for the vhostfd file descriptor name.
uses the numeric value of fd without a prefix, but hotplug will need to use a prefix because file descriptor names passed via add-fd cannot start with a number.
This doesn't use add-fd, it uses qemuMonitorAddDeviceWithFd - similar reasoning, just different command. As an aside, qemuMonitorAddDeviceWithFd has a NULLSTR(fdname), but passes fdname to qemuMonitorSendFileHandle which just assumes it's there as does qemuMonitorJSONSendFileHandle ("s:fdname", fdname).
https://bugzilla.redhat.com/show_bug.cgi?id=1291851
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
Slightly different algorithm than VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO and SCSIVHost) but who's to say the other 2 are more correct (at least w/r/t passing the "%d" or "vsockfd%d" string to qemuBuildVsockDevStr instead of building it in the method). Your call on how to handle. w/ commit message cleanup... Reviewed-by: John Ferlan <jferlan@redhat.com> John
https://bugzilla.redhat.com/show_bug.cgi?id=1291851 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt_private.syms | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6001635916..5405250ee9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -574,6 +574,7 @@ virDomainVideoVGAConfTypeFromString; virDomainVideoVGAConfTypeToString; virDomainVirtTypeFromString; virDomainVirtTypeToString; +virDomainVsockDefFree; virDomainVsockDefNew; virDomainWatchdogActionTypeFromString; virDomainWatchdogActionTypeToString; -- 2.16.1
On 05/30/2018 10:57 AM, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1291851
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt_private.syms | 1 + 1 file changed, 1 insertion(+)
I think this should just be merged w/ patch 5 since that's where it's first used... Reviewed-by: John Ferlan <jferlan@redhat.com> John
Export qemuAssignDeviceVsockAlias, qemuBuildVsockDevStr and qemuProcessOpenVhostVsock for reuse in hotplug. https://bugzilla.redhat.com/show_bug.cgi?id=1291851 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_alias.c | 2 +- src/qemu/qemu_alias.h | 2 ++ src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 8 ++++++++ src/qemu/qemu_process.c | 2 +- src/qemu/qemu_process.h | 2 ++ 6 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 89dec91ed1..72cba7f365 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -533,7 +533,7 @@ qemuAssignDeviceInputAlias(virDomainDefPtr def, } -static int +int qemuAssignDeviceVsockAlias(virDomainVsockDefPtr vsock) { if (vsock->info.alias) diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index 51f64624d7..4ca0aaf9a6 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -70,6 +70,8 @@ int qemuAssignDeviceInputAlias(virDomainDefPtr def, virDomainInputDefPtr input, int idx); +int qemuAssignDeviceVsockAlias(virDomainVsockDefPtr vsock); + int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps); int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 89cd931de6..26aeca05bf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9912,7 +9912,7 @@ qemuBuildSeccompSandboxCommandLine(virCommandPtr cmd, } -static char * +char * qemuBuildVsockDevStr(virDomainDefPtr def, virDomainVsockDefPtr vsock, virQEMUCapsPtr qemuCaps, diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index bbbf152660..36bf822414 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -206,4 +206,12 @@ int qemuBuildInputDevStr(char **devstr, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); +char * +qemuBuildVsockDevStr(virDomainDefPtr def, + virDomainVsockDefPtr vsock, + virQEMUCapsPtr qemuCaps, + const char *fdprefix) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4); + #endif /* __QEMU_COMMAND_H__*/ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 30cc5904e0..68960cc1be 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5948,7 +5948,7 @@ qemuProcessPrepareHostStorage(virQEMUDriverPtr driver, } -static int +int qemuProcessOpenVhostVsock(virDomainVsockDefPtr vsock) { qemuDomainVsockPrivatePtr priv = (qemuDomainVsockPrivatePtr)vsock->privateData; diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 5098eacfe8..531c2a0cc7 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -113,6 +113,8 @@ int qemuProcessPrepareDomain(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags); +int qemuProcessOpenVhostVsock(virDomainVsockDefPtr vsock); + int qemuProcessPrepareHost(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags); -- 2.16.1
On 05/30/2018 10:57 AM, Ján Tomko wrote:
Export qemuAssignDeviceVsockAlias, qemuBuildVsockDevStr and qemuProcessOpenVhostVsock for reuse in hotplug.
https://bugzilla.redhat.com/show_bug.cgi?id=1291851
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_alias.c | 2 +- src/qemu/qemu_alias.h | 2 ++ src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 8 ++++++++ src/qemu/qemu_process.c | 2 +- src/qemu/qemu_process.h | 2 ++ 6 files changed, 15 insertions(+), 3 deletions(-)
I think this can be merged with patch 5 too... Reviewed-by: John Ferlan <jferlan@redhat.com> John
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(-) 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; + } + + 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) + goto cleanup; + + if (!(devstr = qemuBuildVsockDevStr(vm->def, vsock, priv->qemuCaps, fdprefix))) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + if (qemuMonitorAddDeviceWithFd(priv->mon, devstr, vsockPriv->vhostfd, fdname) < 0) + goto exit_monitor; + + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + releaseaddr = false; + goto cleanup; + } + + VIR_STEAL_PTR(vm->def->vsock, vsock); + + ret = 0; + + cleanup: + if (ret < 0) { + virErrorPreserveLast(&originalError); + if (releaseaddr) + qemuDomainReleaseDeviceAddress(vm, &vsock->info, NULL); + virErrorRestore(&originalError); + } + + virDomainVsockDefFree(vsock); + VIR_FREE(devstr); + VIR_FREE(fdname); + return ret; + + exit_monitor: + if (qemuDomainObjExitMonitor(driver, vm) < 0) + releaseaddr = false; + goto cleanup; +} + + static int qemuDomainChangeNetBridge(virDomainObjPtr vm, virDomainNetDefPtr olddev, diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 751cbf61d4..ab298382eb 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -139,6 +139,10 @@ int qemuDomainAttachInputDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainInputDefPtr input); +int qemuDomainAttachVsockDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainVsockDefPtr vsock); + int qemuDomainAttachLease(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainLeaseDefPtr lease); -- 2.16.1
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 [...]
On Mon, Jun 04, 2018 at 12:13:07PM -0400, John Ferlan wrote:
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.
Oops, I forgot to add it. I have sent a separate patch adding it to qemuDomainDeviceDefValidate, so that both domain startup and (after this series) hotplug will be able to catch the error. Jano
On Wed, May 30, 2018 at 04:57:54PM +0200, 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(-)
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; + } + + 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) + goto cleanup; + + if (!(devstr = qemuBuildVsockDevStr(vm->def, vsock, priv->qemuCaps, fdprefix))) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + if (qemuMonitorAddDeviceWithFd(priv->mon, devstr, vsockPriv->vhostfd, fdname) < 0) + goto exit_monitor; + + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + releaseaddr = false; + goto cleanup; + } + + VIR_STEAL_PTR(vm->def->vsock, vsock); + + ret = 0; + + cleanup: + if (ret < 0) { + virErrorPreserveLast(&originalError); + if (releaseaddr) + qemuDomainReleaseDeviceAddress(vm, &vsock->info, NULL); + virErrorRestore(&originalError); + } + + virDomainVsockDefFree(vsock);
This free is bogus - on success we consume the pointer and on failure the caller frees the device. I'll remove it before pushing. Jano
+ VIR_FREE(devstr); + VIR_FREE(fdname); + return ret; +
[...]
+ cleanup: + if (ret < 0) { + virErrorPreserveLast(&originalError); + if (releaseaddr) + qemuDomainReleaseDeviceAddress(vm, &vsock->info, NULL); + virErrorRestore(&originalError); + } + + virDomainVsockDefFree(vsock);
This free is bogus - on success we consume the pointer and on failure the caller frees the device.
I'll remove it before pushing.
yeah, right. Saw this too and wondered, started looking for it, then got distracted... Looking again, now I wonder about qemuDomainAttachMemory John
On Tue, Jun 05, 2018 at 07:40:25AM -0400, John Ferlan wrote:
[...]
+ cleanup: + if (ret < 0) { + virErrorPreserveLast(&originalError); + if (releaseaddr) + qemuDomainReleaseDeviceAddress(vm, &vsock->info, NULL); + virErrorRestore(&originalError); + } + + virDomainVsockDefFree(vsock);
This free is bogus - on success we consume the pointer and on failure the caller frees the device.
I'll remove it before pushing.
yeah, right. Saw this too and wondered, started looking for it, then got distracted... Looking again, now I wonder about qemuDomainAttachMemory
From qemuDomainAttachDeviceLive: case VIR_DOMAIN_DEVICE_MEMORY: /* note that qemuDomainAttachMemory always consumes dev->data.memory * and dispatches DeviceAdded event on success */ ret = qemuDomainAttachMemory(driver, vm, dev->data.memory); dev->data.memory = NULL; break; Jano
Introduce a function for comparing two vsock definitions. https://bugzilla.redhat.com/show_bug.cgi?id=1291851 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 22 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 27e2bd50eb..13023d80fd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17592,6 +17592,24 @@ virDomainInputDefFind(const virDomainDef *def, } +bool +virDomainVsockDefEquals(const virDomainVsockDef *a, + const virDomainVsockDef *b) +{ + if (a->model != b->model) + return false; + + if (a->auto_cid != b->auto_cid) + return false; + + if (a->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&a->info, &b->info)) + return false; + + return true; +} + + char * virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6cc8f8a29b..b6c4090ea1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3312,6 +3312,9 @@ virDomainShmemDefPtr virDomainShmemDefRemove(virDomainDefPtr def, size_t idx) ssize_t virDomainInputDefFind(const virDomainDef *def, const virDomainInputDef *input) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +bool virDomainVsockDefEquals(const virDomainVsockDef *a, + const virDomainVsockDef *b) + 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 5405250ee9..b2decc12fb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -574,6 +574,7 @@ virDomainVideoVGAConfTypeFromString; virDomainVideoVGAConfTypeToString; virDomainVirtTypeFromString; virDomainVirtTypeToString; +virDomainVsockDefEquals; virDomainVsockDefFree; virDomainVsockDefNew; virDomainWatchdogActionTypeFromString; -- 2.16.1
On 05/30/2018 10:57 AM, Ján Tomko wrote:
Introduce a function for comparing two vsock definitions.
https://bugzilla.redhat.com/show_bug.cgi?id=1291851
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 22 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
https://bugzilla.redhat.com/show_bug.cgi?id=1291851 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 5 +++- src/qemu/qemu_hotplug.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_hotplug.h | 3 +++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fa94ae9e38..e030a9e095 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7799,6 +7799,10 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, ret = qemuDomainDetachRedirdevDevice(driver, vm, dev->data.redirdev, async); break; + case VIR_DOMAIN_DEVICE_VSOCK: + ret = qemuDomainDetachVsockDevice(vm, dev->data.vsock, async); + break; + case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: @@ -7811,7 +7815,6 @@ qemuDomainDetachDeviceLive(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 detach of device '%s' is not supported"), diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ada120bcfe..970482307d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4590,6 +4590,26 @@ qemuDomainRemoveInputDevice(virDomainObjPtr vm, } +static int +qemuDomainRemoveVsockDevice(virDomainObjPtr vm, + virDomainVsockDefPtr dev) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + virObjectEventPtr event = NULL; + + VIR_DEBUG("Removing vsock device %s from domain %p %s", + dev->info.alias, vm, vm->def->name); + + event = virDomainEventDeviceRemovedNewFromObj(vm, dev->info.alias); + qemuDomainEventQueue(driver, event); + qemuDomainReleaseDeviceAddress(vm, &dev->info, NULL); + virDomainVsockDefFree(vm->def->vsock); + vm->def->vsock = NULL; + return 0; +} + + static int qemuDomainRemoveRedirdevDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4684,6 +4704,10 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver, ret = qemuDomainRemoveWatchdog(driver, vm, dev->data.watchdog); break; + case VIR_DOMAIN_DEVICE_VSOCK: + ret = qemuDomainRemoveVsockDevice(vm, dev->data.vsock); + break; + case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_FS: @@ -4697,7 +4721,6 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver, 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, _("don't know how to remove a %s device"), @@ -6586,3 +6609,46 @@ qemuDomainDetachInputDevice(virDomainObjPtr vm, qemuDomainResetDeviceRemoval(vm); return ret; } + + +int +qemuDomainDetachVsockDevice(virDomainObjPtr vm, + virDomainVsockDefPtr dev, + bool async) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + virDomainVsockDefPtr vsock = vm->def->vsock; + int ret = -1; + + + if (!vsock || + !virDomainVsockDefEquals(dev, vsock)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("matching vsock device not found")); + return -1; + } + + if (!async) + qemuDomainMarkDeviceForRemoval(vm, &vsock->info); + + qemuDomainObjEnterMonitor(driver, vm); + if (qemuMonitorDelDevice(priv->mon, vsock->info.alias)) { + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + goto cleanup; + } + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + + if (async) { + ret = 0; + } else { + if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) + ret = qemuDomainRemoveVsockDevice(vm, vsock); + } + + cleanup: + if (!async) + qemuDomainResetDeviceRemoval(vm); + return ret; +} diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index ab298382eb..0bcccee8fc 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -201,4 +201,7 @@ int qemuDomainDetachInputDevice(virDomainObjPtr vm, virDomainInputDefPtr def, bool async); +int qemuDomainDetachVsockDevice(virDomainObjPtr vm, + virDomainVsockDefPtr dev, + bool async); #endif /* __QEMU_HOTPLUG_H__ */ -- 2.16.1
On 05/30/2018 10:57 AM, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1291851
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 5 +++- src/qemu/qemu_hotplug.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_hotplug.h | 3 +++ 3 files changed, 74 insertions(+), 2 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
https://bugzilla.redhat.com/show_bug.cgi?id=1291851 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e030a9e095..6496fe4719 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8127,6 +8127,15 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, return -1; break; + case VIR_DOMAIN_DEVICE_VSOCK: + if (vmdef->vsock) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain already has a vsock device")); + return -1; + } + VIR_STEAL_PTR(vmdef->vsock, dev->data.vsock); + break; + case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: @@ -8138,7 +8147,6 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, 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, _("persistent attach of device '%s' is not supported"), @@ -8311,6 +8319,17 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, VIR_DELETE_ELEMENT(vmdef->inputs, idx, vmdef->ninputs); break; + case VIR_DOMAIN_DEVICE_VSOCK: + if (!vmdef->vsock || + !virDomainVsockDefEquals(dev->data.vsock, vmdef->vsock)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("matching vsock device not found")); + return -1; + } + virDomainVsockDefFree(vmdef->vsock); + vmdef->vsock = NULL; + break; + case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: @@ -8322,7 +8341,6 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, 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, _("persistent detach of device '%s' is not supported"), -- 2.16.1
On 05/30/2018 10:57 AM, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1291851
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
participants (2)
-
John Ferlan -
Ján Tomko