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@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list