Re: [libvirt] [PATCH V4 3/6] libxlDomainDetachDeviceLive: handle hostdev parent is network device
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(a)suse.com>
---
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: