qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs
Date: Thu, 15 Dec 2011 12:00:16 +0000

On Thu, Dec 15, 2011 at 11:30 AM, Luiz Capitulino
<address@hidden> wrote:
> 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.

Okay, thanks Kevin and Luiz for explaining.  In this case I know it is
used by a management interface because libvirt has code to use it.

I was my mistake to include the HMP side as part of the "API" but it's
here now and I think we can live with this.

Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]