qemu: Get rid of QEMU_CAPS_ADD_FD (and improve block-commit probing)

[libvirt] [PATCH v2 07/10] qemu: command: Extract opening of TPM backend FDs for mocking purposes

Posted by Peter Krempa 4 years, 9 months ago

Allow mocking of the file descriptor numbers used for the TPM
passthrough mode by extracting the relevant code into an exported
function.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_command.c | 41 +++++++++++++++++++++++++++--------------
 src/qemu/qemu_command.h |  7 +++++++
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ddb90895e0..fa66b8affb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9586,6 +9586,31 @@ qemuBuildTPMDevStr(const virDomainDef *def,
 }


+/* this function is exported so that tests can mock the FDs */
+int
+qemuBuildTPMOpenBackendFDs(const char *tpmdev,
+                           const char *cancel_path,
+                           int *tpmfd,
+                           int *cancelfd)
+{
+    if ((*tpmfd = open(tpmdev, O_RDWR)) < 0) {
+        virReportSystemError(errno, _("Could not open TPM device %s"),
+                             tpmdev);
+        return -1;
+    }
+
+    if ((*cancelfd = open(cancel_path, O_WRONLY)) < 0) {
+        virReportSystemError(errno,
+                             _("Could not open TPM device's cancel "
+                               "path %s"), cancel_path);
+        VIR_FORCE_CLOSE(*tpmfd);
+        return -1;
+    }
+
+    return 0;
+}
+
+
 static char *
 qemuBuildTPMBackendStr(const virDomainDef *def,
                        virCommandPtr cmd,
@@ -9624,12 +9649,8 @@ qemuBuildTPMBackendStr(const virDomainDef *def,
             goto error;

         if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ADD_FD)) {
-            *tpmfd = open(tpmdev, O_RDWR);
-            if (*tpmfd < 0) {
-                virReportSystemError(errno, _("Could not open TPM device %s"),
-                                     tpmdev);
+            if (qemuBuildTPMOpenBackendFDs(tpmdev, cancel_path, tpmfd, cancelfd) < 0)
                 goto error;
-            }

             virCommandPassFD(cmd, *tpmfd,
                              VIR_COMMAND_PASS_FD_CLOSE_PARENT);
@@ -9637,17 +9658,9 @@ qemuBuildTPMBackendStr(const virDomainDef *def,
             if (devset == NULL)
                 goto error;

-            *cancelfd = open(cancel_path, O_WRONLY);
-            if (*cancelfd < 0) {
-                virReportSystemError(errno,
-                                     _("Could not open TPM device's cancel "
-                                       "path %s"), cancel_path);
-                goto error;
-            }
-            VIR_FREE(cancel_path);
-
             virCommandPassFD(cmd, *cancelfd,
                              VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+            VIR_FREE(cancel_path);
             cancel_path = qemuVirCommandGetDevSet(cmd, *cancelfd);
             if (cancel_path == NULL)
                 goto error;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 283bf3120d..7f84f904ce 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -216,4 +216,11 @@ qemuBuildVsockDevStr(virDomainDefPtr def,
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
     ATTRIBUTE_NONNULL(4);

+/* this function is exported so that tests can mock the FDs */
+int
+qemuBuildTPMOpenBackendFDs(const char *tpmdev,
+                           const char *cancel_path,
+                           int *tpmfd,
+                           int *cancelfd);
+
 #endif /* __QEMU_COMMAND_H__*/
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 07/10] qemu: command: Extract opening of TPM backend FDs for mocking purposes

Posted by John Ferlan 4 years, 9 months ago


On 08/15/2018 05:18 AM, Peter Krempa wrote:
> Allow mocking of the file descriptor numbers used for the TPM
> passthrough mode by extracting the relevant code into an exported
> function.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_command.c | 41 +++++++++++++++++++++++++++--------------
>  src/qemu/qemu_command.h |  7 +++++++
>  2 files changed, 34 insertions(+), 14 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index ddb90895e0..fa66b8affb 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9586,6 +9586,31 @@ qemuBuildTPMDevStr(const virDomainDef *def,
>  }
> 

[...]

>  static char *
>  qemuBuildTPMBackendStr(const virDomainDef *def,
>                         virCommandPtr cmd,
> @@ -9624,12 +9649,8 @@ qemuBuildTPMBackendStr(const virDomainDef *def,
>              goto error;
> 
>          if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ADD_FD)) {
> -            *tpmfd = open(tpmdev, O_RDWR);
> -            if (*tpmfd < 0) {
> -                virReportSystemError(errno, _("Could not open TPM device %s"),
> -                                     tpmdev);
> +            if (qemuBuildTPMOpenBackendFDs(tpmdev, cancel_path, tpmfd, cancelfd) < 0)
>                  goto error;
> -            }
> 
>              virCommandPassFD(cmd, *tpmfd,
>                               VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> @@ -9637,17 +9658,9 @@ qemuBuildTPMBackendStr(const virDomainDef *def,
>              if (devset == NULL)

@cancelfd would be leaked...

>                  goto error;
> 

[...]

> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 283bf3120d..7f84f904ce 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -216,4 +216,11 @@ qemuBuildVsockDevStr(virDomainDefPtr def,
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
>      ATTRIBUTE_NONNULL(4);
> 
> +/* this function is exported so that tests can mock the FDs */
> +int
> +qemuBuildTPMOpenBackendFDs(const char *tpmdev,
> +                           const char *cancel_path,
> +                           int *tpmfd,
> +                           int *cancelfd);

should this follow others in here w/r/t ATTRIBUTE_NONNULL for args?

> +
>  #endif /* __QEMU_COMMAND_H__*/
> 

With at lest the leak resolved,

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list