On 03/14/2013 10:20 AM, Daniel P. Berrange 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@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
On Wed, Mar 13, 2013 at 12:03:49PM -0400, Stefan Berger wrote: 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