[libvirt] [PATCH v13 49/49] add pci passthrough to libxl driver
Jim Fehlig
jfehlig at suse.com
Tue Mar 4 01:36:05 UTC 2014
Chunyan Liu wrote:
> Add pci passthrough to libxl driver, support attach-device, detach-device and
> start a vm with pci hostdev specified.
>
> Signed-off-by: Chunyan Liu <cyliu at suse.com>
> ---
> src/libxl/libxl_conf.c | 63 +++++++
> src/libxl/libxl_conf.h | 4 +
> src/libxl/libxl_driver.c | 447 +++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 513 insertions(+), 1 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index ade0a08..8ba3ce3 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1147,6 +1147,66 @@ libxlDriverConfigGet(libxlDriverPrivatePtr driver)
> return cfg;
> }
>
> +int
> +libxlMakePci(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev)
> +{
> + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
> + return -1;
> + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
> + return -1;
> +
> + pcidev->domain = hostdev->source.subsys.u.pci.addr.domain;
> + pcidev->bus = hostdev->source.subsys.u.pci.addr.bus;
> + pcidev->dev = hostdev->source.subsys.u.pci.addr.slot;
> + pcidev->func = hostdev->source.subsys.u.pci.addr.function;
> +
> + return 0;
> +}
> +
> +static int
> +libxlMakePciList(virDomainDefPtr def, libxl_domain_config *d_config)
> +{
> + virDomainHostdevDefPtr *l_hostdevs = def->hostdevs;
> + size_t nhostdevs = def->nhostdevs;
> + size_t npcidevs = 0;
> + libxl_device_pci *x_pcidevs;
> + size_t i, j;
> +
> + if (nhostdevs == 0)
> + return 0;
> +
> + if (VIR_ALLOC_N(x_pcidevs, nhostdevs) < 0)
> + return -1;
> +
> + for (i = 0, j = 0; i < nhostdevs; i++) {
> + if (l_hostdevs[i]->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
> + continue;
> + if (l_hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
> + continue;
> +
> + libxl_device_pci_init(&x_pcidevs[j]);
> +
> + if (libxlMakePci(l_hostdevs[i], &x_pcidevs[j]) < 0)
> + goto error;
> +
> + npcidevs++;
> + j++;
> + }
> +
> + VIR_SHRINK_N(x_pcidevs, nhostdevs, nhostdevs - npcidevs);
> + d_config->pcidevs = x_pcidevs;
> + d_config->num_pcidevs = npcidevs;
> +
> + return 0;
> +
> +error:
> + for (i = 0; i < npcidevs; i++)
> + libxl_device_pci_dispose(&x_pcidevs[i]);
> +
> + VIR_FREE(x_pcidevs);
> + return -1;
> +}
> +
> virCapsPtr
> libxlMakeCapabilities(libxl_ctx *ctx)
> {
> @@ -1195,6 +1255,9 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
> if (libxlMakeVfbList(driver, def, d_config) < 0)
> return -1;
>
> + if (libxlMakePciList(def, d_config) < 0)
> + return -1;
> +
> d_config->on_reboot = def->onReboot;
> d_config->on_poweroff = def->onPoweroff;
> d_config->on_crash = def->onCrash;
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index f089a92..faf0d00 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -39,6 +39,7 @@
> # include "virchrdev.h"
>
>
> +# define LIBXL_DRIVER_NAME "xenlight"
> # define LIBXL_VNC_PORT_MIN 5900
> # define LIBXL_VNC_PORT_MAX 65535
>
> @@ -150,6 +151,9 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
> virDomainGraphicsDefPtr l_vfb, libxl_device_vfb *x_vfb);
>
> int
> +libxlMakePci(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev);
> +
> +int
> libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
> virDomainObjPtr vm, libxl_domain_config *d_config);
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index a79efcb..9035262 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -52,6 +52,7 @@
> #include "virsysinfo.h"
> #include "viraccessapicheck.h"
> #include "viratomic.h"
> +#include "virhostdev.h"
>
> #define VIR_FROM_THIS VIR_FROM_LIBXL
>
> @@ -309,6 +310,12 @@ libxlVmCleanup(libxlDriverPrivatePtr driver,
> int vnc_port;
> char *file;
> size_t i;
> + virHostdevManagerPtr hostdev_mgr;
> +
> + hostdev_mgr = virHostdevManagerGetDefault();
> + if (hostdev_mgr != NULL)
> + virHostdevReAttachDomainHostdevs(hostdev_mgr, LIBXL_DRIVER_NAME,
> + vm->def, VIR_SP_PCI_HOSTDEV, NULL);
>
> vm->def->id = -1;
>
> @@ -699,6 +706,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> #ifdef LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS
> libxl_domain_restore_params params;
> #endif
> + virHostdevManagerPtr hostdev_mgr;
>
> if (libxlDomainObjPrivateInitCtx(vm) < 0)
> return ret;
> @@ -761,6 +769,12 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> goto endjob;
> }
>
> + hostdev_mgr = virHostdevManagerGetDefault();
> + if (hostdev_mgr == NULL ||
> + virHostdevPrepareDomainHostdevs(hostdev_mgr, LIBXL_DRIVER_NAME,
> + vm->def, VIR_SP_PCI_HOSTDEV) < 0)
> + goto endjob;
> +
> /* Unlock virDomainObj while creating the domain */
> virObjectUnlock(vm);
> if (restore_fd < 0) {
> @@ -867,6 +881,7 @@ libxlReconnectDomain(virDomainObjPtr vm,
> libxl_dominfo d_info;
> int len;
> uint8_t *data = NULL;
> + virHostdevManagerPtr hostdev_mgr;
>
> virObjectLock(vm);
>
> @@ -890,6 +905,14 @@ libxlReconnectDomain(virDomainObjPtr vm,
>
> /* Update domid in case it changed (e.g. reboot) while we were gone? */
> vm->def->id = d_info.domid;
> +
> + /* Update hostdev state */
> + hostdev_mgr = virHostdevManagerGetDefault();
> + if (hostdev_mgr == NULL ||
> + virHostdevUpdateDomainActiveHostdevs(hostdev_mgr, LIBXL_DRIVER_NAME,
> + vm->def, VIR_SP_PCI_HOSTDEV) < 0)
> + goto out;
> +
> virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNKNOWN);
>
> if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
> @@ -3274,6 +3297,95 @@ cleanup:
> }
>
> static int
> +libxlDomainAttachHostPciDevice(libxlDomainObjPrivatePtr priv,
> + virDomainObjPtr vm,
> + virDomainHostdevDefPtr hostdev)
> +{
> + libxl_device_pci pcidev;
> + virDomainHostdevDefPtr found;
> + virHostdevManagerPtr hostdev_mgr;
> +
> + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
> + return -1;
> +
> + if (virDomainHostdevFind(vm->def, hostdev, &found) >= 0) {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("target pci device %.4x:%.2x:%.2x.%.1x already exists"),
> + hostdev->source.subsys.u.pci.addr.domain,
> + hostdev->source.subsys.u.pci.addr.bus,
> + hostdev->source.subsys.u.pci.addr.slot,
> + hostdev->source.subsys.u.pci.addr.function);
> + return -1;
> + }
> +
> + if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
> + return -1;
> +
> + hostdev_mgr = virHostdevManagerGetDefault();
> + if (hostdev_mgr == NULL ||
> + virHostdevPreparePciHostdevs(hostdev_mgr, LIBXL_DRIVER_NAME,
> + vm->def->name, vm->def->uuid,
> + &hostdev, 1, 0) < 0)
> + goto cleanup;
>
The cleanup label simply returns -1, so might as well do that here.
> +
> + if (libxlMakePci(hostdev, &pcidev) < 0)
> + goto reattach_hostdev;
> +
> + if (libxl_device_pci_add(priv->ctx, vm->def->id, &pcidev, 0) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("libxenlight failed to attach pci device %.4x:%.2x:%.2x.%.1x"),
> + hostdev->source.subsys.u.pci.addr.domain,
> + hostdev->source.subsys.u.pci.addr.bus,
> + hostdev->source.subsys.u.pci.addr.slot,
> + hostdev->source.subsys.u.pci.addr.function);
> + goto reattach_hostdev;
> + }
> +
> + vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
> + return 0;
> +
> +reattach_hostdev:
>
I think this label should be 'error'.
> + virHostdevReAttachPciHostdevs(hostdev_mgr, LIBXL_DRIVER_NAME,
> + vm->def->name, &hostdev, 1, NULL);
> +
> +cleanup:
>
No need for cleanup, just return -1.
> + return -1;
> +}
> +
> +static int
> +libxlDomainAttachHostDevice(libxlDomainObjPrivatePtr priv,
> + virDomainObjPtr vm,
> + virDomainDeviceDefPtr dev)
> +{
> + virDomainHostdevDefPtr hostdev = dev->data.hostdev;
> +
> + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("hostdev mode '%s' not supported"),
> + virDomainHostdevModeTypeToString(hostdev->mode));
> + return -1;
> + }
> +
> + switch (hostdev->source.subsys.type) {
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> + if (libxlDomainAttachHostPciDevice(priv, vm, hostdev) < 0)
> + goto error;
> + break;
> +
> + default:
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("hostdev subsys type '%s' not supported"),
> + virDomainHostdevSubsysTypeToString(hostdev->source.subsys.type));
> + goto error;
> + }
> +
> + return 0;
> +
> +error:
> + return -1;
>
Nothing done here, so just 'return -1' on error and drop it.
> +}
> +
> +static int
> libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
> virDomainObjPtr vm, virDomainDeviceDefPtr dev)
> {
> @@ -3340,6 +3452,12 @@ libxlDomainAttachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
> dev->data.disk = NULL;
> break;
>
> + case VIR_DOMAIN_DEVICE_HOSTDEV:
> + ret = libxlDomainAttachHostDevice(priv, vm, dev);
> + if (!ret)
> + dev->data.hostdev = NULL;
> + break;
> +
> default:
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("device type '%s' cannot be attached"),
> @@ -3354,6 +3472,8 @@ static int
> libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
> {
> virDomainDiskDefPtr disk;
> + virDomainHostdevDefPtr hostdev;
> + virDomainHostdevDefPtr found;
>
> switch (dev->type) {
> case VIR_DOMAIN_DEVICE_DISK:
> @@ -3368,6 +3488,25 @@ 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_HOSTDEV:
> + hostdev = dev->data.hostdev;
> +
> + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
> + return -1;
> +
> + if (virDomainHostdevFind(vmdef, hostdev, &found) >= 0) {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("target pci device %.4x:%.2x:%.2x.%.1x\
> + already exists"),
> + hostdev->source.subsys.u.pci.addr.domain,
> + hostdev->source.subsys.u.pci.addr.bus,
> + hostdev->source.subsys.u.pci.addr.slot,
> + hostdev->source.subsys.u.pci.addr.function);
> + return -1;
> + }
> +
> + virDomainHostdevInsert(vmdef, hostdev);
> + break;
>
> default:
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -3378,6 +3517,125 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
> }
>
> static int
> +libxlComparePCIDevice(virDomainDefPtr def ATTRIBUTE_UNUSED,
> + virDomainDeviceDefPtr device ATTRIBUTE_UNUSED,
> + virDomainDeviceInfoPtr info1,
> + void *opaque)
> +{
> + virDomainDeviceInfoPtr info2 = opaque;
> +
> + if (info1->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI ||
> + info2->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
> + return 0;
> +
> + if (info1->addr.pci.domain == info2->addr.pci.domain &&
> + info1->addr.pci.bus == info2->addr.pci.bus &&
> + info1->addr.pci.slot == info2->addr.pci.slot &&
> + info1->addr.pci.function != info2->addr.pci.function)
> + return -1;
> + return 0;
> +}
> +
> +static bool
> +libxlIsMultiFunctionDevice(virDomainDefPtr def,
> + virDomainDeviceInfoPtr dev)
> +{
> + if (virDomainDeviceInfoIterate(def, libxlComparePCIDevice, dev) < 0)
> + return true;
> + return false;
> +}
> +
> +static int
> +libxlDomainDetachHostPciDevice(libxlDomainObjPrivatePtr priv,
> + virDomainObjPtr vm,
> + virDomainHostdevDefPtr hostdev)
> +{
> + virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys;
> + libxl_device_pci pcidev;
> + virDomainHostdevDefPtr detach;
> + int idx;
> + virHostdevManagerPtr hostdev_mgr;
> +
> + if (subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
> + return -1;
> +
> + idx = virDomainHostdevFind(vm->def, hostdev, &detach);
> + if (idx < 0) {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("host pci device %.4x:%.2x:%.2x.%.1x not found"),
> + subsys->u.pci.addr.domain, subsys->u.pci.addr.bus,
> + subsys->u.pci.addr.slot, subsys->u.pci.addr.function);
> + return -1;
> + }
> +
> + if (libxlIsMultiFunctionDevice(vm->def, detach->info)) {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("cannot hot unplug multifunction PCI device: %.4x:%.2x:%.2x.%.1x"),
> + subsys->u.pci.addr.domain, subsys->u.pci.addr.bus,
> + subsys->u.pci.addr.slot, subsys->u.pci.addr.function);
> + goto cleanup;
> + }
> +
> +
> + libxl_device_pci_init(&pcidev);
> +
> + if (libxlMakePci(detach, &pcidev) < 0)
> + goto cleanup;
> +
> + if (libxl_device_pci_remove(priv->ctx, vm->def->id, &pcidev, 0) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("libxenlight failed to detach pci device\
> + %.4x:%.2x:%.2x.%.1x"),
> + subsys->u.pci.addr.domain, subsys->u.pci.addr.bus,
> + subsys->u.pci.addr.slot, subsys->u.pci.addr.function);
> + goto cleanup;
> + }
> +
> + libxl_device_pci_dispose(&pcidev);
> +
> + virDomainHostdevRemove(vm->def, idx);
> +
> + hostdev_mgr = virHostdevManagerGetDefault();
> + if (hostdev_mgr != NULL)
> + virHostdevReAttachPciHostdevs(hostdev_mgr, LIBXL_DRIVER_NAME,
> + vm->def->name, &hostdev, 1, NULL);
> +
> + return 0;
> +
> +cleanup:
>
'error' seems like a better name for this label.
> + virDomainHostdevDefFree(detach);
> + return -1;
> +}
> +
> +static int
> +libxlDomainDetachHostDevice(libxlDomainObjPrivatePtr priv,
> + virDomainObjPtr vm,
> + virDomainDeviceDefPtr dev)
> +{
> + virDomainHostdevDefPtr hostdev = dev->data.hostdev;
> + virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys;
> +
> + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("hostdev mode '%s' not supported"),
> + virDomainHostdevModeTypeToString(hostdev->mode));
> + return -1;
> + }
> +
> + switch (subsys->type) {
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> + return libxlDomainDetachHostPciDevice(priv, vm, hostdev);
> +
> + default:
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unexpected hostdev type %d"), subsys->type);
> + break;
> + }
> +
> + return -1;
> +}
> +
> +static int
> libxlDomainDetachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
> virDomainDeviceDefPtr dev)
> {
> @@ -3388,6 +3646,10 @@ libxlDomainDetachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
> ret = libxlDomainDetachDeviceDiskLive(priv, vm, dev);
> break;
>
> + case VIR_DOMAIN_DEVICE_HOSTDEV:
> + ret = libxlDomainDetachHostDevice(priv, vm, dev);
> + break;
> +
> default:
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("device type '%s' cannot be detached"),
> @@ -3398,6 +3660,7 @@ libxlDomainDetachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
> return ret;
> }
>
> +
> static int
> libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
> {
> @@ -4590,10 +4853,188 @@ libxlConnectSupportsFeature(virConnectPtr conn, int feature)
> }
> }
>
> +static int
> +libxlNodeDeviceGetPciInfo(virNodeDeviceDefPtr def,
> + unsigned *domain,
> + unsigned *bus,
> + unsigned *slot,
> + unsigned *function)
> +{
> + virNodeDevCapsDefPtr cap;
> + int ret = -1;
> +
> + cap = def->caps;
> + while (cap) {
> + if (cap->type == VIR_NODE_DEV_CAP_PCI_DEV) {
> + *domain = cap->data.pci_dev.domain;
> + *bus = cap->data.pci_dev.bus;
> + *slot = cap->data.pci_dev.slot;
> + *function = cap->data.pci_dev.function;
> + break;
> + }
> +
> + cap = cap->next;
> + }
> +
> + if (!cap) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("device %s is not a PCI device"), def->name);
> + goto out;
>
You can return -1 here
> + }
> +
> + ret = 0;
>
and 0 here
> +out:
> + return ret;
>
and drop ret.
> +}
> +
> +static int
> +libxlNodeDeviceDetachFlags(virNodeDevicePtr dev,
> + const char *driverName,
> + unsigned int flags)
> +{
> + virPCIDevicePtr pci = NULL;
> + unsigned domain = 0, bus = 0, slot = 0, function = 0;
> + int ret = -1;
> + virNodeDeviceDefPtr def = NULL;
> + char *xml = NULL;
> + virHostdevManagerPtr hostdev_mgr;
> +
> + virCheckFlags(0, -1);
> +
> + xml = virNodeDeviceGetXMLDesc(dev, 0);
> + if (!xml)
> + goto cleanup;
> +
> + def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL);
> + if (!def)
> + goto cleanup;
> +
> + if (virNodeDeviceDetachFlagsEnsureACL(dev->conn, def) < 0)
> + goto cleanup;
> +
> + if (libxlNodeDeviceGetPciInfo(def, &domain, &bus, &slot, &function) < 0)
> + goto cleanup;
> +
> + pci = virPCIDeviceNew(domain, bus, slot, function);
> + if (!pci)
> + goto cleanup;
> +
> + if (!driverName || STREQ(driverName, "xen")) {
> + if (virPCIDeviceSetStubDriver(pci, "pciback") < 0)
> + goto cleanup;
> + } else {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("unsupported driver name '%s'"), driverName);
> + goto cleanup;
> + }
> +
> + hostdev_mgr = virHostdevManagerGetDefault();
> + if (hostdev_mgr == NULL ||
> + virHostdevPciNodeDeviceDetach(hostdev_mgr, pci) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +cleanup:
> + virPCIDeviceFree(pci);
> + virNodeDeviceDefFree(def);
> + VIR_FREE(xml);
> + return ret;
> +}
> +
> +static int
> +libxlNodeDeviceDettach(virNodeDevicePtr dev)
> +{
> + return libxlNodeDeviceDetachFlags(dev, NULL, 0);
> +}
> +
> +static int
> +libxlNodeDeviceReAttach(virNodeDevicePtr dev)
> +{
> + virPCIDevicePtr pci = NULL;
> + unsigned domain = 0, bus = 0, slot = 0, function = 0;
> + int ret = -1;
> + virNodeDeviceDefPtr def = NULL;
> + char *xml = NULL;
> + virHostdevManagerPtr hostdev_mgr;
> +
> + xml = virNodeDeviceGetXMLDesc(dev, 0);
> + if (!xml)
> + goto cleanup;
> +
> + def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL);
> + if (!def)
> + goto cleanup;
> +
> + if (virNodeDeviceReAttachEnsureACL(dev->conn, def) < 0)
> + goto cleanup;
> +
> + if (libxlNodeDeviceGetPciInfo(def, &domain, &bus, &slot, &function) < 0)
> + goto cleanup;
> +
> + pci = virPCIDeviceNew(domain, bus, slot, function);
> + if (!pci)
> + goto cleanup;
> +
> + hostdev_mgr = virHostdevManagerGetDefault();
> + if (hostdev_mgr == NULL ||
> + virHostdevPciNodeDeviceReAttach(hostdev_mgr, pci) < 0)
> + goto out;
>
I think you can just goto cleanup here
> +
> + ret = 0;
> +out:
> + virPCIDeviceFree(pci);
>
moving this to cleanup and removing out. It is safe to call
virPCIDeviceFree with NULL.
> +cleanup:
> + virNodeDeviceDefFree(def);
> + VIR_FREE(xml);
> + return ret;
> +}
> +
> +static int
> +libxlNodeDeviceReset(virNodeDevicePtr dev)
> +{
> + virPCIDevicePtr pci;
> + unsigned domain = 0, bus = 0, slot = 0, function = 0;
> + int ret = -1;
> + virNodeDeviceDefPtr def = NULL;
> + char *xml = NULL;
> + virHostdevManagerPtr hostdev_mgr;
> +
> + xml = virNodeDeviceGetXMLDesc(dev, 0);
> + if (!xml)
> + goto cleanup;
> +
> + def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL);
> + if (!def)
> + goto cleanup;
> +
> + if (virNodeDeviceResetEnsureACL(dev->conn, def) < 0)
> + goto cleanup;
> +
> + if (libxlNodeDeviceGetPciInfo(def, &domain, &bus, &slot, &function) < 0)
> + goto cleanup;
> +
> + pci = virPCIDeviceNew(domain, bus, slot, function);
> + if (!pci)
> + goto cleanup;
> +
> + hostdev_mgr = virHostdevManagerGetDefault();
> + if (hostdev_mgr == NULL ||
> + virHostdevPciNodeDeviceReset(hostdev_mgr, pci) < 0)
> + goto out;
> +
> + ret = 0;
> +out:
> + virPCIDeviceFree(pci);
>
Same comment here, except you will need to initialize pci to NULL.
> +cleanup:
> + virNodeDeviceDefFree(def);
> + VIR_FREE(xml);
> + return ret;
> +}
> +
>
> static virDriver libxlDriver = {
> .no = VIR_DRV_LIBXL,
> - .name = "xenlight",
> + .name = LIBXL_DRIVER_NAME,
> .connectOpen = libxlConnectOpen, /* 0.9.0 */
> .connectClose = libxlConnectClose, /* 0.9.0 */
> .connectGetType = libxlConnectGetType, /* 0.9.0 */
> @@ -4676,6 +5117,10 @@ static virDriver libxlDriver = {
> .connectDomainEventDeregisterAny = libxlConnectDomainEventDeregisterAny, /* 0.9.0 */
> .connectIsAlive = libxlConnectIsAlive, /* 0.9.8 */
> .connectSupportsFeature = libxlConnectSupportsFeature, /* 1.1.1 */
> + .nodeDeviceDettach = libxlNodeDeviceDettach, /* 1.2.2 */
> + .nodeDeviceDetachFlags = libxlNodeDeviceDetachFlags, /* 1.2.2 */
> + .nodeDeviceReAttach = libxlNodeDeviceReAttach, /* 1.2.2 */
> + .nodeDeviceReset = libxlNodeDeviceReset, /* 1.2.2 */
>
1.2.3 :)
I'm hopeful this series will make the next release. I haven't looked at
all the patches in detail as Cedric has, but agree that the split made
an initial skim much easier. Nice work Chunyan!
Regards,
Jim
More information about the libvir-list
mailing list