[libvirt] vTPM support in libvirt

[Cole Robinson]

On 06/27/2009 04:32 PM, Andreas Sommer wrote:
> Index: src/domain_conf.c
> ===================================================================
> RCS file: /data/cvs/libvirt/src/domain_conf.c,v
> retrieving revision 1.88
> diff -u -r1.88 domain_conf.c
> --- src/domain_conf.c	25 Jun 2009 13:55:58 -0000	1.88
> +++ src/domain_conf.c	27 Jun 2009 15:12:28 -0000
> @@ -461,6 +461,8 @@
>      for (i = 0 ; i < def->nhostdevs ; i++)
>          virDomainHostdevDefFree(def->hostdevs[i]);
>      VIR_FREE(def->hostdevs);
> +    
> +    VIR_FREE(def->tpm);
>  
>      VIR_FREE(def->os.type);
>      VIR_FREE(def->os.arch);
> @@ -1622,6 +1624,62 @@
>  }
>  
>  
> +/* Parse the XML definition for a vTPM device (<tpm instance="optional_instance_number">) */
> +static virDomainTpmDefPtr
> +virDomainTpmDefParseXML(virConnectPtr conn,
> +                             xmlNodePtr node, int flags) {
> +

Whitespace is funky here.

> +    flags = flags; /* TODO: Andreas Sommer <AndiDog at web.de> flags unused, what are they used for */
> +

If you change the function definition to be:

virDomainTpmDefParseXML(..., int flags ATTRIBUTE_UNUSED);

You will avoid the compiler warning. The flags attribute is only relevant for
formatting certain XML, and they correctly don't apply here.

> +    virDomainTpmDefPtr def;
> +    char *instance = NULL;
> +    
> +    if (VIR_ALLOC(def) < 0) {
> +        virReportOOMError(conn);
> +        return NULL;
> +    }
> +
> +    instance = virXMLPropString(node, "instance");
> +
> +    /* Attribute "instance" is optional*/
> +    if (instance) {
> +        /* "instance" is defined as a number >= 0 */
> +        if (!c_isdigit(*instance) || *instance == '0') {
> +            virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> +                                 _("TPM instance attribute '%s' found, but must be a number >= 0 (make sure no whitespaces or leading zeroes are contained)"), instance);
> +            goto error;
> +        }
> +        
> +        const char *check = &instance[1];
> +        while (*check && c_isdigit(*check)) ++check;
> +        if (*check) {
> +            virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> +                                 _("TPM instance attribute '%s' contains non-digits, must be a number >= 0 (make sure no whitespaces or leading zeroes are contained)"), instance);
> +            goto error;
> +        }
> +        
> +        int err = virStrToLong_l("instance", NULL, 10, &def->instance);
> +        if (err) {
> +            virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> +                                 _("TPM instance '%s' is not a valid number"), instance);
> +            goto error;
> +        }
> +    }
> +    else
> +        def->instance = -1;
> +    
> +cleanup:
> +    VIR_FREE(instance);
> +
> +    return def;
> +
> +error:
> +    VIR_FREE(def);
> +    def = NULL;
> +    goto cleanup;
> +}
> +
> +
>  static virDomainSoundDefPtr
>  virDomainSoundDefParseXML(virConnectPtr conn,
>                            const xmlNodePtr node,
> @@ -2617,6 +2675,30 @@
>          def->inputs[def->ninputs] = input;
>          def->ninputs++;
>      }
> +    
> +    
> +    /* analysis of the vTPM device */
> +    if ((n = virXPathNodeSet(conn, "./devices/tpm", ctxt, &nodes)) < 0) {
> +        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> +                             "%s", _("cannot extract vTPM device"));
> +        goto error;
> +    }
> +    if (n > 1) {
> +        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> +                             "%s", _("there can only be one vTPM device"));
> +        goto error;
> +    }
> +    if (n == 1) {
> +        virDomainTpmDefPtr tpm = virDomainTpmDefParseXML(conn,
> +                                                         nodes[0],
> +                                                         flags);
> +        
> +        if (!tpm)
> +            goto error;
> +        
> +        def->tpm = tpm;
> +    }
> +    VIR_FREE(nodes);
>  
>  
>      /* analysis of the sound devices */
> @@ -3856,6 +3938,13 @@
>                  goto cleanup;
>      }
>  
> +    if (def->tpm) {
> +        virBufferVSprintf(&buf, "    <tpm");
> +        if (def->tpm->instance >= 0)
> +            virBufferVSprintf(&buf, " instance='%ld'", def->tpm->instance);
> +        virBufferVSprintf(&buf, "/>\n");
> +    }
> +

It's a bit overkill, but I think it's better to follow existing convention and
move this formatting to a function like 'virDomainTpmDefFormat'.

Thanks,
Cole