[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]