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

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

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