[libvirt] [PATCH v3 3/3] tests: add test case for tpm-crb QEMU device command line
Stefan Berger
stefanb at linux.vnet.ibm.com
Tue May 1 14:49:44 UTC 2018
On 05/01/2018 09:14 AM, John Ferlan wrote:
> I'll change the $subj to be:
>
> qemu: Add tpm-crb QEMU device to the command line
>
> On 04/26/2018 01:42 PM, Stefan Berger wrote:
>> Add a test case for the formation of the tpm-crb QEMU device
>> command line.
> And the commit msg changes to:
>
> Alter qemuBuildTPMDevStr to format the tpm-crb on the command line
> and use the enum range checking for valid model.
>
> Add a test case for the formation of the tpm-crb QEMU device
> command line. The qemuxml2argvtest changes cannot use the newer
> DO_TEST_CAPS_LATEST since building of the command line involves
> calling qemuBuildTPMBackendStr which attempts to open the
> path to the device (e.g. /dev/tmp0).
>
>> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
>> ---
>> src/qemu/qemu_command.c | 16 ++++++++++++++-
>> tests/qemuxml2argvdata/tpm-passthrough-crb.args | 26 +++++++++++++++++++++++++
>> tests/qemuxml2argvtest.c | 2 ++
>> 3 files changed, 43 insertions(+), 1 deletion(-)
>> create mode 100644 tests/qemuxml2argvdata/tpm-passthrough-crb.args
>>
> So this does bring up a couple of interesting notes for "additional"
> work or checks...
>
> 1. More recent adjustments for libvirt have begun to use the
> qemuProcessPrepare* type functions in order to perform similar things
> like the open on the /dev/tpm0 in qemuBuildTPMBackendStr. This leaves
> only building the command line in the qemu_command.c code. I'm not even
> sure this works with the TPM model given how the code passes the tpmfd
> to the command line. Still see qemuProcessPrepareChardevDevice (and it's
> corollary qemuProcessCleanupChardevDevice). There is another series
> upstream with a similar problem, see:
>
> https://www.redhat.com/archives/libvir-list/2018-April/msg01797.html
>
> and in particular patch 2 of the series. That may "solve" this open as well.
>
> 2. Does /dev/tpm0 need to play nicely in the QEMU name space code?
> See qemuDomainNamespace{Setup|Teardown}* functions. I have a feeling it
> may, but I'm not 100% sure. Michal Privoznik would be the contact point.
I just tried it using the passthrough device. I don't have a TPM 2 on my
machine, so I had to use the TPM 1.2 /dev/tpm0. I had to stop SELinux
due to the below missing rule but QEMU at least started up. Since TPM
1.2 +CRB combination isn't typically seen in the field, software doesn't
support it, but at least QEMU talks to it.
missing SELinux rule: allow svirt_t tpm_device_t:chr_file { read write };
The QEMU command line parameters looked like this:
[...] -tpmdev
passthrough,id=tpm-tpm0,path=/dev/fdset/1,cancel-path=/dev/fdset/2
-add-fd set=1,fd=29 -add-fd set=2,fd=30 -device
tpm-crb,tpmdev=tpm-tpm0,id=tpm0 [...]
This looks ok.
>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 418729b..198b44e 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -9387,8 +9387,22 @@ qemuBuildTPMDevStr(const virDomainDef *def,
>> virBuffer buf = VIR_BUFFER_INITIALIZER;
>> const virDomainTPMDef *tpm = def->tpm;
>> const char *model = virDomainTPMModelTypeToString(tpm->model);
>> + virQEMUCapsFlags flag;
>>
>> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_TIS)) {
>> + switch (tpm->model) {
>> + case VIR_DOMAIN_TPM_MODEL_TIS:
>> + flag = QEMU_CAPS_DEVICE_TPM_TIS;
>> + break;
>> + case VIR_DOMAIN_TPM_MODEL_CRB:
>> + flag = QEMU_CAPS_DEVICE_TPM_CRB;
>> + break;
>> + case VIR_DOMAIN_TPM_MODEL_LAST:
> Added the default: case; otherwise, ... [1]
>
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("Unknown TPM device model %d"), tpm->model);
> change this to
>
> virReportEnumRangeError(virDomainTPMModel, tpm->model);
>
>> + goto error;
>> + }
>> +
>> + if (!virQEMUCapsGet(qemuCaps, flag)) {
> [1] ... this failed during my build because @flag wouldn't be defined
> supposedly ...
I saw that later on, too. FC23 compiler was fine without it, FC27 complains.
>
>> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> _("The QEMU executable %s does not support TPM "
>> "model %s"),
>> diff --git a/tests/qemuxml2argvdata/tpm-passthrough-crb.args b/tests/qemuxml2argvdata/tpm-passthrough-crb.args
>> new file mode 100644
>> index 0000000..010495d
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/tpm-passthrough-crb.args
> syntax-check tells me there's some long lines in here that need to be
> wrapped using:
>
> tests/test-wrap-argv.pl -i tests/qemuxml2argvdata/tpm-passthrough-crb.args
>
> I've made the adjustments and will push the series once 4.4.0 opens
> (assuming no one else jumps in and comes up with something I've missed).
Thank you.
Stefan
>
> Reviewed-by: John Ferlan <jferlan at redhat.com>
>
> John
>
>
>
>> @@ -0,0 +1,26 @@
>> +LC_ALL=C \
>> +PATH=/bin \
>> +HOME=/home/test \
>> +USER=test \
>> +LOGNAME=test \
>> +QEMU_AUDIO_DRV=none \
>> +/usr/bin/qemu-system-x86_64 \
>> +-name TPM-VM \
>> +-S \
>> +-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \
>> +-m 2048 -\
>> +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 order=c,menu=on \
>> +-usb \
>> +-tpmdev passthrough,id=tpm-tpm0,path=/dev/tpm0,\
>> +cancel-path=/sys/class/misc/tpm0/device/cancel \
>> +-device tpm-crb,tpmdev=tpm-tpm0,id=tpm0 \
>> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index 5b3bd4a..fe2cca4 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -2001,6 +2001,8 @@ mymain(void)
>>
>> DO_TEST("tpm-passthrough",
>> QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS);
>> + DO_TEST("tpm-passthrough-crb",
>> + QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_CRB);
>> DO_TEST_PARSE_ERROR("tpm-no-backend-invalid",
>> QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS);
>>
>>
More information about the libvir-list
mailing list