QEMU-Devel Archive mirror
 help / color / mirror / Atom feed
From: Fabiano Rosas <farosas@suse.de>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: "Steve Sistare" <steven.sistare@oracle.com>,
	qemu-devel@nongnu.org, "Peter Xu" <peterx@redhat.com>,
	"David Hildenbrand" <david@redhat.com>,
	"Igor Mammedov" <imammedo@redhat.com>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
	"Philippe Mathieu-Daude" <philmd@linaro.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: Re: [PATCH V1 01/26] oslib: qemu_clear_cloexec
Date: Tue, 07 May 2024 10:54:35 -0300	[thread overview]
Message-ID: <87zft1lobo.fsf@suse.de> (raw)
In-Reply-To: <Zjns1qd57E86lAGy@redhat.com>

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Mon, May 06, 2024 at 08:27:15PM -0300, Fabiano Rosas wrote:
>> Steve Sistare <steven.sistare@oracle.com> writes:
>> 
>> +cc dgilbert, marcandre
>> 
>> > Define qemu_clear_cloexec, analogous to qemu_set_cloexec.
>> >
>> > Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> > Reviewed-by: Marc-André Lureau <marcandre.lureau@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 <farosas@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


  reply	other threads:[~2024-05-07 13:55 UTC|newest]

Thread overview: 122+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-29 15:55 [PATCH V1 00/26] Live update: cpr-exec Steve Sistare
2024-04-29 15:55 ` [PATCH V1 01/26] oslib: qemu_clear_cloexec Steve Sistare
2024-05-06 23:27   ` Fabiano Rosas
2024-05-07  8:56     ` Daniel P. Berrangé
2024-05-07 13:54       ` Fabiano Rosas [this message]
2024-04-29 15:55 ` [PATCH V1 02/26] vl: helper to request re-exec Steve Sistare
2024-04-29 15:55 ` [PATCH V1 03/26] migration: SAVEVM_FOREACH Steve Sistare
2024-05-06 23:17   ` Fabiano Rosas
2024-05-13 19:27     ` Steven Sistare
2024-05-27 18:14       ` Peter Xu
2024-04-29 15:55 ` [PATCH V1 04/26] migration: delete unused parameter mis Steve Sistare
2024-05-06 21:50   ` Fabiano Rosas
2024-05-27 18:02   ` Peter Xu
2024-04-29 15:55 ` [PATCH V1 05/26] migration: precreate vmstate Steve Sistare
2024-05-07 21:02   ` Fabiano Rosas
2024-05-13 19:28     ` Steven Sistare
2024-05-24 13:56   ` Fabiano Rosas
2024-05-27 18:16   ` Peter Xu
2024-05-28 15:09     ` Steven Sistare via
2024-05-29 18:39       ` Peter Xu
2024-05-30 17:04         ` Steven Sistare via
2024-04-29 15:55 ` [PATCH V1 06/26] migration: precreate vmstate for exec Steve Sistare
2024-05-06 23:34   ` Fabiano Rosas
2024-05-13 19:28     ` Steven Sistare
2024-05-13 21:21       ` Fabiano Rosas
2024-04-29 15:55 ` [PATCH V1 07/26] migration: VMStateId Steve Sistare
2024-05-07 21:03   ` Fabiano Rosas
2024-05-27 18:20   ` Peter Xu
2024-05-28 15:10     ` Steven Sistare via
2024-05-28 17:44       ` Peter Xu
2024-05-29 17:30         ` Steven Sistare via
2024-05-29 18:53           ` Peter Xu
2024-05-30 17:11             ` Steven Sistare via
2024-05-30 18:03               ` Peter Xu
2024-04-29 15:55 ` [PATCH V1 08/26] migration: vmstate_info_void_ptr Steve Sistare
2024-05-07 21:33   ` Fabiano Rosas
2024-05-27 18:31   ` Peter Xu
2024-05-28 15:10     ` Steven Sistare via
2024-05-28 18:21       ` Peter Xu
2024-05-29 17:30         ` Steven Sistare via
2024-04-29 15:55 ` [PATCH V1 09/26] migration: vmstate_register_named Steve Sistare
2024-05-09 14:19   ` Fabiano Rosas
2024-05-09 14:32     ` Fabiano Rosas
2024-05-13 19:29       ` Steven Sistare
2024-04-29 15:55 ` [PATCH V1 10/26] migration: vmstate_unregister_named Steve Sistare
2024-04-29 15:55 ` [PATCH V1 11/26] migration: vmstate_register at init time Steve Sistare
2024-04-29 15:55 ` [PATCH V1 12/26] migration: vmstate factory object Steve Sistare
2024-04-29 15:55 ` [PATCH V1 13/26] physmem: ram_block_create Steve Sistare
2024-05-13 18:37   ` Fabiano Rosas
2024-05-13 19:30     ` Steven Sistare
2024-04-29 15:55 ` [PATCH V1 14/26] physmem: hoist guest_memfd creation Steve Sistare
2024-04-29 15:55 ` [PATCH V1 15/26] physmem: hoist host memory allocation Steve Sistare
2024-04-29 15:55 ` [PATCH V1 16/26] physmem: set ram block idstr earlier Steve Sistare
2024-04-29 15:55 ` [PATCH V1 17/26] machine: memfd-alloc option Steve Sistare
2024-05-28 21:12   ` Peter Xu
2024-05-29 17:31     ` Steven Sistare via
2024-05-29 19:14       ` Peter Xu
2024-05-30 17:11         ` Steven Sistare via
2024-05-30 18:14           ` Peter Xu
2024-05-31 19:32             ` Steven Sistare via
2024-06-03 21:48               ` Peter Xu
2024-06-04  7:13                 ` Daniel P. Berrangé
2024-06-04 15:58                   ` Peter Xu
2024-06-04 16:14                     ` David Hildenbrand
2024-06-04 16:41                       ` Peter Xu
2024-06-04 17:16                         ` David Hildenbrand
2024-06-03 10:17       ` Daniel P. Berrangé
2024-06-03 11:59         ` Steven Sistare via
2024-04-29 15:55 ` [PATCH V1 18/26] migration: cpr-exec-args parameter Steve Sistare
2024-05-02 12:23   ` Markus Armbruster
2024-05-02 16:00     ` Steven Sistare
2024-05-21  8:13   ` Daniel P. Berrangé
2024-04-29 15:55 ` [PATCH V1 19/26] physmem: preserve ram blocks for cpr Steve Sistare
2024-05-28 21:44   ` Peter Xu
2024-05-29 17:31     ` Steven Sistare via
2024-05-29 19:25       ` Peter Xu
2024-05-30 17:12         ` Steven Sistare via
2024-05-30 18:39           ` Peter Xu
2024-05-31 19:32             ` Steven Sistare via
2024-06-03 22:29               ` Peter Xu
2024-04-29 15:55 ` [PATCH V1 20/26] migration: cpr-exec mode Steve Sistare
2024-05-02 12:23   ` Markus Armbruster
2024-05-02 16:00     ` Steven Sistare
2024-05-03  6:26       ` Markus Armbruster
2024-05-21  8:20   ` Daniel P. Berrangé
2024-05-24 14:58   ` Fabiano Rosas
2024-05-27 18:54     ` Steven Sistare via
2024-04-29 15:55 ` [PATCH V1 21/26] migration: migrate_add_blocker_mode Steve Sistare
2024-05-09 17:47   ` Fabiano Rosas
2024-04-29 15:55 ` [PATCH V1 22/26] migration: ram block cpr-exec blockers Steve Sistare
2024-05-09 18:01   ` Fabiano Rosas
2024-05-13 19:29     ` Steven Sistare
2024-04-29 15:55 ` [PATCH V1 23/26] migration: misc " Steve Sistare
2024-05-09 18:05   ` Fabiano Rosas
2024-05-24 12:40   ` Fabiano Rosas
2024-05-27 19:02     ` Steven Sistare via
2024-04-29 15:55 ` [PATCH V1 24/26] seccomp: cpr-exec blocker Steve Sistare
2024-05-09 18:16   ` Fabiano Rosas
2024-05-10  7:54   ` Daniel P. Berrangé
2024-05-13 19:29     ` Steven Sistare
2024-05-21  7:14       ` Daniel P. Berrangé
2024-04-29 15:55 ` [PATCH V1 25/26] migration: fix mismatched GPAs during cpr-exec Steve Sistare
2024-05-09 18:39   ` Fabiano Rosas
2024-04-29 15:55 ` [PATCH V1 26/26] migration: only-migratable-modes Steve Sistare
2024-05-09 19:14   ` Fabiano Rosas
2024-05-13 19:48     ` Steven Sistare
2024-05-13 21:57       ` Fabiano Rosas
2024-05-21  8:05   ` Daniel P. Berrangé
2024-05-02 16:13 ` cpr-exec doc (was Re: [PATCH V1 00/26] Live update: cpr-exec) Steven Sistare
2024-05-02 18:15   ` Peter Xu
2024-05-20 18:30 ` [PATCH V1 00/26] Live update: cpr-exec Steven Sistare
2024-05-20 22:28   ` Fabiano Rosas
2024-05-21  2:31     ` Peter Xu
2024-05-21 11:46       ` Steven Sistare
2024-05-27 17:45         ` Peter Xu
2024-05-28 15:10           ` Steven Sistare via
2024-05-28 16:42             ` Peter Xu
2024-05-30 17:17               ` Steven Sistare via
2024-05-30 19:23                 ` Peter Xu
2024-05-24 13:02 ` Fabiano Rosas
2024-05-24 14:07   ` Steven Sistare
2024-05-27 18:07 ` Peter Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87zft1lobo.fsf@suse.de \
    --to=farosas@suse.de \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=david@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eduardo@habkost.net \
    --cc=imammedo@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=steven.sistare@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).