5:32 a.m.
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.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
8:21 a.m.
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:38 a.m.
"Richard W.M. Jones" <rjones(a)redhat.com> 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.
Looks good.
+1
...
+ struct qemud_driver *driver =
+ (struct qemud_driver *)dom->conn->privateData;
+ char *info, *p, *dummy, *eol;
+ char qemu_dev_name[32];
+ int len;
+ struct qemud_vm *vm = qemudFindVMByID(driver, dom->id);
A couple suggestions: change type of len to "size_t", since
it holds strlen value, and make a few pointers "const".
Doing that exposed a couple of const-incorrect interfaces,
so this fixes those, too.
Plus, add a few "%s" before format-string-without-%
to avoid warnings when building with --disable-nls.
---
src/qemu_conf.h | 4 ++--
src/qemu_driver.c | 21 +++++++++++----------
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/src/qemu_conf.h b/src/qemu_conf.h
index 9f09ec1..735da48 100644
--- a/src/qemu_conf.h
+++ b/src/qemu_conf.h
@@ -318,13 +318,13 @@ struct qemud_driver {
static inline int
-qemudIsActiveVM(struct qemud_vm *vm)
+qemudIsActiveVM(const struct qemud_vm *vm)
{
return vm->id != -1;
}
static inline int
-qemudIsActiveNetwork(struct qemud_network *network)
+qemudIsActiveNetwork(const struct qemud_network *network)
{
return network->active;
}
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index f97ef18..d9a7aca 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1315,8 +1315,9 @@ static void qemudDispatchVMEvent(int fd, int events, void *opaque)
{
qemudDispatchVMFailure(driver, vm, fd);
}
-static int qemudMonitorCommand(struct qemud_driver *driver ATTRIBUTE_UNUSED,
- struct qemud_vm *vm,
+static int qemudMonitorCommand(const struct qemud_driver *driver
+ ATTRIBUTE_UNUSED,
+ const struct qemud_vm *vm,
const char *cmd,
char **reply) {
int size = 0;
@@ -2520,12 +2521,12 @@ qemudDomainBlockStats (virDomainPtr dom,
const char *path,
struct _virDomainBlockStats *stats)
{
- struct qemud_driver *driver =
- (struct qemud_driver *)dom->conn->privateData;
- char *info, *p, *dummy, *eol;
+ const struct qemud_driver *driver = (void *)dom->conn->privateData;
+ char *dummy, *info;
+ const char *p, *eol;
char qemu_dev_name[32];
- int len;
- struct qemud_vm *vm = qemudFindVMByID(driver, dom->id);
+ size_t len;
+ const struct qemud_vm *vm = qemudFindVMByID(driver, dom->id);
if (!vm) {
qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
@@ -2534,7 +2535,7 @@ qemudDomainBlockStats (virDomainPtr dom,
}
if (!qemudIsActiveVM (vm)) {
qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _("domain is not running"));
+ "%s", _("domain is not running"));
return -1;
}
@@ -2564,7 +2565,7 @@ qemudDomainBlockStats (virDomainPtr dom,
if (qemudMonitorCommand (driver, vm, "info blockstats", &info) < 0)
{
qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _("'info blockstats' command failed"));
+ "%s", _("'info blockstats' command
failed"));
return -1;
}
@@ -2578,7 +2579,7 @@ qemudDomainBlockStats (virDomainPtr dom,
if (STREQLEN (info, "info ", 5)) {
free (info);
qemudReportError (dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
- _("'info blockstats' not supported by this
qemu"));
+ "%s", _("'info blockstats' not supported
by this qemu"));
return -1;
}
--
1.5.4.3.221.gf57a
8:49 a.m.
On Tue, Feb 26, 2008 at 03:38:56PM +0100, Jim Meyering wrote:
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index f97ef18..d9a7aca 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1315,8 +1315,9 @@ static void qemudDispatchVMEvent(int fd, int events, void *opaque)
{
qemudDispatchVMFailure(driver, vm, fd);
}
-static int qemudMonitorCommand(struct qemud_driver *driver ATTRIBUTE_UNUSED,
- struct qemud_vm *vm,
+static int qemudMonitorCommand(const struct qemud_driver *driver
+ ATTRIBUTE_UNUSED,
+ const struct qemud_vm *vm,
I don't like having annotations for a param on a separate line, because
it doesn't read naturally. If we really need to wrap this, then lets
follow the style of putting the return type on a line before , eg
static int
qemudMonitorCommand(const struct qemud_driver *driver ATTRIBUTE_UNUSED,
const struct qemud_vm *vm,
If it is still over 80 characters with this convention, then so be it.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
8:53 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Tue, Feb 26, 2008 at 03:38:56PM +0100, Jim Meyering wrote:
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index f97ef18..d9a7aca 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -1315,8 +1315,9 @@ static void qemudDispatchVMEvent(int fd, int events, void
*opaque) {
> qemudDispatchVMFailure(driver, vm, fd);
> }
>
> -static int qemudMonitorCommand(struct qemud_driver *driver ATTRIBUTE_UNUSED,
> - struct qemud_vm *vm,
> +static int qemudMonitorCommand(const struct qemud_driver *driver
> + ATTRIBUTE_UNUSED,
> + const struct qemud_vm *vm,
I don't like having annotations for a param on a separate line, because
it doesn't read naturally. If we really need to wrap this, then lets
follow the style of putting the return type on a line before , eg
static int
qemudMonitorCommand(const struct qemud_driver *driver ATTRIBUTE_UNUSED,
const struct qemud_vm *vm,
Good idea.
I prefer that anyhow, because it's what I'm used to and since
some naive start-of-function-finding tools work better that way.
If it is still over 80 characters with this convention, then so be
it.
12:40 p.m.
Thanks everyone. I've committed that with the suggested changes.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
6114
days inactive
6114
days old
5 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Jim Meyering -
Richard W.M. Jones