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

Re: [libvirt] [PATCH] Fix restore of QEMU guests with PCI device reservation



On Wed, Feb 03, 2010 at 04:32:47PM +0000, Daniel P. Berrange wrote:
> When restoring from a saved guest image, the XML would already
> contain the PCI slot ID of the IDE controller & video card.
> The attempt to explicitly reserve this upfront would thus fail
> everytime.
>
>   slot at time of need, rather than upfront
> ---
>  src/qemu/qemu_conf.c |   83 ++++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 63 insertions(+), 20 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 389db7b..3d83a8f 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1797,6 +1797,13 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev)
>  {
>      char *addr;
>  
> +    if (dev->addr.pci.domain != 0 ||
> +        dev->addr.pci.bus != 0) {
> +        qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s",
> +                         _("Only PCI domain 0 and bus 0 are available"));
> +        return NULL;
> +    }
> +
>      if (virAsprintf(&addr, "%d:%d:%d",
>                      dev->addr.pci.domain,
>                      dev->addr.pci.bus,
> @@ -1817,6 +1824,8 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
>      if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
>          char *addr = qemuPCIAddressAsString(dev);
>  
> +        VIR_DEBUG("Remembering PCI addr %s", addr);
> +
>          if (virHashAddEntry(addrs->used, addr, addr) < 0) {
>              VIR_FREE(addr);
>              return -1;
> @@ -1858,6 +1867,8 @@ int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
>      if (!addr)
>          return -1;
>  
> +    VIR_DEBUG("Reserving PCI addr %s", addr);
> +
>      if (virHashLookup(addrs->used, addr)) {
>          qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
>                           _("unable to reserve PCI address %s"), addr);
> @@ -1870,6 +1881,9 @@ int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
>          return -1;
>      }
>  
> +    if (dev->addr.pci.slot > addrs->nextslot)
> +        addrs->nextslot = dev->addr.pci.slot + 1;
> +
>      return 0;
>  }
>  
> @@ -1947,6 +1961,8 @@ int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
>  
>          addr = qemuPCIAddressAsString(&maybe);
>  
> +        VIR_DEBUG("Allocating PCI addr %s", addr);
> +
>          if (virHashLookup(addrs->used, addr)) {
>              VIR_FREE(addr);
>              continue;
> @@ -1981,12 +1997,13 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs)
>      /* Host bridge */
>      if (qemuDomainPCIAddressReserveSlot(addrs, 0) < 0)
>          goto error;
> -    /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller) */
> -    if (qemuDomainPCIAddressReserveSlot(addrs, 1) < 0)
> -        goto error;
> -    /* VGA */
> -    if (qemuDomainPCIAddressReserveSlot(addrs, 2) < 0)
> -        goto error;
> +
> +    /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller)
> +     * at slot 1....reserve it later
> +     */
> +
> +    /* VGA at slot 2.... reserve it later */
> +
>      /* VirtIO Balloon */
>      if (qemuDomainPCIAddressReserveSlot(addrs, 3) < 0)
>          goto error;
> @@ -2033,23 +2050,34 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs)
>              goto error;
>      }
>      for (i = 0; i < def->nvideos ; i++) {
> -        if (def->videos[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> -            continue;
>          /* First VGA is hardcoded slot=2 */
>          if (i == 0) {
> -            def->videos[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> -            def->videos[i]->info.addr.pci.domain = 0;
> -            def->videos[i]->info.addr.pci.bus = 0;
> -            def->videos[i]->info.addr.pci.slot = 2;
> -            def->videos[i]->info.addr.pci.function = 0;
> +            if (def->videos[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> +                if (def->videos[i]->info.addr.pci.domain != 0 ||
> +                    def->videos[i]->info.addr.pci.bus != 0 ||
> +                    def->videos[i]->info.addr.pci.slot != 2 ||
> +                    def->videos[i]->info.addr.pci.function != 0) {
> +                    qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s",
> +                                     _("Primary video card must have PCI address 0:0:2.0"));
> +                    goto error;
> +                }
> +            } else {
> +                def->videos[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> +                def->videos[i]->info.addr.pci.domain = 0;
> +                def->videos[i]->info.addr.pci.bus = 0;
> +                def->videos[i]->info.addr.pci.slot = 2;
> +                def->videos[i]->info.addr.pci.function = 0;
> +                if (qemuDomainPCIAddressReserveSlot(addrs, 2) < 0)
> +                    goto error;
> +            }

  Ah, okay, understood ...

>          } else {
> +            if (def->videos[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> +                continue;
>              if (qemuDomainPCIAddressSetNextAddr(addrs, &def->videos[i]->info) < 0)
>                  goto error;
>          }
>      }
>      for (i = 0; i < def->ncontrollers ; i++) {
> -        if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> -            continue;
>          /* FDC lives behind the ISA bridge */
>          if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_FDC)
>              continue;
> @@ -2057,12 +2085,27 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs)
>          /* First IDE controller lives on the PIIX3 at slot=1, function=1 */
>          if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE &&
>              def->controllers[i]->idx == 0) {
> -            def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> -            def->controllers[i]->info.addr.pci.domain = 0;
> -            def->controllers[i]->info.addr.pci.bus = 0;
> -            def->controllers[i]->info.addr.pci.slot = 1;
> -            def->controllers[i]->info.addr.pci.function = 1;
> +            if (def->controllers[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> +                if (def->videos[i]->info.addr.pci.domain != 0 ||
> +                    def->videos[i]->info.addr.pci.bus != 0 ||
> +                    def->videos[i]->info.addr.pci.slot != 2 ||
> +                    def->videos[i]->info.addr.pci.function != 0) {
> +                    qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s",
> +                                     _("Primary IDE controller must have PCI address 0:0:1.1"));
> +                    goto error;
> +                }
> +            } else {
> +                def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> +                def->controllers[i]->info.addr.pci.domain = 0;
> +                def->controllers[i]->info.addr.pci.bus = 0;
> +                def->controllers[i]->info.addr.pci.slot = 1;
> +                def->controllers[i]->info.addr.pci.function = 1;
> +                if (qemuDomainPCIAddressReserveSlot(addrs, 1) < 0)
> +                    goto error;
> +            }
>          } else {
> +            if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> +                continue;
>              if (qemuDomainPCIAddressSetNextAddr(addrs, &def->controllers[i]->info) < 0)
>                  goto error;
>          }

  Okay, ACK !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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