On 5/14/19 2:18 PM, Markus Armbruster wrote: > Peter Maydell <peter.mayd...@linaro.org> writes: > >> On Mon, 13 May 2019 at 14:21, Markus Armbruster <arm...@redhat.com> wrote: >>> Perhaps I should do it just for this file while I touch it anyway. The >>> question to ask: should parse_acl_file() obey the locale for whitespace >>> recognition? >> >> I vote for "no". >> >> Q: do we document the format of the ACL file anywhere ? > > Support for it was added in commit bdef79a2994, v1.1. Just code, no > documentation. > > Grepping for qemu-bridge-helper finds just qemu-options.hx. Contains > -help output and some .texi that goes into qemu-doc and the manual page. > None of it mentions how qemu-bridge-helper is run, or the ACL file > feature, let alone what its format might be. > > I'm afraid all we have is the commit message. Which doesn't really > define the file format, it merely gives a bunch of examples. > > As far as I can tell, qemu-bridge-helper is for use with -netdev tap and > -netdev bridge. > > Both variations of -netdev call net_bridge_run_helper() to run the > helper. First argument is -netdev parameter "helper", default usually > "$prefix/libexec/qemu-bridge-helper". Second argument is parameter > "br", default "br0". > > If @helper contains space or tab, net_bridge_run_helper() guesses its a > full command, else it guesses its the name of the executable. Bad > magic. > > If it guesses name of executable, it execv()s this executable with > arguments "--use-vnet", "--fd=FD", "--br=@bridge". > > If it guesses full command, it appends "--use-vnet --fd=FD", where FD is > the helper's half of the socketpair used to connect QEMU and the helper. > It further appends "--br=@bridge", unless @helper contains "--br=". > More bad magic. > > It executes the resulting string with sh -c. Magic cherry on top. > > When the helper fails, netdev creation fails. > > The helper we ship with QEMU unconditionally tries to read > "$prefix/etc/bridge.conf". Fatal error if this file doesn't exist. > Errors in this file are fatal. Errors in files it includes are not > fatal; instead, the remainder of the erroneous file is ignored. > *Boggle* > > As far as I can tell, libvirt runs qemu-bridge-helper itself (Paolo's > commit 2d80fbb14df). Makes sense, because running QEMU with the > necessary privileges would be unwise, and so would be letting it execute > setuid helpers. Also bypasses the bad magic in QEMU's > net_bridge_run_helper(). > > qemu-bridge-helper should have a manual page, and its handling of errors > in ACL include files needs work. There's probably more; I just glanced > at it. I'm not volunteering, though. It lacks a maintainer. Should we > add it to Jason's "Network device backends"? > > -netdev's helper parameter is seriously underdocumented. Document or > deprecate? >
I understood the project policy is "deprecate until maintained or tested"... If not, we might start to think about it :)
