On Tue, Dec 08, 2015 at 10:04:55AM -0700, Eric Blake wrote: > On 12/08/2015 07:59 AM, Daniel P. Berrange wrote: > > > So for this my plan is to stop using the QEMU 'file' backend for char > > devs and instead pass across a pre-opened file descriptor, connected > > to virtlogd. There is no "officially documented" way to pass in a > > file descriptor to QEMU chardevs, but since QEMU uses qemu_open(), > > we can make use of the fdset feature to achieve this. eg > > > > eg, consider fd 33 is the write end of a pipe file descriptor > > I can (in theory) do > > > > -add-fd set=2,fd=33 -chardev file,id=charserial0,path=/dev/fdset/2 > > > > Now in practice this doesn't work, because qmp_chardev_open_file() > > passes the O_CREAT|O_TRUNC flags in, which means the qemu_open() > > call will fail when using the pipe FD pased in via fdsets. > > Is it just the O_TRUNC that is failing? If so, there is a recent patch > to add an 'append':true flag that switches O_TRUNC off in favor of O_APPEND: > https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg00762.html Yes, it is the ftruncate() call in qemu_dup_flags, called from qemu_open that is failing. > Or is it that the pipe is one-way, but chardev insists on O_RDWR and > fails because it is not two-way? The chardev file: backend wants a O_RDONLY file - it won't accept an O_RDWR file in fact, so we must use a pipe with it. > > After more investigation I found it *is* possible to use a socketpair > > and a pipe backend though... > > > > -add-fd set=2,fd=33 -chardev pipe,id=charserial0,path=/dev/fdset/2 > > Yes, a socketpair is bi-directional, so it supports O_RDWR opening. Yep. > > ..because for reasons I don't understand, if QEMU can't open $PATH.in > > and $PATH.out, it'll fallback to just opening $PATH in read-write > > mode even. AFAICT, this is pretty useless with pipes since they > > are unidirectional, but, it works nicely with socketpairs, where > > virtlogd has one of the socketpairs and QEMU gets passed the other > > via fdset. > > Is it something where we'd want to support two pipes, and open > /dev/fdset/2 tied to char.in and /dev/fdset/3 tied to char.out, where > uni-directional pipes are again okay? In theory we could do, but it would need us to special case the code, as just taking '/dev/fdset/2' and appending '.in' obviously doesn't work. I don't think this really matters though - using a socketpair is just fine. > > I can easily check this works for historical QEMU versions back > > to when fdsets support was added to chardevs, but I'm working if > > the QEMU maintainers consider this usage acceptable over the long > > term, and if so, should we explicitly document it as supported ? > > It seems like a bi-directional socketpair as the single endpoint for a > chardev is useful enough to support and document, but I'm not the > maintainer to give final say-so. > > > > > If not, should we introduce a more explicit syntax for passing in > > a pre-opened FD for chardevs ? eg > > > > -add-fd set=2,fd=33 -chardev fd,id=charserial0,path=/dev/fdset/2 > > > > Difference to the line you tried above: > > > -add-fd set=2,fd=33 -chardev file,id=charserial0,path=/dev/fdset/2 > > is 'fd' instead of 'file'. But if we're going to add a new protocol, do > we even need to go through the "/dev/fdset/..." name, or can we just > pass the fd number directly? > > > Or just make -chardev file,id=charserial0,path=/dev/fdset/2 actually > > work ? > > I'd lean more to this case - the whole point of fdsets was that we don't > have to add multiple fd protocols; that everyone that understood file > syntax and uses qemu_open() magically gained fd support. Yeah, that is a good point about not inventing multiple fd protocols. >From that POV I'd be happy enough if we documented & supported that 'pipe' can be used with a socketpair, and 'file' can be used with an pipe (once append=true support added) > > eg should we make something like this work: > > > > -add-fd set=2,fd=33 > > -chardev pipe,id=charserial0file,path=/dev/fdset/2 > > -chardev socket,id=charserial0tcp,host=127.0.0.1,port=9999,telnet,server,nowait > > -chardev multiplex,id=charserial0,muxA=charserial0file,muxB=charserial1 > > wouldn't muxB be charserial0tcp (not charserial1)? Yes, silly typo. > > > -serial isa-serial,chardev=charserial0,id=serial0 > > But the idea of a multiplex protocol that has multiple data sinks (guest > output copied to all sinks) and a single source (at most one source can > provide input to the guest) makes sense on the surface. 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
