monitor.c mixes a lot of different things in a single file: The core
monitor infrastructure, HMP infrastrcture, QMP infrastructure, and the
implementation of several HMP and QMP commands. Almost worse, struct
Monitor mixes state for HMP, for QMP, and state actually shared between
all monitors. monitor.c must be linked with a system emulator and even
requires per-target compilation because some of the commands it
implements access system emulator state.

The reason why I care about this is that I'm working on a protoype for a
storage daemon, which wants to use QMP (but probably not HMP) and
obviously doesn't have any system emulator state. So I'm interested in
some core monitor parts that can be linked to non-system-emulator tools.

This series first creates separate structs MonitorQMP and MonitorHMP
which inherit from Monitor, and then moves the associated infrastructure
code into separate source files.

While the split is probably not perfect, I think it's an improvement of
the current state even for QEMU proper, and it's good enough so I can
link my storage daemon against just monitor/core.o and monitor/qmp.o and
get a useless QMP monitor that parses the JSON input and rejects
everything as an unknown command.

Next I'll try to teach it a subset of QMP commands that can actually be
supported in a tool, but while there will be a few follow-up patches to
achieve this, I don't expect that this work will bring up much that
needs to be changed in the splitting process done in this series.

v2:
- Fix coding style while moving files to make checkpatch happier
- Updated file name references in docs/devel/writing-qmp-commands.txt
- Updated MAINTAINERS for moved and newly created files
- Created monitor/trace-events instead of using the root directory one
- Move {hmp,qmp}.c to monitor/{hmp,qmp}-cmds.c

Kevin Wolf (11):
  monitor: Remove unused password prompting fields
  monitor: Split monitor_init in HMP and QMP function
  monitor: Make MonitorQMP a child class of Monitor
  monitor: Create MonitorHMP with readline state
  monitor: Move cmd_table to MonitorHMP
  Move monitor.c to monitor/misc.c
  monitor: Move {hmp,qmp}.c to monitor/{hmp,qmp}-cmds.c
  monitor: Create monitor_int.h with common definitions
  monitor: Split out monitor/qmp.c
  monitor: Split out monitor/hmp.c
  monitor: Split out monitor/monitor.c

 docs/devel/writing-qmp-commands.txt |   11 +-
 include/monitor/monitor.h           |    8 +-
 monitor/monitor_int.h               |  208 ++
 monitor.c                           | 4727 ---------------------------
 hmp.c => monitor/hmp-cmds.c         |    6 +-
 monitor/hmp.c                       | 1387 ++++++++
 monitor/misc.c                      | 2407 ++++++++++++++
 monitor/monitor.c                   |  605 ++++
 qmp.c => monitor/qmp-cmds.c         |    2 +-
 monitor/qmp.c                       |  404 +++
 MAINTAINERS                         |   13 +-
 Makefile.objs                       |    4 +-
 Makefile.target                     |    3 +-
 monitor/Makefile.objs               |    3 +
 monitor/trace-events                |   15 +
 trace-events                        |   10 -
 16 files changed, 5060 insertions(+), 4753 deletions(-)
 create mode 100644 monitor/monitor_int.h
 delete mode 100644 monitor.c
 rename hmp.c => monitor/hmp-cmds.c (99%)
 create mode 100644 monitor/hmp.c
 create mode 100644 monitor/misc.c
 create mode 100644 monitor/monitor.c
 rename qmp.c => monitor/qmp-cmds.c (99%)
 create mode 100644 monitor/qmp.c
 create mode 100644 monitor/Makefile.objs
 create mode 100644 monitor/trace-events

-- 
2.20.1

Patchew URL: https://patchew.org/QEMU/20190611134043.9524-1-kwolf@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v2 00/11] monitor: Split monitor.c in core/HMP/QMP/misc
Type: series
Message-id: 20190611134043.9524-1-kwolf@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20190611134043.9524-1-kwolf@redhat.com -> patchew/20190611134043.9524-1-kwolf@redhat.com
Switched to a new branch 'test'
4db331c519 monitor: Split out monitor/monitor.c
899a21f035 monitor: Split out monitor/hmp.c
50416d4aeb monitor: Split out monitor/qmp.c
31d1fe8e42 monitor: Create monitor_int.h with common definitions
2c1ac4ecbf monitor: Move {hmp, qmp}.c to monitor/{hmp, qmp}-cmds.c
37f8c28eca Move monitor.c to monitor/misc.c
00674ed1c9 monitor: Move cmd_table to MonitorHMP
8a46bc8c85 monitor: Create MonitorHMP with readline state
af6e2aad95 monitor: Make MonitorQMP a child class of Monitor
c48181e5ab monitor: Split monitor_init in HMP and QMP function
82eb18147f monitor: Remove unused password prompting fields

=== OUTPUT BEGIN ===
1/11 Checking commit 82eb18147fdf (monitor: Remove unused password prompting fields)
2/11 Checking commit c48181e5ab24 (monitor: Split monitor_init in HMP and QMP function)
3/11 Checking commit af6e2aad957c (monitor: Make MonitorQMP a child class of Monitor)
4/11 Checking commit 8a46bc8c859e (monitor: Create MonitorHMP with readline state)
5/11 Checking commit 00674ed1c9c7 (monitor: Move cmd_table to MonitorHMP)
6/11 Checking commit 37f8c28eca6f (Move monitor.c to monitor/misc.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#78: 
new file mode 100644

total: 0 errors, 1 warnings, 78 lines checked

Patch 6/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/11 Checking commit 2c1ac4ecbfd5 (monitor: Move {hmp, qmp}.c to monitor/{hmp, qmp}-cmds.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#102: 
rename from hmp.c

total: 0 errors, 1 warnings, 73 lines checked

Patch 7/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/11 Checking commit 31d1fe8e4208 (monitor: Create monitor_int.h with common definitions)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#171: 
new file mode 100644

total: 0 errors, 1 warnings, 290 lines checked

Patch 8/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/11 Checking commit 50416d4aebc5 (monitor: Split out monitor/qmp.c)
ERROR: return is not a function, parentheses are not required
#585: FILE: monitor/monitor_int.h:154:
+    return (mon->flags & MONITOR_USE_CONTROL);

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#609: 
new file mode 100644

total: 1 errors, 1 warnings, 966 lines checked

Patch 9/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

10/11 Checking commit 899a21f0358e (monitor: Split out monitor/hmp.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41: 
new file mode 100644

ERROR: consider using qemu_strtoull in preference to strtoull
#409: FILE: monitor/hmp.c:364:
+        n = strtoull(pch, &p, 0);

WARNING: Block comments use a leading /* on a separate line
#1291: FILE: monitor/hmp.c:1246:
+    /* if the line ends with a space, it means we want to complete the

WARNING: Block comments use a trailing */ on a separate line
#1292: FILE: monitor/hmp.c:1247:
+     * next arg */

total: 1 errors, 3 warnings, 2878 lines checked

Patch 10/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

11/11 Checking commit 4db331c5192d (monitor: Split out monitor/monitor.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#706: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#1313: FILE: monitor/monitor.c:603:
+        { /* end of list */ }

total: 0 errors, 2 warnings, 1273 lines checked

Patch 11/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190611134043.9524-1-kwolf@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Kevin Wolf <kwolf@redhat.com> writes:

> monitor.c mixes a lot of different things in a single file: The core
> monitor infrastructure, HMP infrastrcture, QMP infrastructure, and the
> implementation of several HMP and QMP commands. Almost worse, struct
> Monitor mixes state for HMP, for QMP, and state actually shared between
> all monitors. monitor.c must be linked with a system emulator and even
> requires per-target compilation because some of the commands it
> implements access system emulator state.

Also: it's so fat it hasn't seen its feet in years.

> The reason why I care about this is that I'm working on a protoype for a
> storage daemon, which wants to use QMP (but probably not HMP) and
> obviously doesn't have any system emulator state. So I'm interested in
> some core monitor parts that can be linked to non-system-emulator tools.
>
> This series first creates separate structs MonitorQMP and MonitorHMP
> which inherit from Monitor, and then moves the associated infrastructure
> code into separate source files.
>
> While the split is probably not perfect,

It's not :)

>                                          I think it's an improvement of
> the current state even for QEMU proper,

It very much is!

There are a few issues to address, but nothing structural.  Looking
forward to v3.

>                                         and it's good enough so I can
> link my storage daemon against just monitor/core.o and monitor/qmp.o and
> get a useless QMP monitor that parses the JSON input and rejects
> everything as an unknown command.
>
> Next I'll try to teach it a subset of QMP commands that can actually be
> supported in a tool, but while there will be a few follow-up patches to
> achieve this, I don't expect that this work will bring up much that
> needs to be changed in the splitting process done in this series.

Patchew URL: https://patchew.org/QEMU/20190611134043.9524-1-kwolf@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20190611134043.9524-1-kwolf@redhat.com
Subject: [Qemu-devel] [PATCH v2 00/11] monitor: Split monitor.c in core/HMP/QMP/misc
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20190611134043.9524-1-kwolf@redhat.com -> patchew/20190611134043.9524-1-kwolf@redhat.com
Switched to a new branch 'test'
a38b11f monitor: Split out monitor/monitor.c
2e3d743 monitor: Split out monitor/hmp.c
ec837c0 monitor: Split out monitor/qmp.c
7768f93 monitor: Create monitor_int.h with common definitions
212dab6 monitor: Move {hmp, qmp}.c to monitor/{hmp, qmp}-cmds.c
f91cabf Move monitor.c to monitor/misc.c
29cf561 monitor: Move cmd_table to MonitorHMP
bdc1972 monitor: Create MonitorHMP with readline state
b9b4f94 monitor: Make MonitorQMP a child class of Monitor
96e8406 monitor: Split monitor_init in HMP and QMP function
56ef4bc monitor: Remove unused password prompting fields

=== OUTPUT BEGIN ===
1/11 Checking commit 56ef4bc625de (monitor: Remove unused password prompting fields)
2/11 Checking commit 96e84064dbfb (monitor: Split monitor_init in HMP and QMP function)
3/11 Checking commit b9b4f94fff3f (monitor: Make MonitorQMP a child class of Monitor)
4/11 Checking commit bdc1972d7f1c (monitor: Create MonitorHMP with readline state)
5/11 Checking commit 29cf56123d68 (monitor: Move cmd_table to MonitorHMP)
6/11 Checking commit f91cabfa3ac2 (Move monitor.c to monitor/misc.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#78: 
new file mode 100644

total: 0 errors, 1 warnings, 78 lines checked

Patch 6/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/11 Checking commit 212dab63ea80 (monitor: Move {hmp, qmp}.c to monitor/{hmp, qmp}-cmds.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#102: 
rename from hmp.c

total: 0 errors, 1 warnings, 73 lines checked

Patch 7/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/11 Checking commit 7768f93ab68d (monitor: Create monitor_int.h with common definitions)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#171: 
new file mode 100644

total: 0 errors, 1 warnings, 290 lines checked

Patch 8/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/11 Checking commit ec837c059664 (monitor: Split out monitor/qmp.c)
ERROR: return is not a function, parentheses are not required
#586: FILE: monitor/monitor_int.h:154:
+    return (mon->flags & MONITOR_USE_CONTROL);

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#610: 
new file mode 100644

total: 1 errors, 1 warnings, 967 lines checked

Patch 9/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

10/11 Checking commit 2e3d7432ffd3 (monitor: Split out monitor/hmp.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41: 
new file mode 100644

ERROR: consider using qemu_strtoull in preference to strtoull
#409: FILE: monitor/hmp.c:364:
+        n = strtoull(pch, &p, 0);

WARNING: Block comments use a leading /* on a separate line
#1291: FILE: monitor/hmp.c:1246:
+    /* if the line ends with a space, it means we want to complete the

WARNING: Block comments use a trailing */ on a separate line
#1292: FILE: monitor/hmp.c:1247:
+     * next arg */

total: 1 errors, 3 warnings, 2878 lines checked

Patch 10/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

11/11 Checking commit a38b11f617c4 (monitor: Split out monitor/monitor.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#706: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#1313: FILE: monitor/monitor.c:603:
+        { /* end of list */ }

total: 0 errors, 2 warnings, 1273 lines checked

Patch 11/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190611134043.9524-1-kwolf@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com