[Merge] lp:~snappy-dev/goget-ubuntu-touch/all-snaps into lp:goget-ubuntu-touch

Michael Vogt michael.vogt at canonical.com
Thu Oct 22 07:28:12 UTC 2015


Just added some comments so that we don't forget them :)

Diff comments:

> === modified file 'diskimage/common.go'
> --- diskimage/common.go	2015-09-09 14:36:46 +0000
> +++ diskimage/common.go	2015-10-22 07:16:55 +0000
> @@ -190,16 +190,19 @@
>  
>  // BaseImage implements the basic primitives to manage images.
>  type BaseImage struct {
> -	baseMount string
> -	hardware  HardwareDescription
> -	location  string
> -	oem       OemDescription
> -	parts     []partition
> -	partCount int
> -	size      int64
> -	rootSize  int
> +	baseMount  string
> +	bindMounts []string
> +	hardware   HardwareDescription
> +	location   string
> +	oem        OemDescription
> +	parts      []partition
> +	partCount  int
> +	size       int64
> +	rootSize   int
>  }
>  
> +var bindMounts = []string{"dev", "sys", "proc", filepath.Join("sys", "firmware")}

It looks like we are doing this twice (once here, once in snappy).

> +
>  // Mount mounts the image. This also maps the loop device.
>  func (img *BaseImage) Mount() error {
>  	if err := img.doMap(); err != nil {
> 
> === modified file 'ubuntu-device-flash/snappy.go'
> --- ubuntu-device-flash/snappy.go	2015-09-16 11:13:20 +0000
> +++ ubuntu-device-flash/snappy.go	2015-10-22 07:16:55 +0000
> @@ -339,7 +343,21 @@
>  	return hw, err
>  }
>  
> -func (s *Snapper) setup(filePathChan <-chan string, fileCount int) error {
> +func (s *Snapper) bindMount(d string) (string, error) {

I gues the name could be better, something like bindMountInWritable or so

> +	src := filepath.Join(s.img.Writable(), "system-data", d)
> +	dst := filepath.Join(s.img.System(), d)
> +
> +	if err := os.MkdirAll(src, 0755); err != nil {
> +		return "", err
> +	}
> +	cmd := exec.Command("mount", "--bind", src, dst)
> +	if o, err := cmd.CombinedOutput(); err != nil {
> +		return "", fmt.Errorf("os snap mount failed with: %s %v ", err, string(o))
> +	}
> +	return dst, nil
> +}
> +
> +func (s *Snapper) setup(systemImageFiles []Files) error {
>  	printOut("Mounting...")
>  	if err := s.img.Mount(); err != nil {
>  		return err
> @@ -362,14 +379,82 @@
>  
>  	systemPath := s.img.System()
>  
> +	// setup a fake system
> +	if s.oem.PartitionLayout() == "minimal" {
> +		if err := os.MkdirAll(systemPath, 0755); err != nil {
> +			return err
> +		}
> +		// mount os snap
> +		cmd := exec.Command("mount", s.OS, systemPath)
> +		if o, err := cmd.CombinedOutput(); err != nil {
> +			return fmt.Errorf("os snap mount failed with: %s %v ", err, string(o))
> +		}
> +		defer exec.Command("umount", systemPath).Run()
> +
> +		// we need to do what "writable-paths" normally does on
> +		// boot for etc/systemd/system, i.e. copy all the stuff
> +		// from the os into the writable partition. normally
> +		// this is the job of the initrd, however it won't touch
> +		// the dir if there are files in there already. and a
> +		// kernel/os install will create auto-mount units in there
> +		src := filepath.Join(systemPath, "etc", "systemd", "system")
> +		dst := filepath.Join(s.img.Writable(), "system-data", "etc", "systemd")
> +		if err := os.MkdirAll(dst, 0755); err != nil {
> +			return err
> +		}
> +		cmd = exec.Command("cp", "-a", src, dst)
> +		if o, err := cmd.CombinedOutput(); err != nil {
> +			return fmt.Errorf("copy failed: %s %s", err, o)
> +		}
> +
> +		// bind mount all relevant dirs
> +		for _, d := range []string{"apps", "oem", "var/lib/snappy", "var/lib/apps", "etc/systemd/system/", "tmp"} {
> +			dst, err := s.bindMount(d)
> +			if err != nil {
> +				return err
> +			}
> +			defer exec.Command("umount", dst).Run()
> +		}
> +
> +		// bind mount /boot/efi
> +		dst = filepath.Join(systemPath, "/boot/efi")
> +		cmd = exec.Command("mount", "--bind", s.img.Boot(), dst)
> +		if o, err := cmd.CombinedOutput(); err != nil {
> +			return fmt.Errorf("boot bind mount failed with: %s %v ", err, string(o))
> +		}
> +		defer exec.Command("umount", dst).Run()
> +
> +		// grub needs this
> +		grubUbuntu := filepath.Join(s.img.Boot(), "EFI/ubuntu/grub")
> +		os.MkdirAll(grubUbuntu, 0755)
> +
> +		// and /boot/grub
> +		src = grubUbuntu
> +		dst = filepath.Join(systemPath, "/boot/grub")
> +		cmd = exec.Command("mount", "--bind", src, dst)
> +		if o, err := cmd.CombinedOutput(); err != nil {
> +			return fmt.Errorf("boot/ubuntu bind mount failed with: %s %v ", err, string(o))
> +		}
> +		defer exec.Command("umount", dst).Run()
> +
> +		// TERRIBLE but we need a /boot/grub/grub.cfg so that

It looks like this is no longer needed, I removed it and its still fine it seems.

> +		//          the kernel and os snap can be installed
> +		oemGrubCfg := filepath.Join(s.stagingRootPath, "oem", "generic-amd64", "current", "grub.cfg")
> +		cmd = exec.Command("cp", oemGrubCfg, grubUbuntu)
> +		o, err := cmd.CombinedOutput()
> +		if err != nil {
> +			return fmt.Errorf("failed to copy %s %s", err, o)
> +		}
> +	}
> +
> +	if err := s.img.SetupBoot(); err != nil {
> +		return err
> +	}
> +
>  	if err := s.install(systemPath); err != nil {
>  		return err
>  	}
>  
> -	if err := s.img.SetupBoot(); err != nil {
> -		return err
> -	}
> -
>  	for i := range s.customizationFunc {
>  		if err := s.customizationFunc[i](); err != nil {
>  			return err


-- 
https://code.launchpad.net/~snappy-dev/goget-ubuntu-touch/all-snaps/+merge/275273
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~snappy-dev/goget-ubuntu-touch/all-snaps into lp:goget-ubuntu-touch.



More information about the Ubuntu-reviews mailing list