On Thu, Dec 15, 2011 at 01:37:51PM +0100, Kevin Wolf wrote: > Am 15.12.2011 13:00, schrieb Stefan Hajnoczi: > > On Thu, Dec 15, 2011 at 11:30 AM, Luiz Capitulino > > <lcapitul...@redhat.com> wrote: > >> On Thu, 15 Dec 2011 11:34:07 +0100 > >> Kevin Wolf <kw...@redhat.com> 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.
Yeah, that's a good discussion to have. We need to get everyone on the same page. I am starting a new thread where we can discuss this with libvir-list and qemu-devel. Stefan