3:29 a.m.
Commit 2ed6cc7bec41dd344d41ea1531f6760c93099128 "Expose event loop
implementation as a public API" turned a failure to initialize the
default event loop into a fatal error in virsh on Windows. Before that
commit such a failure was ignored.
virEventRegisterDefaultImpl calls virEventPollInit that calls
virSetNonBlock that calls ioctl that is replaced by gnulib and calls
ioctlsocket. ioctlsocket fails because the given FD is a pipe but
ioctlsocket expects a socket.
A version that works on pipes on Windows looks like this. Although the
pipe is actually not a named pipe the call to SetNamedPipeHandleState
doesn't fail at least.
int
virSetPipeNonBlock(int fd)
{
DWORD mode = PIPE_NOWAIT;
HANDLE handle = _get_osfhandle(fd)
BOOL result = SetNamedPipeHandleState(handle, &mode, NULL, NULL);
return result ? 0 : -1;
}
So, is the event loop stuff supposed to work on Windows at all and we
should get it fixed? Or do we just put an #ifndef WIN32 around
virEventRegisterDefaultImpl in virsh, because the only event loop user
in virsh is the console command that is disabled on Windows anyway?
Matthias
6:02 a.m.
On Tue, Mar 15, 2011 at 09:29:14AM +0100, Matthias Bolte wrote:
Commit 2ed6cc7bec41dd344d41ea1531f6760c93099128 "Expose event
loop
implementation as a public API" turned a failure to initialize the
default event loop into a fatal error in virsh on Windows. Before that
commit such a failure was ignored.
virEventRegisterDefaultImpl calls virEventPollInit that calls
virSetNonBlock that calls ioctl that is replaced by gnulib and calls
ioctlsocket. ioctlsocket fails because the given FD is a pipe but
ioctlsocket expects a socket.
Hmm, isn't this actually a bug in gnulib's ioctl replacement then ?
The goal of gnulib is that you never use a SOCKET in any of the
POSIX APIs it wraps. It hides the WinSock horribleness so that you
can just use normal FDs everywhere.
A version that works on pipes on Windows looks like this. Although
the
pipe is actually not a named pipe the call to SetNamedPipeHandleState
doesn't fail at least.
int
virSetPipeNonBlock(int fd)
{
DWORD mode = PIPE_NOWAIT;
HANDLE handle = _get_osfhandle(fd)
BOOL result = SetNamedPipeHandleState(handle, &mode, NULL, NULL);
return result ? 0 : -1;
}
So, is the event loop stuff supposed to work on Windows at all and we
should get it fixed? Or do we just put an #ifndef WIN32 around
virEventRegisterDefaultImpl in virsh, because the only event loop user
in virsh is the console command that is disabled on Windows anyway?
There were several reasons the console stuff used to be disabled
- It relied on opening /dev/pts/XXX which obviously only
makes sense for local connection, not remote ones.
- It uses the terminal attribute APIs to put the console
into raw mode.
The event loop is intended to be portable, and certainly, now it is
a public API we should make it work. The opening of /dev/pts/XXX is
no longer a blocker for virsh console on Win32, since we access the
console via the streams APIs. So if we can figure out what todo with
the terminal attribute APIs, virsh console should work on Win32 (or
any other platform). This is one of the nice benefits of the streams
API
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:32 p.m.
On 03/15/2011 05:02 AM, Daniel P. Berrange wrote:
On Tue, Mar 15, 2011 at 09:29:14AM +0100, Matthias Bolte wrote:
> Commit 2ed6cc7bec41dd344d41ea1531f6760c93099128 "Expose event loop
> implementation as a public API" turned a failure to initialize the
> default event loop into a fatal error in virsh on Windows. Before that
> commit such a failure was ignored.
>
> virEventRegisterDefaultImpl calls virEventPollInit that calls
> virSetNonBlock that calls ioctl that is replaced by gnulib and calls
> ioctlsocket. ioctlsocket fails because the given FD is a pipe but
> ioctlsocket expects a socket.
Hmm, isn't this actually a bug in gnulib's ioctl replacement then ?
Not necessarily. FIONBIO is not a standardized ioctl() call (for that
matter, ioctl() is only standardized for STREAMS, which is an obsolete
Solaris concept not really supported in Linux). Rather, the gnulib
ioctl() wrapper exists so you can call undo the rest of gnulib's munging
of mingw socket fds, since the real mingw ioctl() has to operate on the
socket rather than the (nicer) fd.
The goal of gnulib is that you never use a SOCKET in any of the
POSIX APIs it wraps. It hides the WinSock horribleness so that you
can just use normal FDs everywhere.
But use of ioctl() is already non-portable; so a better solution would
be one that reduces the amount of direct ioctl in gnulib in the first
place (well, some ioctls are necessary, like lxc or kvm startup, but
that's in Linux-specific code and not expected to be portable elsewhere,
when contrasted with this bug report about the use of ioctl() in the
main generic polling loop).
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
6:27 p.m.
[adding bug-gnulib and Paolo as author of rpl_ioctl]
On 03/15/2011 02:29 AM, Matthias Bolte wrote:
Commit 2ed6cc7bec41dd344d41ea1531f6760c93099128 "Expose event
loop
implementation as a public API" turned a failure to initialize the
default event loop into a fatal error in virsh on Windows. Before that
commit such a failure was ignored.
virEventRegisterDefaultImpl calls virEventPollInit that calls
virSetNonBlock that calls ioctl that is replaced by gnulib and calls
ioctlsocket. ioctlsocket fails because the given FD is a pipe but
ioctlsocket expects a socket.
A version that works on pipes on Windows looks like this. Although the
pipe is actually not a named pipe the call to SetNamedPipeHandleState
doesn't fail at least.
int
virSetPipeNonBlock(int fd)
{
DWORD mode = PIPE_NOWAIT;
HANDLE handle = _get_osfhandle(fd)
BOOL result = SetNamedPipeHandleState(handle, &mode, NULL, NULL);
return result ? 0 : -1;
}
So, is the event loop stuff supposed to work on Windows at all and we
should get it fixed? Or do we just put an #ifndef WIN32 around
virEventRegisterDefaultImpl in virsh, because the only event loop user
in virsh is the console command that is disabled on Windows anyway?
Right now, gnulib's rpl_ioctl doesn't do any inspection of the various
requests handed it, nor does it check whether the request makes sense on
a non-socket. Sniffing the FIONBIO request and handling it for pipes as
well as sockets might make sense.
And while gnulib has rpl_fcntl for mingw, it does not yet support
F_SETFL or F_GETFL to inspect/set O_NONBLOCK on non-file fds. I'm not
sure how much effort that would be to add; I suppose that if F_GETFL
doesn't support all possible flags, but does support enough that we
could detect whether a read-modify-write F_GETFL/F_SETFL sequence is
trying to set or clear just O_NONBLOCK, that it would be sufficient for
the task at hand.
But rather than exposing fcntl, maybe the better thing to do is have
gnulib provide a simple wrapper API module that controls whether an fd
is blocking (similar to how the gnulib cloexec module hides
fcntl(F_GETFD/F_SETFD) or a mingw counterpart).
Paolo, any thoughts on the best approach to take? (I know which way I'm
leaning, but want some feedback before I give away my bias).
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
1:48 a.m.
On 03/29/2011 01:27 AM, Eric Blake wrote:
Paolo, any thoughts on the best approach to take? (I know which way
I'm
leaning, but want some feedback before I give away my bias).
Without guessing what your bias is, I also :) prefer to implement
{g,s}et_nonblock_flag functions. It would use either
SetNamedPipeHandleState or ioctlsocket (using the socket detection trick
in sockets.c to detect sockets, and then GetFileType to detect pipes if
it fails).
This module can then be used to build a fcntl F_GETFL/F_SETFL
implementation, but it is not very important to do so.
Paolo
2:26 a.m.
On Tue, Mar 29, 2011 at 8:48 AM, Paolo Bonzini <bonzini(a)gnu.org> wrote:
On 03/29/2011 01:27 AM, Eric Blake wrote:
>
> Paolo, any thoughts on the best approach to take? (I know which way I'm
> leaning, but want some feedback before I give away my bias).
Without guessing what your bias is, I also :) prefer to implement
{g,s}et_nonblock_flag functions. It would use either
SetNamedPipeHandleState or ioctlsocket (using the socket detection trick in
sockets.c to detect sockets, and then GetFileType to detect pipes if it
fails).
This module can then be used to build a fcntl F_GETFL/F_SETFL
implementation, but it is not very important to do so.
Could you also implement SOCK_NONBLOCK ?
Bastien
Paolo
6:25 p.m.
On 03/15/2011 02:29 AM, Matthias Bolte wrote:
Commit 2ed6cc7bec41dd344d41ea1531f6760c93099128 "Expose event
loop
implementation as a public API" turned a failure to initialize the
default event loop into a fatal error in virsh on Windows. Before that
commit such a failure was ignored.
Does this patch fix things? If so, we should definitely get it into 0.9.0:
https://www.redhat.com/archives/libvir-list/2011-March/msg01553.html
(but be sure to pick up the logic correction before testing):
https://www.redhat.com/archives/libvir-list/2011-March/msg01554.html
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:38 a.m.
2011/4/1 Eric Blake <eblake(a)redhat.com>:
On 03/15/2011 02:29 AM, Matthias Bolte wrote:
> Commit 2ed6cc7bec41dd344d41ea1531f6760c93099128 "Expose event loop
> implementation as a public API" turned a failure to initialize the
> default event loop into a fatal error in virsh on Windows. Before that
> commit such a failure was ignored.
Does this patch fix things? If so, we should definitely get it into 0.9.0:
https://www.redhat.com/archives/libvir-list/2011-March/msg01553.html
(but be sure to pick up the logic correction before testing):
https://www.redhat.com/archives/libvir-list/2011-March/msg01554.html
Yes, at least virSetNonBlock doesn't fail anymore. But now
virSetCloseExec is the next thing that fails, because it calls
virSetInherit and virSetInherit always returns -1 on Windows.
Matthias
4983
days inactive
5001
days old
7 comments
5 participants
participants (5)
-
Bastien ROUCARIES -
Daniel P. Berrange -
Eric Blake -
Matthias Bolte -
Paolo Bonzini