Re: [PATCH v3] treewide: use qemu_dup() instead of dup()

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|