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@xxxxxxxxxx> > --- > 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list