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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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;
>+
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[...]
>> + 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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.