Re: [PATCH v3 05/15] osdep: export qemu_open_cloexec()

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

On Fri, May 13, 2022 at 08:08:11PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Used in the next patch, to simplify qga code.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qemu/osdep.h |  1 +
>  util/osdep.c         | 10 ++++++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 67cc465416..64f51cfb7a 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -489,6 +489,7 @@ void sigaction_invoke(struct sigaction *action,
>   */
>  int qemu_open_old(const char *name, int flags, ...);
>  int qemu_open(const char *name, int flags, Error **errp);
> +int qemu_open_cloexec(const char *name, int flags, mode_t mode, Error **errp);

I don't think we should be exporting this - it is just a variant of the
'qemu_open_old' method that we wanted callers to stop using in favour
of explicitly deciding between 'qemu_open' and 'qemu_create'.

>  int qemu_create(const char *name, int flags, mode_t mode, Error **errp);
>  int qemu_close(int fd);
>  int qemu_unlink(const char *name);
> diff --git a/util/osdep.c b/util/osdep.c
> index 60fcbbaebe..545a88e1fd 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -279,9 +279,11 @@ int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive)
>  }
>  #endif
>  
> -static int qemu_open_cloexec(const char *name, int flags, mode_t mode)
> +int qemu_open_cloexec(const char *name, int flags, mode_t mode, Error **errp)
>  {
> +    ERRP_GUARD();
>      int ret;
> +
>  #ifdef O_CLOEXEC
>      ret = open(name, flags | O_CLOEXEC, mode);
>  #else
> @@ -290,6 +292,10 @@ static int qemu_open_cloexec(const char *name, int flags, mode_t mode)
>          qemu_set_cloexec(ret);
>      }
>  #endif
> +    if (ret == -1) {
> +        error_setg_errno(errp, errno, "Failed to open file '%s'", name);
> +    }
> +

This will mean that qemu_open_internal() caller will now be overwriting
an existing error message.

>      return ret;
>  }
>  
> @@ -327,7 +333,7 @@ qemu_open_internal(const char *name, int flags, mode_t mode, Error **errp)
>      }
>  #endif
>  
> -    ret = qemu_open_cloexec(name, flags, mode);
> +    ret = qemu_open_cloexec(name, flags, mode, NULL);
>  
>      if (ret == -1) {
>          const char *action = flags & O_CREAT ? "create" : "open";
> -- 
> 2.36.1
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|