From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48987) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNmXJ-0004rA-Js for qemu-devel@nongnu.org; Tue, 17 Feb 2015 13:06:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YNmXF-0008CQ-Ht for qemu-devel@nongnu.org; Tue, 17 Feb 2015 13:06:17 -0500 Received: from mx2.parallels.com ([199.115.105.18]:51211) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNmXF-0007y4-Bq for qemu-devel@nongnu.org; Tue, 17 Feb 2015 13:06:13 -0500 Message-ID: <54E382DB.6030509@openvz.org> Date: Tue, 17 Feb 2015 21:05:15 +0300 From: "Denis V. Lunev" MIME-Version: 1.0 References: <1424142892-7275-1-git-send-email-mdroth@linux.vnet.ibm.com> <1424142892-7275-4-git-send-email-mdroth@linux.vnet.ibm.com> <54E35DEF.3020309@redhat.com> <54E36719.8040900@openvz.org> <20150217175635.13315.80188@loki> In-Reply-To: <20150217175635.13315.80188@loki> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth , "Denis V. Lunev" , Eric Blake , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, Simon Zolin , Olga Krishtal 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 >>>> >>>> Moved the code that sets non-blocking flag on fd into a separate function. >>>> >>>> Signed-off-by: Simon Zolin >>>> Reviewed-by: Roman Kagan >>>> Signed-off-by: Denis V. Lunev >>>> CC: Michael Roth >>>> CC: Eric Blake >>>> Signed-off-by: Michael Roth >>>> --- >>>> 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.