[libvirt] [PATCH V1 2/6] Parse TPM in domain XML

[Daniel P. Berrange]

On Wed, Mar 13, 2013 at 12:03:50PM -0400, Stefan Berger wrote:
> Parse the domain XML with TPM support.
> 
> Convert the strings from QEMU's QMP TPM commands into
> capability flags.
> 
> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
> 
> ---
>  docs/schemas/domaincommon.rng |   43 ++++++
>  src/conf/domain_conf.c        |  268 ++++++++++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h        |   34 +++++
>  src/libvirt_private.syms      |    5 
>  src/qemu/qemu_capabilities.c  |   59 +++++++++
>  5 files changed, 409 insertions(+)

The QEMU changes should be a separate patch from any XML schema
changes.

> @@ -1515,6 +1522,24 @@ void virDomainHostdevDefClear(virDomainH
>      }
>  }
>  
> +void virDomainTPMDefFree(virDomainTPMDefPtr def)
> +{
> +    if (!def)
> +        return;
> +
> +    switch (def->type) {
> +    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> +        VIR_FREE(def->data.passthrough.path);
> +        VIR_FREE(def->data.passthrough.cancel_path);
> +        break;
> +    case VIR_DOMAIN_TPM_TYPE_LAST:
> +        break;
> +    }
> +
> +    virDomainDeviceInfoClear(&def->info);
> +    VIR_FREE(def);
> +}
> +
>  void virDomainHostdevDefFree(virDomainHostdevDefPtr def)
>  {
>      if (!def)
> @@ -1776,6 +1801,8 @@ void virDomainDefFree(virDomainDefPtr de
>  
>      virDomainRNGDefFree(def->rng);
>  
> +    virDomainTPMDefFree(def->tpm);
> +
>      VIR_FREE(def->os.type);
>      VIR_FREE(def->os.machine);
>      VIR_FREE(def->os.init);
> @@ -6312,6 +6339,192 @@ error:
>      goto cleanup;
>  }
>  
> +/*
> + * Check whether the given base path, e.g.,  /sys/class/misc/tpm0/device,
> + * is the sysfs entry of a TPM. A TPM sysfs entry should be uniquely
> + * recognizable by the file entries 'pcrs' and 'cancel'.
> + * Upon success 'true' is returned and the basebath buffer has '/cancel'
> + * appended.
> + */
> +static bool
> +virDomainTPMCheckSysfsCancel(char *basepath, size_t bufsz)
> +{
> +    char *path = NULL;
> +    struct stat statbuf;
> +
> +    if (virAsprintf(&path, "%s/pcrs", basepath) < 0) {
> +        virReportOOMError();
> +        goto error;
> +    }
> +    if (stat(path, &statbuf) == -1 || !S_ISREG(statbuf.st_mode))
> +        goto error;
> +
> +    VIR_FREE(path);
> +
> +    if (virAsprintf(&path, "%s/cancel", basepath) < 0) {
> +        virReportOOMError();
> +        goto error;
> +    }
> +
> +    if (stat(path, &statbuf) == -1 || !S_ISREG(statbuf.st_mode))
> +        goto error;
> +
> +    if (!virStrncpy(basepath, path, strlen(path) + 1, bufsz)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Basepath buffer is too small"));
> +        goto error;
> +    }
> +
> +    VIR_FREE(path);
> +
> +    return true;
> +
> +error:
> +    VIR_FREE(path);
> +    return false;
> +}
> +
> +
> +static char *
> +virDomainTPMFindCancelPath(void)
> +{
> +    unsigned int idx;
> +    int len;
> +    DIR *pnp_dir;
> +    char path[100], *p;
> +    struct dirent entry, *result;
> +    bool found = false;
> +
> +    snprintf(path, sizeof(path), "/sys/class/misc");
> +    pnp_dir = opendir(path);
> +    if (pnp_dir != NULL) {
> +        while (readdir_r(pnp_dir, &entry, &result) == 0 &&
> +               result != NULL) {
> +            if (sscanf(entry.d_name, "tpm%u%n", &idx, &len) < 1 ||
> +                len <= strlen("tpm") ||
> +                len != strlen(entry.d_name)) {
> +                continue;
> +            }
> +            snprintf(path, sizeof(path), "/sys/class/misc/%s/device",
> +                     entry.d_name);
> +            if (!virDomainTPMCheckSysfsCancel(path, sizeof(path))) {
> +                continue;
> +            }
> +
> +            found = true;
> +            break;
> +        }
> +        closedir(pnp_dir);
> +    }
> +
> +    if (found) {
> +        if (!(p = strdup(path)))
> +            virReportOOMError();
> +        return p;
> +    }
> +
> +    return NULL;
> +}

These two funtions have nothing todo with the XML schema so should
not be in this file. Either some helper file in src/util or in the
QEMU code.

> +    switch (def->type) {
> +    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> +        path = virXPathString("string(./backend/device/@path)", ctxt);
> +        if (!path && !(path = strdup(VIR_DOMAIN_TPM_DEFAULT_DEVICE))) {
> +            virReportOOMError();
> +            goto error;
> +        }
> +        def->data.passthrough.path = path;
> +        path = NULL;
> +        /* cancel_path is read-only */
> +        def->data.passthrough.cancel_path = virDomainTPMFindCancelPath();

This data is driver specific state so should not be in the
XML schema structs - it should be in QEMU code where needed

> +# define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"
> +
> +typedef struct _virDomainTPMDef virDomainTPMDef;
> +typedef virDomainTPMDef *virDomainTPMDefPtr;
> +struct _virDomainTPMDef {
> +    enum virDomainTPMBackendType type;
> +    virDomainDeviceInfo info;
> +    enum virDomainTPMModel model;
> +    union {
> +        struct {
> +            char *path;
> +            char *cancel_path;

Per previous comments 'cancel_path' should not be in this struct.

> +        } passthrough;
> +    } data;
> +};
> +

> Index: libvirt/src/qemu/qemu_capabilities.c
> ===================================================================
> --- libvirt.orig/src/qemu/qemu_capabilities.c
> +++ libvirt/src/qemu/qemu_capabilities.c
> @@ -2110,6 +2110,63 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEM
>  
>  
>  static int
> +virQEMUCapsProbeQMPTPM(virQEMUCapsPtr qemuCaps,
> +                       qemuMonitorPtr mon)
> +{
> +    int nentries, i;
> +    char **entries = NULL;
> +    struct typeToCaps {
> +        int type;
> +        enum virQEMUCapsFlags caps;
> +    };
> +    const struct typeToCaps tpmTypesToCaps[] = {
> +        {
> +            .type = VIR_DOMAIN_TPM_TYPE_PASSTHROUGH,
> +            .caps = QEMU_CAPS_DEVICE_TPM_PASSTHROUGH,
> +        },
> +    };
> +    const struct typeToCaps tpmModelsToCaps[] = {
> +        {
> +            .type = VIR_DOMAIN_TPM_MODEL_TIS,
> +            .caps = QEMU_CAPS_DEVICE_TPM_TIS,
> +        },
> +    };
> +
> +    if ((nentries = qemuMonitorGetTPMModels(mon, &entries)) < 0)
> +        return -1;
> +
> +    if (nentries > 0) {
> +        for (i = 0; i < ARRAY_CARDINALITY(tpmModelsToCaps); i++) {
> +            const char *needle = virDomainTPMModelTypeToString(
> +                tpmModelsToCaps[i].type);
> +            if (virStrArrayHasString(entries, nentries, needle))
> +                virQEMUCapsSet(qemuCaps, tpmModelsToCaps[i].caps);
> +        }
> +        for (i = 0; i < nentries; i++)
> +            VIR_FREE(entries[i]);

If you make the monitor commands return a NULL terminated list,
you can just use virStringFreeList. This also avoids the need
to pass around 'nentries'; to virStrArrayHasString.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|