From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:48081) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RiQiT-0003bP-Jz for qemu-devel@nongnu.org; Wed, 04 Jan 2012 08:17:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RiQiS-00015B-JR for qemu-devel@nongnu.org; Wed, 04 Jan 2012 08:17:17 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39145) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RiQiS-000157-AX for qemu-devel@nongnu.org; Wed, 04 Jan 2012 08:17:16 -0500 Date: Wed, 4 Jan 2012 11:17:05 -0200 From: Luiz Capitulino Message-ID: <20120104111705.6355b829@doriath> In-Reply-To: References: <1323784351-25531-1-git-send-email-stefanha@linux.vnet.ibm.com> <1323784351-25531-9-git-send-email-stefanha@linux.vnet.ibm.com> <4EE8B8BC.2040804@redhat.com> <20111215082758.GD26425@stefanha-thinkpad.localdomain> <4EE9CD1F.4010107@redhat.com> <20111215093018.79bc6814@doriath> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Marcelo Tosatti , Stefan Hajnoczi , qemu-devel@nongnu.org On Thu, 15 Dec 2011 12:00:16 +0000 Stefan Hajnoczi wrote: > On Thu, Dec 15, 2011 at 11:30 AM, Luiz Capitulino > wrote: > > On Thu, 15 Dec 2011 11:34:07 +0100 > > Kevin Wolf 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) > >> >>> =A0 =A0 =A0qapi_free_PciInfoList(info); > >> >>> =A0} > >> >>> > >> >>> +void hmp_info_block_jobs(Monitor *mon) > >> >>> +{ > >> >>> + =A0 =A0BlockJobInfoList *list; > >> >>> + =A0 =A0Error *err =3D NULL; > >> >>> + > >> >>> + =A0 =A0list =3D qmp_query_block_jobs(&err); > >> >>> + =A0 =A0assert(!err); > >> >>> + > >> >>> + =A0 =A0if (!list) { > >> >>> + =A0 =A0 =A0 =A0monitor_printf(mon, "No active jobs\n"); > >> >>> + =A0 =A0 =A0 =A0return; > >> >>> + =A0 =A0} > >> >>> + > >> >>> + =A0 =A0while (list) { > >> >>> + =A0 =A0 =A0 =A0/* The HMP output for streaming jobs is special b= ecause historically it > >> >>> + =A0 =A0 =A0 =A0 * was different from other job types so applicat= ions may depend on the > >> >>> + =A0 =A0 =A0 =A0 * exact string. > >> >>> + =A0 =A0 =A0 =A0 */ > >> >> > >> >> 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 jo= bs > >> > in the future. > >> > > >> > You can still build libvirt HMP-only by disabling the yajl library > >> > dependency. =A0The approach I've taken is to make the interfaces ava= ilable > >> > over both HMP and QMP (and so has the libvirt-side code). > >> > > >> > In any case, we have defined both HMP and QMP. =A0Libvirt 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. >=20 > 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. >=20 > 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. I'll have to nack this. Having stable guarantees with something that's not in the tree is just impossible, not even mentioning it's HMP. Having said that, the HMP string can be whatever you want. I will just ask you to drop the comment and won't refuse patches that change/improve it.