WARNING - OLD ARCHIVES

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/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH V6 3/3] libxl, Introduce a QMP client
From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
Date: Fri, 8 Jul 2011 18:19:41 +0100
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Delivery-date: Fri, 08 Jul 2011 10:21:06 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; 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; bh=kq3GqD27rXCC+2RVMDROYpDQDBtatKKr7xr+V1ioiqM=; b=deAawLJGCz8QFuh5+ZU2DOInrznb+6pAwQ4dWky7sPglJPJIhBOBs7WtMEBI9mBHG3 6AZh9VyEDsjgOWqA6Ytd1XQZBFSICWWQaT4gYPC8KMVsHve1mXbi9nZuMUGCVAjFE6+R hKdYt2bAT09hI9GkSj8iuOqKop3Mh19IPYZjY=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <19987.19099.184523.836257@xxxxxxxxxxxxxxxxxxxxxxxx>
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: <1309455045-3062-1-git-send-email-anthony.perard@xxxxxxxxxx> <1309455045-3062-4-git-send-email-anthony.perard@xxxxxxxxxx> <19987.19099.184523.836257@xxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Tue, Jul 5, 2011 at 18:32, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> wrote:
>
>> +static void qmp_handle_error_response(libxl__qmp_handler *qmp,
>> +                                      const libxl__json_object *resp)
>> +{
>> +    callback_id_pair *pp = qmp_get_callback_from_id(qmp, resp);
> ...
>> +    if (pp) {
>> +        if (pp->id == qmp->wait_for_id) {
>> +            /* tell that the id have been processed */
>> +            qmp->wait_for_id = 0;
>> +        }
>> +        SIMPLEQ_REMOVE(&qmp->callback_list, pp, callback_id_pair, next);
>> +        free(pp);
>
> I think this needs to call the callback, or the code which set up the
> callback will surely just continue to wait forever.

Ok, I will just call the callback with NULL, so, the callback will
fail explicitly. That will not change anything for the only callback I
have.

> Later: I see that you have a single wait_for_id.  What if multiple
> callers want to use a single qmp for multiple things ?  Or is
> "wait_for_id" simply the one that you're waiting on synchronously ?

The second.
Multiple callers can call qmp_send_synchronous multiple time.

>> +static int qmp_next(libxl__qmp_handler *qmp)
>> +{
> ...
>> +        ret = select(qmp->qmp_fd + 1, &rfds, NULL, NULL, &timeout);
>> +        if (ret > 0) {
>> +            rd = read(qmp->qmp_fd, qmp->buffer, QMP_RECEIVE_BUFFER_SIZE);
>> +            if (rd > 0) {
>> +                break;
> ...
>> +    s = qmp->buffer;
>> +    s_end = qmp->buffer + rd;
>> +    while (s < s_end) {
>> +        libxl__json_object *o = libxl__json_parse(qmp->ctx, &qmp->yajl_ctx,
>> +                                                  &s, s_end - s);
>
> This assumes that the response will be received in a single read().
> This is not correct.  read() may return partial results; it may also
> aggregate multiple writes from the sending qemu into a single read()
> result.

Actually, json_parse takes care of these. It tells if it's a partiel
result and how much of the buffer have been read. So the function
assume that the caller will call qmp_next again if it's needed (like
in case a callback have not been called).
I tried the function with a small buffer (not enough for an entire
response) and it works fine.

> You need to pull data into the buffer and then test the buffer for
> completeness (eg by looking for the cr-lf), and split the buffer up
> into packets yourself, and if there are partial packets left over
> go round and read again.

At least, I will check if the protocole is respected, and I will not
relie anymore on json_parse to tell me the end of a packet.

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>