Re: [libvirt PATCH 05/10] virDomainDeviceCCWAddressParseXML: Use virXMLProp*

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 16, 2021 at 14:20:55 +0200, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake <twiederh@xxxxxxxxxx>
> ---
>  src/conf/device_conf.c | 51 +++++++++++++++++-------------------------
>  1 file changed, 21 insertions(+), 30 deletions(-)
> 
> diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
> index 951b7a348e..621ff1b476 100644
> --- a/src/conf/device_conf.c
> +++ b/src/conf/device_conf.c
> @@ -266,43 +266,34 @@ int
>  virDomainDeviceCCWAddressParseXML(xmlNodePtr node,
>                                    virDomainDeviceCCWAddress *addr)
>  {
> -    g_autofree char *cssid = virXMLPropString(node, "cssid");
> -    g_autofree char *ssid = virXMLPropString(node, "ssid");
> -    g_autofree char *devno = virXMLPropString(node, "devno");
> +    int cssid, ssid, devno;

No multiple variable declarations on single line.

>  
>      memset(addr, 0, sizeof(*addr));
>  
> +    if ((cssid = virXMLPropUInt(node, "cssid", 0, VIR_XML_PROP_OPTIONAL,
> +                                &addr->cssid)) < 0)
> +        return -1;
> +
> +    if ((ssid = virXMLPropUInt(node, "ssid", 0, VIR_XML_PROP_OPTIONAL,
> +                               &addr->ssid)) < 0)
> +        return -1;
> +
> +    if ((devno = virXMLPropUInt(node, "devno", 0, VIR_XML_PROP_OPTIONAL,
> +                                &addr->devno)) < 0)
> +        return -1;
> +
> +    if (!virDomainDeviceCCWAddressIsValid(addr)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Invalid specification for virtio ccw address: cssid='%u' ssid='%u' devno='%u'"),
> +                       addr->cssid, addr->ssid, addr->devno);

This misrepresents user's input and could even be confusing. While the
parser uses 0 for number base autodetection, libvirt outputs the values
in hex format. (See the appropriate case in virDomainDeviceInfoFormat)

Reporting them as base 10 numbers could lead to a red herring chase.

> +        return -1;
> +    }




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux