[ovs-dev] [worker v2 13/14] system-stats: Use "smap" instead of "shash".

Ethan Jackson ethan at nicira.com
Wed Jul 11 21:53:53 UTC 2012


Looks good to me.


Ethan

On Wed, Jun 27, 2012 at 1:25 PM, Ben Pfaff <blp at nicira.com> wrote:
> "smap" is now the appropriate data structure for a string-to-string map.
>
> Also changes ovsdb_datum_from_shash() into ovsdb_datum_from_smap() since
> system-stats related code was the only client.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/ovsdb-data.c        |   19 ++++++++-------
>  lib/ovsdb-data.h        |    5 ++-
>  vswitchd/bridge.c       |    6 ++--
>  vswitchd/system-stats.c |   58 +++++++++++++++++++++-------------------------
>  vswitchd/system-stats.h |    6 +++-
>  5 files changed, 47 insertions(+), 47 deletions(-)
>
> diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
> index 58c2a10..4fd3468 100644
> --- a/lib/ovsdb-data.c
> +++ b/lib/ovsdb-data.c
> @@ -29,6 +29,7 @@
>  #include "ovsdb-parser.h"
>  #include "json.h"
>  #include "shash.h"
> +#include "smap.h"
>  #include "sort.h"
>  #include "unicode.h"
>
> @@ -1524,27 +1525,27 @@ ovsdb_datum_to_bare(const struct ovsdb_datum *datum,
>  }
>
>  /* Initializes 'datum' as a string-to-string map whose contents are taken from
> - * 'sh'.  Destroys 'sh'. */
> + * 'smap'.  Destroys 'smap'. */
>  void
> -ovsdb_datum_from_shash(struct ovsdb_datum *datum, struct shash *sh)
> +ovsdb_datum_from_smap(struct ovsdb_datum *datum, struct smap *smap)
>  {
> -    struct shash_node *node, *next;
> +    struct smap_node *node, *next;
>      size_t i;
>
> -    datum->n = shash_count(sh);
> +    datum->n = smap_count(smap);
>      datum->keys = xmalloc(datum->n * sizeof *datum->keys);
>      datum->values = xmalloc(datum->n * sizeof *datum->values);
>
>      i = 0;
> -    SHASH_FOR_EACH_SAFE (node, next, sh) {
> -        datum->keys[i].string = node->name;
> -        datum->values[i].string = node->data;
> -        shash_steal(sh, node);
> +    SMAP_FOR_EACH_SAFE (node, next, smap) {
> +        datum->keys[i].string = node->key;
> +        datum->values[i].string = node->value;
> +        smap_steal(smap, node);
>          i++;
>      }
>      assert(i == datum->n);
>
> -    shash_destroy(sh);
> +    smap_destroy(smap);
>      ovsdb_datum_sort_unique(datum, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING);
>  }
>
> diff --git a/lib/ovsdb-data.h b/lib/ovsdb-data.h
> index 3609866..2e31cc5 100644
> --- a/lib/ovsdb-data.h
> +++ b/lib/ovsdb-data.h
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010, 2011 Nicira, Inc.
> +/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -23,6 +23,7 @@
>
>  struct ds;
>  struct ovsdb_symbol_table;
> +struct smap;
>
>  /* One value of an atomic type (given by enum ovs_atomic_type). */
>  union ovsdb_atom {
> @@ -172,7 +173,7 @@ void ovsdb_datum_to_string(const struct ovsdb_datum *,
>  void ovsdb_datum_to_bare(const struct ovsdb_datum *,
>                           const struct ovsdb_type *, struct ds *);
>
> -void ovsdb_datum_from_shash(struct ovsdb_datum *, struct shash *);
> +void ovsdb_datum_from_smap(struct ovsdb_datum *, struct smap *);
>
>  /* Comparison. */
>  uint32_t ovsdb_datum_hash(const struct ovsdb_datum *,
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 67e29d2..2478dde 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1870,14 +1870,14 @@ static void
>  refresh_system_stats(const struct ovsrec_open_vswitch *cfg)
>  {
>      struct ovsdb_datum datum;
> -    struct shash stats;
> +    struct smap stats;
>
> -    shash_init(&stats);
> +    smap_init(&stats);
>      if (enable_system_stats(cfg)) {
>          get_system_stats(&stats);
>      }
>
> -    ovsdb_datum_from_shash(&datum, &stats);
> +    ovsdb_datum_from_smap(&datum, &stats);
>      ovsdb_idl_txn_write(&cfg->header_, &ovsrec_open_vswitch_col_statistics,
>                          &datum);
>  }
> diff --git a/vswitchd/system-stats.c b/vswitchd/system-stats.c
> index cecd8f4..4dc2723 100644
> --- a/vswitchd/system-stats.c
> +++ b/vswitchd/system-stats.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2010 Nicira, Inc.
> +/* Copyright (c) 2010, 2012 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -15,6 +15,8 @@
>
>  #include <config.h>
>
> +#include "system-stats.h"
> +
>  #include <assert.h>
>  #include <ctype.h>
>  #include <dirent.h>
> @@ -34,7 +36,7 @@
>  #include "dirs.h"
>  #include "dynamic-string.h"
>  #include "shash.h"
> -#include "system-stats.h"
> +#include "smap.h"
>  #include "timeval.h"
>  #include "vlog.h"
>
> @@ -52,24 +54,23 @@ VLOG_DEFINE_THIS_MODULE(system_stats);
>  #endif
>
>  static void
> -get_cpu_cores(struct shash *stats)
> +get_cpu_cores(struct smap *stats)
>  {
>      long int n_cores = sysconf(_SC_NPROCESSORS_ONLN);
>      if (n_cores > 0) {
> -        shash_add(stats, "cpu", xasprintf("%ld", n_cores));
> +        smap_add_format(stats, "cpu", "%ld", n_cores);
>      }
>  }
>
>  static void
> -get_load_average(struct shash *stats OVS_UNUSED)
> +get_load_average(struct smap *stats OVS_UNUSED)
>  {
>  #if HAVE_GETLOADAVG
>      double loadavg[3];
>
>      if (getloadavg(loadavg, 3) == 3) {
> -        shash_add(stats, "load_average",
> -                  xasprintf("%.2f,%.2f,%.2f",
> -                            loadavg[0], loadavg[1], loadavg[2]));
> +        smap_add_format(stats, "load_average", "%.2f,%.2f,%.2f",
> +                        loadavg[0], loadavg[1], loadavg[2]);
>      }
>  #endif
>  }
> @@ -90,7 +91,7 @@ get_page_size(void)
>  }
>
>  static void
> -get_memory_stats(struct shash *stats)
> +get_memory_stats(struct smap *stats)
>  {
>      if (!LINUX) {
>          unsigned int pagesize = get_page_size();
> @@ -108,7 +109,7 @@ get_memory_stats(struct shash *stats)
>
>          mem_total = phys_pages * (pagesize / 1024);
>          mem_used = (phys_pages - avphys_pages) * (pagesize / 1024);
> -        shash_add(stats, "memory", xasprintf("%d,%d", mem_total, mem_used));
> +        smap_add_format(stats, "memory", "%d,%d", mem_total, mem_used);
>      } else {
>          static const char file_name[] = "/proc/meminfo";
>          int mem_used, mem_cache, swap_used;
> @@ -152,9 +153,8 @@ get_memory_stats(struct shash *stats)
>          mem_used = mem_total - mem_free;
>          mem_cache = buffers + cached;
>          swap_used = swap_total - swap_free;
> -        shash_add(stats, "memory",
> -                  xasprintf("%d,%d,%d,%d,%d", mem_total, mem_used, mem_cache,
> -                            swap_total, swap_used));
> +        smap_add_format(stats, "memory", "%d,%d,%d,%d,%d",
> +                        mem_total, mem_used, mem_cache, swap_total, swap_used);
>      }
>  }
>
> @@ -385,7 +385,7 @@ get_process_info(pid_t pid, struct process_info *pinfo)
>  }
>
>  static void
> -get_process_stats(struct shash *stats)
> +get_process_stats(struct smap *stats)
>  {
>      struct dirent *de;
>      DIR *dir;
> @@ -398,9 +398,9 @@ get_process_stats(struct shash *stats)
>
>      while ((de = readdir(dir)) != NULL) {
>          struct process_info pinfo;
> -        char *key, *value;
>          char *file_name;
>          char *extension;
> +        char *key;
>          pid_t pid;
>
>  #ifdef _DIRENT_HAVE_D_TYPE
> @@ -423,27 +423,23 @@ get_process_stats(struct shash *stats)
>
>          key = xasprintf("process_%.*s",
>                          (int) (extension - de->d_name), de->d_name);
> -        if (shash_find(stats, key)) {
> -            free(key);
> -            continue;
> -        }
> -
> -        if (LINUX && get_process_info(pid, &pinfo)) {
> -            value = xasprintf("%lu,%lu,%lld,%d,%lld,%lld",
> -                              pinfo.vsz, pinfo.rss, pinfo.cputime,
> -                              pinfo.crashes, pinfo.booted, pinfo.uptime);
> -        } else {
> -            value = xstrdup("");
> +        if (!smap_get(stats, key)) {
> +            if (LINUX && get_process_info(pid, &pinfo)) {
> +                smap_add_format(stats, key, "%lu,%lu,%lld,%d,%lld,%lld",
> +                                pinfo.vsz, pinfo.rss, pinfo.cputime,
> +                                pinfo.crashes, pinfo.booted, pinfo.uptime);
> +            } else {
> +                smap_add(stats, key, "");
> +            }
>          }
> -
> -        shash_add_nocopy(stats, key, value);
> +        free(key);
>      }
>
>      closedir(dir);
>  }
>
>  static void
> -get_filesys_stats(struct shash *stats OVS_UNUSED)
> +get_filesys_stats(struct smap *stats OVS_UNUSED)
>  {
>  #if HAVE_SETMNTENT && HAVE_STATVFS
>      static const char file_name[] = "/etc/mtab";
> @@ -489,14 +485,14 @@ get_filesys_stats(struct shash *stats OVS_UNUSED)
>      endmntent(stream);
>
>      if (s.length) {
> -        shash_add(stats, "file_systems", ds_steal_cstr(&s));
> +        smap_add(stats, "file_systems", ds_cstr(&s));
>      }
>      ds_destroy(&s);
>  #endif  /* HAVE_SETMNTENT && HAVE_STATVFS */
>  }
>
>  void
> -get_system_stats(struct shash *stats)
> +get_system_stats(struct smap *stats)
>  {
>      get_cpu_cores(stats);
>      get_load_average(stats);
> diff --git a/vswitchd/system-stats.h b/vswitchd/system-stats.h
> index ac4a65e..9f965c6 100644
> --- a/vswitchd/system-stats.h
> +++ b/vswitchd/system-stats.h
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2010 Nicira, Inc.
> +/* Copyright (c) 2010, 2012 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -16,6 +16,8 @@
>  #ifndef VSWITCHD_SYSTEM_STATS
>  #define VSWITCHD_SYSTEM_STATS 1
>
> -void get_system_stats(struct shash *);
> +struct smap;
> +
> +void get_system_stats(struct smap *);
>
>  #endif /* vswitchd/system-stats.h */
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list