[libvirt] [PATCH v2] qemu: Assign device address before qemuDomainSupportsNicdev

Wang Yechao posted 1 patch 5 years, 3 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1545046234-24667-1-git-send-email-wang.yechao255@zte.com.cn
src/qemu/qemu_hotplug.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
[libvirt] [PATCH v2] qemu: Assign device address before qemuDomainSupportsNicdev
Posted by Wang Yechao 5 years, 3 months ago
qemuDomainSupportsNicdev will check the device address type on
aarch64. If it is invoked before device address assigned, hotadd
vhostuser interface with no address specified will get error.
Let qemuDomainEnsurePCIAddress run before qemuDomainSupportsNicdev.

Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn>
---
v1 patch:
https://www.redhat.com/archives/libvir-list/2018-December/msg00435.html

Changes in v2:
 - do not modify the address type, let qemuDomainEnsurePCIAddress do it.
---
 src/qemu/qemu_hotplug.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8da0233..f25c8db 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1369,6 +1369,25 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
     if (qemuAssignDeviceNetAlias(vm->def, net, -1) < 0)
         goto cleanup;
 
+    if (qemuDomainIsS390CCW(vm->def) &&
+        net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
+        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CCW)) {
+        net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
+        if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def)))
+            goto cleanup;
+        if (virDomainCCWAddressAssign(&net->info, ccwaddrs,
+                                      !net->info.addr.ccw.assigned) < 0)
+            goto cleanup;
+    } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("virtio-s390 net device cannot be hotplugged."));
+        goto cleanup;
+    } else if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) {
+        goto cleanup;
+    }
+
+    releaseaddr = true;
+
     switch (actualType) {
     case VIR_DOMAIN_NET_TYPE_BRIDGE:
     case VIR_DOMAIN_NET_TYPE_NETWORK:
@@ -1503,25 +1522,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
             goto cleanup;
     }
 
-    if (qemuDomainIsS390CCW(vm->def) &&
-        net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
-        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CCW)) {
-        net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
-        if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def)))
-            goto cleanup;
-        if (virDomainCCWAddressAssign(&net->info, ccwaddrs,
-                                      !net->info.addr.ccw.assigned) < 0)
-            goto cleanup;
-    } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("virtio-s390 net device cannot be hotplugged."));
-        goto cleanup;
-    } else if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) {
-        goto cleanup;
-    }
-
-    releaseaddr = true;
-
     if (VIR_ALLOC_N(tapfdName, tapfdSize) < 0 ||
         VIR_ALLOC_N(vhostfdName, vhostfdSize) < 0)
         goto cleanup;
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt][PATCH v2] qemu: Assign device address before qemuDomainSupportsNicdev
Posted by wang.yechao255@zte.com.cn 5 years, 3 months ago
ping this patch.

> qemuDomainSupportsNicdev will check the device address type on
> aarch64. If it is invoked before device address assigned, hotadd
> vhostuser interface with no address specified will get error.
> Let qemuDomainEnsurePCIAddress run before qemuDomainSupportsNicdev.
>
> Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn>
> ---
> v1 patch:
> https://www.redhat.com/archives/libvir-list/2018-December/msg00435.html
>
> Changes in v2:
>  - do not modify the address type, let qemuDomainEnsurePCIAddress do it.
> ---
>  src/qemu/qemu_hotplug.c | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 8da0233..f25c8db 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1369,6 +1369,25 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>      if (qemuAssignDeviceNetAlias(vm->def, net, -1) < 0)
>          goto cleanup;
>
> +    if (qemuDomainIsS390CCW(vm->def) &&
> +        net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
> +        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CCW)) {
> +        net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
> +        if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def)))
> +            goto cleanup;
> +        if (virDomainCCWAddressAssign(&net->info, ccwaddrs,
> +                                      !net->info.addr.ccw.assigned) < 0)
> +            goto cleanup;
> +    } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("virtio-s390 net device cannot be hotplugged."));
> +        goto cleanup;
> +    } else if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) {
> +        goto cleanup;
> +    }
> +
> +    releaseaddr = true;
> +
>      switch (actualType) {
>      case VIR_DOMAIN_NET_TYPE_BRIDGE:
>      case VIR_DOMAIN_NET_TYPE_NETWORK:
> @@ -1503,25 +1522,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>              goto cleanup;
>      }
>
> -    if (qemuDomainIsS390CCW(vm->def) &&
> -        net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
> -        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CCW)) {
> -        net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
> -        if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def)))
> -            goto cleanup;
> -        if (virDomainCCWAddressAssign(&net->info, ccwaddrs,
> -                                      !net->info.addr.ccw.assigned) < 0)
> -            goto cleanup;
> -    } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("virtio-s390 net device cannot be hotplugged."));
> -        goto cleanup;
> -    } else if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) {
> -        goto cleanup;
> -    }
> -
> -    releaseaddr = true;
> -
>      if (VIR_ALLOC_N(tapfdName, tapfdSize) < 0 ||
>          VIR_ALLOC_N(vhostfdName, vhostfdSize) < 0)
>          goto cleanup;

---
Best wishes
Wang Yechao--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt][PATCH v2] qemu: Assign device address before qemuDomainSupportsNicdev
Posted by wang.yechao255@zte.com.cn 5 years, 2 months ago
ping.

> qemuDomainSupportsNicdev will check the device address type on
> aarch64. If it is invoked before device address assigned, hotadd
> vhostuser interface with no address specified will get error.
> Let qemuDomainEnsurePCIAddress run before qemuDomainSupportsNicdev.
>
> Signed-off-by: Wang Yechao
> ---
> v1 patch:
> https://www.redhat.com/archives/libvir-list/2018-December/msg00435.html
>
> Changes in v2:
>  - do not modify the address type, let qemuDomainEnsurePCIAddress do it.
> ---
>  src/qemu/qemu_hotplug.c | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 8da0233..f25c8db 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1369,6 +1369,25 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>      if (qemuAssignDeviceNetAlias(vm->def, net, -1) < 0)
>          goto cleanup;
>
> +    if (qemuDomainIsS390CCW(vm->def) &&
> +        net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
> +        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CCW)) {
> +        net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
> +        if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def)))
> +            goto cleanup;
> +        if (virDomainCCWAddressAssign(&net->info, ccwaddrs,
> +                                      !net->info.addr.ccw.assigned) < 0)
> +            goto cleanup;
> +    } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("virtio-s390 net device cannot be hotplugged."));
> +        goto cleanup;
> +    } else if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) {
> +        goto cleanup;
> +    }
> +
> +    releaseaddr = true;
> +
>      switch (actualType) {
>      case VIR_DOMAIN_NET_TYPE_BRIDGE:
>      case VIR_DOMAIN_NET_TYPE_NETWORK:
> @@ -1503,25 +1522,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>              goto cleanup;
>      }
>
> -    if (qemuDomainIsS390CCW(vm->def) &&
> -        net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
> -        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CCW)) {
> -        net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
> -        if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def)))
> -            goto cleanup;
> -        if (virDomainCCWAddressAssign(&net->info, ccwaddrs,
> -                                      !net->info.addr.ccw.assigned) < 0)
> -            goto cleanup;
> -    } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("virtio-s390 net device cannot be hotplugged."));
> -        goto cleanup;
> -    } else if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) {
> -        goto cleanup;
> -    }
> -
> -    releaseaddr = true;
> -
>      if (VIR_ALLOC_N(tapfdName, tapfdSize) < 0 ||
>          VIR_ALLOC_N(vhostfdName, vhostfdSize) < 0)
>          goto cleanup;

---
Best wishes
Wang Yechao--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Assign device address before qemuDomainSupportsNicdev
Posted by John Ferlan 5 years, 2 months ago
$SUBJ:

s/address before qemuDomainSupportsNicdev/addresses earlier in
qemuDomainAttachNetDevice/


On 12/17/18 6:30 AM, Wang Yechao wrote:
> qemuDomainSupportsNicdev will check the device address type on
> aarch64. If it is invoked before device address assigned, hotadd
> vhostuser interface with no address specified will get error.
> Let qemuDomainEnsurePCIAddress run before qemuDomainSupportsNicdev.
> 

I would change this to:

If code in the @actualType switch needs to have/know which PCI Address
is being used, then we must assign it earlier.  In particular a
vhost-user device needs to call qemuDomainSupportsNicdev which requires
an address to be defined.


> Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn>
> ---
> v1 patch:
> https://www.redhat.com/archives/libvir-list/2018-December/msg00435.html
> 
> Changes in v2:
>  - do not modify the address type, let qemuDomainEnsurePCIAddress do it.
> ---
>  src/qemu/qemu_hotplug.c | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 

One concern I have is when moving code is checking anything that gets
called in between the moved code for "anomalies".  The only one that the
when @actualType is VIR_DOMAIN_NET_TYPE_HOSTDEV, then
qemuDomainAttachHostDevice is called.  Still AttachHostDevice will
eventually check the address anyway, so I believe it's fine.

As long as you're OK with my commit message adjustments and no one jumps
in with something else...

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt][PATCH v2] qemu: Assign device address before qemuDomainSupportsNicdev
Posted by wang.yechao255@zte.com.cn 5 years, 2 months ago
> $SUBJ:
>
> s/address before qemuDomainSupportsNicdev/addresses earlier in
> qemuDomainAttachNetDevice/
>
>
> On 12/17/18 6:30 AM, Wang Yechao wrote:
> > qemuDomainSupportsNicdev will check the device address type on
> > aarch64. If it is invoked before device address assigned, hotadd
> > vhostuser interface with no address specified will get error.
> > Let qemuDomainEnsurePCIAddress run before qemuDomainSupportsNicdev.
> >
>
> I would change this to:
>
> If code in the @actualType switch needs to have/know which PCI Address
> is being used, then we must assign it earlier.  In particular a
> vhost-user device needs to call qemuDomainSupportsNicdev which requires
> an address to be defined.
>
>
> > Signed-off-by: Wang Yechao <wang yechao255 zte com cn>
> > ---
> > v1 patch:
> > https://www.redhat.com/archives/libvir-list/2018-December/msg00435.html
> >
> > Changes in v2:
> >  - do not modify the address type, let qemuDomainEnsurePCIAddress do it.
> > ---
> >  src/qemu/qemu_hotplug.c | 38 +++++++++++++++++++-------------------
> >  1 file changed, 19 insertions(+), 19 deletions(-)
> >
>
> One concern I have is when moving code is checking anything that gets
> called in between the moved code for "anomalies".  The only one that the
> when @actualType is VIR_DOMAIN_NET_TYPE_HOSTDEV, then
> qemuDomainAttachHostDevice is called.  Still AttachHostDevice will
> eventually check the address anyway, so I believe it's fine.
>
> As long as you're OK with my commit message adjustments and no one jumps
> in with something else...
>
> Reviewed-by: John Ferlan <jferlan redhat com>
>
> John

I am OK with your commit message adjustments, it's more clearer and better.

---
Best wishes
Wang Yechao--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list