Hi On Tue, Jul 26, 2022 at 12:44 PM Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote: > > On Tue, Jul 26, 2022 at 12:32:32PM +0400, marcandre.lureau@xxxxxxxxxx wrote: > > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx> > > > > As Ed Swierk explained back in 2006: > > https://lists.nongnu.org/archive/html/qemu-devel/2006-12/msg00160.html > > > > "When qemu writes into the pipe, it immediately reads back what it just > > wrote and treats it as a monitor command, endlessly breathing its own > > exhaust." > > > > This is similarly confusing when using the chardev with a serial device, > > as reported in https://bugzilla.redhat.com/show_bug.cgi?id=2106975. > > > > It seems we have kept the support for bidirectional pipes for historical > > reasons and odd systems, however it's not documented in qemu -chardev > > options. I suggest to stop supporting it, for portability reasons. > > Hmm, I always assumed that in this scenario the pipe was operating > in output-only mode. Obviously not the case with the code as it > exists, but perhaps this would be useful ? eg its good as a serial > console logging mechanism at least. The current "-chardev pipe,id=id,path=path" option handling will first check the presence of unidirectional "path.in" & "path.out" (although they are opened RDWR...), and fallback on bidirectional "path". We could allow for the presence of "path.out" alone, although this may be a behaviour/breaking change: diff --git a/chardev/char-pipe.c b/chardev/char-pipe.c index 7db963035e..f78bcd7daf 100644 --- a/chardev/char-pipe.c +++ b/chardev/char-pipe.c @@ -137,12 +137,12 @@ static void qemu_chr_open_pipe(Chardev *chr, g_free(filename_in); g_free(filename_out); if (fd_in < 0 || fd_out < 0) { + if (fd_out >= 0) { + goto out; + } if (fd_in >= 0) { close(fd_in); } - if (fd_out >= 0) { - close(fd_out); - } warn_report("Support for bidirectional pipe is deprecated,"); warn_report("please use portable one-way pipes instead (%s.in & %s.out).", filename, filename); @@ -152,6 +152,7 @@ static void qemu_chr_open_pipe(Chardev *chr, return; } } +out: qemu_chr_open_fd(chr, fd_in, fd_out); or we introduce a new "access=write" option, or a new chardev type ? > > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx> > > --- > > docs/about/deprecated.rst | 6 ++++++ > > chardev/char-pipe.c | 4 ++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > > index 7ee26626d5cf..dd5ca30d527b 100644 > > --- a/docs/about/deprecated.rst > > +++ b/docs/about/deprecated.rst > > @@ -45,6 +45,12 @@ transmit audio through the VNC protocol. > > ``tty`` and ``parport`` are aliases that will be removed. Instead, the > > actual backend names ``serial`` and ``parallel`` should be used. > > > > +``-chardev pipe`` support for bidirectional pipes (since 7.1) > > +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' > > + > > +For portability reasons, the support for bidirectional ``pipe`` will > > +be removed. Instead, use ``file.in`` & ``file.out`` unidirectional pipes. > > + > > Short-form boolean options (since 6.0) > > '''''''''''''''''''''''''''''''''''''' > > > > diff --git a/chardev/char-pipe.c b/chardev/char-pipe.c > > index 66d3b8509183..7db963035e7d 100644 > > --- a/chardev/char-pipe.c > > +++ b/chardev/char-pipe.c > > @@ -27,6 +27,7 @@ > > #include "qemu/main-loop.h" > > #include "qemu/module.h" > > #include "qemu/option.h" > > +#include "qemu/error-report.h" > > #include "chardev/char.h" > > > > #ifdef _WIN32 > > @@ -142,6 +143,9 @@ static void qemu_chr_open_pipe(Chardev *chr, > > if (fd_out >= 0) { > > close(fd_out); > > } > > + warn_report("Support for bidirectional pipe is deprecated,"); > > + warn_report("please use portable one-way pipes instead (%s.in & %s.out).", > > + filename, filename); > > TFR(fd_in = fd_out = qemu_open_old(filename, O_RDWR | O_BINARY)); > > if (fd_in < 0) { > > error_setg_file_open(errp, errno, filename); > > -- > > 2.37.0.rc0 > > > > With regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| >
