[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH] docs: Discourage users from using fwcfg



On 09/08/20 11:12, Daniel P. Berrangé wrote:
> On Tue, Sep 08, 2020 at 11:02:10AM +0200, Laszlo Ersek wrote:
>> On 09/07/20 15:48, Michal Privoznik wrote:
>>> Even though this was brought up in upstream discussion [1] it
>>> missed my patches: users should prefer <oemStrings/> over fwcfg.
>>> The reason is that fwcfg is considered somewhat internal to QEMU
>>> and it has limited number of slots and neither of these applies
>>> to <oemStrings/>.
>>>
>>> While I'm at it, I'm fixing the example too (because it contains
>>> incorrect element name) and clarifying sysfs/ exposure.
>>>
>>> 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html
>>>
>>> Reported-by: Laszlo Ersek <lersek redhat com>
>>> Signed-off-by: Michal Privoznik <mprivozn redhat com>
>>> ---
>>>  docs/formatdomain.rst | 14 +++++++++-----
>>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
>>> index 1979dfb8d3..821ffe8d60 100644
>>> --- a/docs/formatdomain.rst
>>> +++ b/docs/formatdomain.rst
>>> @@ -509,18 +509,22 @@ layout of sub-elements, with supported values of:
>>>     Some hypervisors provide unified way to tweak how firmware configures itself,
>>>     or may contain tables to be installed for the guest OS, for instance boot
>>>     order, ACPI, SMBIOS, etc. It even allows users to define their own config
>>> -   blobs. In case of QEMU, these then appear under domain's sysfs, under
>>> +   blobs. In case of QEMU, these then appear under domain's sysfs (if the guest
>>> +   kernel has FW_CFG_SYSFS config option enabled), under
>>>     ``/sys/firmware/qemu_fw_cfg``. Note, that these values apply regardless the
>>>     <smbios/> mode under <os/>. :since:`Since 6.5.0`
>>>
>>> +   **Please note that because of limited number of data slots use of fwcfg is
>>> +   strongly discouraged and <oemStrings/> should be used instead**.
>>
>> please replace:
>>
>>   strongly discouraged
>>
>> with:
>>
>>   strongly discouraged for configuring any guest-side component other
>>   than the firmware
>>
>> (
>>
>> Consider for example the following feature:
>>
>>   https://bugzilla.tianocore.org/show_bug.cgi?id=2681
>>
>> Namely, the following QEMU switches:
>>
>>   -fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn]
>>   -fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn]
>>
>> alter the behavior of OVMF and ArmVirtQemu. These flags are meant to be
>> stable. They do not need dedicated QEMU or libvirtd enablement. They
>> influence firmware behavior. So <sysinfo type='fwcfg'> is perfectly fine
>> (even ideal!) for tweaking them, through the domain XML. What's not fine
>> is configuring any random guest payload via <sysinfo type='fwcfg'>.
>>
>> The point is that people who parse new fw_cfg files in edk2 such as
>> "opt/org.tianocore/IPv6PXESupport" are conscious of the slot count in
>> QEMU. They *can* bump the "x-file-slots" property in QEMU, for new
>> machine types, they just need to be aware of the property.
> 
> I'd suggest that QEMU always sets x-file-slots to be 10 entries larger
> than the number of officially known slots. That leads some scope for
> app usage without risk of hitting the limits.

Right, at least I believe we've tried maintaining a "safety margin". The
"officially known slots" is sort of a fuzzy count one can deduce from
grepping the QEMU, SeaBIOS and edk2 source trees. So when we'd like to
introduce a new file, we redo those greps, and compare with the current
x-file-slots value (for the latest machine types, that is).

> 
> 
>>> -          <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/>
>>> -        </smbios>
>>> +          <entry name='opt/com.example/config' file='/tmp/provision.ign'/>
>>
>> We have a functional -- working, stable -- example for name+file as
>> well:
>>
>> - name: etc/edk2/https/cacerts
>> - file: /etc/pki/ca-trust/extracted/edk2/cacerts.bin
> 
> I don't think we should document that in libvirt, since it is something
> libvirt will set silently behind the scenes. The ignition example is a
> acceptable real world example of expected usage in libvirt.

OK. I wasn't sure if specific libvirt enablement was going to occur for
the CA certs passing.

Thanks,
Laszlo


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]