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@xxxxxxxxxx> >>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> >>> --- >>> 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
