On Thu, Sep 26, 2024 at 11:38 PM Stefan Berger <stefanb@xxxxxxxxxxxxx> wrote:
>
> To avoid passing TPM emulator parameters around individually, move them
> into a structure and pass around the structure.
>
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
Reviewed-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> ---
> src/conf/domain_conf.h | 24 ++++++++++++----------
> src/conf/virconftypes.h | 2 ++
> src/qemu/qemu_tpm.c | 45 ++++++++++++++++-------------------------
> 3 files changed, 32 insertions(+), 39 deletions(-)
>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index a15af4fae3..e5aee3c2cf 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1465,6 +1465,18 @@ typedef enum {
>
> #define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"
>
> +struct _virDomainTPMEmulatorDef {
> + virDomainTPMVersion version;
> + virDomainChrSourceDef *source;
> + char *storagepath;
> + char *logfile;
> + unsigned int debug;
> + unsigned char secretuuid[VIR_UUID_BUFLEN];
> + bool hassecretuuid;
> + bool persistent_state;
> + virBitmap *activePcrBanks;
> +};
> +
> struct _virDomainTPMDef {
> virObject *privateData;
>
> @@ -1475,17 +1487,7 @@ struct _virDomainTPMDef {
> struct {
> virDomainChrSourceDef *source;
> } passthrough;
> - struct {
> - virDomainTPMVersion version;
> - virDomainChrSourceDef *source;
> - char *storagepath;
> - char *logfile;
> - unsigned int debug;
> - unsigned char secretuuid[VIR_UUID_BUFLEN];
> - bool hassecretuuid;
> - bool persistent_state;
> - virBitmap *activePcrBanks;
> - } emulator;
> + virDomainTPMEmulatorDef emulator;
> struct {
> virDomainChrSourceDef *source;
> } external;
> diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h
> index f18ebcca10..59be61cea4 100644
> --- a/src/conf/virconftypes.h
> +++ b/src/conf/virconftypes.h
> @@ -234,6 +234,8 @@ typedef struct _virDomainAudioDef virDomainAudioDef;
>
> typedef struct _virDomainTPMDef virDomainTPMDef;
>
> +typedef struct _virDomainTPMEmulatorDef virDomainTPMEmulatorDef;
> +
> typedef struct _virDomainThreadSchedParam virDomainThreadSchedParam;
>
> typedef struct _virDomainTimerCatchupDef virDomainTimerCatchupDef;
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 2f17918cbb..592fcc62fa 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -350,10 +350,8 @@ qemuTPMVirCommandAddEncryption(virCommand *cmd,
> * @swtpm_user: The userid to switch to when setting up the TPM;
> * typically this should be the uid of 'tss' or 'root'
> * @swtpm_group: The group id to switch to
> - * @logfile: The file to write the log into; it must be writable
> - * for the user given by userid or 'tss'
> - * @tpmversion: The version of the TPM, either a TPM 1.2 or TPM 2
> - * @encryption: pointer to virStorageEncryption holding secret
> + * @emulator: emulator parameters
> + * @secretuuid: UUID describing virStorageEncryption holding secret
> * @incomingMigration: whether we have an incoming migration
> *
> * Setup the external swtpm by creating endorsement key and
> @@ -366,8 +364,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
> bool privileged,
> uid_t swtpm_user,
> gid_t swtpm_group,
> - const char *logfile,
> - const virDomainTPMVersion tpmversion,
> + const virDomainTPMEmulatorDef *emulator,
> const unsigned char *secretuuid,
> bool incomingMigration)
> {
> @@ -380,9 +377,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
> if (!swtpm_setup)
> return -1;
>
> - if (!privileged && tpmversion == VIR_DOMAIN_TPM_VERSION_1_2 &&
> + if (!privileged && emulator->version == VIR_DOMAIN_TPM_VERSION_1_2 &&
> !virTPMSwtpmSetupCapsGet(VIR_TPM_SWTPM_SETUP_FEATURE_TPM12_NOT_NEED_ROOT)) {
> - return virFileWriteStr(logfile,
> + return virFileWriteStr(emulator->logfile,
> _("Did not create EK and certificates since this requires privileged mode for a TPM 1.2\n"), 0600);
> }
>
> @@ -397,7 +394,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
> virCommandSetUID(cmd, swtpm_user);
> virCommandSetGID(cmd, swtpm_group);
>
> - switch (tpmversion) {
> + switch (emulator->version) {
> case VIR_DOMAIN_TPM_VERSION_1_2:
> break;
> case VIR_DOMAIN_TPM_VERSION_2_0:
> @@ -415,7 +412,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
> virCommandAddArgList(cmd,
> "--tpm-state", storagepath,
> "--vmid", vmid,
> - "--logfile", logfile,
> + "--logfile", emulator->logfile,
> "--createek",
> "--create-ek-cert",
> "--create-platform-cert",
> @@ -425,7 +422,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
> } else {
> virCommandAddArgList(cmd,
> "--tpm-state", storagepath,
> - "--logfile", logfile,
> + "--logfile", emulator->logfile,
> "--overwrite",
> NULL);
> }
> @@ -435,7 +432,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
> if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Could not run '%1$s'. exitstatus: %2$d; Check error log '%3$s' for details."),
> - swtpm_setup, exitstatus, logfile);
> + swtpm_setup, exitstatus, emulator->logfile);
> return -1;
> }
>
> @@ -469,10 +466,7 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks)
> * @swtpm_user: The userid to switch to when setting up the TPM;
> * typically this should be the uid of 'tss' or 'root'
> * @swtpm_group: The group id to switch to
> - * @activePcrBanks: The string describing the active PCR banks
> - * @logfile: The file to write the log into; it must be writable
> - * for the user given by userid or 'tss'
> - * @tpmversion: The version of the TPM, either a TPM 1.2 or TPM 2
> + * @emulator: emulator parameters
> * @secretuuid: The secret's UUID needed for state encryption
> *
> * Reconfigure the active PCR banks of a TPM 2.
> @@ -481,9 +475,7 @@ static int
> qemuTPMEmulatorReconfigure(const char *storagepath,
> uid_t swtpm_user,
> gid_t swtpm_group,
> - virBitmap *activePcrBanks,
> - const char *logfile,
> - const virDomainTPMVersion tpmversion,
> + const virDomainTPMEmulatorDef *emulator,
> const unsigned char *secretuuid)
> {
> g_autoptr(virCommand) cmd = NULL;
> @@ -494,8 +486,8 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
> if (!swtpm_setup)
> return -1;
>
> - if (tpmversion != VIR_DOMAIN_TPM_VERSION_2_0 ||
> - (activePcrBanksStr = qemuTPMPcrBankBitmapToStr(activePcrBanks)) == NULL ||
> + if (emulator->version != VIR_DOMAIN_TPM_VERSION_2_0 ||
> + (activePcrBanksStr = qemuTPMPcrBankBitmapToStr(emulator->activePcrBanks)) == NULL ||
> !virTPMSwtpmSetupCapsGet(VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS))
> return 0;
>
> @@ -511,7 +503,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
>
> virCommandAddArgList(cmd,
> "--tpm-state", storagepath,
> - "--logfile", logfile,
> + "--logfile", emulator->logfile,
> "--pcr-banks", activePcrBanksStr,
> "--reconfigure",
> NULL);
> @@ -521,7 +513,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
> if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Could not run '%1$s --reconfigure'. exitstatus: %2$d; Check error log '%3$s' for details."),
> - swtpm_setup, exitstatus, logfile);
> + swtpm_setup, exitstatus, emulator->logfile);
> return -1;
> }
>
> @@ -582,17 +574,14 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> if (created &&
> qemuTPMEmulatorRunSetup(tpm->data.emulator.storagepath, vmname, vmuuid,
> privileged, swtpm_user, swtpm_group,
> - tpm->data.emulator.logfile,
> - tpm->data.emulator.version,
> + &tpm->data.emulator,
> secretuuid, incomingMigration) < 0)
> goto error;
>
> if (!incomingMigration &&
> qemuTPMEmulatorReconfigure(tpm->data.emulator.storagepath,
> swtpm_user, swtpm_group,
> - tpm->data.emulator.activePcrBanks,
> - tpm->data.emulator.logfile,
> - tpm->data.emulator.version,
> + &tpm->data.emulator,
> secretuuid) < 0)
> goto error;
>
> --
> 2.46.1
>
