Hi
On Sat, Dec 29, 2018 at 2:34 PM Paulo Neves <ptsne...@gmail.com> wrote:
>
> If a user requires a virtual serial device like provided
> by the pty char device, the user needs to accept the
> returned device name. This makes the program need to
> have smarts to parse or communicate with qemu to get the
> pty device.
> With this patch the program can pass the path where
> a symlink to the pty device will be, removing the
> need for 2 way communication or smarts.
This seems reasonable. Your patch doesn't apply on master, can you rebase it?
> ---
> chardev/char-pty.c | 38 ++++++++++++++++++++++++++++++++++++--
> chardev/char.c | 6 +++++-
> qapi/char.json | 4 ++--
> 3 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index 761ae6d..b465263 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -38,6 +38,7 @@
> typedef struct {
> Chardev parent;
> QIOChannel *ioc;
> + char *link_name;
> int read_bytes;
>
> /* Protected by the Chardev chr_write_lock. */
> @@ -231,6 +232,11 @@ static void char_pty_finalize(Object *obj)
> qemu_mutex_lock(&chr->chr_write_lock);
> pty_chr_state(chr, 0);
> object_unref(OBJECT(s->ioc));
> +
> + if (s->link_name) {
> + unlink(s->link_name);
> + }
> +
I suspect a g_free(s->link_name) is missing
> if (s->timer_tag) {
> g_source_remove(s->timer_tag);
> s->timer_tag = 0;
> @@ -244,8 +250,9 @@ static void char_pty_open(Chardev *chr,
> bool *be_opened,
> Error **errp)
> {
> + ChardevHostdev *opts = backend->u.pty.data;
> PtyChardev *s;
> - int master_fd, slave_fd;
> + int master_fd, slave_fd, symlink_ret;
> char pty_name[PATH_MAX];
> char *name;
>
> @@ -256,13 +263,23 @@ static void char_pty_open(Chardev *chr,
> }
>
> close(slave_fd);
> +
> + s = PTY_CHARDEV(chr)
> + s->link_name = opts->device;
You should g_strdup() it
> + symlink_ret = symlink(pty_name, s->link_name);
> +
> + if (symlink_ret < 0) {
> + close(master_fd);
> + error_setg_errno(errp, errno, "Failed to create symlink to PTY");
> + return;
> + }
> +
> qemu_set_nonblock(master_fd);
>
> chr->filename = g_strdup_printf("pty:%s", pty_name);
> error_report("char device redirected to %s (label %s)",
> pty_name, chr->label);
>
> - s = PTY_CHARDEV(chr);
> s->ioc = QIO_CHANNEL(qio_channel_file_new_fd(master_fd));
> name = g_strdup_printf("chardev-pty-%s", chr->label);
> qio_channel_set_name(QIO_CHANNEL(s->ioc), name);
> @@ -271,6 +288,22 @@ static void char_pty_open(Chardev *chr,
> *be_opened = false;
> }
>
> +static void char_pty_parse(QemuOpts *opts, ChardevBackend *backend,
> + Error **errp)
> +{
> + const char *symlink_path = qemu_opt_get(opts, "path");
> + if(symlink_path == NULL) {
> + error_setg(errp, "chardev: pty symlink: no device path given");
> + return;
> +
> + }
> + ChardevHostdev *dev;
> +
> + backend->type = CHARDEV_BACKEND_KIND_PTY;
> + dev = backend->u.pipe.data = g_new0(ChardevHostdev, 1);
> + qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(dev));
> + dev->device = g_strdup(symlink_path);
> +}
> static void char_pty_class_init(ObjectClass *oc, void *data)
> {
> ChardevClass *cc = CHARDEV_CLASS(oc);
> @@ -279,6 +312,7 @@ static void char_pty_class_init(ObjectClass *oc, void
> *data)
> cc->chr_write = char_pty_chr_write;
> cc->chr_update_read_handler = pty_chr_update_read_handler;
> cc->chr_add_watch = pty_chr_add_watch;
> + cc->parse = char_pty_parse;
> }
>
> static const TypeInfo char_pty_type_info = {
> diff --git a/chardev/char.c b/chardev/char.c
> index 5d52cd5..e4c5371 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -357,7 +357,6 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const
> char *filename)
> }
>
> if (strcmp(filename, "null") == 0 ||
> - strcmp(filename, "pty") == 0 ||
> strcmp(filename, "msmouse") == 0 ||
> strcmp(filename, "wctablet") == 0 ||
> strcmp(filename, "braille") == 0 ||
> @@ -402,6 +401,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 6de0f29..dae4231 100644
> --- a/qapi/char.json
> +++ b/qapi/char.json
> @@ -224,7 +224,7 @@
> ##
> # @ChardevHostdev:
> #
> -# Configuration info for device and pipe chardevs.
> +# Configuration info for device, pty and pipe chardevs.
> #
> # @device: The name of the special file for the device,
> # i.e. /dev/ttyS0 on Unix or COM1: on Windows
> @@ -380,7 +380,7 @@
> 'pipe' : 'ChardevHostdev',
> 'socket' : 'ChardevSocket',
> 'udp' : 'ChardevUdp',
> - 'pty' : 'ChardevCommon',
> + 'pty' : 'ChardevHostdev',
Better to create a new ChardevPty instead (ChardevHostdev is for host
device, here it is not)
> 'null' : 'ChardevCommon',
> 'mux' : 'ChardevMux',
> 'msmouse': 'ChardevCommon',
> --
> 2.7.4
>
>
--
Marc-André Lureau
