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(a)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