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

[Markus Armbruster <armbru@redhat.com>]

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