Re: [libvirt] [PATCH] openvz: fixed an config file parsing error

The OpenVZ driver reported an error on parsing some OpenVZ config parameters (e.g. diskspace). This issue is due to the driver made two incorrect assumptions about the value of the parameters: 1. Assume paramaeter is just a number (without unit suffix). 2. Assume the number is an integer. Actually, an OpenVZ config parameter may consist of a scalar and a unit, and the scalar is not necessary an integer (e.g. 2.2G). This patch is for fixing this issue. --- src/openvz/openvz_conf.c | 109 +++++++++++++++++++++++++++++++++++++++++----- src/util/virutil.c | 84 ++++++++++++++++++++++++----------- src/util/virutil.h | 3 + 3 files changed, 157 insertions(+), 39 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 856c9f5..0b163b4 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -51,6 +51,7 @@ #include "virfile.h" #include "vircommand.h" #include "virstring.h" +#include "c-ctype.h" #define VIR_FROM_THIS VIR_FROM_OPENVZ @@ -127,6 +128,32 @@ int openvzExtractVersion(struct openvz_driver *driver) } +/* Split a string, which is a config parameter, into a scalar and a unit. + * Recognized unit: + * - Gigabyte: 'G' or 'g' + * - Megabyte: 'M' or 'm' + * - Kilobyte: 'K' or 'k' + * - Page: 'P' or 'g' */

+static int +openvzParseParamUnit(char* str, char *unit) +{ + char last; + int len = strlen(str); + + if (len == 0) + return -1; + + last = c_tolower(str[len - 1]); + if (last == 'g' || last == 'm' || last == 'k' || last == 'p') {

+ str[len - 1] = '\0'; + if (unit) + *unit = last; + } + + return 0; +} +

+ /* Parse config values of the form barrier:limit into barrier and limit */ static int openvzParseBarrierLimit(const char* value, @@ -146,7 +173,59 @@ openvzParseBarrierLimit(const char* value, goto error; } else { if (barrier != NULL) { - if (virStrToLong_ull(token, NULL, 10, barrier)) + if (openvzParseParamUnit(token, NULL) < 0) + goto error; + + if (virStrToLong_ull(token, NULL, 10, barrier) < 0) + goto error; + } + } + token = strtok_r(NULL, ":", &saveptr); + if (token == NULL) { + goto error; + } else { + if (limit != NULL) { + if (openvzParseParamUnit(token, NULL) < 0) + goto error; + + if (virStrToLong_ull(token, NULL, 10, limit) < 0) + goto error; + } + } + ret = 0; + error: + VIR_FREE(str); + return ret; +} + + +/* Just like openvzParseBarrierLimit, above, but produce "double" + * barrier and limit. This version also outputs the units of barrier + * and limit, which are bunit and lunit, if they are present.*/ +static int +openvzParseBarrierLimitDouble(const char* value, + double *barrier, + double *limit, + char *bunit, + char *lunit) +{ + char *token; + char *saveptr = NULL; + char *str; + int ret = -1; + + if (VIR_STRDUP(str, value) < 0) + goto error; + + token = strtok_r(str, ":", &saveptr); + if (token == NULL) { + goto error; + } else { + if (barrier != NULL) { + if (openvzParseParamUnit(token, bunit) < 0) + goto error; + + if (virStrToDouble(token, NULL, barrier) < 0) goto error; } } @@ -155,7 +234,10 @@ openvzParseBarrierLimit(const char* value, goto error; } else { if (limit != NULL) { - if (virStrToLong_ull(token, NULL, 10, limit)) + if (openvzParseParamUnit(token, lunit) < 0) + goto error; + + if (virStrToDouble(token, NULL, limit) < 0) goto error; } } @@ -352,7 +434,8 @@ openvzReadFSConf(virDomainDefPtr def, char *veid_str = NULL; char *temp = NULL; const char *param; - unsigned long long barrier, limit; + double barrier, limit; + char bunit, lunit; ret = openvzReadVPSConfigParam(veid, "OSTEMPLATE", &temp); if (ret < 0) { @@ -396,21 +479,23 @@ openvzReadFSConf(virDomainDefPtr def, param = "DISKSPACE"; ret = openvzReadVPSConfigParam(veid, param, &temp); if (ret > 0) { - if (openvzParseBarrierLimit(temp, &barrier, &limit)) { + bunit = 'k'; /* default unit is kilobyte */ + lunit = 'k'; /* default unit is kilobyte */ + + if (openvzParseBarrierLimitDouble(temp, &barrier, &limit, &bunit, &lunit)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not read '%s' from config for container %d"), param, veid); goto error; } else { - /* Ensure that we can multiply by 1024 without overflowing. */ - if (barrier > ULLONG_MAX / 1024 || - limit > ULLONG_MAX / 1024) { - virReportSystemError(VIR_ERR_OVERFLOW, "%s", - _("Unable to parse quota")); + if (virScaleDouble(&barrier, &bunit) < 0) goto error; - } - fs->space_soft_limit = barrier * 1024; /* unit is bytes */ - fs->space_hard_limit = limit * 1024; /* unit is bytes */ + + if (virScaleDouble(&limit, &lunit) < 0) + goto error; + + fs->space_soft_limit = (unsigned long long)barrier; + fs->space_hard_limit = (unsigned long long)limit; } } diff --git a/src/util/virutil.c b/src/util/virutil.c index 2edbec5..3b6e617 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -268,28 +268,12 @@ virHexToBin(unsigned char c) } } -/* Scale an integer VALUE in-place by an optional case-insensitive - * SUFFIX, defaulting to SCALE if suffix is NULL or empty (scale is - * typically 1 or 1024). Recognized suffixes include 'b' or 'bytes', - * as well as power-of-two scaling via binary abbreviations ('KiB', - * 'MiB', ...) or their one-letter counterpart ('k', 'M', ...), and - * power-of-ten scaling via SI abbreviations ('KB', 'MB', ...). - * Ensure that the result does not exceed LIMIT. Return 0 on success, - * -1 with error message raised on failure. */ int -virScaleInteger(unsigned long long *value, const char *suffix, - unsigned long long scale, unsigned long long limit) +virSuffixToScale(const char *suffix, unsigned long long *scale) { - if (!suffix || !*suffix) { - if (!scale) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid scale %llu"), scale); - return -1; - } - suffix = ""; - } else if (STRCASEEQ(suffix, "b") || STRCASEEQ(suffix, "byte") || + if (STRCASEEQ(suffix, "b") || STRCASEEQ(suffix, "byte") || STRCASEEQ(suffix, "bytes")) { - scale = 1; + *scale = 1; } else { int base; @@ -299,28 +283,28 @@ virScaleInteger(unsigned long long *value, const char *suffix, base = 1000; } else { virReportError(VIR_ERR_INVALID_ARG, - _("unknown suffix '%s'"), suffix); + _("unknown suffix '%s'"), suffix); return -1; } - scale = 1; + *scale = 1; switch (c_tolower(*suffix)) { case 'e': - scale *= base; + *scale *= base; /* fallthrough */ case 'p': - scale *= base; + *scale *= base; /* fallthrough */ case 't': - scale *= base; + *scale *= base; /* fallthrough */ case 'g': - scale *= base; + *scale *= base; /* fallthrough */ case 'm': - scale *= base; + *scale *= base; /* fallthrough */ case 'k': - scale *= base; + *scale *= base; break; default: virReportError(VIR_ERR_INVALID_ARG, @@ -329,6 +313,34 @@ virScaleInteger(unsigned long long *value, const char *suffix, } } + return 0; +} + +/* Scale an integer VALUE in-place by an optional case-insensitive + * SUFFIX, defaulting to SCALE if suffix is NULL or empty (scale is + * typically 1 or 1024). Recognized suffixes include 'b' or 'bytes', + * as well as power-of-two scaling via binary abbreviations ('KiB', + * 'MiB', ...) or their one-letter counterpart ('k', 'M', ...), and + * power-of-ten scaling via SI abbreviations ('KB', 'MB', ...). + * Ensure that the result does not exceed LIMIT. Return 0 on success, + * -1 with error message raised on failure. */ +int +virScaleInteger(unsigned long long *value, const char *suffix, + unsigned long long scale, unsigned long long limit) +{ + if (!suffix || !*suffix) { + if (!scale) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid scale %llu"), scale); + return -1; + } + suffix = ""; + } else if (virSuffixToScale(suffix, &scale) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("unable to parse suffix '%s'"), suffix); + return -1;

+ } + if (*value && *value > (limit / scale)) { virReportError(VIR_ERR_OVERFLOW, _("value too large: %llu%s"), *value, suffix); @@ -338,6 +350,24 @@ virScaleInteger(unsigned long long *value, const char *suffix, return 0; } +/* Just like virScaleInteger, above, but produce an "double" value. */ +int +virScaleDouble(double *value, const char *suffix) +{ + unsigned long long scale; + + if (!suffix || !*suffix) + return -1; + + if (virSuffixToScale(suffix, &scale) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("unable to parse suffix '%s'"), suffix); + return -1; + } + *value *= scale; + return 0; +} + /** * virParseNumber: diff --git a/src/util/virutil.h b/src/util/virutil.h index 89b7923..d74b581 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -57,6 +57,9 @@ int virScaleInteger(unsigned long long *value, const char *suffix, unsigned long long scale, unsigned long long limit) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; +int virScaleDouble(double *value, const char *suffix) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; +

int virHexToBin(unsigned char c); int virParseNumber(const char **str);