[elbe-devel] [PATCH] grubinstaller: mount all labels

Torben Hohn torben.hohn at linutronix.de
Fri Jan 5 10:25:04 CET 2018


On Wed, Jan 03, 2018 at 12:27:36PM +0100, John Ogness wrote:
> The current grub installation implementation assumes that all files
> needed to install grub are located in / and /boot partitions.
> However, the user may specify other partitions (such as /usr) that
> are also needed when installing grub.
> 
> Instead of handling /boot as a special case, store all non-root
> labels and mount all of them when installing grub.
> 
> Signed-off-by: John Ogness <john.ogness at linutronix.de>
> ---
>  This has only been tested with grubinstaller202,
>  i.e. it has *not* been tested with grubinstaller199.

removing wheezy support then ?


> 
>  elbepack/hdimg.py | 51 ++++++++++++++++++++++++++-------------------------
>  1 file changed, 26 insertions(+), 25 deletions(-)
> 
> diff --git a/elbepack/hdimg.py b/elbepack/hdimg.py
> index aca0420..7cdf5e7 100644
> --- a/elbepack/hdimg.py
> +++ b/elbepack/hdimg.py
> @@ -176,11 +176,10 @@ class grubinstaller_base( object ):
>      def __init__( self, outf ):
>          self.outf = outf
>          self.root = None
> -        self.boot = None
> +        self.fs = []
>  
> -    def set_boot_entry( self, entry ):
> -        print("setting boot entry")
> -        self.boot = entry
> +    def add_fs_entry( self, entry ):
> +        self.fs.append (entry)
>  
>      def set_root_entry( self, entry ):
>          self.root = entry
> @@ -202,8 +201,8 @@ class grubinstaller202( grubinstaller_base ):
>              self.outf.do( 'kpartx -as /dev/poop0' );
>              self.outf.do( 'mount /dev/mapper/poop0p%d %s' % (self.root.partnum,imagemnt))
>  
> -            if self.boot:
> -                self.outf.do( 'mount /dev/mapper/poop0p%d %s' % (self.boot.partnum,os.path.join( imagemnt, "boot" ) ) )
> +            for entry in self.fs:
> +                self.outf.do( 'mount /dev/mapper/poop0p%d %s' % (entry.partnum, os.path.join( imagemnt, "." + entry.mountpoint ) ) )

i dont really understand what this is supposed to do...

prior to this change /boot was mounted to "$imagemnt/boot" so it would
sit in /boot after "chroot $imagemnt" now it goes to /.boot ?

what did you test here ? 

this code mounts the filesystems together. So that chroot can be used,
and grub can write boot sector and stuff.
If you want to support arbitrary combinations, you have to obey mount
depths, etc. (You cant mount /usr/share before mounting /usr for
example)

The code for ordered mounting already exists. 
Since this looks like its an improvement over the old code, i would
still ACK this, if it could error out, when mountdepths > 1 are used.

But i am not convinced, that this code works. 



>  
>              self.outf.do( "mount --bind /dev %s" % os.path.join( imagemnt, "dev" ) )
>              self.outf.do( "mount --bind /proc %s" % os.path.join( imagemnt, "proc" ) )
> @@ -226,8 +225,8 @@ class grubinstaller202( grubinstaller_base ):
>              self.outf.do( "umount %s" % os.path.join( imagemnt, "proc" ), allow_fail=True )
>              self.outf.do( "umount %s" % os.path.join( imagemnt, "sys" ), allow_fail=True )
>  
> -            if self.boot:
> -                self.outf.do( 'umount /dev/mapper/poop0p%d' % self.boot.partnum, allow_fail=True )
> +            for entry in self.fs:
> +                self.outf.do( 'umount /dev/mapper/poop0p%d' % entry.partnum, allow_fail=True )
>  
>              self.outf.do( 'umount /dev/mapper/poop0p%d' % self.root.partnum, allow_fail=True )
>  
> @@ -243,22 +242,24 @@ class grubinstaller199( grubinstaller_base ):
>          imagemnt = os.path.join(target, "imagemnt" )
>          try:
>              self.outf.do( 'cp -a /dev/loop0 /dev/poop0' )
> -            self.outf.do( 'cp -a /dev/loop1 /dev/poop1' )
> -            self.outf.do( 'cp -a /dev/loop2 /dev/poop2' )
> +            self.outf.do( 'cp -a /dev/loop%d /dev/poop%d' % (self.root.partnum, self.root.partnum) )
> +            for entry in self.fs:
> +                self.outf.do( 'cp -a /dev/loop%d /dev/poop%d' % (entry.partnum, entry.partnum) )
>  
>              self.outf.do( 'losetup /dev/poop0 "%s"' % self.root.filename )
> -            self.root.losetup( self.outf, "poop1" )
> -            self.outf.do( 'mount /dev/poop1 %s' % imagemnt )
> +            self.root.losetup( self.outf, "poop%d" % self.root.partnum )
> +            self.outf.do( 'mount /dev/poop%d %s' % (self.root.partnum, imagemnt) )
>  
> -            if self.boot:
> -                self.boot.losetup( self.outf, "poop2" )
> -                self.outf.do( 'mount /dev/poop2 %s' % (os.path.join( imagemnt, "boot" ) ) )
> +            for entry in self.fs:
> +                entry.losetup( self.outf, "poop%d" % entry.partnum )
> +                self.outf.do( 'mount /dev/poop%d %s' % (entry.partnum, os.path.join( imagemnt, "." + entry.mountpoint ) ) )
>  
>              devmap = open( os.path.join( imagemnt, "boot/grub/device.map" ), "w" )
>              devmap.write( "(hd0) /dev/poop0\n" )
> -            devmap.write( "(hd0,%s) /dev/poop1\n" % self.root.number )
> -            if self.boot:
> -                devmap.write( "(hd0,%s) /dev/poop2\n" % self.boot.number )
> +            devmap.write( "(hd0,%s) /dev/poop%d\n" % (self.root.number, self.root.partnum) )
> +
> +            for entry in self.fs:
> +                devmap.write( "(hd0,%s) /dev/poop%d\n" % (entry.number, entry.partnum) )
>  
>              devmap.close()
>  
> @@ -281,12 +282,12 @@ class grubinstaller199( grubinstaller_base ):
>  
>              self.outf.do( "losetup -d /dev/poop0", allow_fail=True )
>  
> -            if self.boot:
> -                self.outf.do( 'umount /dev/poop2', allow_fail=True )
> -                self.outf.do( 'losetup -d /dev/poop2', allow_fail=True )
> +            for entry in self.fs:
> +                self.outf.do( 'umount /dev/poop%d' % entry.partnum, allow_fail=True )
> +                self.outf.do( 'losetup -d /dev/poop%d' % entry.partnum, allow_fail=True )
>  
> -            self.outf.do( 'umount /dev/poop1', allow_fail=True )
> -            self.outf.do( 'losetup -d /dev/poop1', allow_fail=True )
> +            self.outf.do( 'umount /dev/poop%d' % self.root.partnum, allow_fail=True )
> +            self.outf.do( 'losetup -d /dev/poop%d' % self.root.partnum, allow_fail=True )
>  
>  class simple_fstype(object):
>      def __init__(self, typ):
> @@ -335,8 +336,8 @@ def create_label(outf, disk, part, ppart, fslabel, target, grub):
>  
>      if entry.mountpoint == "/":
>          grub.set_root_entry( entry )
> -    elif entry.mountpoint == "/boot":
> -        grub.set_boot_entry( entry )
> +    else:
> +        grub.add_fs_entry( entry )
>  
>      entry.losetup( outf, "loop0" )
>      outf.do( 'mkfs.%s %s %s /dev/loop0' % ( entry.fstype, entry.mkfsopt, entry.get_label_opt() ) )
> -- 
> 2.1.4
> 
> _______________________________________________
> elbe-devel mailing list
> elbe-devel at linutronix.de
> https://lists.linutronix.de/mailman/listinfo/elbe-devel

-- 
Mit freundlichen Grüßen
Torben Hohn

Linutronix GmbH

Standort: Bremen

Phone: +49 7556 25 999 18; Fax.: +49 7556 25 999 99

Firmensitz / Registered Office: D-88690 Uhldingen, Bahnhofstr. 3
Registergericht / Local District Court: Amtsgericht Freiburg i. Br.; HRB
Nr. / Trade register no.: 700 806

Geschäftsführer / Managing Directors: Heinz Egger, Thomas Gleixner

Eine Bitte von uns: Sollten Sie diese E-Mail irrtümlich erhalten haben,
benachrichtigen Sie uns in diesem Falle bitte sobald wie es Ihnen
möglich ist, durch Antwort-Mail. Vielen Dank!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.linutronix.de/pipermail/elbe-devel/attachments/20180105/18783a4e/attachment.sig>


More information about the elbe-devel mailing list