[libvirt] [PATCHv2] conf: more useful error message when pci function is out of range

[Ján Tomko]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150728/6098952d/attachment-0001.sig>