This patch extends the TPM's device XML with TPM 2 support. This only works
for the emulator type backend and looks as follows:
<tpm model='tpm-tis'>
<backend type='emulator' version='2'/>
</tpm>
The swtpm process now has --tpm2 as an additional parameter:
system_u:system_r:svirt_t:s0:c597,c632 tss 18477 11.8 0.0 28364 3868 ? Rs 11:13 13:50 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm2,mode=0640 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --tpm2 --pid file=/var/run/libvirt/qemu/swtpm/testvm-swtpm.pid
The version of the TPM can be changed and the state of the TPM is preserved.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
---
docs/formatdomain.html.in | 15 ++++-
docs/schemas/domaincommon.rng | 12 ++++
src/conf/domain_conf.c | 27 ++++++++-
src/conf/domain_conf.h | 6 ++
src/qemu/qemu_tpm.c | 64 +++++++++++++++++++++-
.../tpm-emulator-tpm2.x86_64-latest.args | 33 +++++++++++
tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 30 ++++++++++
tests/qemuxml2argvtest.c | 1 +
tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml | 34 ++++++++++++
9 files changed, 217 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 08a57bd751..043c8da56f 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7719,7 +7719,7 @@ qemu-kvm -net nic,model=? /dev/null
...
<devices>
<tpm model='tpm-tis'>
- <backend type='emulator'>
+ <backend type='emulator' version='2'>
</backend>
</tpm>
</devices>
@@ -7769,6 +7769,19 @@ qemu-kvm -net nic,model=? /dev/null
</dd>
</dl>
</dd>
+ <dt><code>version</code></dt>
+ <dd>
+ <p>
+ The <code>version</code> attribute indicates the version
+ of the TPM. By default a TPM 1.2 is created. This attribute
+ only works with the <code>emulator</code> backend. The following
+ versions are supported:
+ </p>
+ <ul>
+ <li>'1.2' : creates a TPM 1.2</li>
+ <li>'2' : creates a TPM 2</li>
+ </ul>
+ </dd>
</dl>
<h4><a id="elementsNVRAM">NVRAM device</a></h4>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3582cb5019..f11833075a 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4130,6 +4130,18 @@
</attribute>
</group>
</choice>
+ <choice>
+ <group>
+ <optional>
+ <attribute name="version">
+ <choice>
+ <value>1.2</value>
+ <value>2</value>
+ </choice>
+ </attribute>
+ </optional>
+ </group>
+ </choice>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 15dd490d17..79904789ee 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12657,7 +12657,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt,
* or like this:
*
* <tpm model='tpm-tis'>
- * <backend type='emulator'/>
+ * <backend type='emulator' version='2'/>
* </tpm>
*/
static virDomainTPMDefPtr
@@ -12670,6 +12670,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
char *path = NULL;
char *model = NULL;
char *backend = NULL;
+ char *version = NULL;
virDomainTPMDefPtr def;
xmlNodePtr save = ctxt->node;
xmlNodePtr *backends = NULL;
@@ -12716,6 +12717,20 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
+ version = virXMLPropString(backends[0], "version");
+ if (!version || STREQ(version, "1.2")) {
+ def->version = VIR_DOMAIN_TPM_VERSION_1_2;
+ /* only TIS available for emulator */
+ if (def->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
+ def->model = VIR_DOMAIN_TPM_MODEL_TIS;
+ } else if (STREQ(version, "2")) {
+ def->version = VIR_DOMAIN_TPM_VERSION_2;
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported TPM version '%s'"),
+ version);
+ }
+
switch (def->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
path = virXPathString("string(./backend/device/@path)", ctxt);
@@ -12740,6 +12755,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
VIR_FREE(model);
VIR_FREE(backend);
VIR_FREE(backends);
+ VIR_FREE(version);
ctxt->node = save;
return def;
@@ -21836,6 +21852,12 @@ virDomainTPMDefCheckABIStability(virDomainTPMDefPtr src,
return false;
}
+ if (src->version != dst->version) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Target TPM version doesn't match source"));
+ return false;
+ }
+
return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info);
}
@@ -24941,6 +24963,9 @@ virDomainTPMDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, "<backend type='%s'",
virDomainTPMBackendTypeToString(def->type));
+ if (def->version == VIR_DOMAIN_TPM_VERSION_2)
+ virBufferAddLit(buf, " version='2'");
+
switch (def->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
virBufferAddLit(buf, ">\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 92466278ab..e2409899bc 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1291,12 +1291,18 @@ typedef enum {
VIR_DOMAIN_TPM_TYPE_LAST
} virDomainTPMBackendType;
+typedef enum {
+ VIR_DOMAIN_TPM_VERSION_1_2,
+ VIR_DOMAIN_TPM_VERSION_2,
+} virDomainTPMVersion;
+
# define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"
struct _virDomainTPMDef {
virDomainTPMBackendType type;
virDomainDeviceInfo info;
virDomainTPMModel model;
+ virDomainTPMVersion version;
union {
struct {
virDomainChrSourceDef source;
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 11b91aa915..508685c455 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -54,6 +54,41 @@ static char *swtpm_path;
static char *swtpm_setup;
static char *swtpm_ioctl;
+bool swtpm_supports_tpm2;
+
+/*
+ * qemuTPMCheckForTPM2Support
+ *
+ * Check whether swtpm_setup supports TPM 2
+ */
+static void
+qemuTPMCheckForTPM2Support(void)
+{
+ virCommandPtr cmd;
+ char *help = NULL;
+
+ if (!swtpm_setup)
+ return;
+
+ cmd = virCommandNew(swtpm_setup);
+ if (!cmd)
+ return;
+
+ virCommandAddArg(cmd, "--help");
+ virCommandSetOutputBuffer(cmd, &help);
+
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ if (strstr(help, "--tpm2"))
+ swtpm_supports_tpm2 = true;
+
+ cleanup:
+ virCommandFree(cmd);
+ VIR_FREE(help);
+}
+
+
/*
* qemuTPMEmulatorInit
*
@@ -93,6 +128,7 @@ qemuTPMEmulatorInit(void)
VIR_FREE(swtpm_setup);
return -1;
}
+ qemuTPMCheckForTPM2Support();
}
if (!swtpm_ioctl) {
@@ -120,16 +156,29 @@ qemuTPMEmulatorInit(void)
*
* @swtpmStorageDir: directory for swtpm persistent state
* @uuid: The UUID of the VM for which to create the storage
+ * @tpmversion: version of the TPM
*
* Create the swtpm's storage path
*/
static char *
qemuTPMCreateEmulatorStoragePath(const char *swtpmStorageDir,
- const char *uuidstr)
+ const char *uuidstr,
+ virDomainTPMVersion tpmversion)
{
char *path = NULL;
+ const char *dir = "";
- ignore_value(virAsprintf(&path, "%s/%s/tpm1.2", swtpmStorageDir, uuidstr));
+ switch (tpmversion) {
+ case VIR_DOMAIN_TPM_VERSION_1_2:
+ dir = "tpm1.2";
+ break;
+ case VIR_DOMAIN_TPM_VERSION_2:
+ dir = "tpm2";
+ break;
+ }
+
+ ignore_value(virAsprintf(&path, "%s/%s/%s", swtpmStorageDir, uuidstr,
+ dir));
return path;
}
@@ -290,7 +339,8 @@ qemuTPMEmulatorInitPaths(virDomainTPMDefPtr tpm,
if (!tpm->data.emulator.storagepath &&
!(tpm->data.emulator.storagepath =
- qemuTPMCreateEmulatorStoragePath(swtpmStorageDir, uuidstr)))
+ qemuTPMCreateEmulatorStoragePath(swtpmStorageDir, uuidstr,
+ tpm->version)))
return -1;
return 0;
@@ -514,6 +564,14 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
virCommandSetUID(cmd, swtpm_user);
virCommandSetGID(cmd, swtpm_group);
+ switch (tpm->version) {
+ case VIR_DOMAIN_TPM_VERSION_1_2:
+ break;
+ case VIR_DOMAIN_TPM_VERSION_2:
+ virCommandAddArg(cmd, "--tpm2");
+ break;
+ }
+
return cmd;
error:
diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args b/tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args
new file mode 100644
index 0000000000..82b676f966
--- /dev/null
+++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args
@@ -0,0 +1,33 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest=TPM-VM,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-TPM-VM/master-key.aes \
+-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \
+-m 2048 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 11d7cd22-da89-3094-6212-079a48a309a1 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-TPM-VM/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot menu=on,strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-tpmdev emulator,id=tpm-tpm0,chardev=chrtpm \
+-chardev socket,id=chrtpm,path=/dev/test \
+-device tpm-tis,tpmdev=tpm-tpm0,id=tpm0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
new file mode 100644
index 0000000000..7546930d19
--- /dev/null
+++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
@@ -0,0 +1,30 @@
+<domain type='qemu'>
+ <name>TPM-VM</name>
+ <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid>
+ <memory unit='KiB'>2097152</memory>
+ <currentMemory unit='KiB'>512288</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type>
+ <boot dev='hd'/>
+ <bootmenu enable='yes'/>
+ </os>
+ <features>
+ <acpi/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <tpm model='tpm-tis'>
+ <backend type='emulator' version='2'/>
+ </tpm>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 587f15242e..a4801407b6 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2027,6 +2027,7 @@ mymain(void)
DO_TEST_PARSE_ERROR("tpm-no-backend-invalid",
QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS);
DO_TEST_CAPS_LATEST("tpm-emulator");
+ DO_TEST_CAPS_LATEST("tpm-emulator-tpm2");
DO_TEST_PARSE_ERROR("pci-domain-invalid", NONE);
DO_TEST_PARSE_ERROR("pci-bus-invalid", NONE);
diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml
new file mode 100644
index 0000000000..eff55fc5df
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml
@@ -0,0 +1,34 @@
+<domain type='qemu'>
+ <name>TPM-VM</name>
+ <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid>
+ <memory unit='KiB'>2097152</memory>
+ <currentMemory unit='KiB'>512288</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type>
+ <boot dev='hd'/>
+ <bootmenu enable='yes'/>
+ </os>
+ <features>
+ <acpi/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <tpm model='tpm-tis'>
+ <backend type='emulator' version='2'/>
+ </tpm>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
--
2.14.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, May 22, 2018 at 10:44 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> This patch extends the TPM's device XML with TPM 2 support. This only works
> for the emulator type backend and looks as follows:
>
> <tpm model='tpm-tis'>
> <backend type='emulator' version='2'/>
> </tpm>
>
> The swtpm process now has --tpm2 as an additional parameter:
>
> system_u:system_r:svirt_t:s0:c597,c632 tss 18477 11.8 0.0 28364 3868 ? Rs 11:13 13:50 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm2,mode=0640 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --tpm2 --pid file=/var/run/libvirt/qemu/swtpm/testvm-swtpm.pid
>
> The version of the TPM can be changed and the state of the TPM is preserved.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> ---
> docs/formatdomain.html.in | 15 ++++-
> docs/schemas/domaincommon.rng | 12 ++++
> src/conf/domain_conf.c | 27 ++++++++-
> src/conf/domain_conf.h | 6 ++
> src/qemu/qemu_tpm.c | 64 +++++++++++++++++++++-
> .../tpm-emulator-tpm2.x86_64-latest.args | 33 +++++++++++
> tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 30 ++++++++++
> tests/qemuxml2argvtest.c | 1 +
> tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml | 34 ++++++++++++
> 9 files changed, 217 insertions(+), 5 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args
> create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
> create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 08a57bd751..043c8da56f 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -7719,7 +7719,7 @@ qemu-kvm -net nic,model=? /dev/null
> ...
> <devices>
> <tpm model='tpm-tis'>
> - <backend type='emulator'>
> + <backend type='emulator' version='2'>
> </backend>
> </tpm>
> </devices>
> @@ -7769,6 +7769,19 @@ qemu-kvm -net nic,model=? /dev/null
> </dd>
> </dl>
> </dd>
> + <dt><code>version</code></dt>
> + <dd>
> + <p>
> + The <code>version</code> attribute indicates the version
> + of the TPM. By default a TPM 1.2 is created. This attribute
> + only works with the <code>emulator</code> backend. The following
> + versions are supported:
> + </p>
> + <ul>
> + <li>'1.2' : creates a TPM 1.2</li>
> + <li>'2' : creates a TPM 2</li>
> + </ul>
> + </dd>
> </dl>
>
> <h4><a id="elementsNVRAM">NVRAM device</a></h4>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 3582cb5019..f11833075a 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4130,6 +4130,18 @@
> </attribute>
> </group>
> </choice>
> + <choice>
> + <group>
> + <optional>
> + <attribute name="version">
> + <choice>
> + <value>1.2</value>
> + <value>2</value>
> + </choice>
> + </attribute>
> + </optional>
> + </group>
> + </choice>
> </element>
> </define>
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 15dd490d17..79904789ee 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -12657,7 +12657,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt,
> * or like this:
> *
> * <tpm model='tpm-tis'>
> - * <backend type='emulator'/>
> + * <backend type='emulator' version='2'/>
> * </tpm>
> */
> static virDomainTPMDefPtr
> @@ -12670,6 +12670,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
> char *path = NULL;
> char *model = NULL;
> char *backend = NULL;
> + char *version = NULL;
> virDomainTPMDefPtr def;
> xmlNodePtr save = ctxt->node;
> xmlNodePtr *backends = NULL;
> @@ -12716,6 +12717,20 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
> goto error;
> }
>
> + version = virXMLPropString(backends[0], "version");
> + if (!version || STREQ(version, "1.2")) {
> + def->version = VIR_DOMAIN_TPM_VERSION_1_2;
> + /* only TIS available for emulator */
> + if (def->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
> + def->model = VIR_DOMAIN_TPM_MODEL_TIS;
This will silently overwrite an already defined model - is this
intended? Also this seems like some kind of validation logic - not sure
if virDomainTPMDefParseXML is the right place for this.
> + } else if (STREQ(version, "2")) {
> + def->version = VIR_DOMAIN_TPM_VERSION_2;
[…snip]
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/24/2018 08:17 AM, Marc Hartmayer wrote:
> On Tue, May 22, 2018 at 10:44 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>> This patch extends the TPM's device XML with TPM 2 support. This only works
>> for the emulator type backend and looks as follows:
>>
>> <tpm model='tpm-tis'>
>> <backend type='emulator' version='2'/>
>> </tpm>
>>
>> The swtpm process now has --tpm2 as an additional parameter:
>>
>> system_u:system_r:svirt_t:s0:c597,c632 tss 18477 11.8 0.0 28364 3868 ? Rs 11:13 13:50 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm2,mode=0640 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --tpm2 --pid file=/var/run/libvirt/qemu/swtpm/testvm-swtpm.pid
>>
>> The version of the TPM can be changed and the state of the TPM is preserved.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> Reviewed-by: John Ferlan <jferlan@redhat.com>
>> ---
>> docs/formatdomain.html.in | 15 ++++-
>> docs/schemas/domaincommon.rng | 12 ++++
>> src/conf/domain_conf.c | 27 ++++++++-
>> src/conf/domain_conf.h | 6 ++
>> src/qemu/qemu_tpm.c | 64 +++++++++++++++++++++-
>> .../tpm-emulator-tpm2.x86_64-latest.args | 33 +++++++++++
>> tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 30 ++++++++++
>> tests/qemuxml2argvtest.c | 1 +
>> tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml | 34 ++++++++++++
>> 9 files changed, 217 insertions(+), 5 deletions(-)
>> create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args
>> create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
>> create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 08a57bd751..043c8da56f 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -7719,7 +7719,7 @@ qemu-kvm -net nic,model=? /dev/null
>> ...
>> <devices>
>> <tpm model='tpm-tis'>
>> - <backend type='emulator'>
>> + <backend type='emulator' version='2'>
>> </backend>
>> </tpm>
>> </devices>
>> @@ -7769,6 +7769,19 @@ qemu-kvm -net nic,model=? /dev/null
>> </dd>
>> </dl>
>> </dd>
>> + <dt><code>version</code></dt>
>> + <dd>
>> + <p>
>> + The <code>version</code> attribute indicates the version
>> + of the TPM. By default a TPM 1.2 is created. This attribute
>> + only works with the <code>emulator</code> backend. The following
>> + versions are supported:
>> + </p>
>> + <ul>
>> + <li>'1.2' : creates a TPM 1.2</li>
>> + <li>'2' : creates a TPM 2</li>
>> + </ul>
>> + </dd>
>> </dl>
>>
>> <h4><a id="elementsNVRAM">NVRAM device</a></h4>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 3582cb5019..f11833075a 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -4130,6 +4130,18 @@
>> </attribute>
>> </group>
>> </choice>
>> + <choice>
>> + <group>
>> + <optional>
>> + <attribute name="version">
>> + <choice>
>> + <value>1.2</value>
>> + <value>2</value>
>> + </choice>
>> + </attribute>
>> + </optional>
>> + </group>
>> + </choice>
>> </element>
>> </define>
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 15dd490d17..79904789ee 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -12657,7 +12657,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt,
>> * or like this:
>> *
>> * <tpm model='tpm-tis'>
>> - * <backend type='emulator'/>
>> + * <backend type='emulator' version='2'/>
>> * </tpm>
>> */
>> static virDomainTPMDefPtr
>> @@ -12670,6 +12670,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
>> char *path = NULL;
>> char *model = NULL;
>> char *backend = NULL;
>> + char *version = NULL;
>> virDomainTPMDefPtr def;
>> xmlNodePtr save = ctxt->node;
>> xmlNodePtr *backends = NULL;
>> @@ -12716,6 +12717,20 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
>> goto error;
>> }
>>
>> + version = virXMLPropString(backends[0], "version");
>> + if (!version || STREQ(version, "1.2")) {
>> + def->version = VIR_DOMAIN_TPM_VERSION_1_2;
>> + /* only TIS available for emulator */
>> + if (def->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
>> + def->model = VIR_DOMAIN_TPM_MODEL_TIS;
> This will silently overwrite an already defined model - is this
> intended? Also this seems like some kind of validation logic - not sure
> if virDomainTPMDefParseXML is the right place for this.
TPM 1.2 can typically only be used with the TIS. The CRB interface works
only with TPM 2. So, yes, it's intentional.
Stefan
>
>> + } else if (STREQ(version, "2")) {
>> + def->version = VIR_DOMAIN_TPM_VERSION_2;
> […snip]
>
> Beste Grüße / Kind regards
> Marc Hartmayer
>
> IBM Deutschland Research & Development GmbH
> Vorsitzende des Aufsichtsrats: Martina Koederitz
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, May 24, 2018 at 02:17:13PM +0200, Marc Hartmayer wrote:
>On Tue, May 22, 2018 at 10:44 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>> This patch extends the TPM's device XML with TPM 2 support. This only works
>> for the emulator type backend and looks as follows:
>>
>> <tpm model='tpm-tis'>
>> <backend type='emulator' version='2'/>
>> </tpm>
>>
>> The swtpm process now has --tpm2 as an additional parameter:
>>
>> system_u:system_r:svirt_t:s0:c597,c632 tss 18477 11.8 0.0 28364 3868 ? Rs 11:13 13:50 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm2,mode=0640 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --tpm2 --pid file=/var/run/libvirt/qemu/swtpm/testvm-swtpm.pid
>>
>> The version of the TPM can be changed and the state of the TPM is preserved.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> Reviewed-by: John Ferlan <jferlan@redhat.com>
>> ---
>> docs/formatdomain.html.in | 15 ++++-
>> docs/schemas/domaincommon.rng | 12 ++++
>> src/conf/domain_conf.c | 27 ++++++++-
>> src/conf/domain_conf.h | 6 ++
>> src/qemu/qemu_tpm.c | 64 +++++++++++++++++++++-
>> .../tpm-emulator-tpm2.x86_64-latest.args | 33 +++++++++++
>> tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 30 ++++++++++
>> tests/qemuxml2argvtest.c | 1 +
>> tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml | 34 ++++++++++++
>> 9 files changed, 217 insertions(+), 5 deletions(-)
>> create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args
>> create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
>> create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 08a57bd751..043c8da56f 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -7719,7 +7719,7 @@ qemu-kvm -net nic,model=? /dev/null
>> ...
>> <devices>
>> <tpm model='tpm-tis'>
>> - <backend type='emulator'>
>> + <backend type='emulator' version='2'>
>> </backend>
>> </tpm>
>> </devices>
>> @@ -7769,6 +7769,19 @@ qemu-kvm -net nic,model=? /dev/null
>> </dd>
>> </dl>
>> </dd>
>> + <dt><code>version</code></dt>
>> + <dd>
>> + <p>
>> + The <code>version</code> attribute indicates the version
>> + of the TPM. By default a TPM 1.2 is created. This attribute
>> + only works with the <code>emulator</code> backend. The following
>> + versions are supported:
>> + </p>
>> + <ul>
>> + <li>'1.2' : creates a TPM 1.2</li>
>> + <li>'2' : creates a TPM 2</li>
>> + </ul>
>> + </dd>
>> </dl>
>>
>> <h4><a id="elementsNVRAM">NVRAM device</a></h4>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 3582cb5019..f11833075a 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -4130,6 +4130,18 @@
>> </attribute>
>> </group>
>> </choice>
>> + <choice>
>> + <group>
>> + <optional>
>> + <attribute name="version">
>> + <choice>
>> + <value>1.2</value>
>> + <value>2</value>
>> + </choice>
>> + </attribute>
>> + </optional>
>> + </group>
>> + </choice>
>> </element>
>> </define>
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 15dd490d17..79904789ee 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -12657,7 +12657,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt,
>> * or like this:
>> *
>> * <tpm model='tpm-tis'>
>> - * <backend type='emulator'/>
>> + * <backend type='emulator' version='2'/>
>> * </tpm>
>> */
>> static virDomainTPMDefPtr
>> @@ -12670,6 +12670,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
>> char *path = NULL;
>> char *model = NULL;
>> char *backend = NULL;
>> + char *version = NULL;
>> virDomainTPMDefPtr def;
>> xmlNodePtr save = ctxt->node;
>> xmlNodePtr *backends = NULL;
>> @@ -12716,6 +12717,20 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
>> goto error;
>> }
>>
>> + version = virXMLPropString(backends[0], "version");
>> + if (!version || STREQ(version, "1.2")) {
>> + def->version = VIR_DOMAIN_TPM_VERSION_1_2;
>> + /* only TIS available for emulator */
>> + if (def->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
>> + def->model = VIR_DOMAIN_TPM_MODEL_TIS;
>
>This will silently overwrite an already defined model - is this
>intended? Also this seems like some kind of validation logic - not sure
>if virDomainTPMDefParseXML is the right place for this.
>
Yes, DefParse would ideally just convert what was provided in the XML
to our internal data types. Setting defaults belongs in PostParse
(either in src/conf or in src/qemu) and for validation we have
qemu.*DefValidate.
Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, May 22, 2018 at 04:44:51PM -0400, Stefan Berger wrote:
>This patch extends the TPM's device XML with TPM 2 support. This only works
>for the emulator type backend and looks as follows:
>
> <tpm model='tpm-tis'>
> <backend type='emulator' version='2'/>
> </tpm>
>
>The swtpm process now has --tpm2 as an additional parameter:
>
>system_u:system_r:svirt_t:s0:c597,c632 tss 18477 11.8 0.0 28364 3868 ? Rs 11:13 13:50 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm2,mode=0640 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --tpm2 --pid file=/var/run/libvirt/qemu/swtpm/testvm-swtpm.pid
>
>The version of the TPM can be changed and the state of the TPM is preserved.
>
>Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>Reviewed-by: John Ferlan <jferlan@redhat.com>
>---
> docs/formatdomain.html.in | 15 ++++-
> docs/schemas/domaincommon.rng | 12 ++++
> src/conf/domain_conf.c | 27 ++++++++-
> src/conf/domain_conf.h | 6 ++
> src/qemu/qemu_tpm.c | 64 +++++++++++++++++++++-
> .../tpm-emulator-tpm2.x86_64-latest.args | 33 +++++++++++
> tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 30 ++++++++++
> tests/qemuxml2argvtest.c | 1 +
> tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml | 34 ++++++++++++
> 9 files changed, 217 insertions(+), 5 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args
> create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
> create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml
>
>@@ -24941,6 +24963,9 @@ virDomainTPMDefFormat(virBufferPtr buf,
> virBufferAsprintf(buf, "<backend type='%s'",
> virDomainTPMBackendTypeToString(def->type));
>
>+ if (def->version == VIR_DOMAIN_TPM_VERSION_2)
>+ virBufferAddLit(buf, " version='2'");
>+
Any reason for not formatting version 1.2?
We should format implicit defaults in the XML if possible.
> switch (def->type) {
> case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> virBufferAddLit(buf, ">\n");
>diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>index 92466278ab..e2409899bc 100644
>--- a/src/conf/domain_conf.h
>+++ b/src/conf/domain_conf.h
>@@ -1291,12 +1291,18 @@ typedef enum {
> VIR_DOMAIN_TPM_TYPE_LAST
> } virDomainTPMBackendType;
>
>+typedef enum {
>+ VIR_DOMAIN_TPM_VERSION_1_2,
>+ VIR_DOMAIN_TPM_VERSION_2,
>+} virDomainTPMVersion;
With a corresponding VIR_ENUM_IMPL and VIR_ENUM_DECL,
you can use the *{To,From}String functions for parsing/formatting
the version.
>+
> # define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"
>
> struct _virDomainTPMDef {
> virDomainTPMBackendType type;
> virDomainDeviceInfo info;
> virDomainTPMModel model;
>+ virDomainTPMVersion version;
> union {
> struct {
> virDomainChrSourceDef source;
>diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
>index 11b91aa915..508685c455 100644
>--- a/src/qemu/qemu_tpm.c
>+++ b/src/qemu/qemu_tpm.c
>@@ -54,6 +54,41 @@ static char *swtpm_path;
> static char *swtpm_setup;
> static char *swtpm_ioctl;
>
>+bool swtpm_supports_tpm2;
>+
>+/*
>+ * qemuTPMCheckForTPM2Support
>+ *
>+ * Check whether swtpm_setup supports TPM 2
>+ */
>+static void
>+qemuTPMCheckForTPM2Support(void)
>+{
>+ virCommandPtr cmd;
>+ char *help = NULL;
>+
>+ if (!swtpm_setup)
>+ return;
>+
>+ cmd = virCommandNew(swtpm_setup);
>+ if (!cmd)
>+ return;
>+
>+ virCommandAddArg(cmd, "--help");
>+ virCommandSetOutputBuffer(cmd, &help);
>+
>+ if (virCommandRun(cmd, NULL) < 0)
>+ goto cleanup;
>+
>+ if (strstr(help, "--tpm2"))
>+ swtpm_supports_tpm2 = true;
This bool is never read.
Given that version 2 has to be requested in the XML and we don't try to
use it automatically, I'd suggest just dropping this function. We don't
need to parse another tool's --help output to make up for the removal
of parsing --help of qemu and qemu-img.
Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/23/2018 11:55 AM, Ján Tomko wrote:
> On Tue, May 22, 2018 at 04:44:51PM -0400, Stefan Berger wrote:
>> This patch extends the TPM's device XML with TPM 2 support. This only
>> works
>> for the emulator type backend and looks as follows:
>>
>> <tpm model='tpm-tis'>
>> <backend type='emulator' version='2'/>
>> </tpm>
>>
>> The swtpm process now has --tpm2 as an additional parameter:
>>
>> system_u:system_r:svirt_t:s0:c597,c632 tss 18477 11.8 0.0 28364
>> 3868 ? Rs 11:13 13:50 /usr/bin/swtpm socket --daemon --ctrl
>> type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660
>> --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm2,mode=0640 --log
>> file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --tpm2 --pid
>> file=/var/run/libvirt/qemu/swtpm/testvm-swtpm.pid
>>
>> The version of the TPM can be changed and the state of the TPM is
>> preserved.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> Reviewed-by: John Ferlan <jferlan@redhat.com>
>> ---
>> docs/formatdomain.html.in | 15 ++++-
>> docs/schemas/domaincommon.rng | 12 ++++
>> src/conf/domain_conf.c | 27 ++++++++-
>> src/conf/domain_conf.h | 6 ++
>> src/qemu/qemu_tpm.c | 64
>> +++++++++++++++++++++-
>> .../tpm-emulator-tpm2.x86_64-latest.args | 33 +++++++++++
>> tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 30 ++++++++++
>> tests/qemuxml2argvtest.c | 1 +
>> tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml | 34 ++++++++++++
>> 9 files changed, 217 insertions(+), 5 deletions(-)
>> create mode 100644
>> tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args
>> create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
>> create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml
>>
>
>> @@ -24941,6 +24963,9 @@ virDomainTPMDefFormat(virBufferPtr buf,
>> virBufferAsprintf(buf, "<backend type='%s'",
>> virDomainTPMBackendTypeToString(def->type));
>>
>> + if (def->version == VIR_DOMAIN_TPM_VERSION_2)
>> + virBufferAddLit(buf, " version='2'");
>> +
>
> Any reason for not formatting version 1.2?
> We should format implicit defaults in the XML if possible.
Basically I did it because the previous default 1.2 didn't have it. So I
though I'd keep it as is for 1.2 and only write out 2.
>
>> switch (def->type) {
>> case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
>> virBufferAddLit(buf, ">\n");
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 92466278ab..e2409899bc 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -1291,12 +1291,18 @@ typedef enum {
>> VIR_DOMAIN_TPM_TYPE_LAST
>> } virDomainTPMBackendType;
>>
>> +typedef enum {
>> + VIR_DOMAIN_TPM_VERSION_1_2,
>> + VIR_DOMAIN_TPM_VERSION_2,
>> +} virDomainTPMVersion;
>
> With a corresponding VIR_ENUM_IMPL and VIR_ENUM_DECL,
> you can use the *{To,From}String functions for parsing/formatting
> the version.
>
>> +
>> # define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"
>>
>> struct _virDomainTPMDef {
>> virDomainTPMBackendType type;
>> virDomainDeviceInfo info;
>> virDomainTPMModel model;
>> + virDomainTPMVersion version;
>> union {
>> struct {
>> virDomainChrSourceDef source;
>> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
>> index 11b91aa915..508685c455 100644
>> --- a/src/qemu/qemu_tpm.c
>> +++ b/src/qemu/qemu_tpm.c
>> @@ -54,6 +54,41 @@ static char *swtpm_path;
>> static char *swtpm_setup;
>> static char *swtpm_ioctl;
>>
>> +bool swtpm_supports_tpm2;
>> +
>> +/*
>> + * qemuTPMCheckForTPM2Support
>> + *
>> + * Check whether swtpm_setup supports TPM 2
>> + */
>> +static void
>> +qemuTPMCheckForTPM2Support(void)
>> +{
>> + virCommandPtr cmd;
>> + char *help = NULL;
>> +
>> + if (!swtpm_setup)
>> + return;
>> +
>> + cmd = virCommandNew(swtpm_setup);
>> + if (!cmd)
>> + return;
>> +
>> + virCommandAddArg(cmd, "--help");
>> + virCommandSetOutputBuffer(cmd, &help);
>> +
>> + if (virCommandRun(cmd, NULL) < 0)
>> + goto cleanup;
>> +
>> + if (strstr(help, "--tpm2"))
>> + swtpm_supports_tpm2 = true;
>
> This bool is never read.
Maybe while doing some of the recent changes the reading of the variable
got lost.
>
> Given that version 2 has to be requested in the XML and we don't try to
> use it automatically, I'd suggest just dropping this function. We don't
> need to parse another tool's --help output to make up for the removal
> of parsing --help of qemu and qemu-img.
swtpm doesn't have all the bells and whistles of QEMU that we would have
a JSON interface to query the features from. So if a bad command line
parameter is passed to swtpm, it will dump the help screen. Here's the
output I get from trying to run a VM with an attached TPM 2 but there's
no TPM 2 support compiled into swtpm (basically because it was created
from the master branch not from the preview branch):
# virsh start testvm-tpm2
Error: Failed to start domain testvm-tpm2
error: internal error: Could not start 'swtpm'. exitstatus: 1, error:
socket: unrecognized option '--tpm2'
Usage: /usr/bin/swtpm socket [options]
The following options are supported:
-p|--port <port> : use the given port
-f|--fd <fd> : use the given socket file descriptor
[...]
I think a more controlled error message would be better basically
stating 'Local swtpm installation does not support TPM 2'.
Stefan
>
> Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, May 23, 2018 at 02:33:08PM -0400, Stefan Berger wrote:
>On 05/23/2018 11:55 AM, Ján Tomko wrote:
>> On Tue, May 22, 2018 at 04:44:51PM -0400, Stefan Berger wrote:
>>> @@ -24941,6 +24963,9 @@ virDomainTPMDefFormat(virBufferPtr buf,
>>> virBufferAsprintf(buf, "<backend type='%s'",
>>> virDomainTPMBackendTypeToString(def->type));
>>>
>>> + if (def->version == VIR_DOMAIN_TPM_VERSION_2)
>>> + virBufferAddLit(buf, " version='2'");
>>> +
>>
>> Any reason for not formatting version 1.2?
>> We should format implicit defaults in the XML if possible.
>
>Basically I did it because the previous default 1.2 didn't have it. So I
>though I'd keep it as is for 1.2 and only write out 2.
>
What previous default?
<backend type='emulator'/> is introduced by this series.
We should format the configurable attributes even when they are at their
default values.
>>
>>> switch (def->type) {
>>> case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
>>> virBufferAddLit(buf, ">\n");
>>
>> Given that version 2 has to be requested in the XML and we don't try to
>> use it automatically, I'd suggest just dropping this function. We don't
>> need to parse another tool's --help output to make up for the removal
>> of parsing --help of qemu and qemu-img.
>
>swtpm doesn't have all the bells and whistles of QEMU that we would have
>a JSON interface to query the features from.
With QEMU, we actually need to know some capabilities upfront, because
different versions have had different ways of requesting the same
functionality. Giving nicer errors for unsupported features is just a
bonus.
With qemu-img, we only care about one way of representing the
functionality and let it print an error if something's compiled out.
>So if a bad command line
>parameter is passed to swtpm, it will dump the help screen. Here's the
>output I get from trying to run a VM with an attached TPM 2 but there's
>no TPM 2 support compiled into swtpm (basically because it was created
>from the master branch not from the preview branch):
>
># virsh start testvm-tpm2
>Error: Failed to start domain testvm-tpm2
>error: internal error: Could not start 'swtpm'. exitstatus: 1, error:
>socket: unrecognized option '--tpm2'
>Usage: /usr/bin/swtpm socket [options]
>
>The following options are supported:
>
>-p|--port <port> : use the given port
>-f|--fd <fd> : use the given socket file descriptor
>[...]
>
>
>I think a more controlled error message would be better basically
>stating 'Local swtpm installation does not support TPM 2'.
>
Yes, but not worth introducing all the code and extra exec() for all the
successful starts.
Jano
> Stefan
>
>>
>> Jano
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/24/2018 03:08 AM, Ján Tomko wrote:
> On Wed, May 23, 2018 at 02:33:08PM -0400, Stefan Berger wrote:
>> On 05/23/2018 11:55 AM, Ján Tomko wrote:
>>> On Tue, May 22, 2018 at 04:44:51PM -0400, Stefan Berger wrote:
>>>> @@ -24941,6 +24963,9 @@ virDomainTPMDefFormat(virBufferPtr buf,
>>>> virBufferAsprintf(buf, "<backend type='%s'",
>>>> virDomainTPMBackendTypeToString(def->type));
>>>>
>>>> + if (def->version == VIR_DOMAIN_TPM_VERSION_2)
>>>> + virBufferAddLit(buf, " version='2'");
>>>> +
>>>
>>> Any reason for not formatting version 1.2?
>>> We should format implicit defaults in the XML if possible.
>>
>> Basically I did it because the previous default 1.2 didn't have it. So I
>> though I'd keep it as is for 1.2 and only write out 2.
>>
>
> What previous default?
> <backend type='emulator'/> is introduced by this series.
>
> We should format the configurable attributes even when they are at their
> default values.
I'll post a v7 with this and other changes.
>
>>>
>>>> switch (def->type) {
>>>> case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
>>>> virBufferAddLit(buf, ">\n");
>>>
>>> Given that version 2 has to be requested in the XML and we don't try to
>>> use it automatically, I'd suggest just dropping this function. We don't
>>> need to parse another tool's --help output to make up for the removal
>>> of parsing --help of qemu and qemu-img.
>>
>> swtpm doesn't have all the bells and whistles of QEMU that we would have
>> a JSON interface to query the features from.
>
> With QEMU, we actually need to know some capabilities upfront, because
> different versions have had different ways of requesting the same
> functionality. Giving nicer errors for unsupported features is just a
> bonus.
>
> With qemu-img, we only care about one way of representing the
> functionality and let it print an error if something's compiled out.
Ok, then let me remove it. v7 will have that change. Not sure what to
do about the existing Reviewed-by's. Intend to keep them.
Stefan
>
>> So if a bad command line
>> parameter is passed to swtpm, it will dump the help screen. Here's the
>> output I get from trying to run a VM with an attached TPM 2 but there's
>> no TPM 2 support compiled into swtpm (basically because it was created
>> from the master branch not from the preview branch):
>>
>> # virsh start testvm-tpm2
>> Error: Failed to start domain testvm-tpm2
>> error: internal error: Could not start 'swtpm'. exitstatus: 1, error:
>> socket: unrecognized option '--tpm2'
>> Usage: /usr/bin/swtpm socket [options]
>>
>> The following options are supported:
>>
>> -p|--port <port> : use the given port
>> -f|--fd <fd> : use the given socket file descriptor
>> [...]
>>
>>
>> I think a more controlled error message would be better basically
>> stating 'Local swtpm installation does not support TPM 2'.
>>
>
> Yes, but not worth introducing all the code and extra exec() for all the
> successful starts.
>
> Jano
>
>> Stefan
>>
>>>
>>> Jano
>>
>>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, May 24, 2018 at 07:22:30AM -0400, Stefan Berger wrote: >On 05/24/2018 03:08 AM, Ján Tomko wrote: >> On Wed, May 23, 2018 at 02:33:08PM -0400, Stefan Berger wrote: >>> swtpm doesn't have all the bells and whistles of QEMU that we would have >>> a JSON interface to query the features from. >> >> With QEMU, we actually need to know some capabilities upfront, because >> different versions have had different ways of requesting the same >> functionality. Giving nicer errors for unsupported features is just a >> bonus. >> >> With qemu-img, we only care about one way of representing the >> functionality and let it print an error if something's compiled out. > >Ok, then let me remove it. v7 will have that change. Not sure what to >do about the existing Reviewed-by's. Intend to keep them. The usual thing to do is keep them unless you make a significant change to the commit. Its presence in the commit message should tell the reviewer that he does not need to look at it again. Jano -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.