[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&gt; --- 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