| Message ID | fc2539281a00353777c10f37dea6ccdb6aae4285.1504112256.git.jcody@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Live block optional disable | expand |
On Wed, Aug 30, 2017 at 01:01:41PM -0400, Jeff Cody wrote: > From: Jeffrey Cody <jcody@redhat.com> > > If configured without live block operations enabled, unregister the > live block operation commands. > > Signed-off-by: Jeff Cody <jcody@redhat.com> > --- > monitor.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/monitor.c b/monitor.c > index e0f8801..de0a70e 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -998,6 +998,22 @@ static void qmp_unregister_commands_hack(void) > && !defined(TARGET_S390X) > qmp_unregister_command(&qmp_commands, "query-cpu-definitions"); > #endif > +#ifndef CONFIG_LIVE_BLOCK_OPS > + qmp_unregister_command(&qmp_commands, "block-stream"); > + qmp_unregister_command(&qmp_commands, "block-commit"); > + qmp_unregister_command(&qmp_commands, "drive-mirror"); > + qmp_unregister_command(&qmp_commands, "blockdev-mirror"); > + qmp_unregister_command(&qmp_commands, "drive-backup"); > + qmp_unregister_command(&qmp_commands, "blockdev-backup"); > + qmp_unregister_command(&qmp_commands, "blockdev-snapshot"); > + qmp_unregister_command(&qmp_commands, "blockdev-snapshot-sync"); > + qmp_unregister_command(&qmp_commands, "block-job-set-speed"); > + qmp_unregister_command(&qmp_commands, "block-job-cancel"); > + qmp_unregister_command(&qmp_commands, "block-job-pause"); > + qmp_unregister_command(&qmp_commands, "block-job-resume"); > + qmp_unregister_command(&qmp_commands, "block-job-complete"); > + qmp_unregister_command(&qmp_commands, "query-block-jobs"); > +#endif I suggest using the new mechanisms added by: [PATCH 00/26] qapi: add #if pre-processor conditions to generated code
On 08/30/2017 12:24 PM, Eduardo Habkost wrote: > On Wed, Aug 30, 2017 at 01:01:41PM -0400, Jeff Cody wrote: >> From: Jeffrey Cody <jcody@redhat.com> >> >> If configured without live block operations enabled, unregister the >> live block operation commands. >> >> Signed-off-by: Jeff Cody <jcody@redhat.com> >> --- >> monitor.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> > > I suggest using the new mechanisms added by: > > [PATCH 00/26] qapi: add #if pre-processor conditions to generated code Those haven't landed yet, but as both series are proposed for 2.11, I indeed agree that basing this series on top of that one will be a bit cleaner.
Eric Blake <eblake@redhat.com> writes: > On 08/30/2017 12:24 PM, Eduardo Habkost wrote: >> On Wed, Aug 30, 2017 at 01:01:41PM -0400, Jeff Cody wrote: >>> From: Jeffrey Cody <jcody@redhat.com> >>> >>> If configured without live block operations enabled, unregister the >>> live block operation commands. >>> >>> Signed-off-by: Jeff Cody <jcody@redhat.com> >>> --- >>> monitor.c | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> > >> >> I suggest using the new mechanisms added by: >> >> [PATCH 00/26] qapi: add #if pre-processor conditions to generated code > > Those haven't landed yet, but as both series are proposed for 2.11, I > indeed agree that basing this series on top of that one will be a bit > cleaner. Rebasing shouldn't be hard. However, we then have to hold it until the QAPI series lands. I don't think holding is necessary, as the conflicts between the two are obvious, and should be straightforward to resolve.
Am 30.08.2017 um 19:01 hat Jeff Cody geschrieben: > From: Jeffrey Cody <jcody@redhat.com> > > If configured without live block operations enabled, unregister the > live block operation commands. > > Signed-off-by: Jeff Cody <jcody@redhat.com> How sure are we that libvirt will like a qemu that advertises these commands in the schema, but doesn't actually provide them? Kevin
On Wed, Sep 06, 2017 at 03:00:41PM +0200, Kevin Wolf wrote: > Am 30.08.2017 um 19:01 hat Jeff Cody geschrieben: > > From: Jeffrey Cody <jcody@redhat.com> > > > > If configured without live block operations enabled, unregister the > > live block operation commands. > > > > Signed-off-by: Jeff Cody <jcody@redhat.com> > > How sure are we that libvirt will like a qemu that advertises these > commands in the schema, but doesn't actually provide them? If libvirt wants to know if a command is available, it uses 'query-commands', not 'query-qmp-schema'. The only query-qmp-schema elements used by latest libvirt are gluster-related blockdev-add options (see libvirt/src/qemu/qemu_capabilities.c:virQEMUCapsQMPSchemaQueries]]).
On 09/06/2017 10:02 AM, Eduardo Habkost wrote: > On Wed, Sep 06, 2017 at 03:00:41PM +0200, Kevin Wolf wrote: >> Am 30.08.2017 um 19:01 hat Jeff Cody geschrieben: >>> From: Jeffrey Cody <jcody@redhat.com> >>> >>> If configured without live block operations enabled, unregister the >>> live block operation commands. >>> >>> Signed-off-by: Jeff Cody <jcody@redhat.com> >> >> How sure are we that libvirt will like a qemu that advertises these >> commands in the schema, but doesn't actually provide them? > > If libvirt wants to know if a command is available, it uses > 'query-commands', not 'query-qmp-schema'. > > The only query-qmp-schema elements used by latest libvirt are > gluster-related blockdev-add options (see > libvirt/src/qemu/qemu_capabilities.c:virQEMUCapsQMPSchemaQueries]]). Indeed, libvirt is currently just fine with the fact that query-commands introspection hides disabled commands, even if they are still leaked in query-qmp-schema. Besides, Marc-Andre's work on adding #if support to QAPI should probably also land in 2.11, at which point the hack goes away (because then we ARE properly hiding things from the schema introspection).
diff --git a/monitor.c b/monitor.c index e0f8801..de0a70e 100644 --- a/monitor.c +++ b/monitor.c @@ -998,6 +998,22 @@ static void qmp_unregister_commands_hack(void) && !defined(TARGET_S390X) qmp_unregister_command(&qmp_commands, "query-cpu-definitions"); #endif +#ifndef CONFIG_LIVE_BLOCK_OPS + qmp_unregister_command(&qmp_commands, "block-stream"); + qmp_unregister_command(&qmp_commands, "block-commit"); + qmp_unregister_command(&qmp_commands, "drive-mirror"); + qmp_unregister_command(&qmp_commands, "blockdev-mirror"); + qmp_unregister_command(&qmp_commands, "drive-backup"); + qmp_unregister_command(&qmp_commands, "blockdev-backup"); + qmp_unregister_command(&qmp_commands, "blockdev-snapshot"); + qmp_unregister_command(&qmp_commands, "blockdev-snapshot-sync"); + qmp_unregister_command(&qmp_commands, "block-job-set-speed"); + qmp_unregister_command(&qmp_commands, "block-job-cancel"); + qmp_unregister_command(&qmp_commands, "block-job-pause"); + qmp_unregister_command(&qmp_commands, "block-job-resume"); + qmp_unregister_command(&qmp_commands, "block-job-complete"); + qmp_unregister_command(&qmp_commands, "query-block-jobs"); +#endif } void monitor_init_qmp_commands(void)