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

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(a)linux.vnet.ibm.com&gt; > --- > 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.

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

> + 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 (!(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...

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" } >