[libvirt] [PATCH 1/5] qemu: don't assume slot 0 is unused/reserved.

[John Ferlan]

On 11/19/2015 01:24 PM, Laine Stump wrote:
> When qemuAssignDevicePCISlots() is looking for companion controllers
> for a USB controller that has no PCI address specified, it initializes
> a virDevicePCIAddress to 0000:00:00.0, fills it in with the
> companion's address if one is found, then checks whether or not there
> was a find based on slot == 0. On a system with a single PCI bus, that
> is a valid way to check, because slot 0 is reserved, but on most other
> PCI buses, slot 0 is not reserved, and is open for use by any
> device. This patch adds a separate bool that is set when a companion
> is found rather than relying on the faulty information provided with
> "slot == 0".
> ---
>  src/qemu/qemu_command.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 8fdf90c..3c867de 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2519,11 +2519,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
>          /* USB2 needs special handling to put all companions in the same slot */
>          if (IS_USB2_CONTROLLER(def->controllers[i])) {
>              virDevicePCIAddress addr = { 0, 0, 0, 0, false };
> +            bool foundAddr = false;
> +
>              memset(&tmp_addr, 0, sizeof(tmp_addr));
>              for (j = 0; j < i; j++) {
>                  if (IS_USB2_CONTROLLER(def->controllers[j]) &&
>                      def->controllers[j]->idx == def->controllers[i]->idx) {
>                      addr = def->controllers[j]->info.addr.pci;
> +                    foundAddr = true;
>                      break;
>                  }
>              }
> @@ -2544,7 +2547,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
>                  break;
>              }
>  
> -            if (addr.slot == 0) {
> +            if (!foundAddr) {

This will make more sense once we hit patch 2. Just below here where
multi = 0, can we take the opportunity to either update it now (I'm fine
with just changing in this patch as long as it's attributed in the
commit message).  Or perhaps we add a patch before this one that just
changes to use the "right" TRISTATE value.

ACK what's here (since it seems reasonable to me).  I can trust you know
how to add the appropriate TRISTATE patch too ;-)

John


>                  /* This is the first part of the controller, so need
>                   * to find a free slot & then reserve a function */
>                  if (virDomainPCIAddressGetNextSlot(addrs, &tmp_addr, flags) < 0)
>