Merge lp:~mvo/snappy/snappy-bootsuccess-env into lp:~snappy-dev/snappy/snappy-moved-to-github

[Michael Vogt]

Change bootsuccessful uboot to use fw_setenv instead of modifying a
text-file.

Change bootsuccessful uboot to use fw_setenv instead of modifying a
text-file.

This merge should wait until a full end-to-end test was performed.

[Sergio Schvezov (sergiusens)]

review: Needs Information

[Michael Vogt (mvo)]

Thanks for the review, you raised some important points. I addressed them now. Please let me know if there is anything else that should be fixed.

[Snappy Tarmac (snappydevtarmac)]

Voting does not meet specified criteria. Required: Approve >= 1, Disapprove == 0. Got: 1 Needs Information.

[Michael Vogt (mvo)]

review: Approve

=== modified file 'partition/bootloader.go'
--- partition/bootloader.go	2015-07-01 14:48:33 +0000
+++ partition/bootloader.go	2015-07-17 18:15:23 +0000
@@ -37,6 +37,8 @@
 	// that the boot of the new rootfs was successful).
 	bootloaderBootmodeVar = "snappy_mode"
 
+	bootloaderTrialBootVar = "snappy_trial_boot"
+
 	// Initial and final values
 	bootloaderBootmodeTry     = "try"
 	bootloaderBootmodeSuccess = "regular"

=== modified file 'partition/bootloader_grub.go'
--- partition/bootloader_grub.go	2015-07-01 14:48:33 +0000
+++ partition/bootloader_grub.go	2015-07-17 18:15:23 +0000
@@ -32,16 +32,14 @@
 	bootloaderGrubConfigFileReal = "/boot/grub/grub.cfg"
 	bootloaderGrubEnvFileReal    = "/boot/grub/grubenv"
 
-	bootloaderGrubEnvCmdReal       = "/usr/bin/grub-editenv"
-	bootloaderGrubTrialBootVarReal = "snappy_trial_boot"
+	bootloaderGrubEnvCmdReal = "/usr/bin/grub-editenv"
 )
 
 // var to make it testable
 var (
-	bootloaderGrubDir          = bootloaderGrubDirReal
-	bootloaderGrubConfigFile   = bootloaderGrubConfigFileReal
-	bootloaderGrubTrialBootVar = bootloaderGrubTrialBootVarReal
-	bootloaderGrubEnvFile      = bootloaderGrubEnvFileReal
+	bootloaderGrubDir        = bootloaderGrubDirReal
+	bootloaderGrubConfigFile = bootloaderGrubConfigFileReal
+	bootloaderGrubEnvFile    = bootloaderGrubEnvFileReal
 
 	bootloaderGrubEnvCmd = bootloaderGrubEnvCmdReal
 )
@@ -122,9 +120,8 @@
 }
 
 func (g *grub) MarkCurrentBootSuccessful(currentRootfs string) (err error) {
-	// Clear the variable set by grub on boot to denote a good
-	// boot.
-	if err := g.unsetBootVar(bootloaderGrubTrialBootVar); err != nil {
+	// Clear the variable set on boot to denote a good boot.
+	if err := g.unsetBootVar(bootloaderTrialBootVar); err != nil {
 		return err
 	}
 

=== modified file 'partition/bootloader_uboot.go'
--- partition/bootloader_uboot.go	2015-07-01 06:27:50 +0000
+++ partition/bootloader_uboot.go	2015-07-17 18:15:23 +0000
@@ -41,9 +41,11 @@
 	// this partition is "good".
 	bootloaderUbootStampFileReal = "/boot/uboot/snappy-stamp.txt"
 
-	// the main uEnv.txt u-boot config file sources this snappy
-	// boot-specific config file.
+	// DEPRECATED:
 	bootloaderUbootEnvFileReal = "/boot/uboot/snappy-system.txt"
+
+	// the real uboot env
+	bootloaderUbootFwEnvFileReal = "/boot/uboot/uboot.env"
 )
 
 // var to make it testable
@@ -52,6 +54,7 @@
 	bootloaderUbootConfigFile = bootloaderUbootConfigFileReal
 	bootloaderUbootStampFile  = bootloaderUbootStampFileReal
 	bootloaderUbootEnvFile    = bootloaderUbootEnvFileReal
+	bootloaderUbootFwEnvFile  = bootloaderUbootFwEnvFileReal
 	atomicFileUpdate          = atomicFileUpdateImpl
 )
 
@@ -186,7 +189,7 @@
 	return file.Sync()
 }
 
-func (u *uboot) MarkCurrentBootSuccessful(currentRootfs string) (err error) {
+func (u *uboot) markCurrentBootSuccessfulLegacy(currentRootfs string) error {
 	changes := []configFileChange{
 		configFileChange{Name: bootloaderBootmodeVar,
 			Value: bootloaderBootmodeSuccess,
@@ -203,6 +206,69 @@
 	return os.RemoveAll(bootloaderUbootStampFile)
 }
 
+// fw_setenv gets the location of the configuration to use from the
+// file /etc/fw_env.config
+func (u *uboot) unsetBootVar(name string) error {
+	if u.hasBootVar(name) {
+		return runCommand("fw_setenv", name)
+	}
+
+	return nil
+}
+
+func (u *uboot) setBootVar(name, value string) error {
+	// we ignore the error here, the interface of fw_printenv
+	// is very simplistic, the github.com/mvo5/uboot-go code
+	// will fix that
+	curVal, _ := u.getBootVar(name)
+	if curVal != value {
+		return runCommand("fw_setenv", name, value)
+	}
+
+	return nil
+}
+
+func (u *uboot) hasBootVar(name string) bool {
+	// FIXME: too simplistic, this will be replaces with
+	//        github.com/mvo5/uboot-go/
+	err := runCommand("fw_printenv", name)
+	return err == nil
+}
+
+func (u *uboot) getBootVar(name string) (string, error) {
+	output, err := runCommandWithStdout("fw_printenv", "-n", name)
+	if err != nil {
+		return "", err
+	}
+
+	return output, nil
+}
+
+// FIXME: this is super similar to grub now, refactor to extract the
+//        common code
+func (u *uboot) markCurrentBootSuccessfulFwEnv(currentRootfs string) error {
+	// Clear the variable set on boot to denote a good boot.
+	if err := u.unsetBootVar(bootloaderTrialBootVar); err != nil {
+		return err
+	}
+
+	if err := u.setBootVar(bootloaderRootfsVar, currentRootfs); err != nil {
+		return err
+	}
+
+	return u.setBootVar(bootloaderBootmodeVar, bootloaderBootmodeSuccess)
+}
+
+func (u *uboot) MarkCurrentBootSuccessful(currentRootfs string) error {
+	// modern system
+	if helpers.FileExists(bootloaderUbootFwEnvFile) {
+		return u.markCurrentBootSuccessfulFwEnv(currentRootfs)
+	}
+
+	// legacy
+	return u.markCurrentBootSuccessfulLegacy(currentRootfs)
+}
+
 // Write lines to file atomically. File does not have to preexist.
 // FIXME: put into utils package
 func atomicFileUpdateImpl(file string, lines []string) (err error) {

=== modified file 'partition/bootloader_uboot_test.go'
--- partition/bootloader_uboot_test.go	2015-06-29 23:15:00 +0000
+++ partition/bootloader_uboot_test.go	2015-07-17 18:15:23 +0000
@@ -318,3 +318,55 @@
 	c.Check(strings.Contains(string(bytes), "snappy_mode=regular"), Equals, true)
 	c.Check(strings.Contains(string(bytes), "snappy_ab=a"), Equals, true)
 }
+
+func (s *PartitionTestSuite) TestUbootMarkCurrentBootSuccessfulFwEnv(c *C) {
+	s.makeFakeUbootEnv(c)
+
+	err := ioutil.WriteFile(bootloaderUbootFwEnvFile, []byte(""), 0644)
+	c.Assert(err, IsNil)
+
+	partition := New()
+	u := newUboot(partition)
+	c.Assert(u, NotNil)
+
+	allCommands = nil
+	runCommand = mockRunCommandWithCapture
+	err = u.MarkCurrentBootSuccessful("b")
+	c.Assert(err, IsNil)
+	c.Assert(allCommands, HasLen, 4)
+	c.Assert(allCommands[0], DeepEquals, singleCommand{"fw_printenv", bootloaderTrialBootVar})
+	c.Assert(allCommands[1], DeepEquals, singleCommand{"fw_setenv", bootloaderTrialBootVar})
+	c.Assert(allCommands[2], DeepEquals, singleCommand{"fw_setenv", bootloaderRootfsVar, "b"})
+	c.Assert(allCommands[3], DeepEquals, singleCommand{"fw_setenv", bootloaderBootmodeVar, bootloaderBootmodeSuccess})
+}
+
+func (s *PartitionTestSuite) TestUbootSetEnv(c *C) {
+	s.makeFakeUbootEnv(c)
+
+	err := ioutil.WriteFile(bootloaderUbootFwEnvFile, []byte(""), 0644)
+	c.Assert(err, IsNil)
+
+	partition := New()
+	u := newUboot(partition)
+	c.Assert(u, NotNil)
+
+	// we simulate here that fw_printenv bootloaderreturns
+	runCommandWithStdout = func(args ...string) (string, error) {
+		if args[0] == "fw_printenv" && args[2] == bootloaderRootfsVar {
+			return "b", nil
+		}
+
+		return "something", nil
+	}
+
+	allCommands = nil
+	runCommand = mockRunCommandWithCapture
+	err = u.(*uboot).setBootVar(bootloaderRootfsVar, "b")
+	c.Assert(err, IsNil)
+	c.Assert(allCommands, HasLen, 0)
+
+	err = u.(*uboot).setBootVar(bootloaderRootfsVar, "a")
+	c.Assert(err, IsNil)
+	c.Assert(allCommands, HasLen, 1)
+	c.Assert(allCommands[0], DeepEquals, singleCommand{"fw_setenv", bootloaderRootfsVar, "a"})
+}

=== modified file 'partition/partition_test.go'
--- partition/partition_test.go	2015-07-01 14:48:33 +0000
+++ partition/partition_test.go	2015-07-17 18:15:23 +0000
@@ -60,6 +60,7 @@
 	bootloaderUbootDir = filepath.Join(s.tempdir, "boot", "uboot")
 	bootloaderUbootConfigFile = filepath.Join(bootloaderUbootDir, "uEnv.txt")
 	bootloaderUbootEnvFile = filepath.Join(bootloaderUbootDir, "uEnv.txt")
+	bootloaderUbootFwEnvFile = filepath.Join(bootloaderUbootDir, "uboot.env")
 	bootloaderUbootStampFile = filepath.Join(bootloaderUbootDir, "snappy-stamp.txt")
 
 	c.Assert(mounts, DeepEquals, mountEntryArray(nil))