[libvirt] [PATCH 1/2] node_memory: Do not fail if there is parameter unsupported

Osier Yang jyang at redhat.com
Mon Nov 26 15:26:08 UTC 2012


ping

On 2012年11月22日 11:14, Osier Yang wrote:
> It makes no sense to fail the whole command if there is parameter
> unsupported by the kernel. This patch fixes it by ignoring the
> unsupported parameter for getMemoryParameters, and ignoring the
> unsupported parameter for setMemoryParameters too if there are
> more than one parameters to set, otherwise error out.
> ---
>   src/libvirt.c  |   12 +++--
>   src/nodeinfo.c |  119 ++++++++++++++++++++++++++++++++++----------------------
>   2 files changed, 79 insertions(+), 52 deletions(-)
>
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 4af6089..93ec068 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -6746,10 +6746,10 @@ error:
>    * @nparams: pointer to number of memory parameters; input and output
>    * @flags: extra flags; not used yet, so callers should always pass 0
>    *
> - * Get all node memory parameters.  On input, @nparams gives the size
> - * of the @params array; on output, @nparams gives how many slots were
> - * filled with parameter information, which might be less but will
> - * not exceed the input value.
> + * Get all node memory parameters (parameter unsupported by OS will be
> + * ignored).  On input, @nparams gives the size of the @params array;
> + * on output, @nparams gives how many slots were filled with parameter
> + * information, which might be less but will not exceed the input value.
>    *
>    * As a special case, calling with @params as NULL and @nparams as 0 on
>    * input will cause @nparams on output to contain the number of parameters
> @@ -6811,7 +6811,9 @@ error:
>    *           value nparams of virDomainGetSchedulerType)
>    * @flags: extra flags; not used yet, so callers should always pass 0
>    *
> - * Change all or a subset of the node memory tunables.
> + * Change all or a subset of the node memory tunables. Tunable
> + * unsupported by OS wll be ignored if @nparams is not 1, otherwise
> + * the function fails.
>    *
>    * Note that it's not recommended to use this function while the
>    * outside tuning program is running (such as ksmtuned under Linux),
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index 75d6524..718a3a4 100644
> --- a/src/nodeinfo.c
> +++ b/src/nodeinfo.c
> @@ -1058,21 +1058,13 @@ nodeGetCPUBitmap(int *max_id ATTRIBUTE_UNUSED)
>
>   #ifdef __linux__
>   static int
> -nodeSetMemoryParameterValue(const char *field,
> +nodeSetMemoryParameterValue(const char *path,
>                               virTypedParameterPtr param)
>   {
> -    char *path = NULL;
>       char *strval = NULL;
>       int ret = -1;
>       int rc = -1;
>
> -    if (virAsprintf(&path, "%s/%s",
> -                    SYSFS_MEMORY_SHARED_PATH, field)<  0) {
> -        virReportOOMError();
> -        ret = -2;
> -        goto cleanup;
> -    }
> -
>       if (virAsprintf(&strval, "%u", param->value.ui) == -1) {
>           virReportOOMError();
>           ret = -2;
> @@ -1080,13 +1072,12 @@ nodeSetMemoryParameterValue(const char *field,
>       }
>
>       if ((rc = virFileWriteStr(path, strval, 0))<  0) {
> -        virReportSystemError(-rc, _("failed to set %s"), field);
> +        virReportSystemError(-rc, _("failed to set %s"), param->field);
>           goto cleanup;
>       }
>
>       ret = 0;
>   cleanup:
> -    VIR_FREE(path);
>       VIR_FREE(strval);
>       return ret;
>   }
> @@ -1101,7 +1092,9 @@ nodeSetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
>       virCheckFlags(0, -1);
>
>   #ifdef __linux__
> -    int ret = 0;
> +    char *path = NULL;
> +    int ret = -1;
> +    int rc;
>       int i;
>
>       if (virTypedParameterArrayValidate(params, nparams,
> @@ -1117,30 +1110,40 @@ nodeSetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
>       for (i = 0; i<  nparams; i++) {
>           virTypedParameterPtr param =&params[i];
>
> -        if (STREQ(param->field,
> -                  VIR_NODE_MEMORY_SHARED_PAGES_TO_SCAN)) {
> -            ret = nodeSetMemoryParameterValue("pages_to_scan", param);
> +        char *field = strchr(param->field, '_');
> +        field++;
> +        if (virAsprintf(&path, "%s/%s",
> +                        SYSFS_MEMORY_SHARED_PATH, field)<  0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
>
> -            /* Out of memory */
> -            if (ret == -2)
> -                return -1;
> -        } else if (STREQ(param->field,
> -                         VIR_NODE_MEMORY_SHARED_SLEEP_MILLISECS)) {
> -            ret = nodeSetMemoryParameterValue("sleep_millisecs", param);
> +        /* Ignore the unsupported the param field if there is other
> +         * parameter to set, otherwise error out.
> +         */
> +        if (!virFileExists(path)) {
> +            if (nparams == 1) {
> +                virReportError(VIR_ERR_OPERATION_INVALID,
> +                               _("Parameter '%s' is not supported by "
> +                                 "this kernel"), param->field);
> +                goto cleanup;
> +            } else {
> +                VIR_WARN("Parameter '%s' is not supported by this kernel",
> +                         param->field);
> +                continue;
> +            }
> +        }
>
> -            /* Out of memory */
> -            if (ret == -2)
> -                return -1;
> -        } else if (STREQ(param->field,
> -                         VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES)) {
> -            ret = nodeSetMemoryParameterValue("merge_across_nodes", param);
> +        rc = nodeSetMemoryParameterValue(path, param);
>
> -            /* Out of memory */
> -            if (ret == -2)
> -                return -1;
> -        }
> +        /* Out of memory */
> +        if (rc == -2)
> +            goto cleanup;
>       }
>
> +    ret = 0;
> +cleanup:
> +    VIR_FREE(path);
>       return ret;
>   #else
>       virReportError(VIR_ERR_NO_SUPPORT, "%s",
> @@ -1167,6 +1170,11 @@ nodeGetMemoryParameterValue(const char *field,
>           goto cleanup;
>       }
>
> +    if (!virFileExists(path)) {
> +        ret = -2;
> +        goto cleanup;
> +    }
> +
>       if (virFileReadAll(path, 1024,&buf)<  0)
>           goto cleanup;
>
> @@ -1217,6 +1225,7 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
>       unsigned long long pages_volatile;
>       unsigned long long full_scans = 0;
>       int i;
> +    int ret;
>
>       if ((*nparams) == 0) {
>           *nparams = NODE_MEMORY_PARAMETERS_NUM;
> @@ -1228,8 +1237,10 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
>
>           switch (i) {
>           case 0:
> -            if (nodeGetMemoryParameterValue("pages_to_scan",
> -&pages_to_scan)<  0)
> +            ret = nodeGetMemoryParameterValue("pages_to_scan",&pages_to_scan);
> +            if (ret == -2)
> +                continue;
> +            else if (ret == -1)
>                   return -1;
>
>               if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_PAGES_TO_SCAN,
> @@ -1239,8 +1250,10 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
>               break;
>
>           case 1:
> -            if (nodeGetMemoryParameterValue("sleep_millisecs",
> -&sleep_millisecs)<  0)
> +            ret = nodeGetMemoryParameterValue("sleep_millisecs",&sleep_millisecs);
> +            if (ret == -2)
> +                continue;
> +            else if (ret == -1)
>                   return -1;
>
>               if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_SLEEP_MILLISECS,
> @@ -1250,8 +1263,10 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
>               break;
>
>           case 2:
> -            if (nodeGetMemoryParameterValue("pages_shared",
> -&pages_shared)<  0)
> +            ret = nodeGetMemoryParameterValue("pages_shared",&pages_shared);
> +            if (ret == -2)
> +                continue;
> +            else if (ret == -1)
>                   return -1;
>
>               if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_PAGES_SHARED,
> @@ -1261,8 +1276,10 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
>               break;
>
>           case 3:
> -            if (nodeGetMemoryParameterValue("pages_sharing",
> -&pages_sharing)<  0)
> +            ret = nodeGetMemoryParameterValue("pages_sharing",&pages_sharing);
> +            if (ret == -2)
> +                continue;
> +            else if (ret == -1)
>                   return -1;
>
>               if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_PAGES_SHARING,
> @@ -1272,8 +1289,10 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
>               break;
>
>           case 4:
> -            if (nodeGetMemoryParameterValue("pages_unshared",
> -&pages_unshared)<  0)
> +            ret = nodeGetMemoryParameterValue("pages_unshared",&pages_unshared);
> +            if (ret == -2)
> +                continue;
> +            else if (ret == -1)
>                   return -1;
>
>               if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_PAGES_UNSHARED,
> @@ -1283,8 +1302,10 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
>               break;
>
>           case 5:
> -            if (nodeGetMemoryParameterValue("pages_volatile",
> -&pages_volatile)<  0)
> +            ret = nodeGetMemoryParameterValue("pages_volatile",&pages_volatile);
> +            if (ret == -2)
> +                continue;
> +            else if (ret == -1)
>                   return -1;
>
>               if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_PAGES_VOLATILE,
> @@ -1294,8 +1315,10 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
>               break;
>
>           case 6:
> -            if (nodeGetMemoryParameterValue("full_scans",
> -&full_scans)<  0)
> +            ret = nodeGetMemoryParameterValue("full_scans",&full_scans);
> +            if (ret == -2)
> +                continue;
> +            else if (ret == -1)
>                   return -1;
>
>               if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_FULL_SCANS,
> @@ -1305,8 +1328,10 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
>               break;
>
>           case 7:
> -            if (nodeGetMemoryParameterValue("merge_across_nodes",
> -&merge_across_nodes)<  0)
> +            ret = nodeGetMemoryParameterValue("merge_across_nodes",&merge_across_nodes);
> +            if (ret == -2)
> +                continue;
> +            else if (ret == -1)
>                   return -1;
>
>               if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES,




More information about the libvir-list mailing list