On 07/16/2014 07:53 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.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> .gitignore | 1 +
> Makefile.am | 2 +-
> cfg.mk | 2 +-
> configure.ac | 1 +
> examples/domtop/Makefile.am | 27 +++
> examples/domtop/domtop.c | 388 ++++++++++++++++++++++++++++++++++++++++++++
> libvirt.spec.in | 2 +-
> 7 files changed, 420 insertions(+), 3 deletions(-)
> create mode 100644 examples/domtop/Makefile.am
> create mode 100644 examples/domtop/domtop.c
>
[first round review - I still plan to compile and examine what happens
when actually running the program, which may result in more comments...]
> +++ b/cfg.mk
> @@ -1078,7 +1078,7 @@ exclude_file_name_regexp--sc_prohibit_sprintf = \
> exclude_file_name_regexp--sc_prohibit_strncpy = ^src/util/virstring\.c$$
>
> exclude_file_name_regexp--sc_prohibit_strtol = \
> - ^(src/(util/virsexpr|(vbox|xen|xenxs)/.*)\.c)|(examples/domsuspend/suspend.c)$$
> + ^(src/(util/virsexpr|(vbox|xen|xenxs)/.*)\.c)|(examples/domsuspend/suspend.c)|(examples/domtop/domtop.c)$$
Long line. I'd be happy with the shorter equivalent:
^(src/(util/virsexpr|(vbox|xen|xenxs)/.*)|examples/dom.*/.*)\.c$$
[side question - why are we allowing strtol in vbox, xen, and xenxs?
Probably worth an independent cleanup there]
> +++ b/examples/domtop/domtop.c
> @@ -0,0 +1,388 @@
> +
> +static int debug;
> +static int run_top = 1;
Worth including <stdbool.h> and making these bool?
> +
> + printf("\n%s [options] [domain name]\n\n"
> + " options:\n"
> + " -d | --debug enable debug printings\n"
s/printings/messages/
> + " -h | --help print this help\n"
> + " -c | --connect=URI hypervisor connection URI\n"
> + " -D | --delay=X delay between updates in miliseconds\n",
s/miliseconds/milliseconds/
> + unified_progname);
> +}
> +
> +static int
> +parse_argv(int argc, char *argv[],
> + const char **uri,
> + const char **dom_name,
> + unsigned int *mili_seconds)
s/mili_seconds/milliseconds/
> +
> + while ((arg = getopt_long(argc, argv, "+:dhc:D:", opt, NULL)) != -1) {
> + switch (arg) {
> + case 'd':
> + debug = 1;
again, bool might be nicer here.
> +
> + printf("Running domains:\n");
> + printf("----------------\n");
Personal preference - I like puts() rather than printf() when there is
no % in the format string.
> +
> +static int
> +do_top(virConnectPtr conn,
> + const char *dom_name,
> + unsigned int mili_seconds)
s/mili_seconds/milliseconds/
Overall looks fairly useful.
--
Eric Blake eblake redhat com +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