[libvirt] [PATCH v3 10/18] qemu: set/use proper pciConnectFlags during hotplug

[Andrea Bolognani]

On Tue, 2016-09-20 at 15:14 -0400, Laine Stump wrote:
> Before now, all the qemu hotplug functions assumed that all devices to
> be hotplugged were legacy PCI endpoint devices
> (VIR_PCI_CONNECT_TYPE_PCI_DEVICE). This worked out "okay", because all
> devices *are* legacy PCI endpoint devices on x86/440fx machinetypes,
> and hotplug didn't work properly on machinetypes using PCIe anyway
> (hotplugging onto a legacy PCI slot doesn't work, and until commit
> b87703cf any attempt to manually specify a PCIe address for a
> hotplugged device would be erroneously rejected).
> 
> This patch makes all qemu hotplug operations honor the pciConnectFlags
> set by the single all-knowing function
> qemuDomainDeviceConnectFlagsInternal().

[...]
> @@ -485,17 +485,17 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs,
>  
>  int
>  virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,
> -                              virDomainDeviceInfoPtr dev)
> +                              virDomainDeviceInfoPtr dev,
> +                              virDomainPCIConnectFlags flags)
>  {
>      int ret = -1;
>      char *addrStr = NULL;
> -    /* Flags should be set according to the particular device,
> -     * but only the caller knows the type of device. Currently this
> -     * function is only used for hot-plug, though, and hot-plug is
> -     * only supported for standard PCI devices, so we can safely use
> -     * the setting below */
> -    virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE |
> -                                      VIR_PCI_CONNECT_TYPE_PCI_DEVICE);
> +
> +    /* if pciConnectFlags is 0, the particular model of this device

s/pciConnectFlags/flags/

[...]
> @@ -2032,6 +2032,33 @@ qemuDomainAssignAddresses(virDomainDefPtr def,
>      return 0;
>  }
>  
> +/**
> + * qemuDomainEnsurePCIAddress:
> + *
> + * if @dev should have a PCI address but doesn't, assign
> + * an address on a compatible PCI bus. Validate that any
> + * existing address is on a compatible bus.
> + *
> + * @obj: the virDomainObjPtr for the domain. This will include
> + *       qemuCaps and address cache (if there is one)
> + *
> + * @dev: the device that we need to ensure has a PCI address
> + *
> + * returns 0 on success (and any new address set in dev->...info) -1
> + * on failure.

The new address is set, not returned, so that information
doesn't belong in this section. The description already
mention the fact that an address will be assigned if needed
anyway.

[...]
> @@ -1584,10 +1590,12 @@ qemuDomainChrRemove(virDomainDefPtr vmdef,
>  }
>  
>  static int
> -qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def,
> +qemuDomainAttachChrDeviceAssignAddr(virDomainObjPtr vm,
>                                      qemuDomainObjPrivatePtr priv,

Please change the type of the first argument from
virDomainDefPtr to virDomainObjPtr in a separate commit,
and use that chance to remove the qemuDomainObjPrivatePtr
argument altogether - you can retrieve it from @vm the
same way you already retrieve the DomainDef.

ACK

-- 
Andrea Bolognani / Red Hat / Virtualization