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

Re: [libvirt] [PATCH] virNetDevBandwidthClear: Improve error handling



On 09/18/12 15:52, Martin Kletzander wrote:
Two changes are introduced in this patch. The first change removes
ATTRIBUTE_RETURN_CHECK from virNetDevBandwidthClear, because it was
called with ignore_value always, anyway. The function is used even
when it's not necessary to call it, just for cleanup purposes. The
second change is an added parameter "ignore_error" for the same
function, which basically means "Ignore any errors from the commands
that will be run". This parameter, when true, doesn't suppress any
libvirt errors, just the exit value of commands ran.
---
  src/network/bridge_driver.c   |  4 ++--
  src/util/virnetdevbandwidth.c | 15 ++++++++++-----
  src/util/virnetdevbandwidth.h |  6 +++---
  3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0e38016..f153cdd 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2161,7 +2161,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
      return 0;

   err5:
-    ignore_value(virNetDevBandwidthClear(network->def->bridge));
+    virNetDevBandwidthClear(network->def->bridge, true);

   err4:
      if (!save_err)
@@ -2206,7 +2206,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
  static int networkShutdownNetworkVirtual(struct network_driver *driver,
                                          virNetworkObjPtr network)
  {
-    ignore_value(virNetDevBandwidthClear(network->def->bridge));
+    virNetDevBandwidthClear(network->def->bridge, true);

      if (network->radvdPid > 0) {
          char *radvdpidbase;
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index b23ccd3..98f6b68 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -1,5 +1,5 @@
  /*
- * Copyright (C) 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2009-2012 Red Hat, Inc.
   *
   * This library is free software; you can redistribute it and/or
   * modify it under the terms of the GNU Lesser General Public
@@ -69,7 +69,7 @@ virNetDevBandwidthSet(const char *ifname,
          goto cleanup;
      }

-    ignore_value(virNetDevBandwidthClear(ifname));
+    virNetDevBandwidthClear(ifname, true);


All 3 of the call paths for this function call it with ignore_error set to true. I think that it's not necessary to have the feature to turn on errors now, if nobody actually uses it.


      if (bandwidth->in) {
          if (virAsprintf(&average, "%llukbps", bandwidth->in->average) < 0)
@@ -163,15 +163,19 @@ cleanup:
   * Return 0 on success, -1 otherwise.
   */
  int
-virNetDevBandwidthClear(const char *ifname)
+virNetDevBandwidthClear(const char *ifname, bool ignore_error)
  {
      int ret = 0;
+    int exitstatus, *es = NULL;
      virCommandPtr cmd = NULL;

+    if (ignore_error)
+        es = &exitstatus;
+
      cmd = virCommandNew(TC);
      virCommandAddArgList(cmd, "qdisc", "del", "dev", ifname, "root", NULL);

-    if (virCommandRun(cmd, NULL) < 0)
+    if (virCommandRun(cmd, es) < 0)
          ret = -1;

      virCommandFree(cmd);
@@ -179,8 +183,9 @@ virNetDevBandwidthClear(const char *ifname)
      cmd = virCommandNew(TC);
      virCommandAddArgList(cmd, "qdisc",  "del", "dev", ifname, "ingress", NULL);

-    if (virCommandRun(cmd, NULL) < 0)
+    if (virCommandRun(cmd, es) < 0)
          ret = -1;
+
      virCommandFree(cmd);

      return ret;
diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
index 18384ae..f15b489 100644
--- a/src/util/virnetdevbandwidth.h
+++ b/src/util/virnetdevbandwidth.h
@@ -1,5 +1,5 @@
  /*
- * Copyright (C) 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2009-2012 Red Hat, Inc.
   *
   * This library is free software; you can redistribute it and/or
   * modify it under the terms of the GNU Lesser General Public
@@ -43,8 +43,8 @@ void virNetDevBandwidthFree(virNetDevBandwidthPtr def);

  int virNetDevBandwidthSet(const char *ifname, virNetDevBandwidthPtr bandwidth)
      ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
-int virNetDevBandwidthClear(const char *ifname)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+int virNetDevBandwidthClear(const char *ifname, bool ignore_error)
+    ATTRIBUTE_NONNULL(1);
  int virNetDevBandwidthCopy(virNetDevBandwidthPtr *dest, const virNetDevBandwidthPtr src)
      ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;


ACK if you leave out the ignore_error argument.

Peter


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