[libvirt] [PATCH v4 2/4] conf: Implement isolation rules

[Laine Stump]

On 07/15/2017 11:30 AM, Andrea Bolognani wrote:
> These rules will make it possible for libvirt to
> automatically assign PCI addresses in a way that
> respects any isolation constraints devices might
> have.
>
> Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> Reviewed-by: Laine Stump <laine at laine.org>
> ---
>  src/conf/domain_addr.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++---
>  src/conf/domain_addr.h |  3 +++
>  2 files changed, 72 insertions(+), 4 deletions(-)

[...]

>
> @@ -763,7 +797,7 @@ static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
>  virDomainPCIAddressGetNextAddr(virDomainPCIAddressSetPtr addrs,
>                                 virPCIDeviceAddressPtr next_addr,
>                                 virDomainPCIConnectFlags flags,
> -                               unsigned int isolationGroup ATTRIBUTE_UNUSED,
> +                               unsigned int isolationGroup,
>                                 int function)
>  {
>      virPCIDeviceAddress a = { 0 };
> @@ -779,12 +813,41 @@ virDomainPCIAddressGetNextAddr(virDomainPCIAddressSetPtr addrs,
>      else
>          a.function = function;
>  
> -    /* "Begin at the beginning," the King said, very gravely, "and go on
> -     * till you come to the end: then stop." */
> +    /* When looking for a suitable bus for the device, start by being
> +     * very strict and ignoring all those where the isolation groups
> +     * don't match. This ensures all devices sharing the same isolation
> +     * group will end up on the same bus */
>      for (a.bus = 0; a.bus < addrs->nbuses; a.bus++) {
>          virDomainPCIAddressBusPtr bus = &addrs->buses[a.bus];
>          bool found = false;
>  
> +        if (bus->isolationGroup != isolationGroup)
> +            continue;
> +
> +        a.slot = bus->minSlot;

Something for a followup patch - it occurred to me while looking at this
that due to your earlier patch changing the "find next" algorithm to
always start over at bus='0' slot='0',
virDomainPCIAddressFindUnusedFunctionOnBus() is now always called with
a.slot == bus->minSlot. We should change that function internally to
start at bus->minSlot rather than starting at a.slot - then we can
remove the "pre-load" of a.slot = bus->minSlot in this function.
Behavior will be identical, but the code will be easier for an outsider
to follow (and look less like it has been hacked up multiple times from
its original form :-P)

My Reviewed-By still stands BTW.


> +
> +        if (virDomainPCIAddressFindUnusedFunctionOnBus(bus, &a, function,
> +                                                       flags, &found) < 0) {
> +            goto error;
> +        }
> +
> +        if (found)