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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list