Re: [PATCH] Implement virDomainBlockStats for QEMU/KVM

[Jim Meyering <jim@xxxxxxxxxxxx>]

"Richard W.M. Jones" <rjones@xxxxxxxxxx> 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

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list