[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 05/15] osdep: export qemu_open_cloexec()
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v3 05/15] osdep: export qemu_open_cloexec() |
Date: |
Mon, 23 May 2022 18:56:09 +0100 |
User-agent: |
Mutt/2.2.1 (2022-02-19) |
On Mon, May 23, 2022 at 07:30:42PM +0200, Marc-André Lureau wrote:
> 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.
Right, but exporting this as 'qemu_open_cloexec' is going to mislead
people into thinking it is a better version of 'qemu_open'. This will
cause us to loose support for /dev/fdset in places where we actually
need it.
It is pretty harmless to have /dev/fdset there, even if the tool does
not need it - that's been the case with many QEMU tools for many years.
If we think it is actually a real problem though, we should just have
a way to toggle it on/off from the existing APIs.
eg put 'bool allow_fdset = true" in softmmu/vl.c, and
'bool allow_fdset = false' in stubs/open.c, and then make
qemu_open_internal conditionalize itself on this global
variable, so only the system emulators get fdset support
activated.
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 :|
[PATCH v3 04/15] qga: flatten safe_open_or_create(), marcandre . lureau, 2022/05/13
[PATCH v3 06/15] qga: use qemu_open_cloexec() for safe_open_or_create(), marcandre . lureau, 2022/05/13
[PATCH v3 07/15] qga: throw an Error in ga_channel_open(), marcandre . lureau, 2022/05/13
[PATCH v3 08/15] qga: replace qemu_open_old() with qemu_open_cloexec(), marcandre . lureau, 2022/05/13