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


RE: [Xen-devel] [PATCH] libxl: make libxl communicate with xenstored by

To: "Jun Zhu (Intern)" <Jun.Zhu@xxxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH] libxl: make libxl communicate with xenstored by socket or xenbus driver
From: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Date: Thu, 9 Sep 2010 18:09:52 +0100
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Thu, 09 Sep 2010 10:10:51 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <433DDF91DFB08148BAD3FDB6FDDA314C9F35F3BB60@xxxxxxxxxxxxxxxxxxxxxxxxx>
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: <433DDF91DFB08148BAD3FDB6FDDA314C9F35F3BB60@xxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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 ?


Xen-devel mailing list

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