Thanks for your help so far. I changed the basics needed for vTPM support, which include XML <--> native configuration stuff (you will see it in the patch). I'm not really sure what else has to be done - maybe reading from xenstore?? Hope you can give me some tips on what is still missing. I did already try "./virsh domxml-from-native xen-xm some_config_file_that_contains_the_vtpm_line" and it gave me the correct XML answer :) Please find attached the unified diff. Best regards Andreas Daniel P. Berrange wrote:

On Fri, Jun 26, 2009 at 09:30:53AM +0100, Andreas Sommer wrote:

...

I made some changes to the code and would like to try it out by installing libvirt. As you said, there's an example on how to build and install it, but how do I need to configure it in order to get it installed on my system and not in some $HOME directory?? Yesterday I tried

./configure make make install

then virsh is installed but cannot find libvirt.so.0. Can you help me with that or edit the installation instructions on the website?

If you're just doing dev changes, it is best not to install it into your main system. Instead just build and run directly from the source tree.

eg, what I normally do is

./autogen.sh --enable-compile-warnings=error --prefix=/usr --localstatedir=/var --sysconfdir=/etc make

And *not* make install.

Then as root just stop the normal libvirtd daemon that comes with your distro, and run the one you just compiled manually, eg

$ su - root # /etc/init.d/libvirtd stop # /home/berrange/work/libvirt/qemud/libvirtd

And likewise, use the 'virsh' binary directly from the source tree

$ cd $HOME/work/libvirt/src $ ./virsh

Daniel

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) { + + flags = flags; /* TODO: Andreas Sommer <AndiDog@web.de> flags unused, what are they used for */ + + 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"); + } + for (n = 0 ; n < def->nsounds ; n++) if (virDomainSoundDefFormat(conn, &buf, def->sounds[n]) < 0) goto cleanup; Index: src/domain_conf.h =================================================================== RCS file: /data/cvs/libvirt/src/domain_conf.h,v retrieving revision 1.49 diff -u -r1.49 domain_conf.h --- src/domain_conf.h 16 Jun 2009 15:42:46 -0000 1.49 +++ src/domain_conf.h 27 Jun 2009 15:12:28 -0000 @@ -275,6 +275,21 @@ VIR_DOMAIN_GRAPHICS_TYPE_LAST, }; +typedef struct _virDomainTpmDef virDomainTpmDef; +typedef virDomainTpmDef *virDomainTpmDefPtr; +struct _virDomainTpmDef { + /* + "instance" defines the vTPM ID which should be used for the domain. This is optional + in the XML domain description. If it isn't present, the following value must be -1. + */ + long instance; + + /* + Note that the "backend" value is left out of libvirt because only dom0 is supported yet. + In the future development of Xen there might be stubdomains containing the TPM backend module. + */ +}; + typedef struct _virDomainGraphicsDef virDomainGraphicsDef; typedef virDomainGraphicsDef *virDomainGraphicsDefPtr; struct _virDomainGraphicsDef { @@ -504,6 +519,9 @@ /* Only 1 */ virDomainChrDefPtr console; virSecurityLabelDef seclabel; + + /* vTPM */ + virDomainTpmDefPtr tpm; }; /* Guest VM runtime state */ Index: src/util.c =================================================================== RCS file: /data/cvs/libvirt/src/util.c,v retrieving revision 1.111 diff -u -r1.111 util.c --- src/util.c 26 Jun 2009 15:08:04 -0000 1.111 +++ src/util.c 27 Jun 2009 15:12:29 -0000 @@ -1403,6 +1403,25 @@ return 0; } +/* Just like virStrToLong_i, above, but produce a "long" value */ +int +virStrToLong_l(char const *s, char **end_ptr, int base, long *result) +{ + long int val; + char *p; + int err; + + errno = 0; + val = strtol(s, &p, base); + err = (errno || (!end_ptr && *p) || p == s || (long) val != val); + if (end_ptr) + *end_ptr = p; + if (err) + return -1; + *result = val; + return 0; +} + /* Just like virStrToLong_i, above, but produce an "long long" value. */ int virStrToLong_ll(char const *s, char **end_ptr, int base, long long *result) Index: src/util.h =================================================================== RCS file: /data/cvs/libvirt/src/util.h,v retrieving revision 1.50 diff -u -r1.50 util.h --- src/util.h 11 Jun 2009 13:18:56 -0000 1.50 +++ src/util.h 27 Jun 2009 15:12:29 -0000 @@ -145,6 +145,10 @@ char **end_ptr, int base, unsigned int *result); +int virStrToLong_l(char const *s, + char **end_ptr, + int base, + long *result); int virStrToLong_ll(char const *s, char **end_ptr, int base, Index: src/xm_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xm_internal.c,v retrieving revision 1.128 diff -u -r1.128 xm_internal.c --- src/xm_internal.c 19 Jun 2009 12:34:31 -0000 1.128 +++ src/xm_internal.c 27 Jun 2009 15:12:30 -0000 @@ -1353,6 +1353,41 @@ } } + list = virConfGetValue(conf, "vtpm"); + if (list && list->type == VIR_CONF_LIST && list->list && + list->list->type == VIR_CONF_STRING) { + + if (VIR_ALLOC(def->tpm) < 0) + goto no_memory; + list = list->list; + + /* + "backend" options is ignored here because yet only + zero is supported (TPM backend on dom0) + */ + const char *instance; + + if (STRPREFIX(list->str, "instance=")) + instance = list->str; + else { + instance = strstr(list->str, ",instance="); + if (instance == NULL) + instance = strstr(list->str, " instance="); + if (instance == NULL) + instance = strstr(list->str, "\tinstance="); + } + + if (instance) { + /* parse instance number */ + if (virStrToLong_l(instance+10, NULL, 10, &def->tpm->instance) < 0 || + def->tpm->instance < 0) + def->tpm->instance = -1; + } + else + def->tpm->instance = -1; + } + + if (hvm) { virDomainChrDefPtr chr = NULL; @@ -2408,6 +2443,38 @@ goto no_memory; } } + + /* + vTPM configuration + + Both "instance" and "backend" values are optional. "backend" is set to zero because + the TPM backend driver is yet on dom0. In the future, there might be implementations which + have a domU that contains the backend driver. + */ + if (def->tpm) { + virConfValuePtr tpmValue = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + virConfValuePtr tmp = NULL; + + if (VIR_ALLOC(tmp) < 0) + goto no_memory; + + if (def->tpm->instance >= 0) + virBufferVSprintf(&buf, "instance=%ld,", def->tpm->instance); + + virBufferAddLit(&buf, ",backend=0"); + + tmp->type = VIR_CONF_STRING; + tmp->str = virBufferContentAndReset(&buf); + tmp->next = NULL; + + tpmValue->type = VIR_CONF_LIST; + tpmValue->list = tmp; + + if (virConfSetValue(conf, "vtpm", tpmValue) < 0) + goto no_memory; + } + /* analyze of the devices */ if (VIR_ALLOC(diskVal) < 0)

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