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

Re: [virt-tools-list] [PATCHv2 1/2] To add the huge page configuration in virt-manager



On 12/23/2011 03:06 AM, Cole Robinson wrote:
On 12/15/2011 01:15 AM, Li Zhang wrote:
This patch is to add the call back functions for
setting huge page. Clicking GTKCheck button will
invoke these functions.

Signed-off-by: Li Zhang<zhlcindy linux vnet ibm com>
---
  src/virtManager/details.py |   23 ++++++++++++++++++++++-
  src/virtManager/domain.py  |    7 +++++++
  2 files changed, 29 insertions(+), 1 deletions(-)


Code looks fine, but how this all works from a user perspective isn't very nice.


If a user doesn't know what this feature is and wants to enable it, they
currently have to do a few steps outside of virt-manager (not sure if this is
100% correct):

- mount hugetlbfs on the host
- allocate some memory to hugetlbfs with sysctl
- restart libvirtd to see the change

I agree. The user may be not admin. Errors will occur.

If they click this check box but don't mount hugetlbfs or restart libvirt,
their VM will fail to boot. Not sure what effect not allocating memory will
have, if any.

If a user allocate memory bigger than pre-allocated memory,
Qemu will invoke allocating memory errors.
For PowerKVM kernel, allocating huge page table fails.
It will invoke allocating HPT errors.

I think there some defects in libvirt to allocate memory.
There is no limitation about the memory allocation.
User may also assign an integer which is not (n*pagesize),
when the memory allocation fails.


I think there are a few parts that should be improved here:

- libvirt capabilities XML should expose whether hugetlbfs is mounted on the
host, and perhaps how much memory is allocated to it. the latter bit isn't
that interesting to virt-manager but smarter apps might want it. this should
probably be something like<hugepage mounted='yes|no'/>  so that apps don't
need to play guessing games whether libvirt has any idea.

That's good. We can add such a limitation, if it hugetlbfs is not
 mounted, the hugepage check box is disabled, not letting user to
 enable it.

- wrapping this into capabilities should make it easy for libvirt to not
require a restart to notice that hugetlbfs was actually mounted.


- someone needs to confirm that launching qemu with -mem-path /dev/hugepages
and 0 memory allocated to hugetlbfs has no detrimental effects (or fails
gracefully with a useful error). We want to avoid the situation where a host
misconfiguration causes the guest to underperform or fail strangely.

I think libvirt can add the limitation of memory size.

So before committing this I'd like to at least see those first two bits in
libvirt. That will allow virt-manager to give some reasonable feedback to the
user about whether the operation will even work.

Understand, I think these considerations are more reasonable. I will
see whether all of these can be applied.


Thanks,
Cole

diff --git a/src/virtManager/details.py b/src/virtManager/details.py
index fcd0652..92a13d8 100644
--- a/src/virtManager/details.py
+++ b/src/virtManager/details.py
@@ -38,7 +38,7 @@ from virtManager import util as util
  import virtinst

  # Parameters that can be editted in the details window
-EDIT_TOTAL = 35
+EDIT_TOTAL = 36
  (EDIT_NAME,
  EDIT_ACPI,
  EDIT_APIC,
@@ -52,6 +52,7 @@ EDIT_CPU,
  EDIT_TOPOLOGY,

  EDIT_MEM,
+EDIT_HUGEPG,

  EDIT_AUTOSTART,
  EDIT_BOOTORDER,
@@ -396,6 +397,7 @@ class vmmDetails(vmmGObjectUI):

              "on_config_memory_changed": self.config_memory_changed,
              "on_config_maxmem_changed": self.config_maxmem_changed,
+            "on_config_hugepage_changed": self.config_hugepage_changed,

              "on_config_boot_moveup_clicked" : (self.config_boot_move, True),
              "on_config_boot_movedown_clicked" : (self.config_boot_move,
@@ -1702,6 +1704,15 @@ class vmmDetails(vmmGObjectUI):
              maxadj.value = mem
          maxadj.lower = mem

+    def config_hugepage_changed(self, ignore):
+        self.enable_apply(EDIT_HUGEPG)
+        widget = self.widget("config-hugepage")
+        incon = widget.get_inconsistent()
+        widget.set_inconsistent(False)
+        if incon:
+            widget.set_active(True)
+        self.enable_apply(EDIT_HUGEPG)
+
      def generate_cpuset(self):
          mem = int(self.vm.get_memory()) / 1024 / 1024
          return virtinst.Guest.generate_cpuset(self.conn.vmm, mem)
@@ -2067,6 +2078,12 @@ class vmmDetails(vmmGObjectUI):
              add_define(self.vm.define_both_mem, curmem, maxmem)
              add_hotplug(self.vm.hotplug_both_mem, curmem, maxmem)

+        if self.editted(EDIT_HUGEPG):
+            enable_hugepage = self.widget("config-hugepage").get_active()
+            if self.widget("config-hugepage").get_inconsistent():
+                enable_hugepage = None
+            add_define(self.vm.define_hugepage, enable_hugepage)
+
          return self._change_config_helper(df, da, hf, ha)

      # Boot device / Autostart
@@ -2710,6 +2727,7 @@ class vmmDetails(vmmGObjectUI):
          host_mem = self.vm.conn.host_memory_size() / 1024
          vm_cur_mem = self.vm.get_memory() / 1024.0
          vm_max_mem = self.vm.maximum_memory() / 1024.0
+        vm_huge_page = self.vm.get_hugepage()

          host_mem_widget.set_text("%d MB" % (int(round(host_mem))))

@@ -2721,6 +2739,9 @@ class vmmDetails(vmmGObjectUI):
          if not self.widget("config-memory").get_property("sensitive"):
              maxmem.lower = curmem.value

+        self.widget("config-hugepage").set_active(bool(vm_huge_page))
+        self.widget("config-hugepage").set_inconsistent(
+                                vm_huge_page is None and self.is_customize_dialog)

      def refresh_disk_page(self):
          disk = self.get_hw_selection(HW_LIST_COL_DEVICE)
diff --git a/src/virtManager/domain.py b/src/virtManager/domain.py
index f1e601b..e29a1c0 100644
--- a/src/virtManager/domain.py
+++ b/src/virtManager/domain.py
@@ -481,6 +481,11 @@ class vmmDomain(vmmLibvirtObject):
              guest.maxmemory = int(int(maxmem) / 1024)
          return self._redefine_guest(change)

+    def define_hugepage(self, newvalue):
+        def change(guest):
+            guest.hugepage = newvalue
+        return self._redefine_guest(change)
+
      # Security define methods

      def define_seclabel(self, model, t, label):
@@ -847,6 +852,8 @@ class vmmDomain(vmmLibvirtObject):
          return int(self._get_guest().memory * 1024)
      def maximum_memory(self):
          return int(self._get_guest().maxmemory * 1024)
+    def get_hugepage(self):
+        return self._get_guest().hugepage

      def vcpu_count(self):
          return int(self._get_guest().vcpus)



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