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

On Tue, Sep 08, 2020 at 10:22:34AM +0200, Laszlo Ersek wrote: > On 09/08/20 08:37, Martin Kletzander wrote: >> On Mon, Sep 07, 2020 at 03:38:23PM +0100, Daniel P. Berrangé wrote: >>> On Mon, Sep 07, 2020 at 04:20:02PM +0200, Michal Privoznik wrote: >>>> On 9/7/20 3:57 PM, Martin Kletzander wrote: >>>>> On Mon, Sep 07, 2020 at 03:48:16PM +0200, 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(-) >>>>>> >>>> >>>> >>>>> It's nice that you say that, but people who would like to use >>>> fw_cfg for >>>>> passing >>>>> in a huge blob, which is saved in a file, will read this, go to >>>>> <oemStrings/> >>>>> and see that there is no way to pass a file as an input.  Should >>>> that be >>>>> dealt >>>>> with somehow?  Or would that be discouraged as well? >>>> >>>> Unfortunately, QEMU doesn't allow reading OEM strings from a file (at >>>> least >>>> quick glance over hw/smbios/smbios.c doesn't show any signs it's >>>> allowed). >>> >>> Indeed, that is an RFE I've never got around to >>> >> >> Do you mean RFE for QEMU or libvirt? >> >> Are there any limitations for the oemStrings?  Libvirt could read the >> file and >> pass the data to QEMU as it is not something that is supposed to be >> writeable or >> shareable.  I understand it could be the first time we do something like >> this, >> but it might not be that bad of a precedence. >> >> Just so we are on the same page, I'm not against this patch, I just want >> to save >> our users the frustration that I, myself, experienced so many times.  >> There's >> something deep hurting when you finally find exactly what you're looking >> for, >> only to learn that it is deprecated with another option which does not >> provide >> the same functionality. > > This description is not entirely accurate. I would put it like this: you > stumble upon a mis-use of a feature that was originally meant for > something else. It seems that the original consumers of the feature have > always been unhappy about the mis-use (it presents a risk for the > subsystem they are responsible for), in spite of the mis-use possibly > scratching your itch too. > > That shouldn't hurt your feelings -- the point is that it's not > "deprecation"; the original thing has never been condoned for this > particular creative kind of abuse. Instead, the feature that could > scratch your itch has never existed. I understand why you wish that was the case, but I don't think that is the reality today. The QEMU manual page has explicitly documented -fw_cfg, even going as far as illustrating its usage for passing application specific data to the guest: [quote] Add named fw_cfg entry with contents from string str. The terminating NUL character of the contents of str will not be included as part of the fw_cfg item data. To insert contents with embedded NUL characters, you have to use the file parameter. The fw_cfg entries are passed by QEMU through to the guest. Example: -fw_cfg name=opt/com.mycompany/blob,file=./my_blob.bin creates an fw_cfg entry named opt/com.mycompany/blob with contents from ./my_blob.bin. [/quote] Similarly the -help output merely lists -fw_cfg as a "debug/expert" option, and doesn't give any indication it is not for application usage. IOW, saying that this is not for general application usage is directly contradicting QEMU's own documentation that exists today. If you want to change such that this option is marked not for application usage, then that is certainly a deprecation of an existing documented feature in QEMU.