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

Re: [libvirt] [PATCH 1/3] virsh-host: fix pagesize unit of freepages



On 22.09.2014 12:14, Jincheng Miao wrote:
The unit of '--pagesize' of freepages is kibibytes.

https://bugzilla.redhat.com/show_bug.cgi?id=1145048

Signed-off-by: Jincheng Miao <jmiao redhat com>
---
  tools/virsh-host.c | 27 ++++++++++++++++++---------
  1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 7fc2120..6dcf77e 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -293,7 +293,7 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
      bool ret = false;
      unsigned int npages;
      unsigned int *pagesize = NULL;
-    unsigned long long tmp = 0;
+    unsigned int tmp = 0;
      int cell;
      unsigned long long *counts = NULL;
      size_t i, j;
@@ -304,9 +304,20 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
      xmlXPathContextPtr ctxt = NULL;
      bool all = vshCommandOptBool(cmd, "all");
      bool cellno = vshCommandOptBool(cmd, "cellno");
+    bool pagesz = vshCommandOptBool(cmd, "pagesize");

      VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno);

+    if (vshCommandOptUInt(cmd, "pagesize", &tmp) < 0) {
+        vshError(ctl, "%s", _("Invalid pagesize argument"));
+        goto cleanup;
+    }
+

No, previously we accepted something like '--pagesize 2M'. This undo this feature.

+    if ((pagesz || cellno) && tmp < 1) {
+        vshError(ctl, "%s", _("page size must be at least 1KiB"));
+        goto cleanup;
+    }
+
      if (all) {
          if (!(cap_xml = virConnectGetCapabilities(ctl->conn))) {
              vshError(ctl, "%s", _("unable to get node capabilities"));
@@ -363,6 +374,8 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)

              vshPrint(ctl, _("Node %d:\n"), cell);
              for (j = 0; j < npages; j++) {
+                if (pagesz && tmp != pagesize[j])
+                    continue;

Instead of this, I'd honor --pagesize when creating @pagesize array to call viNodeGetFreePages() a few lines above.

                  vshPrint(ctl, "%uKiB: %lld\n", pagesize[j], counts[j]);
              }
              vshPrint(ctl, "%c", '\n');
@@ -380,20 +393,16 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
          }

          if (cell < -1) {
-            vshError(ctl, "%s", _("cell number must be non-negative integer or -1"));
+            vshError(ctl, "%s",
+                     _("cell number must be non-negative integer or -1"));
              goto cleanup;
          }

-        if (vshCommandOptScaledInt(cmd, "pagesize", &tmp, 1, UINT_MAX) < 0) {

In fact this is the bit that's buggy here. It should have been 1024 instead of 1.

-            vshError(ctl, "%s", _("page size has to be a number"));
-            goto cleanup;
-        }
          /* page size is expected in kibibytes */
          pagesize = vshMalloc(ctl, sizeof(*pagesize));
-        *pagesize = tmp / 1024;

-        if (!pagesize[0]) {
-            vshError(ctl, "%s", _("page size must be at least 1KiB"));
+        if (vshCommandOptUInt(cmd, "pagesize", pagesize) < 0) {
+            vshError(ctl, "%s", _("Invalid cellno argument"));
              goto cleanup;
          }



Michal


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