Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2

2018-05-24 Thread Ján Tomko 

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


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2

2018-05-24 Thread Ján Tomko 

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


  


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 
Reviewed-by: John Ferlan 
---
 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
   
 
   
+  version
+  
+
+  The version attribute indicates the version
+  of the TPM. By default a TPM 1.2 is created. This attribute
+  only works with the emulator backend. The following
+  versions are supported:
+
+
+  '1.2' : creates a TPM 1.2
+  '2' :  creates a TPM 2
+
+  
 

 NVRAM device
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 @@
   
 
   
+  
+
+  
+
+  
+1.2
+2
+  
+   
+  
+
+  
 
   

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:
  *
  * 
- *   
+ *   
  * 
  */
 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


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2

2018-05-24 Thread Stefan Berger 

On 05/24/2018 08:17 AM, Marc Hartmayer wrote:

On Tue, May 22, 2018 at 10:44 PM +0200, 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:

 
   
 

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 
Reviewed-by: John Ferlan 
---
  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

  

+  version
+  
+
+  The version attribute indicates the version
+  of the TPM. By default a TPM 1.2 is created. This attribute
+  only works with the emulator backend. The following
+  versions are supported:
+
+
+  '1.2' : creates a TPM 1.2
+  '2' :  creates a TPM 2
+
+  
  

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

  

+  
+
+  
+
+  
+1.2
+2
+  
+   
+  
+
+  
  


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:
   *
   * 
- *   
+ *   
   * 
   */
  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

Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2

2018-05-24 Thread Marc Hartmayer 
On Tue, May 22, 2018 at 10:44 PM +0200, 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:
>
> 
>   
> 
>
> 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 
> Reviewed-by: John Ferlan 
> ---
>  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
>
>  
>
> +  version
> +  
> +
> +  The version attribute indicates the version
> +  of the TPM. By default a TPM 1.2 is created. This attribute
> +  only works with the emulator backend. The following
> +  versions are supported:
> +
> +
> +  '1.2' : creates a TPM 1.2
> +  '2' :  creates a TPM 2
> +
> +  
>  
>
>  NVRAM device
> 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 @@
>
>  
>
> +  
> +
> +  
> +
> +  
> +1.2
> +2
> +  
> +   
> +  
> +
> +  
>  
>
>
> 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:
>   *
>   * 
> - *   
> + *   
>   * 
>   */
>  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

Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2

2018-05-24 Thread Stefan Berger 

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, "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?
 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  : use the given port
-f|--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

Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2

2018-05-24 Thread Ján Tomko 

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, "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?
 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  : use the given port
-f|--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





signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2

2018-05-23 Thread Stefan Berger 

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:

   
 
   

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 
Reviewed-by: John Ferlan 
---
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, "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, );
+
+    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  : use the given port
-f|--fd  : use the given socket file descriptor
[...]


I

Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2

2018-05-23 Thread Ján Tomko 

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:

   
 
   

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 
Reviewed-by: John Ferlan 
---
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, "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, );
+
+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


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list