12:52 a.m.
*** BLURB HERE ***
Michal Privoznik (3):
virConfSetValue: Simplify condition
virConfType: switch to VIR_ENUM_{DECL,IMPL}
virconf: Introduce VIR_CONF_ULONG
daemon/libvirtd-config.c | 55 ++++++++++++++++++++++++---------------
src/libvirt_private.syms | 2 ++
src/locking/lock_daemon_config.c | 24 ++++++++++++-----
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 | 12 +++++++--
src/util/virconf.h | 29 +++++++--------------
src/xenconfig/xen_common.c | 6 ++---
tests/libvirtdconftest.c | 9 ++++---
11 files changed, 119 insertions(+), 77 deletions(-)
--
2.0.4
12:52 a.m.
New subject: [libvirt] [PATCH v1 1/3] virConfSetValue: Simplify condition
There's no need for condition of the following form:
if (str && STREQ(str, dst))
since we have STREQ_NULLABLE macro that handles NULL cases.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/virconf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 0a17eff..74e695a 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -884,7 +884,7 @@ virConfSetValue(virConfPtr conf,
cur = conf->entries;
while (cur != NULL) {
- if ((cur->name != NULL) && (STREQ(cur->name, setting)))
+ if (STREQ_NULLABLE(cur->name, setting))
break;
prev = cur;
cur = cur->next;
--
2.0.4
4:15 p.m.
New subject: [libvirt] [PATCH v1 1/3] virConfSetValue: Simplify condition
On Tue, Dec 09, 2014 at 04:52:14PM +0100, Michal Privoznik wrote:
There's no need for condition of the following form:
if (str && STREQ(str, dst))
since we have STREQ_NULLABLE macro that handles NULL cases.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/virconf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 0a17eff..74e695a 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -884,7 +884,7 @@ virConfSetValue(virConfPtr conf,
cur = conf->entries;
while (cur != NULL) {
- if ((cur->name != NULL) && (STREQ(cur->name, setting)))
+ if (STREQ_NULLABLE(cur->name, setting))
break;
prev = cur;
cur = cur->next;
--
2.0.4
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
12:52 a.m.
New subject: [libvirt] [PATCH v1 2/3] virConfType: switch to VIR_ENUM_{DECL, IMPL}
There's no need to implement ToString() function like we do if we
can use our shiny macros.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
daemon/libvirtd-config.c | 4 ++--
src/libvirt_private.syms | 2 ++
src/locking/lock_daemon_config.c | 4 ++--
src/util/virconf.c | 6 ++++++
src/util/virconf.h | 28 +++++++++-------------------
5 files changed, 21 insertions(+), 23 deletions(-)
diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
index c31ef16..929dd1a 100644
--- a/daemon/libvirtd-config.c
+++ b/daemon/libvirtd-config.c
@@ -123,8 +123,8 @@ checkType(virConfValuePtr p, const char *filename,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("remoteReadConfigFile: %s: %s: invalid type:"
" got %s; expected %s"), filename, key,
- virConfTypeName(p->type),
- virConfTypeName(required_type));
+ virConfTypeToString(p->type),
+ virConfTypeToString(required_type));
return -1;
}
return 0;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6df2784..57cf4d3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1211,6 +1211,8 @@ virConfNew;
virConfReadFile;
virConfReadMem;
virConfSetValue;
+virConfTypeFromString;
+virConfTypeToString;
virConfWalk;
virConfWriteFile;
virConfWriteMem;
diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c
index 62b8b02..abe6aba 100644
--- a/src/locking/lock_daemon_config.c
+++ b/src/locking/lock_daemon_config.c
@@ -47,8 +47,8 @@ checkType(virConfValuePtr p, const char *filename,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("remoteReadConfigFile: %s: %s: invalid type:"
" got %s; expected %s"), filename, key,
- virConfTypeName(p->type),
- virConfTypeName(required_type));
+ virConfTypeToString(p->type),
+ virConfTypeToString(required_type));
return -1;
}
return 0;
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 74e695a..5b4b4c3 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -80,6 +80,12 @@ struct _virConfParserCtxt {
* *
************************************************************************/
+VIR_ENUM_IMPL(virConf, VIR_CONF_LAST,
+ "*unexpected*",
+ "long",
+ "string",
+ "list");
+
typedef struct _virConfEntry virConfEntry;
typedef virConfEntry *virConfEntryPtr;
diff --git a/src/util/virconf.h b/src/util/virconf.h
index 2a6b050..6176d43 100644
--- a/src/util/virconf.h
+++ b/src/util/virconf.h
@@ -23,6 +23,8 @@
#ifndef __VIR_CONF_H__
# define __VIR_CONF_H__
+# include "virutil.h"
+
/**
* virConfType:
* one of the possible type for a value from the configuration file
@@ -30,12 +32,15 @@
* TODO: we probably need a float too.
*/
typedef enum {
- VIR_CONF_NONE = 0, /* undefined */
- VIR_CONF_LONG = 1, /* a long int */
- VIR_CONF_STRING = 2, /* a string */
- VIR_CONF_LIST = 3 /* a list */
+ VIR_CONF_NONE = 0, /* undefined */
+ VIR_CONF_LONG, /* a long int */
+ VIR_CONF_STRING, /* a string */
+ VIR_CONF_LIST, /* a list */
+ VIR_CONF_LAST, /* sentinel */
} virConfType;
+VIR_ENUM_DECL(virConf)
+
typedef enum {
VIR_CONF_FLAG_VMX_FORMAT = 1, /* allow ':', '.' and '-' in
names for compatibility
with VMware VMX configuration file, but restrict
@@ -45,21 +50,6 @@ typedef enum {
to string only and don't expect quotes for
values */
} virConfFlags;
-static inline const char *
-virConfTypeName (virConfType t)
-{
- switch (t) {
- case VIR_CONF_LONG:
- return "long";
- case VIR_CONF_STRING:
- return "string";
- case VIR_CONF_LIST:
- return "list";
- default:
- return "*unexpected*";
- }
-}
-
/**
* virConfValue:
* a value from the configuration file
--
2.0.4
4:16 p.m.
New subject: [libvirt] [PATCH v1 2/3] virConfType: switch to VIR_ENUM_{DECL, IMPL}
On Tue, Dec 09, 2014 at 04:52:15PM +0100, Michal Privoznik wrote:
There's no need to implement ToString() function like we do if we
can use our shiny macros.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
daemon/libvirtd-config.c | 4 ++--
src/libvirt_private.syms | 2 ++
src/locking/lock_daemon_config.c | 4 ++--
src/util/virconf.c | 6 ++++++
src/util/virconf.h | 28 +++++++++-------------------
5 files changed, 21 insertions(+), 23 deletions(-)
[...]
diff --git a/src/util/virconf.h b/src/util/virconf.h
index 2a6b050..6176d43 100644
--- a/src/util/virconf.h
+++ b/src/util/virconf.h
@@ -23,6 +23,8 @@
#ifndef __VIR_CONF_H__
# define __VIR_CONF_H__
+# include "virutil.h"
+
/**
* virConfType:
* one of the possible type for a value from the configuration file
@@ -30,12 +32,15 @@
* TODO: we probably need a float too.
*/
typedef enum {
- VIR_CONF_NONE = 0, /* undefined */
- VIR_CONF_LONG = 1, /* a long int */
- VIR_CONF_STRING = 2, /* a string */
- VIR_CONF_LIST = 3 /* a list */
+ VIR_CONF_NONE = 0, /* undefined */
+ VIR_CONF_LONG, /* a long int */
+ VIR_CONF_STRING, /* a string */
+ VIR_CONF_LIST, /* a list */
Could you add an empty line here before the sentinel?
ACK with that changed.
+ VIR_CONF_LAST, /* sentinel */
} virConfType;
12:52 a.m.
New subject: [libvirt] [PATCH v1 3/3] virconf: Introduce VIR_CONF_ULONG
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(a)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
6:40 p.m.
New subject: [libvirt] [PATCH v1 3/3] virconf: Introduce VIR_CONF_ULONG
On Tue, Dec 09, 2014 at 04:52:16PM +0100, Michal Privoznik wrote:
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(a)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/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
Special-casing the ULONG and adding a '-2' somewhere in the tests
would show you've done the right thing.
Anyway, ACK, but I'd wait after release since this does not fix
anything, strictly speaking.
Martin
3771
days inactive
3772
days old
6 comments
2 participants
participants (2)
-
Martin Kletzander -
Michal Privoznik