[libvirt] [PATCH] iotune: setting an invalid value now reports error

[Ján Tomko]

On 08/21/2014 02:28 PM, Erik Skultety wrote:
> When trying to set an invalid value into iotune element, standard
> behavior was to not report any error, rather to reset all affected
> subelements of the iotune element back to 0 which results in ignoring
> those particular subelements by XML generator. Patch further
> examines the return code of the virXPathULongLong function
> and in case of an invalid non-integer value raises an error.
> Fixed to preserve consistency with invalid value checking
> of other elements.
> 
> Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1131811
> ---
>  src/conf/domain_conf.c | 55 +++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 43 insertions(+), 12 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9557020..592aa94 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5416,6 +5416,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>      char *mirrorType = NULL;
>      int expected_secret_usage = -1;
>      int auth_secret_usage = -1;
> +    int ret = 0;
>  
>      if (!(def = virDomainDiskDefNew()))
>          return NULL;
> @@ -5644,40 +5645,70 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>                      goto error;
>                  }
>              } else if (xmlStrEqual(cur->name, BAD_CAST "iotune")) {
> -                if (virXPathULongLong("string(./iotune/total_bytes_sec)",
> +                ret = virXPathULongLong("string(./iotune/total_bytes_sec)",
>                                        ctxt,

The indentation is off here - c and " should be on the same column.

> -                                      &def->blkdeviotune.total_bytes_sec) < 0) {
> +                                      &def->blkdeviotune.total_bytes_sec);
> +                if (ret < 0 && ret != -2) {
>                      def->blkdeviotune.total_bytes_sec = 0;
> +                } else if (ret == -2) {
> +                    virReportError(VIR_ERR_XML_ERROR, "%s",
> +                                   _("total throughput limit must be an integer"));
> +                    goto error;
>                  }

I'd rewrite these conditions as
if (ret == -2) {
    goto error;
} else if (ret < 0) {
}

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140822/4367446a/attachment-0001.sig>