breaking VTY config compat -- was: catch-22 VTY error

On Tue, May 09, 2017 at 12:46:38PM +0200, Neels Hofmeyr wrote: A patch for this exists on gerrit now and sees some interesting discussion: https://gerrit.osmocom.org/3880 Maybe we can move the discussion here for easier reading / writing: (copying the comments so far from gerrit) msuraev: I fully agree, and there already was some discussion on the ML, which lead up to this patch: see thread before and after https://lists.osmocom.org/pipermail/openbsc/2017-May/010652.html ("catch-22 VTY error") I am touching this now because I think we're releasing a burst of people needing to adjust their vty config files these days (with osmo-msc/bsc separation and osmo-ggsn, the new SIGTRAN and whatnot), and it would make sense to do this indenting change at the same time. Actually I think it might be worth it to make this indenting strictness optional, so that e.g. the old osmo-nitb can still request the old implicit-go-to-parent way, while new binaries from a given version on can switch to the new strict indenting. Though we're writing a lot here now, I would welcome more discussion on the ML... hwelte: Currently the VTY only remembers a single node. It relies on the node commands to pick a child node, and on go_parent_cb() to pick the right parent for each child. i.e. there is no state about parent nodes at all. To remember which depths each parent node was on, we would need to create such state per parent node. So the effort is non-trivial. But nevertheless, I like that, because we will hardly ever step in deeper than say five child node levels and keeping the state is far from performance / memory critical. Also it would obsolete the need for the go_parent callbacks, and we could actually define the child->parent relations merely from entering a child. (One step further could even define the parent->child relations statically when defining VTY commands, instead of writing code to enter another node, opening up the possibility to validate that the parent/child structure is sound). My main question is, how much of this code do we want to change? Is there a consideration like with llist, that we don't want to deviate from upstream too much? That's a bit harder, because for writing the config we simply vty_out(). There would need to be some wrapper function prepending indent, plus some function indicating that we stepped in and out of a child node to modify the prepended indent. We still have to take care that we indicate that properly: network bts 0 arfcn 123 bts 1 From 'arfcn' up to bts 1, we still have to manually indicate that we intended to step out of the bts 0 child node. Also structures like this are valid: network bts 0 bts 1 i.e. bts 0 would intend to enter a child node, but bts 1 follows right away on the same level, i.e. steps out of it again. There isn't any way that saves us from indicating those levels manually; we can only save ourselves from varying spaces while in a specific node by accident. Not sure if it's worth the trouble / easier to just indicate the depth by manual spaces. For <thing I don't remember> I once wrote a vty_write wrapper that accepted the parent node and level of indent flexibly, so that two different binaries could hook a library vty on arbitrary node depth levels... pretty straightforward. (end of quote) ~N