qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring


From: Denis V. Lunev
Subject: Re: [Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring
Date: Tue, 17 Feb 2015 21:05:15 +0300
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.4.0

On 17/02/15 20:56, Michael Roth wrote:
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 <address@hidden>

Moved the code that sets non-blocking flag on fd into a separate function.

Signed-off-by: Simon Zolin <address@hidden>
Reviewed-by: Roman Kagan <address@hidden>
Signed-off-by: Denis V. Lunev <address@hidden>
CC: Michael Roth <address@hidden>
CC: Eric Blake <address@hidden>
Signed-off-by: Michael Roth <address@hidden>
---
   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).


OK to me.

I'll check Win32 version with Olga to be somewhat
sure with upcoming cleanup.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]