Re: [RFC PATCH 18/18] qemu-storage-daemon: Add --monitor option

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3851 bytes --]

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 <kwolf@redhat.com>
>>> ---
>>>  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
> 



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