Patchwork [resend] runqemu: Add option for custom BIOS directory

login
register
mail settings
Submitter Ricardo Neri
Date March 8, 2014, 12:30 a.m.
Message ID <1394238610-25905-1-git-send-email-ricardo.neri-calderon@linux.intel.com>
Download mbox | patch
Permalink /patch/68295/
State New
Headers show

Comments

Ricardo Neri - March 8, 2014, 12:30 a.m.
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(+)
Saul Wold - March 9, 2014, 8:09 a.m.
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!

> +           echo "Assuming $CUSTOMBIOSDIR really means $OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR"
> +           SCRIPT_QEMU_OPT="$SCRIPT_QEMU_OPT -L $OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR"
> +       else
> +           echo "Custom BIOS directory $OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR not found."
> +           exit 1;
> +    fi
> +fi
> +
>   . $INTERNAL_SCRIPT
>   exit $?
>
Ricardo Neri - March 11, 2014, 3:14 a.m.
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?

BR,
Ricardo
> 
> > +           echo "Assuming $CUSTOMBIOSDIR really means $OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR"
> > +           SCRIPT_QEMU_OPT="$SCRIPT_QEMU_OPT -L $OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR"
> > +       else
> > +           echo "Custom BIOS directory $OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR not found."
> > +           exit 1;
> > +    fi
> > +fi
> > +
> >   . $INTERNAL_SCRIPT
> >   exit $?
> >
Ricardo Neri - March 17, 2014, 11:02 p.m.
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.

Thanks and BR,
Ricardo
> 
> 
> BR,
> Ricardo
> > 
> > > +           echo "Assuming $CUSTOMBIOSDIR really means $OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR"
> > > +           SCRIPT_QEMU_OPT="$SCRIPT_QEMU_OPT -L $OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR"
> > > +       else
> > > +           echo "Custom BIOS directory $OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR not found."
> > > +           exit 1;
> > > +    fi
> > > +fi
> > > +
> > >   . $INTERNAL_SCRIPT
> > >   exit $?
> > >
> 
>
Richard Purdie - March 19, 2014, 2:17 p.m.
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
Ricardo Neri - March 20, 2014, 3:27 a.m.
On Wed, 2014-03-19 at 14:17 +0000, Richard Purdie wrote:
> 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.

Thanks for your comments! I will implement as we have discussed and
resubmit.

BR,
Ricardo
> 
> Cheers,
> 
> Richard
>

Patch

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
+           echo "Assuming $CUSTOMBIOSDIR really means $OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR"
+           SCRIPT_QEMU_OPT="$SCRIPT_QEMU_OPT -L $OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR"
+       else
+           echo "Custom BIOS directory $OECORE_NATIVE_SYSROOT/$CUSTOMBIOSDIR not found."
+           exit 1;
+    fi
+fi
+
 . $INTERNAL_SCRIPT
 exit $?