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

[Cole Robinson]

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 at 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

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.

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.

- 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.

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.

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)