Re: [Xen-devel] [PATCH] libxl: make is_hvm log on errors, and make callers check its error returns

[Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>]

On Mon, 2010-08-23 at 17:59 +0100, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> ---
>  tools/libxl/libxl.c     |   31 +++++++++++++++++++++----------
>  tools/libxl/libxl_dom.c |   14 ++++++++++----
>  tools/libxl/libxl_pci.c |   14 ++++++++------
>  3 files changed, 39 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 6eb17e8..c31e6bf 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -427,9 +427,10 @@ out:
>  int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid)
>  {
>      libxl_gc gc = LIBXL_INIT_GC(ctx);
> -    int rc = 0;
> +    int hvm, rc = 0;
>  
> -    if (is_hvm(ctx, domid)) {
> +    hvm = is_hvm(ctx, domid);  if (hvm<0) { rc = hvm; goto out; }
> +    if (hvm) {
>          XL_LOG(ctx, XL_LOG_DEBUG, "Called domain_resume on "
>                  "non-cooperative hvm domain %u", domid);
>          rc = ERROR_NI;
> @@ -645,11 +646,13 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *ctx, int *nb_vm)
>  int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
>                           uint32_t domid, int fd)
>  {
> -    int hvm = is_hvm(ctx, domid);
> +    int hvm;
>      int live = info != NULL && info->flags & XL_SUSPEND_LIVE;
>      int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
>      int rc = 0;
>  
> +    hvm = is_hvm(ctx, domid);  if (hvm<0) return hvm;
> +

I hate this coding style, it's really easy to miss whats going on when
scanning code and two things are done on one line. I think either lines
ought to be added or change declaration to:

int get_vm_mode(libxl_ctx, int domid, int *hvm);

so at least compiler can tell you if you're using it the way it was
intended.

Gianni


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