[libvirt PATCH 00/14] qemu: Improve swtpm internal API
Clean up inconsistencies to make navigating the module easier. Andrea Bolognani (14): qemu: Rename qemuExtTPMStartEmulator() qemu: Fully document qemuTPMEmulatorStart() qemu: Rename qemuTPM{Create,Delete}EmulatorStorage() qemu: Document qemuTPMEmulatorDeleteStorage() qemu: Drop qemuTPMEmulatorInitStorage() qemu: Make qemuTPMEmulatorCreateStorage() take a virDomainTPMDef* qemu: Introduce qemuExtTPMEmulatorSetupCgroup() qemu: Introduce qemuTPMEmulatorCleanupHost() qemu: Rename path-building functions qemu: Call virDomainDefGetShortName() less frequently qemu: Fix description of swtpmStateDir qemu: Move utility functions close together qemu: Move entry points close together qemu: Move high-level actions close together src/qemu/qemu_tpm.c | 606 +++++++++++++++++++++++--------------------- 1 file changed, 323 insertions(+), 283 deletions(-) -- 2.34.1
Its counterpart is qemuTPMEmulatorStop(). Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 9c5d1ffed4..c82e5921d7 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -875,9 +875,8 @@ qemuExtTPMCleanupHost(virDomainDef *def) } -/* - * qemuExtTPMStartEmulator: - * +/** + * qemuTPMEmulatorStart: * @driver: QEMU driver * @vm: the domain object * @incomingMigration: whether we have an incoming migration @@ -887,10 +886,10 @@ qemuExtTPMCleanupHost(virDomainDef *def) * - start the external TPM Emulator and sync with it before QEMU start */ static int -qemuExtTPMStartEmulator(virQEMUDriver *driver, - virDomainObj *vm, - virDomainTPMDef *tpm, - bool incomingMigration) +qemuTPMEmulatorStart(virQEMUDriver *driver, + virDomainObj *vm, + virDomainTPMDef *tpm, + bool incomingMigration) { g_autoptr(virCommand) cmd = NULL; VIR_AUTOCLOSE errfd = -1; @@ -994,8 +993,8 @@ qemuExtTPMStart(virQEMUDriver *driver, if (vm->def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) continue; - return qemuExtTPMStartEmulator(driver, vm, vm->def->tpms[i], - incomingMigration); + return qemuTPMEmulatorStart(driver, vm, vm->def->tpms[i], + incomingMigration); } return 0; -- 2.34.1
The @tpm argument was not mentioned. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index c82e5921d7..627c7e631d 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -879,6 +879,7 @@ qemuExtTPMCleanupHost(virDomainDef *def) * qemuTPMEmulatorStart: * @driver: QEMU driver * @vm: the domain object + * @tpm: TPM definition * @incomingMigration: whether we have an incoming migration * * Start the external TPM Emulator: -- 2.34.1
Other functions that operate on a single TPM emulator follow the qemuTPMEmulatorDoThing() naming convention. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 627c7e631d..8d66a2127e 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -123,9 +123,8 @@ qemuTPMEmulatorInitStorage(const char *swtpmStorageDir) } -/* - * qemuTPMCreateEmulatorStorage - * +/** + * qemuTPMEmulatorCreateStorage: * @storagepath: directory for swtpm's persistent state * @created: a pointer to a bool that will be set to true if the * storage was created because it did not exist yet @@ -137,7 +136,7 @@ qemuTPMEmulatorInitStorage(const char *swtpmStorageDir) * Adapt ownership of the directory and all swtpm's state files there. */ static int -qemuTPMCreateEmulatorStorage(const char *storagepath, +qemuTPMEmulatorCreateStorage(const char *storagepath, bool *created, uid_t swtpm_user, gid_t swtpm_group) @@ -168,7 +167,7 @@ qemuTPMCreateEmulatorStorage(const char *storagepath, static void -qemuTPMDeleteEmulatorStorage(virDomainTPMDef *tpm) +qemuTPMEmulatorDeleteStorage(virDomainTPMDef *tpm) { g_autofree char *path = g_path_get_dirname(tpm->data.emulator.storagepath); @@ -686,7 +685,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, if (!swtpm) return NULL; - if (qemuTPMCreateEmulatorStorage(tpm->data.emulator.storagepath, + if (qemuTPMEmulatorCreateStorage(tpm->data.emulator.storagepath, &created, swtpm_user, swtpm_group) < 0) return NULL; @@ -765,7 +764,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, error: if (created) - qemuTPMDeleteEmulatorStorage(tpm); + qemuTPMEmulatorDeleteStorage(tpm); return NULL; } @@ -870,7 +869,7 @@ qemuExtTPMCleanupHost(virDomainDef *def) continue; if (!def->tpms[i]->data.emulator.persistent_state) - qemuTPMDeleteEmulatorStorage(def->tpms[i]); + qemuTPMEmulatorDeleteStorage(def->tpms[i]); } } -- 2.34.1
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 8d66a2127e..182209bda6 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -166,6 +166,12 @@ qemuTPMEmulatorCreateStorage(const char *storagepath, } +/** + * qemuTPMEmulatorDeleteStorage: + * @tpm: TPM definition + * + * Delete all persistent storage associated with the swtpm. + */ static void qemuTPMEmulatorDeleteStorage(virDomainTPMDef *tpm) { -- 2.34.1
Absorb it into qemuTPMEmulatorCreateStorage(), its only caller. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 182209bda6..127a2c80be 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -99,30 +99,6 @@ qemuTPMCreateEmulatorLogPath(const char *logDir, } -/* - * qemuTPMEmulatorInitStorage - * - * Initialize the TPM Emulator storage by creating its root directory, - * which is typically found in /var/lib/libvirt/tpm. - * - */ -static int -qemuTPMEmulatorInitStorage(const char *swtpmStorageDir) -{ - int rc = 0; - - /* allow others to cd into this dir */ - if (g_mkdir_with_parents(swtpmStorageDir, 0711) < 0) { - virReportSystemError(errno, - _("Could not create TPM directory %s"), - swtpmStorageDir); - rc = -1; - } - - return rc; -} - - /** * qemuTPMEmulatorCreateStorage: * @storagepath: directory for swtpm's persistent state @@ -143,8 +119,13 @@ qemuTPMEmulatorCreateStorage(const char *storagepath, { g_autofree char *swtpmStorageDir = g_path_get_dirname(storagepath); - if (qemuTPMEmulatorInitStorage(swtpmStorageDir) < 0) + /* allow others to cd into this dir */ + if (g_mkdir_with_parents(swtpmStorageDir, 0711) < 0) { + virReportSystemError(errno, + _("Could not create TPM directory %s"), + swtpmStorageDir); return -1; + } *created = false; -- 2.34.1
This matches how qemuTPMEmulatorDeleteStorage() expects to be called. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 127a2c80be..2196b5c567 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -101,7 +101,7 @@ qemuTPMCreateEmulatorLogPath(const char *logDir, /** * qemuTPMEmulatorCreateStorage: - * @storagepath: directory for swtpm's persistent state + * @tpm: TPM definition for an emulator type * @created: a pointer to a bool that will be set to true if the * storage was created because it did not exist yet * @swtpm_user: The uid that needs to be able to access the directory @@ -112,11 +112,12 @@ qemuTPMCreateEmulatorLogPath(const char *logDir, * Adapt ownership of the directory and all swtpm's state files there. */ static int -qemuTPMEmulatorCreateStorage(const char *storagepath, +qemuTPMEmulatorCreateStorage(virDomainTPMDef *tpm, bool *created, uid_t swtpm_user, gid_t swtpm_group) { + const char *storagepath = tpm->data.emulator.storagepath; g_autofree char *swtpmStorageDir = g_path_get_dirname(storagepath); /* allow others to cd into this dir */ @@ -672,8 +673,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, if (!swtpm) return NULL; - if (qemuTPMEmulatorCreateStorage(tpm->data.emulator.storagepath, - &created, swtpm_user, swtpm_group) < 0) + if (qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group) < 0) return NULL; if (tpm->data.emulator.hassecretuuid) -- 2.34.1
This leaves qemuExtTPMSetupCgroup() to only deal with looping over TPM devices, same as other qemuExtTPMDoThing() functions. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 2196b5c567..2a2e585080 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -797,6 +797,36 @@ qemuTPMEmulatorStop(const char *swtpmStateDir, } +/** + * qemuExtTPMEmulatorSetupCgroup: + * @swtpmStateDir: directory for swtpm's persistent state + * @shortName: short and unique name of the domain + * @cgroup: cgroup to add the swtpm process to + * + * Add the swtpm process to the appropriate cgroup. + */ +static int +qemuExtTPMEmulatorSetupCgroup(const char *swtpmStateDir, + const char *shortName, + virCgroup *cgroup) +{ + int rc; + pid_t pid; + + rc = qemuTPMEmulatorGetPid(swtpmStateDir, shortName, &pid); + if (rc < 0 || (rc == 0 && pid == (pid_t)-1)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not get process id of swtpm")); + return -1; + } + + if (virCgroupAddProcess(cgroup, pid) < 0) + return -1; + + return 0; +} + + int qemuExtTPMInitPaths(virQEMUDriver *driver, virDomainDef *def) @@ -1019,8 +1049,6 @@ qemuExtTPMSetupCgroup(virQEMUDriver *driver, virCgroup *cgroup) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - int rc; - pid_t pid; size_t i; for (i = 0; i < def->ntpms; i++) { @@ -1032,13 +1060,8 @@ qemuExtTPMSetupCgroup(virQEMUDriver *driver, shortName = virDomainDefGetShortName(def); if (!shortName) return -1; - rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid); - if (rc < 0 || (rc == 0 && pid == (pid_t)-1)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not get process id of swtpm")); - return -1; - } - if (virCgroupAddProcess(cgroup, pid) < 0) + + if (qemuExtTPMEmulatorSetupCgroup(cfg->swtpmStateDir, shortName, cgroup) < 0) return -1; } -- 2.34.1
This leaves qemuExtTPMCleanupHost() to only deal with looping over TPM devices, same as other qemuExtTPMDoThing() functions. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 2a2e585080..754897bf93 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -259,6 +259,20 @@ qemuTPMEmulatorGetPid(const char *swtpmStateDir, } +/** + * qemuTPMEmulatorCleanupHost: + * @tpm: TPM definition + * + * Clean up persistent storage for the swtpm. + */ +static void +qemuTPMEmulatorCleanupHost(virDomainTPMDef *tpm) +{ + if (!tpm->data.emulator.persistent_state) + qemuTPMEmulatorDeleteStorage(tpm); +} + + /* * qemuTPMEmulatorPrepareHost: * @@ -885,8 +899,7 @@ qemuExtTPMCleanupHost(virDomainDef *def) if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) continue; - if (!def->tpms[i]->data.emulator.persistent_state) - qemuTPMEmulatorDeleteStorage(def->tpms[i]); + qemuTPMEmulatorCleanupHost(def->tpms[i]); } } -- 2.34.1
Using the word "create" can give users the impression that disk operations will be performed, when in reality all these functions do is string formatting. Follow the naming convention established by virBuildPath(), virFileBuildPath() and virPidFileBuildPath(). Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 69 +++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 754897bf93..d78acf8215 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -50,19 +50,18 @@ VIR_LOG_INIT("qemu.tpm"); -/* - * qemuTPMCreateEmulatorStoragePath - * +/** + * qemuTPMEmulatorStorageBuildPath: * @swtpmStorageDir: directory for swtpm persistent state - * @uuid: The UUID of the VM for which to create the storage + * @uuidstr: UUID of the VM * @tpmversion: version of the TPM * - * Create the swtpm's storage path + * Generate the swtpm's storage path. */ static char * -qemuTPMCreateEmulatorStoragePath(const char *swtpmStorageDir, - const char *uuidstr, - virDomainTPMVersion tpmversion) +qemuTPMEmulatorStorageBuildPath(const char *swtpmStorageDir, + const char *uuidstr, + virDomainTPMVersion tpmversion) { char *path = NULL; const char *dir = ""; @@ -85,15 +84,15 @@ qemuTPMCreateEmulatorStoragePath(const char *swtpmStorageDir, /** - * qemuTPMCreateEmulatorLogPath: - * @logDir: directory where swtpm writes its logs into + * qemuTPMEmulatorLogBuildPath: + * @logDir: directory for swtpm log files * @vmname: name of the VM * - * Create the swtpm's log path. + * Generate the swtpm's log path. */ static char* -qemuTPMCreateEmulatorLogPath(const char *logDir, - const char *vmname) +qemuTPMEmulatorLogBuildPath(const char *logDir, + const char *vmname) { return g_strdup_printf("%s/%s-swtpm.log", logDir, vmname); } @@ -163,17 +162,16 @@ qemuTPMEmulatorDeleteStorage(virDomainTPMDef *tpm) } -/* - * qemuTPMCreateEmulatorSocket: - * - * @swtpmStateDir: the directory where to create the socket in +/** + * qemuTPMEmulatorSocketBuildPath: + * @swtpmStateDir: directory for swtpm runtime state * @shortName: short and unique name of the domain * - * Create the Unix socket path from the given parameters + * Generate the swtpm's Unix socket path. */ static char * -qemuTPMCreateEmulatorSocket(const char *swtpmStateDir, - const char *shortName) +qemuTPMEmulatorSocketBuildPath(const char *swtpmStateDir, + const char *shortName) { return g_strdup_printf("%s/%s-swtpm.sock", swtpmStateDir, shortName); } @@ -202,25 +200,29 @@ qemuTPMEmulatorInitPaths(virDomainTPMDef *tpm, if (!tpm->data.emulator.storagepath && !(tpm->data.emulator.storagepath = - qemuTPMCreateEmulatorStoragePath(swtpmStorageDir, uuidstr, - tpm->version))) + qemuTPMEmulatorStorageBuildPath(swtpmStorageDir, uuidstr, + tpm->version))) return -1; if (!tpm->data.emulator.logfile) { - tpm->data.emulator.logfile = qemuTPMCreateEmulatorLogPath(logDir, - vmname); + tpm->data.emulator.logfile = qemuTPMEmulatorLogBuildPath(logDir, + vmname); } return 0; } -/* - * qemuTPMCreatePidFilename +/** + * qemuTPMEmulatorPidFileBuildPath: + * @swtpmStateDir: directory for swtpm runtime state + * @shortName: short and unique name of the domain + * + * Generate the swtpm's pidfile path. */ static char * -qemuTPMEmulatorCreatePidFilename(const char *swtpmStateDir, - const char *shortName) +qemuTPMEmulatorPidFileBuildPath(const char *swtpmStateDir, + const char *shortName) { g_autofree char *devicename = NULL; @@ -246,9 +248,8 @@ qemuTPMEmulatorGetPid(const char *swtpmStateDir, const char *shortName, pid_t *pid) { - g_autofree char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir, - shortName); - + g_autofree char *pidfile = qemuTPMEmulatorPidFileBuildPath(swtpmStateDir, + shortName); if (!pidfile) return -1; @@ -333,7 +334,7 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDef *tpm, /* create the socket filename */ if (!tpm->data.emulator.source->data.nix.path && !(tpm->data.emulator.source->data.nix.path = - qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName))) + qemuTPMEmulatorSocketBuildPath(swtpmStateDir, shortName))) return -1; tpm->data.emulator.source->type = VIR_DOMAIN_CHR_TYPE_UNIX; @@ -790,7 +791,7 @@ qemuTPMEmulatorStop(const char *swtpmStateDir, if (!swtpm_ioctl) return; - if (!(pathname = qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName))) + if (!(pathname = qemuTPMEmulatorSocketBuildPath(swtpmStateDir, shortName))) return; if (!virFileExists(pathname)) @@ -949,7 +950,7 @@ qemuTPMEmulatorStart(virQEMUDriver *driver, if (qemuExtDeviceLogCommand(driver, vm, cmd, "TPM Emulator") < 0) return -1; - if (!(pidfile = qemuTPMEmulatorCreatePidFilename(cfg->swtpmStateDir, shortName))) + if (!(pidfile = qemuTPMEmulatorPidFileBuildPath(cfg->swtpmStateDir, shortName))) return -1; virCommandDaemonize(cmd); -- 2.34.1
When looping over TPM devices for a domain, we can avoid calling this function for each iteration and call it once per domain instead. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index d78acf8215..b38aa5e7ce 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -869,17 +869,16 @@ qemuExtTPMPrepareHost(virQEMUDriver *driver, virDomainDef *def) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - g_autofree char *shortName = NULL; + g_autofree char *shortName = virDomainDefGetShortName(def); size_t i; + if (!shortName) + return -1; + for (i = 0; i < def->ntpms; i++) { if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) continue; - shortName = virDomainDefGetShortName(def); - if (!shortName) - return -1; - return qemuTPMEmulatorPrepareHost(def->tpms[i], cfg->swtpmLogDir, cfg->swtpm_user, cfg->swtpm_group, @@ -910,6 +909,7 @@ qemuExtTPMCleanupHost(virDomainDef *def) * @driver: QEMU driver * @vm: the domain object * @tpm: TPM definition + * @shortName: short and unique name of the domain * @incomingMigration: whether we have an incoming migration * * Start the external TPM Emulator: @@ -919,22 +919,19 @@ qemuExtTPMCleanupHost(virDomainDef *def) static int qemuTPMEmulatorStart(virQEMUDriver *driver, virDomainObj *vm, + const char *shortName, virDomainTPMDef *tpm, bool incomingMigration) { g_autoptr(virCommand) cmd = NULL; VIR_AUTOCLOSE errfd = -1; g_autoptr(virQEMUDriverConfig) cfg = NULL; - g_autofree char *shortName = virDomainDefGetShortName(vm->def); g_autofree char *pidfile = NULL; virTimeBackOffVar timebackoff; const unsigned long long timeout = 1000; /* ms */ int cmdret = 0; pid_t pid = -1; - if (!shortName) - return -1; - cfg = virQEMUDriverGetConfig(driver); /* stop any left-over TPM emulator for this VM */ @@ -1018,13 +1015,17 @@ qemuExtTPMStart(virQEMUDriver *driver, virDomainObj *vm, bool incomingMigration) { + g_autofree char *shortName = virDomainDefGetShortName(vm->def); size_t i; + if (!shortName) + return -1; + for (i = 0; i < vm->def->ntpms; i++) { if (vm->def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) continue; - return qemuTPMEmulatorStart(driver, vm, vm->def->tpms[i], + return qemuTPMEmulatorStart(driver, vm, shortName, vm->def->tpms[i], incomingMigration); } @@ -1037,18 +1038,16 @@ qemuExtTPMStop(virQEMUDriver *driver, virDomainObj *vm) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autofree char *shortName = virDomainDefGetShortName(vm->def); size_t i; - for (i = 0; i < vm->def->ntpms; i++) { - g_autofree char *shortName = NULL; + if (!shortName) + return; + for (i = 0; i < vm->def->ntpms; i++) { if (vm->def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) continue; - shortName = virDomainDefGetShortName(vm->def); - if (!shortName) - return; - qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); qemuSecurityCleanupTPMEmulator(driver, vm); } @@ -1063,18 +1062,16 @@ qemuExtTPMSetupCgroup(virQEMUDriver *driver, virCgroup *cgroup) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autofree char *shortName = virDomainDefGetShortName(def); size_t i; - for (i = 0; i < def->ntpms; i++) { - g_autofree char *shortName = NULL; + if (!shortName) + return -1; + for (i = 0; i < def->ntpms; i++) { if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) continue; - shortName = virDomainDefGetShortName(def); - if (!shortName) - return -1; - if (qemuExtTPMEmulatorSetupCgroup(cfg->swtpmStateDir, shortName, cgroup) < 0) return -1; } -- 2.34.1
This directory contains runtime state, not persistent state. The latter goes into swtpmStorageDir. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index b38aa5e7ce..c0e875627d 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -281,7 +281,7 @@ qemuTPMEmulatorCleanupHost(virDomainTPMDef *tpm) * @logDir: directory where swtpm writes its logs into * @swtpm_user: uid to run the swtpm with * @swtpm_group: gid to run the swtpm with - * @swtpmStateDir: directory for swtpm's persistent state + * @swtpmStateDir: directory for swtpm runtime state * @qemu_user: uid that qemu will run with; we share the socket file with it * @shortName: short and unique name of the domain * @@ -814,7 +814,7 @@ qemuTPMEmulatorStop(const char *swtpmStateDir, /** * qemuExtTPMEmulatorSetupCgroup: - * @swtpmStateDir: directory for swtpm's persistent state + * @swtpmStateDir: directory for swtpm runtime state * @shortName: short and unique name of the domain * @cgroup: cgroup to add the swtpm process to * -- 2.34.1
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 124 ++++++++++++++++++++++---------------------- 1 file changed, 62 insertions(+), 62 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index c0e875627d..264b9b1d60 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -98,6 +98,68 @@ qemuTPMEmulatorLogBuildPath(const char *logDir, } +/** + * qemuTPMEmulatorSocketBuildPath: + * @swtpmStateDir: directory for swtpm runtime state + * @shortName: short and unique name of the domain + * + * Generate the swtpm's Unix socket path. + */ +static char * +qemuTPMEmulatorSocketBuildPath(const char *swtpmStateDir, + const char *shortName) +{ + return g_strdup_printf("%s/%s-swtpm.sock", swtpmStateDir, shortName); +} + + +/** + * qemuTPMEmulatorPidFileBuildPath: + * @swtpmStateDir: directory for swtpm runtime state + * @shortName: short and unique name of the domain + * + * Generate the swtpm's pidfile path. + */ +static char * +qemuTPMEmulatorPidFileBuildPath(const char *swtpmStateDir, + const char *shortName) +{ + g_autofree char *devicename = NULL; + + devicename = g_strdup_printf("%s-swtpm", shortName); + + return virPidFileBuildPath(swtpmStateDir, devicename); +} + + +/* + * qemuTPMEmulatorGetPid + * + * @swtpmStateDir: the directory where swtpm writes the pidfile into + * @shortName: short name of the domain + * @pid: pointer to pid + * + * Return -1 upon error, or zero on successful reading of the pidfile. + * If the PID was not still alive, zero will be returned, and @pid will be + * set to -1; + */ +static int +qemuTPMEmulatorGetPid(const char *swtpmStateDir, + const char *shortName, + pid_t *pid) +{ + g_autofree char *pidfile = qemuTPMEmulatorPidFileBuildPath(swtpmStateDir, + shortName); + if (!pidfile) + return -1; + + if (virPidFileReadPathIfLocked(pidfile, pid) < 0) + return -1; + + return 0; +} + + /** * qemuTPMEmulatorCreateStorage: * @tpm: TPM definition for an emulator type @@ -162,21 +224,6 @@ qemuTPMEmulatorDeleteStorage(virDomainTPMDef *tpm) } -/** - * qemuTPMEmulatorSocketBuildPath: - * @swtpmStateDir: directory for swtpm runtime state - * @shortName: short and unique name of the domain - * - * Generate the swtpm's Unix socket path. - */ -static char * -qemuTPMEmulatorSocketBuildPath(const char *swtpmStateDir, - const char *shortName) -{ - return g_strdup_printf("%s/%s-swtpm.sock", swtpmStateDir, shortName); -} - - /* * qemuTPMEmulatorInitPaths: * @@ -213,53 +260,6 @@ qemuTPMEmulatorInitPaths(virDomainTPMDef *tpm, } -/** - * qemuTPMEmulatorPidFileBuildPath: - * @swtpmStateDir: directory for swtpm runtime state - * @shortName: short and unique name of the domain - * - * Generate the swtpm's pidfile path. - */ -static char * -qemuTPMEmulatorPidFileBuildPath(const char *swtpmStateDir, - const char *shortName) -{ - g_autofree char *devicename = NULL; - - devicename = g_strdup_printf("%s-swtpm", shortName); - - return virPidFileBuildPath(swtpmStateDir, devicename); -} - - -/* - * qemuTPMEmulatorGetPid - * - * @swtpmStateDir: the directory where swtpm writes the pidfile into - * @shortName: short name of the domain - * @pid: pointer to pid - * - * Return -1 upon error, or zero on successful reading of the pidfile. - * If the PID was not still alive, zero will be returned, and @pid will be - * set to -1; - */ -static int -qemuTPMEmulatorGetPid(const char *swtpmStateDir, - const char *shortName, - pid_t *pid) -{ - g_autofree char *pidfile = qemuTPMEmulatorPidFileBuildPath(swtpmStateDir, - shortName); - if (!pidfile) - return -1; - - if (virPidFileReadPathIfLocked(pidfile, pid) < 0) - return -1; - - return 0; -} - - /** * qemuTPMEmulatorCleanupHost: * @tpm: TPM definition -- 2.34.1
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 133 +++++++++++++++++++++++--------------------- 1 file changed, 71 insertions(+), 62 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 264b9b1d60..4c0b42e7ff 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -842,68 +842,6 @@ qemuExtTPMEmulatorSetupCgroup(const char *swtpmStateDir, } -int -qemuExtTPMInitPaths(virQEMUDriver *driver, - virDomainDef *def) -{ - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - size_t i; - - for (i = 0; i < def->ntpms; i++) { - if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) - continue; - - return qemuTPMEmulatorInitPaths(def->tpms[i], - cfg->swtpmStorageDir, - cfg->swtpmLogDir, - def->name, - def->uuid); - } - - return 0; -} - - -int -qemuExtTPMPrepareHost(virQEMUDriver *driver, - virDomainDef *def) -{ - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - g_autofree char *shortName = virDomainDefGetShortName(def); - size_t i; - - if (!shortName) - return -1; - - for (i = 0; i < def->ntpms; i++) { - if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) - continue; - - return qemuTPMEmulatorPrepareHost(def->tpms[i], cfg->swtpmLogDir, - cfg->swtpm_user, - cfg->swtpm_group, - cfg->swtpmStateDir, cfg->user, - shortName); - } - - return 0; -} - - -void -qemuExtTPMCleanupHost(virDomainDef *def) -{ - size_t i; - - for (i = 0; i < def->ntpms; i++) { - if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) - continue; - - qemuTPMEmulatorCleanupHost(def->tpms[i]); - } -} - - /** * qemuTPMEmulatorStart: * @driver: QEMU driver @@ -1010,6 +948,77 @@ qemuTPMEmulatorStart(virQEMUDriver *driver, } +/* --------------------- + * Module entry points + * --------------------- + * + * These are the public functions that will be called by other parts + * of the QEMU driver. + */ + + +int +qemuExtTPMInitPaths(virQEMUDriver *driver, + virDomainDef *def) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + size_t i; + + for (i = 0; i < def->ntpms; i++) { + if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) + continue; + + return qemuTPMEmulatorInitPaths(def->tpms[i], + cfg->swtpmStorageDir, + cfg->swtpmLogDir, + def->name, + def->uuid); + } + + return 0; +} + + +int +qemuExtTPMPrepareHost(virQEMUDriver *driver, + virDomainDef *def) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autofree char *shortName = virDomainDefGetShortName(def); + size_t i; + + if (!shortName) + return -1; + + for (i = 0; i < def->ntpms; i++) { + if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) + continue; + + return qemuTPMEmulatorPrepareHost(def->tpms[i], cfg->swtpmLogDir, + cfg->swtpm_user, + cfg->swtpm_group, + cfg->swtpmStateDir, cfg->user, + shortName); + } + + return 0; +} + + +void +qemuExtTPMCleanupHost(virDomainDef *def) +{ + size_t i; + + for (i = 0; i < def->ntpms; i++) { + if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) + continue; + + qemuTPMEmulatorCleanupHost(def->tpms[i]); + } +} + + int qemuExtTPMStart(virQEMUDriver *driver, virDomainObj *vm, -- 2.34.1
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_tpm.c | 245 +++++++++++++++++++++++--------------------- 1 file changed, 128 insertions(+), 117 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 4c0b42e7ff..50f9caabf3 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -224,123 +224,6 @@ qemuTPMEmulatorDeleteStorage(virDomainTPMDef *tpm) } -/* - * qemuTPMEmulatorInitPaths: - * - * @tpm: TPM definition for an emulator type - * @swtpmStorageDir: the general swtpm storage dir which is used as a base - * directory for creating VM specific directories - * @logDir: directory where swtpm writes its logs into - * @vmname: name of the VM - * @uuid: the UUID of the VM - */ -static int -qemuTPMEmulatorInitPaths(virDomainTPMDef *tpm, - const char *swtpmStorageDir, - const char *logDir, - const char *vmname, - const unsigned char *uuid) -{ - char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(uuid, uuidstr); - - if (!tpm->data.emulator.storagepath && - !(tpm->data.emulator.storagepath = - qemuTPMEmulatorStorageBuildPath(swtpmStorageDir, uuidstr, - tpm->version))) - return -1; - - if (!tpm->data.emulator.logfile) { - tpm->data.emulator.logfile = qemuTPMEmulatorLogBuildPath(logDir, - vmname); - } - - return 0; -} - - -/** - * qemuTPMEmulatorCleanupHost: - * @tpm: TPM definition - * - * Clean up persistent storage for the swtpm. - */ -static void -qemuTPMEmulatorCleanupHost(virDomainTPMDef *tpm) -{ - if (!tpm->data.emulator.persistent_state) - qemuTPMEmulatorDeleteStorage(tpm); -} - - -/* - * qemuTPMEmulatorPrepareHost: - * - * @tpm: tpm definition - * @logDir: directory where swtpm writes its logs into - * @swtpm_user: uid to run the swtpm with - * @swtpm_group: gid to run the swtpm with - * @swtpmStateDir: directory for swtpm runtime state - * @qemu_user: uid that qemu will run with; we share the socket file with it - * @shortName: short and unique name of the domain - * - * Prepare the log directory for the swtpm and adjust ownership of it and the - * log file we will be using. Prepare the state directory where we will share - * the socket between tss and qemu users. - */ -static int -qemuTPMEmulatorPrepareHost(virDomainTPMDef *tpm, - const char *logDir, - uid_t swtpm_user, - gid_t swtpm_group, - const char *swtpmStateDir, - uid_t qemu_user, - const char *shortName) -{ - /* create log dir ... allow 'tss' user to cd into it */ - if (g_mkdir_with_parents(logDir, 0711) < 0) - return -1; - - /* ... and adjust ownership */ - if (virDirCreate(logDir, 0730, swtpm_user, swtpm_group, - VIR_DIR_CREATE_ALLOW_EXIST) < 0) - return -1; - - if (!virFileExists(tpm->data.emulator.logfile) && - virFileTouch(tpm->data.emulator.logfile, 0644) < 0) { - return -1; - } - - /* ... and make sure it can be accessed by swtpm_user */ - if (chown(tpm->data.emulator.logfile, swtpm_user, swtpm_group) < 0) { - virReportSystemError(errno, - _("Could not chown on swtpm logfile %s"), - tpm->data.emulator.logfile); - return -1; - } - - /* - create our swtpm state dir ... - - QEMU user needs to be able to access the socket there - - swtpm group needs to be able to create files there - - in privileged mode 0570 would be enough, for non-privileged mode - we need 0770 - */ - if (virDirCreate(swtpmStateDir, 0770, qemu_user, swtpm_group, - VIR_DIR_CREATE_ALLOW_EXIST) < 0) - return -1; - - /* create the socket filename */ - if (!tpm->data.emulator.source->data.nix.path && - !(tpm->data.emulator.source->data.nix.path = - qemuTPMEmulatorSocketBuildPath(swtpmStateDir, shortName))) - return -1; - tpm->data.emulator.source->type = VIR_DOMAIN_CHR_TYPE_UNIX; - - return 0; -} - /* * qemuTPMSetupEncryption * @@ -772,6 +655,134 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, } +/* -------------------- + * High-level actions + * -------------------- + * + * Each of these corresponds to one of the public entry points + * defined below, but operates on a single TPM device instead of the + * entire VM. + */ + + +/* + * qemuTPMEmulatorInitPaths: + * + * @tpm: TPM definition for an emulator type + * @swtpmStorageDir: the general swtpm storage dir which is used as a base + * directory for creating VM specific directories + * @logDir: directory where swtpm writes its logs into + * @vmname: name of the VM + * @uuid: the UUID of the VM + */ +static int +qemuTPMEmulatorInitPaths(virDomainTPMDef *tpm, + const char *swtpmStorageDir, + const char *logDir, + const char *vmname, + const unsigned char *uuid) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(uuid, uuidstr); + + if (!tpm->data.emulator.storagepath && + !(tpm->data.emulator.storagepath = + qemuTPMEmulatorStorageBuildPath(swtpmStorageDir, uuidstr, + tpm->version))) + return -1; + + if (!tpm->data.emulator.logfile) { + tpm->data.emulator.logfile = qemuTPMEmulatorLogBuildPath(logDir, + vmname); + } + + return 0; +} + + +/** + * qemuTPMEmulatorCleanupHost: + * @tpm: TPM definition + * + * Clean up persistent storage for the swtpm. + */ +static void +qemuTPMEmulatorCleanupHost(virDomainTPMDef *tpm) +{ + if (!tpm->data.emulator.persistent_state) + qemuTPMEmulatorDeleteStorage(tpm); +} + + +/* + * qemuTPMEmulatorPrepareHost: + * + * @tpm: tpm definition + * @logDir: directory where swtpm writes its logs into + * @swtpm_user: uid to run the swtpm with + * @swtpm_group: gid to run the swtpm with + * @swtpmStateDir: directory for swtpm runtime state + * @qemu_user: uid that qemu will run with; we share the socket file with it + * @shortName: short and unique name of the domain + * + * Prepare the log directory for the swtpm and adjust ownership of it and the + * log file we will be using. Prepare the state directory where we will share + * the socket between tss and qemu users. + */ +static int +qemuTPMEmulatorPrepareHost(virDomainTPMDef *tpm, + const char *logDir, + uid_t swtpm_user, + gid_t swtpm_group, + const char *swtpmStateDir, + uid_t qemu_user, + const char *shortName) +{ + /* create log dir ... allow 'tss' user to cd into it */ + if (g_mkdir_with_parents(logDir, 0711) < 0) + return -1; + + /* ... and adjust ownership */ + if (virDirCreate(logDir, 0730, swtpm_user, swtpm_group, + VIR_DIR_CREATE_ALLOW_EXIST) < 0) + return -1; + + if (!virFileExists(tpm->data.emulator.logfile) && + virFileTouch(tpm->data.emulator.logfile, 0644) < 0) { + return -1; + } + + /* ... and make sure it can be accessed by swtpm_user */ + if (chown(tpm->data.emulator.logfile, swtpm_user, swtpm_group) < 0) { + virReportSystemError(errno, + _("Could not chown on swtpm logfile %s"), + tpm->data.emulator.logfile); + return -1; + } + + /* + create our swtpm state dir ... + - QEMU user needs to be able to access the socket there + - swtpm group needs to be able to create files there + - in privileged mode 0570 would be enough, for non-privileged mode + we need 0770 + */ + if (virDirCreate(swtpmStateDir, 0770, qemu_user, swtpm_group, + VIR_DIR_CREATE_ALLOW_EXIST) < 0) + return -1; + + /* create the socket filename */ + if (!tpm->data.emulator.source->data.nix.path && + !(tpm->data.emulator.source->data.nix.path = + qemuTPMEmulatorSocketBuildPath(swtpmStateDir, shortName))) + return -1; + tpm->data.emulator.source->type = VIR_DOMAIN_CHR_TYPE_UNIX; + + return 0; +} + + /* * qemuTPMEmulatorStop * @swtpmStateDir: A directory where the socket is located -- 2.34.1
On a Friday in 2022, Andrea Bolognani wrote:
Clean up inconsistencies to make navigating the module easier.
Andrea Bolognani (14): qemu: Rename qemuExtTPMStartEmulator() qemu: Fully document qemuTPMEmulatorStart() qemu: Rename qemuTPM{Create,Delete}EmulatorStorage() qemu: Document qemuTPMEmulatorDeleteStorage() qemu: Drop qemuTPMEmulatorInitStorage() qemu: Make qemuTPMEmulatorCreateStorage() take a virDomainTPMDef* qemu: Introduce qemuExtTPMEmulatorSetupCgroup() qemu: Introduce qemuTPMEmulatorCleanupHost() qemu: Rename path-building functions qemu: Call virDomainDefGetShortName() less frequently qemu: Fix description of swtpmStateDir qemu: Move utility functions close together qemu: Move entry points close together qemu: Move high-level actions close together
src/qemu/qemu_tpm.c | 606 +++++++++++++++++++++++--------------------- 1 file changed, 323 insertions(+), 283 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Andrea Bolognani -
Ján Tomko