All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] treewide: use qemu_dup() instead of dup()
@ 2026-10-15  1:34 Filip Hejsek
  2026-10-15  8:04 ` Daniel P. Berrangé
  2026-10-15 11:41 ` Marc-André Lureau
  0 siblings, 2 replies; 5+ messages in thread
From: Filip Hejsek @ 2026-10-15  1:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Daniel P. Berrangé, Paolo Bonzini,
	Marc-André Lureau, Michael Tokarev, Laurent Vivier,
	Eric Blake, Vladimir Sementsov-Ogievskiy, qemu-block,
	Maciej S. Szmigiero, Peter Xu, Fabiano Rosas, Mark Kanda,
	Ben Chaney, Igor Mammedov, Ani Sinha

Use qemu_dup() to ensure the cloexec flag is set on dup'd fds.

The only direct users of dup() are now guest syscalls in the user
emulators and the implementation of qemu_dup() itself.

Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
For each call I replaced, I checked how the resulting fd is used
afterwards. In summary:

hw/virtio/virtio.c:
    the fd is passed to memory_region_init_ram_from_fd, used for mmaping

io/channel-file.c:
    the fd is passed to qio_channel_file_new_fd, used for reading/writing

migration/cpr-exec.c:
    the cloexec flag is cleared

qemu-nbd.c:
    the fd is stored into old_stderr, then dup2'd back to STDERR_FILENO

system/vl.c:
    no change in behavior, the code was already setting the cloexec flag

ui/dmabuf.c:
    the function is called by qemu_spice_gl_update, which passes the fds
    to SPICE server

tests/qtest/bios-tables-test.c:
    temp dup of stdout, dup2'd back to STDOUT_FILENO

tests/unit/test-util-sockets.c:
    one fd is dup'd again and returned from mock monitor_get_fd; another
    is converted to string and given as address to socket_connect

tests/unit/test-vmstate.c:
    passed to qio_channel_file_new_fd to create a channel

Changes in v3:
- Also replace the use in migration/cpr-exec.c and the tests
- Link to v2: https://lore.kernel.org/qemu-devel/20261013-use-qemu-dup-v2-1-12ecd94263b7@gmail.com

Changes in v2:
- Replace the use in qemu-nbd.c too
- Changed commit message
- Link to v1: https://lore.kernel.org/qemu-devel/20261012-use-qemu-dup-v1-1-03d94852a6c8@gmail.com
---
 hw/virtio/virtio.c             | 2 +-
 io/channel-file.c              | 2 +-
 migration/cpr-exec.c           | 2 +-
 qemu-nbd.c                     | 2 +-
 system/vl.c                    | 9 +--------
 tests/qtest/bios-tables-test.c | 2 +-
 tests/unit/test-util-sockets.c | 6 +++---
 tests/unit/test-vmstate.c      | 2 +-
 ui/dmabuf.c                    | 2 +-
 9 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index c9f9e5b511..70779fbd56 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3204,7 +3204,7 @@ VirtioSharedMemoryMapping *virtio_shared_memory_mapping_new(uint8_t shmid,
         return NULL;
     }
 
-    fd = dup(fd);
+    fd = qemu_dup(fd);
     if (fd < 0) {
         error_report("Failed to duplicate fd: %s", strerror(errno));
         return NULL;
diff --git a/io/channel-file.c b/io/channel-file.c
index 5cef75a67c..3f707e80c9 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -48,7 +48,7 @@ qio_channel_file_new_fd(int fd)
 QIOChannelFile *
 qio_channel_file_new_dupfd(int fd, Error **errp)
 {
-    int newfd = dup(fd);
+    int newfd = qemu_dup(fd);
 
     if (newfd < 0) {
         error_setg_errno(errp, errno, "Could not dup FD %d", fd);
diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
index daa50916d2..2f38db30bb 100644
--- a/migration/cpr-exec.c
+++ b/migration/cpr-exec.c
@@ -44,7 +44,7 @@ bool cpr_exec_persist_state(QEMUFile *f, Error **errp)
 {
     QIOChannelFile *fioc = QIO_CHANNEL_FILE(qemu_file_get_ioc(f));
     /* coverity[leaked_storage] - mfd intentionally kept open across exec() */
-    int mfd = dup(fioc->fd);
+    int mfd = qemu_dup(fioc->fd);
     char val[16];
 
     /* Remember mfd in environment for post-exec load */
diff --git a/qemu-nbd.c b/qemu-nbd.c
index ed5895861b..db1a70b008 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -975,7 +975,7 @@ int main(int argc, char **argv)
 
             /* Remember parent's stderr if we will be restoring it. */
             if (opts.verbose /* fork_process is set */) {
-                opts.old_stderr = dup(STDERR_FILENO);
+                opts.old_stderr = qemu_dup(STDERR_FILENO);
                 if (opts.old_stderr < 0) {
                     error_report("Could not dup original stderr: %s",
                                  strerror(errno));
diff --git a/system/vl.c b/system/vl.c
index 468a9fc247..9381e3d7f7 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -614,14 +614,7 @@ static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
         return -1;
     }
 
-#ifdef F_DUPFD_CLOEXEC
-    dupfd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
-#else
-    dupfd = dup(fd);
-    if (dupfd != -1) {
-        qemu_set_cloexec(dupfd);
-    }
-#endif
+    dupfd = qemu_dup(fd);
     if (dupfd == -1) {
         error_setg_errno(errp, errno, "error duplicating fd");
         return -1;
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index f6dfa4131b..c4838322fb 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -529,7 +529,7 @@ static void test_acpi_asl(test_data *data)
                     const char *diff_cmd = diff_env ? diff_env : "diff -U 16";
                     char *diff = g_strdup_printf("%s %s %s", diff_cmd,
                                                  exp_sdt->asl_file, sdt->asl_file);
-                    int out = dup(STDOUT_FILENO);
+                    int out = qemu_dup(STDOUT_FILENO);
                     int ret G_GNUC_UNUSED;
                     int dupret;
 
diff --git a/tests/unit/test-util-sockets.c b/tests/unit/test-util-sockets.c
index 006f5e579c..c47ad6c82d 100644
--- a/tests/unit/test-util-sockets.c
+++ b/tests/unit/test-util-sockets.c
@@ -62,7 +62,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
         error_setg(errp, "No fd named %s", fdname);
         return -1;
     }
-    return dup(mon_fd);
+    return qemu_dup(mon_fd);
 }
 
 /*
@@ -115,7 +115,7 @@ static void test_socket_fd_pass_name_bad(void)
 
     cur_mon = g_malloc(1); /* Fake a monitor */
     mon_fdname = "myfd";
-    mon_fd = dup(STDOUT_FILENO);
+    mon_fd = qemu_dup(STDOUT_FILENO);
     g_assert_cmpint(mon_fd, >, STDERR_FILENO);
 
     addr.type = SOCKET_ADDRESS_TYPE_FD;
@@ -189,7 +189,7 @@ static void test_socket_fd_pass_num_bad(void)
     int fd, sfd;
 
     g_assert(cur_mon == NULL);
-    sfd = dup(STDOUT_FILENO);
+    sfd = qemu_dup(STDOUT_FILENO);
 
     addr.type = SOCKET_ADDRESS_TYPE_FD;
     addr.u.fd.str = g_strdup_printf("%d", sfd);
diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
index 23f3173070..57f1e3263f 100644
--- a/tests/unit/test-vmstate.c
+++ b/tests/unit/test-vmstate.c
@@ -42,7 +42,7 @@ static QEMUFile *open_test_file(bool write)
     QIOChannel *ioc;
     QEMUFile *f;
 
-    fd = dup(temp_fd);
+    fd = qemu_dup(temp_fd);
     g_assert(fd >= 0);
     lseek(fd, 0, SEEK_SET);
     if (write) {
diff --git a/ui/dmabuf.c b/ui/dmabuf.c
index 497f82cc61..6d5dffdf84 100644
--- a/ui/dmabuf.c
+++ b/ui/dmabuf.c
@@ -95,7 +95,7 @@ void qemu_dmabuf_dup_fds(QemuDmaBuf *dmabuf, int *fds, int nfds)
     assert(nfds >= dmabuf->num_planes);
 
     for (i = 0; i < dmabuf->num_planes; i++) {
-        fds[i] = dmabuf->fd[i] >= 0 ? dup(dmabuf->fd[i]) : -1;
+        fds[i] = dmabuf->fd[i] >= 0 ? qemu_dup(dmabuf->fd[i]) : -1;
     }
 }
 

---
base-commit: 615ece3c406b262996e569f29ea6eed0d81c4d8d
change-id: 20261012-use-qemu-dup-38628dbac3ee

Best regards,
--  
Filip Hejsek <filip.hejsek@gmail.com>



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] treewide: use qemu_dup() instead of dup()
  2026-10-15  1:34 [PATCH v3] treewide: use qemu_dup() instead of dup() Filip Hejsek
@ 2026-10-15  8:04 ` Daniel P. Berrangé
  2026-10-15 10:09   ` Filip Hejsek
  2026-10-15 11:41 ` Marc-André Lureau
  1 sibling, 1 reply; 5+ messages in thread
From: Daniel P. Berrangé @ 2026-10-15  8:04 UTC (permalink / raw)
  To: Filip Hejsek
  Cc: qemu-devel, Michael S. Tsirkin, Paolo Bonzini,
	Marc-André Lureau, Michael Tokarev, Laurent Vivier,
	Eric Blake, Vladimir Sementsov-Ogievskiy, qemu-block,
	Maciej S. Szmigiero, Peter Xu, Fabiano Rosas, Mark Kanda,
	Ben Chaney, Igor Mammedov, Ani Sinha

On Thu, Oct 15, 2026 at 03:34:43AM +0200, Filip Hejsek wrote:
> Use qemu_dup() to ensure the cloexec flag is set on dup'd fds.
> 
> The only direct users of dup() are now guest syscalls in the user
> emulators and the implementation of qemu_dup() itself.
> 
> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> ---
> For each call I replaced, I checked how the resulting fd is used
> afterwards. In summary:
> 
> hw/virtio/virtio.c:
>     the fd is passed to memory_region_init_ram_from_fd, used for mmaping
> 
> io/channel-file.c:
>     the fd is passed to qio_channel_file_new_fd, used for reading/writing
> 
> migration/cpr-exec.c:
>     the cloexec flag is cleared
> 
> qemu-nbd.c:
>     the fd is stored into old_stderr, then dup2'd back to STDERR_FILENO
> 
> system/vl.c:
>     no change in behavior, the code was already setting the cloexec flag
> 
> ui/dmabuf.c:
>     the function is called by qemu_spice_gl_update, which passes the fds
>     to SPICE server
> 
> tests/qtest/bios-tables-test.c:
>     temp dup of stdout, dup2'd back to STDOUT_FILENO
> 
> tests/unit/test-util-sockets.c:
>     one fd is dup'd again and returned from mock monitor_get_fd; another
>     is converted to string and given as address to socket_connect
> 
> tests/unit/test-vmstate.c:
>     passed to qio_channel_file_new_fd to create a channel
> 
> Changes in v3:
> - Also replace the use in migration/cpr-exec.c and the tests
> - Link to v2: https://lore.kernel.org/qemu-devel/20261013-use-qemu-dup-v2-1-12ecd94263b7@gmail.com
> 
> Changes in v2:
> - Replace the use in qemu-nbd.c too
> - Changed commit message
> - Link to v1: https://lore.kernel.org/qemu-devel/20261012-use-qemu-dup-v1-1-03d94852a6c8@gmail.com
> ---
>  hw/virtio/virtio.c             | 2 +-
>  io/channel-file.c              | 2 +-
>  migration/cpr-exec.c           | 2 +-
>  qemu-nbd.c                     | 2 +-
>  system/vl.c                    | 9 +--------
>  tests/qtest/bios-tables-test.c | 2 +-
>  tests/unit/test-util-sockets.c | 6 +++---
>  tests/unit/test-vmstate.c      | 2 +-
>  ui/dmabuf.c                    | 2 +-
>  9 files changed, 11 insertions(+), 18 deletions(-)


> diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
> index daa50916d2..2f38db30bb 100644
> --- a/migration/cpr-exec.c
> +++ b/migration/cpr-exec.c
> @@ -44,7 +44,7 @@ bool cpr_exec_persist_state(QEMUFile *f, Error **errp)
>  {
>      QIOChannelFile *fioc = QIO_CHANNEL_FILE(qemu_file_get_ioc(f));
>      /* coverity[leaked_storage] - mfd intentionally kept open across exec() */
> -    int mfd = dup(fioc->fd);
> +    int mfd = qemu_dup(fioc->fd);
>      char val[16];

This change needed to be:

   int mfd = qemu_dup(fioc->fd);
   qemu_clear_cloexec(fioc->fd);   


If that is added:

 Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] treewide: use qemu_dup() instead of dup()
  2026-10-15  8:04 ` Daniel P. Berrangé
@ 2026-10-15 10:09   ` Filip Hejsek
  2026-10-15 10:10     ` Daniel P. Berrangé
  0 siblings, 1 reply; 5+ messages in thread
From: Filip Hejsek @ 2026-10-15 10:09 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: qemu-devel, Michael S. Tsirkin, Paolo Bonzini,
	Marc-André Lureau, Michael Tokarev, Laurent Vivier,
	Eric Blake, Vladimir Sementsov-Ogievskiy, qemu-block,
	Maciej S. Szmigiero, Peter Xu, Fabiano Rosas, Mark Kanda,
	Ben Chaney, Igor Mammedov, Ani Sinha

On Thu, 2026-10-15 at 09:04 +0100, Daniel P. Berrangé wrote:
> On Thu, Oct 15, 2026 at 03:34:43AM +0200, Filip Hejsek wrote:
> > Use qemu_dup() to ensure the cloexec flag is set on dup'd fds.
> > 
> > The only direct users of dup() are now guest syscalls in the user
> > emulators and the implementation of qemu_dup() itself.
> > 
> > Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> > ---
> > For each call I replaced, I checked how the resulting fd is used
> > afterwards. In summary:
> > 
> > hw/virtio/virtio.c:
> >     the fd is passed to memory_region_init_ram_from_fd, used for mmaping
> > 
> > io/channel-file.c:
> >     the fd is passed to qio_channel_file_new_fd, used for reading/writing
> > 
> > migration/cpr-exec.c:
> >     the cloexec flag is cleared
> > 
> > qemu-nbd.c:
> >     the fd is stored into old_stderr, then dup2'd back to STDERR_FILENO
> > 
> > system/vl.c:
> >     no change in behavior, the code was already setting the cloexec flag
> > 
> > ui/dmabuf.c:
> >     the function is called by qemu_spice_gl_update, which passes the fds
> >     to SPICE server
> > 
> > tests/qtest/bios-tables-test.c:
> >     temp dup of stdout, dup2'd back to STDOUT_FILENO
> > 
> > tests/unit/test-util-sockets.c:
> >     one fd is dup'd again and returned from mock monitor_get_fd; another
> >     is converted to string and given as address to socket_connect
> > 
> > tests/unit/test-vmstate.c:
> >     passed to qio_channel_file_new_fd to create a channel
> > 
> > Changes in v3:
> > - Also replace the use in migration/cpr-exec.c and the tests
> > - Link to v2: https://lore.kernel.org/qemu-devel/20261013-use-qemu-dup-v2-1-12ecd94263b7@gmail.com
> > 
> > Changes in v2:
> > - Replace the use in qemu-nbd.c too
> > - Changed commit message
> > - Link to v1: https://lore.kernel.org/qemu-devel/20261012-use-qemu-dup-v1-1-03d94852a6c8@gmail.com
> > ---
> >  hw/virtio/virtio.c             | 2 +-
> >  io/channel-file.c              | 2 +-
> >  migration/cpr-exec.c           | 2 +-
> >  qemu-nbd.c                     | 2 +-
> >  system/vl.c                    | 9 +--------
> >  tests/qtest/bios-tables-test.c | 2 +-
> >  tests/unit/test-util-sockets.c | 6 +++---
> >  tests/unit/test-vmstate.c      | 2 +-
> >  ui/dmabuf.c                    | 2 +-
> >  9 files changed, 11 insertions(+), 18 deletions(-)
> 
> 
> > diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
> > index daa50916d2..2f38db30bb 100644
> > --- a/migration/cpr-exec.c
> > +++ b/migration/cpr-exec.c
> > @@ -44,7 +44,7 @@ bool cpr_exec_persist_state(QEMUFile *f, Error **errp)
> >  {
> >      QIOChannelFile *fioc = QIO_CHANNEL_FILE(qemu_file_get_ioc(f));
> >      /* coverity[leaked_storage] - mfd intentionally kept open across exec() */
> > -    int mfd = dup(fioc->fd);
> > +    int mfd = qemu_dup(fioc->fd);
> >      char val[16];
> 
> This change needed to be:
> 
>    int mfd = qemu_dup(fioc->fd);
>    qemu_clear_cloexec(fioc->fd);   
> 

There was a call to qemu_clear_cloexec(mfd) already, even before the
change, just outside the visible hunk:

   bool cpr_exec_persist_state(QEMUFile *f, Error **errp)
   {
       QIOChannelFile *fioc = QIO_CHANNEL_FILE(qemu_file_get_ioc(f));
       /* coverity[leaked_storage] - mfd intentionally kept open across exec() */
       int mfd = qemu_dup(fioc->fd);
       char val[16];
   
       /* Remember mfd in environment for post-exec load */
       qemu_clear_cloexec(mfd);
       snprintf(val, sizeof(val), "%d", mfd);
       if (!g_setenv(CPR_EXEC_STATE_NAME, val, 1)) {
           error_setg(errp, "Setting env %s = %s failed", CPR_EXEC_STATE_NAME, val);
           return false;
       }
   
       return true;
   }

> 
> If that is added:
> 
>  Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>  
> 
> With regards,
> Daniel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] treewide: use qemu_dup() instead of dup()
  2026-10-15 10:09   ` Filip Hejsek
@ 2026-10-15 10:10     ` Daniel P. Berrangé
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel P. Berrangé @ 2026-10-15 10:10 UTC (permalink / raw)
  To: Filip Hejsek
  Cc: qemu-devel, Michael S. Tsirkin, Paolo Bonzini,
	Marc-André Lureau, Michael Tokarev, Laurent Vivier,
	Eric Blake, Vladimir Sementsov-Ogievskiy, qemu-block,
	Maciej S. Szmigiero, Peter Xu, Fabiano Rosas, Mark Kanda,
	Ben Chaney, Igor Mammedov, Ani Sinha

On Thu, Oct 15, 2026 at 12:09:22PM +0200, Filip Hejsek wrote:
> On Thu, 2026-10-15 at 09:04 +0100, Daniel P. Berrangé wrote:
> > On Thu, Oct 15, 2026 at 03:34:43AM +0200, Filip Hejsek wrote:
> > > Use qemu_dup() to ensure the cloexec flag is set on dup'd fds.
> > > 
> > > The only direct users of dup() are now guest syscalls in the user
> > > emulators and the implementation of qemu_dup() itself.
> > > 
> > > Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> > > ---
> > > For each call I replaced, I checked how the resulting fd is used
> > > afterwards. In summary:
> > > 
> > > hw/virtio/virtio.c:
> > >     the fd is passed to memory_region_init_ram_from_fd, used for mmaping
> > > 
> > > io/channel-file.c:
> > >     the fd is passed to qio_channel_file_new_fd, used for reading/writing
> > > 
> > > migration/cpr-exec.c:
> > >     the cloexec flag is cleared
> > > 
> > > qemu-nbd.c:
> > >     the fd is stored into old_stderr, then dup2'd back to STDERR_FILENO
> > > 
> > > system/vl.c:
> > >     no change in behavior, the code was already setting the cloexec flag
> > > 
> > > ui/dmabuf.c:
> > >     the function is called by qemu_spice_gl_update, which passes the fds
> > >     to SPICE server
> > > 
> > > tests/qtest/bios-tables-test.c:
> > >     temp dup of stdout, dup2'd back to STDOUT_FILENO
> > > 
> > > tests/unit/test-util-sockets.c:
> > >     one fd is dup'd again and returned from mock monitor_get_fd; another
> > >     is converted to string and given as address to socket_connect
> > > 
> > > tests/unit/test-vmstate.c:
> > >     passed to qio_channel_file_new_fd to create a channel
> > > 
> > > Changes in v3:
> > > - Also replace the use in migration/cpr-exec.c and the tests
> > > - Link to v2: https://lore.kernel.org/qemu-devel/20261013-use-qemu-dup-v2-1-12ecd94263b7@gmail.com
> > > 
> > > Changes in v2:
> > > - Replace the use in qemu-nbd.c too
> > > - Changed commit message
> > > - Link to v1: https://lore.kernel.org/qemu-devel/20261012-use-qemu-dup-v1-1-03d94852a6c8@gmail.com
> > > ---
> > >  hw/virtio/virtio.c             | 2 +-
> > >  io/channel-file.c              | 2 +-
> > >  migration/cpr-exec.c           | 2 +-
> > >  qemu-nbd.c                     | 2 +-
> > >  system/vl.c                    | 9 +--------
> > >  tests/qtest/bios-tables-test.c | 2 +-
> > >  tests/unit/test-util-sockets.c | 6 +++---
> > >  tests/unit/test-vmstate.c      | 2 +-
> > >  ui/dmabuf.c                    | 2 +-
> > >  9 files changed, 11 insertions(+), 18 deletions(-)
> > 
> > 
> > > diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
> > > index daa50916d2..2f38db30bb 100644
> > > --- a/migration/cpr-exec.c
> > > +++ b/migration/cpr-exec.c
> > > @@ -44,7 +44,7 @@ bool cpr_exec_persist_state(QEMUFile *f, Error **errp)
> > >  {
> > >      QIOChannelFile *fioc = QIO_CHANNEL_FILE(qemu_file_get_ioc(f));
> > >      /* coverity[leaked_storage] - mfd intentionally kept open across exec() */
> > > -    int mfd = dup(fioc->fd);
> > > +    int mfd = qemu_dup(fioc->fd);
> > >      char val[16];
> > 
> > This change needed to be:
> > 
> >    int mfd = qemu_dup(fioc->fd);
> >    qemu_clear_cloexec(fioc->fd);   
> > 
> 
> There was a call to qemu_clear_cloexec(mfd) already, even before the
> change, just outside the visible hunk:

Oppps, yes, thanks for pointing that out. In that case,

  Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>



With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] treewide: use qemu_dup() instead of dup()
  2026-10-15  1:34 [PATCH v3] treewide: use qemu_dup() instead of dup() Filip Hejsek
  2026-10-15  8:04 ` Daniel P. Berrangé
@ 2026-10-15 11:41 ` Marc-André Lureau
  1 sibling, 0 replies; 5+ messages in thread
From: Marc-André Lureau @ 2026-10-15 11:41 UTC (permalink / raw)
  To: Filip Hejsek
  Cc: qemu-devel, Michael S. Tsirkin, Daniel P. Berrangé,
	Paolo Bonzini, Michael Tokarev, Laurent Vivier, Eric Blake,
	Vladimir Sementsov-Ogievskiy, qemu-block, Maciej S. Szmigiero,
	Peter Xu, Fabiano Rosas, Mark Kanda, Ben Chaney, Igor Mammedov,
	Ani Sinha

On Thu, Oct 15, 2026 at 5:36 AM Filip Hejsek <filip.hejsek@gmail.com> wrote:
>
> Use qemu_dup() to ensure the cloexec flag is set on dup'd fds.
>
> The only direct users of dup() are now guest syscalls in the user
> emulators and the implementation of qemu_dup() itself.
>
> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>

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

> ---
> For each call I replaced, I checked how the resulting fd is used
> afterwards. In summary:
>
> hw/virtio/virtio.c:
>     the fd is passed to memory_region_init_ram_from_fd, used for mmaping
>
> io/channel-file.c:
>     the fd is passed to qio_channel_file_new_fd, used for reading/writing
>
> migration/cpr-exec.c:
>     the cloexec flag is cleared
>
> qemu-nbd.c:
>     the fd is stored into old_stderr, then dup2'd back to STDERR_FILENO
>
> system/vl.c:
>     no change in behavior, the code was already setting the cloexec flag
>
> ui/dmabuf.c:
>     the function is called by qemu_spice_gl_update, which passes the fds
>     to SPICE server
>
> tests/qtest/bios-tables-test.c:
>     temp dup of stdout, dup2'd back to STDOUT_FILENO
>
> tests/unit/test-util-sockets.c:
>     one fd is dup'd again and returned from mock monitor_get_fd; another
>     is converted to string and given as address to socket_connect
>
> tests/unit/test-vmstate.c:
>     passed to qio_channel_file_new_fd to create a channel
>
> Changes in v3:
> - Also replace the use in migration/cpr-exec.c and the tests
> - Link to v2: https://lore.kernel.org/qemu-devel/20261013-use-qemu-dup-v2-1-12ecd94263b7@gmail.com
>
> Changes in v2:
> - Replace the use in qemu-nbd.c too
> - Changed commit message
> - Link to v1: https://lore.kernel.org/qemu-devel/20261012-use-qemu-dup-v1-1-03d94852a6c8@gmail.com
> ---
>  hw/virtio/virtio.c             | 2 +-
>  io/channel-file.c              | 2 +-
>  migration/cpr-exec.c           | 2 +-
>  qemu-nbd.c                     | 2 +-
>  system/vl.c                    | 9 +--------
>  tests/qtest/bios-tables-test.c | 2 +-
>  tests/unit/test-util-sockets.c | 6 +++---
>  tests/unit/test-vmstate.c      | 2 +-
>  ui/dmabuf.c                    | 2 +-
>  9 files changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index c9f9e5b511..70779fbd56 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3204,7 +3204,7 @@ VirtioSharedMemoryMapping *virtio_shared_memory_mapping_new(uint8_t shmid,
>          return NULL;
>      }
>
> -    fd = dup(fd);
> +    fd = qemu_dup(fd);
>      if (fd < 0) {
>          error_report("Failed to duplicate fd: %s", strerror(errno));
>          return NULL;
> diff --git a/io/channel-file.c b/io/channel-file.c
> index 5cef75a67c..3f707e80c9 100644
> --- a/io/channel-file.c
> +++ b/io/channel-file.c
> @@ -48,7 +48,7 @@ qio_channel_file_new_fd(int fd)
>  QIOChannelFile *
>  qio_channel_file_new_dupfd(int fd, Error **errp)
>  {
> -    int newfd = dup(fd);
> +    int newfd = qemu_dup(fd);
>
>      if (newfd < 0) {
>          error_setg_errno(errp, errno, "Could not dup FD %d", fd);
> diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
> index daa50916d2..2f38db30bb 100644
> --- a/migration/cpr-exec.c
> +++ b/migration/cpr-exec.c
> @@ -44,7 +44,7 @@ bool cpr_exec_persist_state(QEMUFile *f, Error **errp)
>  {
>      QIOChannelFile *fioc = QIO_CHANNEL_FILE(qemu_file_get_ioc(f));
>      /* coverity[leaked_storage] - mfd intentionally kept open across exec() */
> -    int mfd = dup(fioc->fd);
> +    int mfd = qemu_dup(fioc->fd);
>      char val[16];
>
>      /* Remember mfd in environment for post-exec load */
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index ed5895861b..db1a70b008 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -975,7 +975,7 @@ int main(int argc, char **argv)
>
>              /* Remember parent's stderr if we will be restoring it. */
>              if (opts.verbose /* fork_process is set */) {
> -                opts.old_stderr = dup(STDERR_FILENO);
> +                opts.old_stderr = qemu_dup(STDERR_FILENO);
>                  if (opts.old_stderr < 0) {
>                      error_report("Could not dup original stderr: %s",
>                                   strerror(errno));
> diff --git a/system/vl.c b/system/vl.c
> index 468a9fc247..9381e3d7f7 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -614,14 +614,7 @@ static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
>          return -1;
>      }
>
> -#ifdef F_DUPFD_CLOEXEC
> -    dupfd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
> -#else
> -    dupfd = dup(fd);
> -    if (dupfd != -1) {
> -        qemu_set_cloexec(dupfd);
> -    }
> -#endif
> +    dupfd = qemu_dup(fd);
>      if (dupfd == -1) {
>          error_setg_errno(errp, errno, "error duplicating fd");
>          return -1;
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index f6dfa4131b..c4838322fb 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -529,7 +529,7 @@ static void test_acpi_asl(test_data *data)
>                      const char *diff_cmd = diff_env ? diff_env : "diff -U 16";
>                      char *diff = g_strdup_printf("%s %s %s", diff_cmd,
>                                                   exp_sdt->asl_file, sdt->asl_file);
> -                    int out = dup(STDOUT_FILENO);
> +                    int out = qemu_dup(STDOUT_FILENO);
>                      int ret G_GNUC_UNUSED;
>                      int dupret;
>
> diff --git a/tests/unit/test-util-sockets.c b/tests/unit/test-util-sockets.c
> index 006f5e579c..c47ad6c82d 100644
> --- a/tests/unit/test-util-sockets.c
> +++ b/tests/unit/test-util-sockets.c
> @@ -62,7 +62,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
>          error_setg(errp, "No fd named %s", fdname);
>          return -1;
>      }
> -    return dup(mon_fd);
> +    return qemu_dup(mon_fd);
>  }
>
>  /*
> @@ -115,7 +115,7 @@ static void test_socket_fd_pass_name_bad(void)
>
>      cur_mon = g_malloc(1); /* Fake a monitor */
>      mon_fdname = "myfd";
> -    mon_fd = dup(STDOUT_FILENO);
> +    mon_fd = qemu_dup(STDOUT_FILENO);
>      g_assert_cmpint(mon_fd, >, STDERR_FILENO);
>
>      addr.type = SOCKET_ADDRESS_TYPE_FD;
> @@ -189,7 +189,7 @@ static void test_socket_fd_pass_num_bad(void)
>      int fd, sfd;
>
>      g_assert(cur_mon == NULL);
> -    sfd = dup(STDOUT_FILENO);
> +    sfd = qemu_dup(STDOUT_FILENO);
>
>      addr.type = SOCKET_ADDRESS_TYPE_FD;
>      addr.u.fd.str = g_strdup_printf("%d", sfd);
> diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
> index 23f3173070..57f1e3263f 100644
> --- a/tests/unit/test-vmstate.c
> +++ b/tests/unit/test-vmstate.c
> @@ -42,7 +42,7 @@ static QEMUFile *open_test_file(bool write)
>      QIOChannel *ioc;
>      QEMUFile *f;
>
> -    fd = dup(temp_fd);
> +    fd = qemu_dup(temp_fd);
>      g_assert(fd >= 0);
>      lseek(fd, 0, SEEK_SET);
>      if (write) {
> diff --git a/ui/dmabuf.c b/ui/dmabuf.c
> index 497f82cc61..6d5dffdf84 100644
> --- a/ui/dmabuf.c
> +++ b/ui/dmabuf.c
> @@ -95,7 +95,7 @@ void qemu_dmabuf_dup_fds(QemuDmaBuf *dmabuf, int *fds, int nfds)
>      assert(nfds >= dmabuf->num_planes);
>
>      for (i = 0; i < dmabuf->num_planes; i++) {
> -        fds[i] = dmabuf->fd[i] >= 0 ? dup(dmabuf->fd[i]) : -1;
> +        fds[i] = dmabuf->fd[i] >= 0 ? qemu_dup(dmabuf->fd[i]) : -1;
>      }
>  }
>
>
> ---
> base-commit: 615ece3c406b262996e569f29ea6eed0d81c4d8d
> change-id: 20261012-use-qemu-dup-38628dbac3ee
>
> Best regards,
> --
> Filip Hejsek <filip.hejsek@gmail.com>
>
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-15 11:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15  1:34 [PATCH v3] treewide: use qemu_dup() instead of dup() Filip Hejsek
2026-10-15  8:04 ` Daniel P. Berrangé
2026-10-15 10:09   ` Filip Hejsek
2026-10-15 10:10     ` Daniel P. Berrangé
2026-10-15 11:41 ` Marc-André Lureau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.