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

Re: [libvirt] [PATCH] conf: avoid NULL deref for pmsuspended domain state



On 2013年01月24日 08:12, Eric Blake wrote:
While working with a pmsuspend vs. snapshot issue, I noticed that
the state file in /var/run/libvirt/qemu/dom.xml contained a rather
suspicious "(null)" string, which does not round-trip well through
a libvirtd restart.  Had I been on a platform other than glibc
where printf("%s",NULL) crashes instead of printing (null), we might
have noticed the problem much sooner.

And in fixing that problem, I also noticed that we had several
missing states, because we were #defining several *_LAST names
to a value _different_ than what they were already given as enums
in libvirt.h.  Yuck.  I got rid of default: labels in the case
statements, because they get in the way of gcc's -Wswitch helping
us ensure we cover all enum values.

* src/conf/domain_conf.c (virDomainStateReasonToString)
(virDomainStateReasonFromString): Fill in missing domain states;
rewrite case statement to let compiler enforce checking.
(VIR_DOMAIN_NOSTATE_LAST, VIR_DOMAIN_RUNNING_LAST)
(VIR_DOMAIN_BLOCKED_LAST, VIR_DOMAIN_PAUSED_LAST)
(VIR_DOMAIN_SHUTDOWN_LAST, VIR_DOMAIN_SHUTOFF_LAST)
(VIR_DOMAIN_CRASHED_LAST): Drop dead defines.
(VIR_DOMAIN_PMSUSPENDED_LAST): Drop dead define.
(virDomainPMSuspendedReason): Add missing enum function.
(virDomainRunningReason, virDomainPausedReason): Add missing enum
value.
* src/conf/domain_conf.h (virDomainPMSuspendedReason): Declare
missing functions.
* src/libvirt_private.syms (domain_conf.h): Export them.
---
  src/conf/domain_conf.c   | 32 +++++++++++++++++++-------------
  src/conf/domain_conf.h   |  1 +
  src/libvirt_private.syms |  6 ++++--
  3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7273790..5782abb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -574,11 +574,9 @@ VIR_ENUM_IMPL(virDomainState, VIR_DOMAIN_LAST,
                "crashed",
                "pmsuspended")

-#define VIR_DOMAIN_NOSTATE_LAST (VIR_DOMAIN_NOSTATE_UNKNOWN + 1)
  VIR_ENUM_IMPL(virDomainNostateReason, VIR_DOMAIN_NOSTATE_LAST,
                "unknown")

-#define VIR_DOMAIN_RUNNING_LAST (VIR_DOMAIN_RUNNING_SAVE_CANCELED + 1)
  VIR_ENUM_IMPL(virDomainRunningReason, VIR_DOMAIN_RUNNING_LAST,
                "unknown",
                "booted",
@@ -587,13 +585,12 @@ VIR_ENUM_IMPL(virDomainRunningReason, VIR_DOMAIN_RUNNING_LAST,
                "from snapshot",
                "unpaused",
                "migration canceled",
-              "save canceled")
+              "save canceled",
+              "wakeup")

-#define VIR_DOMAIN_BLOCKED_LAST (VIR_DOMAIN_BLOCKED_UNKNOWN + 1)
  VIR_ENUM_IMPL(virDomainBlockedReason, VIR_DOMAIN_BLOCKED_LAST,
                "unknown")

-#define VIR_DOMAIN_PAUSED_LAST (VIR_DOMAIN_PAUSED_SHUTTING_DOWN + 1)
  VIR_ENUM_IMPL(virDomainPausedReason, VIR_DOMAIN_PAUSED_LAST,
                "unknown",
                "user",
@@ -603,14 +600,13 @@ VIR_ENUM_IMPL(virDomainPausedReason, VIR_DOMAIN_PAUSED_LAST,
                "ioerror",
                "watchdog",
                "from snapshot",
-              "shutdown")
+              "shutdown",
+              "snapshot")

-#define VIR_DOMAIN_SHUTDOWN_LAST (VIR_DOMAIN_SHUTDOWN_USER + 1)
  VIR_ENUM_IMPL(virDomainShutdownReason, VIR_DOMAIN_SHUTDOWN_LAST,
                "unknown",
                "user")

-#define VIR_DOMAIN_SHUTOFF_LAST (VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT + 1)
  VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST,
                "unknown",
                "shutdown",
@@ -621,10 +617,12 @@ VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST,
                "failed",
                "from snapshot")

-#define VIR_DOMAIN_CRASHED_LAST (VIR_DOMAIN_CRASHED_UNKNOWN + 1)
  VIR_ENUM_IMPL(virDomainCrashedReason, VIR_DOMAIN_CRASHED_LAST,
                "unknown")

+VIR_ENUM_IMPL(virDomainPMSuspendedReason, VIR_DOMAIN_PMSUSPENDED_LAST,
+              "unknown")
+
  VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOMAIN_SECLABEL_LAST,
                "default",
                "none",
@@ -15438,9 +15436,13 @@ virDomainStateReasonToString(virDomainState state, int reason)
          return virDomainShutoffReasonTypeToString(reason);
      case VIR_DOMAIN_CRASHED:
          return virDomainCrashedReasonTypeToString(reason);
-    default:
-        return NULL;
+    case VIR_DOMAIN_PMSUSPENDED:
+        return virDomainPMSuspendedReasonTypeToString(reason);
+    case VIR_DOMAIN_LAST:
+        break;
      }
+    VIR_WARN("Unexpected domain state: %d", state);
+    return NULL;
  }


@@ -15462,9 +15464,13 @@ virDomainStateReasonFromString(virDomainState state, const char *reason)
          return virDomainShutoffReasonTypeFromString(reason);
      case VIR_DOMAIN_CRASHED:
          return virDomainCrashedReasonTypeFromString(reason);
-    default:
-        return -1;
+    case VIR_DOMAIN_PMSUSPENDED:
+        return virDomainPMSuspendedReasonTypeFromString(reason);
+    case VIR_DOMAIN_LAST:
+        break;
      }
+    VIR_WARN("Unexpected domain state: %d", state);
+    return -1;
  }


diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index abbfe86..9a9e0b1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2315,6 +2315,7 @@ VIR_ENUM_DECL(virDomainPausedReason)
  VIR_ENUM_DECL(virDomainShutdownReason)
  VIR_ENUM_DECL(virDomainShutoffReason)
  VIR_ENUM_DECL(virDomainCrashedReason)
+VIR_ENUM_DECL(virDomainPMSuspendedReason)

  const char *virDomainStateReasonToString(virDomainState state, int reason);
  int virDomainStateReasonFromString(virDomainState state, const char *reason);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fc23adc..57ecc36 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -487,10 +487,12 @@ virDomainObjSetState;
  virDomainObjTaint;
  virDomainPausedReasonTypeFromString;
  virDomainPausedReasonTypeToString;
-virDomainPciRombarModeTypeFromString;
-virDomainPciRombarModeTypeToString;
  virDomainPMStateTypeFromString;
  virDomainPMStateTypeToString;
+virDomainPMSuspendedReasonTypeFromString;
+virDomainPMSuspendedReasonTypeToString;
+virDomainPciRombarModeTypeFromString;
+virDomainPciRombarModeTypeToString;
  virDomainRedirdevBusTypeFromString;
  virDomainRedirdevBusTypeToString;
  virDomainRemoveInactive;

ACK, with the emacs tail. But it will be nice to
use the emacs tail for all the private syms files
before pushing.

Osier


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