[PATCH V5 06/25] oslib: qemu_clr_cloexec

Steve Sistare posted 25 patches 4 years, 7 months ago
Maintainers: Juan Quintela <quintela@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, Stefan Hajnoczi <stefanha@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Alex Williamson <alex.williamson@redhat.com>, Eric Blake <eblake@redhat.com>, Mark Kanda <mark.kanda@oracle.com>, Steve Sistare <steven.sistare@oracle.com>, "Alex Bennée" <alex.bennee@linaro.org>, Cornelia Huck <cohuck@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Stefan Weil <sw@weilnetz.de>, Eduardo Habkost <ehabkost@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
[PATCH V5 06/25] oslib: qemu_clr_cloexec
Posted by Steve Sistare 4 years, 7 months ago
Define qemu_clr_cloexec, analogous to qemu_set_cloexec.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 include/qemu/osdep.h | 1 +
 util/oslib-posix.c   | 9 +++++++++
 util/oslib-win32.c   | 4 ++++
 3 files changed, 14 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index c91a78b..3d6a6ca 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -637,6 +637,7 @@ static inline void qemu_timersub(const struct timeval *val1,
 #endif
 
 void qemu_set_cloexec(int fd);
+void qemu_clr_cloexec(int fd);
 
 /* Starting on QEMU 2.5, qemu_hw_version() returns "2.5+" by default
  * instead of QEMU_VERSION, so setting hw_version on MachineClass
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e8bdb02..97577f1 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -309,6 +309,15 @@ void qemu_set_cloexec(int fd)
     assert(f != -1);
 }
 
+void qemu_clr_cloexec(int fd)
+{
+    int f;
+    f = fcntl(fd, F_GETFD);
+    assert(f != -1);
+    f = fcntl(fd, F_SETFD, f & ~FD_CLOEXEC);
+    assert(f != -1);
+}
+
 /*
  * Creates a pipe with FD_CLOEXEC set on both file descriptors
  */
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index af559ef..46e94d9 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -265,6 +265,10 @@ void qemu_set_cloexec(int fd)
 {
 }
 
+void qemu_clr_cloexec(int fd)
+{
+}
+
 /* Offset between 1/1/1601 and 1/1/1970 in 100 nanosec units */
 #define _W32_FT_OFFSET (116444736000000000ULL)
 
-- 
1.8.3.1


Re: [PATCH V5 06/25] oslib: qemu_clr_cloexec
Posted by Marc-André Lureau 4 years, 7 months ago
Hi

On Wed, Jul 7, 2021 at 9:33 PM Steve Sistare <steven.sistare@oracle.com>
wrote:

> Define qemu_clr_cloexec, analogous to qemu_set_cloexec.
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  include/qemu/osdep.h | 1 +
>  util/oslib-posix.c   | 9 +++++++++
>  util/oslib-win32.c   | 4 ++++
>  3 files changed, 14 insertions(+)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index c91a78b..3d6a6ca 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -637,6 +637,7 @@ static inline void qemu_timersub(const struct timeval
> *val1,
>  #endif
>
>  void qemu_set_cloexec(int fd);
> +void qemu_clr_cloexec(int fd);
>

I wish we would have a single function to set or unset, tbh. (as _clr_
isn't as readable to me)

 /* Starting on QEMU 2.5, qemu_hw_version() returns "2.5+" by default
>   * instead of QEMU_VERSION, so setting hw_version on MachineClass
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index e8bdb02..97577f1 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -309,6 +309,15 @@ void qemu_set_cloexec(int fd)
>      assert(f != -1);
>  }
>
> +void qemu_clr_cloexec(int fd)
> +{
> +    int f;
> +    f = fcntl(fd, F_GETFD);
> +    assert(f != -1);
> +    f = fcntl(fd, F_SETFD, f & ~FD_CLOEXEC);
> +    assert(f != -1);
> +}
>

(asserting() may not be very judicious for calls that we intend to make
during running time, but that's the way it is so far)

+
>  /*
>   * Creates a pipe with FD_CLOEXEC set on both file descriptors
>   */
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index af559ef..46e94d9 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -265,6 +265,10 @@ void qemu_set_cloexec(int fd)
>  {
>  }
>
> +void qemu_clr_cloexec(int fd)
> +{
> +}
> +
>  /* Offset between 1/1/1601 and 1/1/1970 in 100 nanosec units */
>  #define _W32_FT_OFFSET (116444736000000000ULL)
>
> --
> 1.8.3.1
>
>
>

-- 
Marc-André Lureau
Re: [PATCH V5 06/25] oslib: qemu_clr_cloexec
Posted by Steven Sistare 4 years, 7 months ago
On 7/8/2021 9:58 AM, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jul 7, 2021 at 9:33 PM Steve Sistare <steven.sistare@oracle.com <mailto:steven.sistare@oracle.com>> wrote:
> 
>     Define qemu_clr_cloexec, analogous to qemu_set_cloexec.
> 
>     Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com <mailto:dgilbert@redhat.com>>
>     Signed-off-by: Steve Sistare <steven.sistare@oracle.com <mailto:steven.sistare@oracle.com>>
>     ---
>      include/qemu/osdep.h | 1 +
>      util/oslib-posix.c   | 9 +++++++++
>      util/oslib-win32.c   | 4 ++++
>      3 files changed, 14 insertions(+)
> 
>     diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>     index c91a78b..3d6a6ca 100644
>     --- a/include/qemu/osdep.h
>     +++ b/include/qemu/osdep.h
>     @@ -637,6 +637,7 @@ static inline void qemu_timersub(const struct timeval *val1,
>      #endif
> 
>      void qemu_set_cloexec(int fd);
>     +void qemu_clr_cloexec(int fd);
> 
> 
> I wish we would have a single function to set or unset, tbh. (as _clr_ isn't as readable to me)

I would rather not replace the existing qemu_set_cloexec calls, but I will expand clr to clear.

>      /* Starting on QEMU 2.5, qemu_hw_version() returns "2.5+" by default
>       * instead of QEMU_VERSION, so setting hw_version on MachineClass
>     diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>     index e8bdb02..97577f1 100644
>     --- a/util/oslib-posix.c
>     +++ b/util/oslib-posix.c
>     @@ -309,6 +309,15 @@ void qemu_set_cloexec(int fd)
>          assert(f != -1);
>      }
> 
>     +void qemu_clr_cloexec(int fd)
>     +{
>     +    int f;
>     +    f = fcntl(fd, F_GETFD);
>     +    assert(f != -1);
>     +    f = fcntl(fd, F_SETFD, f & ~FD_CLOEXEC);
>     +    assert(f != -1);
>     +}
> 
> 
> (asserting() may not be very judicious for calls that we intend to make during running time, but that's the way it is so far)

yep.

- Steve

>     +
>      /*
>       * Creates a pipe with FD_CLOEXEC set on both file descriptors
>       */
>     diff --git a/util/oslib-win32.c b/util/oslib-win32.c
>     index af559ef..46e94d9 100644
>     --- a/util/oslib-win32.c
>     +++ b/util/oslib-win32.c
>     @@ -265,6 +265,10 @@ void qemu_set_cloexec(int fd)
>      {
>      }
> 
>     +void qemu_clr_cloexec(int fd)
>     +{
>     +}
>     +
>      /* Offset between 1/1/1601 and 1/1/1970 in 100 nanosec units */
>      #define _W32_FT_OFFSET (116444736000000000ULL)
> 
>     -- 
>     1.8.3.1
> 
> 
> 
> 
> -- 
> Marc-André Lureau