Re: [RFC PATCH 18/18] qemu-storage-daemon: Add --monitor option
Kevin Wolf writes:
> Am 12.11.2019 um 15:25 hat Markus Armbruster geschrieben:
>> Kevin Wolf writes:
>>
>> > This adds and parses the --monitor option, so that a QMP monitor can be
>> > used in the storage daemon. The monitor offers commands defined in the
>> > QAPI schema at storage-daemon/qapi/qapi-schema.json.
>>
>> I feel we should explain the module sharing between the two QAPI
>> schemata here. It's not exactly trivial, as we'll see below.
>>
>> > Signed-off-by: Kevin Wolf
>> > ---
>> > storage-daemon/qapi/qapi-schema.json | 15
>> > qemu-storage-daemon.c| 34
>> > Makefile | 30
>> > Makefile.objs| 4 ++--
>> > monitor/Makefile.objs| 2 ++
>> > qapi/Makefile.objs | 5
>> > qom/Makefile.objs| 1 +
>> > scripts/qapi/gen.py | 5
>> > storage-daemon/Makefile.objs | 1 +
>> > storage-daemon/qapi/Makefile.objs| 1 +
>> > 10 files changed, 96 insertions(+), 2 deletions(-)
>> > create mode 100644 storage-daemon/qapi/qapi-schema.json
>> > create mode 100644 storage-daemon/Makefile.objs
>> > create mode 100644 storage-daemon/qapi/Makefile.objs
>> >
>> > diff --git a/storage-daemon/qapi/qapi-schema.json
>> > b/storage-daemon/qapi/qapi-schema.json
>> > new file mode 100644
>> > index 00..58c561ebea
>> > --- /dev/null
>> > +++ b/storage-daemon/qapi/qapi-schema.json
>> > @@ -0,0 +1,15 @@
>> > +# -*- Mode: Python -*-
>> > +
>> > +{ 'include': '../../qapi/pragma.json' }
>> > +
>> > +{ 'include': '../../qapi/block.json' }
>> > +{ 'include': '../../qapi/block-core.json' }
>> > +{ 'include': '../../qapi/char.json' }
>> > +{ 'include': '../../qapi/common.json' }
>> > +{ 'include': '../../qapi/crypto.json' }
>> > +{ 'include': '../../qapi/introspect.json' }
>> > +{ 'include': '../../qapi/job.json' }
>> > +{ 'include': '../../qapi/monitor.json' }
>> > +{ 'include': '../../qapi/qom.json' }
>> > +{ 'include': '../../qapi/sockets.json' }
>> > +{ 'include': '../../qapi/transaction.json' }
>> > diff --git a/qemu-storage-daemon.c b/qemu-storage-daemon.c
>> > index 46e0a6ea56..4939e6b41f 100644
>> > --- a/qemu-storage-daemon.c
>> > +++ b/qemu-storage-daemon.c
>> > @@ -28,12 +28,16 @@
>> > #include "block/nbd.h"
>> > #include "chardev/char.h"
>> > #include "crypto/init.h"
>> > +#include "monitor/monitor.h"
>> > +#include "monitor/monitor-internal.h"
>> >
>> > #include "qapi/error.h"
>> > #include "qapi/qapi-commands-block.h"
>> > #include "qapi/qapi-commands-block-core.h"
>> > +#include "qapi/qapi-commands-monitor.h"
>>
>> Aren't these three redundant with ...
>>
>> > #include "qapi/qapi-visit-block.h"
>> > #include "qapi/qapi-visit-block-core.h"
>> > +#include "qapi/qmp/qstring.h"
>> > #include "qapi/qobject-input-visitor.h"
>> >
>> > #include "qemu-common.h"
>> > @@ -46,6 +50,8 @@
>> > #include "qemu/option.h"
>> > #include "qom/object_interfaces.h"
>> >
>> > +#include "storage-daemon/qapi/qapi-commands.h"
>> > +
>>
>> ... this one now?
>
> Looks like it.
>
>> > #include "sysemu/runstate.h"
>> > #include "trace/control.h"
>> >
>> > @@ -58,6 +64,11 @@ void qemu_system_killed(int signal, pid_t pid)
>> > exit_requested = true;
>> > }
>> >
>> > +void qmp_quit(Error **errp)
>> > +{
>> > +exit_requested = true;
>> > +}
>> > +
>> > static void help(void)
>> > {
>> > printf(
>> > @@ -101,6 +112,7 @@ enum {
>> > OPTION_OBJECT = 256,
>> > OPTION_BLOCKDEV,
>> > OPTION_CHARDEV,
>> > +OPTION_MONITOR,
>> > OPTION_NBD_SERVER,
>> > OPTION_EXPORT,
>> > };
>>
>> Recommend to keep these sorted alphabetically.
>>
>> > @@ -116,6 +128,17 @@ static QemuOptsList qemu_object_opts = {
>> > },
>> > };
>> >
>> > +static void init_qmp_commands(void)
>> > +{
>> > +qmp_init_marshal(_commands);
>> > +qmp_register_command(_commands, "query-qmp-schema",
>> > + qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG);
>> > +
>> > +QTAILQ_INIT(_cap_negotiation_commands);
>> > +qmp_register_command(_cap_negotiation_commands,
>> > "qmp_capabilities",
>> > + qmp_marshal_qmp_capabilities,
>> > QCO_ALLOW_PRECONFIG);
>> > +}
>>
>> Duplicates monitor_init_qmp_commands() less two 'gen': false commands
>> that don't exist in the storage daemon.
>>
>> Hmm, could we let commands.py generate command registration even for
>> 'gen': false commands?
>
> Possibly. Are you telling me to do it or is it just an idea for a later
> cleanup?
Exploring the idea would be nice, but it's definitely not required.
>> > +
>> > static void init_export(BlockExport *export, Error **errp)
>> > {
>> > switch (export->type) {
>> > @@ -138,6 +161,7 @@ static int process_options(int argc, char *argv[],
>> > Error **errp)
>> > {"object", required_argument, 0, OPTION_OBJECT},
>> >
Re: [RFC PATCH 18/18] qemu-storage-daemon: Add --monitor option
Am 12.11.2019 um 15:25 hat Markus Armbruster geschrieben:
> Kevin Wolf writes:
>
> > This adds and parses the --monitor option, so that a QMP monitor can be
> > used in the storage daemon. The monitor offers commands defined in the
> > QAPI schema at storage-daemon/qapi/qapi-schema.json.
>
> I feel we should explain the module sharing between the two QAPI
> schemata here. It's not exactly trivial, as we'll see below.
>
> > Signed-off-by: Kevin Wolf
> > ---
> > storage-daemon/qapi/qapi-schema.json | 15
> > qemu-storage-daemon.c| 34
> > Makefile | 30
> > Makefile.objs| 4 ++--
> > monitor/Makefile.objs| 2 ++
> > qapi/Makefile.objs | 5
> > qom/Makefile.objs| 1 +
> > scripts/qapi/gen.py | 5
> > storage-daemon/Makefile.objs | 1 +
> > storage-daemon/qapi/Makefile.objs| 1 +
> > 10 files changed, 96 insertions(+), 2 deletions(-)
> > create mode 100644 storage-daemon/qapi/qapi-schema.json
> > create mode 100644 storage-daemon/Makefile.objs
> > create mode 100644 storage-daemon/qapi/Makefile.objs
> >
> > diff --git a/storage-daemon/qapi/qapi-schema.json
> > b/storage-daemon/qapi/qapi-schema.json
> > new file mode 100644
> > index 00..58c561ebea
> > --- /dev/null
> > +++ b/storage-daemon/qapi/qapi-schema.json
> > @@ -0,0 +1,15 @@
> > +# -*- Mode: Python -*-
> > +
> > +{ 'include': '../../qapi/pragma.json' }
> > +
> > +{ 'include': '../../qapi/block.json' }
> > +{ 'include': '../../qapi/block-core.json' }
> > +{ 'include': '../../qapi/char.json' }
> > +{ 'include': '../../qapi/common.json' }
> > +{ 'include': '../../qapi/crypto.json' }
> > +{ 'include': '../../qapi/introspect.json' }
> > +{ 'include': '../../qapi/job.json' }
> > +{ 'include': '../../qapi/monitor.json' }
> > +{ 'include': '../../qapi/qom.json' }
> > +{ 'include': '../../qapi/sockets.json' }
> > +{ 'include': '../../qapi/transaction.json' }
> > diff --git a/qemu-storage-daemon.c b/qemu-storage-daemon.c
> > index 46e0a6ea56..4939e6b41f 100644
> > --- a/qemu-storage-daemon.c
> > +++ b/qemu-storage-daemon.c
> > @@ -28,12 +28,16 @@
> > #include "block/nbd.h"
> > #include "chardev/char.h"
> > #include "crypto/init.h"
> > +#include "monitor/monitor.h"
> > +#include "monitor/monitor-internal.h"
> >
> > #include "qapi/error.h"
> > #include "qapi/qapi-commands-block.h"
> > #include "qapi/qapi-commands-block-core.h"
> > +#include "qapi/qapi-commands-monitor.h"
>
> Aren't these three redundant with ...
>
> > #include "qapi/qapi-visit-block.h"
> > #include "qapi/qapi-visit-block-core.h"
> > +#include "qapi/qmp/qstring.h"
> > #include "qapi/qobject-input-visitor.h"
> >
> > #include "qemu-common.h"
> > @@ -46,6 +50,8 @@
> > #include "qemu/option.h"
> > #include "qom/object_interfaces.h"
> >
> > +#include "storage-daemon/qapi/qapi-commands.h"
> > +
>
> ... this one now?
Looks like it.
> > #include "sysemu/runstate.h"
> > #include "trace/control.h"
> >
> > @@ -58,6 +64,11 @@ void qemu_system_killed(int signal, pid_t pid)
> > exit_requested = true;
> > }
> >
> > +void qmp_quit(Error **errp)
> > +{
> > +exit_requested = true;
> > +}
> > +
> > static void help(void)
> > {
> > printf(
> > @@ -101,6 +112,7 @@ enum {
> > OPTION_OBJECT = 256,
> > OPTION_BLOCKDEV,
> > OPTION_CHARDEV,
> > +OPTION_MONITOR,
> > OPTION_NBD_SERVER,
> > OPTION_EXPORT,
> > };
>
> Recommend to keep these sorted alphabetically.
>
> > @@ -116,6 +128,17 @@ static QemuOptsList qemu_object_opts = {
> > },
> > };
> >
> > +static void init_qmp_commands(void)
> > +{
> > +qmp_init_marshal(_commands);
> > +qmp_register_command(_commands, "query-qmp-schema",
> > + qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG);
> > +
> > +QTAILQ_INIT(_cap_negotiation_commands);
> > +qmp_register_command(_cap_negotiation_commands, "qmp_capabilities",
> > + qmp_marshal_qmp_capabilities,
> > QCO_ALLOW_PRECONFIG);
> > +}
>
> Duplicates monitor_init_qmp_commands() less two 'gen': false commands
> that don't exist in the storage daemon.
>
> Hmm, could we let commands.py generate command registration even for
> 'gen': false commands?
Possibly. Are you telling me to do it or is it just an idea for a later
cleanup?
> > +
> > static void init_export(BlockExport *export, Error **errp)
> > {
> > switch (export->type) {
> > @@ -138,6 +161,7 @@ static int process_options(int argc, char *argv[],
> > Error **errp)
> > {"object", required_argument, 0, OPTION_OBJECT},
> > {"blockdev", required_argument, 0, OPTION_BLOCKDEV},
> > {"chardev", required_argument, 0, OPTION_CHARDEV},
> > +{"monitor", required_argument, 0, OPTION_MONITOR},
> > {"nbd-server", required_argument, 0,
Re: [RFC PATCH 18/18] qemu-storage-daemon: Add --monitor option
Kevin Wolf writes:
> This adds and parses the --monitor option, so that a QMP monitor can be
> used in the storage daemon. The monitor offers commands defined in the
> QAPI schema at storage-daemon/qapi/qapi-schema.json.
I feel we should explain the module sharing between the two QAPI
schemata here. It's not exactly trivial, as we'll see below.
> Signed-off-by: Kevin Wolf
> ---
> storage-daemon/qapi/qapi-schema.json | 15
> qemu-storage-daemon.c| 34
> Makefile | 30
> Makefile.objs| 4 ++--
> monitor/Makefile.objs| 2 ++
> qapi/Makefile.objs | 5
> qom/Makefile.objs| 1 +
> scripts/qapi/gen.py | 5
> storage-daemon/Makefile.objs | 1 +
> storage-daemon/qapi/Makefile.objs| 1 +
> 10 files changed, 96 insertions(+), 2 deletions(-)
> create mode 100644 storage-daemon/qapi/qapi-schema.json
> create mode 100644 storage-daemon/Makefile.objs
> create mode 100644 storage-daemon/qapi/Makefile.objs
>
> diff --git a/storage-daemon/qapi/qapi-schema.json
> b/storage-daemon/qapi/qapi-schema.json
> new file mode 100644
> index 00..58c561ebea
> --- /dev/null
> +++ b/storage-daemon/qapi/qapi-schema.json
> @@ -0,0 +1,15 @@
> +# -*- Mode: Python -*-
> +
> +{ 'include': '../../qapi/pragma.json' }
> +
> +{ 'include': '../../qapi/block.json' }
> +{ 'include': '../../qapi/block-core.json' }
> +{ 'include': '../../qapi/char.json' }
> +{ 'include': '../../qapi/common.json' }
> +{ 'include': '../../qapi/crypto.json' }
> +{ 'include': '../../qapi/introspect.json' }
> +{ 'include': '../../qapi/job.json' }
> +{ 'include': '../../qapi/monitor.json' }
> +{ 'include': '../../qapi/qom.json' }
> +{ 'include': '../../qapi/sockets.json' }
> +{ 'include': '../../qapi/transaction.json' }
> diff --git a/qemu-storage-daemon.c b/qemu-storage-daemon.c
> index 46e0a6ea56..4939e6b41f 100644
> --- a/qemu-storage-daemon.c
> +++ b/qemu-storage-daemon.c
> @@ -28,12 +28,16 @@
> #include "block/nbd.h"
> #include "chardev/char.h"
> #include "crypto/init.h"
> +#include "monitor/monitor.h"
> +#include "monitor/monitor-internal.h"
>
> #include "qapi/error.h"
> #include "qapi/qapi-commands-block.h"
> #include "qapi/qapi-commands-block-core.h"
> +#include "qapi/qapi-commands-monitor.h"
Aren't these three redundant with ...
> #include "qapi/qapi-visit-block.h"
> #include "qapi/qapi-visit-block-core.h"
> +#include "qapi/qmp/qstring.h"
> #include "qapi/qobject-input-visitor.h"
>
> #include "qemu-common.h"
> @@ -46,6 +50,8 @@
> #include "qemu/option.h"
> #include "qom/object_interfaces.h"
>
> +#include "storage-daemon/qapi/qapi-commands.h"
> +
... this one now?
> #include "sysemu/runstate.h"
> #include "trace/control.h"
>
> @@ -58,6 +64,11 @@ void qemu_system_killed(int signal, pid_t pid)
> exit_requested = true;
> }
>
> +void qmp_quit(Error **errp)
> +{
> +exit_requested = true;
> +}
> +
> static void help(void)
> {
> printf(
> @@ -101,6 +112,7 @@ enum {
> OPTION_OBJECT = 256,
> OPTION_BLOCKDEV,
> OPTION_CHARDEV,
> +OPTION_MONITOR,
> OPTION_NBD_SERVER,
> OPTION_EXPORT,
> };
Recommend to keep these sorted alphabetically.
> @@ -116,6 +128,17 @@ static QemuOptsList qemu_object_opts = {
> },
> };
>
> +static void init_qmp_commands(void)
> +{
> +qmp_init_marshal(_commands);
> +qmp_register_command(_commands, "query-qmp-schema",
> + qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG);
> +
> +QTAILQ_INIT(_cap_negotiation_commands);
> +qmp_register_command(_cap_negotiation_commands, "qmp_capabilities",
> + qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
> +}
Duplicates monitor_init_qmp_commands() less two 'gen': false commands
that don't exist in the storage daemon.
Hmm, could we let commands.py generate command registration even for
'gen': false commands?
> +
> static void init_export(BlockExport *export, Error **errp)
> {
> switch (export->type) {
> @@ -138,6 +161,7 @@ static int process_options(int argc, char *argv[], Error
> **errp)
> {"object", required_argument, 0, OPTION_OBJECT},
> {"blockdev", required_argument, 0, OPTION_BLOCKDEV},
> {"chardev", required_argument, 0, OPTION_CHARDEV},
> +{"monitor", required_argument, 0, OPTION_MONITOR},
> {"nbd-server", required_argument, 0, OPTION_NBD_SERVER},
> {"export", required_argument, 0, OPTION_EXPORT},
> {"version", no_argument, 0, 'V'},
> @@ -208,6 +232,14 @@ static int process_options(int argc, char *argv[], Error
> **errp)
> qemu_opts_del(opts);
> break;
> }
> +case OPTION_MONITOR:
> +{
> +QemuOpts *opts = qemu_opts_parse(_mon_opts,
> +
Re: [RFC PATCH 18/18] qemu-storage-daemon: Add --monitor option
Quick observation: --help fails to mention --monitor.
Re: [RFC PATCH 18/18] qemu-storage-daemon: Add --monitor option
On 07.11.19 11:12, Kevin Wolf wrote:
> Am 06.11.2019 um 15:32 hat Max Reitz geschrieben:
>> On 17.10.19 15:02, Kevin Wolf wrote:
>>> This adds and parses the --monitor option, so that a QMP monitor can be
>>> used in the storage daemon. The monitor offers commands defined in the
>>> QAPI schema at storage-daemon/qapi/qapi-schema.json.
>>>
>>> Signed-off-by: Kevin Wolf
>>> ---
>>> storage-daemon/qapi/qapi-schema.json | 15
>>> qemu-storage-daemon.c| 34
>>> Makefile | 30
>>> Makefile.objs| 4 ++--
>>> monitor/Makefile.objs| 2 ++
>>> qapi/Makefile.objs | 5
>>> qom/Makefile.objs| 1 +
>>> scripts/qapi/gen.py | 5
>>> storage-daemon/Makefile.objs | 1 +
>>> storage-daemon/qapi/Makefile.objs| 1 +
>>> 10 files changed, 96 insertions(+), 2 deletions(-)
>>> create mode 100644 storage-daemon/qapi/qapi-schema.json
>>> create mode 100644 storage-daemon/Makefile.objs
>>> create mode 100644 storage-daemon/qapi/Makefile.objs
>>
>> [...]
>>
>>> diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
>>> index 3e04e299ed..03d256f0a4 100644
>>> --- a/qapi/Makefile.objs
>>> +++ b/qapi/Makefile.objs
>>> @@ -30,3 +30,8 @@ obj-y += $(QAPI_TARGET_MODULES:%=qapi-events-%.o)
>>> obj-y += qapi-events.o
>>> obj-y += $(QAPI_TARGET_MODULES:%=qapi-commands-%.o)
>>> obj-y += qapi-commands.o
>>> +
>>> +QAPI_MODULES_STORAGE_DAEMON = block block-core char common crypto
>>> introspect
>>> +QAPI_MODULES_STORAGE_DAEMON += job monitor qom sockets pragma transaction
>>
>> I took a look into the rest, and I wonder whether query-iothreads from
>> misc.json would be useful, too.
>
> Possibly. It would be a separate patch, but I can add it.
>
> The question is just where to move query-iothreads. Do we have a good
> place, or do I need to separate misc.json and a new misc-sysemu.json?
I’d just put it in block.json because of the “io”... O:-)
>>> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
>>> index 796c17c38a..c25634f673 100644
>>> --- a/scripts/qapi/gen.py
>>> +++ b/scripts/qapi/gen.py
>>> @@ -44,6 +44,11 @@ class QAPIGen(object):
>>> return ''
>>>
>>> def write(self, output_dir):
>>> +# Include paths starting with ../ are used to reuse modules of the
>>> main
>>> +# schema in specialised schemas. Don't overwrite the files that are
>>> +# already generated for the main schema.
>>> +if self.fname.startswith('../'):
>>> +return
>>
>> Sorry, but I’m about to ask a clueless question: How do we ensure that
>> the main schema is generated before something else could make sure of
>> the specialized schemas?
>
> "Make sure"?
Oops, s/ sure/ use/.
> I think the order of the generation doesn't matter because generating
> the storage daemon files doesn't actually access the main ones.
> Generated C files shouldn't be a problem either because if we link an
> object file into a binary, we have a make dependency for it.
I was mostly wondering about the fact that make mustn’t try to compile
the “generated files” (which aren’t really generated here) before they
are actually generated when the main schema is processed.
Max
> Maybe the only a bit trickier question is whether we have the
> dependencies right so that qemu-storage-daemon.c is only built after the
> header files of both the main schema and the specific one have been
> generated. If I understand the Makefile correctly, generated-files-y
> takes care of this, and this patch adds all new header files to it if I
> didn't miss any.
>
> Kevin
>
signature.asc
Description: OpenPGP digital signature
Re: [RFC PATCH 18/18] qemu-storage-daemon: Add --monitor option
Am 06.11.2019 um 15:32 hat Max Reitz geschrieben:
> On 17.10.19 15:02, Kevin Wolf wrote:
> > This adds and parses the --monitor option, so that a QMP monitor can be
> > used in the storage daemon. The monitor offers commands defined in the
> > QAPI schema at storage-daemon/qapi/qapi-schema.json.
> >
> > Signed-off-by: Kevin Wolf
> > ---
> > storage-daemon/qapi/qapi-schema.json | 15
> > qemu-storage-daemon.c| 34
> > Makefile | 30
> > Makefile.objs| 4 ++--
> > monitor/Makefile.objs| 2 ++
> > qapi/Makefile.objs | 5
> > qom/Makefile.objs| 1 +
> > scripts/qapi/gen.py | 5
> > storage-daemon/Makefile.objs | 1 +
> > storage-daemon/qapi/Makefile.objs| 1 +
> > 10 files changed, 96 insertions(+), 2 deletions(-)
> > create mode 100644 storage-daemon/qapi/qapi-schema.json
> > create mode 100644 storage-daemon/Makefile.objs
> > create mode 100644 storage-daemon/qapi/Makefile.objs
>
> [...]
>
> > diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
> > index 3e04e299ed..03d256f0a4 100644
> > --- a/qapi/Makefile.objs
> > +++ b/qapi/Makefile.objs
> > @@ -30,3 +30,8 @@ obj-y += $(QAPI_TARGET_MODULES:%=qapi-events-%.o)
> > obj-y += qapi-events.o
> > obj-y += $(QAPI_TARGET_MODULES:%=qapi-commands-%.o)
> > obj-y += qapi-commands.o
> > +
> > +QAPI_MODULES_STORAGE_DAEMON = block block-core char common crypto
> > introspect
> > +QAPI_MODULES_STORAGE_DAEMON += job monitor qom sockets pragma transaction
>
> I took a look into the rest, and I wonder whether query-iothreads from
> misc.json would be useful, too.
Possibly. It would be a separate patch, but I can add it.
The question is just where to move query-iothreads. Do we have a good
place, or do I need to separate misc.json and a new misc-sysemu.json?
> > diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> > index 796c17c38a..c25634f673 100644
> > --- a/scripts/qapi/gen.py
> > +++ b/scripts/qapi/gen.py
> > @@ -44,6 +44,11 @@ class QAPIGen(object):
> > return ''
> >
> > def write(self, output_dir):
> > +# Include paths starting with ../ are used to reuse modules of the
> > main
> > +# schema in specialised schemas. Don't overwrite the files that are
> > +# already generated for the main schema.
> > +if self.fname.startswith('../'):
> > +return
>
> Sorry, but I’m about to ask a clueless question: How do we ensure that
> the main schema is generated before something else could make sure of
> the specialized schemas?
"Make sure"?
I think the order of the generation doesn't matter because generating
the storage daemon files doesn't actually access the main ones.
Generated C files shouldn't be a problem either because if we link an
object file into a binary, we have a make dependency for it.
Maybe the only a bit trickier question is whether we have the
dependencies right so that qemu-storage-daemon.c is only built after the
header files of both the main schema and the specific one have been
generated. If I understand the Makefile correctly, generated-files-y
takes care of this, and this patch adds all new header files to it if I
didn't miss any.
Kevin
signature.asc
Description: PGP signature
Re: [RFC PATCH 18/18] qemu-storage-daemon: Add --monitor option
On 17.10.19 15:02, Kevin Wolf wrote:
> This adds and parses the --monitor option, so that a QMP monitor can be
> used in the storage daemon. The monitor offers commands defined in the
> QAPI schema at storage-daemon/qapi/qapi-schema.json.
>
> Signed-off-by: Kevin Wolf
> ---
> storage-daemon/qapi/qapi-schema.json | 15
> qemu-storage-daemon.c| 34
> Makefile | 30
> Makefile.objs| 4 ++--
> monitor/Makefile.objs| 2 ++
> qapi/Makefile.objs | 5
> qom/Makefile.objs| 1 +
> scripts/qapi/gen.py | 5
> storage-daemon/Makefile.objs | 1 +
> storage-daemon/qapi/Makefile.objs| 1 +
> 10 files changed, 96 insertions(+), 2 deletions(-)
> create mode 100644 storage-daemon/qapi/qapi-schema.json
> create mode 100644 storage-daemon/Makefile.objs
> create mode 100644 storage-daemon/qapi/Makefile.objs
[...]
> diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
> index 3e04e299ed..03d256f0a4 100644
> --- a/qapi/Makefile.objs
> +++ b/qapi/Makefile.objs
> @@ -30,3 +30,8 @@ obj-y += $(QAPI_TARGET_MODULES:%=qapi-events-%.o)
> obj-y += qapi-events.o
> obj-y += $(QAPI_TARGET_MODULES:%=qapi-commands-%.o)
> obj-y += qapi-commands.o
> +
> +QAPI_MODULES_STORAGE_DAEMON = block block-core char common crypto introspect
> +QAPI_MODULES_STORAGE_DAEMON += job monitor qom sockets pragma transaction
I took a look into the rest, and I wonder whether query-iothreads from
misc.json would be useful, too.
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index 796c17c38a..c25634f673 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -44,6 +44,11 @@ class QAPIGen(object):
> return ''
>
> def write(self, output_dir):
> +# Include paths starting with ../ are used to reuse modules of the
> main
> +# schema in specialised schemas. Don't overwrite the files that are
> +# already generated for the main schema.
> +if self.fname.startswith('../'):
> +return
Sorry, but I’m about to ask a clueless question: How do we ensure that
the main schema is generated before something else could make sure of
the specialized schemas?
Max
> pathname = os.path.join(output_dir, self.fname)
> dir = os.path.dirname(pathname)
> if dir:
signature.asc
Description: OpenPGP digital signature
[RFC PATCH 18/18] qemu-storage-daemon: Add --monitor option
This adds and parses the --monitor option, so that a QMP monitor can be
used in the storage daemon. The monitor offers commands defined in the
QAPI schema at storage-daemon/qapi/qapi-schema.json.
Signed-off-by: Kevin Wolf
---
storage-daemon/qapi/qapi-schema.json | 15
qemu-storage-daemon.c| 34
Makefile | 30
Makefile.objs| 4 ++--
monitor/Makefile.objs| 2 ++
qapi/Makefile.objs | 5
qom/Makefile.objs| 1 +
scripts/qapi/gen.py | 5
storage-daemon/Makefile.objs | 1 +
storage-daemon/qapi/Makefile.objs| 1 +
10 files changed, 96 insertions(+), 2 deletions(-)
create mode 100644 storage-daemon/qapi/qapi-schema.json
create mode 100644 storage-daemon/Makefile.objs
create mode 100644 storage-daemon/qapi/Makefile.objs
diff --git a/storage-daemon/qapi/qapi-schema.json
b/storage-daemon/qapi/qapi-schema.json
new file mode 100644
index 00..58c561ebea
--- /dev/null
+++ b/storage-daemon/qapi/qapi-schema.json
@@ -0,0 +1,15 @@
+# -*- Mode: Python -*-
+
+{ 'include': '../../qapi/pragma.json' }
+
+{ 'include': '../../qapi/block.json' }
+{ 'include': '../../qapi/block-core.json' }
+{ 'include': '../../qapi/char.json' }
+{ 'include': '../../qapi/common.json' }
+{ 'include': '../../qapi/crypto.json' }
+{ 'include': '../../qapi/introspect.json' }
+{ 'include': '../../qapi/job.json' }
+{ 'include': '../../qapi/monitor.json' }
+{ 'include': '../../qapi/qom.json' }
+{ 'include': '../../qapi/sockets.json' }
+{ 'include': '../../qapi/transaction.json' }
diff --git a/qemu-storage-daemon.c b/qemu-storage-daemon.c
index 46e0a6ea56..4939e6b41f 100644
--- a/qemu-storage-daemon.c
+++ b/qemu-storage-daemon.c
@@ -28,12 +28,16 @@
#include "block/nbd.h"
#include "chardev/char.h"
#include "crypto/init.h"
+#include "monitor/monitor.h"
+#include "monitor/monitor-internal.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-block.h"
#include "qapi/qapi-commands-block-core.h"
+#include "qapi/qapi-commands-monitor.h"
#include "qapi/qapi-visit-block.h"
#include "qapi/qapi-visit-block-core.h"
+#include "qapi/qmp/qstring.h"
#include "qapi/qobject-input-visitor.h"
#include "qemu-common.h"
@@ -46,6 +50,8 @@
#include "qemu/option.h"
#include "qom/object_interfaces.h"
+#include "storage-daemon/qapi/qapi-commands.h"
+
#include "sysemu/runstate.h"
#include "trace/control.h"
@@ -58,6 +64,11 @@ void qemu_system_killed(int signal, pid_t pid)
exit_requested = true;
}
+void qmp_quit(Error **errp)
+{
+exit_requested = true;
+}
+
static void help(void)
{
printf(
@@ -101,6 +112,7 @@ enum {
OPTION_OBJECT = 256,
OPTION_BLOCKDEV,
OPTION_CHARDEV,
+OPTION_MONITOR,
OPTION_NBD_SERVER,
OPTION_EXPORT,
};
@@ -116,6 +128,17 @@ static QemuOptsList qemu_object_opts = {
},
};
+static void init_qmp_commands(void)
+{
+qmp_init_marshal(_commands);
+qmp_register_command(_commands, "query-qmp-schema",
+ qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG);
+
+QTAILQ_INIT(_cap_negotiation_commands);
+qmp_register_command(_cap_negotiation_commands, "qmp_capabilities",
+ qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
+}
+
static void init_export(BlockExport *export, Error **errp)
{
switch (export->type) {
@@ -138,6 +161,7 @@ static int process_options(int argc, char *argv[], Error
**errp)
{"object", required_argument, 0, OPTION_OBJECT},
{"blockdev", required_argument, 0, OPTION_BLOCKDEV},
{"chardev", required_argument, 0, OPTION_CHARDEV},
+{"monitor", required_argument, 0, OPTION_MONITOR},
{"nbd-server", required_argument, 0, OPTION_NBD_SERVER},
{"export", required_argument, 0, OPTION_EXPORT},
{"version", no_argument, 0, 'V'},
@@ -208,6 +232,14 @@ static int process_options(int argc, char *argv[], Error
**errp)
qemu_opts_del(opts);
break;
}
+case OPTION_MONITOR:
+{
+QemuOpts *opts = qemu_opts_parse(_mon_opts,
+ optarg, true, _fatal);
+monitor_init_opts(opts, false, _fatal);
+qemu_opts_del(opts);
+break;
+}
case OPTION_NBD_SERVER:
{
Visitor *v;
@@ -272,6 +304,8 @@ int main(int argc, char *argv[])
qemu_add_opts(_trace_opts);
qcrypto_init(_fatal);
bdrv_init();
+monitor_init_globals_core();
+init_qmp_commands();
if (qemu_init_main_loop(_err)) {
error_report_err(local_err);
diff --git a/Makefile b/Makefile
index 0e3e98582d..e367d2b28a 100644
--- a/Makefile
+++ b/Makefile
@@ -121,7 +121,26 @@ GENERATED_QAPI_FILES +=
$(QAPI_MODULES:%=qapi/qapi-events-%.c)
