On Mon, Feb 06, 2017 at 10:23:36AM +0800, Eli Qiao wrote:
This patch adds some utils struct and functions to expose resctrl information.
virResCtrlAvailable: If resctrl interface exist on host virResCtrlGet: get specify type resource contral information virResCtrlInit: initialize resctrl struct from the host's sys fs. ResCtrlAll[]: an array to maintain resource control information.
Signed-off-by: Eli Qiao <liyong.qiao@intel.com> --- include/libvirt/virterror.h | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 4 + src/util/virerror.c | 1 + src/util/virresctrl.c | 330 ++++++++++++++++++++++++++++++++++++++++++++ src/util/virresctrl.h | 89 ++++++++++++ 6 files changed, 426 insertions(+) create mode 100644 src/util/virresctrl.c create mode 100644 src/util/virresctrl.h
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c new file mode 100644 index 0000000..63bc808 --- /dev/null +++ b/src/util/virresctrl.c @@ -0,0 +1,330 @@ +/* + * virresctrl.c: methods for managing resource contral + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Eli Qiao <liyong.qiao@intel.com> + */ +#include <config.h> + +#include <sys/ioctl.h> +#if defined HAVE_SYS_SYSCALL_H +# include <sys/syscall.h> +#endif +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> + +#include "virresctrl.h" +#include "viralloc.h" +#include "virerror.h" +#include "virfile.h" +#include "virhostcpu.h" +#include "virlog.h" +#include "virstring.h" +#include "nodeinfo.h" + +VIR_LOG_INIT("util.resctrl"); + +#define VIR_FROM_THIS VIR_FROM_RESCTRL + +static unsigned int host_id = 0; + +static virResCtrl ResCtrlAll[] = {
Lowercase for global variable names please.
+ { + .name = "L3", + .cache_level = "l3", + }, + { + .name = "L3DATA", + .cache_level = "l3", + }, + { + .name = "L3CODE", + .cache_level = "l3", + }, + { + .name = "L2", + .cache_level = "l2", + }, +}; + +static int virResCtrlGetInfoStr(const int type, const char *item, char **str) +{ + int ret = 0; + char *tmp; + char *path; + + if (asprintf(&path, "%s/%s/%s", RESCTRL_INFO_DIR, ResCtrlAll[type].name, item) < 0) + return -1;
Use of asprintf is forbidden in libvirt - use virAsprintf. Please make sure to run 'make check' and 'make syntax-check' on each patch to catch this kind of policy error. There's quite a few other style rules missed in these patches - i won't list them all since 'make syntax-check' can tell you.
+ if (virFileReadAll(path, 10, str) < 0) { + ret = -1; + goto cleanup; + } + + if ((tmp = strchr(*str, '\n'))) { + *tmp = '\0'; + } + +cleanup: + VIR_FREE(path); + return ret; +} + + +static int virResCtrlGetCPUValue(const char* path, char** value) +{ + int ret = -1; + char* tmp;
The '*' should be next to the variable name, not the type name. Multiple other cases follow
+ + if(virFileReadAll(path, 10, value) < 0) { + goto cleanup; + } + if ((tmp = strchr(*value, '\n'))) { + *tmp = '\0'; + } + ret = 0; +cleanup: + return ret; +} +
+int virResCtrlInit(void) { + int i = 0; + char *tmp; + int rc = 0; + + for(i = 0; i < RDT_NUM_RESOURCES; i++) { + if ((rc = asprintf(&tmp, "%s/%s", RESCTRL_INFO_DIR, ResCtrlAll[i].name)) < 0) { + continue;
Silently ignoring OOM is not good - please return a proper error - using virAsprintf would do so.
+ } + if (virFileExists(tmp)) { + if ((rc = virResCtrlGetConfig(i)) < 0 ) + VIR_WARN("Ignor error while get config for %d", i);
Again don't ignore errors like this - this should be fatal.
+ } + + VIR_FREE(tmp); + } + return rc; +} + +bool virResCtrlAvailable(void) { + if (!virFileExists(RESCTRL_INFO_DIR)) + return false; + return true; +} + +virResCtrlPtr virResCtrlGet(int type) { + return &ResCtrlAll[type]; +}
diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h new file mode 100644 index 0000000..f713e66 --- /dev/null +++ b/src/util/virresctrl.h @@ -0,0 +1,89 @@
+ +#ifndef __VIR_RESCTRL_H__ +# define __VIR_RESCTRL_H__ + +# include "virutil.h" +# include "virbitmap.h" + +#define RESCTRL_DIR "/sys/fs/resctrl" +#define RESCTRL_INFO_DIR "/sys/fs/resctrl/info" +#define SYSFS_SYSTEM_PATH "/sys/devices/system" + +#define MAX_CPU_SOCKET_NUM 8 +#define MAX_CBM_BIT_LEN 32 +#define MAX_SCHEMATA_LEN 1024 +#define MAX_FILE_LEN ( 10 * 1024 * 1024)
Doesn't seem like any of this needs to be in the header file
+ +enum { + RDT_RESOURCE_L3, + RDT_RESOURCE_L3DATA, + RDT_RESOURCE_L3CODE, + RDT_RESOURCE_L2, + /* Must be the last */ + RDT_NUM_RESOURCES,
Use a VIR_ prefix on these constants. Also the libvirt naming convention is to use RDT_RESOURCE_LAST as the last element.
+}; + + +typedef struct _virResCacheBank virResCacheBank; +typedef virResCacheBank *virResCacheBankPtr; +struct _virResCacheBank { + unsigned int host_id; + unsigned long long cache_size; + unsigned long long cache_left; + unsigned long long cache_min; + virBitmapPtr cpu_mask; +};
+typedef struct _virResCtrl virResCtrl; +typedef virResCtrl *virResCtrlPtr; +struct _virResCtrl { + bool enabled; + const char *name; + int num_closid; + int cbm_len; + int min_cbm_bits; + const char* cache_level; + int num_banks; + virResCacheBankPtr cache_banks; +};
Can any of these fields change at runtime, or are they all immutable once populated.
+bool virResCtrlAvailable(void); +int virResCtrlInit(void); +virResCtrlPtr virResCtrlGet(int);
API docs for these would be nice, especially describing whether virResCtrlPtr returned from this method is immutable or not. Since libvirt is multi-threaded this is important to know. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|