[libvirt] [PATCH 2/2] device_conf: PCI address pci_0000_00_00_0 is also valid

[Eric Blake]

On 08/05/2014 10:18 AM, Pavel Hrdina wrote:
> All the variables used for pci addr in virDevicePCIAddress are
> unsigned and therefore in this validation function they never cannot
> be less than zero. This check will fail only for the address above,
> but that address is valid so we can safely return true instead.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1055331
> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  src/conf/device_conf.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
> index 61c73dc..797b2b6 100644
> --- a/src/conf/device_conf.c
> +++ b/src/conf/device_conf.c
> @@ -44,7 +44,8 @@ bool virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr)
>      /* PCI bus has 32 slots and 8 functions per slot */
>      if (addr->slot >= 32 || addr->function >= 8)
>          return false;
> -    return addr->domain || addr->bus || addr->slot;
> +
> +    return true;
>  }

I think we're going to need more than this :(

domain_conf.c uses it to feed this code in virDomainNetFindIdx:

    bool PCIAddrSpecified = virDomainDeviceAddressIsValid(&net->info,

VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);

Right now, if you omit an <address> element, it is 0-initialized and the
in-memory structure happens to be pci_0000_00_00_0, but PCIAddrSpecified
is set to false, which then lets us auto-pick an appropriate (non-zero)
address.  If you explicitly specify an all-0 address, you get the same
behavior.

With your patch, it looks like you want to allow explicit specification
of an all-0 address.  But for that to work, we now need to modify
domain_conf.h (and/or device_conf.h) to ALSO have a bool flag that says
whether an address was specified (since you are allowing explicit
specification of all-0, where we should NOT auto-assign a different
address) or omitted.

NACK to this version; the correct solution is one where an explicit
all-0 address behaves differently than omitting <address>.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140805/f5b3d94b/attachment-0001.sig>