Daniel P. Berrangé <berra...@redhat.com> writes:
> On Mon, May 06, 2024 at 08:27:15PM -0300, Fabiano Rosas wrote:
>> Steve Sistare <steven.sist...@oracle.com> writes:
>>
>> +cc dgilbert, marcandre
>>
>> > Define qemu_clear_cloexec, analogous to qemu_set_cloexec.
>> >
>> > Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
>> > Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
>> > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>>
>> A v1 patch with two reviews already, from people from another company
>> and they're not in CC. Looks suspicious. =)
>
> It is ok in this case - the cpr work has been going on a long
> time and the original series that got partial reviews has been
> split up somewhat. So its "v1" of this series of patches, but
> not "v1" of what we've seen posted on qemu-devel in the past
I know =) I searched the archives to make sure those r-bs were actually
provided by them and I also remember the series from back then.
On that topic, but not related to this patch at all, I would prefer if
we had a no-preexisting r-bs rule. I don't see any value in an r-b that
already comes present in v1 and has not been provided through the
list. There's the obvious concern about bad faith, but also that we
might lose track of a series and maintainers/tools might take those r-bs
as a sign of the code actually being reviewed properly (which may or may
not be true).
In the case people develop a series inside the company and then post to
the list, an sob seems to be adequate enough.
>
>>
>> Here's a fresh one, hopefully it won't spend another 4 years in the
>> drawer:
>>
>> Reviewed-by: Fabiano Rosas <faro...@suse.de>
>>
>> > ---
>> > include/qemu/osdep.h | 9 +++++++++
>> > util/oslib-posix.c | 9 +++++++++
>> > util/oslib-win32.c | 4 ++++
>> > 3 files changed, 22 insertions(+)
>> >
>> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> > index c7053cd..b58f312 100644
>> > --- a/include/qemu/osdep.h
>> > +++ b/include/qemu/osdep.h
>> > @@ -660,6 +660,15 @@ ssize_t qemu_write_full(int fd, const void *buf,
>> > size_t count)
>> >
>> > void qemu_set_cloexec(int fd);
>> >
>> > +/*
>> > + * Clear FD_CLOEXEC for a descriptor.
>> > + *
>> > + * The caller must guarantee that no other fork+exec's occur before the
>> > + * exec that is intended to inherit this descriptor, eg by suspending CPUs
>> > + * and blocking monitor commands.
>> > + */
>> > +void qemu_clear_cloexec(int fd);
>> > +
>> > /* Return a dynamically allocated directory path that is appropriate for
>> > storing
>> > * local state.
>> > *
>> > diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>> > index e764416..614c3e5 100644
>> > --- a/util/oslib-posix.c
>> > +++ b/util/oslib-posix.c
>> > @@ -272,6 +272,15 @@ int qemu_socketpair(int domain, int type, int
>> > protocol, int sv[2])
>> > return ret;
>> > }
>> >
>> > +void qemu_clear_cloexec(int fd)
>> > +{
>> > + int f;
>> > + f = fcntl(fd, F_GETFD);
>> > + assert(f != -1);
>> > + f = fcntl(fd, F_SETFD, f & ~FD_CLOEXEC);
>> > + assert(f != -1);
>> > +}
>> > +
>> > char *
>> > qemu_get_local_state_dir(void)
>> > {
>> > diff --git a/util/oslib-win32.c b/util/oslib-win32.c
>> > index b623830..c3e969a 100644
>> > --- a/util/oslib-win32.c
>> > +++ b/util/oslib-win32.c
>> > @@ -222,6 +222,10 @@ void qemu_set_cloexec(int fd)
>> > {
>> > }
>> >
>> > +void qemu_clear_cloexec(int fd)
>> > +{
>> > +}
>> > +
>> > int qemu_get_thread_id(void)
>> > {
>> > return GetCurrentThreadId();
>>
>
> With regards,
> Daniel
