[libvirt] [PATCH 0/3] libxl: support hotplug of <interface> device
This patch series is to add support for attach/detaching an <interface> device. At the same time, add two fixes (1/3 and 3/3) Chunyan Liu (3): libxl: add HOSTDEV type in libxlDomainDetachDeviceConfig libxl: support hotplug of <interface> libxl: fix return value error Attach|DetachDeviceFlags .gnulib | 2 +- src/libxl/libxl_domain.c | 12 ++- src/libxl/libxl_driver.c | 193 +++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 180 insertions(+), 27 deletions(-) -- 1.8.4.5
Missing HOSTDEV type in libxlDomainDetachDeviceConfig. Add it. Signed-off-by: Chunyan Liu <cyliu@suse.com> --- src/libxl/libxl_driver.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index b27581e..5e08bba 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2985,7 +2985,8 @@ static int libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk, detach; - int ret = -1; + virDomainHostdevDefPtr hostdev, det_hostdev; + int idx; switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -2993,18 +2994,30 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) if (!(detach = virDomainDiskRemoveByName(vmdef, disk->dst))) { virReportError(VIR_ERR_INVALID_ARG, _("no target device %s"), disk->dst); - break; + return -1; } virDomainDiskDefFree(detach); - ret = 0; break; + + case VIR_DOMAIN_DEVICE_HOSTDEV: { + hostdev = dev->data.hostdev; + if ((idx = virDomainHostdevFind(vmdef, hostdev, &det_hostdev)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("device not present in domain configuration")); + return -1; + } + virDomainHostdevRemove(vmdef, idx); + virDomainHostdevDefFree(det_hostdev); + break; + } + default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("persistent detach of device is not supported")); - break; + return -1; } - return ret; + return 0; } static int -- 1.8.4.5
Add code to support attach/detaching a network device. Signed-off-by: Chunyan Liu <cyliu@suse.com> --- src/libxl/libxl_domain.c | 12 +++- src/libxl/libxl_driver.c | 146 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 149 insertions(+), 9 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 0c86601..6780e34 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -482,8 +482,16 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, STRNEQ(def->os.type, "hvm")) dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; - if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { - virDomainHostdevDefPtr hostdev = dev->data.hostdev; + if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV || + (dev->type == VIR_DOMAIN_DEVICE_NET && + dev->data.net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV)) { + + virDomainHostdevDefPtr hostdev; + + if (dev->type == VIR_DOMAIN_DEVICE_NET) + hostdev = &(dev->data.net)->data.hostdev.def; + else + hostdev = dev->data.hostdev; /* forbid capabilities mode hostdev in this kind of hypervisor */ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES) { diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 5e08bba..9cd56b5 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -55,6 +55,7 @@ #include "viraccessapicheck.h" #include "viratomic.h" #include "virhostdev.h" +#include "network/bridge_driver.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -2674,10 +2675,8 @@ static int libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver, libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, - virDomainDeviceDefPtr dev) + virDomainHostdevDefPtr hostdev) { - virDomainHostdevDefPtr hostdev = dev->data.hostdev; - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("hostdev mode '%s' not supported"), @@ -2756,6 +2755,60 @@ libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv, } static int +libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, + libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, + virDomainNetDefPtr net) +{ + int actualType; + libxl_device_nic nic; + int ret = -1; + + /* preallocate new slot for device */ + if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0) + return -1; + + /* If appropriate, grab a physical device from the configured + * network's pool of devices, or resolve bridge device name + * to the one defined in the network definition. + */ + if (networkAllocateActualDevice(vm->def, net) < 0) + return -1; + + actualType = virDomainNetGetActualType(net); + + if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + /* This is really a "smart hostdev", so it should be attached + * as a hostdev (the hostdev code will reach over into the + * netdev-specific code as appropriate), then also added to + * the nets list (see out:) if successful. + */ + ret = libxlDomainAttachHostDevice(driver, priv, vm, + virDomainNetGetActualHostdev(net)); + goto out; + } + + libxl_device_nic_init(&nic); + if (libxlMakeNic(vm->def, net, &nic) < 0) + goto cleanup; + + if (libxl_device_nic_add(priv->ctx, vm->def->id, &nic, 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("libxenlight failed to attach network device")); + goto cleanup; + } + + ret = 0; + + cleanup: + libxl_device_nic_dispose(&nic); + out: + if (!ret) + vm->def->nets[vm->def->nnets++] = net; + return ret; +} + +static int libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver, libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, @@ -2770,8 +2823,16 @@ libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver, dev->data.disk = NULL; break; + case VIR_DOMAIN_DEVICE_NET: + ret = libxlDomainAttachNetDevice(driver, priv, vm, + dev->data.net); + if (!ret) + dev->data.net = NULL; + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: - ret = libxlDomainAttachHostDevice(driver, priv, vm, dev); + ret = libxlDomainAttachHostDevice(driver, priv, vm, + dev->data.hostdev); if (!ret) dev->data.hostdev = NULL; break; @@ -2790,6 +2851,7 @@ static int libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk; + virDomainNetDefPtr net; virDomainHostdevDefPtr hostdev; virDomainHostdevDefPtr found; @@ -2806,6 +2868,14 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ dev->data.disk = NULL; break; + + case VIR_DOMAIN_DEVICE_NET: + net = dev->data.net; + if (virDomainNetInsert(vmdef, net)) + return -1; + dev->data.net = NULL; + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: hostdev = dev->data.hostdev; @@ -2928,9 +2998,8 @@ static int libxlDomainDetachHostDevice(libxlDriverPrivatePtr driver, libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, - virDomainDeviceDefPtr dev) + virDomainHostdevDefPtr hostdev) { - virDomainHostdevDefPtr hostdev = dev->data.hostdev; virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { @@ -2954,6 +3023,53 @@ libxlDomainDetachHostDevice(libxlDriverPrivatePtr driver, } static int +libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver, + libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, + virDomainNetDefPtr net) +{ + int detachidx; + virDomainNetDefPtr detach = NULL; + libxl_device_nic nic; + char mac[VIR_MAC_STRING_BUFLEN]; + int ret = -1; + + if ((detachidx = virDomainNetFindIdx(vm->def, net)) < 0) + return -1; + + detach = vm->def->nets[detachidx]; + + if (virDomainNetGetActualType(detach) == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + /* This is really a "smart hostdev", so it should be attached as a + * hostdev, then also removed from nets list (see out:) if successful. + */ + ret = libxlDomainDetachHostDevice(driver, priv, vm, + virDomainNetGetActualHostdev(detach)); + goto out; + } + + libxl_device_nic_init(&nic); + if (libxl_mac_to_device_nic(priv->ctx, vm->def->id, + virMacAddrFormat(&detach->mac, mac), &nic)) + goto cleanup; + + if (libxl_device_nic_remove(priv->ctx, vm->def->id, &nic, 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("libxenlight failed to detach network device")); + goto cleanup; + } + + ret = 0; + + cleanup: + libxl_device_nic_dispose(&nic); + out: + if (!ret) + virDomainNetRemove(vm->def, detachidx); + return ret; +} + +static int libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver, libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, @@ -2966,8 +3082,14 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver, ret = libxlDomainDetachDeviceDiskLive(priv, vm, dev); break; + case VIR_DOMAIN_DEVICE_NET: + ret = libxlDomainDetachNetDevice(driver, priv, vm, + dev->data.net); + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: - ret = libxlDomainDetachHostDevice(driver, priv, vm, dev); + ret = libxlDomainDetachHostDevice(driver, priv, vm, + dev->data.hostdev); break; default: @@ -2986,6 +3108,7 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk, detach; virDomainHostdevDefPtr hostdev, det_hostdev; + virDomainNetDefPtr net; int idx; switch (dev->type) { @@ -2999,6 +3122,15 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) virDomainDiskDefFree(detach); break; + case VIR_DOMAIN_DEVICE_NET: + net = dev->data.net; + if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + return -1; + + /* this is guaranteed to succeed */ + virDomainNetDefFree(virDomainNetRemove(vmdef, idx)); + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: { hostdev = dev->data.hostdev; if ((idx = virDomainHostdevFind(vmdef, hostdev, &det_hostdev)) < 0) { -- 1.8.4.5
Code logic in libxlDomainAttachDeviceFlags and libxlDomainDetachDeviceFlags is wrong with return value in error cases. 'ret' was being set to 0 if 'flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG' was false. Then if something like virDomainDeviceDefParse() failed in the VIR_DOMAIN_DEVICE_MODIFY_LIVE logic, the error would be reported but the function would return success. Signed-off-by: Chunyan Liu <cyliu@suse.com> --- src/libxl/libxl_driver.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 9cd56b5..5fbff1c 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3285,10 +3285,8 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, driver->xmlopt))) goto endjob; - if ((ret = libxlDomainAttachDeviceConfig(vmdef, dev)) < 0) + if (libxlDomainAttachDeviceConfig(vmdef, dev) < 0) goto endjob; - } else { - ret = 0; } if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { @@ -3299,7 +3297,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, VIR_DOMAIN_XML_INACTIVE))) goto endjob; - if ((ret = libxlDomainAttachDeviceLive(driver, priv, vm, dev)) < 0) + if (libxlDomainAttachDeviceLive(driver, priv, vm, dev) < 0) goto endjob; /* @@ -3307,11 +3305,13 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, * changed even if we attach the device failed. */ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) - ret = -1; + goto endjob; } + ret = 0; + /* Finally, if no error until here, we can save config. */ - if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) { + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { ret = virDomainSaveConfig(cfg->configDir, vmdef); if (!ret) { virDomainObjAssignDef(vm, vmdef, false, NULL); @@ -3396,10 +3396,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, driver->xmlopt))) goto endjob; - if ((ret = libxlDomainDetachDeviceConfig(vmdef, dev)) < 0) + if (libxlDomainDetachDeviceConfig(vmdef, dev) < 0) goto endjob; - } else { - ret = 0; } if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { @@ -3410,7 +3408,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, VIR_DOMAIN_XML_INACTIVE))) goto endjob; - if ((ret = libxlDomainDetachDeviceLive(driver, priv, vm, dev)) < 0) + if (libxlDomainDetachDeviceLive(driver, priv, vm, dev) < 0) goto endjob; /* @@ -3418,11 +3416,13 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, * changed even if we attach the device failed. */ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) - ret = -1; + goto endjob; } + ret = 0; + /* Finally, if no error until here, we can save config. */ - if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) { + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { ret = virDomainSaveConfig(cfg->configDir, vmdef); if (!ret) { virDomainObjAssignDef(vm, vmdef, false, NULL); -- 1.8.4.5
Chunyan Liu wrote:
This patch series is to add support for attach/detaching an <interface> device. At the same time, add two fixes (1/3 and 3/3)
Chunyan Liu (3): libxl: add HOSTDEV type in libxlDomainDetachDeviceConfig libxl: support hotplug of <interface> libxl: fix return value error Attach|DetachDeviceFlags
.gnulib | 2 +-
Odd that your cover letter includes the gnulib bump but patches do not. Did you use an old cover letter? Note: Chunyan and I have iterated on this series off-list. We started looking at an internal bug report, stumbled across further issues, and this series was the result. ACK series. I'll push this in a bit. Regards, Jim
src/libxl/libxl_domain.c | 12 ++- src/libxl/libxl_driver.c | 193 +++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 180 insertions(+), 27 deletions(-)
participants (2)
-
Chunyan Liu -
Jim Fehlig