This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] Re: [PATCH] libxl, Introduce a QMP client

To: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] Re: [PATCH] libxl, Introduce a QMP client
From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
Date: Mon, 6 Jun 2011 17:15:33 +0100
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Mon, 06 Jun 2011 09:16:38 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:from :date:x-google-sender-auth:message-id:subject:to:cc:content-type :content-transfer-encoding; bh=Wvn+hYCL7y2kVGvbLnb3scQfW8kMreYkIs0/o5jLhxA=; b=g2Y3nxSW/Odo1zbA7hoF/n45/EpPBBmd7XeOvyEm7S+lGhDkjx7FFVAwiZbjJEshbE TMlTTVJ7IUBh/CEJyDIoYq3B8p50Eo/QQrhHLJqL4TmKb9+IIOsJgtd6x6T63ftV9mqj sCjJTozqIZOfNnfKm0g5gIPlXVBuTicyo5uR8=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:from:date :x-google-sender-auth:message-id:subject:to:cc:content-type :content-transfer-encoding; b=PFkLe/zQXMpgZYU72fHOMoGM69l9rcjEMyZZghFg9QQHwgekZY3Aq/z0l+aWHydrZQ 1IRn6Z7V2KK9e5+kJoMqX0Pn10Z5Lz24znKii9QgReKYuGaO3ff+kCVO7yuv4V3q4QnD krReSv+5d5l1zOwPZeOxUQ7Ubox/LRN48wJ6M=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1307375125.775.479.camel@xxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1307366804-310-1-git-send-email-anthony.perard@xxxxxxxxxx> <1307375125.775.479.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Mon, Jun 6, 2011 at 16:45, Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx> wrote:
> 2011-06-06 at 14:26 +0100, Anthony Perard wrote:
>> From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
>> QMP stands for QEMU Monitor Protocol and it is used to query information
>> from QEMU or to control QEMU.
>> This implementation will ask QEMU the list of chardevice and store the
>> path to serial0 in xenstored. So we will be able to use xl console with
>> QEMU upstream.
>> In order to connect to the QMP server, a socket file is created in
>> /var/run/xen/qmp-$(domid).
>> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> I didn't yet review this in detail but I have a few initial
> questions/thoughts.
> Am I correct that QMP is intended to provide a stable interface going
> forwards? Or are we tying ourselves to a particular qemu version and/or
> will we need version specific bits (and associated configuration stuff)
> in the future?

QMP is intended to provide a stable interface. Once a command have
been shipped, it should not been modified.

> I think we should try where possible to keep this stuff entirely within
> libxl. The existing libxl event API is a bit of a mess but I think if it
> were cleaned up (IanJ has a plan I think) then it would be the right
> place to integrate the libxl and caller event loops.
> For the time being though I think libxl should provide the fd and not
> expect the caller to construct the path and open it etc. IOW
> libxl_qmp_initialize should not take a socket option, it should
> construct the path, do the open internally and return the fd.

OK, I will do that, with a #define of the path somewhere.

>> -        ret = select(fd + 1, &rfds, NULL, NULL, NULL);
>> +        ret = select(fd > qmp_fd ? fd + 1 : qmp_fd + 1, &rfds, NULL, NULL, 
>> NULL);
>>          if (!ret)
>>              continue;
>> +        if (FD_ISSET(fd, &rfds)) {
>>          libxl_get_event(ctx, &event);
>>          switch (event.type) {
>>              case LIBXL_EVENT_TYPE_DOMAIN_DEATH:
>> @@ -1687,6 +1706,10 @@ start:
>>                  break;
>>          }
>>          libxl_free_event(&event);
>> +        }
>> +        if (FD_ISSET(qmp_fd, &rfds)) {
>> +            libxl_qmp_do_next(qmp_handler);
>> +        }
> Looks like some re-indentation is needed in these two hunks?

Oops, indeed, I forget to change that.


Anthony PERARD

Xen-devel mailing list