RE: [Xen-devel] [PATCH] libxl: make libxl communicate with xenstored by socket or xenbus driver

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Jun Zhu (Intern) writes ("RE: [Xen-devel] [PATCH] libxl: make libxl communicate 
with xenstored by socket or xenbus driver"):
> I make another one.

Thanks.  (It helps if you say "v2" in the Subject line.)

> -    rc = xs_writev(xsh, XBT_NULL, starting->dom_path, kvs);
> -    if (rc)
> -        return;
> -    xs_daemon_close(xsh);
> +     /* we mustn't use the parent's handle in the child */
> +     xsh = libxl_xs_open();

This, and some of the other parts of your patch, have what seem to be
unintended whitespace and formatting changes.  Can you please make
sure that:
  * Your indents are exactly 4 spaces
  * You don't put any tabs in (your MUA may mangle them, and anyway
     we don't like tabs)
  * Curly backets (aka braces) should go on the same line as the
    if and else.

> +    if (!xsh)
> +     {
> +             fprintf(stderr, "cannot connect to xenstore");
> +             free(kvs[1]);
> +             return;
> +     }
> +    xs_writev(xsh, XBT_NULL, starting->dom_path, kvs);
> +    libxl_xs_close(xsh);
> +
> +     free(kvs[1]);

This would be better written with the "goto out" idiom, I think ?

> -    ctx->xsh = xs_daemon_open();
> -    if (!ctx->xsh) return ERROR_FAIL;
> -    return 0;
> +     ctx->xsh = libxl_xs_open();
> +     if (!ctx->xsh)
> +     {
> +             XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno,
> +                             "cannot connect to xenstore");

This logging, if it is appropriate, should surely be done in
libxl_xs_open, rather than coded out in each call ?

Thanks,
Ian.

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