Hi
On Mon, Jun 3, 2024 at 4:23 PM Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Fri, 31 May 2024 at 22:21, Octavian Purdila <ta...@google.com> wrote:
> >
> > Add path option to the pty char backend which will create a symbolic
> > link to the given path that points to the allocated PTY.
> >
> > Based on patch from Paulo Neves:
> >
> > https://patchew.org/QEMU/1548509635-15776-1-git-send-email-ptsne...@gmail.com/
> >
> > Tested with the following invocations that the link is created and
> > removed when qemu stops:
> >
> > qemu-system-x86_64 -nodefaults -mon chardev=compat_monitor \
> > -chardev pty,path=test,id=compat_monitor0
> >
> > qemu-system-x86_64 -nodefaults -monitor pty:test
> >
> > Also tested that when a link path is not passed invocations still work,
> > e.g.:
> >
> > qemu-system-x86_64 -monitor pty
>
> Could we have some justification here for why the new
> functionality is useful, please? (e.g. what new use cases
> it permits).
>
It avoids the need to HMP/QMP query the allocated pty path. I don't
think there are other benefits.
> > --- a/qapi/char.json
> > +++ b/qapi/char.json
> > @@ -509,7 +509,7 @@
> > ##
> > # @ChardevHostdevWrapper:
> > #
> > -# @data: Configuration info for device and pipe chardevs
> > +# @data: Configuration info for device, pty and pipe chardevs
> > #
> > # Since: 1.4
> > ##
> > @@ -650,7 +650,7 @@
> > 'pipe': 'ChardevHostdevWrapper',
> > 'socket': 'ChardevSocketWrapper',
> > 'udp': 'ChardevUdpWrapper',
> > - 'pty': 'ChardevCommonWrapper',
> > + 'pty': 'ChardevHostdevWrapper',
> > 'null': 'ChardevCommonWrapper',
> > 'mux': 'ChardevMuxWrapper',
> > 'msmouse': 'ChardevCommonWrapper',
>
> Does this break QAPI compatibility?
>
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 8ca7f34ef0..5eec194242 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -3569,7 +3569,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> > "-chardev
> > console,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> > "-chardev
> > serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> > #else
> > - "-chardev pty,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> > + "-chardev
> > pty,id=id[,path=path][,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> > "-chardev
> > stdio,id=id[,mux=on|off][,signal=on|off][,logfile=PATH][,logappend=on|off]\n"
> > #endif
> > #ifdef CONFIG_BRLAPI
> > @@ -3808,12 +3808,16 @@ The available backends are:
> >
> > ``path`` specifies the name of the serial device to open.
> >
> > -``-chardev pty,id=id``
> > +``-chardev pty,id=id[,path=path]``
> > Create a new pseudo-terminal on the host and connect to it. ``pty``
> > does not take any options.
>
> We just added an option, so we should delete the line saying
> that it doesn't take any options :-)
>
> >
> > ``pty`` is not available on Windows hosts.
> >
> > + ``path`` specifies the symbolic link path to be created that
> > + points to the pty device.
>
> I think we could usefully make this a little less terse. Perhaps
> If ``path`` is specified, QEMU will create a symbolic link at
> that location which points to the new PTY device.
> ?
>
> thanks
> -- PMM
>
