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