Re: [RFC PATCH 00/18] Add qemu-storage-daemon - Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Kevin Wolf <kwolf@redhat.com>,
	"qemu-block@nongnu.org" <qemu-block@nongnu.org>
Cc: "pkrempa@redhat.com" <pkrempa@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"mreitz@redhat.com" <mreitz@redhat.com>,
	"armbru@redhat.com" <armbru@redhat.com>
Subject: Re: [RFC PATCH 00/18] Add qemu-storage-daemon
Date: Thu, 24 Oct 2019 13:55:05 +0000	[thread overview]
Message-ID: <45be7a0c-f6ae-5d18-7297-697311046fbd@virtuozzo.com> (raw)
In-Reply-To: <20191017130204.16131-1-kwolf@redhat.com>

Hi!

This reflects our idea of using qemu binary instead of qemu-img for doing
block-layer operations offline.

What is the practical difference between qemu-storage-daemon and starting
qemu binary in stopped state?

17.10.2019 16:01, 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 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.

qemu binary can do that too, with -S option..

> 
>    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.
> 
> * 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.
> 
> I'm posting this as an RFC mainly for two reasons:
> 
> 1. The monitor integration, which could be argued to be a little hackish
>     (some generated QAPI source files are built from a separate QAPI
>     schema, but the per-module ones are taken from the system emulator)
>     and Markus will want to have a closer look there. But from the IRC
>     discussions we had, we seem to agree on the general approach here.
> 
> 2. I'm not completely sure if the command line syntax is the final
>     version that we want to support long-term. Many options directly use
>     QAPI visitors (--blockdev, --export, --nbd-server) and should be
>     fine. However, others use QemuOpts (--chardev, --monitor, --object).
> 
>     This is the same as in the system emulator, so we wouldn't be adding
>     a new problem, but as there was talk about QAPIfying the command
>     line, and I wouldn't want later syntax changes or adding lots of
>     compatibility code to a new tool, I thought we should probably
>     discuss whether QAPIfying from the start would be an option.
> 
> I would like to have something merged for 4.2, but I'm considering
> marking the whole tool as experimental and unstable ABI by requiring a
> command line option like --experimental for the tool to even start if we
> know that we want to change some things later.
> 
> This series is based on:
> * [PATCH] blockdev: Use error_report() in hmp_commit()
> * [PATCH 0/7] qapi: Cleanups and test speedup
> 
> Based-on: <20191015123932.12214-1-kwolf@redhat.com>
> Based-on: <20191001191514.11208-1-armbru@redhat.com>
> 
> Kevin Wolf (18):
>    qemu-storage-daemon: Add barebone tool
>    qemu-storage-daemon: Add --object option
>    stubs: Add arch_type
>    stubs: Add blk_by_qdev_id()
>    qemu-storage-daemon: Add --blockdev 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
>    monitor: Move monitor option parsing to monitor/monitor.c
>    stubs: Update monitor stubs for qemu-storage-daemon
>    qapi: Create module 'monitor'
>    monitor: Create monitor/qmp-cmds-monitor.c
>    qapi: Support empty modules
>    qapi: Create 'pragma' module
>    monitor: Move qmp_query_qmp_schema to qmp-cmds-monitor.c
>    qemu-storage-daemon: Add --monitor option
> 
>   qapi/block.json                          |  65 ++++-
>   qapi/misc.json                           | 212 ---------------
>   qapi/monitor.json                        | 218 +++++++++++++++
>   qapi/pragma.json                         |  24 ++
>   qapi/qapi-schema.json                    |  26 +-
>   storage-daemon/qapi/qapi-schema.json     |  15 ++
>   configure                                |   2 +-
>   include/block/nbd.h                      |   1 +
>   include/monitor/monitor.h                |   4 +
>   include/sysemu/arch_init.h               |   2 +
>   include/sysemu/sysemu.h                  |   1 -
>   monitor/monitor-internal.h               |   4 +
>   blockdev-nbd.c                           |  30 ++-
>   monitor/hmp-cmds.c                       |  22 +-
>   monitor/misc.c                           | 126 ---------
>   monitor/monitor.c                        |  52 ++++
>   monitor/qmp-cmds-monitor.c               | 169 ++++++++++++
>   monitor/qmp-cmds.c                       |  15 +-
>   monitor/qmp.c                            |   2 +-
>   qemu-storage-daemon.c                    | 326 +++++++++++++++++++++++
>   stubs/arch_type.c                        |   4 +
>   stubs/blk-by-qdev-id.c                   |   9 +
>   stubs/monitor-core.c                     |  21 ++
>   stubs/monitor.c                          |  15 +-
>   tests/qmp-test.c                         |   2 +-
>   ui/gtk.c                                 |   1 +
>   vl.c                                     |  45 +---
>   Makefile                                 |  34 +++
>   Makefile.objs                            |   9 +
>   block/Makefile.objs                      |   2 +-
>   monitor/Makefile.objs                    |   5 +-
>   qapi/Makefile.objs                       |   9 +-
>   qom/Makefile.objs                        |   1 +
>   scripts/qapi/gen.py                      |   5 +
>   scripts/qapi/schema.py                   |   9 +
>   storage-daemon/Makefile.objs             |   1 +
>   storage-daemon/qapi/Makefile.objs        |   1 +
>   stubs/Makefile.objs                      |   3 +
>   tests/qapi-schema/comments.out           |   2 +
>   tests/qapi-schema/doc-bad-section.out    |   2 +
>   tests/qapi-schema/doc-good.out           |   2 +
>   tests/qapi-schema/empty.out              |   2 +
>   tests/qapi-schema/event-case.out         |   2 +
>   tests/qapi-schema/include-repetition.out |   4 +
>   tests/qapi-schema/include-simple.out     |   3 +
>   tests/qapi-schema/indented-expr.out      |   2 +
>   tests/qapi-schema/qapi-schema-test.out   |   4 +
>   47 files changed, 1052 insertions(+), 463 deletions(-)
>   create mode 100644 qapi/monitor.json
>   create mode 100644 qapi/pragma.json
>   create mode 100644 storage-daemon/qapi/qapi-schema.json
>   create mode 100644 monitor/qmp-cmds-monitor.c
>   create mode 100644 qemu-storage-daemon.c
>   create mode 100644 stubs/arch_type.c
>   create mode 100644 stubs/blk-by-qdev-id.c
>   create mode 100644 stubs/monitor-core.c
>   create mode 100644 storage-daemon/Makefile.objs
>   create mode 100644 storage-daemon/qapi/Makefile.objs
> 

-- 
Best regards,
Vladimir