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

From: Stefan Hajnoczi <stefanha@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, coiby.xu@gmail.com,
	armbru@redhat.com, mreitz@redhat.com
Subject: Re: [PATCH v2 00/20] Add qemu-storage-daemon
Date: Fri, 28 Feb 2020 11:16:26 +0000	[thread overview]
Message-ID: <20200228111626.GC326000@stefanha-x1.localdomain> (raw)
In-Reply-To: <20200224143008.13362-1-kwolf@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 6010 bytes --]

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 +-
>  qemu-deprecated.texi                 |   4 +
>  qom/Makefile.objs                    |   1 +
>  storage-daemon/Makefile.objs         |   1 +
>  storage-daemon/qapi/Makefile.objs    |   1 +
>  storage-daemon/qapi/qapi-schema.json |  26 +
>  stubs/Makefile.objs                  |   2 +
>  42 files changed, 1955 insertions(+), 1272 deletions(-)
>  create mode 100644 qapi/pragma.json
>  create mode 100644 block/qapi-sysemu.c
>  create mode 100644 qemu-storage-daemon.c
>  create mode 100644 stubs/arch_type.c
>  create mode 100644 stubs/monitor-core.c
>  create mode 100644 storage-daemon/Makefile.objs
>  create mode 100644 storage-daemon/qapi/Makefile.objs
>  create mode 100644 storage-daemon/qapi/qapi-schema.json

I haven't reviewed the patches in detail since they are mostly concerned
with command-line interfaces and monitor changes.

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]