On Thu, Jul 23, 2015 at 04:37:05PM -0400, Laine Stump wrote: > If a pci address had a function number out of range, the error message > would be: > > Insufficient specification for PCI address > > which is logged by virDevicePCIAddressParseXML() after > virDevicePCIAddressIsValid returns a failure. > > This patch enhances virDevicePCIAddressIsValid() to optionally report > the error itself (since it is the place that decides which part of the > address is "invalid"), and uses that feature when calling from > virDevicePCIAddressParseXML(), so that the error will be more useful, > e.g.: > > Invalid PCI address function=0x8, must be <= 7 > > Previously, virDevicePCIAddressIsValid didn't check for the > theoretical limits of domain or bus, only for slot or function. While > adding log messages, we also correct that ommission. > (The RNG for PCI > addresses already enforces this limit, which by the way means that we > can't add any negative tests for this - as far as I know our > domainschematest has no provisions for passing XML that is supposed to > fail). > Any XML file with a name ending in *-invalid.xml is expected to fail validation, see tests/schematestutils.sh > Note that virDevicePCIAddressIsValid() can only check against the > absolute maximum attribute values for *any* possible PCI controller, > not for the actual maximums of the specific controller that this > device is attaching to; fortunately there is later more specific > validation for guest-side PCI addresses when building the set of > assigned PCI addresses. For host-side PCI addresses (e.g. for > <hostdev> and for network device pools), we rely on the error that > will be logged when it is found that the device doesn't actually > exist. > > This resolves: > > https://bugzilla.redhat.com/show_bug.cgi?id=1004596 > --- > Change from V1: > > V1 simply removed the validation from virDevicePCIAddressParseXML() in > favor of allowing things later in the parse process to figure out the > out-of-bounds conditions, but jtomko pointed out that there are other > uses of virDevicePCIAddressParseXML() aside from the guest-side PCI > addresses of devices, and that the other uses relied on the one > validation check in virDevicePCIAddressParseXML() to make sure the > address was sensible. So instead of simply removing the check, I had > to make it smarter about what it logged. In the process, I added > checks for max domain and bus. > > > src/conf/device_conf.c | 52 +++++++++++++++++++++++++++++++++++++++++--------- > src/conf/device_conf.h | 5 +++-- > src/conf/domain_conf.c | 2 +- > 3 files changed, 47 insertions(+), 12 deletions(-) > > diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c > index e7b7957..4e15d38 100644 > --- a/src/conf/device_conf.c > +++ b/src/conf/device_conf.c > @@ -1,7 +1,7 @@ > /* > * device_conf.c: device XML handling > * > - * Copyright (C) 2006-2012 Red Hat, Inc. > + * Copyright (C) 2006-2015 Red Hat, Inc. > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public > @@ -55,12 +55,49 @@ VIR_ENUM_IMPL(virNetDevFeature, > "rdma", > "txudptnl") > > -int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr) > +int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr, Returning bool would match the usage more closely, but that's pre-existing. ACK Jan
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list