[RFC,00/18] Add qemu-storage-daemon
mbox series

Message ID 20191017130204.16131-1-kwolf@redhat.com
Headers show
Series
  • Add qemu-storage-daemon
Related show

Message

Kevin Wolf Oct. 17, 2019, 1:01 p.m. UTC
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.

  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

Comments

Kevin Wolf Oct. 24, 2019, 11:33 a.m. UTC | #1
Am 17.10.2019 um 15:01 hat Kevin Wolf geschrieben:
> 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:
> [...]

Ping?
Vladimir Sementsov-Ogievskiy Oct. 24, 2019, 1:55 p.m. UTC | #2
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
>
Stefan Hajnoczi Nov. 5, 2019, 3:52 p.m. UTC | #3
On Thu, Oct 17, 2019 at 03:01:46PM +0200, 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.
> 
>   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.

No comments on the command-line, QAPI, or monitor aspects, but the
general idea of having a qemu-storage-daemon is likely to be useful.

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Max Reitz Nov. 6, 2019, 2:37 p.m. UTC | #4
On 17.10.19 15: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.

Looks good to me.

I remember a discussion at some KVM Forum a couple of years ago where
someone (Berto?) was asking about adding QMP to qemu-nbd.  I found it a
pragmatic solution, but I remember that Markus was against it, based on
the fact that we wanted qemu -M none.

Well, but anyway.  Just as I didn’t have anything against adding QMP to
qemu-nbd, I don’t have anything against adding a new application that
kind of fulfills the same purpose.  And I think introducing a new
application instead of reusing qemu-nbd that focuses on all-around QAPI
compatibility (which qemu-nbd decidedly does not have) makes sense.


The only thing I don’t like is the name, but that’s what <Tab> is for. :-)

Max
Kevin Wolf Nov. 6, 2019, 2:58 p.m. UTC | #5
Am 06.11.2019 um 15:37 hat Max Reitz geschrieben:
> On 17.10.19 15: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.
> 
> Looks good to me.
> 
> I remember a discussion at some KVM Forum a couple of years ago where
> someone (Berto?) was asking about adding QMP to qemu-nbd.  I found it a
> pragmatic solution, but I remember that Markus was against it, based on
> the fact that we wanted qemu -M none.

Yes, but it turned out that qemu -M none is a bit too heavyweight in
practice and fixing that would involve a lot of work. As I understand it
(mostly what I took from discussions on the list), even if someone were
interested in doing that and started now, it's the kind of thing that
would take multiple years.

As long as we keep the code simple and the interesting parts are just
reused and shared with the system emulator and other tools, it shouldn't
be hard to maintain.

> Well, but anyway.  Just as I didn’t have anything against adding QMP to
> qemu-nbd, I don’t have anything against adding a new application that
> kind of fulfills the same purpose.  And I think introducing a new
> application instead of reusing qemu-nbd that focuses on all-around QAPI
> compatibility (which qemu-nbd decidedly does not have) makes sense.

Yes, QAPI is one big reason for creating a new tool that doesn't need to
support the old qemu-nbd command line. Another is that we can add other
types of exports that are not NBD.

> The only thing I don’t like is the name, but that’s what <Tab> is for.
> :-)

I'm open for suggestions, but I thought 'qsd' was a bit too terse. :-)

(Actually, maybe we could even pick something that doesn't mention
storage or block? After all, it can do all kinds of QEMU backends in
theory. Not sure if there's any standalone use for them, but who
knows...)

Kevin
Max Reitz Nov. 6, 2019, 3:35 p.m. UTC | #6
On 06.11.19 15:58, Kevin Wolf wrote:
> Am 06.11.2019 um 15:37 hat Max Reitz geschrieben:
>> On 17.10.19 15: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.
>>
>> Looks good to me.
>>
>> I remember a discussion at some KVM Forum a couple of years ago where
>> someone (Berto?) was asking about adding QMP to qemu-nbd.  I found it a
>> pragmatic solution, but I remember that Markus was against it, based on
>> the fact that we wanted qemu -M none.
> 
> Yes, but it turned out that qemu -M none is a bit too heavyweight in
> practice and fixing that would involve a lot of work. As I understand it
> (mostly what I took from discussions on the list), even if someone were
> interested in doing that and started now, it's the kind of thing that
> would take multiple years.

I didn’t want to give the impression I wouldn’t agree. O:-)

(I agree completely, and basically that was my
understanding/opinion/feeling back when we discussed it, too.)

> As long as we keep the code simple and the interesting parts are just
> reused and shared with the system emulator and other tools, it shouldn't
> be hard to maintain.
> 
>> Well, but anyway.  Just as I didn’t have anything against adding QMP to
>> qemu-nbd, I don’t have anything against adding a new application that
>> kind of fulfills the same purpose.  And I think introducing a new
>> application instead of reusing qemu-nbd that focuses on all-around QAPI
>> compatibility (which qemu-nbd decidedly does not have) makes sense.
> 
> Yes, QAPI is one big reason for creating a new tool that doesn't need to
> support the old qemu-nbd command line. Another is that we can add other
> types of exports that are not NBD.

Sure.

>> The only thing I don’t like is the name, but that’s what <Tab> is for.
>> :-)
> 
> I'm open for suggestions, but I thought 'qsd' was a bit too terse. :-)
> 
> (Actually, maybe we could even pick something that doesn't mention
> storage or block? After all, it can do all kinds of QEMU backends in
> theory. Not sure if there's any standalone use for them, but who
> knows...)

Be careful, if we stuff too much into it, we’ll end up with just qemu
again. :-)

Max
Eric Blake Nov. 6, 2019, 5:13 p.m. UTC | #7
On 11/6/19 8:58 AM, Kevin Wolf wrote:

>> Well, but anyway.  Just as I didn’t have anything against adding QMP to
>> qemu-nbd, I don’t have anything against adding a new application that
>> kind of fulfills the same purpose.  And I think introducing a new
>> application instead of reusing qemu-nbd that focuses on all-around QAPI
>> compatibility (which qemu-nbd decidedly does not have) makes sense.
> 
> Yes, QAPI is one big reason for creating a new tool that doesn't need to
> support the old qemu-nbd command line. Another is that we can add other
> types of exports that are not NBD.

If we could make qemu-nbd a thin wrapper around the new tool (even if 
the two are not necessarily command-line compatible), that might be 
worthwhile.

> 
>> The only thing I don’t like is the name, but that’s what <Tab> is for.
>> :-)
> 
> I'm open for suggestions, but I thought 'qsd' was a bit too terse. :-)
> 
> (Actually, maybe we could even pick something that doesn't mention
> storage or block? After all, it can do all kinds of QEMU backends in
> theory. Not sure if there's any standalone use for them, but who
> knows...)

Maybe 'qback', for qemu-backend?
Daniel P. Berrangé Nov. 7, 2019, 10:33 a.m. UTC | #8
On Thu, Oct 17, 2019 at 03:01:46PM +0200, 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.
> 
>   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 think that following what the QEMU emulators currently do for
CLI args should be an explicit anti-goal, because we know that it is
a long standing source of pain.  Fixing it in the emulator binaries
is hard due to backward compatibility, but for this new binary we
have a clean slate.

This feels like a good opportunity to implement & demonstrate what
we think QEMU configuration ought to look like. Work done for this
in the qemu-storage-daemon may well help us understand how we'll
be able to move the QEMU emulators into a new scheme later.

My personal wish would be to have no use of QemuOpts at all.

Use GOptionContext *only* for parsing command line arguments
related to execution of the daemon - ie things like --help,
--version, --daemon, --pid-file.

The use a "--config /path/to/json/file" arg to point to the config
file for everything else using QAPI schema to describe it fully.

When loading the config file, things should be created in order
in which they are specified. ie don't try and group things,
otherwise we end up back with the horrific hacks for objects
where some are created early & some late.



For an ambitious stretch goal, I think we should seriously consider
whether our use of chardevs is appropriate in all cases that exist,
and whether we can avoid the trap of over-using chardev in the new
storage daemon since it is a clean slate in terms of user facing
CLI config.

chardevs are designed for & reasonably well suited to attaching to
devices like serial ports, parallel ports, etc. You have a 1:1
remote:local peer relationship. The transport is a dumb byte
stream, nothing special needed on top & the user can cope with
any type of chardev.

Many cases where we've used chardevs as a backend in QEMU are a
poor fit. We just used chardevs as an "easy" way to configure a
UNIX or TCP socket from the CLI, and don't care about, nor work
with, any othuer chardev backends. As a result of this misuse
we've had to put in an increasing number of hacks in the chardev
code to deal with fact that callers want to know about  & use
socket semantics. eg FD passing, the chardev reconnection polling
code.

The monitor is a prime example of a bad fit - it would be better
suited by simply referencing a SocketAddress QAPI type, instead
of having the chardev indirection. It would then directly use
the QIOChannelSocket APIs and avoid the inconvenient chardev
abstractions which are a source of complexity & instability for
no net gain.  vhostuser is another prime example, responsible
for much of the complexity & bugs recently added to chardevs
to expose socket semantics


This is a long winded way of saying that we should consider what
syntax we expose for the monitor socket configuration with the
new daemon. Even if the internal code still uses a chardev for
the forseeable future, we have the option to hide this from the
user facing configuration. Let the user specify a SocketAddress,
which we use to secretly instantiate a chardev. Eventually we can
convert the monitor code to stop using a chardev internally too,
with a suitable deprecation period for main QEMU binarijes.


Regards,
Daniel
Kevin Wolf Nov. 7, 2019, 12:03 p.m. UTC | #9
Am 07.11.2019 um 11:33 hat Daniel P. Berrangé geschrieben:
> On Thu, Oct 17, 2019 at 03:01:46PM +0200, Kevin Wolf wrote:
> > 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 think that following what the QEMU emulators currently do for
> CLI args should be an explicit anti-goal, because we know that it is
> a long standing source of pain.  Fixing it in the emulator binaries
> is hard due to backward compatibility, but for this new binary we
> have a clean slate.
> 
> This feels like a good opportunity to implement & demonstrate what
> we think QEMU configuration ought to look like. Work done for this
> in the qemu-storage-daemon may well help us understand how we'll
> be able to move the QEMU emulators into a new scheme later.

It might be, which is why I'm asking. Now that the storage daemon has
missed 4.2, we have a little more time to decide what the command line
should look like in detail.

However, I don't think this is something that should delay the storage
daemon until after 5.0.

> My personal wish would be to have no use of QemuOpts at all.
> 
> Use GOptionContext *only* for parsing command line arguments
> related to execution of the daemon - ie things like --help,
> --version, --daemon, --pid-file.

I really don't believe that the solution for having too much variety in
option parsing is adding in yet another type. GOptionContext is not
something I'm considering at the moment.

But it's a getopt() replacement, not something that could actually parse
the more complex options, so it's a separate question anyway. If we ever
want to use it, we can replace getopt() in all binaries at once.

> The use a "--config /path/to/json/file" arg to point to the config
> file for everything else using QAPI schema to describe it fully.

If this implies that the storage daemon can only do useful things if you
specify a config file, I disagree.

I agree that it's not really nice if you can't use a config file to
specify a lengthy configuration and that supporting one would be good.

But it is at least equally unfriendly to require a config file for
simple configurations where using command line arguments is easily
possible.

> When loading the config file, things should be created in order
> in which they are specified. ie don't try and group things,
> otherwise we end up back with the horrific hacks for objects
> where some are created early & some late.

Yes. This is how the storage daemon command line works, too.

I think Markus already had some patches for command line QAPIfication
that were incomplete at least for the system emulator. It might be
easier to make it feature complete for the storage daemon because it
supports much less problematic options. Maybe he can post a useful
subset (if it's too much work to clean up the full thing right now) and
we can work from there.

The one that I expect to be a bit tricky to be QAPIfied is --object.

> For an ambitious stretch goal, I think we should seriously consider
> whether our use of chardevs is appropriate in all cases that exist,
> and whether we can avoid the trap of over-using chardev in the new
> storage daemon since it is a clean slate in terms of user facing
> CLI config.
> 
> chardevs are designed for & reasonably well suited to attaching to
> devices like serial ports, parallel ports, etc. You have a 1:1
> remote:local peer relationship. The transport is a dumb byte
> stream, nothing special needed on top & the user can cope with
> any type of chardev.
> 
> Many cases where we've used chardevs as a backend in QEMU are a
> poor fit. We just used chardevs as an "easy" way to configure a
> UNIX or TCP socket from the CLI, and don't care about, nor work
> with, any othuer chardev backends. As a result of this misuse
> we've had to put in an increasing number of hacks in the chardev
> code to deal with fact that callers want to know about  & use
> socket semantics. eg FD passing, the chardev reconnection polling
> code.
> 
> The monitor is a prime example of a bad fit - it would be better
> suited by simply referencing a SocketAddress QAPI type, instead
> of having the chardev indirection. It would then directly use
> the QIOChannelSocket APIs and avoid the inconvenient chardev
> abstractions which are a source of complexity & instability for
> no net gain.  vhostuser is another prime example, responsible
> for much of the complexity & bugs recently added to chardevs
> to expose socket semantics
> 
> 
> This is a long winded way of saying that we should consider what
> syntax we expose for the monitor socket configuration with the
> new daemon. Even if the internal code still uses a chardev for
> the forseeable future, we have the option to hide this from the
> user facing configuration. Let the user specify a SocketAddress,
> which we use to secretly instantiate a chardev. Eventually we can
> convert the monitor code to stop using a chardev internally too,
> with a suitable deprecation period for main QEMU binarijes.

The monitor is actually the prime example for chardev backends like
stdio or vc. I'm using them all the time and they aren't covered by
SocketAddress, so I'm afraid SocketAddress alone is not an option.

In any case, the goal of the storage daemon code is to be as boring as
possible. It should just be a thin wrapper around existing code that
glues the command line, monitor and various backends together.

So any change to the usage of chardevs (that is more than a few lines of
wrapper code) would probably have to be made in the system emulator
first, otherwise we'd end up duplicating a lot of code for the storage
daemon instead of just reusing it. I guess using SocketAddress for the
storage daemon and internall creating a chardev-socket would be possible
with a few lines if that covered stdio - but it doesn't.

Kevin
Kevin Wolf Nov. 14, 2019, 10:44 a.m. UTC | #10
Am 24.10.2019 um 15:55 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 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?

If I'm doing things right, QEMU should be able to do the exact same
things as the storage daemon (and more). So functionality isn't what
makes the daemon desirable.

One point to consider is that the QEMU binary with a full system
emulator is (and has to be) relatively heavyweight compared to the
daemon.

I think libvirt once said they didn't want to use the qemu binary on the
grounds that it takes too long to start, though I'm not sure if that's
really an argument. I just did a quick test (qemu with -M none
-nodefaults -display none) and it's 30 vs. 60 ms on my laptop. I get the
same factor 2 for RSS.

More interesting is maybe the overhead in terms of binary size and
dependencies, especially if you need only either the storage daemon _or_
the QEMU binary (e.g. consider a case where the storage daemon runs in
one container and the VM in another).

Having two binaries allows to cut down the dependencies, and to some
extent also the binary size, for both binaries: The storage daemon
doesn't need anything related to system emulation, UI, network etc., and
in QEMU we can probably compile out most of the block layer then, which
gets rid of dependencies like libiscsi, librbd, libgfapi, etc.

As a bonus, this might even reduce the attack surface a little.

So yes, I agree that the storage daemon doesn't offer any functionality
that QEMU can't offer and we don't have a clear-cut requirement that
unambiguously calls for a separate storage daemon, but I still see some
advantage in having two separate binaries.

Another thing to mention is that on IRC, Stefan suggested the other day
to export block devices from the storage daemon not as vhost-user, but
using muser instead (i.e. providing a whole PCI device) and exporting
the existing virtio-blk-pci implementation. This would pull qdev and
device emulations into the storage daemon. I think that would be the
point where using the QEMU binary instead might make more sense (and
maybe compile it twice with different options if need be).

Kevin
Stefan Hajnoczi Nov. 21, 2019, 10:32 a.m. UTC | #11
On Wed, Nov 06, 2019 at 03:58:00PM +0100, Kevin Wolf wrote:
> Am 06.11.2019 um 15:37 hat Max Reitz geschrieben:
> > On 17.10.19 15:01, Kevin Wolf wrote:
> > The only thing I don’t like is the name, but that’s what <Tab> is for.
> > :-)
> 
> I'm open for suggestions, but I thought 'qsd' was a bit too terse. :-)
> 
> (Actually, maybe we could even pick something that doesn't mention
> storage or block? After all, it can do all kinds of QEMU backends in
> theory. Not sure if there's any standalone use for them, but who
> knows...)

It's likely that non-storage use cases will want to a daemon too.  Maybe
just qemu-daemon?

Stefan
Kevin Wolf Nov. 21, 2019, 11:08 a.m. UTC | #12
Am 21.11.2019 um 11:32 hat Stefan Hajnoczi geschrieben:
> On Wed, Nov 06, 2019 at 03:58:00PM +0100, Kevin Wolf wrote:
> > Am 06.11.2019 um 15:37 hat Max Reitz geschrieben:
> > > On 17.10.19 15:01, Kevin Wolf wrote:
> > > The only thing I don’t like is the name, but that’s what <Tab> is for.
> > > :-)
> > 
> > I'm open for suggestions, but I thought 'qsd' was a bit too terse. :-)
> > 
> > (Actually, maybe we could even pick something that doesn't mention
> > storage or block? After all, it can do all kinds of QEMU backends in
> > theory. Not sure if there's any standalone use for them, but who
> > knows...)
> 
> It's likely that non-storage use cases will want to a daemon too.  Maybe
> just qemu-daemon?

Do you have specific use cases in mind? Most backends seem rather
useless without a device to attach them to. In older QEMU versions,
maybe you could put multiple network backends into a common vlan to
forward between them, but even that vlan concept doesn't exist any more.

Kevin
Stefan Hajnoczi Dec. 12, 2019, 11:16 a.m. UTC | #13
On Thu, Nov 21, 2019 at 12:08:16PM +0100, Kevin Wolf wrote:
> Am 21.11.2019 um 11:32 hat Stefan Hajnoczi geschrieben:
> > On Wed, Nov 06, 2019 at 03:58:00PM +0100, Kevin Wolf wrote:
> > > Am 06.11.2019 um 15:37 hat Max Reitz geschrieben:
> > > > On 17.10.19 15:01, Kevin Wolf wrote:
> > > > The only thing I don’t like is the name, but that’s what <Tab> is for.
> > > > :-)
> > > 
> > > I'm open for suggestions, but I thought 'qsd' was a bit too terse. :-)
> > > 
> > > (Actually, maybe we could even pick something that doesn't mention
> > > storage or block? After all, it can do all kinds of QEMU backends in
> > > theory. Not sure if there's any standalone use for them, but who
> > > knows...)
> > 
> > It's likely that non-storage use cases will want to a daemon too.  Maybe
> > just qemu-daemon?
> 
> Do you have specific use cases in mind? Most backends seem rather
> useless without a device to attach them to. In older QEMU versions,
> maybe you could put multiple network backends into a common vlan to
> forward between them, but even that vlan concept doesn't exist any more.

I was thinking about the model where device emulation happens in the
daemon.  We've since agreed that device emulation will continue to
happen in a more traditional QEMU process, so I can't think of
non-storage use cases at the moment.

Stefan