Re: [PATCH v13 49/49] add pci passthrough to libxl driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxx>
> ---
>  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

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]