[libvirt] [PATCH v3 08/14] qemu: Extend qemu_conf with tpm-emulator support

Stefan Berger stefanb at linux.vnet.ibm.com
Wed May 9 18:24:30 UTC 2018


On 05/08/2018 04:30 PM, John Ferlan wrote:
>
> On 05/04/2018 04:21 PM, Stefan Berger wrote:
>> Extend qemu_conf with user and group for running the tpm-emulator
>> and add directories to the configuration for the locations of the
>> log, state, and socket of the tpm-emulator.
>>
>> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
>> ---
>>   src/qemu/libvirtd_qemu.aug         |  5 +++++
>>   src/qemu/qemu.conf                 |  8 +++++++
>>   src/qemu/qemu_conf.c               | 43 ++++++++++++++++++++++++++++++++++++++
>>   src/qemu/qemu_conf.h               |  6 ++++++
>>   src/qemu/test_libvirtd_qemu.aug.in |  2 ++
>>   5 files changed, 64 insertions(+)
>>
> I think you'd need to also alter libvirt.spec.in since you're adding new
> directories... That's one of those make rpm type activities IIRC.

Adding that to this patch.

>
>> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
>> index c19bf3a..23bfe67 100644
>> --- a/src/qemu/libvirtd_qemu.aug
>> +++ b/src/qemu/libvirtd_qemu.aug
>> @@ -118,6 +118,9 @@ module Libvirtd_qemu =
>>      let vxhs_entry = bool_entry "vxhs_tls"
>>                    | str_entry "vxhs_tls_x509_cert_dir"
>>   
>> +   let swtpm_user_entry = str_entry "swtpm_user"
>> +   let swtpm_group_entry = str_entry "swtpm_group"
>> +
>>      (* Each entry in the config is one of the following ... *)
>>      let entry = default_tls_entry
>>                | vnc_entry
>> @@ -137,6 +140,8 @@ module Libvirtd_qemu =
>>                | gluster_debug_level_entry
>>                | memory_entry
>>                | vxhs_entry
>> +             | swtpm_user_entry
>> +             | swtpm_group_entry
>>   
>>      let comment = [ label "#comment" . del /#[ \t]*/ "# " .  store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ]
>>      let empty = [ label "#empty" . eol ]
>> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
>> index 3444185..26a6dc7 100644
>> --- a/src/qemu/qemu.conf
>> +++ b/src/qemu/qemu.conf
>> @@ -779,3 +779,11 @@
>>   # This directory is used for memoryBacking source if configured as file.
>>   # NOTE: big files will be stored here
>>   #memory_backing_dir = "/var/lib/libvirt/qemu/ram"
>> +
>> +# User for the swtpm TPM Emulator
>> +#
>> +# Default is 'tss'; this is the same user that tcsd (TrouSerS) installs
>> +# and uses; alternative is 'root'
>> +#
>> +#swtpm_user = "tss"
>> +#swtpm_group = "tss"
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index bfbb572..99c37c6 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -159,6 +159,10 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>>                           "%s/log/libvirt/qemu", LOCALSTATEDIR) < 0)
>>               goto error;
>>   
>> +        if (virAsprintf(&cfg->swtpmLogDir,
>> +                        "%s/log/swtpm/libvirt/qemu", LOCALSTATEDIR) < 0)
>> +            goto error;
>> +
>>           if (VIR_STRDUP(cfg->configBaseDir, SYSCONFDIR "/libvirt") < 0)
>>               goto error;
>>   
>> @@ -166,6 +170,10 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>>                         "%s/run/libvirt/qemu", LOCALSTATEDIR) < 0)
>>               goto error;
>>   
>> +        if (virAsprintf(&cfg->swtpmStateDir,
>> +                       "%s/run/libvirt/qemu/swtpm", LOCALSTATEDIR) < 0)
>> +            goto error;
>> +
>>           if (virAsprintf(&cfg->cacheDir,
>>                         "%s/cache/libvirt/qemu", LOCALSTATEDIR) < 0)
>>               goto error;
>> @@ -186,6 +194,13 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>>               goto error;
>>           if (virAsprintf(&cfg->memoryBackingDir, "%s/ram", cfg->libDir) < 0)
>>               goto error;
>> +        if (virAsprintf(&cfg->swtpmStorageDir, "%s/lib/libvirt/swtpm",
>> +                        LOCALSTATEDIR) < 0)
>> +            goto error;
>> +        if (virGetUserID("tss", &cfg->swtpm_user) < 0)
>> +            cfg->swtpm_user = 0; /* fall back to root */
>> +        if (virGetGroupID("tss", &cfg->swtpm_group) < 0)
>> +            cfg->swtpm_group = 0; /* fall back to root */
>>       } else {
>>           char *rundir;
>>           char *cachedir;
>> @@ -199,6 +214,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>>               VIR_FREE(cachedir);
>>               goto error;
>>           }
>> +        if (virAsprintf(&cfg->swtpmLogDir,
>> +                        "%s/qemu/log", cachedir) < 0) {
> Is it intentionally the same as ->logDir?  Or did you want to have it's
> own?  Doesn't matter to me - just asking.

Yes. Permissions are not an issue in this case while in the privileged 
case I had to put the swtpm logs elsewhere due to file permissions.

>
>> +            VIR_FREE(cachedir);
>> +            goto error;
>> +        }
>>           if (virAsprintf(&cfg->cacheDir, "%s/qemu/cache", cachedir) < 0) {
>>               VIR_FREE(cachedir);
>>               goto error;
>> @@ -214,6 +234,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>>           }
>>           VIR_FREE(rundir);
>>   
>> +        if (virAsprintf(&cfg->swtpmStateDir, "%s/swtpm", cfg->stateDir) < 0)
>> +            goto error;
>> +
> This one has it's own...  although I wonder if it should be swtpm/run to
> mimic cfg->stateDir

If 'run' implies that the directory can be deleted, like seems to be the 
case of /var/run/ between reboots, then we cannot put it there since the 
state of the TPM needs to be preserved.
>
>>           if (!(cfg->configBaseDir = virGetUserConfigDirectory()))
>>               goto error;
>>   
>> @@ -233,6 +256,10 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>>               goto error;
>>           if (virAsprintf(&cfg->memoryBackingDir, "%s/qemu/ram", cfg->configBaseDir) < 0)
>>               goto error;
>> +        if (virAsprintf(&cfg->swtpmStorageDir, "%s/qemu/swtpm", cfg->configBaseDir) < 0)
>> +            goto error;
> As does this one... and I think the path here is fine as it matches
> other uses.
>
>> +        cfg->swtpm_user = -1;
>> +        cfg->swtpm_group = -1;
> Use the (uid_t) and (gid_t) cast's...

Done.
>
> John
>
>>       }
>>   
>>       if (virAsprintf(&cfg->configDir, "%s/qemu", cfg->configBaseDir) < 0)
>> @@ -351,7 +378,9 @@ static void virQEMUDriverConfigDispose(void *obj)
>>       VIR_FREE(cfg->configDir);
>>       VIR_FREE(cfg->autostartDir);
>>       VIR_FREE(cfg->logDir);
>> +    VIR_FREE(cfg->swtpmLogDir);
>>       VIR_FREE(cfg->stateDir);
>> +    VIR_FREE(cfg->swtpmStateDir);
>>   
>>       VIR_FREE(cfg->libDir);
>>       VIR_FREE(cfg->cacheDir);
>> @@ -400,6 +429,7 @@ static void virQEMUDriverConfigDispose(void *obj)
>>       virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
>>   
>>       VIR_FREE(cfg->memoryBackingDir);
>> +    VIR_FREE(cfg->swtpmStorageDir);
>>   }
>>   
>>   
>> @@ -471,6 +501,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
>>       size_t i, j;
>>       char *stdioHandler = NULL;
>>       char *user = NULL, *group = NULL;
>> +    char *swtpm_user = NULL, *swtpm_group = NULL;
>>       char **controllers = NULL;
>>       char **hugetlbfs = NULL;
>>       char **nvram = NULL;
>> @@ -907,6 +938,16 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
>>       if (virConfGetValueString(conf, "memory_backing_dir", &cfg->memoryBackingDir) < 0)
>>           goto cleanup;
>>   
>> +    if (virConfGetValueString(conf, "swtpm_user", &swtpm_user) < 0)
>> +        goto cleanup;
>> +    if (swtpm_user && virGetUserID(swtpm_user, &cfg->swtpm_user) < 0)
>> +        goto cleanup;
>> +
>> +    if (virConfGetValueString(conf, "swtpm_group", &swtpm_group) < 0)
>> +        goto cleanup;
>> +    if (swtpm_group && virGetGroupID(swtpm_group, &cfg->swtpm_group) < 0)
>> +        goto cleanup;
>> +
>>       ret = 0;
>>   
>>    cleanup:
>> @@ -917,6 +958,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
>>       VIR_FREE(corestr);
>>       VIR_FREE(user);
>>       VIR_FREE(group);
>> +    VIR_FREE(swtpm_user);
>> +    VIR_FREE(swtpm_group);
>>       virConfFree(conf);
>>       return ret;
>>   }
>> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
>> index e1ad546..19dc0bc 100644
>> --- a/src/qemu/qemu_conf.h
>> +++ b/src/qemu/qemu_conf.h
>> @@ -102,7 +102,9 @@ struct _virQEMUDriverConfig {
>>       char *configDir;
>>       char *autostartDir;
>>       char *logDir;
>> +    char *swtpmLogDir;
>>       char *stateDir;
>> +    char *swtpmStateDir;
>>       /* These two directories are ones QEMU processes use (so must match
>>        * the QEMU user/group */
>>       char *libDir;
>> @@ -111,6 +113,7 @@ struct _virQEMUDriverConfig {
>>       char *snapshotDir;
>>       char *channelTargetDir;
>>       char *nvramDir;
>> +    char *swtpmStorageDir;
>>   
>>       char *defaultTLSx509certdir;
>>       bool checkdefaultTLSx509certdir;
>> @@ -206,6 +209,9 @@ struct _virQEMUDriverConfig {
>>   
>>       bool vxhsTLS;
>>       char *vxhsTLSx509certdir;
>> +
>> +    uid_t swtpm_user;
>> +    gid_t swtpm_group;
>>   };
>>   
>>   /* Main driver state */
>> diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
>> index 688e5b9..6d6e1d4 100644
>> --- a/src/qemu/test_libvirtd_qemu.aug.in
>> +++ b/src/qemu/test_libvirtd_qemu.aug.in
>> @@ -100,3 +100,5 @@ module Test_libvirtd_qemu =
>>       { "1" = "mount" }
>>   }
>>   { "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" }
>> +{ "swtpm_user" = "tss" }
>> +{ "swtpm_group" = "tss" }
>>




More information about the libvir-list mailing list