[PATCH v4 11/11] qemu: Read back the profile name after creation of a TPM instance

Stefan Berger posted 11 patches 1 week ago
[PATCH v4 11/11] qemu: Read back the profile name after creation of a TPM instance
Posted by Stefan Berger 1 week ago
Get the JSON profile that the swtpm instance was created with from the
output of 'swtpm socket --tpm2 --print-info 0x20 --tpmstate ...'. Get the
name of the profile from the JSON and set it in the current and persistent
emulator descriptions as 'name' attribute and have the persistent
description stored with this update. The user should avoid setting this
'name' attribute since it is meant to be read-only. The following is
an example of how the XML could look like:

  <profile source='local:restricted' name='custom:restricted'/>

If the user provided no profile node, and therefore swtpm_setup picked its
default profile, the XML may now shows the 'name' attribute with the name
of the profile. This makes the 'source' attribute now optional.

  <profile name='default-v1'/>

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 docs/formatdomain.rst             |  16 ++---
 src/conf/domain_conf.c            |  18 +++---
 src/conf/domain_conf.h            |   1 +
 src/conf/schemas/domaincommon.rng |  13 +++-
 src/qemu/qemu_extdevice.c         |   5 +-
 src/qemu/qemu_tpm.c               | 100 ++++++++++++++++++++++++++++--
 src/qemu/qemu_tpm.h               |   3 +-
 src/util/virtpm.c                 |   1 +
 src/util/virtpm.h                 |   1 +
 9 files changed, 135 insertions(+), 23 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 6539f620fa..20c86087ef 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -8131,7 +8131,7 @@ Example: usage of the TPM Emulator
            <active_pcr_banks>
                <sha256/>
            </active_pcr_banks>
-           <profile source='local:restricted' remove_disabled='check'/>
+           <profile source='local:restricted' remove_disabled='check' name='custom:restricted'/>
          </backend>
        </tpm>
      </devices>
@@ -8229,12 +8229,14 @@ Example: usage of the TPM Emulator
 ``profile``
    The ``profile`` node is used to set a profile for a TPM 2.0 given in the
    source attribute. This profile will be set when the TPM is initially
-   created and after that cannot be changed anymore. If no profile is provided,
-   then swtpm will use the latest built-in 'default' profile or the default
-   profile set in swtpm_setup.conf. Otherwise swtpm_setup will search for a
-   profile with the given name with appended .json suffix in a configurable
-   local and then in a distro directory. If none could be found in either, it
-   will fall back trying to use a built-in one.
+   created and after that cannot be changed anymore. Once a profile has been
+   set the name attribute will be updated with the name of the profile that
+   is running. If no profile is provided, then swtpm will use the latest
+   built-in 'default' profile or the default profile set in swtpm_setup.conf.
+   Otherwise swtpm_setup will search for a profile with the given name with
+   appended .json suffix in a configurable local and then in a distro
+   directory. If none could be found in either, it will fall back trying to
+   use a built-in one.
 
    The built-in 'null' profile provides backwards compatibility with
    libtpms v0.9 but also restricts the user to use only TPM features that were
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7d91e3e958..6f6898014b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3479,6 +3479,7 @@ void virDomainTPMDefFree(virDomainTPMDef *def)
         g_free(def->data.emulator.logfile);
         virBitmapFree(def->data.emulator.activePcrBanks);
         g_free(def->data.emulator.profile_source);
+        g_free(def->data.emulator.profile_name);
         break;
     case VIR_DOMAIN_TPM_TYPE_EXTERNAL:
         virObjectUnref(def->data.external.source);
@@ -10926,10 +10927,6 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
 
         if ((profile = virXPathNode("./backend/profile[1]", ctxt))) {
             def->data.emulator.profile_source = virXMLPropString(profile, "source");
-            if (!def->data.emulator.profile_source) {
-                virReportError(VIR_ERR_XML_ERROR, "%s", _("missing profile source"));
-                goto error;
-            }
             if (virXMLPropEnum(profile, "remove_disabled",
                                virDomainTPMProfileRemoveDisabledTypeFromString,
                                VIR_XML_PROP_NONZERO,
@@ -10938,6 +10935,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
             if (profile_remove_disabled != VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_NONE)
                 def->data.emulator.profile_remove_disabled =
                     virDomainTPMProfileRemoveDisabledTypeToString(profile_remove_disabled);
+            def->data.emulator.profile_name = virXMLPropString(profile, "name");
         }
         break;
     case VIR_DOMAIN_TPM_TYPE_EXTERNAL:
@@ -25134,12 +25132,18 @@ virDomainTPMDefFormat(virBuffer *buf,
                               virDomainTPMSourceTypeTypeToString(def->data.emulator.source_type));
             virBufferEscapeString(&backendChildBuf, " path='%s'/>\n", def->data.emulator.source_path);
         }
-        if (def->data.emulator.profile_source) {
-            virBufferAsprintf(&backendChildBuf, "<profile source='%s'",
-                              def->data.emulator.profile_source);
+        if (def->data.emulator.profile_source ||
+            def->data.emulator.profile_name) {
+            virBufferAddLit(&backendChildBuf, "<profile");
+            if (def->data.emulator.profile_source)
+                virBufferAsprintf(&backendChildBuf, " source='%s'",
+                                  def->data.emulator.profile_source);
             if (def->data.emulator.profile_remove_disabled)
                virBufferAsprintf(&backendChildBuf, " remove_disabled='%s'",
                                  def->data.emulator.profile_remove_disabled);
+            if (def->data.emulator.profile_name)
+                virBufferAsprintf(&backendChildBuf, " name='%s'",
+                                  def->data.emulator.profile_name);
             virBufferAddLit(&backendChildBuf, "/>\n");
         }
         break;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bd2740af26..45421d4772 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1493,6 +1493,7 @@ struct _virDomainTPMEmulatorDef {
     bool persistent_state;
     virBitmap *activePcrBanks;
     char *profile_source; /* 'source' profile was created from */
+    char *profile_name;   /* name read from active profile */
     const char *profile_remove_disabled;
 };
 
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index d94ff9b4c3..e26e65fd7c 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -6056,9 +6056,11 @@
   <define name="tpm-backend-emulator-profile">
     <optional>
       <element name="profile">
-        <attribute name="source">
-          <ref name="profileName"/>
-        </attribute>
+        <optional>
+          <attribute name="source">
+            <ref name="profileName"/>
+          </attribute>
+        </optional>
         <optional>
           <attribute name="remove_disabled">
             <choice>
@@ -6067,6 +6069,11 @@
             </choice>
           </attribute>
         </optional>
+        <optional>
+          <attribute name="name">
+            <ref name="profileName"/>
+          </attribute>
+        </optional>
       </element>
     </optional>
   </define>
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index dc1bb56237..a6f31f9773 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -175,6 +175,7 @@ qemuExtDevicesStart(virQEMUDriver *driver,
                     virDomainObj *vm,
                     bool incomingMigration)
 {
+    virDomainDef *persistentDef = vm->newDef;
     virDomainDef *def = vm->def;
     size_t i;
 
@@ -189,9 +190,11 @@ qemuExtDevicesStart(virQEMUDriver *driver,
 
     for (i = 0; i < def->ntpms; i++) {
         virDomainTPMDef *tpm = def->tpms[i];
+        virDomainTPMDef *persistentTPMDef = persistentDef->tpms[i];
 
         if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
-            qemuExtTPMStart(driver, vm, tpm, incomingMigration) < 0)
+            qemuExtTPMStart(driver, vm, tpm, persistentTPMDef,
+                            incomingMigration) < 0)
             return -1;
     }
 
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index a7eee501bf..2fb3796910 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -627,15 +627,89 @@ qemuTPMVirCommandSwtpmAddTPMState(virCommand *cmd,
     }
 }
 
+/* qemuTPMEmulatorUpdateProfileName:
+ *
+ * @emulator: TPM emulator definition
+ * @persistentTPMDef: TPM definition from the persistent domain definition
+ * @cfg: virQEMUDriverConfig
+ * @saveDef: whether caller should save the persistent domain def
+ */
+static int
+qemuTPMEmulatorUpdateProfileName(virDomainTPMEmulatorDef *emulator,
+                                 virDomainTPMDef *persistentTPMDef,
+                                 const virQEMUDriverConfig *cfg,
+                                 bool *saveDef)
+{
+    g_autoptr(virJSONValue) object = NULL;
+    g_autofree char *stderr_buf = NULL;
+    g_autofree char *stdout_buf = NULL;
+    g_autoptr(virCommand) cmd = NULL;
+    g_autofree char *swtpm = NULL;
+    virJSONValue *active_profile;
+    const char *profile_name;
+    int exitstatus;
+
+    if (emulator->version != VIR_DOMAIN_TPM_VERSION_2_0 ||
+        !virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PRINT_INFO))
+        return 0;
+
+    swtpm = virTPMGetSwtpm();
+    if (!swtpm)
+        return -1;
+
+    cmd = virCommandNew(swtpm);
+
+    virCommandSetUID(cmd, cfg->swtpm_user); /* should be uid of 'tss' or 'root' */
+    virCommandSetGID(cmd, cfg->swtpm_group);
+
+    virCommandAddArgList(cmd, "socket", "--print-info", "0x20", "--tpm2", NULL);
+
+    qemuTPMVirCommandSwtpmAddTPMState(cmd, emulator);
+
+    if (qemuTPMVirCommandSwtpmAddEncryption(cmd, emulator, swtpm) < 0)
+        return -1;
+
+    virCommandClearCaps(cmd);
+
+    virCommandSetOutputBuffer(cmd, &stdout_buf);
+    virCommandSetErrorBuffer(cmd, &stderr_buf);
+
+    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not run '%1$s --print-info'. exitstatus: %2$d; stderr: %3$s\n"),
+                          swtpm, exitstatus, stderr_buf);
+        return -1;
+    }
+
+    if (!(object = virJSONValueFromString(stdout_buf)))
+        return -1;
+
+    if (!(active_profile = virJSONValueObjectGetObject(object, "ActiveProfile")))
+        return -1;
+
+    profile_name = g_strdup(virJSONValueObjectGetString(active_profile, "Name"));
+
+    g_free(emulator->profile_name);
+    emulator->profile_name = g_strdup(profile_name);
+
+    *saveDef = true;
+    g_free(persistentTPMDef->data.emulator.profile_name);
+    persistentTPMDef->data.emulator.profile_name = g_strdup(profile_name);
+
+    return 0;
+}
+
 /*
  * qemuTPMEmulatorBuildCommand:
  *
  * @tpm: TPM definition
+ * @persistentTPMDef: TPM definition from the persistent domain definition
  * @vmname: The name of the VM
  * @vmuuid: The UUID of the VM
  * @privileged: whether we are running in privileged mode
  * @cfg: virQEMUDriverConfig
  * @incomingMigration: whether we have an incoming migration
+ * @saveDef: whether caller should save the persistent domain def
  *
  * Create the virCommand use for starting the emulator
  * Do some initializations on the way, such as creation of storage
@@ -643,11 +717,13 @@ qemuTPMVirCommandSwtpmAddTPMState(virCommand *cmd,
  */
 static virCommand *
 qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
+                            virDomainTPMDef *persistentTPMDef,
                             const char *vmname,
                             const unsigned char *vmuuid,
                             bool privileged,
                             const virQEMUDriverConfig *cfg,
-                            bool incomingMigration)
+                            bool incomingMigration,
+                            bool *saveDef)
 {
     g_autoptr(virCommand) cmd = NULL;
     bool created = false;
@@ -696,6 +772,11 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
                                 incomingMigration) < 0)
         goto error;
 
+    if (run_setup && !incomingMigration &&
+        qemuTPMEmulatorUpdateProfileName(&tpm->data.emulator, persistentTPMDef,
+                                         cfg, saveDef) < 0)
+        goto error;
+
     if (!incomingMigration &&
         qemuTPMEmulatorReconfigure(&tpm->data.emulator, cfg, secretuuid) < 0)
         goto error;
@@ -995,6 +1076,7 @@ qemuExtTPMEmulatorSetupCgroup(const char *swtpmStateDir,
  * @driver: QEMU driver
  * @vm: the domain object
  * @tpm: TPM definition
+ * @persistentTPMDef: TPM definition from persistent domain definition
  * @shortName: short and unique name of the domain
  * @incomingMigration: whether we have an incoming migration
  *
@@ -1007,6 +1089,7 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
                      virDomainObj *vm,
                      const char *shortName,
                      virDomainTPMDef *tpm,
+                     virDomainTPMDef *persistentTPMDef,
                      bool incomingMigration)
 {
     g_autoptr(virCommand) cmd = NULL;
@@ -1015,6 +1098,7 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
     g_autofree char *pidfile = NULL;
     virTimeBackOffVar timebackoff;
     const unsigned long long timeout = 1000; /* ms */
+    bool saveDef = false;
     pid_t pid = -1;
     bool lockMetadataException = false;
 
@@ -1023,12 +1107,18 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
     /* stop any left-over TPM emulator for this VM */
     qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName);
 
-    if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, vm->def->name, vm->def->uuid,
+    if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, persistentTPMDef,
+                                            vm->def->name, vm->def->uuid,
                                             driver->privileged,
                                             cfg,
-                                            incomingMigration)))
+                                            incomingMigration,
+                                            &saveDef)))
         return -1;
 
+    if (saveDef &&
+        virDomainDefSave(vm->newDef, driver->xmlopt, cfg->configDir) < 0)
+        goto error;
+
     if (qemuExtDeviceLogCommand(driver, vm, cmd, "TPM Emulator") < 0)
         return -1;
 
@@ -1212,6 +1302,7 @@ int
 qemuExtTPMStart(virQEMUDriver *driver,
                 virDomainObj *vm,
                 virDomainTPMDef *tpm,
+                virDomainTPMDef *persistentTPMDef,
                 bool incomingMigration)
 {
     g_autofree char *shortName = virDomainDefGetShortName(vm->def);
@@ -1219,7 +1310,8 @@ qemuExtTPMStart(virQEMUDriver *driver,
     if (!shortName)
         return -1;
 
-    return qemuTPMEmulatorStart(driver, vm, shortName, tpm, incomingMigration);
+    return qemuTPMEmulatorStart(driver, vm, shortName, tpm, persistentTPMDef,
+                                incomingMigration);
 }
 
 
diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h
index 3071dc3f71..7096060a2a 100644
--- a/src/qemu/qemu_tpm.h
+++ b/src/qemu/qemu_tpm.h
@@ -44,9 +44,10 @@ void qemuExtTPMCleanupHost(virQEMUDriver *driver,
 int qemuExtTPMStart(virQEMUDriver *driver,
                     virDomainObj *vm,
                     virDomainTPMDef *def,
+                    virDomainTPMDef *persistentDefTPM,
                     bool incomingMigration)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-    ATTRIBUTE_NONNULL(3)
+    ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
     G_GNUC_WARN_UNUSED_RESULT;
 
 void qemuExtTPMStop(virQEMUDriver *driver,
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index 1c736b0229..4016ad8fc4 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -42,6 +42,7 @@ VIR_ENUM_IMPL(virTPMSwtpmFeature,
               "cmdarg-migration",
               "nvram-backend-dir",
               "nvram-backend-file",
+              "cmdarg-print-info",
 );
 
 VIR_ENUM_IMPL(virTPMSwtpmSetupFeature,
diff --git a/src/util/virtpm.h b/src/util/virtpm.h
index 9ca09c2d80..03fb92629a 100644
--- a/src/util/virtpm.h
+++ b/src/util/virtpm.h
@@ -33,6 +33,7 @@ typedef enum {
     VIR_TPM_SWTPM_FEATURE_CMDARG_MIGRATION,
     VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_DIR,
     VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_FILE,
+    VIR_TPM_SWTPM_FEATURE_CMDARG_PRINT_INFO,
 
     VIR_TPM_SWTPM_FEATURE_LAST
 } virTPMSwtpmFeature;
-- 
2.47.0
Re: [PATCH v4 11/11] qemu: Read back the profile name after creation of a TPM instance
Posted by Michal Prívozník 6 days, 3 hours ago
On 11/13/24 18:39, Stefan Berger wrote:
> Get the JSON profile that the swtpm instance was created with from the
> output of 'swtpm socket --tpm2 --print-info 0x20 --tpmstate ...'. Get the
> name of the profile from the JSON and set it in the current and persistent
> emulator descriptions as 'name' attribute and have the persistent
> description stored with this update. The user should avoid setting this
> 'name' attribute since it is meant to be read-only. The following is
> an example of how the XML could look like:
> 
>   <profile source='local:restricted' name='custom:restricted'/>
> 
> If the user provided no profile node, and therefore swtpm_setup picked its
> default profile, the XML may now shows the 'name' attribute with the name
> of the profile. This makes the 'source' attribute now optional.
> 
>   <profile name='default-v1'/>
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  docs/formatdomain.rst             |  16 ++---
>  src/conf/domain_conf.c            |  18 +++---
>  src/conf/domain_conf.h            |   1 +
>  src/conf/schemas/domaincommon.rng |  13 +++-
>  src/qemu/qemu_extdevice.c         |   5 +-
>  src/qemu/qemu_tpm.c               | 100 ++++++++++++++++++++++++++++--
>  src/qemu/qemu_tpm.h               |   3 +-
>  src/util/virtpm.c                 |   1 +
>  src/util/virtpm.h                 |   1 +
>  9 files changed, 135 insertions(+), 23 deletions(-)
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 6539f620fa..20c86087ef 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -8131,7 +8131,7 @@ Example: usage of the TPM Emulator
>             <active_pcr_banks>
>                 <sha256/>
>             </active_pcr_banks>
> -           <profile source='local:restricted' remove_disabled='check'/>
> +           <profile source='local:restricted' remove_disabled='check' name='custom:restricted'/>
>           </backend>
>         </tpm>
>       </devices>
> @@ -8229,12 +8229,14 @@ Example: usage of the TPM Emulator
>  ``profile``
>     The ``profile`` node is used to set a profile for a TPM 2.0 given in the
>     source attribute. This profile will be set when the TPM is initially
> -   created and after that cannot be changed anymore. If no profile is provided,
> -   then swtpm will use the latest built-in 'default' profile or the default
> -   profile set in swtpm_setup.conf. Otherwise swtpm_setup will search for a
> -   profile with the given name with appended .json suffix in a configurable
> -   local and then in a distro directory. If none could be found in either, it
> -   will fall back trying to use a built-in one.
> +   created and after that cannot be changed anymore. Once a profile has been
> +   set the name attribute will be updated with the name of the profile that
> +   is running. If no profile is provided, then swtpm will use the latest
> +   built-in 'default' profile or the default profile set in swtpm_setup.conf.
> +   Otherwise swtpm_setup will search for a profile with the given name with
> +   appended .json suffix in a configurable local and then in a distro
> +   directory. If none could be found in either, it will fall back trying to
> +   use a built-in one.
>  
>     The built-in 'null' profile provides backwards compatibility with
>     libtpms v0.9 but also restricts the user to use only TPM features that were
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7d91e3e958..6f6898014b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3479,6 +3479,7 @@ void virDomainTPMDefFree(virDomainTPMDef *def)
>          g_free(def->data.emulator.logfile);
>          virBitmapFree(def->data.emulator.activePcrBanks);
>          g_free(def->data.emulator.profile_source);
> +        g_free(def->data.emulator.profile_name);
>          break;
>      case VIR_DOMAIN_TPM_TYPE_EXTERNAL:
>          virObjectUnref(def->data.external.source);
> @@ -10926,10 +10927,6 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
>  
>          if ((profile = virXPathNode("./backend/profile[1]", ctxt))) {
>              def->data.emulator.profile_source = virXMLPropString(profile, "source");
> -            if (!def->data.emulator.profile_source) {
> -                virReportError(VIR_ERR_XML_ERROR, "%s", _("missing profile source"));
> -                goto error;
> -            }
>              if (virXMLPropEnum(profile, "remove_disabled",
>                                 virDomainTPMProfileRemoveDisabledTypeFromString,
>                                 VIR_XML_PROP_NONZERO,
> @@ -10938,6 +10935,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
>              if (profile_remove_disabled != VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_NONE)
>                  def->data.emulator.profile_remove_disabled =
>                      virDomainTPMProfileRemoveDisabledTypeToString(profile_remove_disabled);
> +            def->data.emulator.profile_name = virXMLPropString(profile, "name");
>          }
>          break;
>      case VIR_DOMAIN_TPM_TYPE_EXTERNAL:
> @@ -25134,12 +25132,18 @@ virDomainTPMDefFormat(virBuffer *buf,
>                                virDomainTPMSourceTypeTypeToString(def->data.emulator.source_type));
>              virBufferEscapeString(&backendChildBuf, " path='%s'/>\n", def->data.emulator.source_path);
>          }
> -        if (def->data.emulator.profile_source) {
> -            virBufferAsprintf(&backendChildBuf, "<profile source='%s'",
> -                              def->data.emulator.profile_source);
> +        if (def->data.emulator.profile_source ||
> +            def->data.emulator.profile_name) {
> +            virBufferAddLit(&backendChildBuf, "<profile");
> +            if (def->data.emulator.profile_source)
> +                virBufferAsprintf(&backendChildBuf, " source='%s'",
> +                                  def->data.emulator.profile_source);
>              if (def->data.emulator.profile_remove_disabled)
>                 virBufferAsprintf(&backendChildBuf, " remove_disabled='%s'",
>                                   def->data.emulator.profile_remove_disabled);
> +            if (def->data.emulator.profile_name)
> +                virBufferAsprintf(&backendChildBuf, " name='%s'",
> +                                  def->data.emulator.profile_name);
>              virBufferAddLit(&backendChildBuf, "/>\n");
>          }
>          break;
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index bd2740af26..45421d4772 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1493,6 +1493,7 @@ struct _virDomainTPMEmulatorDef {
>      bool persistent_state;
>      virBitmap *activePcrBanks;
>      char *profile_source; /* 'source' profile was created from */
> +    char *profile_name;   /* name read from active profile */
>      const char *profile_remove_disabled;
>  };
>  
> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> index d94ff9b4c3..e26e65fd7c 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -6056,9 +6056,11 @@
>    <define name="tpm-backend-emulator-profile">
>      <optional>
>        <element name="profile">
> -        <attribute name="source">
> -          <ref name="profileName"/>
> -        </attribute>
> +        <optional>
> +          <attribute name="source">
> +            <ref name="profileName"/>
> +          </attribute>
> +        </optional>
>          <optional>
>            <attribute name="remove_disabled">
>              <choice>
> @@ -6067,6 +6069,11 @@
>              </choice>
>            </attribute>
>          </optional>
> +        <optional>
> +          <attribute name="name">
> +            <ref name="profileName"/>
> +          </attribute>
> +        </optional>
>        </element>
>      </optional>
>    </define>
> diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
> index dc1bb56237..a6f31f9773 100644
> --- a/src/qemu/qemu_extdevice.c
> +++ b/src/qemu/qemu_extdevice.c
> @@ -175,6 +175,7 @@ qemuExtDevicesStart(virQEMUDriver *driver,
>                      virDomainObj *vm,
>                      bool incomingMigration)
>  {
> +    virDomainDef *persistentDef = vm->newDef;
>      virDomainDef *def = vm->def;
>      size_t i;
>  
> @@ -189,9 +190,11 @@ qemuExtDevicesStart(virQEMUDriver *driver,
>  
>      for (i = 0; i < def->ntpms; i++) {
>          virDomainTPMDef *tpm = def->tpms[i];
> +        virDomainTPMDef *persistentTPMDef = persistentDef->tpms[i];
>  
>          if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
> -            qemuExtTPMStart(driver, vm, tpm, incomingMigration) < 0)
> +            qemuExtTPMStart(driver, vm, tpm, persistentTPMDef,
> +                            incomingMigration) < 0)
>              return -1;
>      }
>  
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index a7eee501bf..2fb3796910 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -627,15 +627,89 @@ qemuTPMVirCommandSwtpmAddTPMState(virCommand *cmd,
>      }
>  }
>  
> +/* qemuTPMEmulatorUpdateProfileName:
> + *
> + * @emulator: TPM emulator definition
> + * @persistentTPMDef: TPM definition from the persistent domain definition
> + * @cfg: virQEMUDriverConfig
> + * @saveDef: whether caller should save the persistent domain def
> + */
> +static int
> +qemuTPMEmulatorUpdateProfileName(virDomainTPMEmulatorDef *emulator,
> +                                 virDomainTPMDef *persistentTPMDef,
> +                                 const virQEMUDriverConfig *cfg,
> +                                 bool *saveDef)
> +{
> +    g_autoptr(virJSONValue) object = NULL;
> +    g_autofree char *stderr_buf = NULL;
> +    g_autofree char *stdout_buf = NULL;
> +    g_autoptr(virCommand) cmd = NULL;
> +    g_autofree char *swtpm = NULL;
> +    virJSONValue *active_profile;
> +    const char *profile_name;
> +    int exitstatus;
> +
> +    if (emulator->version != VIR_DOMAIN_TPM_VERSION_2_0 ||
> +        !virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PRINT_INFO))
> +        return 0;
> +
> +    swtpm = virTPMGetSwtpm();
> +    if (!swtpm)
> +        return -1;
> +
> +    cmd = virCommandNew(swtpm);
> +
> +    virCommandSetUID(cmd, cfg->swtpm_user); /* should be uid of 'tss' or 'root' */
> +    virCommandSetGID(cmd, cfg->swtpm_group);
> +
> +    virCommandAddArgList(cmd, "socket", "--print-info", "0x20", "--tpm2", NULL);
> +
> +    qemuTPMVirCommandSwtpmAddTPMState(cmd, emulator);
> +
> +    if (qemuTPMVirCommandSwtpmAddEncryption(cmd, emulator, swtpm) < 0)
> +        return -1;
> +
> +    virCommandClearCaps(cmd);
> +
> +    virCommandSetOutputBuffer(cmd, &stdout_buf);
> +    virCommandSetErrorBuffer(cmd, &stderr_buf);
> +
> +    if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Could not run '%1$s --print-info'. exitstatus: %2$d; stderr: %3$s\n"),

We don't really put newlines at the end of messages.

> +                          swtpm, exitstatus, stderr_buf);
> +        return -1;
> +    }
> +
> +    if (!(object = virJSONValueFromString(stdout_buf)))
> +        return -1;
> +
> +    if (!(active_profile = virJSONValueObjectGetObject(object, "ActiveProfile")))
> +        return -1;
> +
> +    profile_name = g_strdup(virJSONValueObjectGetString(active_profile, "Name"));

This g_strdup() looks suspicios and surely must lead to a memleak.

> +
> +    g_free(emulator->profile_name);
> +    emulator->profile_name = g_strdup(profile_name);
> +
> +    *saveDef = true;
> +    g_free(persistentTPMDef->data.emulator.profile_name);
> +    persistentTPMDef->data.emulator.profile_name = g_strdup(profile_name);
> +
> +    return 0;
> +}
> +

Michal
Re: [PATCH v4 11/11] qemu: Read back the profile name after creation of a TPM instance
Posted by Stefan Berger 5 days, 15 hours ago

On 11/15/24 4:19 AM, Michal Prívozník wrote:
> On 11/13/24 18:39, Stefan Berger wrote:
> 
>> +                          swtpm, exitstatus, stderr_buf);
>> +        return -1;
>> +    }
>> +
>> +    if (!(object = virJSONValueFromString(stdout_buf)))
>> +        return -1;
>> +
>> +    if (!(active_profile = virJSONValueObjectGetObject(object, "ActiveProfile")))
>> +        return -1;
>> +
>> +    profile_name = g_strdup(virJSONValueObjectGetString(active_profile, "Name"));
> 
> This g_strdup() looks suspicios and surely must lead to a memleak.

Correct, one too many g_strdup's here.

> 
>> +
>> +    g_free(emulator->profile_name);
>> +    emulator->profile_name = g_strdup(profile_name);
>> +
>> +    *saveDef = true;
>> +    g_free(persistentTPMDef->data.emulator.profile_name);
>> +    persistentTPMDef->data.emulator.profile_name = g_strdup(profile_name);
>> +
>> +    return 0;
>> +}
>> +
> 
> Michal
>