Re: [PATCH V8 13/39] oslib: qemu_clear_cloexec

[Steven Sistare]

On 6/16/2022 12:07 PM, Daniel P. Berrangé wrote:
> On Wed, Jun 15, 2022 at 07:52:00AM -0700, Steve Sistare wrote:
>> Define qemu_clear_cloexec, analogous to qemu_set_cloexec.
>>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>  include/qemu/osdep.h | 1 +
>>  util/oslib-posix.c   | 9 +++++++++
>>  util/oslib-win32.c   | 4 ++++
>>  3 files changed, 14 insertions(+)
>>
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index b1c161c..e916f3b 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -548,6 +548,7 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t 
>> count)
>>      G_GNUC_WARN_UNUSED_RESULT;
>>  
>>  void qemu_set_cloexec(int fd);
>> +void qemu_clear_cloexec(int fd);
> 
> I'm a little wary of adding this helper without any accompanying
> comment.
> 
> It is almost never correct to use this new method in a threaded
> program like QEMU, unless you have strong confidence that all
> the other threads are idle and not liable to perform a fork+exec
> for any other reason.
> 
> IIUC, this can be satisfied by the CPR code because it will be
> used only immediately before exec'ing the updated QEMU binary,
> and it has suspended any other CPUs and not other monitor
> commands are concurrently running.
> 
> IOW, I just ask that you put a comment with a big warning that
> essentially no one should use this method, except CPR code.
> 
> With regards,
> Daniel

Sure thing, will do - Steve