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

Re: [libvirt] [PATCH 2/8] Add host PCI device hotplug support



On Thu, Aug 13, 2009 at 05:44:31PM +0100, Mark McLoughlin wrote:
> Attaching a host PCI device to a qemu guest is done with a
> straightforward 'pci_add auto host host=XX:XX.X' command.
> 
> Like with NIC and disk hotplug, we need to retain the guest PCI address
> assigned by qemu so that we can use it for hot-unplug.
> 
> Identifying a device for detach is done using the host PCI address.
> 
> Managed mode is handled by detaching/resetting the device before
> attaching it to the guest and re-attaching it after detaching it from
> the guest.
> 
> * src/qemu_driver.c: add qemudDomainAttachHostPciDevice() and
>   qemudDomainDetachHostPciDevice()
> 
> * src/domain_conf.h: add somewhere to store the guest PCI address
> 
> * src/domain_conf.c: handle formatting and parsing the guest PCI
>   address
> ---
>  src/domain_conf.c |   33 +++++++-
>  src/domain_conf.h |   13 +++
>  src/qemu_driver.c |  210 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 252 insertions(+), 4 deletions(-)
> 
> diff --git a/src/domain_conf.c b/src/domain_conf.c
> index 2301a96..bad53f7 100644
> --- a/src/domain_conf.c
> +++ b/src/domain_conf.c
> @@ -1977,7 +1977,8 @@ out:
>  static int
>  virDomainHostdevSubsysPciDefParseXML(virConnectPtr conn,
>                                       const xmlNodePtr node,
> -                                     virDomainHostdevDefPtr def) {
> +                                     virDomainHostdevDefPtr def,
> +                                     int flags) {
>  
>      int ret = -1;
>      xmlNodePtr cur;
> @@ -2049,6 +2050,20 @@ virDomainHostdevSubsysPciDefParseXML(virConnectPtr conn,
>                                           _("pci address needs function id"));
>                      goto out;
>                  }
> +            } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) &&
> +                       xmlStrEqual(cur->name, BAD_CAST "state")) {
> +                char *devaddr = virXMLPropString(cur, "devaddr");
> +                if (devaddr &&
> +                    sscanf(devaddr, "%x:%x:%x",
> +                           &def->source.subsys.u.pci.guest_addr.domain,
> +                           &def->source.subsys.u.pci.guest_addr.bus,
> +                           &def->source.subsys.u.pci.guest_addr.slot) < 3) {
> +                    virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> +                                         _("Unable to parse devaddr parameter '%s'"),
> +                                         devaddr);
> +                    VIR_FREE(devaddr);
> +                    goto out;
> +                }
>              } else {
>                  virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
>                                       _("unknown pci source type '%s'"),
> @@ -2123,7 +2138,7 @@ virDomainHostdevDefParseXML(virConnectPtr conn,
>                  }
>                  if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
>                      def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
> -                        if (virDomainHostdevSubsysPciDefParseXML(conn, cur, def) < 0)
> +                        if (virDomainHostdevSubsysPciDefParseXML(conn, cur, def, flags) < 0)
>                              goto error;
>                  }
>              } else {
> @@ -3937,7 +3952,8 @@ virDomainGraphicsDefFormat(virConnectPtr conn,
>  static int
>  virDomainHostdevDefFormat(virConnectPtr conn,
>                            virBufferPtr buf,
> -                          virDomainHostdevDefPtr def)
> +                          virDomainHostdevDefPtr def,
> +                          int flags)
>  {
>      const char *mode = virDomainHostdevModeTypeToString(def->mode);
>      const char *type;
> @@ -3978,6 +3994,15 @@ virDomainHostdevDefFormat(virConnectPtr conn,
>                            def->source.subsys.u.pci.bus,
>                            def->source.subsys.u.pci.slot,
>                            def->source.subsys.u.pci.function);
> +        if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) {
> +            virBufferAddLit(buf, "      <state");
> +            if (virHostdevHasValidGuestAddr(def))
> +                virBufferVSprintf(buf, " devaddr='%.4x:%.2x:%.2x'",
> +                                  def->source.subsys.u.pci.guest_addr.domain,
> +                                  def->source.subsys.u.pci.guest_addr.bus,
> +                                  def->source.subsys.u.pci.guest_addr.slot);
> +            virBufferAddLit(buf, "/>\n");
> +        }
>      }
>  
>      virBufferAddLit(buf, "      </source>\n");
> @@ -4192,7 +4217,7 @@ char *virDomainDefFormat(virConnectPtr conn,
>              goto cleanup;
>  
>      for (n = 0 ; n < def->nhostdevs ; n++)
> -        if (virDomainHostdevDefFormat(conn, &buf, def->hostdevs[n]) < 0)
> +        if (virDomainHostdevDefFormat(conn, &buf, def->hostdevs[n], flags) < 0)
>              goto cleanup;
>  
>      virBufferAddLit(&buf, "  </devices>\n");
> diff --git a/src/domain_conf.h b/src/domain_conf.h
> index 63fca76..44302be 100644
> --- a/src/domain_conf.h
> +++ b/src/domain_conf.h
> @@ -391,6 +391,11 @@ struct _virDomainHostdevDef {
>                       unsigned bus;
>                       unsigned slot;
>                       unsigned function;
> +                    struct {
> +                        unsigned domain;
> +                        unsigned bus;
> +                        unsigned slot;
> +                    } guest_addr;
>                  } pci;
>              } u;
>          } subsys;
> @@ -404,6 +409,14 @@ struct _virDomainHostdevDef {
>      char* target;
>  };
>  
> +static inline int
> +virHostdevHasValidGuestAddr(virDomainHostdevDefPtr def)
> +{
> +    return def->source.subsys.u.pci.guest_addr.domain ||
> +           def->source.subsys.u.pci.guest_addr.bus ||
> +           def->source.subsys.u.pci.guest_addr.slot;
> +}
> +
>  /* Flags for the 'type' field in next struct */
>  enum virDomainDeviceType {
>      VIR_DOMAIN_DEVICE_DISK,
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index f181f27..041e3da 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -5148,6 +5148,80 @@ cleanup:
>      return -1;
>  }
>  
> +static int qemudDomainAttachHostPciDevice(virConnectPtr conn,
> +                                          virDomainObjPtr vm,
> +                                          virDomainDeviceDefPtr dev)
> +{
> +    virDomainHostdevDefPtr hostdev = dev->data.hostdev;
> +    char *cmd, *reply;
> +    unsigned domain, bus, slot;
> +
> +    if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) {
> +        virReportOOMError(conn);
> +        return -1;
> +    }
> +
> +    if (hostdev->managed) {
> +        pciDevice *pci = pciGetDevice(conn,
> +                                      hostdev->source.subsys.u.pci.domain,
> +                                      hostdev->source.subsys.u.pci.bus,
> +                                      hostdev->source.subsys.u.pci.slot,
> +                                      hostdev->source.subsys.u.pci.function);
> +        if (!dev)
> +            return -1;
> +
> +        if (pciDettachDevice(conn, pci) < 0 ||
> +            pciResetDevice(conn, pci) < 0) {
> +            pciFreeDevice(conn, pci);
> +            return -1;
> +        }
> +
> +        pciFreeDevice(conn, pci);
> +    }
> +
> +    if (virAsprintf(&cmd, "pci_add auto host host=%.2x:%.2x.%.1x",
> +                    hostdev->source.subsys.u.pci.bus,
> +                    hostdev->source.subsys.u.pci.slot,
> +                    hostdev->source.subsys.u.pci.function) < 0) {
> +        virReportOOMError(conn);
> +        return -1;
> +    }
> +
> +    if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
> +        qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> +                         "%s", _("cannot attach host pci device"));
> +        VIR_FREE(cmd);
> +        return -1;
> +    }
> +
> +    if (strstr(reply, "invalid type: host")) {
> +        qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT, "%s",
> +                         _("PCI device assignment is not supported by this version of qemu"));
> +        VIR_FREE(cmd);
> +        VIR_FREE(reply);
> +        return -1;
> +    }
> +
> +    if (qemudParsePciAddReply(vm, reply, &domain, &bus, &slot) < 0) {
> +        qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> +                         _("parsing pci_add reply failed: %s"), reply);
> +        VIR_FREE(cmd);
> +        VIR_FREE(reply);
> +        return -1;
> +    }
> +
> +    hostdev->source.subsys.u.pci.guest_addr.domain = domain;
> +    hostdev->source.subsys.u.pci.guest_addr.bus    = bus;
> +    hostdev->source.subsys.u.pci.guest_addr.slot   = slot;
> +
> +    vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
> +
> +    VIR_FREE(reply);
> +    VIR_FREE(cmd);
> +
> +    return 0;
> +}
> +
>  static int qemudDomainAttachHostUsbDevice(virConnectPtr conn,
>                                            virDomainObjPtr vm,
>                                            virDomainDeviceDefPtr dev)
> @@ -5218,6 +5292,8 @@ static int qemudDomainAttachHostDevice(virConnectPtr conn,
>          return -1;
>  
>      switch (hostdev->source.subsys.type) {
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> +        return qemudDomainAttachHostPciDevice(conn, vm, dev);
>      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
>          return qemudDomainAttachHostUsbDevice(conn, vm, dev);
>      default:
> @@ -5545,6 +5621,138 @@ cleanup:
>      return ret;
>  }
>  
> +static int qemudDomainDetachHostPciDevice(virConnectPtr conn,
> +                                          virDomainObjPtr vm,
> +                                          virDomainDeviceDefPtr dev)
> +{
> +    virDomainHostdevDefPtr detach;
> +    char *cmd, *reply;
> +    int i, ret;
> +
> +    for (i = 0 ; i < vm->def->nhostdevs ; i++) {
> +        unsigned domain   = vm->def->hostdevs[i]->source.subsys.u.pci.domain;
> +        unsigned bus      = vm->def->hostdevs[i]->source.subsys.u.pci.bus;
> +        unsigned slot     = vm->def->hostdevs[i]->source.subsys.u.pci.slot;
> +        unsigned function = vm->def->hostdevs[i]->source.subsys.u.pci.function;
> +
> +        if (dev->data.hostdev->source.subsys.u.pci.domain   == domain &&
> +            dev->data.hostdev->source.subsys.u.pci.bus      == bus &&
> +            dev->data.hostdev->source.subsys.u.pci.slot     == slot &&
> +            dev->data.hostdev->source.subsys.u.pci.function == function) {
> +            detach = vm->def->hostdevs[i];
> +            break;
> +        }
> +    }
> +
> +    if (!detach) {
> +        qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> +                         _("host pci device %.4x:%.2x:%.2x.%.1x not found"),
> +                         dev->data.hostdev->source.subsys.u.pci.domain,
> +                         dev->data.hostdev->source.subsys.u.pci.bus,
> +                         dev->data.hostdev->source.subsys.u.pci.slot,
> +                         dev->data.hostdev->source.subsys.u.pci.function);
> +        return -1;
> +    }
> +
> +    if (!virHostdevHasValidGuestAddr(detach)) {
> +        qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> +                         "%s", _("hostdev cannot be detached - device state missing"));
> +        return -1;
> +    }
> +
> +    if (virAsprintf(&cmd, "pci_del pci_addr=%.4x:%.2x:%.2x",
> +                    detach->source.subsys.u.pci.guest_addr.domain,
> +                    detach->source.subsys.u.pci.guest_addr.bus,
> +                    detach->source.subsys.u.pci.guest_addr.slot) < 0) {
> +        virReportOOMError(conn);
> +        return -1;
> +    }
> +
> +    if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
> +        qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> +                         "%s", _("cannot detach host pci device"));
> +        VIR_FREE(cmd);
> +        return -1;
> +    }
> +
> +    DEBUG("%s: pci_del reply: %s", vm->def->name,  reply);
> +
> +    /* If the command fails due to a wrong PCI address qemu prints
> +     * 'invalid pci address'; nothing is printed on success */
> +    if (strstr(reply, "Invalid pci address")) {
> +        qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> +                         _("failed to detach host pci device: invalid PCI address %.4x:%.2x:%.2x: %s"),
> +                         detach->source.subsys.u.pci.guest_addr.domain,
> +                         detach->source.subsys.u.pci.guest_addr.bus,
> +                         detach->source.subsys.u.pci.guest_addr.slot,
> +                         reply);
> +        VIR_FREE(reply);
> +        VIR_FREE(cmd);
> +        return -1;
> +    }
> +
> +    VIR_FREE(reply);
> +    VIR_FREE(cmd);
> +
> +    ret = 0;
> +
> +    if (detach->managed) {
> +        pciDevice *pci = pciGetDevice(conn,
> +                                      detach->source.subsys.u.pci.domain,
> +                                      detach->source.subsys.u.pci.bus,
> +                                      detach->source.subsys.u.pci.slot,
> +                                      detach->source.subsys.u.pci.function);
> +        if (!pci || pciReAttachDevice(conn, pci) < 0)
> +            ret = -1;
> +        if (pci)
> +            pciFreeDevice(conn, pci);
> +    }
> +
> +    if (vm->def->nhostdevs > 1) {
> +        vm->def->hostdevs[i] = vm->def->hostdevs[--vm->def->nhostdevs];
> +        if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs) < 0) {
> +            virReportOOMError(conn);
> +            ret = -1;
> +        }
> +    } else {
> +        VIR_FREE(vm->def->hostdevs[0]);
> +        vm->def->nhostdevs = 0;
> +    }
> +
> +    return ret;
> +}
> +
> +static int qemudDomainDetachHostDevice(virConnectPtr conn,
> +                                       struct qemud_driver *driver,
> +                                       virDomainObjPtr vm,
> +                                       virDomainDeviceDefPtr dev)
> +{
> +    virDomainHostdevDefPtr hostdev = dev->data.hostdev;
> +    int ret;
> +
> +    if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
> +        qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT,
> +                         _("hostdev mode '%s' not supported"),
> +                         virDomainHostdevModeTypeToString(hostdev->mode));
> +        return -1;
> +    }
> +
> +    switch (hostdev->source.subsys.type) {
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> +        ret = qemudDomainDetachHostPciDevice(conn, vm, dev);
> +    default:
> +        qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT,
> +                         _("hostdev subsys type '%s' not supported"),
> +                         virDomainHostdevSubsysTypeToString(hostdev->source.subsys.type));
> +        return -1;
> +    }
> +
> +    if (qemuDomainSetDeviceOwnership(conn, driver, dev, 1) < 0)
> +        VIR_WARN0("Fail to restore disk device ownership");
> +
> +    return ret;
> +}
> +
>  static int qemudDomainDetachDevice(virDomainPtr dom,
>                                     const char *xml) {
>      struct qemud_driver *driver = dom->conn->privateData;
> @@ -5585,6 +5793,8 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
>              VIR_WARN0("Fail to restore disk device ownership");
>      } else if (dev->type == VIR_DOMAIN_DEVICE_NET) {
>          ret = qemudDomainDetachNetDevice(dom->conn, vm, dev);
> +    } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
> +        ret = qemudDomainDetachHostDevice(dom->conn, driver, vm, dev);
>      } else
>          qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
>                           "%s", _("only SCSI or virtio disk device can be detached dynamically"));
> -- 

ACK


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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