Re: [PATCH for 4.5] xl: Return proper error codes for block-attach and block-detach

[Ian Campbell <Ian.Campbell@citrix.com>]

On Wed, 2014-11-12 at 17:31 +0000, George Dunlap wrote:
> @@ -6444,6 +6445,7 @@ int main_blockdetach(int argc, char **argv)
>      rc = libxl_device_disk_remove(ctx, domid, &disk, 0);
>      if (rc) {
>          fprintf(stderr, "libxl_device_disk_remove failed.\n");
> +        return 1;
>      }
>      libxl_device_disk_dispose(&disk);
>      return rc;

This one seems odd to me, you have inadvertently arranged to skip the
disk dispose and the old code returned non-zero in the failure case
already, since it returns rc.

If it is important to return 0 or 1 then perhaps the last line should
just be:
        return rc ? 1 : 0;
        
Or maybe the ultimate caller (i.e. the xl command dispatcher) ought to
be translating libxl ERROR_FOO into suitable process exit codes (perhaps
as simplistically as suggested above).

Skipping the dispose is a memory leak, albeit in a process which is just
about to die, but we like to try and keep xl "valgrind clean" so far as
possible such that leaks in the underlying libxl stand out.

Ian.