mbox series

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

Message ID 20201012121536.3381997-1-philmd@redhat.com
Headers show
Series qapi: Restrict machine (and migration) specific commands | expand

Message

Philippe Mathieu-Daudé Oct. 12, 2020, 12:15 p.m. UTC
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

Comments

Eduardo Habkost Oct. 15, 2020, 6:28 p.m. UTC | #1
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
Markus Armbruster Oct. 19, 2020, 7:55 a.m. UTC | #2
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.
Eduardo Habkost Oct. 19, 2020, 12:30 p.m. UTC | #3
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
Markus Armbruster Oct. 19, 2020, 4:48 p.m. UTC | #4
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.
Philippe Mathieu-Daudé Oct. 19, 2020, 6:04 p.m. UTC | #5
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.
Markus Armbruster Oct. 20, 2020, 5:39 a.m. UTC | #6
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.
Markus Armbruster Oct. 20, 2020, 9:32 a.m. UTC | #7
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!