The @tpm argument was not mentioned. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- 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

Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- 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

This matches how qemuTPMEmulatorDeleteStorage() expects to be called. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- 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 qemuExtTPMCleanupHost() to only deal with looping over TPM devices, same as other qemuExtTPMDoThing() functions. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- 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

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

Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- 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(a)redhat.com&gt; --- 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