On Sat, Jun 27, 2009 at 09:32:27PM +0100, Andreas Sommer wrote:
> 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.
Ok, so your patch has added the generic XML handling, and also added
support to the 'xm' driver which handles /etc/xen config files. The
main thing you have left is adding support to the 'xend_internal.c'
file which is what talks to XenD and is responsible for starting
guests, and handling config of running guests.
> 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 :)
Great.
It would be nice to add a couple of test cases too. For the 'xm_internal.c'
changes, you can add files to tests/xmconfigdata/ and tests/xmconfigtest.c
while for the 'xend_internal' changes the tests are in
tests/sexpr2xmltest.c, tests/xml2sexprtest.c. The 'sxpr' files are what
you get from Xend when doing 'xm list --long GUESTNAME'.
> +
> + /* 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;
> + }
These two loops are redundant really - virStrToLong_l does sufficient
checking of data validity.
> +
> + 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 */
>
> +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;
> + 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;
Before calling 'virBufferContentAndReset' you should do
if (virBufferError(&buf))
goto no_memory;
to check that there hasn't been an OOM condition in the earlier AddLit
or VSprintf() calls.
> +
> + tpmValue->type = VIR_CONF_LIST;
> + tpmValue->list = tmp;
> +
> + if (virConfSetValue(conf, "vtpm", tpmValue) < 0)
> + goto no_memory;
> + }
> +
>
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list