[libvirt] [PATCH v1 3/3] virconf: Introduce VIR_CONF_ULONG
Michal Privoznik
mprivozn at redhat.com
Tue Dec 9 15:52:16 UTC 2014
https://bugzilla.redhat.com/show_bug.cgi?id=1160995
In our config files users are expected to pass several integer values
for different configuration knobs. However, majority of them expect a
nonnegative number and only a few of them accept a negative number too
(notably keepalive_interval in libvirtd.conf).
Therefore, a new type to config value is introduced: VIR_CONF_ULONG
that is set whenever an integer is positive or zero. With this
approach knobs accepting VIR_CONF_LONG should accept VIR_CONF_ULONG
too.
Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
daemon/libvirtd-config.c | 51 ++++++++++++++++++++++++---------------
src/locking/lock_daemon_config.c | 20 ++++++++++++---
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 | 43 ++++++++++++++++++++++-----------
src/util/virconf.c | 4 ++-
src/util/virconf.h | 1 +
src/xenconfig/xen_common.c | 6 ++---
tests/libvirtdconftest.c | 9 ++++---
10 files changed, 97 insertions(+), 53 deletions(-)
diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
index 929dd1a..3694455 100644
--- a/daemon/libvirtd-config.c
+++ b/daemon/libvirtd-config.c
@@ -146,17 +146,30 @@ checkType(virConfValuePtr p, const char *filename,
} \
} while (0)
-/* Like GET_CONF_STR, but for integral values. */
+/* Like GET_CONF_STR, but for signed integral values. */
#define GET_CONF_INT(conf, filename, var_name) \
do { \
virConfValuePtr p = virConfGetValue(conf, #var_name); \
if (p) { \
- if (checkType(p, filename, #var_name, VIR_CONF_LONG) < 0) \
+ 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 integral 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)
+
+
static int
remoteConfigGetAuth(virConfPtr conf,
@@ -361,8 +374,8 @@ daemonConfigLoadOptions(struct daemonConfig *data,
const char *filename,
virConfPtr conf)
{
- GET_CONF_INT(conf, filename, listen_tcp);
- GET_CONF_INT(conf, filename, listen_tls);
+ GET_CONF_UINT(conf, filename, listen_tcp);
+ GET_CONF_UINT(conf, filename, listen_tls);
GET_CONF_STR(conf, filename, tls_port);
GET_CONF_STR(conf, filename, tcp_port);
GET_CONF_STR(conf, filename, listen_addr);
@@ -396,11 +409,11 @@ daemonConfigLoadOptions(struct daemonConfig *data,
GET_CONF_STR(conf, filename, unix_sock_dir);
- GET_CONF_INT(conf, filename, mdns_adv);
+ GET_CONF_UINT(conf, filename, mdns_adv);
GET_CONF_STR(conf, filename, mdns_name);
- GET_CONF_INT(conf, filename, tls_no_sanity_certificate);
- GET_CONF_INT(conf, filename, tls_no_verify_certificate);
+ GET_CONF_UINT(conf, filename, tls_no_sanity_certificate);
+ GET_CONF_UINT(conf, filename, tls_no_verify_certificate);
GET_CONF_STR(conf, filename, key_file);
GET_CONF_STR(conf, filename, cert_file);
@@ -417,29 +430,29 @@ daemonConfigLoadOptions(struct daemonConfig *data,
goto error;
- GET_CONF_INT(conf, filename, min_workers);
- GET_CONF_INT(conf, filename, max_workers);
- GET_CONF_INT(conf, filename, max_clients);
- GET_CONF_INT(conf, filename, max_queued_clients);
- GET_CONF_INT(conf, filename, max_anonymous_clients);
+ GET_CONF_UINT(conf, filename, min_workers);
+ GET_CONF_UINT(conf, filename, max_workers);
+ GET_CONF_UINT(conf, filename, max_clients);
+ GET_CONF_UINT(conf, filename, max_queued_clients);
+ GET_CONF_UINT(conf, filename, max_anonymous_clients);
- GET_CONF_INT(conf, filename, prio_workers);
+ GET_CONF_UINT(conf, filename, prio_workers);
GET_CONF_INT(conf, filename, max_requests);
- GET_CONF_INT(conf, filename, max_client_requests);
+ GET_CONF_UINT(conf, filename, max_client_requests);
- GET_CONF_INT(conf, filename, audit_level);
- GET_CONF_INT(conf, filename, audit_logging);
+ GET_CONF_UINT(conf, filename, audit_level);
+ GET_CONF_UINT(conf, filename, audit_logging);
GET_CONF_STR(conf, filename, host_uuid);
- GET_CONF_INT(conf, filename, log_level);
+ GET_CONF_UINT(conf, filename, log_level);
GET_CONF_STR(conf, filename, log_filters);
GET_CONF_STR(conf, filename, log_outputs);
GET_CONF_INT(conf, filename, keepalive_interval);
- GET_CONF_INT(conf, filename, keepalive_count);
- GET_CONF_INT(conf, filename, keepalive_required);
+ GET_CONF_UINT(conf, filename, keepalive_count);
+ GET_CONF_UINT(conf, filename, keepalive_required);
return 0;
diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c
index abe6aba..8a6d18f 100644
--- a/src/locking/lock_daemon_config.c
+++ b/src/locking/lock_daemon_config.c
@@ -70,12 +70,24 @@ checkType(virConfValuePtr p, const char *filename,
} \
} while (0)
-/* Like GET_CONF_STR, but for integral values. */
+/* 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 (checkType(p, filename, #var_name, VIR_CONF_LONG) < 0) \
+ 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; \
} \
@@ -137,10 +149,10 @@ virLockDaemonConfigLoadOptions(virLockDaemonConfigPtr data,
const char *filename,
virConfPtr conf)
{
- GET_CONF_INT(conf, filename, log_level);
+ GET_CONF_UINT(conf, filename, log_level);
GET_CONF_STR(conf, filename, log_filters);
GET_CONF_STR(conf, filename, log_outputs);
- GET_CONF_INT(conf, filename, max_clients);
+ GET_CONF_UINT(conf, filename, max_clients);
return 0;
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index a642122..2af3f22 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_LONG);
+ CHECK_TYPE("auto_disk_leases", VIR_CONF_ULONG);
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_LONG);
+ CHECK_TYPE("require_lease_for_disks", VIR_CONF_ULONG);
if (p)
driver->requireLeaseForDisks = p->l;
else
diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index d84a419..b24e910 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_LONG);
+ CHECK_TYPE("auto_disk_leases", VIR_CONF_ULONG);
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_LONG);
+ CHECK_TYPE("host_id", VIR_CONF_UONG);
if (p) driver->hostID = p->l;
p = virConfGetValue(conf, "require_lease_for_disks");
- CHECK_TYPE("require_lease_for_disks", VIR_CONF_LONG);
+ CHECK_TYPE("require_lease_for_disks", VIR_CONF_ULONG);
if (p)
driver->requireLeaseForDisks = p->l;
else
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
index e713ff8..b6d1797 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_LONG);
+ CHECK_TYPE("log_with_libvirtd", VIR_CONF_ULONG);
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_LONG);
+ CHECK_TYPE("security_default_confined", VIR_CONF_ULONG);
if (p) cfg->securityDefaultConfined = p->l;
p = virConfGetValue(conf, "security_require_confined");
- CHECK_TYPE("security_require_confined", VIR_CONF_LONG);
+ CHECK_TYPE("security_require_confined", VIR_CONF_ULONG);
if (p) cfg->securityRequireConfined = p->l;
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 4764bef..8d3818e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -412,15 +412,29 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup; \
}
-#define GET_VALUE_LONG(NAME, VAR) \
+#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_LONG); \
+ 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_LONG); \
+ CHECK_TYPE(NAME, VIR_CONF_ULONG); \
if (p) \
VAR = p->l != 0;
@@ -489,7 +503,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
GET_VALUE_STR("spice_password", cfg->spicePassword);
- GET_VALUE_LONG("remote_websocket_port_min", cfg->webSocketPortMin);
+ GET_VALUE_ULONG("remote_websocket_port_min", cfg->webSocketPortMin);
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
@@ -501,7 +515,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
- GET_VALUE_LONG("remote_websocket_port_max", cfg->webSocketPortMax);
+ GET_VALUE_ULONG("remote_websocket_port_max", cfg->webSocketPortMax);
if (cfg->webSocketPortMax > QEMU_WEBSOCKET_PORT_MAX ||
cfg->webSocketPortMax < cfg->webSocketPortMin) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -518,7 +532,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
- GET_VALUE_LONG("remote_display_port_min", cfg->remotePortMin);
+ GET_VALUE_ULONG("remote_display_port_min", cfg->remotePortMin);
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
@@ -530,7 +544,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
- GET_VALUE_LONG("remote_display_port_max", cfg->remotePortMax);
+ GET_VALUE_ULONG("remote_display_port_max", cfg->remotePortMax);
if (cfg->remotePortMax > QEMU_REMOTE_PORT_MAX ||
cfg->remotePortMax < cfg->remotePortMin) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -547,7 +561,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
- GET_VALUE_LONG("migration_port_min", cfg->migrationPortMin);
+ GET_VALUE_ULONG("migration_port_min", cfg->migrationPortMin);
if (cfg->migrationPortMin <= 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("%s: migration_port_min: port must be greater than 0"),
@@ -555,7 +569,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
- GET_VALUE_LONG("migration_port_max", cfg->migrationPortMax);
+ GET_VALUE_ULONG("migration_port_max", cfg->migrationPortMax);
if (cfg->migrationPortMax > 65535 ||
cfg->migrationPortMax < cfg->migrationPortMin) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -694,15 +708,15 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
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_LONG("max_processes", cfg->maxProcesses);
- GET_VALUE_LONG("max_files", cfg->maxFiles);
+ GET_VALUE_ULONG("max_processes", cfg->maxProcesses);
+ GET_VALUE_ULONG("max_files", cfg->maxFiles);
GET_VALUE_STR("lock_manager", cfg->lockManagerName);
- GET_VALUE_LONG("max_queued", cfg->maxQueuedJobs);
+ GET_VALUE_ULONG("max_queued", cfg->maxQueuedJobs);
GET_VALUE_LONG("keepalive_interval", cfg->keepAliveInterval);
- GET_VALUE_LONG("keepalive_count", cfg->keepAliveCount);
+ GET_VALUE_ULONG("keepalive_count", cfg->keepAliveCount);
GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox);
@@ -775,8 +789,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
virConfFree(conf);
return ret;
}
-#undef GET_VALUE_BOOL
#undef GET_VALUE_LONG
+#undef GET_VALUE_ULONG
+#undef GET_VALUE_BOOL
#undef GET_VALUE_STR
virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 5b4b4c3..b1509fe 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -83,6 +83,7 @@ struct _virConfParserCtxt {
VIR_ENUM_IMPL(virConf, VIR_CONF_LAST,
"*unexpected*",
"long",
+ "unsigned long",
"string",
"list");
@@ -273,6 +274,7 @@ virConfSaveValue(virBufferPtr buf, virConfValuePtr val)
case VIR_CONF_NONE:
return -1;
case VIR_CONF_LONG:
+ case VIR_CONF_ULONG:
virBufferAsprintf(buf, "%ld", val->l);
break;
case VIR_CONF_STRING:
@@ -534,9 +536,9 @@ 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)
return NULL;
- type = VIR_CONF_LONG;
} else {
virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("expecting a value"));
return NULL;
diff --git a/src/util/virconf.h b/src/util/virconf.h
index 6176d43..2b4b943 100644
--- a/src/util/virconf.h
+++ b/src/util/virconf.h
@@ -34,6 +34,7 @@
typedef enum {
VIR_CONF_NONE = 0, /* undefined */
VIR_CONF_LONG, /* a long int */
+ VIR_CONF_ULONG, /* an unsigned long int */
VIR_CONF_STRING, /* a string */
VIR_CONF_LIST, /* a list */
VIR_CONF_LAST, /* sentinel */
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index 7f4ec89..33ac127 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_LONG) {
+ if (val->type == VIR_CONF_ULONG) {
*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_LONG) {
+ if (val->type == VIR_CONF_ULONG) {
*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_LONG) {
+ if (val->type == VIR_CONF_ULONG) {
*value = val->l;
} else if (val->type == VIR_CONF_STRING) {
if (virStrToLong_ull(val->str, NULL, 10, value) < 0) {
diff --git a/tests/libvirtdconftest.c b/tests/libvirtdconftest.c
index 8b93f4e..d589d51 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_LONG;
+ *type = VIR_CONF_ULONG;
replace = "\"foo\"";
} else if (*tmp == '[') {
*type = VIR_CONF_LIST;
@@ -130,15 +130,16 @@ testCorrupt(const void *opaque)
#endif
switch (type) {
- case VIR_CONF_LONG:
- if (!strstr(err->message, "invalid type: got string; expected long")) {
+ case VIR_CONF_ULONG:
+ 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'",
err->message);
ret = -1;
}
break;
case VIR_CONF_STRING:
- if (!strstr(err->message, "invalid type: got long; expected string")) {
+ if (!strstr(err->message, "invalid type: got unsigned long; expected string")) {
VIR_DEBUG("Wrong error for string: '%s'",
err->message);
ret = -1;
--
2.0.4
More information about the libvir-list
mailing list