Re: [Qemu-devel] [RFC v3 PATCH 40/45] multi-process/mon: Refactor monitor/chardev functions out of vl.c

[Jag Raman]

On 9/4/2019 4:37 AM, Dr. David Alan Gilbert wrote:
> * Jagannathan Raman (address@hidden) wrote:
> > Some of the initialization helper functions w.r.t monitor & chardev
> > in vl.c are also used by the remote process. Therefore, these functions
> > are refactored into a shared file that both QEMU & remote process
> > could use.
> >
> > Signed-off-by: Elena Ufimtseva <address@hidden>
> > Signed-off-by: John G Johnson <address@hidden>
> > Signed-off-by: Jagannathan Raman <address@hidden>
>
> Shouldn't the monitor parts be moved into something under monitor/ and
> the chardev parts be moved into chardev/ ?

Sure, will do.

Thanks!
--
Jag

> Dave
>
> > ---
> >   New patch in v3
> >
> >   vl-parse.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   vl.c       | 91 -------------------------------------------------------------
> >   vl.h       |  4 +++
> >   3 files changed, 96 insertions(+), 91 deletions(-)
> >
> > diff --git a/vl-parse.c b/vl-parse.c
> > index d3716d1..bee904d 100644
> > --- a/vl-parse.c
> > +++ b/vl-parse.c
> > @@ -155,3 +155,95 @@ int device_init_func(void *opaque, QemuOpts *opts, Error 
> > **errp)
> >       object_unref(OBJECT(dev));
> >       return 0;
> >   }
> > +
> > +void monitor_parse(const char *optarg, const char *mode, bool pretty)
> > +{
> > +    static int monitor_device_index;
> > +    QemuOpts *opts;
> > +    const char *p;
> > +    char label[32];
> > +
> > +    if (strstart(optarg, "chardev:", &p)) {
> > +        snprintf(label, sizeof(label), "%s", p);
> > +    } else {
> > +        snprintf(label, sizeof(label), "compat_monitor%d",
> > +                 monitor_device_index);
> > +        opts = qemu_chr_parse_compat(label, optarg, true);
> > +        if (!opts) {
> > +            error_report("parse error: %s", optarg);
> > +            exit(1);
> > +        }
> > +    }
> > +
> > +    opts = qemu_opts_create(qemu_find_opts("mon"), label, 1, &error_fatal);
> > +    qemu_opt_set(opts, "mode", mode, &error_abort);
> > +    qemu_opt_set(opts, "chardev", label, &error_abort);
> > +    if (!strcmp(mode, "control")) {
> > +        qemu_opt_set_bool(opts, "pretty", pretty, &error_abort);
> > +    } else {
> > +        assert(pretty == false);
> > +    }
> > +    monitor_device_index++;
> > +}
> > +
> > +int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
> > +{
> > +    Chardev *chr;
> > +    bool qmp;
> > +    bool pretty = false;
> > +    const char *chardev;
> > +    const char *mode;
> > +
> > +    mode = qemu_opt_get(opts, "mode");
> > +    if (mode == NULL) {
> > +        mode = "readline";
> > +    }
> > +    if (strcmp(mode, "readline") == 0) {
> > +        qmp = false;
> > +    } else if (strcmp(mode, "control") == 0) {
> > +        qmp = true;
> > +    } else {
> > +        error_setg(errp, "unknown monitor mode \"%s\"", mode);
> > +        return -1;
> > +    }
> > +
> > +    if (!qmp && qemu_opt_get(opts, "pretty")) {
> > +        warn_report("'pretty' is deprecated for HMP monitors, it has no effect 
> > "
> > +                    "and will be removed in future versions");
> > +    }
> > +    if (qemu_opt_get_bool(opts, "pretty", 0)) {
> > +        pretty = true;
> > +    }
> > +
> > +    chardev = qemu_opt_get(opts, "chardev");
> > +    if (!chardev) {
> > +        error_report("chardev is required");
> > +        exit(1);
> > +    }
> > +    chr = qemu_chr_find(chardev);
> > +    if (chr == NULL) {
> > +        error_setg(errp, "chardev \"%s\" not found", chardev);
> > +        return -1;
> > +    }
> > +
> > +    if (qmp) {
> > +        monitor_init_qmp(chr, pretty);
> > +    } else {
> > +        monitor_init_hmp(chr, true);
> > +    }
> > +    return 0;
> > +}
> > +
> > +int chardev_init_func(void *opaque, QemuOpts *opts, Error **errp)
> > +{
> > +    Error *local_err = NULL;
> > +
> > +    if (!qemu_chr_new_from_opts(opts, NULL, &local_err)) {
> > +        if (local_err) {
> > +            error_propagate(errp, local_err);
> > +            return -1;
> > +        }
> > +        exit(0);
> > +    }
> > +    return 0;
> > +}
> > diff --git a/vl.c b/vl.c
> > index 08e9c09..3c03405 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2264,19 +2264,6 @@ static int device_help_func(void *opaque, QemuOpts 
> > *opts, Error **errp)
> >       return qdev_device_help(opts);
> >   }
> >   
> > -static int chardev_init_func(void *opaque, QemuOpts *opts, Error **errp)
> > -{
> > -    Error *local_err = NULL;
> > -
> > -    if (!qemu_chr_new_from_opts(opts, NULL, &local_err)) {
> > -        if (local_err) {
> > -            error_propagate(errp, local_err);
> > -            return -1;
> > -        }
> > -        exit(0);
> > -    }
> > -    return 0;
> > -}
> >   
> >   #ifdef CONFIG_VIRTFS
> >   static int fsdev_init_func(void *opaque, QemuOpts *opts, Error **errp)
> > @@ -2285,84 +2272,6 @@ static int fsdev_init_func(void *opaque, QemuOpts *opts, 
> > Error **errp)
> >   }
> >   #endif
> >   
> > -static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
> > -{
> > -    Chardev *chr;
> > -    bool qmp;
> > -    bool pretty = false;
> > -    const char *chardev;
> > -    const char *mode;
> > -
> > -    mode = qemu_opt_get(opts, "mode");
> > -    if (mode == NULL) {
> > -        mode = "readline";
> > -    }
> > -    if (strcmp(mode, "readline") == 0) {
> > -        qmp = false;
> > -    } else if (strcmp(mode, "control") == 0) {
> > -        qmp = true;
> > -    } else {
> > -        error_setg(errp, "unknown monitor mode \"%s\"", mode);
> > -        return -1;
> > -    }
> > -
> > -    if (!qmp && qemu_opt_get(opts, "pretty")) {
> > -        warn_report("'pretty' is deprecated for HMP monitors, it has no effect 
> > "
> > -                    "and will be removed in future versions");
> > -    }
> > -    if (qemu_opt_get_bool(opts, "pretty", 0)) {
> > -        pretty = true;
> > -    }
> > -
> > -    chardev = qemu_opt_get(opts, "chardev");
> > -    if (!chardev) {
> > -        error_report("chardev is required");
> > -        exit(1);
> > -    }
> > -    chr = qemu_chr_find(chardev);
> > -    if (chr == NULL) {
> > -        error_setg(errp, "chardev \"%s\" not found", chardev);
> > -        return -1;
> > -    }
> > -
> > -    if (qmp) {
> > -        monitor_init_qmp(chr, pretty);
> > -    } else {
> > -        monitor_init_hmp(chr, true);
> > -    }
> > -    return 0;
> > -}
> > -
> > -static void monitor_parse(const char *optarg, const char *mode, bool pretty)
> > -{
> > -    static int monitor_device_index = 0;
> > -    QemuOpts *opts;
> > -    const char *p;
> > -    char label[32];
> > -
> > -    if (strstart(optarg, "chardev:", &p)) {
> > -        snprintf(label, sizeof(label), "%s", p);
> > -    } else {
> > -        snprintf(label, sizeof(label), "compat_monitor%d",
> > -                 monitor_device_index);
> > -        opts = qemu_chr_parse_compat(label, optarg, true);
> > -        if (!opts) {
> > -            error_report("parse error: %s", optarg);
> > -            exit(1);
> > -        }
> > -    }
> > -
> > -    opts = qemu_opts_create(qemu_find_opts("mon"), label, 1, &error_fatal);
> > -    qemu_opt_set(opts, "mode", mode, &error_abort);
> > -    qemu_opt_set(opts, "chardev", label, &error_abort);
> > -    if (!strcmp(mode, "control")) {
> > -        qemu_opt_set_bool(opts, "pretty", pretty, &error_abort);
> > -    } else {
> > -        assert(pretty == false);
> > -    }
> > -    monitor_device_index++;
> > -}
> > -
> >   struct device_config {
> >       enum {
> >           DEV_USB,       /* -usbdevice     */
> > diff --git a/vl.h b/vl.h
> > index 8c40fed..21ca747 100644
> > --- a/vl.h
> > +++ b/vl.h
> > @@ -50,5 +50,9 @@ int device_init_func(void *opaque, QemuOpts *opts, Error 
> > **errp);
> >   int rdrive_init_func(void *opaque, QemuOpts *opts, Error **errp);
> >   int rdevice_init_func(void *opaque, QemuOpts *opts, Error **errp);
> >   
> > +void monitor_parse(const char *optarg, const char *mode, bool pretty);
> > +int mon_init_func(void *opaque, QemuOpts *opts, Error **errp);
> > +int chardev_init_func(void *opaque, QemuOpts *opts, Error **errp);
> > +
> >   #endif /* VL_H */
> >   
> > --
> > 1.8.3.1
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK