Hi

On Mon, May 23, 2022 at 2:43 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> 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'.
>


qemu_open() has "/dev/fdset" handling, which qemu-ga and other tools don't
need.

(qemu_open_old() doesn't set CLOEXEC)


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

NULL is passed as errp argument of qemu_open_cloexec() in
qemu_open_internal().


>
> >      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);
> >
>

here


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

-- 
Marc-André Lureau