[PATCH v2 1/2] qemu: Move code to add encryption options for swtpm_setup into function

Stefan Berger posted 2 patches 4 years, 3 months ago
There is a newer version of this series
[PATCH v2 1/2] qemu: Move code to add encryption options for swtpm_setup into function
Posted by Stefan Berger 4 years, 3 months ago
Move the code that adds encryption options for the swtpm_setup command
line into its own function.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/qemu/qemu_tpm.c | 55 +++++++++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 5a05273100..93cb04f49d 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -422,6 +422,42 @@ qemuTPMCreateConfigFiles(const char *swtpm_setup)
 }
 
 
+/*
+ * Add encryption parameters to swtpm_setup command line.
+ *
+ * @cmd: virCommand to add options to
+ * @swtpm_setup: swtpm_setup tool path
+ * @secretuuid: The secret's uuid; may be NULL
+ */
+static int
+qemuTPMVirCommandAddEncryption(virCommand *cmd,
+                               const char *swtpm_setup,
+                               const unsigned char *secretuuid)
+{
+    int pwdfile_fd;
+
+    if (!secretuuid)
+        return 0;
+
+    if (!virTPMSwtpmSetupCapsGet(
+            VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD)) {
+        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+            _("%s does not support passing a passphrase using a file "
+              "descriptor"), swtpm_setup);
+        return -1;
+    }
+    if ((pwdfile_fd = qemuTPMSetupEncryption(secretuuid, cmd)) < 0)
+        return -1;
+
+    virCommandAddArg(cmd, "--pwdfile-fd");
+    virCommandAddArgFormat(cmd, "%d", pwdfile_fd);
+    virCommandAddArgList(cmd, "--cipher", "aes-256-cbc", NULL);
+    virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+
+    return 0;
+}
+
+
 /*
  * qemuTPMEmulatorRunSetup
  *
@@ -495,23 +531,8 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
         break;
     }
 
-    if (secretuuid) {
-        if (!virTPMSwtpmSetupCapsGet(
-                VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD)) {
-            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
-                _("%s does not support passing a passphrase using a file "
-                  "descriptor"), swtpm_setup);
-            return -1;
-        }
-        if ((pwdfile_fd = qemuTPMSetupEncryption(secretuuid, cmd)) < 0)
-            return -1;
-
-        virCommandAddArg(cmd, "--pwdfile-fd");
-        virCommandAddArgFormat(cmd, "%d", pwdfile_fd);
-        virCommandAddArgList(cmd, "--cipher", "aes-256-cbc", NULL);
-        virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
-        pwdfile_fd = -1;
-    }
+    if (qemuTPMVirCommandAddEncryption(cmd, swtpm_setup, secretuuid) < 0)
+        return -1;
 
     if (!incomingMigration) {
         virCommandAddArgList(cmd,
-- 
2.31.1

Re: [PATCH v2 1/2] qemu: Move code to add encryption options for swtpm_setup into function
Posted by Michal Prívozník 4 years, 3 months ago
On 11/1/21 6:23 PM, Stefan Berger wrote:
> Move the code that adds encryption options for the swtpm_setup command
> line into its own function.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  src/qemu/qemu_tpm.c | 55 +++++++++++++++++++++++++++++++--------------
>  1 file changed, 38 insertions(+), 17 deletions(-)
> 
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 5a05273100..93cb04f49d 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -422,6 +422,42 @@ qemuTPMCreateConfigFiles(const char *swtpm_setup)
>  }
>  
>  
> +/*
> + * Add encryption parameters to swtpm_setup command line.
> + *
> + * @cmd: virCommand to add options to
> + * @swtpm_setup: swtpm_setup tool path
> + * @secretuuid: The secret's uuid; may be NULL
> + */
> +static int
> +qemuTPMVirCommandAddEncryption(virCommand *cmd,
> +                               const char *swtpm_setup,
> +                               const unsigned char *secretuuid)
> +{
> +    int pwdfile_fd;
> +
> +    if (!secretuuid)
> +        return 0;
> +
> +    if (!virTPMSwtpmSetupCapsGet(
> +            VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD)) {

We can take this opportunity and move this onto a single line.

> +        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> +            _("%s does not support passing a passphrase using a file "
> +              "descriptor"), swtpm_setup);
> +        return -1;
> +    }
> +    if ((pwdfile_fd = qemuTPMSetupEncryption(secretuuid, cmd)) < 0)
> +        return -1;
> +
> +    virCommandAddArg(cmd, "--pwdfile-fd");
> +    virCommandAddArgFormat(cmd, "%d", pwdfile_fd);
> +    virCommandAddArgList(cmd, "--cipher", "aes-256-cbc", NULL);
> +    virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +
> +    return 0;
> +}
> +
> +
>  /*
>   * qemuTPMEmulatorRunSetup
>   *
> @@ -495,23 +531,8 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>          break;
>      }
>  
> -    if (secretuuid) {
> -        if (!virTPMSwtpmSetupCapsGet(
> -                VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD)) {
> -            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> -                _("%s does not support passing a passphrase using a file "
> -                  "descriptor"), swtpm_setup);
> -            return -1;
> -        }
> -        if ((pwdfile_fd = qemuTPMSetupEncryption(secretuuid, cmd)) < 0)
> -            return -1;
> -
> -        virCommandAddArg(cmd, "--pwdfile-fd");
> -        virCommandAddArgFormat(cmd, "%d", pwdfile_fd);
> -        virCommandAddArgList(cmd, "--cipher", "aes-256-cbc", NULL);
> -        virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> -        pwdfile_fd = -1;

This variable is no longer needed inside this function. Its declaration
can be removed too. Yeah, gcc doesn't warn about unused variable because
it's VIR_AUTOCLOSE(). I don't know about clang.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed because this patch makes sense regardless of 2/2.

Michal

Re: [PATCH v2 1/2] qemu: Move code to add encryption options for swtpm_setup into function
Posted by Marc-André Lureau 4 years, 3 months ago
On Mon, Nov 1, 2021 at 9:23 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
>
> Move the code that adds encryption options for the swtpm_setup command
> line into its own function.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  src/qemu/qemu_tpm.c | 55 +++++++++++++++++++++++++++++++--------------
>  1 file changed, 38 insertions(+), 17 deletions(-)
>
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 5a05273100..93cb04f49d 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -422,6 +422,42 @@ qemuTPMCreateConfigFiles(const char *swtpm_setup)
>  }
>
>
> +/*
> + * Add encryption parameters to swtpm_setup command line.
> + *
> + * @cmd: virCommand to add options to
> + * @swtpm_setup: swtpm_setup tool path
> + * @secretuuid: The secret's uuid; may be NULL
> + */
> +static int
> +qemuTPMVirCommandAddEncryption(virCommand *cmd,
> +                               const char *swtpm_setup,
> +                               const unsigned char *secretuuid)
> +{
> +    int pwdfile_fd;
> +
> +    if (!secretuuid)
> +        return 0;
> +
> +    if (!virTPMSwtpmSetupCapsGet(
> +            VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD)) {
> +        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> +            _("%s does not support passing a passphrase using a file "
> +              "descriptor"), swtpm_setup);
> +        return -1;
> +    }
> +    if ((pwdfile_fd = qemuTPMSetupEncryption(secretuuid, cmd)) < 0)
> +        return -1;
> +
> +    virCommandAddArg(cmd, "--pwdfile-fd");
> +    virCommandAddArgFormat(cmd, "%d", pwdfile_fd);
> +    virCommandAddArgList(cmd, "--cipher", "aes-256-cbc", NULL);
> +    virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +
> +    return 0;
> +}
> +
> +
>  /*
>   * qemuTPMEmulatorRunSetup
>   *
> @@ -495,23 +531,8 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>          break;
>      }
>
> -    if (secretuuid) {
> -        if (!virTPMSwtpmSetupCapsGet(
> -                VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD)) {
> -            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> -                _("%s does not support passing a passphrase using a file "
> -                  "descriptor"), swtpm_setup);
> -            return -1;
> -        }
> -        if ((pwdfile_fd = qemuTPMSetupEncryption(secretuuid, cmd)) < 0)
> -            return -1;
> -
> -        virCommandAddArg(cmd, "--pwdfile-fd");
> -        virCommandAddArgFormat(cmd, "%d", pwdfile_fd);
> -        virCommandAddArgList(cmd, "--cipher", "aes-256-cbc", NULL);
> -        virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> -        pwdfile_fd = -1;
> -    }
> +    if (qemuTPMVirCommandAddEncryption(cmd, swtpm_setup, secretuuid) < 0)
> +        return -1;
>
>      if (!incomingMigration) {
>          virCommandAddArgList(cmd,
> --
> 2.31.1
>