On Tue, Feb 26, 2008 at 11:32:20AM +0000, Richard W.M. Jones wrote:
This little patch just implements the virDomainBlockStats call for qemu & kvm. It does this by using the new 'info blockstats' monitor command which I added to qemu & KVM upstream some months back, and (hopefully) it does the right thing if this command is not available.
sounds good, just a few comments
+/* This uses the 'info blockstats' monitor command which was + * integrated into both qemu & kvm in late 2007. If the command is + * not supported we detect this and return the appropriate error. + */ +static int +qemudDomainBlockStats (virDomainPtr dom, [...] + if (STREQLEN (path, "hd", 2) && path[2] >= 'a' && path[2] <= 'z')
Please fully parenthesize binary expressions relying just on the priority of operators is more risky and IMHO harder to check/understand
+ snprintf (qemu_dev_name, sizeof (qemu_dev_name), + "ide0-hd%d", path[2] - 'a'); + else if (STREQ (path, "cdrom")) + strcpy (qemu_dev_name, "ide1-cd0");
safe but strcpy turns my paranoid mode on :-) [...]
+ while (*p) { + if (STREQLEN (p, qemu_dev_name, len) + && p[len] == ':' && p[len+1] == ' ') {
(STREQLEN (p, qemu_dev_name, len) && (p[len] == ':') && (p[len+1] == ' ')) looks so much more readable to me
+ eol = strchr (p, '\n'); if (!eol) eol = p + strlen (p);
one statement per line, please, plus going to the line, did you changed editors recently :-) ? +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/