[Merge] lp:~snappy-dev/goget-ubuntu-touch/all-snaps into lp:goget-ubuntu-touch
Sergio Schvezov
sergio.schvezov at canonical.com
Thu Oct 22 13:09:25 UTC 2015
On Oct 22, 2015 4:28 AM, "Michael Vogt" <michael.vogt at canonical.com> wrote:
>
> 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).
You will see a bunch of these little things which could go away when
migrating the provisioner into snappy itself.
>
> > +
> > // 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.
Right. GenericBootSetup should be taking care of this.
>
> > + // 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 Snappy Developers is subscribed to branch
lp:~snappy-dev/goget-ubuntu-touch/all-snaps.
>
> Launchpad-Message-Rationale: Subscriber @snappy-dev
> Launchpad-Message-For: snappy-dev
> Launchpad-Notification-Type: code-review
> Launchpad-Branch: ~snappy-dev/goget-ubuntu-touch/all-snaps
> Launchpad-Project: goget-ubuntu-touch
--
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