Re: [PATCH V4 3/6] libxlDomainDetachDeviceLive: handle hostdev parent is network device

[Jim Fehlig <jfehlig@xxxxxxxx>]

On 03/21/2016 02:11 AM, Chunyan Liu wrote:
> When hostdev parent is network device, should call
> libxlDomainDetachNetDevice to detach the device from a higher level.
>
> Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx>
> ---
>  src/libxl/libxl_driver.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 74ebea4..328dac8 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -3443,8 +3443,10 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver,
>  
>   cleanup:
>      libxl_device_nic_dispose(&nic);
> -    if (!ret)
> +    if (!ret) {
> +        networkReleaseActualDevice(vm->def, detach);
>          virDomainNetRemove(vm->def, detachidx);
> +    }

There was a merge conflict here due to my change in patch 2.

>      virObjectUnref(cfg);
>      return ret;
>  }
> @@ -3467,8 +3469,12 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
>              break;
>  
>          case VIR_DOMAIN_DEVICE_HOSTDEV:
> -            ret = libxlDomainDetachHostDevice(driver, vm,
> -                                              dev->data.hostdev);
> +            if (dev->data.hostdev->parent.type == VIR_DOMAIN_DEVICE_NET)
> +                ret = libxlDomainDetachNetDevice(driver, vm,
> +                          dev->data.hostdev->parent.data.net);

Indentation is odd for libvirt coding standards.

> +            else
> +                ret = libxlDomainDetachHostDevice(driver, vm,
> +                                                  dev->data.hostdev);
>              break;

I think it would be good to have a local virDomainHostdevDefPtr variable to
avoid the indirection. Also, the comment in the corresponding qemu code
describing why a network hostdev is treated differently is worth repeating here.
I've squashed in the below change. ACK to 1-3 with the minor adjustments, I'll
push them shortly.

Regards,
Jim

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b2e23c0..cc7f224 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3438,6 +3438,7 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver,
         goto cleanup;
     }
 
+    networkReleaseActualDevice(vm->def, detach);
     virDomainNetRemove(vm->def, detachidx);
     ret = 0;
 
@@ -3452,6 +3453,7 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
                             virDomainObjPtr vm,
                             virDomainDeviceDefPtr dev)
 {
+    virDomainHostdevDefPtr hostdev;
     int ret = -1;
 
     switch (dev->type) {
@@ -3465,8 +3467,16 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
             break;
 
         case VIR_DOMAIN_DEVICE_HOSTDEV:
-            ret = libxlDomainDetachHostDevice(driver, vm,
-                                              dev->data.hostdev);
+            hostdev = dev->data.hostdev;
+
+            /* If this is a network hostdev, we need to use the higher-level
+             * detach function so that mac address / virtualport are reset
+             */
+            if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET)
+                ret = libxlDomainDetachNetDevice(driver, vm,
+                                                 hostdev->parent.data.net);
+            else
+                ret = libxlDomainDetachHostDevice(driver, vm, hostdev);
             break;
 
         default:

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list