From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:41877) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RbA8S-0006bC-65 for qemu-devel@nongnu.org; Thu, 15 Dec 2011 07:10:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RbA8M-0003rY-7p for qemu-devel@nongnu.org; Thu, 15 Dec 2011 07:10:04 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43687) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RbA8L-0003r6-0q for qemu-devel@nongnu.org; Thu, 15 Dec 2011 07:09:57 -0500 Date: Thu, 15 Dec 2011 10:09:49 -0200 From: Luiz Capitulino Message-ID: <20111215100949.03cf048d@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 need to fully review this series to ack or nack this, but unfortunately I= 've had a busy week and will be on vacations for two weeks starting in a few ho= urs. I think it would be needed to have at least Kevin and Anthony or me acking = this.