Re: [PATCH v3 1/7] libxl: Add ERROR_DOMAIN_NOTFOUND for libxl_domain_info when it cannot find the domain

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

Please remember to CC all the relevant maintainers, e.g. Wei and Ian in
addition to me. MAINTAINERS is helpful here.

On Mon, 2015-03-23 at 14:20 -0400, Konrad Rzeszutek Wilk wrote:
> And use that for all of its callers in the tree.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

although I started a conversation in response to a similar patch from
Ian J regarding the semantics (I cc-d you).

> ---
>  tools/libxl/libxl.c         | 19 +++++++++++--------
>  tools/libxl/libxl.h         |  9 ++++++++-
>  tools/libxl/libxl_types.idl |  1 +
>  tools/libxl/xl_cmdimpl.c    |  4 ++--
>  4 files changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 46de4f7..0b57bae 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -455,7 +455,7 @@ int libxl__domain_rename(libxl__gc *gc, uint32_t domid,
>      /* update /vm/<uuid>/name */
>      rc = libxl_domain_info(ctx, &info, domid);
>      if (rc)
> -        goto x_fail;
> +        goto x_rc;
>  
>      uuid = GCSPRINTF(LIBXL_UUID_FMT, LIBXL_UUID_BYTES(info.uuid));
>      vm_name_path = GCSPRINTF("/vm/%s/name", uuid);
> @@ -698,7 +698,7 @@ int libxl_domain_info(libxl_ctx *ctx, libxl_dominfo *info_r,
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
>          return ERROR_FAIL;
>      }
> -    if (ret==0 || xcinfo.domain != domid) return ERROR_INVAL;
> +    if (ret==0 || xcinfo.domain != domid) return ERROR_DOMAIN_NOTFOUND;
>  
>      if (info_r)
>          xcinfo2xlinfo(ctx, &xcinfo, info_r);
> @@ -1577,7 +1577,7 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
>      switch(rc) {
>      case 0:
>          break;
> -    case ERROR_INVAL:
> +    case ERROR_DOMAIN_NOTFOUND:
>          LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "non-existant domain %d", domid);
>      default:
>          goto out;
> @@ -5449,14 +5449,16 @@ static int libxl__set_vcpuonline_xenstore(libxl__gc *gc, uint32_t domid,
>      libxl_dominfo info;
>      char *dompath;
>      xs_transaction_t t;
> -    int i, rc = ERROR_FAIL;
> +    int i, rc;
>  
>      libxl_dominfo_init(&info);
>  
> -    if (libxl_domain_info(CTX, &info, domid) < 0) {
> +    rc = libxl_domain_info(CTX, &info, domid);
> +    if (rc < 0) {
>          LOGE(ERROR, "getting domain info list");
>          goto out;
>      }
> +    rc = ERROR_FAIL;
>      if (!(dompath = libxl__xs_get_dompath(gc, domid)))
>          goto out;
>  
> @@ -5480,14 +5482,15 @@ static int libxl__set_vcpuonline_qmp(libxl__gc *gc, uint32_t domid,
>                                       libxl_bitmap *cpumap)
>  {
>      libxl_dominfo info;
> -    int i;
> +    int i, rc;
>  
>      libxl_dominfo_init(&info);
>  
> -    if (libxl_domain_info(CTX, &info, domid) < 0) {
> +    rc = libxl_domain_info(CTX, &info, domid);
> +    if (rc < 0) {
>          LOGE(ERROR, "getting domain info list");
>          libxl_dominfo_dispose(&info);
> -        return ERROR_FAIL;
> +        return rc;
>      }
>      for (i = 0; i <= info.vcpu_max_id; i++) {
>          if (libxl_bitmap_test(cpumap, i)) {
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 6bc75c5..1cf5699 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -179,6 +179,11 @@
>  #define LIBXL_HAVE_BUILDINFO_HVM_MMIO_HOLE_MEMKB 1
>  
>  /*
> + * libxl_domain_info returns ERROR_DOMAIN_NOTFOUND if the domain
> + * is not present, instead of ERROR_INVAL.
> + */
> +#define LIBXL_HAVE_ERROR_DOMAIN_NOTFOUND 1
> +/*
>   * libxl ABI compatibility
>   *
>   * The only guarantee which libxl makes regarding ABI compatibility
> @@ -1104,7 +1109,9 @@ int libxl_console_get_tty(libxl_ctx *ctx, uint32_t domid, int cons_num,
>   */
>  int libxl_primary_console_get_tty(libxl_ctx *ctx, uint32_t domid_vm, char **path);
>  
> -/* May be called with info_r == NULL to check for domain's existance */
> +/* May be called with info_r == NULL to check for domain's existence.
> + * Returns ERROR_DOMAIN_NOTFOUND if domain does not exist (used to return
> + * ERROR_INVAL for this scenario). */
>  int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
>                        uint32_t domid);
>  
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 0866433..117b61d 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -64,6 +64,7 @@ libxl_error = Enumeration("error", [
>      (-18, "REMUS_DEVOPS_DOES_NOT_MATCH"),
>      (-19, "REMUS_DEVICE_NOT_SUPPORTED"),
>      (-20, "VNUMA_CONFIG_INVALID"),
> +    (-21, "DOMAIN_NOTFOUND"),
>      ], value_namespace = "")
>  
>  libxl_domain_type = Enumeration("domain_type", [
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 394b55d..1c07ac6 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -4736,7 +4736,7 @@ int main_list(int argc, char **argv)
>      } else if (optind == argc-1) {
>          uint32_t domid = find_domain(argv[optind]);
>          rc = libxl_domain_info(ctx, &info_buf, domid);
> -        if (rc == ERROR_INVAL) {
> +        if (rc == ERROR_DOMAIN_NOTFOUND) {
>              fprintf(stderr, "Error: Domain \'%s\' does not exist.\n",
>                  argv[optind]);
>              return -rc;
> @@ -5507,7 +5507,7 @@ int main_sharing(int argc, char **argv)
>      } else if (optind == argc-1) {
>          uint32_t domid = find_domain(argv[optind]);
>          rc = libxl_domain_info(ctx, &info_buf, domid);
> -        if (rc == ERROR_INVAL) {
> +        if (rc == ERROR_DOMAIN_NOTFOUND) {
>              fprintf(stderr, "Error: Domain \'%s\' does not exist.\n",
>                  argv[optind]);
>              return -rc;