[libvirt] [PATCH V1 1/6] Add QMP probing for TPM

[Daniel P. Berrange]

On Thu, Mar 14, 2013 at 10:44:32AM -0400, Stefan Berger wrote:
> On 03/14/2013 10:20 AM, Daniel P. Berrange wrote:
> >On Wed, Mar 13, 2013 at 12:03:49PM -0400, Stefan Berger wrote:
> >>Probe for QEMU's QMP TPM support by querying the lists of
> >>supported TPM models (query-tpm-models) and backend types
> >>(query-tpm-types).
> >>
> >>The setting of the capability flags following the strings
> >>returned from the commands above is only provided in the
> >>patch where domain_conf.c gets TPM support due to dependencies
> >>on functions only introduced there.
> >>
> >>Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
> >>
> >>---
> >>  src/libvirt_private.syms     |    1
> >>  src/qemu/qemu_capabilities.c |   61 ++++++++++++++++++++++++++++
> >>  src/qemu/qemu_capabilities.h |    3 +
> >>  src/qemu/qemu_monitor.c      |   44 ++++++++++++++++++++
> >>  src/qemu/qemu_monitor.h      |    6 ++
> >>  src/qemu/qemu_monitor_json.c |   91 +++++++++++++++++++++++++++++++++++++++++++
> >>  src/qemu/qemu_monitor_json.h |    8 +++
> >>  src/util/virutil.c           |   14 ++++++
> >>  src/util/virutil.h           |    3 +
> >>  9 files changed, 231 insertions(+)
> >>
> >>Index: libvirt/src/qemu/qemu_capabilities.c
> >>===================================================================
> >>--- libvirt.orig/src/qemu/qemu_capabilities.c
> >>+++ libvirt/src/qemu/qemu_capabilities.c
> >>@@ -210,6 +210,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAS
> >>                "rng-random", /* 130 */
> >>                "rng-egd",
> >>+              "tpm-passthrough",
> >>+              "tpm-tis",
> >>      );
> >>  struct _virQEMUCaps {
> >>Index: libvirt/src/qemu/qemu_capabilities.h
> >>===================================================================
> >>--- libvirt.orig/src/qemu/qemu_capabilities.h
> >>+++ libvirt/src/qemu/qemu_capabilities.h
> >>@@ -171,6 +171,8 @@ enum virQEMUCapsFlags {
> >>      QEMU_CAPS_OBJECT_RNG_RANDOM  = 130, /* the rng-random backend for
> >>                                             virtio rng */
> >>      QEMU_CAPS_OBJECT_RNG_EGD     = 131, /* EGD protocol daemon for rng */
> >>+    QEMU_CAPS_DEVICE_TPM_PASSTHROUGH = 132, /* -tpmdev passthrough */
> >>+    QEMU_CAPS_DEVICE_TPM_TIS     = 133, /* -device tpm_tis */
> >I'm wondering whether we really need to spend 2 capability bits on
> >this. Is it possible to build QEMU such that we have one, but not
> >the other ? If not then one capability would suffice
> 
> I had it with a single capability bit QEMU_CAPS_DEVICE_TPM *and* was
> reading the strings about the supported models and backend types
> from QEMU via QMP and held them in qemuCaps. When building the
> command line I then compared the intended model and backend type to
> be used against the list held in qemuCaps. That went fine until I
> wrote the test. There we end up using a pseudo executable
> (/usr/bin/qemu or so) as well and the test failed because it wasn't
> probed for the models and types. So I ended up changing the code so
> that I can set those bits inside the tests rather than having to set
> the needed strings in the array.
> 
> Presumably query-tpm-models and query-tpm-lists allows us to report
> an error from libvirt about non-availability of a model/backend type
> rather than having QEMU report an error which we reflect to the
> user.
> 
> 
> +
> +static int
> +qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, const char *qmpCmd,
> +                              char ***array)
> +{
> +    int ret;
> +    virJSONValuePtr cmd;
> +    virJSONValuePtr reply = NULL;
> +    virJSONValuePtr data;
> +    char **list = NULL;
> +    int n = 0;
> +    size_t i;
> +
> +    *array = NULL;
> +
> +    if (!(cmd = qemuMonitorJSONMakeCommand(qmpCmd, NULL)))
> +        return -1;
> +
> +    ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> +
> +    if (ret == 0)
> +        ret = qemuMonitorJSONCheckError(cmd, reply);
> +
> +    if (ret < 0)
> +        goto cleanup;
> +
> +    ret = -1;
> +
> +    if (!(data = virJSONValueObjectGet(reply, "return"))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("%s reply was missing return data"),
> +                       qmpCmd);
> +        goto cleanup;
> +    }
> 
> >I think everything above this point should be in the main monitor APIs,
> >and this method concern itself solely with converting the virJSONValuePtr
> >into a string array, not actually executing command. This makes the code
> >more portable for use in commands which need control over the command
> >execution to handle errors / pass arguments.
> 
> Hm, this surprises me since this would put JSON specific code into
> the monitor APIs (qemu_monitor.c) while qemu_monitor.c typically
> only serves as a dispatcher for either text monitor or json monitor.

No, you mis-understand me - I mean move the code into these 2 functions:

+int qemuMonitorJSONGetTPMModels(qemuMonitorPtr mon,
+                                char ***tpmmodels)
+{
+    return qemuMonitorJSONGetStringArray(mon, "query-tpm-models", tpmmodels);
+}
+
+
+int qemuMonitorJSONGetTPMTypes(qemuMonitorPtr mon,
+                               char ***tpmtypes)
+{
+    return qemuMonitorJSONGetStringArray(mon, "query-tpm-types", tpmtypes);
+}



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 :|