[PATCH] RFC: char: deprecate usage of bidirectional pipe
Marc-André Lureau
marcandre.lureau at redhat.com
Fri Aug 5 09:55:41 UTC 2022
Hi
On Tue, Jul 26, 2022 at 12:44 PM Daniel P. Berrangé <berrange at redhat.com> wrote:
>
> On Tue, Jul 26, 2022 at 12:32:32PM +0400, marcandre.lureau at redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> >
> > 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 at redhat.com>
> > ---
> > 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 :|
>
More information about the libvir-list
mailing list