RE: [Xen-devel] [PATCH] Clear device-model information when destroying an HVM domain with stubdom

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Jun Zhu (Intern) writes ("RE: [Xen-devel] [PATCH] Clear device-model 
information when destroying an HVM domain with stubdom"):
> I make a new path from the newest unstable version.

Thanks.  Like your previous patch, this seems to have some whitespace
and formatting errors.  Also, please try to keep your lines to 75
characters or less.

>          ret = libxl_domain_destroy(ctx, stubdomid, 0);
> -        goto out;
> +        if (!ret)
> +             {
> +                     XL_LOG(ctx, XL_LOG_ERROR, "Destroy stubdom failed, 
> domid=%d\n", stubdomid);

Surely libxl_domain_destroy will have already logged errors so there
is no need to do so again ?  Also I think you have the sense of the
error test reversed: ret==0 is success so just
  if (ret) goto out;
will do.

> +                     ret = 0;
> +             } else if (ret == 0) {
...
> +                     XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "failed to kill Device 
> Model [%d]",
> +                                     atoi(pid));
> +             }
> +     }
> +     if (!ret)
> +         xs_rm(ctx->xsh, XBT_NULL, libxl_sprintf(&gc, 
> "/local/domain/0/device-model/%d", domid));
>  out:

Your logic here seems inconsistent.  In the one case you skip the
removal by the use of "goto out" and in the other you test "ret".
Normally I think the "goto out" approach is probably better here.

Ian.

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