Re: [PATCH v2 00/20] Add qemu-storage-daemon

2020-03-03 Thread Kevin Wolf
Am 28.02.2020 um 12:16 hat Stefan Hajnoczi geschrieben:
> On Mon, Feb 24, 2020 at 03:29:48PM +0100, Kevin Wolf wrote:
> > This series adds a new tool 'qemu-storage-daemon', which can be used to
> > export and perform operations on block devices. There is some overlap
> > between qemu-img/qemu-nbd and the new qemu-storage-daemon, but there are
> > a few important differences:
> > 
> > * The qemu-storage-daemon has QMP support. The command set is obviously
> >   restricted compared to the system emulator because there is no guest,
> >   but all of the block operations that are not tied to gues devices are
> >   present.
> > 
> >   This means that it can access advanced options or operations that the
> >   qemu-img command line doesn't expose. For example, blockdev-create is
> >   a lot more powerful than 'qemu-img create', and qemu-storage-daemon
> >   allows to execute it without starting a guest.
> > 
> >   Compared to qemu-nbd it means that, for example, block jobs can now be
> >   executed on the server side, and backing chains shared by multiple VMs
> >   can be modified this way.
> > 
> > * The existing tools all have a separately invented one-off syntax for
> >   the job at hand, which usually comes with restrictions compared to the
> >   system emulator. qemu-storage-daemon shares the same syntax with the
> >   system emulator for most options and prefers QAPI based interfaces
> >   where possible (such as --blockdev), so it should be easy to make use
> >   of in libvirt.
> > 
> >   The exception is --chardev, for which not clear design for a QAPIfied
> >   command line exists yet. We'll consider this interface unstable until
> >   we've figured out how to solve it. For now it just uses the same
> >   QemuOpts-based code as the system emulator.
> > 
> > * While this series implements only NBD exports, the storage daemon is
> >   intended to serve multiple protocols and its syntax reflects this. In
> >   the past, we had proposals to add new one-off tools for exporting over
> >   new protocols like FUSE or TCMU.
> > 
> >   With a generic storage daemon, additional export methods have a home
> >   without adding a new tool for each of them.
> > 
> > The plan is to merge qemu-storage-daemon as an experimental feature with
> > a reduced API stability promise in 5.0.
> > 
> > Kevin Wolf (20):
> >   qemu-storage-daemon: Add barebone tool
> >   stubs: Add arch_type
> >   block: Move system emulator QMP commands to block/qapi-sysemu.c
> >   block: Move common QMP commands to block-core QAPI module
> >   block: Move sysemu QMP commands to QAPI block module
> >   qemu-storage-daemon: Add --blockdev option
> >   qapi: Flatten object-add
> >   qemu-storage-daemon: Add --object option
> >   qemu-storage-daemon: Add --nbd-server option
> >   blockdev-nbd: Boxed argument type for nbd-server-add
> >   qemu-storage-daemon: Add --export option
> >   qemu-storage-daemon: Add main loop
> >   qemu-storage-daemon: Add --chardev option
> >   stubs: Update monitor stubs for qemu-storage-daemon
> >   qapi: Create 'pragma' module
> >   monitor: Create QAPIfied monitor_init()
> >   qmp: Fail gracefully if chardev is already in use
> >   hmp: Fail gracefully if chardev is already in use
> >   monitor: Add allow_hmp parameter to monitor_init()
> >   qemu-storage-daemon: Add --monitor option
> > 
> >  qapi/block-core.json | 730 +--
> >  qapi/block.json  | 512 +++
> >  qapi/control.json|  37 ++
> >  qapi/pragma.json |  24 +
> >  qapi/qapi-schema.json|  25 +-
> >  qapi/qom.json|  12 +-
> >  qapi/transaction.json|   2 +-
> >  configure|   2 +-
> >  include/block/nbd.h  |   1 +
> >  include/monitor/monitor.h|   6 +-
> >  include/qom/object_interfaces.h  |   7 +
> >  include/sysemu/arch_init.h   |   2 +
> >  block/qapi-sysemu.c  | 590 ++
> >  blockdev-nbd.c   |  40 +-
> >  blockdev.c   | 559 
> >  chardev/char.c   |   8 +-
> >  gdbstub.c|   2 +-
> >  hw/block/xen-block.c |  11 +-
> >  monitor/hmp-cmds.c   |  21 +-
> >  monitor/hmp.c|   8 +-
> >  monitor/misc.c   |   2 +
> >  monitor/monitor.c|  86 ++--
> >  monitor/qmp-cmds.c   |   2 +-
> >  monitor/qmp.c|  11 +-
> >  qemu-storage-daemon.c| 340 +
> >  qom/qom-qmp-cmds.c   |  42 +-
> >  stubs/arch_type.c|   4 +
> >  stubs/monitor-core.c |  21 +
> >  stubs/monitor.c  |  17 +-
> >  tests/test-util-sockets.c|   4 +-
> >  scripts/qapi/gen.py  |   5 

Re: [PATCH v2 00/20] Add qemu-storage-daemon

2020-02-28 Thread Stefan Hajnoczi
On Mon, Feb 24, 2020 at 03:29:48PM +0100, Kevin Wolf wrote:
> This series adds a new tool 'qemu-storage-daemon', which can be used to
> export and perform operations on block devices. There is some overlap
> between qemu-img/qemu-nbd and the new qemu-storage-daemon, but there are
> a few important differences:
> 
> * The qemu-storage-daemon has QMP support. The command set is obviously
>   restricted compared to the system emulator because there is no guest,
>   but all of the block operations that are not tied to gues devices are
>   present.
> 
>   This means that it can access advanced options or operations that the
>   qemu-img command line doesn't expose. For example, blockdev-create is
>   a lot more powerful than 'qemu-img create', and qemu-storage-daemon
>   allows to execute it without starting a guest.
> 
>   Compared to qemu-nbd it means that, for example, block jobs can now be
>   executed on the server side, and backing chains shared by multiple VMs
>   can be modified this way.
> 
> * The existing tools all have a separately invented one-off syntax for
>   the job at hand, which usually comes with restrictions compared to the
>   system emulator. qemu-storage-daemon shares the same syntax with the
>   system emulator for most options and prefers QAPI based interfaces
>   where possible (such as --blockdev), so it should be easy to make use
>   of in libvirt.
> 
>   The exception is --chardev, for which not clear design for a QAPIfied
>   command line exists yet. We'll consider this interface unstable until
>   we've figured out how to solve it. For now it just uses the same
>   QemuOpts-based code as the system emulator.
> 
> * While this series implements only NBD exports, the storage daemon is
>   intended to serve multiple protocols and its syntax reflects this. In
>   the past, we had proposals to add new one-off tools for exporting over
>   new protocols like FUSE or TCMU.
> 
>   With a generic storage daemon, additional export methods have a home
>   without adding a new tool for each of them.
> 
> The plan is to merge qemu-storage-daemon as an experimental feature with
> a reduced API stability promise in 5.0.
> 
> Kevin Wolf (20):
>   qemu-storage-daemon: Add barebone tool
>   stubs: Add arch_type
>   block: Move system emulator QMP commands to block/qapi-sysemu.c
>   block: Move common QMP commands to block-core QAPI module
>   block: Move sysemu QMP commands to QAPI block module
>   qemu-storage-daemon: Add --blockdev option
>   qapi: Flatten object-add
>   qemu-storage-daemon: Add --object option
>   qemu-storage-daemon: Add --nbd-server option
>   blockdev-nbd: Boxed argument type for nbd-server-add
>   qemu-storage-daemon: Add --export option
>   qemu-storage-daemon: Add main loop
>   qemu-storage-daemon: Add --chardev option
>   stubs: Update monitor stubs for qemu-storage-daemon
>   qapi: Create 'pragma' module
>   monitor: Create QAPIfied monitor_init()
>   qmp: Fail gracefully if chardev is already in use
>   hmp: Fail gracefully if chardev is already in use
>   monitor: Add allow_hmp parameter to monitor_init()
>   qemu-storage-daemon: Add --monitor option
> 
>  qapi/block-core.json | 730 +--
>  qapi/block.json  | 512 +++
>  qapi/control.json|  37 ++
>  qapi/pragma.json |  24 +
>  qapi/qapi-schema.json|  25 +-
>  qapi/qom.json|  12 +-
>  qapi/transaction.json|   2 +-
>  configure|   2 +-
>  include/block/nbd.h  |   1 +
>  include/monitor/monitor.h|   6 +-
>  include/qom/object_interfaces.h  |   7 +
>  include/sysemu/arch_init.h   |   2 +
>  block/qapi-sysemu.c  | 590 ++
>  blockdev-nbd.c   |  40 +-
>  blockdev.c   | 559 
>  chardev/char.c   |   8 +-
>  gdbstub.c|   2 +-
>  hw/block/xen-block.c |  11 +-
>  monitor/hmp-cmds.c   |  21 +-
>  monitor/hmp.c|   8 +-
>  monitor/misc.c   |   2 +
>  monitor/monitor.c|  86 ++--
>  monitor/qmp-cmds.c   |   2 +-
>  monitor/qmp.c|  11 +-
>  qemu-storage-daemon.c| 340 +
>  qom/qom-qmp-cmds.c   |  42 +-
>  stubs/arch_type.c|   4 +
>  stubs/monitor-core.c |  21 +
>  stubs/monitor.c  |  17 +-
>  tests/test-util-sockets.c|   4 +-
>  scripts/qapi/gen.py  |   5 +
>  Makefile |  37 ++
>  Makefile.objs|   9 +
>  block/Makefile.objs  |   4 +-
>  monitor/Makefile.objs|   2 +
>  qapi/Makefile.objs   |   7 +-
>  

Re: [PATCH v2 00/20] Add qemu-storage-daemon

2020-02-24 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200224143008.13362-1-kw...@redhat.com/



Hi,

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

Subject: [PATCH v2 00/20] Add qemu-storage-daemon
Message-id: 20200224143008.13362-1-kw...@redhat.com
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 ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
15c7c50 qemu-storage-daemon: Add --monitor option
c659b9c monitor: Add allow_hmp parameter to monitor_init()
101c369 hmp: Fail gracefully if chardev is already in use
b8da228 qmp: Fail gracefully if chardev is already in use
9a25c79 monitor: Create QAPIfied monitor_init()
5bcc6b7 qapi: Create 'pragma' module
e63b4a1 stubs: Update monitor stubs for qemu-storage-daemon
5df46cf qemu-storage-daemon: Add --chardev option
ec67d87 qemu-storage-daemon: Add main loop
2176838 qemu-storage-daemon: Add --export option
d5531ad blockdev-nbd: Boxed argument type for nbd-server-add
43fa74c qemu-storage-daemon: Add --nbd-server option
0225824 qemu-storage-daemon: Add --object option
cfd2dad qapi: Flatten object-add
aa6c1fa qemu-storage-daemon: Add --blockdev option
db1e4ec block: Move sysemu QMP commands to QAPI block module
745dfca block: Move common QMP commands to block-core QAPI module
ee5879e block: Move system emulator QMP commands to block/qapi-sysemu.c
21f9779 stubs: Add arch_type
bec56f4 qemu-storage-daemon: Add barebone tool

=== OUTPUT BEGIN ===
1/20 Checking commit bec56f4964e3 (qemu-storage-daemon: Add barebone tool)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#46: 
new file mode 100644

total: 0 errors, 1 warnings, 142 lines checked

Patch 1/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/20 Checking commit 21f97790fa55 (stubs: Add arch_type)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#37: 
new file mode 100644

total: 0 errors, 1 warnings, 16 lines checked

Patch 2/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/20 Checking commit ee5879e3fd09 (block: Move system emulator QMP commands to 
block/qapi-sysemu.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#255: FILE: block/qapi-sysemu.c:222:
+/* For tray-less devices, blockdev-open-tray is a no-op (or may not be

WARNING: Block comments use a trailing */ on a separate line
#258: FILE: block/qapi-sysemu.c:225:
+ * value passed here (i.e. false). */

WARNING: Block comments use a leading /* on a separate line
#302: FILE: block/qapi-sysemu.c:269:
+/* For tray-less devices, blockdev-close-tray is a no-op (or may not be

WARNING: Block comments use a trailing */ on a separate line
#306: FILE: block/qapi-sysemu.c:273:
+ * value passed here (i.e. true). */

WARNING: Block comments use a leading /* on a separate line
#439: FILE: block/qapi-sysemu.c:406:
+/* If the medium has been inserted, the device has its own reference, so

WARNING: Block comments use a trailing */ on a separate line
#441: FILE: block/qapi-sysemu.c:408:
+ * the reference must be relinquished anyway */

WARNING: Block comments use a leading /* on a separate line
#548: FILE: block/qapi-sysemu.c:515:
+/* Enable I/O limits if they're not enabled yet, otherwise

WARNING: Block comments use a trailing */ on a separate line
#549: FILE: block/qapi-sysemu.c:516:
+ * just update the throttling group. */

total: 0 errors, 9 warnings, 1187 lines checked

Patch 3/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/20 Checking commit 745dfca103d7 (block: Move common QMP commands to 
block-core QAPI module)
5/20 Checking commit db1e4ec67dea (block: Move sysemu QMP commands to QAPI 
block module)
6/20 Checking commit aa6c1fa307e6 (qemu-storage-daemon: Add --blockdev option)
7/20 Checking commit cfd2dadc400f (qapi: Flatten object-add)
8/20 Checking commit 0225824d841a (qemu-storage-daemon: Add --object option)
WARNING: Block comments use a leading /* on a separate line
#100: FILE: qemu-storage-daemon.c:155:
+/* FIXME The keyval parser rejects 'help' arguments, so we must

WARNING: Block comments use a trailing */ on a separate line
#101: FILE: qemu-storage-daemon.c:156:
+ * unconditionall try QemuOpts first. */

total: 0 errors, 2 warnings, 98 lines checked

Patch 8/20 has style problems, please review.  If any of these errors
are false