[PATCH v3 3/6] obmc-{init, update}.sh: Cope with alternate RW FS types

[Andrew Jeffery]

On Tue, 2016-02-23 at 23:36 +0000, Milton Miller II wrote:
> (me found a new mailer option)
> 
> Around 02/23/2016 05:02PM in some time zone, Joel Stanley wrote:
> >
> >On Wed, Feb 24, 2016 at 1:15 AM, Andrew Jeffery <andrew at aj.id.au>
> >wrote:
> >> Busybox's blkid is a little hamstrung, requiring some processing of
> >>the
> >> output to emulate what can be achieved with a couple of options
> >>with
> >> blkid from util-linux.
> >>
> >> Similar to findmtd(), the code for probe_fs_type() and
> >>blkid_fs_type()
> >> is duplicated between obmc-{init,update}.sh. Some consideration
> >>should
> >> be given to splitting out common functionality into well-defined
> >> sourcable scripts.
> >
> >Why not do that? :)

Because I was trying to avoid bikeshedding where and how, at the
expense of introducing some minor problems :)

> 
> Well, for one , it reduces the reliability of the script and increases 
> the time by requiring opening and finding additional files.  But we 
> could put all these variables in that file with it.
> 
> I vote for later.

Soo: what am I extracting, where am I putting it, and are we going to
bother splitting what we extract into functional pieces? Or just lump
unrelated functionality into a single file?

> 
> >
> >Put an example of the input and what you expect as the output in the
> >comment.
> >
> 
> One can run the command if they need to see it, and its common to
> ubuntu and busybox.
> 
> >> + blkid $1 \
> >> + | tr ' ' '\n' \
> >> + | awk -F= '/TYPE/ { print $2 }' \
> >> + | tr -d '"'
> >> +}
> >> +
> 
> But more awk less pipes!
> 
> -       blkid $1 \
> -               | tr ' ' '\n' \
> -               | awk -F= '/TYPE/ { print $2 }' \
> -               | tr -d '"'
> +       blkid $1 | awk ' { if (match($0, / TYPE="[^"]*"/)) {
> +                               print substr ($0, RSTART+7, RLENGTH-8);
> +                               exit 0; }
> +                       }'

IMO this alternative is less readable: I find it relatively harder to
parse the regex (with character classes and repetition operators) and
then perform substring indexing, than to follow the pipeline of
substitutions and a straight text match. Horses for courses? Less pipes
means less subprocesses but do we really care here?

> 
> busybox awk does not have the gawk match third arg; the exit 0 
> says first arg only which should be redundant, and the extra new line 
> makes it a bit more readable at the expense of being the 4th line.
> alternatively we could look at each arg in a for loop but I decided it
> just added more code than matching the leading space.
> 
> This also gets rid of those trailing backslashes (otherwise accomplished 
> by putting the pipe at the end of the line).

Fair point about the EOL pipes. The backslashes are ugly.

> 
> (I'll submit this as a patch on top; you can fold or rebase).
> 
> >> +probe_fs_type() {>> + fst=$(blkid_fs_type $1)
> >> + echo ${fst:=jffs2}
> >> +}
> >> +
> >> rwfs=$(findmtd rwfs)
> >>
> >> rwdev=/dev/mtdblock${rwfs#mtd}
> >> -rwfst=ext4
> >
> >Would it make sense to do the same as the init case, where you set
> >rwfst to $(probe_fs_type $rwdev)?
> 
> Not as useful because the type needs be able to change 
> between before and after ... so we need 2 options and one can 
> not be probed until after flashing.

You beat me to it :)


> 
> milton
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160224/40e8107a/attachment.sig>