On Tue, Jun 4, 2024 at 1:22 AM 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.
>
> This avoids having to make QMP or HMP monitor queries to find out what
> the new PTY device path is.
>
> 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
>
> Co-authored-by: Paulo Neves <ptsne...@gmail.com>
> Signed-off-by: Paulo Neves <ptsne...@gmail.com>
> [OP: rebase and address original patch review comments]
> Signed-off-by: Octavian Purdila <ta...@google.com>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> Changes since v1:
>
> * Keep the original Signed-off-by from Paulo and add one line
> description with further changes
>
> * Update commit message with justification for why the new
> functionality is useful
>
> * Don't close master_fd when symlink creation fails to avoid double
> close
>
> * Update documentation for clarity
>
> chardev/char-pty.c | 33 +++++++++++++++++++++++++++++++++
> chardev/char.c | 5 +++++
> qapi/char.json | 4 ++--
> qemu-options.hx | 24 ++++++++++++++++++------
> 4 files changed, 58 insertions(+), 8 deletions(-)
>
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index cc2f7617fe..b5a4eb59fc 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -29,6 +29,7 @@
> #include "qemu/sockets.h"
> #include "qemu/error-report.h"
> #include "qemu/module.h"
> +#include "qemu/option.h"
> #include "qemu/qemu-print.h"
>
> #include "chardev/char-io.h"
> @@ -41,6 +42,7 @@ struct PtyChardev {
>
> int connected;
> GSource *timer_src;
> + char *symlink_path;
> };
> typedef struct PtyChardev PtyChardev;
>
> @@ -204,6 +206,12 @@ static void char_pty_finalize(Object *obj)
> Chardev *chr = CHARDEV(obj);
> PtyChardev *s = PTY_CHARDEV(obj);
>
> + /* unlink symlink */
> + if (s->symlink_path) {
> + unlink(s->symlink_path);
> + g_free(s->symlink_path);
> + }
> +
> pty_chr_state(chr, 0);
> object_unref(OBJECT(s->ioc));
> pty_chr_timer_cancel(s);
> @@ -330,6 +338,7 @@ static void char_pty_open(Chardev *chr,
> int master_fd, slave_fd;
> char pty_name[PATH_MAX];
> char *name;
> + char *symlink_path = backend->u.pty.data->device;
>
> master_fd = qemu_openpty_raw(&slave_fd, pty_name);
> if (master_fd < 0) {
> @@ -354,12 +363,36 @@ static void char_pty_open(Chardev *chr,
> g_free(name);
> s->timer_src = NULL;
> *be_opened = false;
> +
> + /* create symbolic link */
> + if (symlink_path) {
> + int res = symlink(pty_name, symlink_path);
> +
> + if (res != 0) {
> + error_setg_errno(errp, errno, "Failed to create PTY symlink");
> + } else {
> + s->symlink_path = g_strdup(symlink_path);
> + }
> + }
> +}
> +
> +static void char_pty_parse(QemuOpts *opts, ChardevBackend *backend,
> + Error **errp)
> +{
> + const char *path = qemu_opt_get(opts, "path");
> + ChardevHostdev *dev;
> +
> + backend->type = CHARDEV_BACKEND_KIND_PTY;
> + dev = backend->u.pty.data = g_new0(ChardevHostdev, 1);
> + qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(dev));
> + dev->device = path ? g_strdup(path) : NULL;
> }
>
> static void char_pty_class_init(ObjectClass *oc, void *data)
> {
> ChardevClass *cc = CHARDEV_CLASS(oc);
>
> + cc->parse = char_pty_parse;
> cc->open = char_pty_open;
> cc->chr_write = char_pty_chr_write;
> cc->chr_update_read_handler = pty_chr_update_read_handler;
> diff --git a/chardev/char.c b/chardev/char.c
> index 3c43fb1278..404c6b8a4f 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -428,6 +428,11 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const
> char *filename,
> qemu_opt_set(opts, "path", p, &error_abort);
> return opts;
> }
> + if (strstart(filename, "pty:", &p)) {
> + qemu_opt_set(opts, "backend", "pty", &error_abort);
> + qemu_opt_set(opts, "path", p, &error_abort);
> + return opts;
> + }
> if (strstart(filename, "tcp:", &p) ||
> strstart(filename, "telnet:", &p) ||
> strstart(filename, "tn3270:", &p) ||
> diff --git a/qapi/char.json b/qapi/char.json
> index 777dde55d9..4c74bfc437 100644
> --- 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',
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 8ca7f34ef0..94ffb1a605 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,18 @@ The available backends are:
>
> ``path`` specifies the name of the serial device to open.
>
> -``-chardev pty,id=id``
> - Create a new pseudo-terminal on the host and connect to it. ``pty``
> - does not take any options.
> +``-chardev pty,id=id[,path=path]``
> + Create a new pseudo-terminal on the host and connect to it.
>
> ``pty`` is not available on Windows hosts.
>
> + If ``path`` is specified, QEMU will create a symbolic link at
> + that location which points to the new PTY device.
> +
> + This avoids having to make QMP or HMP monitor queries to find out
> + what the new PTY device path is.
> +
> +
> ``-chardev stdio,id=id[,signal=on|off]``
> Connect to standard input and standard output of the QEMU process.
>
> @@ -4171,8 +4177,14 @@ SRST
>
> vc:80Cx24C
>
> - ``pty``
> - [Linux only] Pseudo TTY (a new PTY is automatically allocated)
> + ``pty[:path]``
> + [Linux only] Pseudo TTY (a new PTY is automatically allocated).
> +
> + If ``path`` is specified, QEMU will create a symbolic link at
> + that location which points to the new PTY device.
> +
> + This avoids having to make QMP or HMP monitor queries to find
> + out what the new PTY device path is.
>
> ``none``
> No device is allocated. Note that for machine types which
> --
> 2.45.1.288.g0e0cd299f1-goog
>
