On Fri, 2010-09-17 at 15:02 +0100, Jun Zhu (Intern) wrote:
> This version adds gc as a parameter to libxl__xs_open, and uses
> LIBXL__LOG_ERRNO for logging. If some functions cannot use the ctx, It should
> transfer NULL to libxl__xs_open to disable logging. (But it will make users
> difficult to find the problem when no logging is output.)
> To make consistent with other functions in libxl__xshelp.c, I use gc as its
> parameter, not ctx. In the libxl_ctx_init function, I add “libxl__gc gc =
> LIBXL_INIT_GC(ctx)” to get the gc of ctx. Please check this.
>
> Signed-off-by: Jun Zhu <Jun.Zhu@xxxxxxxxxx>
>
> diff -r cca905a429aa tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c Tue Sep 14 15:39:36 2010 +0100
> +++ b/tools/libxl/libxl.c Fri Sep 17 14:58:50 2010 +0100
> @@ -40,6 +40,8 @@
>
> int libxl_ctx_init(libxl_ctx *ctx, int version, xentoollog_logger *lg)
> {
> + libxl__gc gc = LIBXL_INIT_GC(ctx);
> +
If you add one of these then you also need to add a libxl__free_all(&gc)
on every exit path from the function.
Special care needs to be taken in libxl_ctx_init to ensure the ctx is
initialised enough to be able to create a gc. I suspect it is safe as is
but perhaps moving the "gc = LIBXL_INIT_GC(ctx)" later in the function
will prevent accidents? e.g. put it just before this:
+ ctx->xsh = libxl__xs_open(&gc);
if (!ctx->xsh) {
xc_interface_close(ctx->xch);
return ERROR_FAIL;
> diff -r cca905a429aa tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c Tue Sep 14 15:39:36 2010 +0100
> +++ b/tools/libxl/libxl_dom.c Fri Sep 17 14:58:50 2010 +0100
> @@ -326,14 +326,16 @@
>
> snprintf(path, sizeof(path),
> "/local/domain/0/device-model/%u/logdirty/cmd", domid);
>
> - xsh = xs_daemon_open();
> + xsh = libxl__xs_open(NULL);
> + if (!xsh)
> + return;
This is libxl__domain_suspend_common_switch_qemu_logdirty[0] which is
used as a callback from xc_domain_suspend. IMHO any function which takes
a callback should also take a void * closure, which in this case could
be used to pass the context from the caller of xc_domain_save to this
function. Given that xc does not do this I suppose this is fine as is.
[0] BTW please add this to your ~/.hgrc to automatically annotate
functions in diffs:
"""
[diff]
showfunc = True
"""
> diff -r cca905a429aa tools/libxl/libxl_utils.c
> --- a/tools/libxl/libxl_utils.c Tue Sep 14 15:39:36 2010 +0100
> +++ b/tools/libxl/libxl_utils.c Fri Sep 17 14:58:50 2010 +0100
> @@ -366,9 +366,11 @@
>
>
> int libxl_ctx_postfork(libxl_ctx *ctx) {
> + libxl__gc gc = LIBXL_INIT_GC(ctx);
> if (ctx->xsh) xs_daemon_destroy_postfork(ctx->xsh);
> - ctx->xsh = xs_daemon_open();
> - if (!ctx->xsh) return ERROR_FAIL;
> + ctx->xsh = libxl__xs_open(&gc);
> + if (!ctx->xsh)
> + return ERROR_FAIL;
> return 0;
> }
Another place where libxl__free_all(&gc) is needed.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home