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
|