From: Michael Roth <mdroth@linux.vnet.ibm.com>
To: "Denis V. Lunev" <den@openvz.org>
Cc: Olga Krishtal <okrishtal@parallels.com>,
qemu-devel@nongnu.org, Stefan Weil <sw@weilnetz.de>
Subject: Re: [Qemu-devel] [PATCH 3/3] qga: set file descriptor in qmp_guest_file_open non-blocking on Win32
Date: Tue, 27 Oct 2015 14:49:28 -0500 [thread overview]
Message-ID: <20151027194928.4255.73297@loki> (raw)
In-Reply-To: <562FCCF5.7090807@openvz.org>
Quoting Denis V. Lunev (2015-10-27 14:13:57)
> On 10/27/2015 10:11 PM, Michael Roth wrote:
> > Quoting Denis V. Lunev (2015-10-27 12:48:43)
> >> From: Olga Krishtal <okrishtal@parallels.com>
> >>
> >> Set fd non-blocking to avoid common use cases (like reading from a
> >> named pipe) from hanging the agent. This was missed in the original
> >> code.
> >>
> >> The patch introduces analog of qemu_set_non/block for HANDLES.
> >> The usage of handles in qemu_set_non/block is impossible, because for
> >> win32 there is a difference between file discriptors and file handles,
> >> and all file ops are made via Win32 api.
> > If this is specific to HANDLEs, why do we need to cast back and forth
> > between int64_t and HANDLE? I haven't build tested, but it seems like
> > this would break for 32-bit mingw builds.
> >
> > I would define these as qemu_set_*_by_handle(HANDLE fh, ...) instead
> > and make them win32 only. If someone wants to introduce a FILE*
> > variant for posix they can introduce it as
> > qemu_set_*_by_handle(FILE *fh, ...) rather than us needing to
> > abstract away the handle type.
>
> may be it would be better to add static function for this in QGA for now?
I'd be fine with either approach. It could be generally useful for
other w32 users. But if we're thinking about dropping the QGA
use case soon then maybe having it live in QGA is best.
>
> I am eager to drop this code at once for Posix and Windows and
> switch to GLIB like was done for guest exec.
You mean switching all the guest-file-* interfaces to glib? I
took a stab at it once for w32 guest-file-* implementation, but
one issue I hit was that I couldn't figure out how to implement
guest-file-seek to report back the absolute position in the
file, or whether or not we'd hit EOF. You can set position
via g_io_channel_seek_position(), but if they hit EOF, or are
using relative offsets via G_SEEK_CUR, you don't really know
the position and glib doesn't seem to provide a way to query
that. We could maybe work around it by tracking it manually
via guest-file-* calls but that sounds terrible.
Hopefully I just missed something though. Also couldn't figure
out how you can get glib to report that you'd already seeked
to EOF. I had some comments about it in my WIP:
https://github.com/mdroth/qemu/commit/8b2e5c69266bb48e492af9826122c2aaa4a82197#diff-7f29c3e51a7b387cc7717e7be4f6e205R525
>
> Den
>
next prev parent reply other threads:[~2015-10-27 19:49 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-27 17:48 [Qemu-devel] [PATCH v3 for 2.5 0/3] qga: non-blocking fd cleanups Denis V. Lunev
2015-10-27 17:48 ` [Qemu-devel] [PATCH 1/3] qga: drop hand-made guest_file_toggle_flags helper Denis V. Lunev
2015-10-27 17:48 ` [Qemu-devel] [PATCH 2/3] qga: fixed CloseHandle in qmp_guest_file_open Denis V. Lunev
2015-10-27 18:14 ` Stefan Weil
2015-10-27 17:48 ` [Qemu-devel] [PATCH 3/3] qga: set file descriptor in qmp_guest_file_open non-blocking on Win32 Denis V. Lunev
2015-10-27 19:11 ` Michael Roth
2015-10-27 19:13 ` Denis V. Lunev
2015-10-27 19:49 ` Michael Roth [this message]
2015-10-28 15:18 ` Denis V. Lunev
2015-10-27 19:14 ` Michael Roth
-- strict thread matches above, loose matches on Subject: below --
2015-10-28 15:13 [Qemu-devel] [PATCH v4 for 2.5 0/3] qga: non-blocking fd cleanups Denis V. Lunev
2015-10-28 15:13 ` [Qemu-devel] [PATCH 3/3] qga: set file descriptor in qmp_guest_file_open non-blocking on Win32 Denis V. Lunev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151027194928.4255.73297@loki \
--to=mdroth@linux.vnet.ibm.com \
--cc=den@openvz.org \
--cc=okrishtal@parallels.com \
--cc=qemu-devel@nongnu.org \
--cc=sw@weilnetz.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).