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

Re: [libvirt] [PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use



于 2011年08月25日 05:47, Daniel P. Berrange 写道:
On Tue, Aug 23, 2011 at 05:39:41PM +0800, Osier Yang wrote:
* src/qemu/qemu_command.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/

* src/qemu/qemu_driver.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/

* src/qemu/qemu_process.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
---
  src/qemu/qemu_command.c |    4 ++--
  src/qemu/qemu_driver.c  |   16 ++++++++--------
  src/qemu/qemu_process.c |    4 ++--
  3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index dbfc7d9..287ad8d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4084,7 +4084,7 @@ qemuBuildCommandLine(virConnectPtr conn,
          switch(console->targetType) {
          case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
              if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
-                qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+                qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                      _("virtio channel requires QEMU to support -device"));
                  goto error;
              }
@@ -4109,7 +4109,7 @@ qemuBuildCommandLine(virConnectPtr conn,
              break;

          default:
-            qemuReportError(VIR_ERR_NO_SUPPORT,
+            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                              _("unsupported console target type %s"),
                              NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType)));
              goto error;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c8dda73..fc2538a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1552,7 +1552,7 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) {
              vm = NULL;
      } else {
  #endif
-        qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+        qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
                          _("Reboot is not supported without the JSON monitor"));
  #if HAVE_YAJL
      }
@@ -3309,7 +3309,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
                                            cpumap, maplen, maxcpu)<  0)
                  goto cleanup;
          } else {
-            qemuReportError(VIR_ERR_NO_SUPPORT,
+            qemuReportError(VIR_ERR_OPERATION_INVALID,
                              "%s", _("cpu affinity is not supported"));
              goto cleanup;
          }
@@ -3563,7 +3563,7 @@ qemudDomainGetVcpus(virDomainPtr dom,
                          goto cleanup;
                  }
              } else {
-                qemuReportError(VIR_ERR_NO_SUPPORT,
+                qemuReportError(VIR_ERR_OPERATION_INVALID,
                                  "%s", _("cpu affinity is not available"));
                  goto cleanup;
              }
@@ -5637,7 +5637,7 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,
          }

          if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
-            qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't mounted"));
+            qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup isn't mounted"));
              goto cleanup;
          }

@@ -5790,7 +5790,7 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
          }

          if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
-            qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't mounted"));
+            qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup isn't mounted"));
              goto cleanup;
          }

THe use of VIR_ERR_OPERATION_INVALID is not correct here, but I'm not
certain what other error is best. Perhaps ARGUMENT_UNSUPPORTED

For VIR_ERR_OPERATION_INVALID:

    errmsg = _("Requested operation is not valid");

For VIR_ERR_ARGUMENT_UNSUPPORTED:

    errmsg = _("argument unsupported");

From the user's point of view, I think OPERATION_INVALID is more clear
and sensiable. E.g.
   "Requested operation is not valid: blkio cgroup isn't mounted"

   "argument unsupported: blkio cgroup isn't mounted"

Could we just extend the meaning for OPERATION_INVALID
so that it's not just used when the object operated on is not in
correct state, and used for things like above?

Otherwise, I'd prefer INTERNAL_ERROR here.


@@ -6887,8 +6887,8 @@ qemudDomainInterfaceStats (virDomainPtr dom,
                             const char *path ATTRIBUTE_UNUSED,
                             struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
  {
-    qemuReportError(VIR_ERR_NO_SUPPORT,
-                    "%s", __FUNCTION__);
+    qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                    _("interface stats not implemented on this platform"));
      return -1;
  }
  #endif

The original code was correct here.


@@ -8004,7 +8004,7 @@ qemuCPUCompare(virConnectPtr conn,
      qemuDriverLock(driver);

      if (!driver->caps || !driver->caps->host.cpu) {
-        qemuReportError(VIR_ERR_NO_SUPPORT,
+        qemuReportError(VIR_ERR_OPERATION_INVALID,
                          "%s", _("cannot get host CPU capabilities"));
      } else {
          ret = cpuCompareXML(driver->caps->host.cpu, xmlDesc);
The use of OPERATION_INVALID is not correct here. Perhaps
ARGUMENT_UNSUPPORTED

perhaps INTERNAL_ERROR is better here, as translated error string
for ARGUMENT_UNSUPPORTED is quite confused.


diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6f54b30..616c8e2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -261,7 +261,7 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn,
      if (conn->secretDriver == NULL ||
          conn->secretDriver->lookupByUUID == NULL ||
          conn->secretDriver->getValue == NULL) {
-        qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+        qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
                          _("secret storage not supported"));
          goto cleanup;
      }
NO_SUPPORT was the right value here.

@@ -1464,7 +1464,7 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn,
          return 0;

      if (priv->vcpupids == NULL) {
-        qemuReportError(VIR_ERR_NO_SUPPORT,
+        qemuReportError(VIR_ERR_OPERATION_INVALID,
                          "%s", _("cpu affinity is not supported"));
          return -1;
      }
Perhaps ARGUMENT_UNSUPPORTED

Perhaps INTERNAL_ERROR?


Daniel


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