[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/6] node_memory: Implement the internal APIs



On Fri, Sep 14, 2012 at 02:42:17PM +0800, Osier Yang wrote:
> Only implemented for linux platform.
> 
> * src/nodeinfo.h: (Declare node{Get,Set}MemoryParameters)
> * src/nodeinfo.c: (Implement node{Get,Set}MemoryParameters)
> * src/libvirt_private.syms: (Export those two new internal APIs to
>   private symbols)
> ---
>  src/libvirt_private.syms |    2 +
>  src/nodeinfo.c           |  332 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/nodeinfo.h           |   10 ++
>  3 files changed, 344 insertions(+), 0 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 8dfb4ce..23ea9fe 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -879,7 +879,9 @@ nodeGetCPUStats;
>  nodeGetCellsFreeMemory;
>  nodeGetFreeMemory;
>  nodeGetInfo;
> +nodeGetMemoryParameters;
>  nodeGetMemoryStats;
> +nodeSetMemoryParameters;
>  
>  
>  # nwfilter_conf.h
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index e3d4a24..d50705e 100644
> --- a/src/nodeinfo.c
> +++ b/src/nodeinfo.c
> @@ -48,6 +48,7 @@
>  #include "count-one-bits.h"
>  #include "intprops.h"
>  #include "virfile.h"
> +#include "virtypedparam.h"
>  
>  
>  #define VIR_FROM_THIS VIR_FROM_NONE
> @@ -57,6 +58,7 @@
>  # define SYSFS_SYSTEM_PATH "/sys/devices/system"
>  # define PROCSTAT_PATH "/proc/stat"
>  # define MEMINFO_PATH "/proc/meminfo"
> +# define SYSFS_MEMORY_SHARED_PATH "/sys/kernel/mm/ksm"
>  
>  # define LINUX_NB_CPU_STATS 4
>  # define LINUX_NB_MEMORY_STATS_ALL 4
> @@ -933,6 +935,336 @@ nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED,
>  #endif
>  }
>  
> +int
> +nodeSetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                        virTypedParameterPtr params,
> +                        int nparams,
> +                        unsigned int flags)
> +{
> +    virCheckFlags(0, -1);
> +
> +#ifdef __linux__
> +    {
> +        int ret = 0;
> +        int rc;
> +        int i;
> +

  Hum, i'm not sure i like the extra indentation level just for the
sake of making it easier to define local variables, especially as
this function is already takink liberties with the 80 character wide
formatting rule.

> +        if (virTypedParameterArrayValidate(params, nparams,
> +                                           VIR_NODE_MEMORY_SHARED_PAGES_TO_SCAN,
> +                                           VIR_TYPED_PARAM_UINT,
> +                                           VIR_NODE_MEMORY_SHARED_SLEEP_MILLISECS,
> +                                           VIR_TYPED_PARAM_UINT,
> +                                           NULL) < 0)
> +            return -1;
> +
> +        for (i = 0; i < nparams; i++) {
> +            virTypedParameterPtr param = &params[i];
> +
> +            if (STREQ(param->field,
> +                      VIR_NODE_MEMORY_SHARED_PAGES_TO_SCAN)) {
> +                char *pages_to_scan = NULL;
> +                char *strval = NULL;
> +
> +                if (virAsprintf(&pages_to_scan, "%s/%s",
> +                                SYSFS_MEMORY_SHARED_PATH,
> +                                VIR_NODE_MEMORY_SHARED_PAGES_TO_SCAN) < 0) {

   Won't work here, you will have to use the strings directly since
prefixed by "shm_', that will make for more compact code !

  Maybe a couple of helper static functions instead of duplicating
the code for each case would be nicer, and lead to code fitting in
80 columns.

> +                    virReportOOMError();
> +                    return -1;
> +                }
> +
> +                if (virAsprintf(&strval, "%u", param->value.ui) == -1) {
> +                    virReportOOMError();
> +                    VIR_FREE(pages_to_scan);
> +                    return -1;
> +                }
> +
> +                if ((rc = virFileWriteStr(pages_to_scan, strval, 0)) < 0) {
> +                    virReportSystemError(-rc, "%s",
> +                                         _("failed to set pages_to_scan"));
> +                    ret = -1;
> +                }
> +                VIR_FREE(pages_to_scan);
> +                VIR_FREE(strval);
> +            } else if (STREQ(param->field,
> +                             VIR_NODE_MEMORY_SHARED_SLEEP_MILLISECS)) {
> +                char *sleep_millisecs = NULL;
> +                char *strval = NULL;
> +
> +                if (virAsprintf(&sleep_millisecs, "%s/%s",
> +                                SYSFS_MEMORY_SHARED_PATH,
> +                                VIR_NODE_MEMORY_SHARED_SLEEP_MILLISECS) < 0) {
> +                    virReportOOMError();
> +                    return -1;
> +                }
> +
> +                if (virAsprintf(&strval, "%u", param->value.ui) == -1) {
> +                    virReportOOMError();
> +                    VIR_FREE(sleep_millisecs);
> +                    return -1;
> +                }
> +
> +                if ((rc = virFileWriteStr(sleep_millisecs, strval, 0)) < 0) {
> +                    virReportSystemError(-rc, "%s",
> +                                         _("failed to set pages_to_scan"));
> +                    ret = -1;
> +                }
> +                VIR_FREE(sleep_millisecs);
> +                VIR_FREE(strval);
> +            }
> +        }
> +
> +        return ret;
> +    }
> +#else
> +    virReportError(VIR_ERR_NO_SUPPORT, "%s",
> +                   _("node set memory parameters not implemented"
> +                     " on this platform"));
> +    return -1;
> +#endif
> +}
> +
> +#define NODE_MEMORY_PARAMETERS_NUM 7
> +int
> +nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                        virTypedParameterPtr params,
> +                        int *nparams,
> +                        unsigned int flags)
> +{
> +    virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
> +
> +#ifdef __linux__
> +    {
> +        char *path = NULL;
> +        char *buf = NULL;
> +        unsigned int pages_to_scan;
> +        unsigned int sleep_millisecs;
> +        unsigned long long pages_shared;
> +        unsigned long long pages_sharing;
> +        unsigned long long pages_unshared;
> +        unsigned long long pages_volatile;
> +        unsigned long long full_scans = 0;
> +        int ret = -1;
> +        int i;
> +
> +        if ((*nparams) == 0) {
> +            *nparams = NODE_MEMORY_PARAMETERS_NUM;
> +            return 0;
> +        }
> +
> +        for (i = 0; i < *nparams && i < NODE_MEMORY_PARAMETERS_NUM; i++) {
> +            virTypedParameterPtr param = &params[i];
> +            char *tmp = NULL;
> +
> +            switch(i) {
> +            case 0:
> +                if (virAsprintf(&path, "%s/%s", SYSFS_MEMORY_SHARED_PATH,
> +                                VIR_NODE_MEMORY_SHARED_PAGES_TO_SCAN) < 0) {
> +                    virReportOOMError();
> +                    goto cleanup;
> +                }
> +
> +                if (virFileReadAll(path, 1024, &buf) < 0)
> +                    goto cleanup;
> +
> +                if ((tmp = strchr(buf, '\n')))
> +                    *tmp = '\0';
> +
> +                if (virStrToLong_ui(buf, NULL, 10, &pages_to_scan) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                   _("failed to parse pages_to_scan"));
> +                    goto cleanup;
> +                }
> +
> +                if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_PAGES_TO_SCAN,
> +                                            VIR_TYPED_PARAM_UINT, pages_to_scan) < 0)
> +                    goto cleanup;
> +
> +                VIR_FREE(path);
> +                VIR_FREE(buf);
> +                break;
> +
> +            case 1:
> +                if (virAsprintf(&path, "%s/%s", SYSFS_MEMORY_SHARED_PATH,
> +                                VIR_NODE_MEMORY_SHARED_SLEEP_MILLISECS) < 0) {
> +                    virReportOOMError();
> +                    goto cleanup;
> +                }
> +
> +                if (virFileReadAll(path, 1024, &buf) < 0)
> +                    goto cleanup;
> +
> +                if ((tmp = strchr(buf, '\n')))
> +                    *tmp = '\0';
> +
> +                if (virStrToLong_ui(buf, NULL, 10, &sleep_millisecs) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                   _("failed to parse sleep_millisecs"));
> +                    goto cleanup;
> +                }
> +
> +                if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_SLEEP_MILLISECS,
> +                                            VIR_TYPED_PARAM_UINT, sleep_millisecs) < 0)
> +                    goto cleanup;
> +
> +                VIR_FREE(path);
> +                VIR_FREE(buf);
> +                break;
> +
> +            case 2:
> +                if (virAsprintf(&path, "%s/%s", SYSFS_MEMORY_SHARED_PATH,
> +                                VIR_NODE_MEMORY_SHARED_PAGES_SHARED) < 0) {
> +                    virReportOOMError();
> +                    goto cleanup;
> +                }
> +
> +                if (virFileReadAll(path, 1024, &buf) < 0)
> +                    goto cleanup;
> +
> +                if ((tmp = strchr(buf, '\n')))
> +                    *tmp = '\0';
> +
> +                if (virStrToLong_ull(buf, NULL, 10, &pages_shared) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                   _("failed to parse pages_shared"));
> +                    goto cleanup;
> +                }
> +
> +                if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_PAGES_SHARED,
> +                                            VIR_TYPED_PARAM_ULLONG, pages_shared) < 0)
> +                    goto cleanup;
> +
> +                VIR_FREE(path);
> +                VIR_FREE(buf);
> +                break;
> +
> +            case 3:
> +                if (virAsprintf(&path, "%s/%s", SYSFS_MEMORY_SHARED_PATH,
> +                                VIR_NODE_MEMORY_SHARED_PAGES_SHARING) < 0) {
> +                    virReportOOMError();
> +                    goto cleanup;
> +                }
> +
> +                if (virFileReadAll(path, 1024, &buf) < 0)
> +                    goto cleanup;
> +
> +                if ((tmp = strchr(buf, '\n')))
> +                    *tmp = '\0';
> +
> +                if (virStrToLong_ull(buf, NULL, 10, &pages_sharing) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                   _("failed to parse pages_sharing"));
> +                    goto cleanup;
> +                }
> +
> +                if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_PAGES_SHARING,
> +                                            VIR_TYPED_PARAM_ULLONG, pages_sharing) < 0)
> +                    goto cleanup;
> +
> +                VIR_FREE(path);
> +                VIR_FREE(buf);
> +                break;
> +
> +            case 4:
> +                if (virAsprintf(&path, "%s/%s", SYSFS_MEMORY_SHARED_PATH,
> +                                VIR_NODE_MEMORY_SHARED_PAGES_UNSHARED) < 0) {
> +                    virReportOOMError();
> +                    goto cleanup;
> +                }
> +
> +                if (virFileReadAll(path, 1024, &buf) < 0)
> +                    goto cleanup;
> +
> +                if ((tmp = strchr(buf, '\n')))
> +                    *tmp = '\0';
> +
> +                if (virStrToLong_ull(buf, NULL, 10, &pages_unshared) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                   _("failed to parse pages_unshared"));
> +                    goto cleanup;
> +                }
> +
> +                if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_PAGES_UNSHARED,
> +                                            VIR_TYPED_PARAM_ULLONG, pages_unshared) < 0)
> +                    goto cleanup;
> +
> +                VIR_FREE(path);
> +                VIR_FREE(buf);
> +                break;
> +
> +            case 5:
> +                if (virAsprintf(&path, "%s/%s", SYSFS_MEMORY_SHARED_PATH,
> +                                VIR_NODE_MEMORY_SHARED_PAGES_VOLATILE) < 0) {
> +                    virReportOOMError();
> +                    goto cleanup;
> +                }
> +
> +                if (virFileReadAll(path, 1024, &buf) < 0)
> +                    goto cleanup;
> +
> +                if ((tmp = strchr(buf, '\n')))
> +                    *tmp = '\0';
> +
> +                if (virStrToLong_ull(buf, NULL, 10, &pages_volatile) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                   _("failed to parse pages_volatile"));
> +                    goto cleanup;
> +                }
> +
> +                if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_PAGES_VOLATILE,
> +                                            VIR_TYPED_PARAM_ULLONG, pages_volatile) < 0)
> +                    goto cleanup;
> +
> +                VIR_FREE(path);
> +                VIR_FREE(buf);
> +                break;
> +
> +            case 6:
> +                if (virAsprintf(&path, "%s/%s", SYSFS_MEMORY_SHARED_PATH,
> +                                VIR_NODE_MEMORY_SHARED_FULL_SCANS) < 0) {
> +                    virReportOOMError();
> +                    goto cleanup;
> +                }
> +
> +                if (virFileReadAll(path, 1024, &buf) < 0)
> +                    goto cleanup;
> +
> +                if ((tmp = strchr(buf, '\n')))
> +                    *tmp = '\0';
> +
> +                if (virStrToLong_ull(buf, NULL, 10, &full_scans) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                   _("failed to parse full_scans"));
> +                    goto cleanup;
> +                }
> +
> +                if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_FULL_SCANS,
> +                                            VIR_TYPED_PARAM_ULLONG, full_scans) < 0)
> +                    goto cleanup;
> +
> +                VIR_FREE(path);
> +                VIR_FREE(buf);
> +                break;
> +
> +            default:
> +                break;
> +            }
> +        }
> +
> +        ret = 0;
> +
> +    cleanup:
> +        VIR_FREE(path);
> +        VIR_FREE(buf);
> +        return ret;
> +    }
> +#else
> +    virReportError(VIR_ERR_NO_SUPPORT, "%s",
> +                   _("node get memory parameters not implemented"
> +                     " on this platform"));
> +    return -1;
> +#endif
> +}
> +
>  #if HAVE_NUMACTL
>  # if LIBNUMA_API_VERSION <= 1
>  #  define NUMA_MAX_N_CPUS 4096
> diff --git a/src/nodeinfo.h b/src/nodeinfo.h
> index 12090e2..25ad0dd 100644
> --- a/src/nodeinfo.h
> +++ b/src/nodeinfo.h
> @@ -49,4 +49,14 @@ unsigned long long nodeGetFreeMemory(virConnectPtr conn);
>  char *nodeGetCPUmap(virConnectPtr conn,
>                      int *max_id,
>                      const char *mapname);
> +
> +int nodeGetMemoryParameters(virConnectPtr conn,
> +                            virTypedParameterPtr params,
> +                            int *nparams,
> +                            unsigned int flags);
> +
> +int nodeSetMemoryParameters(virConnectPtr conn,
> +                            virTypedParameterPtr params,
> +                            int nparams,
> +                            unsigned int flags);
>  #endif /* __VIR_NODEINFO_H__*/

  No fundamental disagreement, but this needs a bit of rework IMHO

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]