Bugzilla Bug 212794

I am still hitting "mount: Mounting /dev/loop0 on /newroot/mnt/livecd failed:
Invalid argument", with squashfs-tools-3.2_p2 (tried 3.3 as well),
gentoo-sources-2.6.24 and genkernel-3.4.10_pre4. And I kept wondering why it
panics instead of throwing the shell. And then, I found a bug with
test_success:

test_success() {
        error_string=$1
        error_string="${error_string:-run command}"
        # If last command failed send error message and fall back to a shell
        if [ "$?" != '0' ]
        then
                bad_msg 'Failed to $1; failing back to the shell...'
                run_shell
        fi
}

That $? check will always succeed because error_string="${error_string:-run
command}" always succeeds. You need to store $? before hand, as soon as you
enter the function.

So, after I fixed the above, I was thrown into a shell. And I tried to mount my
squashfs manually. Indeed, it failed with same error. Then, I noticed that
/dev/sdd was mounted on /newroot/mnt/cdrom, instead of /dev/sdd1, the first
partition on my USB drive. A pitfall of using a liveCD to boot a liveUSB, I
guess. If I mount /dev/sdd1 on /newroot/mnt/cdrom from the shell and then try
to mount my squashfs, it succeeds in mounting.

Although it succeeds in mounting /dev/sdd (files are actually on first
partition) on /newroot/mnt/cdrom, it can't read all the files properly.

Can the media detection code be modified a little bit to allow for this anamoly
somehow? e.g. ignore the device /dev/sd[a-z] if it contains partitions.

BTW, I decided to open a new bug because I couldn't really open
http://bugs.gentoo.org/show_bug.cgi?id=208477 and it was a different problem
now.

Created an attachment (id=145626) [details]
dumb fix for "don't mount disk if it has at least one partition"

ok, the patch booted me into liveusb fine but I found a problem with the patch.
If /dev/sdd1 is my boot partition, and I have a USB drive large enough that I
have 11 partitions, /dev/sdd11 will be tried instead and /dev/sdd1 will be
skipped. darn!

here is another one.

Created an attachment (id=145628) [details]
both fixes, and better check for disk

One more question: why did we decide to force slowusb?

check_slowusb() {
        [ "${DO_slowusb}" ] || \
        for dir in /sys/bus/usb/drivers/usb-storage/*
        do
                [ -d "${dir}" ] && FORCE_slowusb="1"
        done
}

This just makes my boot 30 seconds longer. If we don't do this and user's boot
fails, he/she still has the option to pass 'slowusb' and get his delay.

Created an attachment (id=145674) [details]
Sleep for scandelay instead of fixed 10 and 20 seconds

I have modified the patch to sleep with sdelay instead of sleep 10 etc. This
makes the sleep for USB driven by the same parameter. Please let me know what
you think of this change.

knock, knock! anybody home?...:-)

No, leave us alone. Annoying poking on bugs just makes us want to shove it
right down to the bottom of the list. Both of the genkernel maintainers are
also members of releng, and we're in the middle of a release cycle, which means
we're very busy. We'll get to it eventually.

(In reply to comment #8)
> No, leave us alone. Annoying poking on bugs just makes us want to shove it
> right down to the bottom of the list. Both of the genkernel maintainers are
> also members of releng, and we're in the middle of a release cycle, which means
> we're very busy. We'll get to it eventually.
> 

Apologies...just used to seeing a comment or two from devs if a patch has been
provided. Both the bugs are of critical nature, but pretty trivial.

Is it fair now (1 week later) to ask for status of the patches supplied here?
Both bugs are real issues, and showstopper for certain configurations. Can we
please include them?

Have you tested that this can still successfully boot a CD? What happens if you
don't have /dev/sdX1 but you have /dev/sdX2? I've seen it happen. What about
arches like hppa where the CD actually contains a partition table (I'm not sure
if the kernel actually recognizes it)?

(In reply to comment #11)
> Have you tested that this can still successfully boot a CD?

I have booted a livecd with it fine.

> What happens if you
> don't have /dev/sdX1 but you have /dev/sdX2?

We can definitely change:

if [ -e "/sys/block/${bsn}" -a -b "${x}1" ]

to

if [ -e "/sys/block/${bsn}" -a -b "${x}[0-9]" ]

in the patch.

> I've seen it happen. What about
> arches like hppa where the CD actually contains a partition table (I'm not sure
> if the kernel actually recognizes it)?
> 

That should work with the changes. In fact, that is a use case for this change.

(In reply to comment #11)
> Have you tested that this can still successfully boot a CD?

I have booted a livecd with it fine. May be some more folks can give it a run
too.

> What happens if you
> don't have /dev/sdX1 but you have /dev/sdX2?

We can definitely change:

if [ -e "/sys/block/${bsn}" -a -b "${x}1" ]

to

if [ -e "/sys/block/${bsn}" -a -b "${x}[0-9]" ]

in the patch.

> I've seen it happen. What about
> arches like hppa where the CD actually contains a partition table (I'm not sure
> if the kernel actually recognizes it)?
> 

That should work with the changes. In fact, that is a use case for this change.

(In reply to comment #13)
> if [ -e "/sys/block/${bsn}" -a -b "${x}[0-9]" ]

Is that even supported syntax, especially with busybox ash's built-in test?

(In reply to comment #14)
> (In reply to comment #13)
> > if [ -e "/sys/block/${bsn}" -a -b "${x}[0-9]" ]
> 
> Is that even supported syntax, especially with busybox ash's built-in test?
> 

On a re-look, it won't work even in bash...because "${x}[0-9]" won't be glob'd
by shell because of "". I tested only if [ -e "/sys/block/${bsn}" -a -b "${x}1"
], which does work in ash. So, your concern for /dev/sda2 being there without a
/dev/sda1, will need to be addressed with a little more work. I will work on
something once I come back from work....:-)

Created an attachment (id=147046) [details]
if at least one partition, skip the device

How about this patch? This will work even if /dev/hda2 is there and /dev/hda1
is missing.

The syntax verified by hand in ash from busybox 1.7.1. I think genekernel uses
that version.

This is in SVN and 3.4.10_pre8... thanks for the patch!

this is fixed.