9:44 a.m.
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(a)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.
> Index:
libvirt/src/libvirt_private.syms===================================================================
> --- libvirt.orig/src/libvirt_private.syms
> +++ libvirt/src/libvirt_private.syms
> @@ -1886,6 +1886,7 @@ virSetUIDGIDWithCaps;
> virSkipSpaces;
> virSkipSpacesAndBackslash;
> virSkipSpacesBackwards;
> +virStrArrayHasString;
> virStrcpy;
> virStrIsPrint;
> virStrncpy;
> Index: libvirt/src/util/virutil.c
> ===================================================================
> --- libvirt.orig/src/util/virutil.c
> +++ libvirt/src/util/virutil.c
> @@ -3379,3 +3379,17 @@ cleanup:
> VIR_FREE(buf);
> return ret;
> }
> +
> +
> +bool
> +virStrArrayHasString(char **strings, size_t n_strings, const char *needle)
> +{
> + size_t i;
> +
> + for (i = 0; i < n_strings; i++) {
> + if (STREQ(strings[i], needle))
> + return true;
> + }
> +
> + return false;
> +}
> Index: libvirt/src/util/virutil.h
> ===================================================================
> --- libvirt.orig/src/util/virutil.h
> +++ libvirt/src/util/virutil.h
> @@ -297,4 +297,7 @@ int virGetDeviceUnprivSGIO(const char *p
> char * virGetUnprivSGIOSysfsPath(const char *path,
> const char *sysfs_dir);
>
> +bool virStrArrayHasString(char **strings, size_t n_strings,
> + const char *needle);
> +
> #endif /* __VIR_UTIL_H__ */
The virStrArrayHasString method doesn't appear to be used in this patch.
If some other patch needs this still, then it should be a separate patch
from the monitor code.
Ok, I'll split this off into a patch of its own.
Stefan