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@xxxxxxxxxxxxxxxxxx>
---
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)
