| Submitter | Alexandru DAMIAN |
|---|---|
| Date | Dec. 17, 2012, 12:55 p.m. |
| Message ID | <1355748923-24459-1-git-send-email-alexandru.damian@intel.com> |
| Download | mbox | patch |
| Permalink | /patch/41201/ |
| State | New |
| Headers | show |
Comments
On 12/17/2012 04:55 AM, Alex DAMIAN wrote: > From: Alexandru DAMIAN <alexandru.damian@intel.com> > > Adds "debugshell" command line parameter for live/install images. > > If the init live fails to find and mount a root-fs image, > dumps to a shell after timeout so that the developer can figure > what's wrong. > > Timeout defaults to 30 seconds, but it can be changed as param > argument. > > Prior art in Ubuntu. Also, leaving a system stale isn't good form. > > Signed-off-by: Alexandru DAMIAN <alexandru.damian@intel.com> > --- > meta/recipes-core/initrdscripts/files/init-live.sh | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/meta/recipes-core/initrdscripts/files/init-live.sh b/meta/recipes-core/initrdscripts/files/init-live.sh > index c591f0d..d99a8ea 100644 > --- a/meta/recipes-core/initrdscripts/files/init-live.sh > +++ b/meta/recipes-core/initrdscripts/files/init-live.sh > @@ -45,7 +45,13 @@ read_args() { > console_params=$arg > else > console_params="$console_params $arg" > - fi > + fi ;; > + debugshell*) > + if [ -z "$optarg" ]; then > + shelltimeout=30 > + else > + shelltimeout=$optarg > + fi > esac > done > } > @@ -75,6 +81,7 @@ early_setup > read_args > > echo "Waiting for removable media..." > +C=0 > while true > do > for i in `ls /media 2>/dev/null`; do > @@ -90,6 +97,15 @@ do > if [ "$found" = "yes" ]; then > break; > fi > + # don't wait for more than $shelltimeout seconds, if it's set > + if [ -n "$shelltimeout" ]; then > + echo -n " " $(( $shelltimeout - $C )) > + if [ $C -ge $shelltimeout ]; then > + echo "..." > + fatal "Cannot find root image on media, dropping to shell " I'd recommend being explicity about what it is looking for when debugshell is used. Which devices where probed, what was the file it looked for. Save the developer from having to find this script in the repository and look it up. -- Darren > + fi > + C=$(( C + 1 )) > + fi > sleep 1 > done > >
On Mon, Dec 17, 2012 at 3:06 PM, Darren Hart <dvhart@linux.intel.com> wrote: > > > On 12/17/2012 04:55 AM, Alex DAMIAN wrote: >> From: Alexandru DAMIAN <alexandru.damian@intel.com> >> >> Adds "debugshell" command line parameter for live/install images. >> >> If the init live fails to find and mount a root-fs image, >> dumps to a shell after timeout so that the developer can figure >> what's wrong. >> >> Timeout defaults to 30 seconds, but it can be changed as param >> argument. >> >> Prior art in Ubuntu. Also, leaving a system stale isn't good form. >> >> Signed-off-by: Alexandru DAMIAN <alexandru.damian@intel.com> One possible improvement (not for this patch but long term) is to use initramfs-framework for this. It provides a modular system and has some nice featues for debug and shell handling so you can say: debug=before:udev shell=after:e2fs Just an example. Another good thing about it is it allow for less code duplication. -- Otavio Salvador O.S. Systems E-mail: otavio@ossystems.com.br http://www.ossystems.com.br Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br
On 17 December 2012 17:09, Otavio Salvador <otavio@ossystems.com.br> wrote: > One possible improvement (not for this patch but long term) is to use > initramfs-framework for this. It provides a modular system and has > some nice featues for debug and shell handling so you can say: > > debug=before:udev shell=after:e2fs > > Just an example. Another good thing about it is it allow for less code > duplication. Good point - I only noticed last week that we've duplicated udev logic in the live scripts and the initramfs-framework (that isn't used by oe-core). Clearly this is Wrong and if we're shipping a framework for a initramfs, we should be using it in our initramfs... Ross
On Mon, Dec 17, 2012 at 3:12 PM, Burton, Ross <ross.burton@intel.com> wrote: > On 17 December 2012 17:09, Otavio Salvador <otavio@ossystems.com.br> wrote: >> One possible improvement (not for this patch but long term) is to use >> initramfs-framework for this. It provides a modular system and has >> some nice featues for debug and shell handling so you can say: >> >> debug=before:udev shell=after:e2fs >> >> Just an example. Another good thing about it is it allow for less code >> duplication. > > Good point - I only noticed last week that we've duplicated udev logic > in the live scripts and the initramfs-framework (that isn't used by > oe-core). Clearly this is Wrong and if we're shipping a framework for > a initramfs, we should be using it in our initramfs... When I wrote the framework I had this on mind and we've been using the framework extensively as it is quite flexible. I do believe all initramfs scripts that are available today in oe-core would be quite simple to convert. -- Otavio Salvador O.S. Systems E-mail: otavio@ossystems.com.br http://www.ossystems.com.br Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br
IMHO, if the system has to fatal() here, something VERY wrong is going on. This isn't your general debug option, this is a last-resort emergency rescue. I suppose that if you need to debug this, the system is so problematic you need to deep-dive into it anyway. I suggest taking it as it is now, and fill in a bugzilla entry about handling it better in initramfs-framework as it should be. Alex On Mon, Dec 17, 2012 at 7:06 PM, Darren Hart <dvhart@linux.intel.com> wrote: > > > On 12/17/2012 04:55 AM, Alex DAMIAN wrote: > > From: Alexandru DAMIAN <alexandru.damian@intel.com> > > > > Adds "debugshell" command line parameter for live/install images. > > > > If the init live fails to find and mount a root-fs image, > > dumps to a shell after timeout so that the developer can figure > > what's wrong. > > > > Timeout defaults to 30 seconds, but it can be changed as param > > argument. > > > > Prior art in Ubuntu. Also, leaving a system stale isn't good form. > > > > Signed-off-by: Alexandru DAMIAN <alexandru.damian@intel.com> > > --- > > meta/recipes-core/initrdscripts/files/init-live.sh | 18 > +++++++++++++++++- > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/meta/recipes-core/initrdscripts/files/init-live.sh > b/meta/recipes-core/initrdscripts/files/init-live.sh > > index c591f0d..d99a8ea 100644 > > --- a/meta/recipes-core/initrdscripts/files/init-live.sh > > +++ b/meta/recipes-core/initrdscripts/files/init-live.sh > > @@ -45,7 +45,13 @@ read_args() { > > console_params=$arg > > else > > console_params="$console_params $arg" > > - fi > > + fi ;; > > + debugshell*) > > + if [ -z "$optarg" ]; then > > + shelltimeout=30 > > + else > > + shelltimeout=$optarg > > + fi > > esac > > done > > } > > @@ -75,6 +81,7 @@ early_setup > > read_args > > > > echo "Waiting for removable media..." > > +C=0 > > while true > > do > > for i in `ls /media 2>/dev/null`; do > > @@ -90,6 +97,15 @@ do > > if [ "$found" = "yes" ]; then > > break; > > fi > > + # don't wait for more than $shelltimeout seconds, if it's set > > + if [ -n "$shelltimeout" ]; then > > + echo -n " " $(( $shelltimeout - $C )) > > + if [ $C -ge $shelltimeout ]; then > > + echo "..." > > + fatal "Cannot find root image on media, dropping to shell " > > I'd recommend being explicity about what it is looking for when > debugshell is used. Which devices where probed, what was the file it > looked for. Save the developer from having to find this script in the > repository and look it up. > > -- > Darren > > > + fi > > + C=$(( C + 1 )) > > + fi > > sleep 1 > > done > > > > > > -- > Darren Hart > Intel Open Source Technology Center > Yocto Project - Technical Lead - Linux Kernel >
On 12/17/2012 11:21 AM, Damian, Alexandru wrote: > IMHO, if the system has to fatal() here, something VERY wrong is going on. > > This isn't your general debug option, this is a last-resort emergency > rescue. I suppose that if you need to debug this, the system is so > problematic you need to deep-dive into it anyway. > > I suggest taking it as it is now, and fill in a bugzilla entry about > handling it better in initramfs-framework as it should be. > I mentioned the below because it is something I've had to verify on several occasions. It is a fairly simple add, and you have the print statement there anyway. If you choose to skip the extra output, I suppose I can send a patch to add it next time I run into it. -- Darren > Alex > > > On Mon, Dec 17, 2012 at 7:06 PM, Darren Hart <dvhart@linux.intel.com > <mailto:dvhart@linux.intel.com>> wrote: > > > > On 12/17/2012 04:55 AM, Alex DAMIAN wrote: > > From: Alexandru DAMIAN <alexandru.damian@intel.com <mailto:alexandru.damian@intel.com>> > > > > Adds "debugshell" command line parameter for live/install images. > > > > If the init live fails to find and mount a root-fs image, > > dumps to a shell after timeout so that the developer can figure > > what's wrong. > > > > Timeout defaults to 30 seconds, but it can be changed as param > > argument. > > > > Prior art in Ubuntu. Also, leaving a system stale isn't good form. > > > > Signed-off-by: Alexandru DAMIAN <alexandru.damian@intel.com <mailto:alexandru.damian@intel.com>> > > --- > > meta/recipes-core/initrdscripts/files/init-live.sh | 18 +++++++++++++++++- > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/meta/recipes-core/initrdscripts/files/init-live.sh b/meta/recipes-core/initrdscripts/files/init-live.sh > > index c591f0d..d99a8ea 100644 > > --- a/meta/recipes-core/initrdscripts/files/init-live.sh > > +++ b/meta/recipes-core/initrdscripts/files/init-live.sh > > @@ -45,7 +45,13 @@ read_args() { > > console_params=$arg > > else > > console_params="$console_params $arg" > > - fi > > + fi ;; > > + debugshell*) > > + if [ -z "$optarg" ]; then > > + shelltimeout=30 > > + else > > + shelltimeout=$optarg > > + fi > > esac > > done > > } > > @@ -75,6 +81,7 @@ early_setup > > read_args > > > > echo "Waiting for removable media..." > > +C=0 > > while true > > do > > for i in `ls /media 2>/dev/null`; do > > @@ -90,6 +97,15 @@ do > > if [ "$found" = "yes" ]; then > > break; > > fi > > + # don't wait for more than $shelltimeout seconds, if it's set > > + if [ -n "$shelltimeout" ]; then > > + echo -n " " $(( $shelltimeout - $C )) > > + if [ $C -ge $shelltimeout ]; then > > + echo "..." > > + fatal "Cannot find root image on media, dropping to shell " > > I'd recommend being explicity about what it is looking for when > debugshell is used. Which devices where probed, what was the file it > looked for. Save the developer from having to find this script in the > repository and look it up. > > -- > Darren > > > + fi > > + C=$(( C + 1 )) > > + fi > > sleep 1 > > done > > > > > > -- > Darren Hart > Intel Open Source Technology Center > Yocto Project - Technical Lead - Linux Kernel > >
Ok, I'll add the print. And re-submit. Alex On Mon, Dec 17, 2012 at 9:50 PM, Darren Hart <dvhart@linux.intel.com> wrote: > > > On 12/17/2012 11:21 AM, Damian, Alexandru wrote: > > IMHO, if the system has to fatal() here, something VERY wrong is going > on. > > > > This isn't your general debug option, this is a last-resort emergency > > rescue. I suppose that if you need to debug this, the system is so > > problematic you need to deep-dive into it anyway. > > > > I suggest taking it as it is now, and fill in a bugzilla entry about > > handling it better in initramfs-framework as it should be. > > > > I mentioned the below because it is something I've had to verify on > several occasions. It is a fairly simple add, and you have the print > statement there anyway. If you choose to skip the extra output, I > suppose I can send a patch to add it next time I run into it. > > -- > Darren > > > Alex > > > > > > On Mon, Dec 17, 2012 at 7:06 PM, Darren Hart <dvhart@linux.intel.com > > <mailto:dvhart@linux.intel.com>> wrote: > > > > > > > > On 12/17/2012 04:55 AM, Alex DAMIAN wrote: > > > From: Alexandru DAMIAN <alexandru.damian@intel.com <mailto: > alexandru.damian@intel.com>> > > > > > > Adds "debugshell" command line parameter for live/install images. > > > > > > If the init live fails to find and mount a root-fs image, > > > dumps to a shell after timeout so that the developer can figure > > > what's wrong. > > > > > > Timeout defaults to 30 seconds, but it can be changed as param > > > argument. > > > > > > Prior art in Ubuntu. Also, leaving a system stale isn't good form. > > > > > > Signed-off-by: Alexandru DAMIAN <alexandru.damian@intel.com<mailto: > alexandru.damian@intel.com>> > > > --- > > > meta/recipes-core/initrdscripts/files/init-live.sh | 18 > +++++++++++++++++- > > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > > > diff --git a/meta/recipes-core/initrdscripts/files/init-live.sh > b/meta/recipes-core/initrdscripts/files/init-live.sh > > > index c591f0d..d99a8ea 100644 > > > --- a/meta/recipes-core/initrdscripts/files/init-live.sh > > > +++ b/meta/recipes-core/initrdscripts/files/init-live.sh > > > @@ -45,7 +45,13 @@ read_args() { > > > console_params=$arg > > > else > > > console_params="$console_params $arg" > > > - fi > > > + fi ;; > > > + debugshell*) > > > + if [ -z "$optarg" ]; then > > > + shelltimeout=30 > > > + else > > > + shelltimeout=$optarg > > > + fi > > > esac > > > done > > > } > > > @@ -75,6 +81,7 @@ early_setup > > > read_args > > > > > > echo "Waiting for removable media..." > > > +C=0 > > > while true > > > do > > > for i in `ls /media 2>/dev/null`; do > > > @@ -90,6 +97,15 @@ do > > > if [ "$found" = "yes" ]; then > > > break; > > > fi > > > + # don't wait for more than $shelltimeout seconds, if it's set > > > + if [ -n "$shelltimeout" ]; then > > > + echo -n " " $(( $shelltimeout - $C )) > > > + if [ $C -ge $shelltimeout ]; then > > > + echo "..." > > > + fatal "Cannot find root image on media, dropping to > shell " > > > > I'd recommend being explicity about what it is looking for when > > debugshell is used. Which devices where probed, what was the file it > > looked for. Save the developer from having to find this script in the > > repository and look it up. > > > > -- > > Darren > > > > > + fi > > > + C=$(( C + 1 )) > > > + fi > > > sleep 1 > > > done > > > > > > > > > > -- > > Darren Hart > > Intel Open Source Technology Center > > Yocto Project - Technical Lead - Linux Kernel > > > > > > -- > Darren Hart > Intel Open Source Technology Center > Yocto Project - Technical Lead - Linux Kernel >
Patch
diff --git a/meta/recipes-core/initrdscripts/files/init-live.sh b/meta/recipes-core/initrdscripts/files/init-live.sh index c591f0d..d99a8ea 100644 --- a/meta/recipes-core/initrdscripts/files/init-live.sh +++ b/meta/recipes-core/initrdscripts/files/init-live.sh @@ -45,7 +45,13 @@ read_args() { console_params=$arg else console_params="$console_params $arg" - fi + fi ;; + debugshell*) + if [ -z "$optarg" ]; then + shelltimeout=30 + else + shelltimeout=$optarg + fi esac done } @@ -75,6 +81,7 @@ early_setup read_args echo "Waiting for removable media..." +C=0 while true do for i in `ls /media 2>/dev/null`; do @@ -90,6 +97,15 @@ do if [ "$found" = "yes" ]; then break; fi + # don't wait for more than $shelltimeout seconds, if it's set + if [ -n "$shelltimeout" ]; then + echo -n " " $(( $shelltimeout - $C )) + if [ $C -ge $shelltimeout ]; then + echo "..." + fatal "Cannot find root image on media, dropping to shell " + fi + C=$(( C + 1 )) + fi sleep 1 done