[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:05, Daniel P. Berrangé wrote:
> 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 redhat com>
>>>>>>> Signed-off-by: Michal Privoznik <mprivozn redhat com>
>>>>>>> ---
>>>>>>> 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.

... fair enough I guess.

Laszlo


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