On Fri, Aug 05, 2022 at 01:55:41PM +0400, Marc-André Lureau wrote: > 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: If we allow path.out alone, then we loose error diagnostic when path.out is succesfully opened, but path.in fails. I wouldn't call that a back compat breakage. My preference would always be to use the exact path that was given as the CLI parameter. IOW, we really ought to have had -chardev pipe,id=id,input=path,output=path and allowed both of them to be optional, eg both of them should semantically mean /dev/null in behavioural terms if omitted IOW we could just deprecate 'path' entirely and introduce this saner approach to config. Alternatively, I would just unconditionally change diff --git a/chardev/char-pipe.c b/chardev/char-pipe.c index 66d3b85091..3dda3d5cc6 100644 --- a/chardev/char-pipe.c +++ b/chardev/char-pipe.c @@ -142,7 +142,7 @@ static void qemu_chr_open_pipe(Chardev *chr, if (fd_out >= 0) { close(fd_out); } - TFR(fd_in = fd_out = qemu_open_old(filename, O_RDWR | O_BINARY)); + TFR(fd_in = fd_out = qemu_open_old(filename, O_WRONLY | O_BINARY)); if (fd_in < 0) { error_setg_file_open(errp, errno, filename); return; given that semantics on any UNIX platform we target are for pipes to be unidirectional, and eating our own output is uselessly broken, we could reasonably justify doing that change simply as a bug fix and ignore any notion of 'deprecation', 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 :|
