[libvirt] [PATCH 1/2] conf: parse integers into long long, instead of long

Daniel P. Berrange berrange at redhat.com
Wed Mar 18 12:36:47 UTC 2015


When parsing integer values, we only used 'long' data type
in the virConfValue struct. This is insufficiently large
to deal with things like guest memory sizes on 32-bit platforms
which are using PAE for addressing > 4 GB of RAM.
---
 daemon/libvirtd-config.c          |   6 +--
 src/locking/lock_daemon_config.c  |   4 +-
 src/locking/lock_driver_lockd.c   |   4 +-
 src/locking/lock_driver_sanlock.c |   6 +--
 src/lxc/lxc_conf.c                |   6 +--
 src/qemu/qemu_conf.c              |   6 +--
 src/util/virconf.c                |  22 ++++----
 src/util/virconf.h                |   6 +--
 src/xenconfig/xen_common.c        |   8 +--
 tests/Makefile.am                 |   5 ++
 tests/libvirtdconftest.c          |   4 +-
 tests/virconftest.c               | 105 ++++++++++++++++++++++++++++++++++++++
 12 files changed, 146 insertions(+), 36 deletions(-)
 create mode 100644 tests/virconftest.c

diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
index 3694455..9344ccc 100644
--- a/daemon/libvirtd-config.c
+++ b/daemon/libvirtd-config.c
@@ -151,8 +151,8 @@ checkType(virConfValuePtr p, const char *filename,
     do {                                                                \
         virConfValuePtr p = virConfGetValue(conf, #var_name);           \
         if (p) {                                                        \
-            if (p->type != VIR_CONF_ULONG &&                            \
-                checkType(p, filename, #var_name, VIR_CONF_LONG) < 0)   \
+            if (p->type != VIR_CONF_ULLONG &&                           \
+                checkType(p, filename, #var_name, VIR_CONF_LLONG) < 0)  \
                 goto error;                                             \
             data->var_name = p->l;                                      \
         }                                                               \
@@ -163,7 +163,7 @@ checkType(virConfValuePtr p, const char *filename,
     do {                                                                \
         virConfValuePtr p = virConfGetValue(conf, #var_name);           \
         if (p) {                                                        \
-            if (checkType(p, filename, #var_name, VIR_CONF_ULONG) < 0)  \
+            if (checkType(p, filename, #var_name, VIR_CONF_ULLONG) < 0) \
                 goto error;                                             \
             data->var_name = p->l;                                      \
         }                                                               \
diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c
index 8a6d18f..20718a2 100644
--- a/src/locking/lock_daemon_config.c
+++ b/src/locking/lock_daemon_config.c
@@ -75,7 +75,7 @@ checkType(virConfValuePtr p, const char *filename,
     do {                                                                \
         virConfValuePtr p = virConfGetValue(conf, #var_name);           \
         if (p) {                                                        \
-            if (p->type != VIR_CONF_ULONG &&                            \
+            if (p->type != VIR_CONF_ULLONG &&                           \
                 checkType(p, filename, #var_name, VIR_CONF_LONG) < 0)   \
                 goto error;                                             \
             data->var_name = p->l;                                      \
@@ -87,7 +87,7 @@ checkType(virConfValuePtr p, const char *filename,
     do {                                                                \
         virConfValuePtr p = virConfGetValue(conf, #var_name);           \
         if (p) {                                                        \
-            if (checkType(p, filename, #var_name, VIR_CONF_ULONG) < 0)  \
+            if (checkType(p, filename, #var_name, VIR_CONF_ULLONG) < 0) \
                 goto error;                                             \
             data->var_name = p->l;                                      \
         }                                                               \
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 72a4a0c..59cdd45 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -108,7 +108,7 @@ static int virLockManagerLockDaemonLoadConfig(const char *configFile)
     }
 
     p = virConfGetValue(conf, "auto_disk_leases");
-    CHECK_TYPE("auto_disk_leases", VIR_CONF_ULONG);
+    CHECK_TYPE("auto_disk_leases", VIR_CONF_ULLONG);
     if (p) driver->autoDiskLease = p->l;
 
     p = virConfGetValue(conf, "file_lockspace_dir");
@@ -142,7 +142,7 @@ static int virLockManagerLockDaemonLoadConfig(const char *configFile)
     }
 
     p = virConfGetValue(conf, "require_lease_for_disks");
-    CHECK_TYPE("require_lease_for_disks", VIR_CONF_ULONG);
+    CHECK_TYPE("require_lease_for_disks", VIR_CONF_ULLONG);
     if (p)
         driver->requireLeaseForDisks = p->l;
     else
diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index dbe79bc..88631df 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -127,7 +127,7 @@ static int virLockManagerSanlockLoadConfig(const char *configFile)
     }
 
     p = virConfGetValue(conf, "auto_disk_leases");
-    CHECK_TYPE("auto_disk_leases", VIR_CONF_ULONG);
+    CHECK_TYPE("auto_disk_leases", VIR_CONF_ULLONG);
     if (p) driver->autoDiskLease = p->l;
 
     p = virConfGetValue(conf, "disk_lease_dir");
@@ -141,11 +141,11 @@ static int virLockManagerSanlockLoadConfig(const char *configFile)
     }
 
     p = virConfGetValue(conf, "host_id");
-    CHECK_TYPE("host_id", VIR_CONF_ULONG);
+    CHECK_TYPE("host_id", VIR_CONF_ULLONG);
     if (p) driver->hostID = p->l;
 
     p = virConfGetValue(conf, "require_lease_for_disks");
-    CHECK_TYPE("require_lease_for_disks", VIR_CONF_ULONG);
+    CHECK_TYPE("require_lease_for_disks", VIR_CONF_ULLONG);
     if (p)
         driver->requireLeaseForDisks = p->l;
     else
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
index d1a3be5..955a19d 100644
--- a/src/lxc/lxc_conf.c
+++ b/src/lxc/lxc_conf.c
@@ -270,7 +270,7 @@ virLXCLoadDriverConfig(virLXCDriverConfigPtr cfg,
     }
 
     p = virConfGetValue(conf, "log_with_libvirtd");
-    CHECK_TYPE("log_with_libvirtd", VIR_CONF_ULONG);
+    CHECK_TYPE("log_with_libvirtd", VIR_CONF_ULLONG);
     if (p) cfg->log_libvirtd = p->l;
 
     p = virConfGetValue(conf, "security_driver");
@@ -283,11 +283,11 @@ virLXCLoadDriverConfig(virLXCDriverConfigPtr cfg,
     }
 
     p = virConfGetValue(conf, "security_default_confined");
-    CHECK_TYPE("security_default_confined", VIR_CONF_ULONG);
+    CHECK_TYPE("security_default_confined", VIR_CONF_ULLONG);
     if (p) cfg->securityDefaultConfined = p->l;
 
     p = virConfGetValue(conf, "security_require_confined");
-    CHECK_TYPE("security_require_confined", VIR_CONF_ULONG);
+    CHECK_TYPE("security_require_confined", VIR_CONF_ULLONG);
     if (p) cfg->securityRequireConfined = p->l;
 
 
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 2cf3905..bca05c9 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -475,19 +475,19 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 
 #define GET_VALUE_LONG(NAME, VAR)                               \
     p = virConfGetValue(conf, NAME);                            \
-    CHECK_TYPE_ALT(NAME, VIR_CONF_LONG, VIR_CONF_ULONG);        \
+    CHECK_TYPE_ALT(NAME, VIR_CONF_LLONG, VIR_CONF_ULLONG);      \
     if (p)                                                      \
         VAR = p->l;
 
 #define GET_VALUE_ULONG(NAME, VAR)    \
     p = virConfGetValue(conf, NAME);  \
-    CHECK_TYPE(NAME, VIR_CONF_ULONG); \
+    CHECK_TYPE(NAME, VIR_CONF_ULLONG);\
     if (p)                            \
         VAR = p->l;
 
 #define GET_VALUE_BOOL(NAME, VAR)     \
     p = virConfGetValue(conf, NAME);  \
-    CHECK_TYPE(NAME, VIR_CONF_ULONG); \
+    CHECK_TYPE(NAME, VIR_CONF_ULLONG);\
     if (p)                            \
         VAR = p->l != 0;
 
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 01e5a6a..9182b17 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -82,8 +82,8 @@ struct _virConfParserCtxt {
 
 VIR_ENUM_IMPL(virConf, VIR_CONF_LAST,
               "*unexpected*",
-              "long",
-              "unsigned long",
+              "long long",
+              "unsigned long long",
               "string",
               "list");
 
@@ -273,9 +273,9 @@ virConfSaveValue(virBufferPtr buf, virConfValuePtr val)
     switch (val->type) {
         case VIR_CONF_NONE:
             return -1;
-        case VIR_CONF_LONG:
-        case VIR_CONF_ULONG:
-            virBufferAsprintf(buf, "%ld", val->l);
+        case VIR_CONF_LLONG:
+        case VIR_CONF_ULLONG:
+            virBufferAsprintf(buf, "%lld", val->l);
             break;
         case VIR_CONF_STRING:
             if (strchr(val->str, '\n') != NULL) {
@@ -346,7 +346,7 @@ virConfSaveEntry(virBufferPtr buf, virConfEntryPtr cur)
  ************************************************************************/
 
 /**
- * virConfParseLong:
+ * virConfParseLLong:
  * @ctxt: the parsing context
  * @val: the result
  *
@@ -355,9 +355,9 @@ virConfSaveEntry(virBufferPtr buf, virConfEntryPtr cur)
  * Returns 0 in case of success and -1 in case of error
  */
 static int
-virConfParseLong(virConfParserCtxtPtr ctxt, long *val)
+virConfParseLLong(virConfParserCtxtPtr ctxt, long long *val)
 {
-    long l = 0;
+    long long l = 0;
     int neg = 0;
 
     if (CUR == '-') {
@@ -467,7 +467,7 @@ virConfParseValue(virConfParserCtxtPtr ctxt)
     virConfValuePtr ret, lst = NULL, tmp, prev;
     virConfType type = VIR_CONF_NONE;
     char *str = NULL;
-    long  l = 0;
+    long long  l = 0;
 
     SKIP_BLANKS;
     if (ctxt->cur >= ctxt->end) {
@@ -536,8 +536,8 @@ virConfParseValue(virConfParserCtxtPtr ctxt)
                          _("numbers not allowed in VMX format"));
             return NULL;
         }
-        type = (c_isdigit(CUR) || CUR == '+') ? VIR_CONF_ULONG : VIR_CONF_LONG;
-        if (virConfParseLong(ctxt, &l) < 0)
+        type = (c_isdigit(CUR) || CUR == '+') ? VIR_CONF_ULLONG : VIR_CONF_LLONG;
+        if (virConfParseLLong(ctxt, &l) < 0)
             return NULL;
     } else {
         virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("expecting a value"));
diff --git a/src/util/virconf.h b/src/util/virconf.h
index 2b4b943..971bb5e 100644
--- a/src/util/virconf.h
+++ b/src/util/virconf.h
@@ -33,8 +33,8 @@
  */
 typedef enum {
     VIR_CONF_NONE = 0,      /* undefined */
-    VIR_CONF_LONG,          /* a long int */
-    VIR_CONF_ULONG,         /* an unsigned long int */
+    VIR_CONF_LLONG,          /* a long long int */
+    VIR_CONF_ULLONG,         /* an unsigned long long int */
     VIR_CONF_STRING,        /* a string */
     VIR_CONF_LIST,          /* a list */
     VIR_CONF_LAST,          /* sentinel */
@@ -61,7 +61,7 @@ typedef virConfValue *virConfValuePtr;
 struct _virConfValue {
     virConfType type;		/* the virConfType */
     virConfValuePtr next;	/* next element if in a list */
-    long  l;			/* long integer */
+    long long  l;		/* long long integer */
     char *str;			/* pointer to 0 terminated string */
     virConfValuePtr list;	/* list of a list */
 };
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index 5f8eb71..f53e137 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -57,7 +57,7 @@ xenConfigGetBool(virConfPtr conf,
         return 0;
     }
 
-    if (val->type == VIR_CONF_ULONG) {
+    if (val->type == VIR_CONF_ULLONG) {
         *value = val->l ? 1 : 0;
     } else if (val->type == VIR_CONF_STRING) {
         *value = STREQ(val->str, "1") ? 1 : 0;
@@ -87,7 +87,7 @@ xenConfigGetULong(virConfPtr conf,
         return 0;
     }
 
-    if (val->type == VIR_CONF_ULONG) {
+    if (val->type == VIR_CONF_ULLONG) {
         *value = val->l;
     } else if (val->type == VIR_CONF_STRING) {
         if (virStrToLong_ul(val->str, NULL, 10, value) < 0) {
@@ -121,7 +121,7 @@ xenConfigGetULongLong(virConfPtr conf,
         return 0;
     }
 
-    if (val->type == VIR_CONF_ULONG) {
+    if (val->type == VIR_CONF_ULLONG) {
         *value = val->l;
     } else if (val->type == VIR_CONF_STRING) {
         if (virStrToLong_ull(val->str, NULL, 10, value) < 0) {
@@ -275,7 +275,7 @@ xenConfigSetInt(virConfPtr conf, const char *setting, long long l)
     if (VIR_ALLOC(value) < 0)
         return -1;
 
-    value->type = VIR_CONF_LONG;
+    value->type = VIR_CONF_LLONG;
     value->next = NULL;
     value->l = l;
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9277c13..05ee574 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -148,6 +148,7 @@ EXTRA_DIST =		\
 test_helpers = commandhelper ssh test_conf
 test_programs = virshtest sockettest \
 	nodeinfotest virbuftest \
+	virconftest \
 	commandtest seclabeltest \
 	virhashtest \
 	viratomictest \
@@ -1128,6 +1129,10 @@ virbuftest_SOURCES = \
 	virbuftest.c testutils.h testutils.c
 virbuftest_LDADD = $(LDADDS)
 
+virconftest_SOURCES = \
+	virconftest.c testutils.h testutils.c
+virconftest_LDADD = $(LDADDS)
+
 virhashtest_SOURCES = \
 	virhashtest.c virhashdata.h testutils.h testutils.c
 virhashtest_LDADD = $(LDADDS)
diff --git a/tests/libvirtdconftest.c b/tests/libvirtdconftest.c
index d589d51..3c4b3cd 100644
--- a/tests/libvirtdconftest.c
+++ b/tests/libvirtdconftest.c
@@ -65,7 +65,7 @@ munge_param(const char *datain,
         if (c_isspace(*tmp))
             continue;
         if (c_isdigit(*tmp)) {
-            *type = VIR_CONF_ULONG;
+            *type = VIR_CONF_ULLONG;
             replace = "\"foo\"";
         } else if (*tmp == '[') {
             *type = VIR_CONF_LIST;
@@ -130,7 +130,7 @@ testCorrupt(const void *opaque)
 #endif
 
     switch (type) {
-    case VIR_CONF_ULONG:
+    case VIR_CONF_ULLONG:
         if (!strstr(err->message, "invalid type: got string; expected unsigned long") &&
             !strstr(err->message, "invalid type: got string; expected long")) {
             VIR_DEBUG("Wrong error for long: '%s'",
diff --git a/tests/virconftest.c b/tests/virconftest.c
new file mode 100644
index 0000000..b481c92
--- /dev/null
+++ b/tests/virconftest.c
@@ -0,0 +1,105 @@
+#include <config.h>
+
+#include "testutils.h"
+#include "virconf.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+static int testConfDataTypes(const void *data ATTRIBUTE_UNUSED)
+{
+    static const char *confdata =
+        "longlongval = -9223372036854775806\n"
+        "ulonglongval = 18446744073709551615\n"
+        "stringval = \"The annoyed programmer spent the whole day shaving yaks\"\n"
+        "listval = [\"Hello\", -9223372036854775806, 18446744073709551615, \"World\"]\n";
+    virConfPtr conf;
+    virConfValuePtr val;
+    int ret = -1;
+
+    if (!(conf = virConfReadMem(confdata,
+                                strlen(confdata),
+                                0)))
+        goto cleanup;
+
+#define GET_SETTING(setting) \
+    if (!(val = virConfGetValue(conf, setting))) {              \
+        fprintf(stderr, "Missing %s entry\n", setting);         \
+        goto cleanup;                                           \
+    }
+
+#define CHECK_INT(datatype, field, value, format)                       \
+    if (val->type != datatype) {                                        \
+        fprintf(stderr, "Expecting type %d, but got type %d\n",         \
+                datatype, val->type);                                   \
+        goto cleanup;                                                   \
+    }                                                                   \
+    if (val->field != value) {                                          \
+        fprintf(stderr, "Expecting " format " but got " format "\n",    \
+                value, val->field);                                     \
+        goto cleanup;                                                   \
+    }
+
+#define CHECK_STRING(value)                                             \
+    if (val->type != VIR_CONF_STRING) {                                 \
+        fprintf(stderr, "Expecting type %d, but got type %d\n",         \
+                VIR_CONF_STRING, val->type);                            \
+        goto cleanup;                                                   \
+    }                                                                   \
+    if (STRNEQ(val->str, value)) {                                      \
+        fprintf(stderr, "Expecting '%s' but got '%s'\n",                \
+                value, val->str);                                       \
+        goto cleanup;                                                   \
+    }
+
+#define CHECK_LIST()                                                    \
+    if (val->type != VIR_CONF_LIST) {                                   \
+        fprintf(stderr, "Expecting type %d, but got type %d\n",         \
+                VIR_CONF_LIST, val->type);                              \
+        goto cleanup;                                                   \
+    }                                                                   \
+
+    GET_SETTING("longlongval");
+    CHECK_INT(VIR_CONF_LLONG, l, -9223372036854775806LL, "%lld");
+
+    GET_SETTING("ulonglongval");
+    CHECK_INT(VIR_CONF_ULLONG, l, 18446744073709551615ULL, "%llu");
+
+    GET_SETTING("stringval");
+    CHECK_STRING("The annoyed programmer spent the whole day shaving yaks");
+
+    GET_SETTING("listval");
+    CHECK_LIST();
+
+    val = val->list;
+    CHECK_STRING("Hello");
+    val = val->next;
+    CHECK_INT(VIR_CONF_LLONG, l, -9223372036854775806LL, "%lld");
+    val = val->next;
+    CHECK_INT(VIR_CONF_ULLONG, l, 18446744073709551615ULL, "%llu");
+    val = val->next;
+    CHECK_STRING("World");
+
+    ret = 0;
+ cleanup:
+    virConfFree(conf);
+    return ret;
+}
+
+
+static int
+mymain(void)
+{
+    int ret = 0;
+
+#define DO_TEST(msg, cb)                                               \
+    do {                                                               \
+        if (virtTestRun("Conf: " msg, cb, NULL) < 0)                   \
+            ret = -1;                                                  \
+    } while (0)
+
+    DO_TEST("Data types", testConfDataTypes);
+
+    return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+VIRT_TEST_MAIN(mymain)
-- 
2.1.0




More information about the libvir-list mailing list