The virconftest is different from all our other tests in that the C program only tests a single in/out config file pair. It relies on a shell wrapper to invoke it once for each test file. This gets rid of the shell wrapper and makes the C program actually run over each test file using the normal test pattern. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- tests/Makefile.am | 7 ++--- tests/virconftest.c | 78 ++++++++++++++++++++++++++++++++++++++++++---------- tests/virconftest.sh | 26 ------------------ 3 files changed, 67 insertions(+), 44 deletions(-) delete mode 100755 tests/virconftest.sh diff --git a/tests/Makefile.am b/tests/Makefile.am index fb2380d..51a8179 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -158,11 +158,11 @@ EXTRA_DIST = \ xml2sexprdata \ xml2vmxdata -test_helpers = commandhelper ssh virconftest +test_helpers = commandhelper ssh test_programs = virshtest sockettest \ virhostcputest virbuftest \ commandtest seclabeltest \ - virhashtest \ + virhashtest virconftest \ viratomictest \ utiltest shunloadtest \ virtimetest viruritest virkeyfiletest \ @@ -360,7 +360,6 @@ test_scripts = libvirtd_test_scripts = \ libvirtd-fail \ libvirtd-pool \ - virconftest.sh \ virsh-cpuset \ virsh-define-dev-segfault \ virsh-int-overflow \ @@ -893,7 +892,7 @@ virshtest_SOURCES = \ virshtest_LDADD = $(LDADDS) virconftest_SOURCES = \ - virconftest.c + virconftest.c testutils.h testutils.c virconftest_LDADD = $(LDADDS) virhostcputest_SOURCES = \ diff --git a/tests/virconftest.c b/tests/virconftest.c index 4d05d8d..c71b491 100644 --- a/tests/virconftest.c +++ b/tests/virconftest.c @@ -1,3 +1,24 @@ +/* + * virconftest.c: Test the config file API + * + * Copyright (C) 2006-2016 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + */ + #include <config.h> #include <unistd.h> @@ -7,42 +28,71 @@ #include <errno.h> #include "virconf.h" #include "viralloc.h" +#include "testutils.h" + + +#define VIR_FROM_THIS VIR_FROM_NONE -int main(int argc, char **argv) +static int testConfRoundTrip(const void *opaque) { - int ret, exit_code = EXIT_FAILURE; + const char *name = opaque; + int ret = -1; virConfPtr conf = NULL; int len = 10000; char *buffer = NULL; + char *srcfile = NULL; + char *dstfile = NULL; - if (argc != 2) { - fprintf(stderr, "Usage: %s conf_file\n", argv[0]); + if (virAsprintf(&srcfile, "%s/virconfdata/%s.conf", + abs_srcdir, name) < 0 || + virAsprintf(&dstfile, "%s/virconfdata/%s.out", + abs_srcdir, name) < 0) goto cleanup; - } if (VIR_ALLOC_N_QUIET(buffer, len) < 0) { fprintf(stderr, "out of memory\n"); goto cleanup; } - conf = virConfReadFile(argv[1], 0); + conf = virConfReadFile(srcfile, 0); if (conf == NULL) { - fprintf(stderr, "Failed to process %s\n", argv[1]); + fprintf(stderr, "Failed to process %s\n", srcfile); goto cleanup; } ret = virConfWriteMem(buffer, &len, conf); if (ret < 0) { - fprintf(stderr, "Failed to serialize %s back\n", argv[1]); - goto cleanup; - } - if (fwrite(buffer, 1, len, stdout) != len) { - fprintf(stderr, "Write failed: %s\n", strerror(errno)); + fprintf(stderr, "Failed to serialize %s back\n", srcfile); goto cleanup; } - exit_code = EXIT_SUCCESS; + if (virTestCompareToFile(buffer, dstfile) < 0) + goto cleanup; + ret = 0; cleanup: + VIR_FREE(srcfile); + VIR_FREE(dstfile); VIR_FREE(buffer); virConfFree(conf); - return exit_code; + return ret; } + + +static int +mymain(void) +{ + int ret = 0; + + if (virTestRun("fc4", testConfRoundTrip, "fc4") < 0) + ret = -1; + + if (virTestRun("libvirtd", testConfRoundTrip, "libvirtd") < 0) + ret = -1; + + if (virTestRun("no-newline", testConfRoundTrip, "no-newline") < 0) + ret = -1; + + return ret; +} + + +VIRT_TEST_MAIN(mymain) diff --git a/tests/virconftest.sh b/tests/virconftest.sh deleted file mode 100755 index 0fd5bbe..0000000 --- a/tests/virconftest.sh +++ /dev/null @@ -1,26 +0,0 @@ -#!/bin/sh - -. "$(dirname $0)/test-lib.sh" - -test_intro $this_test - -fail=0 -i=0 -data_dir=$abs_srcdir/confdata -for f in $(cd "$data_dir" && echo *.conf) -do - i=`expr $i + 1` - "$abs_builddir/test_conf" "$data_dir/$f" > "$f-actual" - expected="$data_dir"/`echo "$f" | sed s+\.conf$+\.out+` - if compare "$expected" "$f-actual"; then - ret=0 - else - ret=1 - fail=1 - fi - test_result $i "$f" $ret -done - -test_final $i $fail - -(exit $fail); exit $fail -- 2.7.4

Currently many users of virConf APIs are defining the same macros for calling virConfValue() and then doing type checking. To remove this repeated code, add a set of typesafe accessor methods. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/libvirt_private.syms | 10 + src/util/virconf.c | 500 ++++++++++++++++++++++++++++++++++++++++++++++- src/util/virconf.h | 34 +++- tests/virconftest.c | 335 +++++++++++++++++++++++++++++++ 4 files changed, 873 insertions(+), 6 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ad0af76..597ce5f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1393,6 +1393,16 @@ virRun; virConfFree; virConfFreeValue; virConfGetValue; +virConfGetValueBool; +virConfGetValueInt; +virConfGetValueLLong; +virConfGetValueSizeT; +virConfGetValueSSizeT; +virConfGetValueString; +virConfGetValueStringList; +virConfGetValueType; +virConfGetValueUInt; +virConfGetValueULLong; virConfLoadConfig; virConfNew; virConfReadFile; diff --git a/src/util/virconf.c b/src/util/virconf.c index 33e7744..5085768 100644 --- a/src/util/virconf.c +++ b/src/util/virconf.c @@ -99,7 +99,7 @@ struct _virConfEntry { }; struct _virConf { - const char* filename; + char *filename; unsigned int flags; virConfEntryPtr entries; }; @@ -204,10 +204,15 @@ static virConfPtr virConfCreate(const char *filename, unsigned int flags) { virConfPtr ret = virConfNew(); - if (ret) { - ret->filename = filename; - ret->flags = flags; + if (!ret) + return NULL; + + if (VIR_STRDUP(ret->filename, filename) < 0) { + VIR_FREE(ret); + return NULL; } + + ret->flags = flags; return ret; } @@ -233,6 +238,7 @@ virConfAddEntry(virConfPtr conf, char *name, virConfValuePtr value, char *comm) if ((comm == NULL) && (name == NULL)) return NULL; + VIR_DEBUG("Add entry %s %p", name, value); if (VIR_ALLOC(ret) < 0) return NULL; @@ -275,8 +281,10 @@ virConfSaveValue(virBufferPtr buf, virConfValuePtr val) case VIR_CONF_NONE: return -1; case VIR_CONF_LONG: + virBufferAsprintf(buf, "%lld", val->l); + break; case VIR_CONF_ULONG: - virBufferAsprintf(buf, "%ld", val->l); + virBufferAsprintf(buf, "%llu", val->l); break; case VIR_CONF_STRING: if (strchr(val->str, '\n') != NULL) { @@ -843,6 +851,7 @@ virConfFree(virConfPtr conf) VIR_FREE(tmp); tmp = next; } + VIR_FREE(conf->filename); VIR_FREE(conf); return 0; } @@ -877,6 +886,487 @@ virConfGetValue(virConfPtr conf, const char *setting) return NULL; } + +/** + * virConfGetValueType: + * @conf: the config object + * @setting: the config entry name + * + * Query the type of the configuration entry @setting. + * + * Returns: the entry type, or VIR_CONF_NONE if not set. + */ +virConfType virConfGetValueType(virConfPtr conf, + const char *setting) +{ + virConfValuePtr cval = virConfGetValue(conf, setting); + if (!cval) + return VIR_CONF_NONE; + + return cval->type; +} + + +/** + * virConfGetValueString: + * @conf: the config object + * @setting: the config entry name + * @value: pointer to hold string value + * + * Get the string value of the config name @setting, storing + * it in @value. If the config entry is not present, then + * @value will be unmodified. + * + * Reports an error if the config entry is set but has + * an unexpected type. + * + * Returns: 1 if the value was present, 0 if missing, -1 on error + */ +int virConfGetValueString(virConfPtr conf, + const char *setting, + char **value) +{ + virConfValuePtr cval = virConfGetValue(conf, setting); + + VIR_DEBUG("Get value string %p %d", + cval, cval ? cval->type : VIR_CONF_NONE); + + if (!cval) + return 0; + + if (cval->type != VIR_CONF_STRING) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: expected a string for '%s' parameter"), + conf->filename, setting); + return -1; + } + + VIR_FREE(*value); + if (VIR_STRDUP(*value, cval->str) < 0) + return -1; + + return 1; +} + + +/** + * virConfGetValueStringList: + * @conf: the config object + * @setting: the config entry name + * @compatString: true to treat string entry as a 1 element list + * @value: pointer to hold NULL terminated string list + * + * Get the string list value of the config name @setting, storing + * it in @value. If the config entry is not present, then + * @value will be unmodified. If @compatString is set to true + * and the value is present as a string, this will be turned into + * a 1 element list. The returned @value will be NULL terminated + * if set. + * + * Reports an error if the config entry is set but has + * an unexpected type. + * + * Returns: 1 if the value was present, 0 if missing, -1 on error + */ +int virConfGetValueStringList(virConfPtr conf, + const char *setting, + bool compatString, + char ***values) +{ + virConfValuePtr cval = virConfGetValue(conf, setting); + size_t len; + virConfValuePtr eval; + + VIR_DEBUG("Get value string list %p %d", + cval, cval ? cval->type : VIR_CONF_NONE); + + if (!cval) + return 0; + + virStringFreeList(*values); + *values = NULL; + + switch (cval->type) { + case VIR_CONF_LIST: + /* Calc length and check items */ + for (len = 0, eval = cval->list; eval; len++, eval = eval->next) { + if (eval->type != VIR_CONF_STRING) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("%s: expected a string list for '%s' parameter"), + conf->filename, setting); + return -1; + } + } + + if (VIR_ALLOC_N(*values, len + 1) < 0) + return -1; + + for (len = 0, eval = cval->list; eval; len++, eval = eval->next) { + if (VIR_STRDUP((*values)[len], eval->str) < 0) { + virStringFreeList(*values); + *values = NULL; + return -1; + } + } + break; + + case VIR_CONF_STRING: + if (compatString) { + if (VIR_ALLOC_N(*values, cval->str ? 2 : 1) < 0) + return -1; + if (cval->str && + VIR_STRDUP((*values)[0], cval->str) < 0) { + VIR_FREE(values); + return -1; + } + break; + } + /* fallthrough */ + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + compatString ? + _("%s: expected a string or string list for '%s' parameter") : + _("%s: expected a string list for '%s' parameter"), + conf->filename, setting); + return -1; + } + + return 1; +} + + +/** + * virConfGetValueBool: + * @conf: the config object + * @setting: the config entry name + * @value: pointer to hold boolean value + * + * Get the boolean value of the config name @setting, storing + * it in @value. If the config entry is not present, then + * @value will be unmodified. + * + * Reports an error if the config entry is set but has + * an unexpected type, or if the value set is not 1 or 0. + * + * Returns: 1 if the value was present, 0 if missing, -1 on error + */ +int virConfGetValueBool(virConfPtr conf, + const char *setting, + bool *value) +{ + virConfValuePtr cval = virConfGetValue(conf, setting); + + VIR_DEBUG("Get value bool %p %d", + cval, cval ? cval->type : VIR_CONF_NONE); + + if (!cval) + return 0; + + if (cval->type != VIR_CONF_ULONG) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: expected a bool for '%s' parameter"), + conf->filename, setting); + return -1; + } + + if (cval->l < 0 || cval->l > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: value for '%s' parameter must be 0 or 1"), + conf->filename, setting); + return -1; + } + + *value = cval->l == 1; + + return 1; +} + + +/** + * virConfGetValueInt: + * @conf: the config object + * @setting: the config entry name + * @value: pointer to hold integer value + * + * Get the integer value of the config name @setting, storing + * it in @value. If the config entry is not present, then + * @value will be unmodified. + * + * Reports an error if the config entry is set but has + * an unexpected type, or if the value is outside the + * range that can be stored in an 'int' + * + * Returns: 1 if the value was present, 0 if missing, -1 on error + */ +int virConfGetValueInt(virConfPtr conf, + const char *setting, + int *value) +{ + virConfValuePtr cval = virConfGetValue(conf, setting); + + VIR_DEBUG("Get value int %p %d", + cval, cval ? cval->type : VIR_CONF_NONE); + + if (!cval) + return 0; + + if (cval->type != VIR_CONF_LONG && + cval->type != VIR_CONF_ULONG) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: expected a signed integer for '%s' parameter"), + conf->filename, setting); + return -1; + } + + if (cval->l > INT_MAX || cval->l < INT_MIN) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: value for '%s' parameter must be in range %d:%d"), + conf->filename, setting, INT_MIN, INT_MAX); + return -1; + } + + *value = cval->l; + + return 1; +} + + +/** + * virConfGetValueUInt: + * @conf: the config object + * @setting: the config entry name + * @value: pointer to hold integer value + * + * Get the unsigned integer value of the config name @setting, storing + * it in @value. If the config entry is not present, then + * @value will be unmodified. + * + * Reports an error if the config entry is set but has + * an unexpected type, or if the value is outside the + * range that can be stored in an 'unsigned int' + * + * Returns: 1 if the value was present, 0 if missing, -1 on error + */ +int virConfGetValueUInt(virConfPtr conf, + const char *setting, + unsigned int *value) +{ + virConfValuePtr cval = virConfGetValue(conf, setting); + + VIR_DEBUG("Get value uint %p %d", + cval, cval ? cval->type : VIR_CONF_NONE); + + if (!cval) + return 0; + + if (cval->type != VIR_CONF_ULONG) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: expected an unsigned integer for '%s' parameter"), + conf->filename, setting); + return -1; + } + + if (cval->l > UINT_MAX || cval->l < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: value for '%s' parameter must be in range 0:%u"), + conf->filename, setting, UINT_MAX); + return -1; + } + + *value = cval->l; + + return 1; +} + + +/** + * virConfGetValueSizeT: + * @conf: the config object + * @setting: the config entry name + * @value: pointer to hold integer value + * + * Get the integer value of the config name @setting, storing + * it in @value. If the config entry is not present, then + * @value will be unmodified. + * + * Reports an error if the config entry is set but has + * an unexpected type, or if the value is outside the + * range that can be stored in a 'size_t' + * + * Returns: 1 if the value was present, 0 if missing, -1 on error + */ +int virConfGetValueSizeT(virConfPtr conf, + const char *setting, + size_t *value) +{ + virConfValuePtr cval = virConfGetValue(conf, setting); + + VIR_DEBUG("Get value size_t %p %d", + cval, cval ? cval->type : VIR_CONF_NONE); + + if (!cval) + return 0; + + if (cval->type != VIR_CONF_ULONG) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: expected an unsigned integer for '%s' parameter"), + conf->filename, setting); + return -1; + } + + if (cval->l > SIZE_MAX || cval->l < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: value for '%s' parameter must be in range 0:%zu"), + conf->filename, setting, SIZE_MAX); + return -1; + } + + *value = cval->l; + + return 1; +} + + +/** + * virConfGetValueSSizeT: + * @conf: the config object + * @setting: the config entry name + * @value: pointer to hold integer value + * + * Get the integer value of the config name @setting, storing + * it in @value. If the config entry is not present, then + * @value will be unmodified. + * + * Reports an error if the config entry is set but has + * an unexpected type, or if the value is outside the + * range that can be stored in an 'ssize_t' + * + * Returns: 1 if the value was present, 0 if missing, -1 on error + */ +int virConfGetValueSSizeT(virConfPtr conf, + const char *setting, + ssize_t *value) +{ + virConfValuePtr cval = virConfGetValue(conf, setting); + + VIR_DEBUG("Get value ssize_t %p %d", + cval, cval ? cval->type : VIR_CONF_NONE); + + if (!cval) + return 0; + + if (cval->type != VIR_CONF_LONG && + cval->type != VIR_CONF_ULONG) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: expected a signed integer for '%s' parameter"), + conf->filename, setting); + return -1; + } + + if (cval->l > SSIZE_MAX || cval->l < (-SSIZE_MAX - 1)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: value for '%s' parameter must be in range %zd:%zd"), + conf->filename, setting, -SSIZE_MAX - 1, SSIZE_MAX); + return -1; + } + + *value = cval->l; + + return 1; +} + + +/** + * virConfGetValueLLong: + * @conf: the config object + * @setting: the config entry name + * @value: pointer to hold integer value + * + * Get the integer value of the config name @setting, storing + * it in @value. If the config entry is not present, then + * @value will be unmodified. + * + * Reports an error if the config entry is set but has + * an unexpected type, or if the value is outside the + * range that can be stored in an 'long long' + * + * Returns: 1 if the value was present, 0 if missing, -1 on error + */ +int virConfGetValueLLong(virConfPtr conf, + const char *setting, + long long *value) +{ + virConfValuePtr cval = virConfGetValue(conf, setting); + + VIR_DEBUG("Get value long long %p %d", + cval, cval ? cval->type : VIR_CONF_NONE); + + if (!cval) + return 0; + + if (cval->type != VIR_CONF_LONG && + cval->type != VIR_CONF_ULONG) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: expected a signed integer for '%s' parameter"), + conf->filename, setting); + return -1; + } + + if (cval->type == VIR_CONF_ULONG && + cval->l > LLONG_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: value for '%s' parameter must be in range 0:%lld"), + conf->filename, setting, LLONG_MAX); + return -1; + } + + *value = cval->l; + + return 1; +} + + +/** + * virConfGetValueULongLong: + * @conf: the config object + * @setting: the config entry name + * @value: pointer to hold integer value + * + * Get the integer value of the config name @setting, storing + * it in @value. If the config entry is not present, then + * @value will be unmodified. + * + * Reports an error if the config entry is set but has + * an unexpected type. + * + * Returns: 1 if the value was present, 0 if missing, -1 on error + */ +int virConfGetValueULLong(virConfPtr conf, + const char *setting, + unsigned long long *value) +{ + virConfValuePtr cval = virConfGetValue(conf, setting); + + VIR_DEBUG("Get value unsigned long long %p %d", + cval, cval ? cval->type : VIR_CONF_NONE); + + if (!cval) + return 0; + + if (cval->type != VIR_CONF_LONG && + cval->type != VIR_CONF_ULONG) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: expected an unsigned integer for '%s' parameter"), + conf->filename, setting); + return -1; + } + + *value = cval->l; + + return 1; +} + /** * virConfSetValue: * @conf: a configuration file handle diff --git a/src/util/virconf.h b/src/util/virconf.h index 239ab39..ccae9d9 100644 --- a/src/util/virconf.h +++ b/src/util/virconf.h @@ -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; /* very long integer */ char *str; /* pointer to 0 terminated string */ virConfValuePtr list; /* list of a list */ }; @@ -85,6 +85,38 @@ int virConfFree(virConfPtr conf); void virConfFreeValue(virConfValuePtr val); virConfValuePtr virConfGetValue(virConfPtr conf, const char *setting); + +virConfType virConfGetValueType(virConfPtr conf, + const char *setting); +int virConfGetValueString(virConfPtr conf, + const char *setting, + char **value); +int virConfGetValueStringList(virConfPtr conf, + const char *setting, + bool compatString, + char ***values); +int virConfGetValueBool(virConfPtr conf, + const char *setting, + bool *value); +int virConfGetValueInt(virConfPtr conf, + const char *setting, + int *value); +int virConfGetValueUInt(virConfPtr conf, + const char *setting, + unsigned int *value); +int virConfGetValueSizeT(virConfPtr conf, + const char *setting, + size_t *value); +int virConfGetValueSSizeT(virConfPtr conf, + const char *setting, + ssize_t *value); +int virConfGetValueLLong(virConfPtr conf, + const char *setting, + long long *value); +int virConfGetValueULLong(virConfPtr conf, + const char *setting, + unsigned long long *value); + int virConfSetValue(virConfPtr conf, const char *setting, virConfValuePtr value); diff --git a/tests/virconftest.c b/tests/virconftest.c index c71b491..d9ebda4 100644 --- a/tests/virconftest.c +++ b/tests/virconftest.c @@ -77,6 +77,329 @@ static int testConfRoundTrip(const void *opaque) } +static int testConfParseInt(const void *opaque ATTRIBUTE_UNUSED) +{ + const char *srcdata = \ + "int = -1729\n" \ + "uint = 1729\n" \ + "llong = -6963472309248\n" \ + "ullong = 6963472309248\n" \ + "size_t = 87539319\n" \ + "ssize_t = -87539319\n" \ + "string = \"foo\"\n"; + + int ret = -1; + virConfPtr conf = virConfReadMem(srcdata, strlen(srcdata), 0); + int iv; + unsigned int ui; + size_t s; + ssize_t ss; + long long l; + unsigned long long ul; + + if (!conf) + return -1; + + if (virConfGetValueType(conf, "int") != + VIR_CONF_LONG) { + fprintf(stderr, "expected a long for 'int'\n"); + goto cleanup; + } + + if (virConfGetValueInt(conf, "int", &iv) < 0) + goto cleanup; + + if (iv != -1729) { + fprintf(stderr, "Expected -1729 got %d\n", iv); + goto cleanup; + } + + if (virConfGetValueInt(conf, "string", &iv) != -1) { + fprintf(stderr, "Expected error for 'string' param\n"); + goto cleanup; + } + + + if (virConfGetValueType(conf, "uint") != + VIR_CONF_ULONG) { + fprintf(stderr, "expected a unsigned long for 'uint'\n"); + goto cleanup; + } + + if (virConfGetValueUInt(conf, "uint", &ui) < 0) + goto cleanup; + + if (ui != 1729) { + fprintf(stderr, "Expected 1729 got %u\n", ui); + goto cleanup; + } + + if (virConfGetValueUInt(conf, "string", &ui) != -1) { + fprintf(stderr, "Expected error for 'string' param\n"); + goto cleanup; + } + + + + if (virConfGetValueType(conf, "llong") != + VIR_CONF_LONG) { + fprintf(stderr, "expected a long for 'llong'\n"); + goto cleanup; + } + + if (virConfGetValueLLong(conf, "llong", &l) < 0) + goto cleanup; + + if (l != -6963472309248) { + fprintf(stderr, "Expected -6963472309248 got %lld\n", l); + goto cleanup; + } + + if (virConfGetValueLLong(conf, "string", &l) != -1) { + fprintf(stderr, "Expected error for 'string' param\n"); + goto cleanup; + } + + + + if (virConfGetValueType(conf, "ullong") != + VIR_CONF_ULONG) { + fprintf(stderr, "expected a unsigned long for 'ullong'\n"); + goto cleanup; + } + + if (virConfGetValueULLong(conf, "ullong", &ul) < 0) + goto cleanup; + + if (ul != 6963472309248) { + fprintf(stderr, "Expected 6963472309248 got %llu\n", ul); + goto cleanup; + } + + if (virConfGetValueULLong(conf, "string", &ul) != -1) { + fprintf(stderr, "Expected error for 'string' param\n"); + goto cleanup; + } + + + + if (virConfGetValueType(conf, "size_t") != + VIR_CONF_ULONG) { + fprintf(stderr, "expected a unsigned long for 'size_T'\n"); + goto cleanup; + } + + if (virConfGetValueSizeT(conf, "size_t", &s) < 0) + goto cleanup; + + if (s != 87539319) { + fprintf(stderr, "Expected 87539319 got %zu\n", s); + goto cleanup; + } + + if (virConfGetValueSizeT(conf, "string", &s) != -1) { + fprintf(stderr, "Expected error for 'string' param\n"); + goto cleanup; + } + + + + if (virConfGetValueType(conf, "ssize_t") != + VIR_CONF_LONG) { + fprintf(stderr, "expected a unsigned long for 'ssize_t'\n"); + goto cleanup; + } + + if (virConfGetValueSSizeT(conf, "ssize_t", &ss) < 0) + goto cleanup; + + if (ss != -87539319) { + fprintf(stderr, "Expected -87539319 got %zd\n", ss); + goto cleanup; + } + + if (virConfGetValueSSizeT(conf, "string", &ss) != -1) { + fprintf(stderr, "Expected error for 'string' param\n"); + goto cleanup; + } + + ret = 0; + cleanup: + virConfFree(conf); + return ret; +} + +static int testConfParseBool(const void *opaque ATTRIBUTE_UNUSED) +{ + const char *srcdata = \ + "false = 0\n" \ + "true = 1\n" \ + "int = 6963472309248\n" \ + "string = \"foo\"\n"; + + int ret = -1; + virConfPtr conf = virConfReadMem(srcdata, strlen(srcdata), 0); + bool f = true; + bool t = false; + + if (!conf) + return -1; + + if (virConfGetValueType(conf, "false") != + VIR_CONF_ULONG) { + fprintf(stderr, "expected a long for 'false'\n"); + goto cleanup; + } + + if (virConfGetValueBool(conf, "false", &f) < 0) + goto cleanup; + + if (f != false) { + fprintf(stderr, "Expected 0 got %d\n", f); + goto cleanup; + } + + + + if (virConfGetValueType(conf, "true") != + VIR_CONF_ULONG) { + fprintf(stderr, "expected a long for 'true'\n"); + goto cleanup; + } + + if (virConfGetValueBool(conf, "true", &t) < 0) + goto cleanup; + + if (t != true) { + fprintf(stderr, "Expected 1 got %d\n", t); + goto cleanup; + } + + + + if (virConfGetValueBool(conf, "int", &t) != -1) { + fprintf(stderr, "Expected error for 'string' param\n"); + goto cleanup; + } + + if (virConfGetValueBool(conf, "string", &t) != -1) { + fprintf(stderr, "Expected error for 'string' param\n"); + goto cleanup; + } + + + ret = 0; + cleanup: + virConfFree(conf); + return ret; +} + + +static int testConfParseString(const void *opaque ATTRIBUTE_UNUSED) +{ + const char *srcdata = \ + "int = 6963472309248\n" \ + "string = \"foo\"\n"; + + int ret = -1; + virConfPtr conf = virConfReadMem(srcdata, strlen(srcdata), 0); + char *str = NULL; + + if (!conf) + return -1; + + if (virConfGetValueType(conf, "string") != + VIR_CONF_STRING) { + fprintf(stderr, "expected a string for 'string'\n"); + goto cleanup; + } + + if (virConfGetValueString(conf, "string", &str) < 0) + goto cleanup; + + if (STRNEQ_NULLABLE(str, "foo")) { + fprintf(stderr, "Expected 'foo' got '%s'\n", str); + goto cleanup; + } + + if (virConfGetValueString(conf, "int", &str) != -1) { + fprintf(stderr, "Expected error for 'int'\n"); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(str); + virConfFree(conf); + return ret; +} + + +static int testConfParseStringList(const void *opaque ATTRIBUTE_UNUSED) +{ + const char *srcdata = \ + "string_list = [\"foo\", \"bar\"]\n" \ + "string = \"foo\"\n"; + + int ret = -1; + virConfPtr conf = virConfReadMem(srcdata, strlen(srcdata), 0); + char **str = NULL; + + if (!conf) + return -1; + + if (virConfGetValueType(conf, "string_list") != + VIR_CONF_LIST) { + fprintf(stderr, "expected a list for 'string_list'\n"); + goto cleanup; + } + + if (virConfGetValueStringList(conf, "string_list", false, &str) < 0) + goto cleanup; + + if (virStringListLength((const char *const*)str) != 2) { + fprintf(stderr, "expected a 2 element list\n"); + goto cleanup; + } + + if (STRNEQ_NULLABLE(str[0], "foo")) { + fprintf(stderr, "Expected 'foo' got '%s'\n", str[0]); + goto cleanup; + } + + if (STRNEQ_NULLABLE(str[1], "bar")) { + fprintf(stderr, "Expected 'bar' got '%s'\n", str[1]); + goto cleanup; + } + + + if (virConfGetValueStringList(conf, "string", false, &str) != -1) { + fprintf(stderr, "Expected error for 'string'\n"); + goto cleanup; + } + + if (virConfGetValueStringList(conf, "string", true, &str) < 0) + goto cleanup; + + if (virStringListLength((const char *const*)str) != 1) { + fprintf(stderr, "expected a 1 element list\n"); + goto cleanup; + } + + if (STRNEQ_NULLABLE(str[0], "foo")) { + fprintf(stderr, "Expected 'foo' got '%s'\n", str[0]); + goto cleanup; + } + + + ret = 0; + cleanup: + virStringFreeList(str); + virConfFree(conf); + return ret; +} + + static int mymain(void) { @@ -91,6 +414,18 @@ mymain(void) if (virTestRun("no-newline", testConfRoundTrip, "no-newline") < 0) ret = -1; + if (virTestRun("int", testConfParseInt, NULL) < 0) + ret = -1; + + if (virTestRun("bool", testConfParseBool, NULL) < 0) + ret = -1; + + if (virTestRun("string", testConfParseString, NULL) < 0) + ret = -1; + + if (virTestRun("string-list", testConfParseStringList, NULL) < 0) + ret = -1; + return ret; } -- 2.7.4

Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/qemu/qemu_conf.c | 395 ++++++++++++++++++++------------------------------- src/qemu/qemu_conf.h | 20 +-- 2 files changed, 165 insertions(+), 250 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 6dfa738..b7f9401 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -386,10 +386,13 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, const char *filename) { virConfPtr conf = NULL; - virConfValuePtr p; int ret = -1; - size_t i; + size_t i, j; char *stdioHandler = NULL; + char *user = NULL, *group = NULL; + char **controllers = NULL; + char **hugetlbfs = NULL; + char **nvram = NULL; /* Just check the file is readable before opening it, otherwise * libvirt emits an error. @@ -402,114 +405,66 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, if (!(conf = virConfReadFile(filename, 0))) goto cleanup; -#define CHECK_TYPE(name, typ) \ - if (p && p->type != (typ)) { \ - virReportError(VIR_ERR_INTERNAL_ERROR, \ - "%s: %s: expected type " #typ, \ - filename, (name)); \ - goto cleanup; \ - } + if (virConfGetValueBool(conf, "vnc_auto_unix_socket", &cfg->vncAutoUnixSocket) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "vnc_tls", &cfg->vncTLS) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "vnc_tls_x509_verify", &cfg->vncTLSx509verify) < 0) + goto cleanup; + if (virConfGetValueString(conf, "vnc_tls_x509_cert_dir", &cfg->vncTLSx509certdir) < 0) + goto cleanup; + if (virConfGetValueString(conf, "vnc_listen", &cfg->vncListen) < 0) + goto cleanup; + if (virConfGetValueString(conf, "vnc_password", &cfg->vncPassword) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "vnc_sasl", &cfg->vncSASL) < 0) + goto cleanup; + if (virConfGetValueString(conf, "vnc_sasl_dir", &cfg->vncSASLdir) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "vnc_allow_host_audio", &cfg->vncAllowHostAudio) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "nographics_allow_host_audio", &cfg->nogfxAllowHostAudio) < 0) + goto cleanup; -#define CHECK_TYPE_ALT(name, type1, type2) \ - if (p && (p->type != (type1) && p->type != (type2))) { \ - virReportError(VIR_ERR_INTERNAL_ERROR, \ - "%s: %s: expected type " #type1, \ - filename, (name)); \ - goto cleanup; \ - } -#define GET_VALUE_LONG(NAME, VAR) \ - p = virConfGetValue(conf, NAME); \ - CHECK_TYPE_ALT(NAME, VIR_CONF_LONG, VIR_CONF_ULONG); \ - if (p) \ - VAR = p->l; - -#define GET_VALUE_ULONG(NAME, VAR) \ - p = virConfGetValue(conf, NAME); \ - CHECK_TYPE(NAME, VIR_CONF_ULONG); \ - if (p) \ - VAR = p->l; - -#define GET_VALUE_BOOL(NAME, VAR) \ - p = virConfGetValue(conf, NAME); \ - CHECK_TYPE(NAME, VIR_CONF_ULONG); \ - if (p) \ - VAR = p->l != 0; - -#define GET_VALUE_STR(NAME, VAR) \ - p = virConfGetValue(conf, NAME); \ - CHECK_TYPE(NAME, VIR_CONF_STRING); \ - if (p && p->str) { \ - VIR_FREE(VAR); \ - if (VIR_STRDUP(VAR, p->str) < 0) \ - goto cleanup; \ - } + if (virConfGetValueStringList(conf, "security_driver", true, &cfg->securityDriverNames) < 0) + goto cleanup; - GET_VALUE_BOOL("vnc_auto_unix_socket", cfg->vncAutoUnixSocket); - GET_VALUE_BOOL("vnc_tls", cfg->vncTLS); - GET_VALUE_BOOL("vnc_tls_x509_verify", cfg->vncTLSx509verify); - GET_VALUE_STR("vnc_tls_x509_cert_dir", cfg->vncTLSx509certdir); - GET_VALUE_STR("vnc_listen", cfg->vncListen); - GET_VALUE_STR("vnc_password", cfg->vncPassword); - GET_VALUE_BOOL("vnc_sasl", cfg->vncSASL); - GET_VALUE_STR("vnc_sasl_dir", cfg->vncSASLdir); - GET_VALUE_BOOL("vnc_allow_host_audio", cfg->vncAllowHostAudio); - GET_VALUE_BOOL("nographics_allow_host_audio", cfg->nogfxAllowHostAudio); - - p = virConfGetValue(conf, "security_driver"); - if (p && p->type == VIR_CONF_LIST) { - size_t len, j; - virConfValuePtr pp; - - /* Calc length and check items */ - for (len = 0, pp = p->list; pp; len++, pp = pp->next) { - if (pp->type != VIR_CONF_STRING) { - virReportError(VIR_ERR_CONF_SYNTAX, "%s", - _("security_driver must be a list of strings")); + for (i = 0; cfg->securityDriverNames[i] != NULL; i++) { + for (j = i; cfg->securityDriverNames[j] != NULL; j++) { + if (STREQ(cfg->securityDriverNames[i], + cfg->securityDriverNames[j])) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("Duplicate security driver %s"), + cfg->securityDriverNames[i]); goto cleanup; } } - - if (VIR_ALLOC_N(cfg->securityDriverNames, len + 1) < 0) - goto cleanup; - - for (i = 0, pp = p->list; pp; i++, pp = pp->next) { - for (j = 0; j < i; j++) { - if (STREQ(pp->str, cfg->securityDriverNames[j])) { - virReportError(VIR_ERR_CONF_SYNTAX, - _("Duplicate security driver %s"), pp->str); - goto cleanup; - } - } - if (VIR_STRDUP(cfg->securityDriverNames[i], pp->str) < 0) - goto cleanup; - } - cfg->securityDriverNames[len] = NULL; - } else { - CHECK_TYPE("security_driver", VIR_CONF_STRING); - if (p && p->str) { - if (VIR_ALLOC_N(cfg->securityDriverNames, 2) < 0) - goto cleanup; - if (VIR_STRDUP(cfg->securityDriverNames[0], p->str) < 0) - goto cleanup; - - cfg->securityDriverNames[1] = NULL; - } } - GET_VALUE_BOOL("security_default_confined", cfg->securityDefaultConfined); - GET_VALUE_BOOL("security_require_confined", cfg->securityRequireConfined); + if (virConfGetValueBool(conf, "security_default_confined", &cfg->securityDefaultConfined) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "security_require_confined", &cfg->securityRequireConfined) < 0) + goto cleanup; - GET_VALUE_BOOL("spice_tls", cfg->spiceTLS); - GET_VALUE_STR("spice_tls_x509_cert_dir", cfg->spiceTLSx509certdir); - GET_VALUE_BOOL("spice_sasl", cfg->spiceSASL); - GET_VALUE_STR("spice_sasl_dir", cfg->spiceSASLdir); - GET_VALUE_STR("spice_listen", cfg->spiceListen); - GET_VALUE_STR("spice_password", cfg->spicePassword); - GET_VALUE_BOOL("spice_auto_unix_socket", cfg->spiceAutoUnixSocket); + if (virConfGetValueBool(conf, "spice_tls", &cfg->spiceTLS) < 0) + goto cleanup; + if (virConfGetValueString(conf, "spice_tls_x509_cert_dir", &cfg->spiceTLSx509certdir) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "spice_sasl", &cfg->spiceSASL) < 0) + goto cleanup; + if (virConfGetValueString(conf, "spice_sasl_dir", &cfg->spiceSASLdir) < 0) + goto cleanup; + if (virConfGetValueString(conf, "spice_listen", &cfg->spiceListen) < 0) + goto cleanup; + if (virConfGetValueString(conf, "spice_password", &cfg->spicePassword) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "spice_auto_unix_socket", &cfg->spiceAutoUnixSocket) < 0) + goto cleanup; - GET_VALUE_ULONG("remote_websocket_port_min", cfg->webSocketPortMin); + if (virConfGetValueUInt(conf, "remote_websocket_port_min", &cfg->webSocketPortMin) < 0) + goto cleanup; if (cfg->webSocketPortMin < QEMU_WEBSOCKET_PORT_MIN) { /* if the port is too low, we can't get the display name * to tell to vnc (usually subtract 5700, e.g. localhost:1 @@ -521,7 +476,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; } - GET_VALUE_ULONG("remote_websocket_port_max", cfg->webSocketPortMax); + if (virConfGetValueUInt(conf, "remote_websocket_port_max", &cfg->webSocketPortMax) < 0) + goto cleanup; if (cfg->webSocketPortMax > QEMU_WEBSOCKET_PORT_MAX || cfg->webSocketPortMax < cfg->webSocketPortMin) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -538,7 +494,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; } - GET_VALUE_ULONG("remote_display_port_min", cfg->remotePortMin); + if (virConfGetValueUInt(conf, "remote_display_port_min", &cfg->remotePortMin) < 0) + goto cleanup; if (cfg->remotePortMin < QEMU_REMOTE_PORT_MIN) { /* if the port is too low, we can't get the display name * to tell to vnc (usually subtract 5900, e.g. localhost:1 @@ -550,7 +507,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; } - GET_VALUE_ULONG("remote_display_port_max", cfg->remotePortMax); + if (virConfGetValueUInt(conf, "remote_display_port_max", &cfg->remotePortMax) < 0) + goto cleanup; if (cfg->remotePortMax > QEMU_REMOTE_PORT_MAX || cfg->remotePortMax < cfg->remotePortMin) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -567,7 +525,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; } - GET_VALUE_ULONG("migration_port_min", cfg->migrationPortMin); + if (virConfGetValueUInt(conf, "migration_port_min", &cfg->migrationPortMin) < 0) + goto cleanup; if (cfg->migrationPortMin <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s: migration_port_min: port must be greater than 0"), @@ -575,7 +534,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; } - GET_VALUE_ULONG("migration_port_max", cfg->migrationPortMax); + if (virConfGetValueUInt(conf, "migration_port_max", &cfg->migrationPortMax) < 0) + goto cleanup; if (cfg->migrationPortMax > 65535 || cfg->migrationPortMax < cfg->migrationPortMin) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -585,79 +545,56 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; } - p = virConfGetValue(conf, "user"); - CHECK_TYPE("user", VIR_CONF_STRING); - if (p && p->str && - virGetUserID(p->str, &cfg->user) < 0) + if (virConfGetValueString(conf, "user", &user) < 0) goto cleanup; - - p = virConfGetValue(conf, "group"); - CHECK_TYPE("group", VIR_CONF_STRING); - if (p && p->str && - virGetGroupID(p->str, &cfg->group) < 0) + if (user && virGetUserID(user, &cfg->user) < 0) goto cleanup; - GET_VALUE_BOOL("dynamic_ownership", cfg->dynamicOwnership); + if (virConfGetValueString(conf, "group", &group) < 0) + goto cleanup; + if (group && virGetGroupID(group, &cfg->group) < 0) + goto cleanup; - p = virConfGetValue(conf, "cgroup_controllers"); - CHECK_TYPE("cgroup_controllers", VIR_CONF_LIST); - if (p) { - cfg->cgroupControllers = 0; - virConfValuePtr pp; - for (i = 0, pp = p->list; pp; ++i, pp = pp->next) { - int ctl; - if (pp->type != VIR_CONF_STRING) { - virReportError(VIR_ERR_CONF_SYNTAX, "%s", - _("cgroup_controllers must be a " - "list of strings")); - goto cleanup; - } + if (virConfGetValueBool(conf, "dynamic_ownership", &cfg->dynamicOwnership) < 0) + goto cleanup; - if ((ctl = virCgroupControllerTypeFromString(pp->str)) < 0) { - virReportError(VIR_ERR_CONF_SYNTAX, - _("Unknown cgroup controller '%s'"), pp->str); - goto cleanup; - } - cfg->cgroupControllers |= (1 << ctl); - } - } + if (virConfGetValueStringList(conf, "cgroup_controllers", false, + &controllers) < 0) + goto cleanup; - p = virConfGetValue(conf, "cgroup_device_acl"); - CHECK_TYPE("cgroup_device_acl", VIR_CONF_LIST); - if (p) { - int len = 0; - virConfValuePtr pp; - for (pp = p->list; pp; pp = pp->next) - len++; - if (VIR_ALLOC_N(cfg->cgroupDeviceACL, 1+len) < 0) + for (i = 0; controllers[i] != NULL; i++) { + int ctl; + if ((ctl = virCgroupControllerTypeFromString(controllers[i])) < 0) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("Unknown cgroup controller '%s'"), + controllers[i]); goto cleanup; - - for (i = 0, pp = p->list; pp; ++i, pp = pp->next) { - if (pp->type != VIR_CONF_STRING) { - virReportError(VIR_ERR_CONF_SYNTAX, "%s", - _("cgroup_device_acl must be a " - "list of strings")); - goto cleanup; - } - if (VIR_STRDUP(cfg->cgroupDeviceACL[i], pp->str) < 0) - goto cleanup; } - cfg->cgroupDeviceACL[i] = NULL; + cfg->cgroupControllers |= (1 << ctl); } - GET_VALUE_STR("save_image_format", cfg->saveImageFormat); - GET_VALUE_STR("dump_image_format", cfg->dumpImageFormat); - GET_VALUE_STR("snapshot_image_format", cfg->snapshotImageFormat); + if (virConfGetValueStringList(conf, "cgroup_device_acl", false, + &cfg->cgroupDeviceACL) < 0) + goto cleanup; + + if (virConfGetValueString(conf, "save_image_format", &cfg->saveImageFormat) < 0) + goto cleanup; + if (virConfGetValueString(conf, "dump_image_format", &cfg->dumpImageFormat) < 0) + goto cleanup; + if (virConfGetValueString(conf, "snapshot_image_format", &cfg->snapshotImageFormat) < 0) + goto cleanup; - GET_VALUE_STR("auto_dump_path", cfg->autoDumpPath); - GET_VALUE_BOOL("auto_dump_bypass_cache", cfg->autoDumpBypassCache); - GET_VALUE_BOOL("auto_start_bypass_cache", cfg->autoStartBypassCache); + if (virConfGetValueString(conf, "auto_dump_path", &cfg->autoDumpPath) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "auto_dump_bypass_cache", &cfg->autoDumpBypassCache) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "auto_start_bypass_cache", &cfg->autoStartBypassCache) < 0) + goto cleanup; - /* Some crazy backcompat. Back in the old days, this was just a pure - * string. We must continue supporting it. These days however, this may be - * an array of strings. */ - p = virConfGetValue(conf, "hugetlbfs_mount"); - if (p) { + if (virConfGetValueStringList(conf, "hugetlbfs_mount", true, + &hugetlbfs) < 0) + goto cleanup; + if (hugetlbfs) { /* There already might be something autodetected. Avoid leaking it. */ while (cfg->nhugetlbfs) { cfg->nhugetlbfs--; @@ -665,60 +602,41 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, } VIR_FREE(cfg->hugetlbfs); - if (p->type == VIR_CONF_LIST) { - size_t len = 0; - virConfValuePtr pp = p->list; - - /* Calc length and check items */ - while (pp) { - if (pp->type != VIR_CONF_STRING) { - virReportError(VIR_ERR_CONF_SYNTAX, "%s", - _("hugetlbfs_mount must be a list of strings")); - goto cleanup; - } - len++; - pp = pp->next; - } + cfg->nhugetlbfs = virStringListLength((const char *const *)hugetlbfs); + if (hugetlbfs[0] && + VIR_ALLOC_N(cfg->hugetlbfs, cfg->nhugetlbfs) < 0) + goto cleanup; - if (len && VIR_ALLOC_N(cfg->hugetlbfs, len) < 0) + for (i = 0; hugetlbfs[i] != NULL; i++) { + if (virQEMUDriverConfigHugeTLBFSInit(&cfg->hugetlbfs[i], + hugetlbfs[i], i != 0) < 0) goto cleanup; - cfg->nhugetlbfs = len; - - pp = p->list; - len = 0; - while (pp) { - if (virQEMUDriverConfigHugeTLBFSInit(&cfg->hugetlbfs[len], - pp->str, !len) < 0) - goto cleanup; - len++; - pp = pp->next; - } - } else { - CHECK_TYPE("hugetlbfs_mount", VIR_CONF_STRING); - if (STRNEQ(p->str, "")) { - if (VIR_ALLOC_N(cfg->hugetlbfs, 1) < 0) - goto cleanup; - cfg->nhugetlbfs = 1; - if (virQEMUDriverConfigHugeTLBFSInit(&cfg->hugetlbfs[0], - p->str, true) < 0) - goto cleanup; - } } } - GET_VALUE_STR("bridge_helper", cfg->bridgeHelperName); + if (virConfGetValueString(conf, "bridge_helper", &cfg->bridgeHelperName) < 0) + goto cleanup; - GET_VALUE_BOOL("mac_filter", cfg->macFilter); + if (virConfGetValueBool(conf, "mac_filter", &cfg->macFilter) < 0) + goto cleanup; - GET_VALUE_BOOL("relaxed_acs_check", cfg->relaxedACS); - GET_VALUE_BOOL("clear_emulator_capabilities", cfg->clearEmulatorCapabilities); - GET_VALUE_BOOL("allow_disk_format_probing", cfg->allowDiskFormatProbing); - GET_VALUE_BOOL("set_process_name", cfg->setProcessName); - GET_VALUE_ULONG("max_processes", cfg->maxProcesses); - GET_VALUE_ULONG("max_files", cfg->maxFiles); + if (virConfGetValueBool(conf, "relaxed_acs_check", &cfg->relaxedACS) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "clear_emulator_capabilities", &cfg->clearEmulatorCapabilities) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "allow_disk_format_probing", &cfg->allowDiskFormatProbing) < 0) + goto cleanup; + if (virConfGetValueBool(conf, "set_process_name", &cfg->setProcessName) < 0) + goto cleanup; + if (virConfGetValueUInt(conf, "max_processes", &cfg->maxProcesses) < 0) + goto cleanup; + if (virConfGetValueUInt(conf, "max_files", &cfg->maxFiles) < 0) + goto cleanup; - GET_VALUE_STR("lock_manager", cfg->lockManagerName); - GET_VALUE_STR("stdio_handler", stdioHandler); + if (virConfGetValueString(conf, "lock_manager", &cfg->lockManagerName) < 0) + goto cleanup; + if (virConfGetValueString(conf, "stdio_handler", &stdioHandler) < 0) + goto cleanup; if (stdioHandler) { if (STREQ(stdioHandler, "logd")) { cfg->stdioLogD = true; @@ -734,14 +652,19 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, VIR_FREE(stdioHandler); } - GET_VALUE_ULONG("max_queued", cfg->maxQueuedJobs); + if (virConfGetValueUInt(conf, "max_queued", &cfg->maxQueuedJobs) < 0) + goto cleanup; - GET_VALUE_LONG("keepalive_interval", cfg->keepAliveInterval); - GET_VALUE_ULONG("keepalive_count", cfg->keepAliveCount); + if (virConfGetValueInt(conf, "keepalive_interval", &cfg->keepAliveInterval) < 0) + goto cleanup; + if (virConfGetValueUInt(conf, "keepalive_count", &cfg->keepAliveCount) < 0) + goto cleanup; - GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox); + if (virConfGetValueInt(conf, "seccomp_sandbox", &cfg->seccompSandbox) < 0) + goto cleanup; - GET_VALUE_STR("migration_host", cfg->migrateHost); + if (virConfGetValueString(conf, "migration_host", &cfg->migrateHost) < 0) + goto cleanup; virStringStripIPv6Brackets(cfg->migrateHost); if (cfg->migrateHost && (STRPREFIX(cfg->migrateHost, "localhost") || @@ -753,7 +676,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; } - GET_VALUE_STR("migration_address", cfg->migrationAddress); + if (virConfGetValueString(conf, "migration_address", &cfg->migrationAddress) < 0) + goto cleanup; virStringStripIPv6Brackets(cfg->migrationAddress); if (cfg->migrationAddress && (STRPREFIX(cfg->migrationAddress, "localhost") || @@ -765,32 +689,22 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; } - GET_VALUE_BOOL("log_timestamp", cfg->logTimestamp); - - if ((p = virConfGetValue(conf, "nvram"))) { - size_t len; - virConfValuePtr pp; - - CHECK_TYPE("nvram", VIR_CONF_LIST); + if (virConfGetValueBool(conf, "log_timestamp", &cfg->logTimestamp) < 0) + goto cleanup; + if (virConfGetValueStringList(conf, "nvram", false, &nvram) < 0) + goto cleanup; + if (nvram) { virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares); - /* Calc length and check items */ - for (len = 0, pp = p->list; pp; len++, pp = pp->next) { - if (pp->type != VIR_CONF_STRING) { - virReportError(VIR_ERR_CONF_SYNTAX, "%s", - _("nvram must be a list of strings")); - goto cleanup; - } - } - if (len && VIR_ALLOC_N(cfg->firmwares, len) < 0) + cfg->nfirmwares = virStringListLength((const char *const *)nvram); + if (nvram[0] && VIR_ALLOC_N(cfg->firmwares, cfg->nfirmwares) < 0) goto cleanup; - cfg->nfirmwares = len; - for (i = 0, pp = p->list; pp; i++, pp = pp->next) { + for (i = 0; nvram[i] != NULL; i++) { if (VIR_ALLOC(cfg->firmwares[i]) < 0) goto cleanup; - if (virFirmwareParse(pp->str, cfg->firmwares[i]) < 0) + if (virFirmwareParse(nvram[i], cfg->firmwares[i]) < 0) goto cleanup; } } @@ -798,13 +712,14 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, ret = 0; cleanup: + virStringFreeList(controllers); + virStringFreeList(hugetlbfs); + virStringFreeList(nvram); + VIR_FREE(user); + VIR_FREE(group); virConfFree(conf); return ret; } -#undef GET_VALUE_LONG -#undef GET_VALUE_ULONG -#undef GET_VALUE_BOOL -#undef GET_VALUE_STR virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver) { diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a09c81d..510cd9a 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -88,7 +88,7 @@ struct _virQEMUDriverConfig { uid_t user; gid_t group; - int dynamicOwnership; + bool dynamicOwnership; int cgroupControllers; char **cgroupDeviceACL; @@ -126,11 +126,11 @@ struct _virQEMUDriverConfig { char *spicePassword; bool spiceAutoUnixSocket; - int remotePortMin; - int remotePortMax; + unsigned int remotePortMin; + unsigned int remotePortMax; - int webSocketPortMin; - int webSocketPortMax; + unsigned int webSocketPortMin; + unsigned int webSocketPortMax; virHugeTLBFSPtr hugetlbfs; size_t nhugetlbfs; @@ -146,10 +146,10 @@ struct _virQEMUDriverConfig { bool allowDiskFormatProbing; bool setProcessName; - int maxProcesses; - int maxFiles; + unsigned int maxProcesses; + unsigned int maxFiles; - int maxQueuedJobs; + unsigned int maxQueuedJobs; char **securityDriverNames; bool securityDefaultConfined; @@ -173,8 +173,8 @@ struct _virQEMUDriverConfig { char *migrateHost; /* The default for -incoming */ char *migrationAddress; - int migrationPortMin; - int migrationPortMax; + unsigned int migrationPortMin; + unsigned int migrationPortMax; bool logTimestamp; bool stdioLogD; -- 2.7.4

On 11.07.2016 11:45, Daniel P. Berrange wrote: This function is now returning an int, therefore s/NULL/-1/. Michal

Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- po/POTFILES.in | 1 - src/logging/log_daemon_config.c | 96 ++++++----------------------------------- src/logging/log_daemon_config.h | 7 +-- 3 files changed, 15 insertions(+), 89 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 6689fd8..a6b6c9c 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -88,7 +88,6 @@ src/locking/lock_driver_sanlock.c src/locking/lock_manager.c src/locking/sanlock_helper.c src/logging/log_daemon.c -src/logging/log_daemon_config.c src/logging/log_daemon_dispatch.c src/logging/log_handler.c src/logging/log_manager.c diff --git a/src/logging/log_daemon_config.c b/src/logging/log_daemon_config.c index 7bf10c6..10b42e9 100644 --- a/src/logging/log_daemon_config.c +++ b/src/logging/log_daemon_config.c @@ -38,61 +38,6 @@ VIR_LOG_INIT("logging.log_daemon_config"); -/* A helper function used by each of the following macros. */ -static int -checkType(virConfValuePtr p, const char *filename, - const char *key, virConfType required_type) -{ - if (p->type != required_type) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("remoteReadConfigFile: %s: %s: invalid type:" - " got %s; expected %s"), filename, key, - virConfTypeToString(p->type), - virConfTypeToString(required_type)); - return -1; - } - return 0; -} - -/* If there is no config data for the key, #var_name, then do nothing. - If there is valid data of type VIR_CONF_STRING, and VIR_STRDUP succeeds, - store the result in var_name. Otherwise, (i.e. invalid type, or VIR_STRDUP - failure), give a diagnostic and "goto" the cleanup-and-fail label. */ -#define GET_CONF_STR(conf, filename, var_name) \ - do { \ - virConfValuePtr p = virConfGetValue(conf, #var_name); \ - if (p) { \ - if (checkType(p, filename, #var_name, VIR_CONF_STRING) < 0) \ - goto error; \ - VIR_FREE(data->var_name); \ - if (VIR_STRDUP(data->var_name, p->str) < 0) \ - goto error; \ - } \ - } while (0) - -/* Like GET_CONF_STR, but for signed integer values. */ -#define GET_CONF_INT(conf, filename, var_name) \ - do { \ - virConfValuePtr p = virConfGetValue(conf, #var_name); \ - if (p) { \ - if (p->type != VIR_CONF_ULONG && \ - checkType(p, filename, #var_name, VIR_CONF_LONG) < 0) \ - goto error; \ - data->var_name = p->l; \ - } \ - } while (0) - -/* Like GET_CONF_STR, but for unsigned integer values. */ -#define GET_CONF_UINT(conf, filename, var_name) \ - do { \ - virConfValuePtr p = virConfGetValue(conf, #var_name); \ - if (p) { \ - if (checkType(p, filename, #var_name, VIR_CONF_ULONG) < 0) \ - goto error; \ - data->var_name = p->l; \ - } \ - } while (0) - int virLogDaemonConfigFilePath(bool privileged, char **configfile) { @@ -148,21 +93,22 @@ virLogDaemonConfigFree(virLogDaemonConfigPtr data) static int virLogDaemonConfigLoadOptions(virLogDaemonConfigPtr data, - const char *filename, virConfPtr conf) { - GET_CONF_UINT(conf, filename, log_level); - GET_CONF_STR(conf, filename, log_filters); - GET_CONF_STR(conf, filename, log_outputs); - GET_CONF_UINT(conf, filename, max_clients); - - GET_CONF_UINT(conf, filename, max_size); - GET_CONF_UINT(conf, filename, max_backups); + if (virConfGetValueUInt(conf, "log_level", &data->log_level) < 0) + return -1; + if (virConfGetValueString(conf, "log_filters", &data->log_filters) < 0) + return -1; + if (virConfGetValueString(conf, "log_outputs", &data->log_filters) < 0) + return -1; + if (virConfGetValueUInt(conf, "max_clients", &data->max_clients) < 0) + return -1; + if (virConfGetValueSizeT(conf, "max_size", &data->max_size) < 0) + return -1; + if (virConfGetValueSizeT(conf, "max_backups", &data->max_backups) < 0) + return -1; return 0; - - error: - return -1; } @@ -185,23 +131,7 @@ virLogDaemonConfigLoadFile(virLogDaemonConfigPtr data, if (!conf) return -1; - ret = virLogDaemonConfigLoadOptions(data, filename, conf); - virConfFree(conf); - return ret; -} - -int virLogDaemonConfigLoadData(virLogDaemonConfigPtr data, - const char *filename, - const char *filedata) -{ - virConfPtr conf; - int ret; - - conf = virConfReadMem(filedata, strlen(filedata), 0); - if (!conf) - return -1; - - ret = virLogDaemonConfigLoadOptions(data, filename, conf); + ret = virLogDaemonConfigLoadOptions(data, conf); virConfFree(conf); return ret; } diff --git a/src/logging/log_daemon_config.h b/src/logging/log_daemon_config.h index 0da7b0b..72d77d5 100644 --- a/src/logging/log_daemon_config.h +++ b/src/logging/log_daemon_config.h @@ -30,10 +30,10 @@ typedef struct _virLogDaemonConfig virLogDaemonConfig; typedef virLogDaemonConfig *virLogDaemonConfigPtr; struct _virLogDaemonConfig { - int log_level; + unsigned int log_level; char *log_filters; char *log_outputs; - int max_clients; + unsigned int max_clients; size_t max_backups; size_t max_size; @@ -46,8 +46,5 @@ void virLogDaemonConfigFree(virLogDaemonConfigPtr data); int virLogDaemonConfigLoadFile(virLogDaemonConfigPtr data, const char *filename, bool allow_missing); -int virLogDaemonConfigLoadData(virLogDaemonConfigPtr data, - const char *filename, - const char *filedata); #endif /* __LIBVIRTD_CONFIG_H__ */ -- 2.7.4

Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/libxl/libxl_conf.c | 53 ++++++++------------------------------------------ 1 file changed, 8 insertions(+), 45 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 584eb8f..146e08a 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1271,22 +1271,11 @@ static int libxlGetAutoballoonConf(libxlDriverConfigPtr cfg, virConfPtr conf) { - virConfValuePtr p; regex_t regex; int res; - p = virConfGetValue(conf, "autoballoon"); - if (p) { - if (p->type != VIR_CONF_ULONG) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", - _("Unexpected type for 'autoballoon' setting")); - - return -1; - } - cfg->autoballoon = p->l != 0; - return 0; - } + if (virConfGetValueBool(conf, "autoballoon", &cfg->autoballoon) < 0) + return -1; if ((res = regcomp(&regex, "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )", @@ -1450,7 +1439,6 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, const char *filename) { virConfPtr conf = NULL; - virConfValuePtr p; int ret = -1; /* defaults for keepalive messages */ @@ -1472,39 +1460,14 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, if (libxlGetAutoballoonConf(cfg, conf) < 0) goto cleanup; - if ((p = virConfGetValue(conf, "lock_manager"))) { - if (p->type != VIR_CONF_STRING) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", - _("Unexpected type for 'lock_manager' setting")); - goto cleanup; - } - - if (VIR_STRDUP(cfg->lockManagerName, p->str) < 0) - goto cleanup; - } - - if ((p = virConfGetValue(conf, "keepalive_interval"))) { - if (p->type != VIR_CONF_LONG && p->type != VIR_CONF_ULONG) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", - _("Unexpected type for 'keepalive_interval' setting")); - goto cleanup; - } - - cfg->keepAliveInterval = p->l; - } + if (virConfGetValueString(conf, "lock_manager", &cfg->lockManagerName) < 0) + goto cleanup; - if ((p = virConfGetValue(conf, "keepalive_count"))) { - if (p->type != VIR_CONF_ULONG) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", - _("Unexpected type for 'keepalive_count' setting")); - goto cleanup; - } + if (virConfGetValueInt(conf, "keepalive_interval", &cfg->keepAliveInterval) < 0) + goto cleanup; - cfg->keepAliveCount = p->l; - } + if (virConfGetValueUInt(conf, "keepalive_count", &cfg->keepAliveCount) < 0) + goto cleanup; ret = 0; -- 2.7.4

Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- tools/virt-login-shell.c | 141 +++++++++++++---------------------------------- 1 file changed, 38 insertions(+), 103 deletions(-) diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c index e8474a4..632d493 100644 --- a/tools/virt-login-shell.c +++ b/tools/virt-login-shell.c @@ -50,46 +50,38 @@ static int virLoginShellAllowedUser(virConfPtr conf, gid_t *groups, size_t ngroups) { - virConfValuePtr p; int ret = -1; - char *ptr = NULL; size_t i; char *gname = NULL; + char **users = NULL, **entries; - p = virConfGetValue(conf, "allowed_users"); - if (p && p->type == VIR_CONF_LIST) { - virConfValuePtr pp; + if (virConfGetValueStringList(conf, "allowed_users", false, &users) < 0) + goto cleanup; - /* Calc length and check items */ - for (pp = p->list; pp; pp = pp->next) { - if (pp->type != VIR_CONF_STRING) { - virReportSystemError(EINVAL, "%s", - _("allowed_users must be a list of strings")); - goto cleanup; - } else { - /* - If string begins with a % this indicates a linux group. - Check to see if the user is in the Linux Group. - */ - if (pp->str[0] == '%') { - ptr = &pp->str[1]; - if (!*ptr) - continue; - for (i = 0; i < ngroups; i++) { - if (!(gname = virGetGroupName(groups[i]))) - continue; - if (fnmatch(ptr, gname, 0) == 0) { - ret = 0; - goto cleanup; - } - VIR_FREE(gname); - } + + for (entries = users; *entries; entries++) { + char *entry = *entries; + /* + If string begins with a % this indicates a linux group. + Check to see if the user is in the Linux Group. + */ + if (entry[0] == '%') { + entry++; + if (!*entry) + continue; + for (i = 0; i < ngroups; i++) { + if (!(gname = virGetGroupName(groups[i]))) continue; - } - if (fnmatch(pp->str, name, 0) == 0) { + if (fnmatch(entry, gname, 0) == 0) { ret = 0; goto cleanup; } + VIR_FREE(gname); + } + } else { + if (fnmatch(entry, name, 0) == 0) { + ret = 0; + goto cleanup; } } } @@ -98,87 +90,30 @@ static int virLoginShellAllowedUser(virConfPtr conf, name, conf_file); cleanup: VIR_FREE(gname); + virStringFreeList(users); return ret; } -static int virLoginShellGetAutoShell(virConfPtr conf, - bool *autoshell) -{ - virConfValuePtr p; - - p = virConfGetValue(conf, "auto_shell"); - if (!p) { - *autoshell = false; - } else if (p->type == VIR_CONF_LONG || - p->type == VIR_CONF_ULONG) { - *autoshell = (p->l != 0); - } else { - virReportSystemError(EINVAL, "%s", - _("auto_shell must be a boolean value")); - return -1; - } - return 0; -} static int virLoginShellGetShellArgv(virConfPtr conf, - char ***retshargv, - size_t *retshargvlen) + char ***shargv, + size_t *shargvlen) { - size_t i; - size_t len; - char **shargv = NULL; - virConfValuePtr p, pp; - - p = virConfGetValue(conf, "shell"); - if (!p) { - len = 1; /* /bin/sh */ - } else if (p->type == VIR_CONF_LIST) { - /* Calc length and check items */ - for (len = 0, pp = p->list; pp; len++, pp = pp->next) { - if (pp->type != VIR_CONF_STRING) { - virReportSystemError(EINVAL, "%s", - _("shell must be a list of strings")); - goto error; - } - } - } else if (p->type == VIR_CONF_STRING) { - len = 1; /* /path/to/shell */ - } else { - virReportSystemError(EINVAL, "%s", - _("shell must be a list of strings")); - goto error; - } - - len++; /* NULL terminator */ - - if (VIR_ALLOC_N(shargv, len) < 0) - goto error; + if (virConfGetValueStringList(conf, "shell", true, shargv) < 0) + return -1; - i = 0; - if (!p) { - if (VIR_STRDUP(shargv[i++], "/bin/sh") < 0) - goto error; - } else if (p->type == VIR_CONF_LIST) { - for (pp = p->list; pp; pp = pp->next) { - if (VIR_STRDUP(shargv[i++], pp->str) < 0) - goto error; + if (!shargv) { + if (VIR_ALLOC_N(*shargv, 2) < 0) + return -1; + if (VIR_STRDUP((*shargv)[0], "/bin/sh") < 0) { + VIR_FREE(*shargv); + return -1; } - } else if (p->type == VIR_CONF_STRING) { - if (VIR_STRDUP(shargv[i++], p->str) < 0) - goto error; + *shargvlen = 1; + } else { + *shargvlen = virStringListLength((const char *const *)shargv); } - - shargv[i] = NULL; - - *retshargvlen = i; - *retshargv = shargv; - return 0; - error: - *retshargv = NULL; - *retshargvlen = 0; - virStringFreeList(shargv); - return -1; } static char *progname; @@ -313,7 +248,7 @@ main(int argc, char **argv) if (virLoginShellGetShellArgv(conf, &shargv, &shargvlen) < 0) goto cleanup; - if (virLoginShellGetAutoShell(conf, &autoshell) < 0) + if (virConfGetValueBool(conf, "auto_shell", &autoshell) < 0) goto cleanup; conn = virConnectOpen("lxc:///"); -- 2.7.4

Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/locking/lock_driver_lockd.c | 61 ++++++++++------------------------------- 1 file changed, 15 insertions(+), 46 deletions(-) diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 1812611..41e7bcb 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -82,7 +82,7 @@ static virLockManagerLockDaemonDriverPtr driver; static int virLockManagerLockDaemonLoadConfig(const char *configFile) { virConfPtr conf; - virConfValuePtr p; + int ret = -1; if (access(configFile, R_OK) == -1) { if (errno != ENOENT) { @@ -97,57 +97,26 @@ static int virLockManagerLockDaemonLoadConfig(const char *configFile) if (!(conf = virConfReadFile(configFile, 0))) return -1; -#define CHECK_TYPE(name, typ) if (p && p->type != (typ)) { \ - virReportError(VIR_ERR_INTERNAL_ERROR, \ - "%s: %s: expected type " #typ, \ - configFile, (name)); \ - virConfFree(conf); \ - return -1; \ - } - - p = virConfGetValue(conf, "auto_disk_leases"); - CHECK_TYPE("auto_disk_leases", VIR_CONF_ULONG); - if (p) driver->autoDiskLease = p->l; + if (virConfGetValueBool(conf, "auto_disk_leases", &driver->autoDiskLease) < 0) + goto cleanup; - p = virConfGetValue(conf, "file_lockspace_dir"); - CHECK_TYPE("file_lockspace_dir", VIR_CONF_STRING); - if (p && p->str) { - VIR_FREE(driver->fileLockSpaceDir); - if (VIR_STRDUP(driver->fileLockSpaceDir, p->str) < 0) { - virConfFree(conf); - return -1; - } - } + if (virConfGetValueString(conf, "file_lockspace_dir", &driver->fileLockSpaceDir) < 0) + goto cleanup; - p = virConfGetValue(conf, "lvm_lockspace_dir"); - CHECK_TYPE("lvm_lockspace_dir", VIR_CONF_STRING); - if (p && p->str) { - VIR_FREE(driver->lvmLockSpaceDir); - if (VIR_STRDUP(driver->lvmLockSpaceDir, p->str) < 0) { - virConfFree(conf); - return -1; - } - } + if (virConfGetValueString(conf, "lvm_lockspace_dir", &driver->lvmLockSpaceDir) < 0) + goto cleanup; - p = virConfGetValue(conf, "scsi_lockspace_dir"); - CHECK_TYPE("scsi_lockspace_dir", VIR_CONF_STRING); - if (p && p->str) { - VIR_FREE(driver->scsiLockSpaceDir); - if (VIR_STRDUP(driver->scsiLockSpaceDir, p->str) < 0) { - virConfFree(conf); - return -1; - } - } + if (virConfGetValueString(conf, "scsi_lockspace_dir", &driver->scsiLockSpaceDir) < 0) + goto cleanup; - p = virConfGetValue(conf, "require_lease_for_disks"); - CHECK_TYPE("require_lease_for_disks", VIR_CONF_ULONG); - if (p) - driver->requireLeaseForDisks = p->l; - else - driver->requireLeaseForDisks = !driver->autoDiskLease; + driver->requireLeaseForDisks = !driver->autoDiskLease; + if (virConfGetValueBool(conf, "require_lease_for_disks", &driver->requireLeaseForDisks) < 0) + goto cleanup; + ret = 0; + cleanup: virConfFree(conf); - return 0; + return ret; } -- 2.7.4

Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/remote/remote_driver.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e9904f8..29552aa 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -864,18 +864,9 @@ doRemoteOpen(virConnectPtr conn, /* Connect to the remote service. */ switch (transport) { case trans_tls: - if (conf && !tls_priority) { - virConfValuePtr val = virConfGetValue(conf, "tls_priority"); - if (val) { - if (val->type != VIR_CONF_STRING) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Config file 'tls_priority' must be a string")); - goto failed; - } - if (VIR_STRDUP(tls_priority, val->str) < 0) - goto failed; - } - } + if (conf && !tls_priority && + virConfGetValueString(conf, "tls_priority", &tls_priority) < 0) + goto failed; #ifdef WITH_GNUTLS priv->tls = virNetTLSContextNewClientPath(pkipath, -- 2.7.4