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

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Mon, Jun 27, 2011 at 17:17, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> wrote:
> Anthony PERARD writes ("[Xen-devel] [PATCH V5 3/3] libxl, Introduce a QMP 
> client"):
>> 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.
>
> Can I make a suggestion ?  I think the formulaic json parser stuff
> could usefully live in a separate file.

Ok, I will cut the file.

>> +static inline yajl_gen_status yajl_gen_asciiz(yajl_gen hand, const
> char *str)
>
> Isn't this a hostage to fortune ?  yajl may grow an identically-named
> function in which case this will no longer build.

Maybe. I will rename to libxl__yajl_gen_asciiz.

>> +#ifdef DEBUG_ANSWER
>> +    if (qmp->g)
>> +        yajl_gen_free(qmp->g);
>> +#endif
>
> Can this #ifdef not be shuffled off somewhere ?  Ie, make a debug
> function (or macro) which we call unconditionally.

Ok, I will remove all the #ifdef debug_answer.

> The rest looks plausible.

Thanks for the review,

-- 
Anthony PERARD

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