From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:59017)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1aowKK-0005Od-N5
	for qemu-devel@nongnu.org; Sat, 09 Apr 2016 13:05:42 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1aowKH-0001Ie-H0
	for qemu-devel@nongnu.org; Sat, 09 Apr 2016 13:05:40 -0400
Received: from mx1.redhat.com ([209.132.183.28]:54091)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1aowKH-0001IL-A9
	for qemu-devel@nongnu.org; Sat, 09 Apr 2016 13:05:37 -0400
Date: Sat, 9 Apr 2016 20:05:32 +0300
From: "Michael S. Tsirkin" <mst@redhat.com>
Message-ID: <20160409200141-mutt-send-email-mst@redhat.com>
References: <1460102035-15972-1-git-send-email-mst@redhat.com>
	<87twjcvz7o.fsf@dusky.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87twjcvz7o.fsf@dusky.pond.sub.org>
Subject: Re: [Qemu-devel] [PATCH v3] fw_cfg: RFQDN rules, documentation
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>, "Gabriel L . Somlo" <somlo@cmu.edu>, Laszlo Ersek <lersek@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>

On Fri, Apr 08, 2016 at 06:31:55PM +0200, Markus Armbruster wrote:
> Interface and doc review only.  Sorry for the delay, I was on vacation.
> 
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > This requires that all -fw_cfg command line users use names of the form
> 
> What's "this"?  Do you mean "Require that ..."?
> 
> > opt/RFQDN/: such names are compatible with QEMU 2.4 and 2.5 as well as
> > future QEMU versions.
> 
> I'd reserve names with dots before the first '/' for users, and use
> RFQDN/ without opt/.  But I can live with opt/.
> >
> > As ability to insert fw_cfg entries in QEMU root is useful for
> > firmware development, add a special prefix: unsupported/root/ that
> > allows that, while making sure users are aware it's unsupported.
> >
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Gabriel L. Somlo <somlo@cmu.edu>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> > ---
> >
> > I put this in my tree for now. If anyone has an issue with this,
> > please speak up!
> [...]
> > diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> > index 5414140..41ce9ca 100644
> > --- a/docs/specs/fw_cfg.txt
> > +++ b/docs/specs/fw_cfg.txt
> > @@ -210,29 +210,29 @@ the following syntax:
> >  
> >      -fw_cfg [name=]<item_name>,file=<path>
> >  
> > -where <item_name> is the fw_cfg item name, and <path> is the location
> > -on the host file system of a file containing the data to be inserted.
> > -
> > -Small enough items may be provided directly as strings on the command
> > -line, using the syntax:
> > +Or
> >  
> >      -fw_cfg [name=]<item_name>,string=<string>
> >  
> > -The terminating NUL character of the content <string> will NOT be
> > -included as part of the fw_cfg item data, which is consistent with
> > -the absence of a NUL terminator for items inserted via the file option.
> > +See QEMU man page for more documentation.
> >  
> > -Both <item_name> and, if applicable, the content <string> are passed
> > -through by QEMU without any interpretation, expansion, or further
> > -processing. Any such processing (potentially performed e.g., by the shell)
> > -is outside of QEMU's responsibility; as such, using plain ASCII characters
> > -is recommended.
> > +Using item_name with plain ASCII characters only is recommended.
> >  
> 
> My first reaction was "wait a second, why are you throwing away useful
> information?"  I had to read on to realize you're *moving* it, to
> qemu-options.hx.  That's fine.  Announcing such moves in the commit
> message makes review easier.
> 
> > -NOTE: Users *SHOULD* choose item names beginning with the prefix "opt/"
> > +Users MUST choose item names beginning with the prefix "opt/RFQDN/"
> 
> Preexisting: no need to shout.
> 
> >  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.mycompany/guestagent/guestblob,file=./my_blob.bin
> 
> Neglects to spell out the obvious: RFQDN is a reverse fully qualified
> domain name which you control.
> 
> I'm not sure I agree with the change from "should" to "must".  What
> happens when a user uses a conflicting name?  Can you categorically rule
> out that specifying a conflicting name could ever be useful, e.g. for
> debugging or testing purposes?  If not, then "must" is wrong.
> 
> > -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().
> 
> Here, "must not" is appropriate.
> 
> > +
> > +For historical reasons "opt/ovmf/" is reserved for use with the OVMF firmware.
> 
> Comma after reasons.
> 
> Here's how I'd skin this cat:
> 
>     Names beginning with "opt/" are reserved for users.  QEMU will never
>     create entries with such names unless explicitly ordered by the
>     user.
> 
>     To avoid clashes among different users, it is strongly recommended
>     that you use names beginning with opt/RFQDN/, where RFQDN is a
>     reverse fully qualified domain name you control.  For instance, if
>     SeaBIOS wanted to define additional names, prefix "opt/org.seabios/"
>     would be appropriate.
> 
>     For historical reasons, "opt/ovmf/" is reserved for OVMF firmware.
> 

Markus, I don't know what to do with your comments below.
I *really* do not want to reopen this can of worms after
I am finally getting some acks.

Can you or can you not live with the scheme proposed with this patch?

-- 
MST