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: NMichal Privoznik <mprivozn@redhat.com>

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;
 

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;
 

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

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

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;
 
 

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)

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;

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 */

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) {

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;