[PATCH] libxl: enable Xen's e820_host setting

[Jim Fehlig]

On 4/13/20 1:17 PM, Marek Marczykowski-Górecki wrote:
> On Thu, Apr 09, 2020 at 02:52:30PM -0600, Jim Fehlig wrote:
>> On 4/9/20 7:14 AM, Daniel P. Berrangé wrote:
>>> On Wed, Apr 08, 2020 at 02:29:16PM -0600, Jim Fehlig wrote:
>>>> Hotplugging PCI devices to Xen PV guests is only possible if the
>>>> libxl_domain_build_info struct has the e820_host field enabled when the
>>>> guest is created. By default it is disabled but libxl will automatically
> 
> This isn't fully true: xl will do that, not libxl. Which means under
> libvirt it will always be disabled.

Ah, thanks for pointing that out! I should have learned by now to take a closer 
look. The default of some settings is handled in xl, whereas others in libxl.

>>>> enable e820_host if the config contains one or more PCI devices, in which
>>>> case hotplugging additional PCI devices later works.
>>>>
>>>> According to xl.cfg(5) man page it is safe to unconditionally enable the
>>>> PV-only e820_host setting. Furthermore xen.git commits 414979ba85 and
>>>> f92337d949, which introduce the setting with a default of disabled, claim
>>>> the setting can be enabled or even removed "once the auto-ballooning of
>>>> guests with PCI devices works". Those commits are from May 2011 so I
>>>> think it is safe to say the issues have been resolved in the meantime.
>>>> Regardless, we should avoid exposing a Xen setting in libvirt that could
>>>> be removed later.
>>>
>>> Does this have any implications for live migration compatibility if you
>>> silently enable this for all guests ?
>>
>> Oh, right. Thanks for the reminder! I'll have to check but I suspect it will.
> 
> Can a VM with PCI device be live migrated? If not, it shouldn't be an
> issue if you enable it only for PCI-having domains (similarly as xl
> does).

This patch enables e820_host regardless if the domain has a PCI device. The 
concern is migrating a domain running on a host without this patch (e820_host 
disabled) to a host with this patch, where magically e820_host becomes enabled 
when the domain config is created. I suspect the OS running inside that domain 
would not be happy.

> Or patch it in libxl, see this discussion:
> http://xen.markmail.org/thread/awcswnywzei4s65e

I would prefer if such odd settings could be handled internally in libxl :-).

>>> In QEMU/KVM if you did this, it would be considered an ABI change and
>>> could break live migration of a guest launched on old libvirt, to a
>>> host running new libvirt.
>>
>> Nod. Do you have any suggestions on how to model this setting in libvirt? I
>> proposed adding a hypervisor feature for Xen in this thread
>>
>> https://www.redhat.com/archives/libvir-list/2020-April/msg00376.html
>>
>> rational being that for PV guests the hypervisor serves as the BIOS and
>> provides the facility to report the memory map to the OS. I couldn't really
>> think of a good fit for it within the <os> element and its children.
> 
> FWIW, in Qubes we have a patches adding e820_host setting here:
> https://github.com/QubesOS/qubes-core-libvirt/
> (patches 8-11)
> Not submitted before, exactly to avoid adding temporary options. But
> since 8+ years later it is still there, I think it's safe to assume it
> will be there for some more. Or at least it's worth to unbreak some
> configurations in the meantime.

Agreed.

> I'll rebase them on master and post here.

Thanks!

Regards,
Jim