
On 9/8/20 11:02 AM, 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
Actually, we have a check that forbids anything else than "opt/" prefix and also "opt/ovmf/" and "opt/org.qemu/" are forbidden: https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_validate.c#L7... Looks like "opt/org.tianocore" slipped through. I think the reasoning is that this is too low level and allows changing values of knobs that are exposed elsewhere (e.g. boot order). It really should be just to append new values, not change existing ones.
(
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".
I think in the light of what I wrote above this makes more sense, doesn't it? Michal