Re: [PATCH][resend] runqemu: Add option for custom BIOS directory

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Mon, 2014-03-17 at 16:02 -0700, Ricardo Neri wrote:
> On Mon, 2014-03-10 at 20:14 -0700, Ricardo Neri wrote:
> > On Sun, 2014-03-09 at 00:09 -0800, Saul Wold wrote:
> > > On 03/07/2014 04:30 PM, Ricardo Neri wrote:
> > > > Add support to specify a directory for custom BIOS, VGA BIOS and
> > > > keymaps as supported by qemu (-L option). Even though this can be
> > > > done through qemuparams, having this option provides better user
> > > > experience by not having to specify a long and cluttered path along
> > > > other qemuparams that the user might want to specify.
> > > >
> > > > This new options assumes that the path provided is relative to
> > > > OECORE_NATIVE_SYSROOT and will check whether it exists before proceeding.
> > > >
> > > > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> > > > ---
> > > >   scripts/runqemu | 14 ++++++++++++++
> > > >   1 file changed, 14 insertions(+)
> > > >
> > > > diff --git a/scripts/runqemu b/scripts/runqemu
> > > > index 619ffb6..9eff90d 100755
> > > > --- a/scripts/runqemu
> > > > +++ b/scripts/runqemu
> > > > @@ -149,6 +149,9 @@ while true; do
> > > >               SCRIPT_KERNEL_OPT="$SCRIPT_KERNEL_OPT console=ttyS0"
> > > >               SERIALSTDIO="1"
> > > >               ;;
> > > > +	"biosdir="*)
> > > > +            CUSTOMBIOSDIR="${arg##biosdir=}"
> > > > +	    ;;
> > > >           "qemuparams="*)
> > > >               SCRIPT_QEMU_EXTRA_OPT="${arg##qemuparams=}"
> > > >
> > > > @@ -489,5 +492,16 @@ if [ ! -f "$INTERNAL_SCRIPT" -o ! -r "$INTERNAL_SCRIPT" ]; then
> > > >   INTERNAL_SCRIPT=`which runqemu-internal`
> > > >   fi
> > > >
> > > > +# Specify directory for BIOS, VGA BIOS and keymaps
> > > > +if [ ! -z "$CUSTOMBIOSDIR" ]; then
> > > > +    if [ -d "$OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR" ]; then
> > > I don't think we should be assuming anything here, if the direcotry is 
> > > passed then it should be given that it is an accurate path to the 
> > > directory that contains the customer bios, don't make people second gess 
> > > that it needs to be in the native sysroot.
> > > 
> > > Sau!
> > 
> > Thanks for your comments Saul! Are you saying that providing paths to
> > items outside the build environment is a supported use case? That is the
> > reason I made it relative to OECORE_NATIVE_SYSROOT. If absolute paths to
> > anywhere can be provided, I could first check whether the provided
> > absolute path exist and, if not, assume that it is relative to
> > OECORE_NATIVE_SYSROOT. Does that make sense to you?
> 
> So what do you think about this? Besides, I would like to comment that
> the idea of making the path relative to OECORE_NATIVE_SYSROOT is that
> the user does not have to write a very long and cluttered path in case
> his/her BIOS directory is inside the build environment. The whole idea
> is to make the script more usable.

I think what you suggested makes sense. Treat relative paths as being in
the sysroot and absolute paths as being external files. You could test
whether the relative path exists first, then assume it must be the
sysroot if it doesn't exist I guess.

Cheers,

Richard