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

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(a)redhat.com&gt; > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > 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. ) > + > :: > > - <smbios type='fwcfg'> > + <sysinfo type='fwcfg'> > <entry name='opt/com.example/name'>example value</entry> I suggest (according to the above): - name: opt/org.tianocore/IPv4PXESupport - value: n > - <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 (This is documented in "OvmfPkg/README" in edk2, but it's not really relevant here.) > + </sysinfo> > > - The ``smbios`` element can have multiple ``entry`` child elements. Each > + The ``sysinfo`` element can have multiple ``entry`` child elements. Each > element then has mandatory ``name`` attribute, which defines the name of the > blob and must begin with ``"opt/"`` and to avoid clashing with other names is > advised to be in form ``"opt/$RFQDN/$name"`` where ``$RFQDN`` is a reverse > It's hard to express this cleanly. - The opt/RFQDN notation is a mitigation for users that are hell-bent on using fw-cfg files of their own purposes (not heeding our advice about not using fw-cfg for such purposes at all). So the idea is, "if you ignore our request, then (a) be prepared to run out of slots, and (b) *at least* use a name pattern (opt/RFQDN) that minimizes conflicts with other, similar-minded users / projects" - For "officially supported" knobs that the firmware looks at, it's fine to use any names -- they avoid conflicts with the above "rogue" files. Examples: - opt/ovmf/ -- reserved for historical reasons - opt/org.tianocore/ -- should never conflict due to RFQDN - etc/edk2/https/... -- should never conflict due to being outside of opt/ So I guess the short rule is, "Feel free to refer to any fw_cfg file name that your firmware officially supports. When defining other fw_cfg file names (i.e., for your own purposes), then prepare for breakage in the long-term, and then at least use the opt/RFQDN/ name pattern".