[libvirt] [PATCH] error: add new error type for reflecting partial API support

[Michal Privoznik]

On 21.07.2011 20:46, Eric Blake wrote:
> VIR_ERR_INVALID_ARG implies that an argument cannot possibly
> be correct, given the current state of the API.
> VIR_ERR_CONFIG_UNSUPPORTED implies that a configuration is
> wrong, but arguments aren't configuration.
> VIR_ERR_NO_SUPPORT implies that a function is completely
> unimplemented.
>
> But in the case of a function that is partially implemented,
> yet the full power of the API is not available for that
> driver, none of the above messages make sense.  Hence a new
> error message, implying that the argument is known to comply
> with the current state of the API, and that while the driver
> supports aspects of the function, it does not support that
> particular use of the argument.
>
> A good use case for this is a driver that supports
> virDomainSaveFlags, but not the dxml argument of that API.
>
> It might be feasible to also use this new error for all functions
> that check flags, and which accept fewer flags than what is possible
> in the public API.  But doing so would get complicated, since
> neither libvirt.c nor the remote driver may do flag filtering,
> and every other driver would have to do a two-part check, first
> using virCheckFlags on all public flags (which gives
> VIR_ERR_INVALID_ARG for an impossible flag), followed by a
> particular mask check for VIR_ERR_ARGUMENT_UNSUPPORTED (for a
> possible public flag but unsupported by this driver).
>
> * include/libvirt/virterror.h (VIR_ERR_ARGUMENT_UNSUPPORTED): New
> error.
> * src/util/virterror.c (virErrorMsg): Give it a message.
> Suggested by Daniel P. Berrange.
> ---
>
> Needed before v3 of 7/16 of the bypass-cache series:
> https://www.redhat.com/archives/libvir-list/2011-July/msg01470.html
>
>   include/libvirt/virterror.h |    4 +++-
>   src/util/virterror.c        |    6 ++++++
>   2 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index efa4796..9cac437 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -4,7 +4,7 @@
>    * Description: Provides the interfaces of the libvirt library to handle
>    *              errors raised while using the library.
>    *
> - * Copy:  Copyright (C) 2006, 2010 Red Hat, Inc.
> + * Copy:  Copyright (C) 2006, 2010-2011 Red Hat, Inc.
>    *
>    * See COPYING.LIB for the License of this software
>    *
> @@ -232,6 +232,8 @@ typedef enum {
>       VIR_ERR_INVALID_DOMAIN_SNAPSHOT = 71,/* invalid domain snapshot */
>       VIR_ERR_NO_DOMAIN_SNAPSHOT = 72,	/* domain snapshot not found */
>       VIR_ERR_INVALID_STREAM = 73,        /* stream pointer not valid */
> +    VIR_ERR_ARGUMENT_UNSUPPORTED = 74,  /* valid API use but unsupported by
> +                                           the given driver */
>   } virErrorNumber;
>
>   /**
> diff --git a/src/util/virterror.c b/src/util/virterror.c
> index 07f8b45..9a27feb 100644
> --- a/src/util/virterror.c
> +++ b/src/util/virterror.c
> @@ -1183,6 +1183,12 @@ virErrorMsg(virErrorNumber error, const char *info)
>               else
>                   errmsg = _("invalid stream pointer in %s");
>               break;
> +        case VIR_ERR_ARGUMENT_UNSUPPORTED:
> +            if (info == NULL)
> +                errmsg = _("argument unsupported");
> +            else
> +                errmsg = _("argument unsupported: %s");
> +            break;
>       }
>       return (errmsg);
>   }
ACK.

I like patches where commit message is longer than actual diff :)

Michal