[libvirt] [PATCH v2 06/10] conf: Extract UUID parsing into its own function

[Martin Kletzander]

On Thu, Dec 10, 2015 at 03:17:01PM -0500, John Ferlan wrote:
>
>
>On 12/01/2015 12:35 PM, Martin Kletzander wrote:
>> Create virDomainDefParseUUID that parses only the UUID from XML
>> definition.  This will be used in future patch(es).
>>
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>>  src/conf/domain_conf.c | 64 +++++++++++++++++++++++++++++++++++---------------
>>  1 file changed, 45 insertions(+), 19 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index de6853a4dbd0..7a295da507c4 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -14446,6 +14446,49 @@ virDomainVcpuParse(virDomainDefPtr def,
>>      return ret;
>>  }
>>
>> +/*
>> + * Parse domain's UUID, optionally generating it if missing.
>> + */
>> +static int
>> +virDomainDefParseUUID(virDomainDefPtr def,
>> +                      xmlXPathContextPtr ctxt,
>> +                      bool required,
>> +                      bool *generated)
>> +{
>> +    char *tmp = virXPathString("string(./uuid[1])", ctxt);
>> +
>> +    if (generated)
>> +        *generated = false;
>> +
>> +    if (tmp) {
>> +        if (virUUIDParse(tmp, def->uuid) < 0) {
>> +            virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                           "%s", _("malformed uuid element"));
>> +            VIR_FREE(tmp);
>> +            return -1;
>> +        }
>> +        VIR_FREE(tmp);
>> +        return 0;
>
>since you're returning anyways...
>
>    int ret;
>    if ((ret = virUUIDParse(...)) < 0)
>        virReportError(...)
>    VIR_FREE(tmp);
>    return ret;
>
>Your code works - IDC either way.
>

I like your way a bit more.

>> +    }
>> +
>> +    if (required) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                       _("Missing required UUID"));
>> +        return -1;
>> +    }
>> +
>> +    if (virUUIDGenerate(def->uuid)) {
>
>if (virUUIDGenerate(...) < 0)
>

And this one is just my fault, of course it needs to be "< 0".

>ACK with at least this one fixed - just to conform with other error path
>uses...
>
>John
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       "%s", _("Failed to generate UUID"));
>> +        return -1;
>> +    }
>> +
>> +    if (generated)
>> +        *generated = true;
>> +
>> +    return 0;
>> +}
>> +
>>  static int
>>  virDomainDefParseName(virDomainDefPtr def,
>>                        xmlXPathContextPtr ctxt)
>> @@ -14587,25 +14630,8 @@ virDomainDefParseXML(xmlDocPtr xml,
>>      if (virDomainDefParseName(def, ctxt) < 0)
>>          goto error;
>>
>> -    /* Extract domain uuid. If both uuid and sysinfo/system/entry/uuid
>> -     * exist, they must match; and if only the latter exists, it can
>> -     * also serve as the uuid. */
>> -    tmp = virXPathString("string(./uuid[1])", ctxt);
>> -    if (!tmp) {
>> -        if (virUUIDGenerate(def->uuid)) {
>> -            virReportError(VIR_ERR_INTERNAL_ERROR,
>> -                           "%s", _("Failed to generate UUID"));
>> -            goto error;
>> -        }
>> -        uuid_generated = true;
>> -    } else {
>> -        if (virUUIDParse(tmp, def->uuid) < 0) {
>> -            virReportError(VIR_ERR_INTERNAL_ERROR,
>> -                           "%s", _("malformed uuid element"));
>> -            goto error;
>> -        }
>> -        VIR_FREE(tmp);
>> -    }
>> +    if (virDomainDefParseUUID(def, ctxt, false, &uuid_generated) < 0)
>> +        goto error;
>>
>>      /* Extract short description of domain (title) */
>>      def->title = virXPathString("string(./title[1])", ctxt);
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151211/35c44db1/attachment-0001.sig>