Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs

[Luiz Capitulino]

On Thu, 15 Dec 2011 11:34:07 +0100
Kevin Wolf <address@hidden> wrote:

> Am 15.12.2011 09:27, schrieb Stefan Hajnoczi:
> > On Wed, Dec 14, 2011 at 03:54:52PM +0100, Kevin Wolf wrote:
> >> Am 13.12.2011 14:52, schrieb Stefan Hajnoczi:
> >>> diff --git a/hmp.c b/hmp.c
> >>> index 66d9d0f..c16d6a1 100644
> >>> --- a/hmp.c
> >>> +++ b/hmp.c
> >>> @@ -499,6 +499,46 @@ void hmp_info_pci(Monitor *mon)
> >>>      qapi_free_PciInfoList(info);
> >>>  }
> >>>  
> >>> +void hmp_info_block_jobs(Monitor *mon)
> >>> +{
> >>> +    BlockJobInfoList *list;
> >>> +    Error *err = NULL;
> >>> +
> >>> +    list = qmp_query_block_jobs(&err);
> >>> +    assert(!err);
> >>> +
> >>> +    if (!list) {
> >>> +        monitor_printf(mon, "No active jobs\n");
> >>> +        return;
> >>> +    }
> >>> +
> >>> +    while (list) {
> >>> +        /* The HMP output for streaming jobs is special because 
> >>> historically it
> >>> +         * was different from other job types so applications may depend 
> >>> on the
> >>> +         * exact string.
> >>> +         */
> >>
> >> Er, what? This is new code. What HMP clients use this string? I know
> >> that libvirt already got support for this before we implemented it, but
> >> shouldn't that be QMP only?
> > 
> > Libvirt HMP uses this particular string, which turned out to be
> > sub-optimal once I realized we might support other types of block jobs
> > in the future.
> > 
> > You can still build libvirt HMP-only by disabling the yajl library
> > dependency.  The approach I've taken is to make the interfaces available
> > over both HMP and QMP (and so has the libvirt-side code).
> > 
> > In any case, we have defined both HMP and QMP.  Libvirt implements both
> > and I don't think there's a reason to provide only QMP.
> > 
> > Luiz: For future features, are we supposed to provide only QMP
> > interfaces, not HMP?
> 
> Of course, qemu should provide them as HMP command. But libvirt
> shouldn't use HMP commands. HMP is intended for human users, not as an
> API for management.

That's correct.

What defines if you're going to have a HMP version of a command is if
you expect it to be used by humans and if you do all its output and
arguments should be user friendly. You should never expect nor assume
it's going to be used by a management interface.

> 
> And I was pretty sure that we all agreed that HMP should be considered
> unstable after a transition period. For new commands there's certainly
> no reason to have a transition period, so I would consider them unstable
> from the very beginning. After all, there are no qemu versions that
> support the feature in question, but don't support QMP.
> 
> Kevin
>