[libvirt] [PATCH] libxl: support <hap> feature

Joao Martins joao.m.martins at oracle.com
Thu Feb 4 22:37:17 UTC 2016

On 02/04/2016 06:22 PM, Jim Fehlig wrote:
> On 02/04/2016 05:54 AM, Joao Martins wrote:
>> On 02/04/2016 01:41 AM, Jim Fehlig wrote:
>>> On 02/03/2016 02:20 PM, Joao Martins wrote:
>>>> On 02/03/2016 04:12 AM, Jim Fehlig wrote:
>>>>> Even though the libxl driver advertises <hap> in capabilities,
>>>>> it is ignored when set in domXML. Enable hap in the
>>>>> libxl_domain_create_info struct when <hap> is specified in domXML.
>>>>> The xm/xl <-> domXML parser/formatter already supports hap but
>>>>> lacked a test with hap enabled.  Add a hap test.
>>> [...]
>>>> FWIW looks good and also:
>>>> Tested-by: Joao Martins <joao.m.martins at oracle.com>
>>> Thanks for reviewing/testing the patch! But after playing with the patch a bit
>>> more, I'm considering dropping it :-/.
>>> Before the patch, <hap> was ignored by the libxl driver, which left the 'hap'
>>> field of libxl_domain_create_info struct unset (= LIBXL__DEFBOOL_DEFAULT). For
>>> HVM guests, libxl will enable hap when libxl_domain_create_info.hap ==
>>> LIBXL__DEFBOOL_DEFAULT. And using hap is good, since using shadow page table is
>>> less efficient. The downside is there is no way to turn hap off.
>>> After the patch it is possible to turn hap on and off via presence or absence of
>>> <hap>. However, there is a *lot* of existing config without <hap> that currently
>>> reaps the benefits of hap nonetheless. After patch, that config would require
>>> adding <hap>, else shadow page table will be used. (Note that in xm and xl
>>> config, hap is enabled in the absence of 'hap='.)
>>> I'm starting to question whether this feature should even be exposed. I think
>>> the only use case is debugging. E.g. turn hap off if suspected hardware bug.
>>> Otherwise you would certainly want the hypervisor to use hap if it is able to do
>>> so. What do you think?
>> Hm, good point. There is another field for VIR_TRISTATE which represents when
>> it's absent (VIR_TRISTATE_SWITCH_ABSENT). So perhaps we would leave the default
>> (= LIBXL_DEFBOOL_DEFAULT) if it's absent, and only set hap if the user
>> explicitly switches on/off?
> But IIUC, absent == off. I.e., there is no way to turn hap off except the
> absence of <hap>.
Ah, right. I was double checking and there's really no way to have it.

> One option might be to extend <hap> with an 'enabled=yes|no' attribute. Then the
> absence of <hap> means hypervisor default. <hap> without the attribute would be
> the same as <hap enabled='yes'/> for backwards compatibility. And of course <hap
> enabled='no'/> disables hap.
And it sounds that it could be applicable for other features too: e.g. pae
(instead of having both pae and nonpae?)

> Note that capabilities currently advertises hap as
>   <hap default='off' toggle='yes'/>
> If folks agree to this option, the default should be changed to 'on'.
>>  I am not sure if the usecase is just debug, but
>> given there are certain performance differences compared to shadow paging
>> (depending on the HVM guest workload) it might be nice to allow the
>> administrator to choose it.
> Agreed. I think the above option accomplishes that.
Yeap, I wonder if the absence of a "default" in the capabilities could turn into
"hypervisor/driver default" since you can explicitly choose "on" or "off"
already. Or perhaps it doesn't make a lot of sense to do that.


> Regards,
> Jim
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

More information about the libvir-list mailing list