[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: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring |
Date: |
Tue, 17 Feb 2015 11:56:35 -0600 |
User-agent: |
alot/0.3.4 |
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).
> 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
- [Qemu-devel] [PATCH 09/10] qga: add memory block command that unsupported, (continued)
- [Qemu-devel] [PATCH 09/10] qga: add memory block command that unsupported, Michael Roth, 2015/02/16
- [Qemu-devel] [PATCH 04/10] qga: implement file commands for Windows guest, Michael Roth, 2015/02/16
- [Qemu-devel] [PATCH 05/10] qga: introduce three guest memory block commmands with stubs, Michael Roth, 2015/02/16
- [Qemu-devel] [PATCH 07/10] qga: implement qmp_guest_set_memory_blocks() for Linux with sysfs, Michael Roth, 2015/02/16
- [Qemu-devel] [PATCH 06/10] qga: implement qmp_guest_get_memory_blocks() for Linux with sysfs, Michael Roth, 2015/02/16
- [Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring, Michael Roth, 2015/02/16
[Qemu-devel] [PATCH 08/10] qga: implement qmp_guest_get_memory_block_size() for Linux with sysfs, Michael Roth, 2015/02/16
Re: [Qemu-devel] [PULL 00/10] Fixes and new commands for QEMU Guest Agent, Michael Roth, 2015/02/17