Re: [libvirt] [PATCH v2] examples: Introduce domtop
On 07/18/2014 03:08 AM, Michal Privoznik wrote:
There's this question on the list that is asked over and over
again.
How do I get {cpu, memory, ...} usage in percentage? Or its modified
version: How do I plot nice graphs like virt-manager does?
It would be nice if we have an example to inspire people. And that's
what domtop should do. Yes, it could be written in different ways, but
I've chosen this one as I think it show explicitly what users need to
implement in order to imitate virt-manager's graphing.
Note: The usage is displayed from host perspective. That is, how much
host CPUs the domain is using. But it should be fairly simple to
switch do just guest CPU usage if needed.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
+++ b/examples/domtop/domtop.c
+
+#define STREQ(a,b) (strcmp(a,b) == 0)
Space after comma, twice.
+
+static void
+print_usage(const char *progname)
+{
+ const char *unified_progname;
+
+ if (!(unified_progname = strrchr(progname, '/')))
+ unified_progname = progname;
+ else
+ unified_progname++;
+
+ printf("\n%s [options] [domain name]\n\n"
+ " options:\n"
+ " -d | --debug enable debug messages\n"
+ " -h | --help print this help\n"
+ " -c | --connect=URI hypervisor connection URI\n"
+ " -D | --delay=X delay between updates in milliseconds\n"
Maybe mention the default, if -D is not specified.
+ "\n"
+ "Print the cumulative usage of each host CPU. \n"
+ "Without any domain name specified the list of \n"
Two instances of trailing whitespace in the output. s/ \n/\n/
+ case 'D':
+ /* strtoul man page suggest clearing errno prior to call */
s/suggest/suggests/
+ errno = 0;
+ val = strtoul(optarg, &p, 10);
+ if (errno || *p || p == optarg) {
+ ERROR("Invalid number: '%s'", optarg);
+ goto cleanup;
+ }
+ *milliseconds = val;
+ if (*milliseconds != val) {
+ ERROR("Integer overflow: %ld", val);
+ goto cleanup;
Looks odd to have 'goto cleanup' here...
+ }
+ break;
+ case ':':
+ ERROR("option '-%c' requires an argument", optopt);
+ exit(EXIT_FAILURE);
+ case '?':
+ if (optopt)
+ ERROR("unsupported option '-%c'. See --help.",
optopt);
+ else
+ ERROR("unsupported option '%s'. See --help.",
argv[optind - 1]);
+ exit(EXIT_FAILURE);
...but direct exit here...
+ default:
+ ERROR("unknown option");
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ if (argc > optind)
+ *dom_name = argv[optind];
+
+ ret = 0;
+ cleanup:
+ return ret;
...especially since there is nothing cleaned up, and the main() function
eventually just exits anyway. I don't care whether you keep the cleanup
label and consistently use it, or whether you just use direct exit()
everywhere in this function, but at least be consistent :)
+static void
+print_cpu_usage(const char *dom_name,
+ size_t cpu,
+ size_t ncpus,
+ unsigned long long then,
+ virTypedParameterPtr then_params,
+ size_t then_nparams,
+ unsigned long long now,
+ virTypedParameterPtr now_params,
+ size_t now_nparams)
+{
+ size_t i, j, k;
+ size_t nparams = now_nparams;
+ bool delim = false;
+
+ for (i = 0; i < ncpus; i++) {
+
+ if (delim)
+ printf("\t");
+ printf("CPU%zu: %.2lf", cpu + i, usage);
+ delim = true;
+ }
+
+ if (delim)
+ printf("\n");
This 'if' is dead (it can only be false if ncpus is 0 - but all hosts
have at least one), so you can unconditionally print the \n.
ACK with nits fixed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)