Quoting Denis V. Lunev (2015-02-17 10:06:49)
> On 17/02/15 18:27, Eric Blake wrote:
> > On 02/16/2015 08:14 PM, Michael Roth wrote:
> >> From: Simon Zolin <szo...@parallels.com>
> >>
> >> Moved the code that sets non-blocking flag on fd into a separate function.
> >>
> >> Signed-off-by: Simon Zolin <szo...@parallels.com>
> >> Reviewed-by: Roman Kagan <rka...@parallels.com>
> >> Signed-off-by: Denis V. Lunev <d...@openvz.org>
> >> CC: Michael Roth <mdr...@linux.vnet.ibm.com>
> >> CC: Eric Blake <ebl...@redhat.com>
> >> Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com>
> >> ---
> >> qga/commands-posix.c | 31 +++++++++++++++++++++++--------
> >> 1 file changed, 23 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> >> index 57409d0..ed527a3 100644
> >> --- a/qga/commands-posix.c
> >> +++ b/qga/commands-posix.c
> >> @@ -376,13 +376,33 @@ safe_open_or_create(const char *path, const char
> >> *mode, Error **errp)
> >> return NULL;
> >> }
> >>
> >> +static int guest_file_toggle_flags(int fd, int flags, bool set, Error
> >> **err)
> >> +{
> > Why are you reinventing qemu_set_nonblock()?
> >
> because we are uneducated :)
>
> Anyway, qemu_set_nonblock() does not handle error
> and resides in a strange header aka "include/qemu/sockets.h"
> Technically I can switch to it immediately. Though error
> check condition will be lost.
>
> What is better at your opinion?
>
> a) return error from qemu_set_nonblock()/qemu_set_block()
I think making the existing functions a non-error-checking
wrapper around qemu_set_{block,nonblock}_error or something
would be best.
These are meant to be os-agnostic utility functions though,
but in the case of qemu-ga the win32 variant won't work as
expected, so I'm not sure it's a good idea to rely on them.
If the lack of it's usage in net/tap* compared to other parts
of QEMU that build on w32 is any indication, that seems to
be the pattern followed by other users.
In any case, since I was actually the one who re-invented it,
and this code just moves it to another function, I think we
can address it as a seperate patch and leave the PULL
intact (unless there are other objections).
> b) drop error check here. The descriptor is just opened
> and we know that it is valid. I could not imagine real
> error other than broken descriptor for this exact fcntl.
>
> Regards,
> Den
