Daniel P. Berrangé <berra...@redhat.com> writes: > On Thu, Jul 18, 2024 at 08:15:01AM +0200, Markus Armbruster wrote: >> Looks like this one fell through the cracks. >> >> Octavian Purdila <ta...@google.com> writes: >> >> > 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. >> >> QMP commands chardev-add and chardev-change return the information you >> want: >> >> # @pty: name of the slave pseudoterminal device, present if and only >> # if a chardev of type 'pty' was created >> >> So does HMP command chardev-add. HMP chardev apparently doesn't, but >> that could be fixed. > > It does print it: > > (qemu) chardev-add pty,id=bar > char device redirected to /dev/pts/12 (label bar)
I fat-fingered "HMP chardev-change". >> So, the use case is basically the command line, right? > > Also cli prints it > > $ qemu-system-x86_64 -chardev pty,id=foo -monitor stdio -display none > char device redirected to /dev/pts/10 (label foo) Good enough for ad hoc use by humans. Management applications should use QMP, which returns it. I guess there's scripts in between. >> > 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> [...] >> > diff --git a/chardev/char-pty.c b/chardev/char-pty.c >> > index cc2f7617fe..5c6172ddba 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); >> > + } >> >> Runs when the chardev object is finalized. >> >> Doesn't run when QEMU crashes. Stale symlink left behind then. Can't >> see how you could avoid that at reasonable cost. Troublesome all the >> same. > > Do we ever guarantee that the finalizer runs ? eg dif we have > > error_setg(&error_exit, .... > > that's a clean exit, not a crash, but I don't think chardev finalizers > will run, as we don't do atexit() hooks for it. Point. >> The feature feels rather doubtful to me, to be honest. > > On the one hand I understand the pain - long ago libvirt had to deal > with parsing the console messages > > char device redirected to /dev/pts/10 (label foo) > > before we switched to using QMP to query this. > > On the other hand, in retrospect libvirt should never have used the 'pty' > backend in the first place. The 'unix' socket backend is a choice as it > has predictable filenames, and it has proper connection oriented semantics, > so QEMU can reliably detect when clients disconnect, which has always been > troublesome for the 'pty' backend. > > So while I can understand the desire to add a 'path' option to 'pty' > to trigger symlink creation, I think we could choose to tell people > to use the 'unix' socket backend instead if they want a predictable > path. This would avoid us creating the difficult to fix bug for > symlink deletion in error conditions. > > What's the key benefit of the 'pty' backend, that 'unix' doesn't > handle ? I think this is the question to answer.
