[Top][All Lists]
[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: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs |
Date: |
Thu, 15 Dec 2011 13:37:51 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20111115 Thunderbird/8.0 |
Am 15.12.2011 13:00, schrieb Stefan Hajnoczi:
> 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.
We probably can, but I would prefer fixing it in libvirt. Possibly the
right fix there would be to remove it entirely from the HMP code - if I
understand right, the HMP code is only meant to support older qemu
versions anyway.
I also don't quite understand why there even is an option to disable QMP
in libvirt, we have always told that HMP will become unstable.
Kevin
- [Qemu-devel] [PATCH v3 0/9] block: generic image streaming, Stefan Hajnoczi, 2011/12/13
- [Qemu-devel] [PATCH v3 1/9] coroutine: add co_sleep_ns() coroutine sleep function, Stefan Hajnoczi, 2011/12/13
- [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs, Stefan Hajnoczi, 2011/12/13
- Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs, Kevin Wolf, 2011/12/14
- Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs, Stefan Hajnoczi, 2011/12/15
- Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs, Kevin Wolf, 2011/12/15
- Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs, Luiz Capitulino, 2011/12/15
- Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs, Stefan Hajnoczi, 2011/12/15
- Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs, Luiz Capitulino, 2011/12/15
- Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs,
Kevin Wolf <=
- Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs, Stefan Hajnoczi, 2011/12/15
[Qemu-devel] [PATCH v3 4/9] block: rate-limit streaming operations, Stefan Hajnoczi, 2011/12/13
[Qemu-devel] [PATCH v3 6/9] qmp: add block_job_set_speed command, Stefan Hajnoczi, 2011/12/13
[Qemu-devel] [PATCH v3 5/9] qmp: add block_stream command, Stefan Hajnoczi, 2011/12/13
[Qemu-devel] [PATCH v3 9/9] test: add image streaming test cases, Stefan Hajnoczi, 2011/12/13
[Qemu-devel] [PATCH v3 3/9] block: add image streaming block job, Stefan Hajnoczi, 2011/12/13
[Qemu-devel] [PATCH v3 2/9] block: add BlockJob interface for long-running operations, Stefan Hajnoczi, 2011/12/13