diff mbox series

[v3,09/15] monitor: Create monitor-internal.h with common definitions

Message ID 20190613153405.24769-10-kwolf@redhat.com
State New
Headers show
Series monitor: Split monitor.c in core/HMP/QMP/misc | expand

Commit Message

Kevin Wolf June 13, 2019, 3:33 p.m. UTC 
Before we can split monitor/misc.c, we need to create a header file that
contains the common definitions that will be used by multiple source
files.

For a start, add the type definitions for Monitor, MonitorHMP and
MonitorQMP and their dependencies. We'll add functions as needed when
splitting monitor/misc.c.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 monitor/monitor-internal.h | 148 +++++++++++++++++++++++++++++++++++++
 monitor/misc.c             | 110 +--------------------------
 MAINTAINERS                |   2 +
 3 files changed, 151 insertions(+), 109 deletions(-)
 create mode 100644 monitor/monitor-internal.h

Comments

Markus Armbruster June 14, 2019, 6:37 a.m. UTC | #1 
Kevin Wolf <kwolf@redhat.com> writes:

> Before we can split monitor/misc.c, we need to create a header file that
> contains the common definitions that will be used by multiple source
> files.
>
> For a start, add the type definitions for Monitor, MonitorHMP and
> MonitorQMP and their dependencies. We'll add functions as needed when
> splitting monitor/misc.c.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  monitor/monitor-internal.h | 148 +++++++++++++++++++++++++++++++++++++
>  monitor/misc.c             | 110 +--------------------------
>  MAINTAINERS                |   2 +
>  3 files changed, 151 insertions(+), 109 deletions(-)
>  create mode 100644 monitor/monitor-internal.h
>
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> new file mode 100644
> index 0000000000..17a632b0ad
> --- /dev/null
> +++ b/monitor/monitor-internal.h
> @@ -0,0 +1,148 @@
> +/*
> + * QEMU monitor
> + *
> + * Copyright (c) 2003-2004 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef MONITOR_INT_H
> +#define MONITOR_INT_H

Rename to MONITOR_INTERNAL_H, so it again matches the file name.  Can
touch up in my tree.

> +
> +#include "monitor/monitor.h"
> +#include "qapi/qmp/qdict.h"

These too are superfluous.  I'm willing to tolerate monitor.h anyway,
since anything including monitor-internal.h is almost certainly going to
need monitor.h, too.

> +#include "qapi/qmp/json-parser.h"
> +#include "qapi/qmp/dispatch.h"
> +#include "qapi/qapi-types-misc.h"
> +
> +#include "qemu/readline.h"
> +#include "chardev/char-fe.h"
> +#include "sysemu/iothread.h"

Another superfluous one.

Happy to drop these two #include in my tree.

[...]

With that:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Kevin Wolf June 14, 2019, 8:47 a.m. UTC | #2 
Am 14.06.2019 um 08:37 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Before we can split monitor/misc.c, we need to create a header file that
> > contains the common definitions that will be used by multiple source
> > files.
> >
> > For a start, add the type definitions for Monitor, MonitorHMP and
> > MonitorQMP and their dependencies. We'll add functions as needed when
> > splitting monitor/misc.c.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  monitor/monitor-internal.h | 148 +++++++++++++++++++++++++++++++++++++
> >  monitor/misc.c             | 110 +--------------------------
> >  MAINTAINERS                |   2 +
> >  3 files changed, 151 insertions(+), 109 deletions(-)
> >  create mode 100644 monitor/monitor-internal.h
> >
> > diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> > new file mode 100644
> > index 0000000000..17a632b0ad
> > --- /dev/null
> > +++ b/monitor/monitor-internal.h
> > @@ -0,0 +1,148 @@
> > +/*
> > + * QEMU monitor
> > + *
> > + * Copyright (c) 2003-2004 Fabrice Bellard
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a copy
> > + * of this software and associated documentation files (the "Software"), to deal
> > + * in the Software without restriction, including without limitation the rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> > + * THE SOFTWARE.
> > + */
> > +
> > +#ifndef MONITOR_INT_H
> > +#define MONITOR_INT_H
> 
> Rename to MONITOR_INTERNAL_H, so it again matches the file name.  Can
> touch up in my tree.

Oops, yes, please do.

> > +
> > +#include "monitor/monitor.h"
> > +#include "qapi/qmp/qdict.h"
> 
> These too are superfluous.  I'm willing to tolerate monitor.h anyway,
> since anything including monitor-internal.h is almost certainly going to
> need monitor.h, too.

I tried to drop them because you suggested so, but it results in compile
errors. On closer look, I think qdict.h can go because the typedef will
be present through qemu/osdep.h, which must be included before this one,
but MonitorHMP is only defined by monitor/monitor.h.

> > +#include "qapi/qmp/json-parser.h"
> > +#include "qapi/qmp/dispatch.h"
> > +#include "qapi/qapi-types-misc.h"
> > +
> > +#include "qemu/readline.h"
> > +#include "chardev/char-fe.h"
> > +#include "sysemu/iothread.h"
> 
> Another superfluous one.

IOThread is only defined by system/iothread.h, so I don't think you can
remove this one either.

> Happy to drop these two #include in my tree.

As long as the result still builds, feel free to drop includes from the
header (and probably add them to source files instead where they will be
missing then).

Kevin
diff mbox series

Patch

diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
new file mode 100644
index 0000000000..17a632b0ad
--- /dev/null
+++ b/monitor/monitor-internal.h
@@ -0,0 +1,148 @@ 
+/*
+ * QEMU monitor
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef MONITOR_INT_H
+#define MONITOR_INT_H
+
+#include "monitor/monitor.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/json-parser.h"
+#include "qapi/qmp/dispatch.h"
+#include "qapi/qapi-types-misc.h"
+
+#include "qemu/readline.h"
+#include "chardev/char-fe.h"
+#include "sysemu/iothread.h"
+
+/*
+ * Supported types:
+ *
+ * 'F'          filename
+ * 'B'          block device name
+ * 's'          string (accept optional quote)
+ * 'S'          it just appends the rest of the string (accept optional quote)
+ * 'O'          option string of the form NAME=VALUE,...
+ *              parsed according to QemuOptsList given by its name
+ *              Example: 'device:O' uses qemu_device_opts.
+ *              Restriction: only lists with empty desc are supported
+ *              TODO lift the restriction
+ * 'i'          32 bit integer
+ * 'l'          target long (32 or 64 bit)
+ * 'M'          Non-negative target long (32 or 64 bit), in user mode the
+ *              value is multiplied by 2^20 (think Mebibyte)
+ * 'o'          octets (aka bytes)
+ *              user mode accepts an optional E, e, P, p, T, t, G, g, M, m,
+ *              K, k suffix, which multiplies the value by 2^60 for suffixes E
+ *              and e, 2^50 for suffixes P and p, 2^40 for suffixes T and t,
+ *              2^30 for suffixes G and g, 2^20 for M and m, 2^10 for K and k
+ * 'T'          double
+ *              user mode accepts an optional ms, us, ns suffix,
+ *              which divides the value by 1e3, 1e6, 1e9, respectively
+ * '/'          optional gdb-like print format (like "/10x")
+ *
+ * '?'          optional type (for all types, except '/')
+ * '.'          other form of optional type (for 'i' and 'l')
+ * 'b'          boolean
+ *              user mode accepts "on" or "off"
+ * '-'          optional parameter (eg. '-f')
+ *
+ */
+
+typedef struct HMPCommand {
+    const char *name;
+    const char *args_type;
+    const char *params;
+    const char *help;
+    const char *flags; /* p=preconfig */
+    void (*cmd)(Monitor *mon, const QDict *qdict);
+    /*
+     * @sub_table is a list of 2nd level of commands. If it does not exist,
+     * cmd should be used. If it exists, sub_table[?].cmd should be
+     * used, and cmd of 1st level plays the role of help function.
+     */
+    struct HMPCommand *sub_table;
+    void (*command_completion)(ReadLineState *rs, int nb_args, const char *str);
+} HMPCommand;
+
+struct Monitor {
+    CharBackend chr;
+    int reset_seen;
+    int flags;
+    int suspend_cnt;            /* Needs to be accessed atomically */
+    bool skip_flush;
+    bool use_io_thread;
+
+    gchar *mon_cpu_path;
+    QTAILQ_ENTRY(Monitor) entry;
+
+    /*
+     * The per-monitor lock. We can't access guest memory when holding
+     * the lock.
+     */
+    QemuMutex mon_lock;
+
+    /*
+     * Members that are protected by the per-monitor lock
+     */
+    QLIST_HEAD(, mon_fd_t) fds;
+    QString *outbuf;
+    guint out_watch;
+    /* Read under either BQL or mon_lock, written with BQL+mon_lock.  */
+    int mux_out;
+};
+
+struct MonitorHMP {
+    Monitor common;
+    /*
+     * State used only in the thread "owning" the monitor.
+     * If @use_io_thread, this is @mon_iothread. (This does not actually happen
+     * in the current state of the code.)
+     * Else, it's the main thread.
+     * These members can be safely accessed without locks.
+     */
+    ReadLineState *rs;
+};
+
+typedef struct {
+    Monitor common;
+    JSONMessageParser parser;
+    /*
+     * When a client connects, we're in capabilities negotiation mode.
+     * @commands is &qmp_cap_negotiation_commands then.  When command
+     * qmp_capabilities succeeds, we go into command mode, and
+     * @command becomes &qmp_commands.
+     */
+    QmpCommandList *commands;
+    bool capab_offered[QMP_CAPABILITY__MAX]; /* capabilities offered */
+    bool capab[QMP_CAPABILITY__MAX];         /* offered and accepted */
+    /*
+     * Protects qmp request/response queue.
+     * Take monitor_lock first when you need both.
+     */
+    QemuMutex qmp_queue_lock;
+    /* Input queue that holds all the parsed QMP requests */
+    GQueue *qmp_requests;
+} MonitorQMP;
+
+#endif
diff --git a/monitor/misc.c b/monitor/misc.c
index e5db04265d..6aa4a21676 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -23,6 +23,7 @@ 
  */
 
 #include "qemu/osdep.h"
+#include "monitor-internal.h"
 #include "qemu/units.h"
 #include <dirent.h>
 #include "cpu.h"
@@ -93,55 +94,6 @@ 
 #include "hw/s390x/storage-attributes.h"
 #endif
 
-/*
- * Supported types:
- *
- * 'F'          filename
- * 'B'          block device name
- * 's'          string (accept optional quote)
- * 'S'          it just appends the rest of the string (accept optional quote)
- * 'O'          option string of the form NAME=VALUE,...
- *              parsed according to QemuOptsList given by its name
- *              Example: 'device:O' uses qemu_device_opts.
- *              Restriction: only lists with empty desc are supported
- *              TODO lift the restriction
- * 'i'          32 bit integer
- * 'l'          target long (32 or 64 bit)
- * 'M'          Non-negative target long (32 or 64 bit), in user mode the
- *              value is multiplied by 2^20 (think Mebibyte)
- * 'o'          octets (aka bytes)
- *              user mode accepts an optional E, e, P, p, T, t, G, g, M, m,
- *              K, k suffix, which multiplies the value by 2^60 for suffixes E
- *              and e, 2^50 for suffixes P and p, 2^40 for suffixes T and t,
- *              2^30 for suffixes G and g, 2^20 for M and m, 2^10 for K and k
- * 'T'          double
- *              user mode accepts an optional ms, us, ns suffix,
- *              which divides the value by 1e3, 1e6, 1e9, respectively
- * '/'          optional gdb-like print format (like "/10x")
- *
- * '?'          optional type (for all types, except '/')
- * '.'          other form of optional type (for 'i' and 'l')
- * 'b'          boolean
- *              user mode accepts "on" or "off"
- * '-'          optional parameter (eg. '-f')
- *
- */
-
-typedef struct HMPCommand {
-    const char *name;
-    const char *args_type;
-    const char *params;
-    const char *help;
-    const char *flags; /* p=preconfig */
-    void (*cmd)(Monitor *mon, const QDict *qdict);
-    /* @sub_table is a list of 2nd level of commands. If it does not exist,
-     * cmd should be used. If it exists, sub_table[?].cmd should be
-     * used, and cmd of 1st level plays the role of help function.
-     */
-    struct HMPCommand *sub_table;
-    void (*command_completion)(ReadLineState *rs, int nb_args, const char *str);
-} HMPCommand;
-
 /* file descriptors passed via SCM_RIGHTS */
 typedef struct mon_fd_t mon_fd_t;
 struct mon_fd_t {
@@ -184,66 +136,6 @@  typedef struct {
     int64_t rate;       /* Minimum time (in ns) between two events */
 } MonitorQAPIEventConf;
 
-struct Monitor {
-    CharBackend chr;
-    int reset_seen;
-    int flags;
-    int suspend_cnt;            /* Needs to be accessed atomically */
-    bool skip_flush;
-    bool use_io_thread;
-
-    gchar *mon_cpu_path;
-    QTAILQ_ENTRY(Monitor) entry;
-
-    /*
-     * The per-monitor lock. We can't access guest memory when holding
-     * the lock.
-     */
-    QemuMutex mon_lock;
-
-    /*
-     * Members that are protected by the per-monitor lock
-     */
-    QLIST_HEAD(, mon_fd_t) fds;
-    QString *outbuf;
-    guint out_watch;
-    /* Read under either BQL or mon_lock, written with BQL+mon_lock.  */
-    int mux_out;
-};
-
-struct MonitorHMP {
-    Monitor common;
-    /*
-     * State used only in the thread "owning" the monitor.
-     * If @use_io_thread, this is @mon_iothread. (This does not actually happen
-     * in the current state of the code.)
-     * Else, it's the main thread.
-     * These members can be safely accessed without locks.
-     */
-    ReadLineState *rs;
-};
-
-typedef struct {
-    Monitor common;
-    JSONMessageParser parser;
-    /*
-     * When a client connects, we're in capabilities negotiation mode.
-     * @commands is &qmp_cap_negotiation_commands then.  When command
-     * qmp_capabilities succeeds, we go into command mode, and
-     * @command becomes &qmp_commands.
-     */
-    QmpCommandList *commands;
-    bool capab_offered[QMP_CAPABILITY__MAX]; /* capabilities offered */
-    bool capab[QMP_CAPABILITY__MAX];         /* offered and accepted */
-    /*
-     * Protects qmp request/response queue.
-     * Take monitor_lock first when you need both.
-     */
-    QemuMutex qmp_queue_lock;
-    /* Input queue that holds all the parsed QMP requests */
-    GQueue *qmp_requests;
-} MonitorQMP;
-
 /* Shared monitor I/O thread */
 IOThread *mon_iothread;
 
diff --git a/MAINTAINERS b/MAINTAINERS
index 884f4b16bb..ebb839d305 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1916,6 +1916,7 @@  F: qapi/run-state.json
 Human Monitor (HMP)
 M: Dr. David Alan Gilbert <dgilbert@redhat.com>
 S: Maintained
+F: monitor/monitor-internal.h
 F: monitor/misc.c
 F: monitor/hmp*
 F: hmp.h
@@ -2038,6 +2039,7 @@  F: tests/check-qom-proplist.c
 QMP
 M: Markus Armbruster <armbru@redhat.com>
 S: Supported
+F: monitor/monitor-internal.h
 F: monitor/qmp*
 F: monitor/misc.c
 F: docs/devel/*qmp-*