[Qemu-devel] [PATCH v3] fw_cfg: RFQDN rules, documentation

[Qemu-devel] [PATCH v3] fw_cfg: RFQDN rules, documentation

[Michael S. Tsirkin]

This requires that all -fw_cfg command line users use names of the form
opt/RFQDN/: such names are compatible with QEMU 2.4 and 2.5 as well as
future QEMU versions.

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!

 vl.c                  | 44 ++++++++++++++++++++++++++++++++++++++++----
 docs/specs/fw_cfg.txt | 34 +++++++++++++++++-----------------
 qemu-options.hx       | 38 +++++++++++++++++++++++++++++++++-----
 3 files changed, 90 insertions(+), 26 deletions(-)

diff --git a/vl.c b/vl.c
index 9df534f..a56a0e6 100644
--- a/vl.c
+++ b/vl.c
@@ -2296,8 +2296,11 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
 {
     gchar *buf;
     size_t size;
-    const char *name, *file, *str;
+    const char *name, *file, *str, *slash, *dot;
     FWCfgState *fw_cfg = (FWCfgState *) opaque;
+    static const char qemu_prefix[] = "opt/org.qemu";
+    static const char ovmf_prefix[] = "opt/ovmf/";
+    static const char unsupported_root_prefix[] = "unsupported/root/";
 
     if (fw_cfg == NULL) {
         error_report("fw_cfg device not available");
@@ -2320,9 +2323,42 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
         error_report("name too long (max. %d char)", FW_CFG_MAX_FILE_PATH - 1);
         return -1;
     }
-    if (strncmp(name, "opt/", 4) != 0) {
-        error_report("warning: externally provided fw_cfg item names "
-                     "should be prefixed with \"opt/\"");
+    /*
+     * Look for and strip unsupported_root_prefix, which is useful for firmware
+     * development, but warn users.
+     */
+    if (!strncmp(name, unsupported_root_prefix,
+                 sizeof(unsupported_root_prefix) - 1)) {
+        error_report("warning: removing prefix \"%s\". "
+                     "Guest or QEMU may crash. "
+                     "Names must be prefixed with \"opt/RFQDN/\"",
+                     unsupported_root_prefix);
+        name += sizeof(unsupported_root_prefix) - 1;
+        if (!nonempty_str(name)) {
+            error_report("invalid argument(s)");
+            return -1;
+        }
+    } else if (!strncmp(name, ovmf_prefix, sizeof(ovmf_prefix) - 1)) {
+        /* Allow the prefix used historically with ovmf. */
+    } else {
+        /*
+         * Don't attempt to validate a valid RFQDN in name, as that's not easy:
+         * we do validate that it includes '.' .
+         */
+        if (strncmp(name, "opt/", 4) ||
+            !(dot = strchr(name + 4, '.')) ||
+            !(slash = strchr(name + 4, '/')) ||
+            dot > slash) {
+            error_report("error: externally provided fw_cfg item names "
+                         "must be prefixed with \"opt/RFQDN/\"");
+            return -1;
+        }
+        if (!strncmp(name, qemu_prefix, sizeof(qemu_prefix) - 1)) {
+            error_report("error: externally provided fw_cfg item names "
+                         "must not use the reserved prefix \"%s\"",
+                         qemu_prefix);
+            return -1;
+        }
     }
     if (nonempty_str(str)) {
         size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
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.
 
-NOTE: Users *SHOULD* choose item names beginning with the prefix "opt/"
+Users MUST choose item names beginning with the prefix "opt/RFQDN/"
 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
 
-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.
+
+Any use of the prefix "opt/org.qemu" is reserved for future use.
diff --git a/qemu-options.hx b/qemu-options.hx
index 587de8f..2109a4f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2864,18 +2864,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 [name=]@var{name},file=@var{file}
 @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 [name=]@var{name},string=@var{str}
-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:
+@table @option
+@item @var{name} determines the name of the entry in the fw_cfg file directory exposed to the guest.
+
+@var{name} must be in the format opt/RFQDN/<item_name>.
+
+Any processing of @var{name} values (potentially performed e.g., by the shell)
+is outside of QEMU's responsibility; as such, using plain ASCII characters is
+recommended.
+@end table
+
+Example:
+@example
+    -fw_cfg name=opt/com.mycompany/guestagent/guestblob,file=./my_blob.bin
+@end example
+
+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.
+
 ETEXI
 
 DEF("serial", HAS_ARG, QEMU_OPTION_serial, \
-- 
MST

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

[Markus Armbruster]

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.

> +
> +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.

What exactly does making users say "unsupported/root/FOO" instead of
"FOO" buy us?

We can warn on "-fw_cfg FOO" just as easily as on "-fw_cfg
/unsupported/root/FOO".

>                                                            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.

We should tell users that messing with stuff outside /opt is
unsupported.  But telling them that only developers get to do
unsupported things is futile.

My take:

    Use of names not beginning with "opt/" is potentially dangerous and
    entirely unsupported.  QEMU will warn if you try.

> +
> +Any use of the prefix "opt/org.qemu" is reserved for future use.
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 587de8f..2109a4f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2864,18 +2864,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"

Not a native speaker, but I'd say "contents" rather than "content".
More of the same below.

>      "-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 [name=]@var{name},file=@var{file}
>  @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 [name=]@var{name},string=@var{str}
> -Add named fw_cfg entry from string.
> +Add named fw_cfg entry with content from string @var{str}, up to the first NUL character.

Isn't "up to first NUL" self-evident for strings, at least in the
context of the QEMU command line?

> +
> +The terminating NUL character of the content @var{str} will NOT be

of @var{str}

> +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

To insert contents with embedded NUL characters, ...

> +the @var{file} option.
> +
> +Both the name and the content are passed by QEMU through to the guest, where:
> +@table @option
> +@item @var{name} determines the name of the entry in the fw_cfg file directory exposed to the guest.
> +
> +@var{name} must be in the format opt/RFQDN/<item_name>.
> +
> +Any processing of @var{name} values (potentially performed e.g., by the shell)
> +is outside of QEMU's responsibility; as such, using plain ASCII characters is
> +recommended.
> +@end table

Sure your markup as table of one element is appropriate?  The resulting
qemu.1 looks bad.  Try "man ./qemu.1" to see.

I understand you're merely moving stuff, but here goes anyway: "any
processing of @var{name} values (potentially performed e.g., by the
shell) is outside of QEMU's responsibility" is confusing, and once you
understand it, trivial.  Shell meta-characters getting interpreted by
the shell isn't news, and not specific to -fw_cfg...

> +
> +Example:
> +@example
> +    -fw_cfg name=opt/com.mycompany/guestagent/guestblob,file=./my_blob.bin
> +@end example
> +
> +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.
> +
>  ETEXI
>  
>  DEF("serial", HAS_ARG, QEMU_OPTION_serial, \

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

[Michael S. Tsirkin]

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

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> 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?

Cutting the comments makes discussing them kind of hard.  My best guess
is you're referring to the part where I challenge mapping
/unsupported/root/FOO to FOO.  I find that baroque.  I can accept
baroque when I see a reason.  That's why I asked for it.  "I got ACKs
for it" is not a particular satisfying reason, but I figure you got
another one.  Do tell :)

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

[Michael S. Tsirkin]

On Mon, Apr 11, 2016 at 01:04:14PM +0200, Markus Armbruster wrote:
> My best guess
> is you're referring to the part where I challenge mapping
> /unsupported/root/FOO to FOO.  I find that baroque.  I can accept
> baroque when I see a reason.  That's why I asked for it.  "I got ACKs
> for it" is not a particular satisfying reason, but I figure you got
> another one.  Do tell :)

The issue I am trying to address is that users can put stuff
outside "opt/" without realizing they are violating any rules.
I do not think printing a warning is sufficient for this:
it is masked by tools too often.
Thus, anything outside "opt/" must be forbidden and cause a failure.

Gerd and others asked for a way to bypass this "for development"
which presumably means they way a quick way to develop guest
code interacting with QEMU without changing qemu proper,
with the intent to merge support into QEMU a bit later.

We could add a special flag for this instead but the mapping
is a couple of lines of code, so seems easier,
the point is to have "unsupported" there.

-- 
MST

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Apr 11, 2016 at 01:04:14PM +0200, Markus Armbruster wrote:
>> My best guess
>> is you're referring to the part where I challenge mapping
>> /unsupported/root/FOO to FOO.  I find that baroque.  I can accept
>> baroque when I see a reason.  That's why I asked for it.  "I got ACKs
>> for it" is not a particular satisfying reason, but I figure you got
>> another one.  Do tell :)
>
> The issue I am trying to address is that users can put stuff
> outside "opt/" without realizing they are violating any rules.
> I do not think printing a warning is sufficient for this:
> it is masked by tools too often.
> Thus, anything outside "opt/" must be forbidden and cause a failure.

I don't buy your "users who don't read warnings must be protected from
their own foolishness and/or the foolishness of their tools" logic.
Specifically, I don't buy the "must".  If we can protect them without
complicating or breaking stuff, sure, why not.  But not at all costs.

Before your patch, -fw_cfg's name parameter is the FW_CFG filename.

Your patch replaces this by a rather baroque mapping:

     unsupported/root/FOO -> FOO
     opt/FOO              -> opt/FOO
     anything else is an error

I object to complicating the user interface just to create some way for
the user to say "I mean it, now go out of my way already" to be
permitted to do things that might conceivably break some day.  Even more
so when they can be expected to break cleanly.

Here's what happens when a -fw_cfg collides with QEMU's own usage:

    $ upstream-qemu -nodefaults -S -display none -fw_cfg etc/boot-fail-wait,string=foo
    upstream-qemu: -fw_cfg etc/boot-fail-wait,string=foo: warning: externally provided fw_cfg item names should be prefixed with "opt/"
    upstream-qemu: -fw_cfg etc/boot-fail-wait,string=foo: duplicate fw_cfg file name: etc/boot-fail-wait
    [Exit 1 ]

I have a hard time coming up with realistic unclean breakage.  Here's my
best one: somebody picks an FW_CFG filename F unwisely, bakes it into
guests of kind A, and uses it with -fw_cfg.  Does nothing for guests of
other kinds, but let's assume the user gives the -fw_cfg anyway.  Then a
new QEMU version comes out that uses the same filename for some other
purpose, and guests of kind B use it.  Now the existing use -fw_cfg
F,... with an old QEMU and a new guest of kind B doesn't do nothing
anymore.

Furthermore, -fw_cfg was added in 2.4.0, so this looks like an
incompatible change.  We don't break backward compatibility just because
we fear some users can't be bothered to read warnings.  We require hard
evidence of somebody suffering to a degree that clearly exceeds the
suffering caused by breaking prior usage through incompatible change.

> Gerd and others asked for a way to bypass this "for development"
> which presumably means they way a quick way to develop guest
> code interacting with QEMU without changing qemu proper,
> with the intent to merge support into QEMU a bit later.
>
> We could add a special flag for this instead but the mapping
> is a couple of lines of code, so seems easier,
> the point is to have "unsupported" there.

Honestly, I consider -fw_cfg misuse pretty much a non-issue.  Quoting
myself:

    We should definitely try not to trap the user.  Obvious usage should
    be as safe as we can make it.  Risky usage should be marked in the
    docs and/or warn on use.

    However, we should not try to stop our users from doing stupid
    things, as that would also stop them from doing clever things.

-fw_cfg is a "Debug/Expert" option.  There is no obvious usage.

Protecting users from themselves 100% is a fool's errand.  You're
welcome to try (I actually appreciate it), but it's insufficient
justification for backward incompatibility and for user interface
baroqueness.

To make forward progress, I recommend to split this patch into an
uncontroversial and a controversial part.  The uncontroversial part are
the RFQDN rules.

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

[Michael S. Tsirkin]

On Wed, Apr 13, 2016 at 10:59:35AM +0200, Markus Armbruster wrote:
> I have a hard time coming up with realistic unclean breakage.

The issue is that Linux is now exposing fw cfg to userspace.
So it's use is about to expand significantly.

This is what I am trying to prevent:
- in 2016, users build a guest using a path XXX outside opt.
  there's a warning on host, but it is not noticed.
- in 2020, qemu starts using path XXX for internal purposes.
- using guest from 2016 now breaks uncleanly on this new qemu
  since guest thinks it's talking to the external tool.

-- 
MST

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

[Michael S. Tsirkin]

On Wed, Apr 13, 2016 at 10:59:35AM +0200, Markus Armbruster wrote:
> If we can protect them without
> complicating or breaking stuff, sure, why not.  But not at all costs.

The stuff we break is precisely the stuff our warnings
say might break at any time. So since you believe users
might be relied on not to ignore warnings, it should be ok ...

As for complicating things - about 5 lines of code are spent
on the unsupported/root/ hack. It *is* a hack but
contained enough not to worry me too much ...

-- 
MST

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, Apr 13, 2016 at 10:59:35AM +0200, Markus Armbruster wrote:
>> I have a hard time coming up with realistic unclean breakage.
>
> The issue is that Linux is now exposing fw cfg to userspace.
> So it's use is about to expand significantly.
>
> This is what I am trying to prevent:
> - in 2016, users build a guest using a path XXX outside opt.
>   there's a warning on host, but it is not noticed.

Amend:

    The guest treats path XXX as optional.

> - in 2020, qemu starts using path XXX for internal purposes.
> - using guest from 2016 now breaks uncleanly on this new qemu

Amend:

    when we're not specifying the optional path XXX with -fw_cfg.

>   since guest thinks it's talking to the external tool.

Okay, that's a much more plausible scenario.  The question remains
whether preventing it justifies the compat break and the additional
interface complexity.

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, Apr 13, 2016 at 10:59:35AM +0200, Markus Armbruster wrote:
>> If we can protect them without
>> complicating or breaking stuff, sure, why not.  But not at all costs.
>
> The stuff we break is precisely the stuff our warnings
> say might break at any time. So since you believe users
> might be relied on not to ignore warnings, it should be ok ...
>
> As for complicating things - about 5 lines of code are spent
> on the unsupported/root/ hack. It *is* a hack but
> contained enough not to worry me too much ...

I'm not worried about the implementation complexity at all.  It's the
user interface complexity.  After this patch, we have a non-trivial
mapping from -fw_cfg name to FW_CFG filename to explain.  Whereas now,
-fw_cfg name *is* the FW_CFG path.

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

[Michael S. Tsirkin]

On Wed, Apr 13, 2016 at 06:19:55PM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, Apr 13, 2016 at 10:59:35AM +0200, Markus Armbruster wrote:
> >> If we can protect them without
> >> complicating or breaking stuff, sure, why not.  But not at all costs.
> >
> > The stuff we break is precisely the stuff our warnings
> > say might break at any time. So since you believe users
> > might be relied on not to ignore warnings, it should be ok ...
> >
> > As for complicating things - about 5 lines of code are spent
> > on the unsupported/root/ hack. It *is* a hack but
> > contained enough not to worry me too much ...
> 
> I'm not worried about the implementation complexity at all.  It's the
> user interface complexity.  After this patch, we have a non-trivial
> mapping from -fw_cfg name to FW_CFG filename to explain.  Whereas now,
> -fw_cfg name *is* the FW_CFG path.

Only for people who ignore the rules. Most people have a trivial
mapping.

-- 
MST

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

[Michael S. Tsirkin]

On Wed, Apr 13, 2016 at 06:17:29PM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, Apr 13, 2016 at 10:59:35AM +0200, Markus Armbruster wrote:
> >> I have a hard time coming up with realistic unclean breakage.
> >
> > The issue is that Linux is now exposing fw cfg to userspace.
> > So it's use is about to expand significantly.
> >
> > This is what I am trying to prevent:
> > - in 2016, users build a guest using a path XXX outside opt.
> >   there's a warning on host, but it is not noticed.
> 
> Amend:
> 
>     The guest treats path XXX as optional.
> 
> > - in 2020, qemu starts using path XXX for internal purposes.
> > - using guest from 2016 now breaks uncleanly on this new qemu
> 
> Amend:
> 
>     when we're not specifying the optional path XXX with -fw_cfg.
> 
> >   since guest thinks it's talking to the external tool.
> 
> Okay, that's a much more plausible scenario.  The question remains
> whether preventing it justifies the compat break and the additional
> interface complexity.

there is no break as long as people follow the rules.

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, Apr 13, 2016 at 06:17:29PM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Wed, Apr 13, 2016 at 10:59:35AM +0200, Markus Armbruster wrote:
>> >> I have a hard time coming up with realistic unclean breakage.
>> >
>> > The issue is that Linux is now exposing fw cfg to userspace.
>> > So it's use is about to expand significantly.
>> >
>> > This is what I am trying to prevent:
>> > - in 2016, users build a guest using a path XXX outside opt.
>> >   there's a warning on host, but it is not noticed.
>> 
>> Amend:
>> 
>>     The guest treats path XXX as optional.
>> 
>> > - in 2020, qemu starts using path XXX for internal purposes.
>> > - using guest from 2016 now breaks uncleanly on this new qemu
>> 
>> Amend:
>> 
>>     when we're not specifying the optional path XXX with -fw_cfg.
>> 
>> >   since guest thinks it's talking to the external tool.
>> 
>> Okay, that's a much more plausible scenario.  The question remains
>> whether preventing it justifies the compat break and the additional
>> interface complexity.
>
> there is no break as long as people follow the rules.

-fw_cfg exists since 2.4.  You can't slap rules onto it in 2.6, and
immediately claim compatibility matters only for usage following these
rules.

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, Apr 13, 2016 at 06:19:55PM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Wed, Apr 13, 2016 at 10:59:35AM +0200, Markus Armbruster wrote:
>> >> If we can protect them without
>> >> complicating or breaking stuff, sure, why not.  But not at all costs.
>> >
>> > The stuff we break is precisely the stuff our warnings
>> > say might break at any time. So since you believe users
>> > might be relied on not to ignore warnings, it should be ok ...
>> >
>> > As for complicating things - about 5 lines of code are spent
>> > on the unsupported/root/ hack. It *is* a hack but
>> > contained enough not to worry me too much ...
>> 
>> I'm not worried about the implementation complexity at all.  It's the
>> user interface complexity.  After this patch, we have a non-trivial
>> mapping from -fw_cfg name to FW_CFG filename to explain.  Whereas now,
>> -fw_cfg name *is* the FW_CFG path.
>
> Only for people who ignore the rules. Most people have a trivial
> mapping.

Even when most usage falls into the simple cases, you still have to
explain all cases.

An interface's complexity is a property of the interface, not of its
usage.

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

[Michael S. Tsirkin]

On Thu, Apr 14, 2016 at 09:36:28AM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, Apr 13, 2016 at 06:17:29PM +0200, Markus Armbruster wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > On Wed, Apr 13, 2016 at 10:59:35AM +0200, Markus Armbruster wrote:
> >> >> I have a hard time coming up with realistic unclean breakage.
> >> >
> >> > The issue is that Linux is now exposing fw cfg to userspace.
> >> > So it's use is about to expand significantly.
> >> >
> >> > This is what I am trying to prevent:
> >> > - in 2016, users build a guest using a path XXX outside opt.
> >> >   there's a warning on host, but it is not noticed.
> >> 
> >> Amend:
> >> 
> >>     The guest treats path XXX as optional.
> >> 
> >> > - in 2020, qemu starts using path XXX for internal purposes.
> >> > - using guest from 2016 now breaks uncleanly on this new qemu
> >> 
> >> Amend:
> >> 
> >>     when we're not specifying the optional path XXX with -fw_cfg.
> >> 
> >> >   since guest thinks it's talking to the external tool.
> >> 
> >> Okay, that's a much more plausible scenario.  The question remains
> >> whether preventing it justifies the compat break and the additional
> >> interface complexity.
> >
> > there is no break as long as people follow the rules.
> 
> -fw_cfg exists since 2.4.  You can't slap rules onto it in 2.6, and
> immediately claim compatibility matters only for usage following these
> rules.

The rule about "opt/" was always there, right? So we can at least
start enforcing that.

-- 
MST

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

[Michael S. Tsirkin]

On Thu, Apr 14, 2016 at 09:39:07AM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, Apr 13, 2016 at 06:19:55PM +0200, Markus Armbruster wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > On Wed, Apr 13, 2016 at 10:59:35AM +0200, Markus Armbruster wrote:
> >> >> If we can protect them without
> >> >> complicating or breaking stuff, sure, why not.  But not at all costs.
> >> >
> >> > The stuff we break is precisely the stuff our warnings
> >> > say might break at any time. So since you believe users
> >> > might be relied on not to ignore warnings, it should be ok ...
> >> >
> >> > As for complicating things - about 5 lines of code are spent
> >> > on the unsupported/root/ hack. It *is* a hack but
> >> > contained enough not to worry me too much ...
> >> 
> >> I'm not worried about the implementation complexity at all.  It's the
> >> user interface complexity.  After this patch, we have a non-trivial
> >> mapping from -fw_cfg name to FW_CFG filename to explain.  Whereas now,
> >> -fw_cfg name *is* the FW_CFG path.
> >
> > Only for people who ignore the rules. Most people have a trivial
> > mapping.
> 
> Even when most usage falls into the simple cases, you still have to
> explain all cases.
> 
> An interface's complexity is a property of the interface, not of its
> usage.

I think I disagree. Make simple things simple and complex things
possible is the rule for a good interface.

-- 
MST

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Apr 14, 2016 at 09:36:28AM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Wed, Apr 13, 2016 at 06:17:29PM +0200, Markus Armbruster wrote:
>> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> >> 
>> >> > On Wed, Apr 13, 2016 at 10:59:35AM +0200, Markus Armbruster wrote:
>> >> >> I have a hard time coming up with realistic unclean breakage.
>> >> >
>> >> > The issue is that Linux is now exposing fw cfg to userspace.
>> >> > So it's use is about to expand significantly.
>> >> >
>> >> > This is what I am trying to prevent:
>> >> > - in 2016, users build a guest using a path XXX outside opt.
>> >> >   there's a warning on host, but it is not noticed.
>> >> 
>> >> Amend:
>> >> 
>> >>     The guest treats path XXX as optional.
>> >> 
>> >> > - in 2020, qemu starts using path XXX for internal purposes.
>> >> > - using guest from 2016 now breaks uncleanly on this new qemu
>> >> 
>> >> Amend:
>> >> 
>> >>     when we're not specifying the optional path XXX with -fw_cfg.
>> >> 
>> >> >   since guest thinks it's talking to the external tool.
>> >> 
>> >> Okay, that's a much more plausible scenario.  The question remains
>> >> whether preventing it justifies the compat break and the additional
>> >> interface complexity.
>> >
>> > there is no break as long as people follow the rules.
>> 
>> -fw_cfg exists since 2.4.  You can't slap rules onto it in 2.6, and
>> immediately claim compatibility matters only for usage following these
>> rules.
>
> The rule about "opt/" was always there, right? So we can at least
> start enforcing that.

This is what's in 2.4:

    NOTE: Users *SHOULD* choose item names beginning with the prefix "opt/"
    when using the "-fw_cfg" command line option, to avoid conflicting with
    item names used internally by QEMU. For instance:

        -fw_cfg name=opt/my_item_name,file=./my_blob.bin

Interpreting the "should" as "or else we'll break your usage without
prior notice" is a bit of a stretch.

As a matter of principle, I'm unwilling to renege on backward
compatibility that way without a convincing reason.  I find your attempt
at protecting users of an arcane -fw_cfg from (some of) their own
foolishness insufficiently convincing.

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Apr 14, 2016 at 09:39:07AM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Wed, Apr 13, 2016 at 06:19:55PM +0200, Markus Armbruster wrote:
>> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> >> 
>> >> > On Wed, Apr 13, 2016 at 10:59:35AM +0200, Markus Armbruster wrote:
>> >> >> If we can protect them without
>> >> >> complicating or breaking stuff, sure, why not.  But not at all costs.
>> >> >
>> >> > The stuff we break is precisely the stuff our warnings
>> >> > say might break at any time. So since you believe users
>> >> > might be relied on not to ignore warnings, it should be ok ...
>> >> >
>> >> > As for complicating things - about 5 lines of code are spent
>> >> > on the unsupported/root/ hack. It *is* a hack but
>> >> > contained enough not to worry me too much ...
>> >> 
>> >> I'm not worried about the implementation complexity at all.  It's the
>> >> user interface complexity.  After this patch, we have a non-trivial
>> >> mapping from -fw_cfg name to FW_CFG filename to explain.  Whereas now,
>> >> -fw_cfg name *is* the FW_CFG path.
>> >
>> > Only for people who ignore the rules. Most people have a trivial
>> > mapping.
>> 
>> Even when most usage falls into the simple cases, you still have to
>> explain all cases.
>> 
>> An interface's complexity is a property of the interface, not of its
>> usage.
>
> I think I disagree. Make simple things simple and complex things
> possible is the rule for a good interface.

No, you're not making a complex thing possible, you're making a simple
(if unadvisable) thing complex!  That's an entirely different cup of
tea.

The design rule that actually applies is that an interface's obvious
uses should be correct uses.

In my opinion, there is no truly obvious use of -fw_cfg.

The closest we get to "obvious" is "use this fixed name to do that",
where somebody else picked the fixed name.  That somebody else should've
picked a name starting with /opt/RFQDN/.

Your proposal proactively breaks some of those third party interfaces
between host and guest (the ones using names outside /opt) to avoid
breakage in hypothetical scenarios in the future.  Users have to prefix
/unsupported/root/ to keep them working.  Could perhaps be viewed as
making an "obvious" but incorrect use less obvious.

Your proposal provides does nothing for badly chosen names within /opt.

The next use case to consider is a user picking a new name for a new
interface between host and guest.  I find the idea that such a user
won't notice warnings farfetched.  But let's assume such users exist.
We're talking about someone who strays out of /opt out of willfulness or
ignorance *and* can't be bothered to read warnings, and because of that
we reject his usage outright to drive home the point.

What good is that going to do?  What are the chances this will make such
a user actually read the docs, pick an appropriate RFQDN and stick to
/opt/RFQDN/?  I think they're remote.  Multipy it by the probability of
this case even happening, and the result is even more remote.

The final use case to consider is development, testing and debugging.
By their nature, these use arbitrary names, and future-proofing is
entirely irrelevant.  In particlar, use of names outside /opt/RFQDN/ is
not wrong there.  It's only wrong when you bake them into a interface
that outlives your development, testing or debugging.  So this use case
gets more complex without any benefit.  Complicating development,
testing or debugging can be tolerable when there's a real benefit to
users.

In short, you're making one special case of one misuse of the FW_CFG
name space more complex in an attempt to make this misuse less obvious
in the interface, while doing nothing for other kinds of misuse.

The question to ask is whether it's worth it.

In my opinion, it isn't worth it, because the additional interface
complexity plus the backward compatibility break, minor as they are,
still dwarf the benefits several times over.

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

[Michael S. Tsirkin]

On Thu, Apr 14, 2016 at 01:29:20PM +0200, Markus Armbruster wrote:
> What good is that going to do?

Enforce a sane policy. It's too easy to misconfigure qemu as it is.
We don't need more knobs that can break guests.

-- 
MST

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

[Markus Armbruster]

[Context restored]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Apr 14, 2016 at 01:29:20PM +0200, Markus Armbruster wrote:
>> The next use case to consider is a user picking a new name for a new
>> interface between host and guest.  I find the idea that such a user
>> won't notice warnings farfetched.  But let's assume such users exist.
>> We're talking about someone who strays out of /opt out of willfulness or
>> ignorance *and* can't be bothered to read warnings, and because of that
>> we reject his usage outright to drive home the point.
>> 
>> What good is that going to do?  What are the chances this will make such
>> a user actually read the docs, pick an appropriate RFQDN and stick to
>> /opt/RFQDN/?  I think they're remote.  Multipy it by the probability of
>> this case even happening, and the result is even more remote.
>
> Enforce a sane policy. It's too easy to misconfigure qemu as it is.
> We don't need more knobs that can break guests.

While that's a valid point, it's not an answer to the question I asked.

My point is that anyone violating the rules despite the warning before
your patch is highly likely to violate them just as badly afterwards.
Therefore, the probability of your patch being of any use there is
1 - highly likely.

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

[Michael S. Tsirkin]

On Fri, Apr 15, 2016 at 05:39:33PM +0200, Markus Armbruster wrote:
> [Context restored]
> 
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Thu, Apr 14, 2016 at 01:29:20PM +0200, Markus Armbruster wrote:
> >> The next use case to consider is a user picking a new name for a new
> >> interface between host and guest.  I find the idea that such a user
> >> won't notice warnings farfetched.  But let's assume such users exist.
> >> We're talking about someone who strays out of /opt out of willfulness or
> >> ignorance *and* can't be bothered to read warnings, and because of that
> >> we reject his usage outright to drive home the point.
> >> 
> >> What good is that going to do?  What are the chances this will make such
> >> a user actually read the docs, pick an appropriate RFQDN and stick to
> >> /opt/RFQDN/?  I think they're remote.  Multipy it by the probability of
> >> this case even happening, and the result is even more remote.
> >
> > Enforce a sane policy. It's too easy to misconfigure qemu as it is.
> > We don't need more knobs that can break guests.
> 
> While that's a valid point, it's not an answer to the question I asked.
> 
> My point is that anyone violating the rules despite the warning before
> your patch is highly likely to violate them just as badly afterwards.
> Therefore, the probability of your patch being of any use there is
> 1 - highly likely.

My point is that's not so because not all users see our warnings. They
will notice that they are passing "unsupported" in the path.

-- 
MST

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

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

[...]
> To make forward progress, I recommend to split this patch into an
> uncontroversial and a controversial part.  The uncontroversial part are
> the RFQDN rules.

I offered Michael to do that for him, and he accepted.  Patch is on its
way.