[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv4 05/10] Implement lxcDomainBlockStats* for lxc driver



On 14.02.2014 18:49, Thorsten Behrens wrote:
Adds lxcDomainBlockStatsFlags and lxcDomainBlockStats functions.
---
  src/lxc/lxc_driver.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 195 insertions(+)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index e31b3ac..e1fcceb 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -76,6 +76,7 @@


  #define LXC_NB_MEM_PARAM  3
+#define LXC_NB_DOMAIN_BLOCK_STAT_PARAM 4


  static int lxcStateInitialize(bool privileged,
@@ -2230,6 +2231,198 @@ lxcDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array,


  static int
+lxcDomainBlockStats(virDomainPtr dom,
+                    const char *path,
+                    struct _virDomainBlockStats *stats)
+{
+    int ret = -1, idx;
+    virDomainObjPtr vm;
+    virDomainDiskDefPtr disk = NULL;
+    virLXCDomainObjPrivatePtr priv;
+
+    if (!(vm = lxcDomObjFromDomain(dom)))
+        return ret;
+
+    priv = vm->privateData;
+
+    if (virDomainBlockStatsEnsureACL(dom->conn, vm->def) < 0)
+        goto cleanup;
+
+    if (!virDomainObjIsActive(vm)) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       "%s", _("domain is not running"));
+        goto cleanup;
+    }
+
+    if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
+        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                       _("blkio cgroup isn't mounted"));
+        goto cleanup;
+    }
+
+    if (!*path) {

....

+        /* empty path - return entire domain blkstats instead */
+        ret = virCgroupGetBlkioIoServiced(priv->cgroup,
+                                          &stats->rd_bytes,
+                                          &stats->wr_bytes,
+                                          &stats->rd_req,
+                                          &stats->wr_req);
+        goto cleanup;
+    }
+
+    if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("invalid path: %s"), path);
+        goto cleanup;
+    }
+    disk = vm->def->disks[idx];
+
+    if (!disk->info.alias) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("missing disk device alias name for %s"), disk->dst);
+        goto cleanup;
+    }
+
+    ret = virCgroupGetBlkioIoDeviceServiced(priv->cgroup,
+                                            disk->info.alias,
+                                            &stats->rd_bytes,
+                                            &stats->wr_bytes,
+                                            &stats->rd_req,
+                                            &stats->wr_req);
+cleanup:
+    if (vm)
+        virObjectUnlock(vm);
+    return ret;
+}
+
+
+static int
+lxcDomainBlockStatsFlags(virDomainPtr dom,
+                         const char * path,
+                         virTypedParameterPtr params,
+                         int * nparams,
+                         unsigned int flags)
+{
+    int tmp, ret = -1, idx;
+    virDomainObjPtr vm;
+    virDomainDiskDefPtr disk = NULL;
+    virLXCDomainObjPrivatePtr priv;
+    long long rd_req, rd_bytes, wr_req, wr_bytes;
+    virTypedParameterPtr param;
+
+    virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+    /* We don't return strings, and thus trivially support this flag.  */
+    flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
+
+    if (!params && !*nparams) {
+        *nparams = LXC_NB_DOMAIN_BLOCK_STAT_PARAM;
+        return 0;
+    }
+
+    if (!(vm = lxcDomObjFromDomain(dom)))
+        return ret;
+
+    priv = vm->privateData;
+
+    if (virDomainBlockStatsFlagsEnsureACL(dom->conn, vm->def) < 0)
+        goto cleanup;
+
+    if (!virDomainObjIsActive(vm)) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       "%s", _("domain is not running"));
+        goto cleanup;
+    }
+
+    if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
+        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                       _("blkio cgroup isn't mounted"));
+        goto cleanup;
+    }
+
+    if (!*path) {

Okay, we allow passing "" as path. But only via C API, not in virsh. So I guess this is still okay - esp. if a quick look at the very next patch address the virsh part of my concern. Although, the documentation to these APIs don't mention passing empty string - which again you're fixing in the next patch. I think I can overlook fact that it should be in this patch (or the order of these two patches should be reversed).

+        /* empty path - return entire domain blkstats instead */
+        if (virCgroupGetBlkioIoServiced(priv->cgroup,
+                                        &rd_bytes,
+                                        &wr_bytes,
+                                        &rd_req,
+                                        &wr_req) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           "%s", _("domain stats query failed"));
+            goto cleanup;
+        }
+    } else {
+        if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
+            virReportError(VIR_ERR_INVALID_ARG,
+                           _("invalid path: %s"), path);
+            goto cleanup;
+        }
+        disk = vm->def->disks[idx];
+
+        if (!disk->info.alias) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("missing disk device alias name for %s"), disk->dst);
+            goto cleanup;
+        }
+
+        if (virCgroupGetBlkioIoDeviceServiced(priv->cgroup,
+                                              disk->info.alias,
+                                              &rd_bytes,
+                                              &wr_bytes,
+                                              &rd_req,
+                                              &wr_req) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           "%s", _("domain stats query failed"));
+            goto cleanup;
+        }
+    }

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]