On 02/09/2012 03:43 AM, Lai Jiangshan wrote:
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
>
> add virBitmapParseCommaSeparetedFormat() for parsing bitmap in
s/Separeted/Separated/
> comma separeted ascii format.
and again
> This format of bitmap is used in Linux sysfs and cpuset.
>
> Changelog:
> - fixed typos.
> - fixed string scan routine.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
> ---
> src/libvirt_private.syms | 2 +
> src/nodeinfo.c | 51 ++++++++++++++++++++++++++++++++++++
> src/nodeinfo.h | 4 +++
> src/util/bitmap.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
> src/util/bitmap.h | 6 +++-
> 5 files changed, 127 insertions(+), 1 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index d6ad36c..14b144f 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -17,6 +17,7 @@ virBitmapFree;
> virBitmapGetBit;
> virBitmapSetBit;
> virBitmapString;
> +virBitmapParseCommaSeparatedFormat;
Sorting.
>
>
> # buf.h
> @@ -797,6 +798,7 @@ nodeGetCellsFreeMemory;
> nodeGetFreeMemory;
> nodeGetInfo;
> nodeGetMemoryStats;
> +nodeGetCPUmap;
>
>
> # nwfilter_conf.h
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index e0b66f7..b0ebb88 100644
> --- a/src/nodeinfo.c
> +++ b/src/nodeinfo.c
> @@ -47,6 +47,7 @@
> #include "count-one-bits.h"
> #include "intprops.h"
> #include "virfile.h"
> +#include "bitmap.h"
>
>
> #define VIR_FROM_THIS VIR_FROM_NONE
> @@ -569,6 +570,33 @@ int linuxNodeGetMemoryStats(FILE *meminfo,
> cleanup:
> return ret;
> }
> +
> +/*
> + * Linux maintains cpu bit map. For example, if cpuid=5's flag is not set
> + * and max cpu is 7. The map file shows 0-4,6-7. This functin parses
s/functin/function/
> + * it and returns bitmap.
> + */
> +static virBitmapPtr linuxParseCPUmap(int *max_cpuid, const char *path)
How is this function different from virDomainCpuSetParse in
domain_conf.c? That function parses out a cpuset rather than a bitmap,
but the syntax string being parsed looks to be a superset of what you
are parsing here.
Maybe it's more efficient to write a generic conversion from cpumap to
bitset, and reuse the existing parser.
I'll have to see how you actually use the parsed bitset later in the
series to see which approach might be best.
> +{
> + char str[1024];
> + FILE *fp;
> + virBitmapPtr map = NULL;
> +
> + fp = fopen(path, "r");
> + if (!fp) {
> + virReportSystemError(errno, _("cannot open %s"), path);
> + goto cleanup;
> + }
> + if (fgets(str, sizeof(str), fp) == NULL) {
Are we sure that is long enough? Suppose I have a machine with 512
cores - I can come up with a cpumap that exceeds 1024 bytes.
> + virReportSystemError(errno, _("cannot read from %s"), path);
> + goto cleanup;
> + }
> + map = virBitmapParseCommaSeparatedFormat(str, max_cpuid);
> +
> +cleanup:
> + VIR_FORCE_FCLOSE(fp);
> + return map;
> +}
> #endif
>
> int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) {
> @@ -712,6 +740,29 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
> #endif
> }
>
> +virBitmapPtr nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED,
> + int *max_id,
max_id has to be marked unused, to avoid gcc warnings on non-Linux
compilation.
> + const char *mapname)
same for mapname.
> +{
> +#ifdef __linux__
> + char *path;
> + virBitmapPtr map;
> +
> + if (virAsprintf(&path, CPU_SYS_PATH "/%s", mapname) < 0) {
> + virReportOOMError();
> + return NULL;
> + }
> +
> + map = linuxParseCPUmap(max_id, path);
> + VIR_FREE(path);
> + return map;
> +#else
> + nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
> + _("node cpumap not implemented on this platform"));
> + return -1;
> +#endif
> +}
> +
> +
> +/**
> + * virBitmapParseCommaSeparatedFormat:
> + *
> + * When bitmap is printed in ascii format, especially in Linux,
> + * comma-separated format is sometimes used. For example, a bitmap 11111011 is
> + * represetned as 0-4,6-7. This function parses comma-separated format
s/represetned/represented/
> + * and returns virBitmap. Found max bit is returned, too.
> + *
> + * This functions stops if characters other than digits, ',', '-' are
> + * found. This function will be useful for parsing linux's bitmap.
> + */
> +virBitmapPtr virBitmapParseCommaSeparatedFormat(char *buf, int *max_bit)
const char *buf, since we aren't modifying it.
> +{
> + char *pos = buf;
const char *pos, since buf is const.
> + virBitmapPtr map = NULL;
> + int val, x;
> +
> + /* at first, find the highest number */
> + val = 0;
> + while ((*pos != '\n') && (*pos != 0)) {
> + if (c_isdigit(*pos)) {
> + virStrToLong_i(pos, &pos, 10, &val);
> + } else if ((*pos == ',') || (*pos == '-')) {
> + ++pos;
> + } else
> + break;
> + }
> + *max_bit = val;
Yeah, I'm definitely starting to think that reusing our existing cpumap
parser, then a loop that converts bytes of the cpumap into longs of a
virBitmap, might be a more reusable prospect.
> -
> +/*
> + * parese comma-separeted bitmap format and allocate virBitmap.
s/parese/parse/; s/separeted/separated/
> + */
> +virBitmapPtr virBitmapParseCommaSeparatedFormat(char *buf, int *max_bit)
> + ATTRIBUTE_RETURN_CHECK;
ATTRIBUTE_NONNULL(1), and decide whether max_bit can be NULL (if so, fix
the code to allow it; if not, add the attribute)
If we go with my reuse suggestion, this would be:
virBitmapPtr virBitmapCreateFromCpumap(char *cpumap, int maplen,
int *max_bit)
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list