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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("[Xen-devel] Re: [PATCH V3] libxl, Introduce a QMP 
client"):
> On Wed, 2011-06-15 at 19:44 +0100, Anthony PERARD wrote:
> > Actually, qemu doesn't remove the socket. So we will have do clean it
> > when the domain is cleaned. Is libxl_domain_destroy() a good function
> > to do that?
> 
> I think so. There must be a function somewhere which signals qemu to
> shutdown (right?) -- I guess that be a good place?

The hard domain destruction code would be the right place.  We should
unconditionally unlink the socket there (rather than try to guess
whether it ought to have been created).

> > It should be a float. But have 0.2 * 10^6 will be better to understand
> > that we have 0.2 second of sleep, but not power in C. If you prefere,
> > I will just wrote 200000, should be OK.
> 
> My concern was just the floating * int multiplication going into a
> function which takes an int. I'm sure it all works out through the up-
> and down-casting and constant propagation etc and turns into the right
> thing, I just wondered if we could avoid it and make things clearer at
> the same time.

I would suggest  200*1000  as that doesn't make it so hard to count
zeroes.  The function is called "usleep" so it's clear that the value
is in microseconds, and 200*1000 us is indeed 0.2s.

> > >> +    yajl_gen_map_open(hand);
> > >> +    yajl_gen_string(hand, (const unsigned char *)ex, strlen(ex));
> > >> +    yajl_gen_string(hand, (const unsigned char *)cmd, strlen(cmd));
> > >> +    yajl_gen_string(hand, (const unsigned char *)"id", 2);

These casts are very unpleasant but seem to be required due to the
broken yajl API.

> > > You'd think yajl would have yajl_gen_asciiz or something.
> > 
> > Unfortunatly, there is no such thing, at least in the debian version
> > (1.?). I did not check the 2.? version of libyajl.
> 
> Perhaps we should have our own little helper function/macro which does
> this?

Yes.  We can hide the cast in it too.

> > >> +    if (callback) {
> > >> +        callback_id_pair *elm = malloc(sizeof (callback_id_pair));

What if malloc fails ?

> > >> +    write(qmp->qmp_fd, buf, len);
> > >> +    write(qmp->qmp_fd, "\r\n", 2);
> > >
> > > These need to handle partial writes?
> > 
> > What did you mean ?
> > Call write twice was easier here than call a snprintf or other.
> 
> I meant handling of write() returning < len or EINTR, EAGAIN etc.

Quite so.

Ian.

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