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

[Li Zhang]

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