Re: [Qemu-devel] [PATCH] fw_cfg: RQFN rules, documentation

[Michael S. Tsirkin]

Thanks a lot for the comments! I'll address them, for now snipping most
of it out and just answering your questions:

> >  when using the "-fw_cfg" command line option, to avoid conflicting with
> > -item names used internally by QEMU. For instance:
> > +item names used internally by QEMU, or by firmware. For instance:
> >  
> > -    -fw_cfg name=opt/my_item_name,file=./my_blob.bin
> > +    -fw_cfg name=opt/com.my_company/guestagent/guestblob,file=./my_blob.bin
> >  
> > -Similarly, QEMU developers *SHOULD NOT* use item names prefixed with
> > +Similarly, QEMU developers MUST NOT use item names prefixed with
> >  "opt/" when inserting items programmatically, e.g. via fw_cfg_add_file().
> > +
> > +For historical reasons "opt/ovmf/" is reserved for use with the OVMF 
> > firmware.
> > +
> > +To simplify guest firmware development, the prefix
> > +unsupported/root/ is automatically stripped from paths, which
> > +allows creating fw_cfg files in the root QEMU directory.  This interface is
> > +strictly for use by developers, its use can cause guest or QEMU crashes, is
> > +unsupported and can be removed at any point.
> > +
> 
> Okay, so you listed three groups of people in the discussion:
> - QEMU developers
> - QEMU firmware developers
> - users
> 
> QEMU developers shall use stuff outside of "opt/" (and in the future,
> maybe under "opt/org.qemu/"). Okay.
> 
> Users shall use "opt/com.my_company/..." style stuff; okay as well.
> 
> QEMU firmware developers will use "unsupported/root/..." when they want
> to mess with the firmware in connection with fw_cfg files that QEMU
> itself may expose under some circumstances. Okay.
> 
> Going forward, QEMU firmware developers shall use -- talking specifics
> now -- "opt/org.tianocore.edk2.ovmf/..." and "opt/org.seabios/..."
> pathnames for genuine firmware settings that QEMU doesn't / shouldn't
> populate itself, but users might want to. Is that right?
> 
> ... My question is, do we need the "opt/" prefix at all (for the future,
> i.e., the non-historical cases)?
> Looking at the last discussion, I
> believe we converged on:
> 
> - QEMU devs (future filenames): org.qemu/...
> - users:                        com.my_company/...
> - QEMU fw devs (future names):  org.tianocore.edk2.ovmf/...
>                                 org.seabios/...
> - QEMU fw devs hacking:         <root-prefix-to-strip>/...
> 
> Did you find something unsafe about this (necessitating "opt/")?
> 

The reason to use the opt/ prefix is to avoid warning
with QEMU 2.4 and 2.5.


....

> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index a770086..1af28ac 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -2860,18 +2860,46 @@ ETEXI
> >  
> >  DEF("fw_cfg", HAS_ARG, QEMU_OPTION_fwcfg,
> >      "-fw_cfg [name=]<name>,file=<file>\n"
> > -    "                add named fw_cfg entry from file\n"
> > +    "                add named fw_cfg entry with content from file\n"
> >      "-fw_cfg [name=]<name>,string=<str>\n"
> > -    "                add named fw_cfg entry from string\n",
> > +    "                add named fw_cfg entry with content from string\n",
> >      QEMU_ARCH_ALL)
> >  STEXI
> > +
> >  @item -fw_cfg address@hidden,address@hidden
> >  @findex -fw_cfg
> > -Add named fw_cfg entry from file. @var{name} determines the name of
> > -the entry in the fw_cfg file directory exposed to the guest.
> > +Add named fw_cfg entry with content from file @var{file}.
> >  
> >  @item -fw_cfg address@hidden,address@hidden
> > -Add named fw_cfg entry from string.
> > +Add named fw_cfg entry with content from string @var{str}, up to the first 
> > NUL character.
> > +
> > +The terminating NUL character of the content @var{str} will NOT be
> > +included as part of the fw_cfg item data. To insert content including
> > +the NUL character, store it in file and insert the item via
> > +the @var{file} option.
> > +
> > +Both the name and the content are passed by QEMU through to the guest, 
> > where:
> > address@hidden @option
> > address@hidden @var{name} determines the name of the entry in the fw_cfg 
> > file directory exposed to the guest.
> 
> Isn't this line overlong? (Maybe it's required by @table, I don't know.)

Seems to be - man page looks very ugly otherwise, with half the sentence
highlighted.

-- 
MST