| Message ID | 20240806010735.2450555-1-tavip@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] chardev: add path option for pty backend | expand |
On Tue, Aug 6, 2024 at 5:09 AM Octavian Purdila <tavip@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-ptsneves@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 > > # check QMP invocation with path set > qemu-system-x86_64 -nodefaults -qmp tcp:localhost:4444,server=on,wait=off > nc localhost 4444 > > {"execute": "qmp_capabilities"} > > {"execute": "chardev-add", "arguments": {"id": "bar", "backend": { > "type": "pty", "data": {"path": "test" }}}} > > # check QMP invocation with path not set > qemu-system-x86_64 -nodefaults -qmp tcp:localhost:4444,server=on,wait=off > nc localhost 4444 > > {"execute": "qmp_capabilities"} > > {"execute": "chardev-add", "arguments": {"id": "bar", "backend": { > "type": "pty", "data": {}}}} > > 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 <ptsneves@gmail.com> > Signed-off-by: Paulo Neves <ptsneves@gmail.com> > [OP: rebase and address original patch review comments] > Signed-off-by: Octavian Purdila <tavip@google.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > Changes since v3: > > * update documentation with note about user's resposability to check > and remove the symlink since it can't be removed on crashes or > certain startup errors > > * use a dedicated ChardevPty structure instead of relying on > ChardevHostdev > > Changes since v2: > > * remove NULL path check, g_strdup() allows NULL inputs > > 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 | 27 ++++++++++++++++++++++++++- > qemu-options.hx | 33 +++++++++++++++++++++++++++------ > 4 files changed, 91 insertions(+), 7 deletions(-) > > diff --git a/chardev/char-pty.c b/chardev/char-pty.c > index cc2f7617fe..cbb21b76ae 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 *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->path) { > + unlink(s->path); > + g_free(s->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 *path = backend->u.pty.data->path; > > 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 (path) { > + int res = symlink(pty_name, path); > + > + if (res != 0) { > + error_setg_errno(errp, errno, "Failed to create PTY symlink"); > + } else { > + s->path = g_strdup(path); > + } > + } > +} > + > +static void char_pty_parse(QemuOpts *opts, ChardevBackend *backend, > + Error **errp) > +{ > + const char *path = qemu_opt_get(opts, "path"); > + ChardevPty *pty; > + > + backend->type = CHARDEV_BACKEND_KIND_PTY; > + pty = backend->u.pty.data = g_new0(ChardevPty, 1); > + qemu_chr_parse_common(opts, qapi_ChardevPty_base(pty)); > + pty->path = g_strdup(path); > } > > 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..953d519066 100644 > --- a/qapi/char.json > +++ b/qapi/char.json > @@ -431,6 +431,20 @@ > 'base': 'ChardevCommon', > 'if': 'CONFIG_SPICE_PROTOCOL' } > > +## > +# @ChardevPty: > +# > +# Configuration info for pty implementation. > +# > +# @path: optional path to create a symbolic link that points to the > +# allocated PTY > +# > +# Since: 9.2 > +## > +{ 'struct': 'ChardevPty', > + 'data': { '*path': 'str' }, > + 'base': 'ChardevCommon' } > + > ## > # @ChardevBackendKind: > # > @@ -630,6 +644,17 @@ > { 'struct': 'ChardevRingbufWrapper', > 'data': { 'data': 'ChardevRingbuf' } } > > + > +## > +# @ChardevPtyWrapper: > +# > +# @data: Configuration info for pty chardevs > +# > +# Since: 9.2 > +## > +{ 'struct': 'ChardevPtyWrapper', > + 'data': { 'data': 'ChardevPty' } } > + > ## > # @ChardevBackend: > # > @@ -650,7 +675,7 @@ > 'pipe': 'ChardevHostdevWrapper', > 'socket': 'ChardevSocketWrapper', > 'udp': 'ChardevUdpWrapper', > - 'pty': 'ChardevCommonWrapper', > + 'pty': 'ChardevPtyWrapper', > 'null': 'ChardevCommonWrapper', > 'mux': 'ChardevMuxWrapper', > 'msmouse': 'ChardevCommonWrapper', > diff --git a/qemu-options.hx b/qemu-options.hx > index 8ca7f34ef0..08d8f8c559 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,22 @@ 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. > + > + Note that while QEMU will remove the symlink when it exits > + gracefully, it will not do so in case of crashes or on certain > + startup errors. It is recommended that the user checks and removes > + the symlink after qemu terminates to account for this. > + > ``-chardev stdio,id=id[,signal=on|off]`` > Connect to standard input and standard output of the QEMU process. > > @@ -4171,8 +4181,19 @@ 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. > + > + Note that while QEMU will remove the symlink when it exits > + gracefully, it will not do so in case of crashes or on certain > + startup errors. It is recommended that the user checks and > + removes the symlink after qemu terminates to account for this. > > ``none`` > No device is allocated. Note that for machine types which > -- > 2.46.0.rc2.264.g509ed76dc8-goog > > >
On Tue, Sep 3, 2024 at 12:14 AM Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > > > > On Tue, Aug 6, 2024 at 5:09 AM Octavian Purdila <tavip@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-ptsneves@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 >> >> # check QMP invocation with path set >> qemu-system-x86_64 -nodefaults -qmp tcp:localhost:4444,server=on,wait=off >> nc localhost 4444 >> > {"execute": "qmp_capabilities"} >> > {"execute": "chardev-add", "arguments": {"id": "bar", "backend": { >> "type": "pty", "data": {"path": "test" }}}} >> >> # check QMP invocation with path not set >> qemu-system-x86_64 -nodefaults -qmp tcp:localhost:4444,server=on,wait=off >> nc localhost 4444 >> > {"execute": "qmp_capabilities"} >> > {"execute": "chardev-add", "arguments": {"id": "bar", "backend": { >> "type": "pty", "data": {}}}} >> >> 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 <ptsneves@gmail.com> >> Signed-off-by: Paulo Neves <ptsneves@gmail.com> >> [OP: rebase and address original patch review comments] >> Signed-off-by: Octavian Purdila <tavip@google.com> > > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Ping, it looks like this got lost. https://patchew.org/QEMU/20240605185050.1678102-1-tavip@google.com/
diff --git a/chardev/char-pty.c b/chardev/char-pty.c index cc2f7617fe..cbb21b76ae 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 *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->path) { + unlink(s->path); + g_free(s->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 *path = backend->u.pty.data->path; 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 (path) { + int res = symlink(pty_name, path); + + if (res != 0) { + error_setg_errno(errp, errno, "Failed to create PTY symlink"); + } else { + s->path = g_strdup(path); + } + } +} + +static void char_pty_parse(QemuOpts *opts, ChardevBackend *backend, + Error **errp) +{ + const char *path = qemu_opt_get(opts, "path"); + ChardevPty *pty; + + backend->type = CHARDEV_BACKEND_KIND_PTY; + pty = backend->u.pty.data = g_new0(ChardevPty, 1); + qemu_chr_parse_common(opts, qapi_ChardevPty_base(pty)); + pty->path = g_strdup(path); } 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..953d519066 100644 --- a/qapi/char.json +++ b/qapi/char.json @@ -431,6 +431,20 @@ 'base': 'ChardevCommon', 'if': 'CONFIG_SPICE_PROTOCOL' } +## +# @ChardevPty: +# +# Configuration info for pty implementation. +# +# @path: optional path to create a symbolic link that points to the +# allocated PTY +# +# Since: 9.2 +## +{ 'struct': 'ChardevPty', + 'data': { '*path': 'str' }, + 'base': 'ChardevCommon' } + ## # @ChardevBackendKind: # @@ -630,6 +644,17 @@ { 'struct': 'ChardevRingbufWrapper', 'data': { 'data': 'ChardevRingbuf' } } + +## +# @ChardevPtyWrapper: +# +# @data: Configuration info for pty chardevs +# +# Since: 9.2 +## +{ 'struct': 'ChardevPtyWrapper', + 'data': { 'data': 'ChardevPty' } } + ## # @ChardevBackend: # @@ -650,7 +675,7 @@ 'pipe': 'ChardevHostdevWrapper', 'socket': 'ChardevSocketWrapper', 'udp': 'ChardevUdpWrapper', - 'pty': 'ChardevCommonWrapper', + 'pty': 'ChardevPtyWrapper', 'null': 'ChardevCommonWrapper', 'mux': 'ChardevMuxWrapper', 'msmouse': 'ChardevCommonWrapper', diff --git a/qemu-options.hx b/qemu-options.hx index 8ca7f34ef0..08d8f8c559 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,22 @@ 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. + + Note that while QEMU will remove the symlink when it exits + gracefully, it will not do so in case of crashes or on certain + startup errors. It is recommended that the user checks and removes + the symlink after qemu terminates to account for this. + ``-chardev stdio,id=id[,signal=on|off]`` Connect to standard input and standard output of the QEMU process. @@ -4171,8 +4181,19 @@ 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. + + Note that while QEMU will remove the symlink when it exits + gracefully, it will not do so in case of crashes or on certain + startup errors. It is recommended that the user checks and + removes the symlink after qemu terminates to account for this. ``none`` No device is allocated. Note that for machine types which