[PATCH v2 0/5] qapi: Restrict machine (and migration) specific commands

Philippe Mathieu-Daudé posted 5 patches 3 years, 6 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20201012121536.3381997-1-philmd@redhat.com
qapi/machine.json      | 168 +++++++++++++++++++++++++++++++++
qapi/migration.json    |  41 ++++++++
qapi/misc.json         | 209 -----------------------------------------
accel/stubs/xen-stub.c |   2 +-
hw/i386/xen/xen-hvm.c  |   2 +-
migration/savevm.c     |   1 -
softmmu/cpus.c         |   1 +
ui/gtk.c               |   1 +
ui/cocoa.m             |   1 +
9 files changed, 214 insertions(+), 212 deletions(-)
[PATCH v2 0/5] qapi: Restrict machine (and migration) specific commands
Posted by Philippe Mathieu-Daudé 3 years, 6 months ago
Reduce the machine code pulled into qemu-storage-daemon.

The series is fully Acked, but Markus wants it reviewed
by the Machine core maintainers.

Since v1:
- Added A-b tags
- Rebased

Philippe Mathieu-Daudé (5):
  qapi: Restrict 'inject-nmi' command to machine code
  qapi: Restrict 'system wakeup/reset/powerdown' commands to
    machine.json
  qapi: Restrict '(p)memsave' command to machine code
  qapi: Restrict 'query-kvm' command to machine code
  qapi: Restrict Xen migration commands to migration.json

 qapi/machine.json      | 168 +++++++++++++++++++++++++++++++++
 qapi/migration.json    |  41 ++++++++
 qapi/misc.json         | 209 -----------------------------------------
 accel/stubs/xen-stub.c |   2 +-
 hw/i386/xen/xen-hvm.c  |   2 +-
 migration/savevm.c     |   1 -
 softmmu/cpus.c         |   1 +
 ui/gtk.c               |   1 +
 ui/cocoa.m             |   1 +
 9 files changed, 214 insertions(+), 212 deletions(-)

-- 
2.26.2


Re: [PATCH v2 0/5] qapi: Restrict machine (and migration) specific commands
Posted by Eduardo Habkost 3 years, 6 months ago
On Mon, Oct 12, 2020 at 02:15:31PM +0200, Philippe Mathieu-Daudé wrote:
> Reduce the machine code pulled into qemu-storage-daemon.
> 
> The series is fully Acked, but Markus wants it reviewed
> by the Machine core maintainers.

I've confirmed that all patches move QAPI schema code without
introducing any additional changes.

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> 
> Since v1:
> - Added A-b tags
> - Rebased
> 
> Philippe Mathieu-Daudé (5):
>   qapi: Restrict 'inject-nmi' command to machine code
>   qapi: Restrict 'system wakeup/reset/powerdown' commands to
>     machine.json
>   qapi: Restrict '(p)memsave' command to machine code
>   qapi: Restrict 'query-kvm' command to machine code
>   qapi: Restrict Xen migration commands to migration.json
> 
>  qapi/machine.json      | 168 +++++++++++++++++++++++++++++++++
>  qapi/migration.json    |  41 ++++++++
>  qapi/misc.json         | 209 -----------------------------------------
>  accel/stubs/xen-stub.c |   2 +-
>  hw/i386/xen/xen-hvm.c  |   2 +-
>  migration/savevm.c     |   1 -
>  softmmu/cpus.c         |   1 +
>  ui/gtk.c               |   1 +
>  ui/cocoa.m             |   1 +
>  9 files changed, 214 insertions(+), 212 deletions(-)
> 
> -- 
> 2.26.2
> 
> 

-- 
Eduardo


Re: [PATCH v2 0/5] qapi: Restrict machine (and migration) specific commands
Posted by Markus Armbruster 3 years, 6 months ago
Eduardo Habkost <ehabkost@redhat.com> writes:

> On Mon, Oct 12, 2020 at 02:15:31PM +0200, Philippe Mathieu-Daudé wrote:
>> Reduce the machine code pulled into qemu-storage-daemon.
>> 
>> The series is fully Acked, but Markus wants it reviewed
>> by the Machine core maintainers.
>
> I've confirmed that all patches move QAPI schema code without
> introducing any additional changes.
>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

I take this as "I agree the things moved to machine.json belong there".
Holler if I'm mistaken.


Re: [PATCH v2 0/5] qapi: Restrict machine (and migration) specific commands
Posted by Eduardo Habkost 3 years, 6 months ago
On Mon, Oct 19, 2020 at 09:55:20AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Mon, Oct 12, 2020 at 02:15:31PM +0200, Philippe Mathieu-Daudé wrote:
> >> Reduce the machine code pulled into qemu-storage-daemon.
> >> 
> >> The series is fully Acked, but Markus wants it reviewed
> >> by the Machine core maintainers.
> >
> > I've confirmed that all patches move QAPI schema code without
> > introducing any additional changes.
> >
> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> I take this as "I agree the things moved to machine.json belong there".
> Holler if I'm mistaken.

I agree machine.json is better than misc.json for them, yes.

I miss short descriptions of the purpose of each file, though.
It would help us decide what's appropriate in the future.

-- 
Eduardo


Re: [PATCH v2 0/5] qapi: Restrict machine (and migration) specific commands
Posted by Markus Armbruster 3 years, 6 months ago
Eduardo Habkost <ehabkost@redhat.com> writes:

> On Mon, Oct 19, 2020 at 09:55:20AM +0200, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > On Mon, Oct 12, 2020 at 02:15:31PM +0200, Philippe Mathieu-Daudé wrote:
>> >> Reduce the machine code pulled into qemu-storage-daemon.
>> >> 
>> >> The series is fully Acked, but Markus wants it reviewed
>> >> by the Machine core maintainers.
>> >
>> > I've confirmed that all patches move QAPI schema code without
>> > introducing any additional changes.
>> >
>> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>> 
>> I take this as "I agree the things moved to machine.json belong there".
>> Holler if I'm mistaken.
>
> I agree machine.json is better than misc.json for them, yes.
>
> I miss short descriptions of the purpose of each file, though.
> It would help us decide what's appropriate in the future.

The QAPI modules are commonly aligned with sub-systems defined in
MAINTAINERS.

Regardless, file comments would be nice.


Re: [PATCH v2 0/5] qapi: Restrict machine (and migration) specific commands
Posted by Philippe Mathieu-Daudé 3 years, 6 months ago
On 10/19/20 6:48 PM, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
>> On Mon, Oct 19, 2020 at 09:55:20AM +0200, Markus Armbruster wrote:
>>> Eduardo Habkost <ehabkost@redhat.com> writes:
>>>
>>>> On Mon, Oct 12, 2020 at 02:15:31PM +0200, Philippe Mathieu-Daudé wrote:
>>>>> Reduce the machine code pulled into qemu-storage-daemon.
>>>>>
>>>>> The series is fully Acked, but Markus wants it reviewed
>>>>> by the Machine core maintainers.
>>>>
>>>> I've confirmed that all patches move QAPI schema code without
>>>> introducing any additional changes.
>>>>
>>>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>>>
>>> I take this as "I agree the things moved to machine.json belong there".
>>> Holler if I'm mistaken.
>>
>> I agree machine.json is better than misc.json for them, yes.
>>
>> I miss short descriptions of the purpose of each file, though.
>> It would help us decide what's appropriate in the future.
> 
> The QAPI modules are commonly aligned with sub-systems defined in
> MAINTAINERS.
> 
> Regardless, file comments would be nice.

I don't understand what you mean/expect by "file comments".
Example?

W.r.t. MAINTAINERS, I can move Xen code to qapi/migration-xen.json;

'query-kvm' is used when no KVM built it, so I'll let it in
machine.json; the others seem to belong in machine.json too,
with no particular justification.


Re: [PATCH v2 0/5] qapi: Restrict machine (and migration) specific commands
Posted by Markus Armbruster 3 years, 6 months ago
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 10/19/20 6:48 PM, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>>> On Mon, Oct 19, 2020 at 09:55:20AM +0200, Markus Armbruster wrote:
>>>> Eduardo Habkost <ehabkost@redhat.com> writes:
>>>>
>>>>> On Mon, Oct 12, 2020 at 02:15:31PM +0200, Philippe Mathieu-Daudé wrote:
>>>>>> Reduce the machine code pulled into qemu-storage-daemon.
>>>>>>
>>>>>> The series is fully Acked, but Markus wants it reviewed
>>>>>> by the Machine core maintainers.
>>>>>
>>>>> I've confirmed that all patches move QAPI schema code without
>>>>> introducing any additional changes.
>>>>>
>>>>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>>>>
>>>> I take this as "I agree the things moved to machine.json belong there".
>>>> Holler if I'm mistaken.
>>>
>>> I agree machine.json is better than misc.json for them, yes.
>>>
>>> I miss short descriptions of the purpose of each file, though.
>>> It would help us decide what's appropriate in the future.
>>
>> The QAPI modules are commonly aligned with sub-systems defined in
>> MAINTAINERS.
>>
>> Regardless, file comments would be nice.
>
> I don't understand what you mean/expect by "file comments".
> Example?

A comment explaining the file, at the beginning of the file.

> W.r.t. MAINTAINERS, I can move Xen code to qapi/migration-xen.json;

How much could be moved, and from where?

Sub-modules don't need to mirror MAINTAINERS slavishly.  We want
reasonably-sized modules, and we want useful get_maintainer.pl output.

> 'query-kvm' is used when no KVM built it, so I'll let it in
> machine.json; the others seem to belong in machine.json too,
> with no particular justification.


Re: [PATCH v2 0/5] qapi: Restrict machine (and migration) specific commands
Posted by Markus Armbruster 3 years, 6 months ago
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Reduce the machine code pulled into qemu-storage-daemon.
>
> The series is fully Acked, but Markus wants it reviewed
> by the Machine core maintainers.

Queued, thanks!