On 2012年07月25日 21:20, Martin Kletzander wrote:
On 07/24/2012 11:18 AM, Osier Yang wrote:
This splits commands commands to monitor domain status into
virsh-domain-monitor.c. The helpers not for common use are moved too.
Standard copyright is added.
* tools/virsh.c:
Remove commands for domain monitoring group and a few helpers (
vshDomainIOErrorToString, vshGetDomainDescription,
vshDomainControlStateToString, vshDomainStateToString) not for
common use.
* tools/virsh-domain-monitor.c:
New file, filled with commands of domain monitor group.
---
tools/virsh-domain-monitor.c | 1685 +++++++++++++++++++++++++++++++++++++
tools/virsh.c | 1904 +++---------------------------------------
2 files changed, 1807 insertions(+), 1782 deletions(-)
create mode 100644 tools/virsh-domain-monitor.c
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
new file mode 100644
index 0000000..1a61f62
--- /dev/null
+++ b/tools/virsh-domain-monitor.c
@@ -0,0 +1,1685 @@
+/*
+ * virsh-domain.c: Commands to monitor domain status
Wrong filename in the header, should be virsh-domain-monitor.c
+ *
+ * Copyright (C) 2005, 2007-2012 Red Hat, Inc
[...]
+cleanup:
+ VIR_FREE(domxml);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(doc);
+
+ return desc;
+}
I'd add a empty line here for the sake of beauty.
+static const char *
+vshDomainControlStateToString(int state)
[...]
+static const char *
+vshDomainStateToString(int state)
+{
+ /* Can't use virDomainStateTypeToString, because we want to mark
+ * * strings for translation. */
This comment has weirdly shifted asterisk.
+ switch ((virDomainState) state) {
[...]
+
+/*
+ * "dommemstats" command
Should be 'dommemstat'
Also the order of the commands doesn't make much sense to me (it could
be in the same order as the domMonitoringCmds struct).
Actually I tried to sort that. But I gave up quickly for it will take
too much time. And honestly, it's boring.
Apart from the tiny beauty nits (I'd give ACK with that), there is a
problem with 'cfg.mk'. It specifies 'virsh.c' as one of the files with
exception for strcasecmp() and this function gets copied into
'virsh-domain-monitor.c'. If this is done because of this particular
occurrence, the file should be added to
'exclude_file_name_regexp--sc_avoid_strcase' in 'cfg.mk', otherwise make
syntax-check will fail.
Good catch, I did 'syntax-check', but not sure why I missed it.
Everything else is clean copy-paste, looks good, so ACK with at least
the 'sytax-check' problem fixed.
Will fix those nits and the syntax-check failure when pushing (
after the whole set is acked). Thanks for the reviewing.
Regards,
Osier
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list