Merge lp:~mvo/snappy/snappy-more-uboot-fiddling into lp:~snappy-dev/snappy/snappy-moved-to-github

[Michael Vogt]

Do all modifications in uboot via setEnvVar/getEnvVar and set the methods for this in the uboot init.

Do all modifications in uboot via setEnvVar/getEnvVar and set the methods for this in the uboot init.

[Oliver Grawert (ogra)]

tested and it works ... i cant really judge the code itself though

review: Approve

[Ricardo Salveti (rsalveti)]

Looks fine.

review: Approve

=== modified file 'partition/bootloader_uboot.go'
--- partition/bootloader_uboot.go	2015-07-23 11:54:14 +0000
+++ partition/bootloader_uboot.go	2015-07-23 18:11:32 +0000
@@ -72,7 +72,10 @@
 	Value string
 }
 
-// newUboot create a new Grub bootloader object
+var setBootVar = func(name, value string) error { return nil }
+var getBootVar = func(name string) (string, error) { return "", nil }
+
+// newUboot create a new Uboot bootloader object
 func newUboot(partition *Partition) bootLoader {
 	if !helpers.FileExists(bootloaderUbootConfigFile) {
 		return nil
@@ -84,6 +87,14 @@
 	}
 	u := uboot{bootloaderType: *b}
 
+	if helpers.FileExists(bootloaderUbootFwEnvFile) {
+		setBootVar = setBootVarFwEnv
+		getBootVar = getBootVarFwEnv
+	} else {
+		setBootVar = setBootVarLegacy
+		getBootVar = getBootVarLegacy
+	}
+
 	return &u
 }
 
@@ -91,36 +102,15 @@
 	return bootloaderNameUboot
 }
 
-// ToggleRootFS make the U-Boot bootloader switch rootfs's.
-//
-// Approach:
-//
-// - Assume the device's installed version of u-boot supports
-//   CONFIG_SUPPORT_RAW_INITRD (that allows u-boot to boot a
-//   standard initrd+kernel on the fat32 disk partition).
-// - Copy the "other" rootfs's kernel+initrd to the boot partition,
-//   renaming them in the process to ensure the next boot uses the
-//   correct versions.
 func (u *uboot) ToggleRootFS(otherRootfs string) (err error) {
-
-	// If the file exists, update it. Otherwise create it.
-	//
-	// The file _should_ always exist, but since it's on a writable
-	// partition, it's possible the admin removed it by mistake. So
-	// recreate to allow the system to boot!
-	changes := []configFileChange{
-		configFileChange{Name: bootloaderRootfsVar,
-			Value: string(otherRootfs),
-		},
-		configFileChange{Name: bootloaderBootmodeVar,
-			Value: bootloaderBootmodeTry,
-		},
+	if err := setBootVar(bootloaderRootfsVar, string(otherRootfs)); err != nil {
+		return err
 	}
 
-	return modifyNameValueFile(bootloaderUbootEnvFile, changes)
+	return setBootVar(bootloaderBootmodeVar, bootloaderBootmodeTry)
 }
 
-func (u *uboot) GetBootVar(name string) (value string, err error) {
+func getBootVarLegacy(name string) (value string, err error) {
 	cfg := goconfigparser.New()
 	cfg.AllowNoSectionHeader = true
 	if err := cfg.ReadFile(bootloaderUbootEnvFile); err != nil {
@@ -130,84 +120,23 @@
 	return cfg.Get("", name)
 }
 
-func (u *uboot) GetNextBootRootFSName() (label string, err error) {
-	value, err := u.GetBootVar(bootloaderRootfsVar)
-	if err != nil {
-		// should never happen
-		return "", err
-	}
-
-	return value, nil
-}
-
-// FIXME: put into utils package
-func readLines(path string) (lines []string, err error) {
-
-	file, err := os.Open(path)
-
-	if err != nil {
-		return nil, err
-	}
-
-	defer file.Close()
-
-	scanner := bufio.NewScanner(file)
-	for scanner.Scan() {
-		lines = append(lines, scanner.Text())
-	}
-
-	return lines, scanner.Err()
-}
-
-// FIXME: put into utils package
-func writeLines(lines []string, path string) (err error) {
-
-	file, err := os.Create(path)
-
-	if err != nil {
-		return err
-	}
-
-	defer func() {
-		e := file.Close()
-		if err == nil {
-			err = e
-		}
-	}()
-
-	writer := bufio.NewWriter(file)
-
-	for _, line := range lines {
-		if _, err := fmt.Fprintln(writer, line); err != nil {
-			return err
-		}
-	}
-
-	if err := writer.Flush(); err != nil {
-		return err
-	}
-
-	return file.Sync()
-}
-
-func (u *uboot) markCurrentBootSuccessfulLegacy(currentRootfs string) error {
+func setBootVarLegacy(name, value string) error {
+	curVal, err := getBootVarLegacy(name)
+	if err == nil && curVal == value {
+		return nil
+	}
+
 	changes := []configFileChange{
-		configFileChange{Name: bootloaderBootmodeVar,
-			Value: bootloaderBootmodeSuccess,
-		},
-		configFileChange{Name: bootloaderRootfsVar,
-			Value: string(currentRootfs),
-		},
-	}
-
-	if err := modifyNameValueFile(bootloaderUbootEnvFile, changes); err != nil {
-		return err
-	}
-
-	return os.RemoveAll(bootloaderUbootStampFile)
+		configFileChange{
+			Name:  name,
+			Value: value,
+		},
+	}
+
+	return modifyNameValueFile(bootloaderUbootEnvFile, changes)
 }
 
-func (u *uboot) setBootVar(name, value string) error {
+func setBootVarFwEnv(name, value string) error {
 	env, err := uenv.Open(bootloaderUbootFwEnvFile)
 	if err != nil {
 		return err
@@ -222,12 +151,7 @@
 	return env.Save()
 }
 
-func (u *uboot) hasBootVar(name string) (bool, error) {
-	v, err := u.getBootVar(name)
-	return v != "", err
-}
-
-func (u *uboot) getBootVar(name string) (string, error) {
+func getBootVarFwEnv(name string) (string, error) {
 	env, err := uenv.Open(bootloaderUbootFwEnvFile)
 	if err != nil {
 		return "", err
@@ -236,29 +160,42 @@
 	return env.Get(name), nil
 }
 
+func (u *uboot) GetBootVar(name string) (value string, err error) {
+	return getBootVar(name)
+}
+
+func (u *uboot) GetNextBootRootFSName() (label string, err error) {
+	value, err := u.GetBootVar(bootloaderRootfsVar)
+	if err != nil {
+		// should never happen
+		return "", err
+	}
+
+	return value, nil
+}
+
 // FIXME: this is super similar to grub now, refactor to extract the
 //        common code
-func (u *uboot) markCurrentBootSuccessfulFwEnv(currentRootfs string) error {
+func (u *uboot) MarkCurrentBootSuccessful(currentRootfs string) error {
 	// Clear the variable set on boot to denote a good boot.
-	if err := u.setBootVar(bootloaderTrialBootVar, "0"); err != nil {
-		return err
-	}
-
-	if err := u.setBootVar(bootloaderRootfsVar, currentRootfs); err != nil {
-		return err
-	}
-
-	return u.setBootVar(bootloaderBootmodeVar, bootloaderBootmodeSuccess)
+	if err := setBootVar(bootloaderTrialBootVar, "0"); err != nil {
+		return err
+	}
+
+	if err := setBootVar(bootloaderRootfsVar, currentRootfs); err != nil {
+		return err
+	}
+
+	if err := setBootVar(bootloaderBootmodeVar, bootloaderBootmodeSuccess); err != nil {
+		return err
+	}
+
+	// legacy support, does not error if the file is not there
+	return os.RemoveAll(bootloaderUbootStampFile)
 }
 
-func (u *uboot) MarkCurrentBootSuccessful(currentRootfs string) error {
-	// modern system
-	if helpers.FileExists(bootloaderUbootFwEnvFile) {
-		return u.markCurrentBootSuccessfulFwEnv(currentRootfs)
-	}
-
-	// legacy
-	return u.markCurrentBootSuccessfulLegacy(currentRootfs)
+func (u *uboot) BootDir() string {
+	return bootloaderUbootDir
 }
 
 // Write lines to file atomically. File does not have to preexist.
@@ -349,6 +286,52 @@
 	return nil
 }
 
-func (u *uboot) BootDir() string {
-	return bootloaderUbootDir
+// FIXME: put into utils package
+func readLines(path string) (lines []string, err error) {
+
+	file, err := os.Open(path)
+
+	if err != nil {
+		return nil, err
+	}
+
+	defer file.Close()
+
+	scanner := bufio.NewScanner(file)
+	for scanner.Scan() {
+		lines = append(lines, scanner.Text())
+	}
+
+	return lines, scanner.Err()
+}
+
+// FIXME: put into utils package
+func writeLines(lines []string, path string) (err error) {
+
+	file, err := os.Create(path)
+
+	if err != nil {
+		return err
+	}
+
+	defer func() {
+		e := file.Close()
+		if err == nil {
+			err = e
+		}
+	}()
+
+	writer := bufio.NewWriter(file)
+
+	for _, line := range lines {
+		if _, err := fmt.Fprintln(writer, line); err != nil {
+			return err
+		}
+	}
+
+	if err := writer.Flush(); err != nil {
+		return err
+	}
+
+	return file.Sync()
 }

=== modified file 'partition/bootloader_uboot_test.go'
--- partition/bootloader_uboot_test.go	2015-07-23 11:54:14 +0000
+++ partition/bootloader_uboot_test.go	2015-07-23 18:11:32 +0000
@@ -59,6 +59,8 @@
 snappy_stamp=snappy-stamp.txt
 # either "regular" (normal boot) or "try" when trying a new version
 snappy_mode=regular
+# compat
+snappy_trial_boot=0
 # if we are trying a new version, check if stamp file is already there to revert
 # to other version
 snappy_boot=if test "${snappy_mode}" = "try"; then if test -e mmc ${bootpart} ${snappy_stamp}; then if test "${snappy_ab}" = "a"; then setenv snappy_ab "b"; else setenv snappy_ab "a"; fi; else fatwrite mmc ${mmcdev}:${mmcpart} 0x0 ${snappy_stamp} 0; fi; fi; run loadfiles; setenv mmcroot /dev/disk/by-label/system-${snappy_ab} ${snappy_cmdline}; run mmcargs; bootz ${loadaddr} ${initrd_addr}:${initrd_size} ${fdtaddr}
@@ -363,7 +365,7 @@
 	u := newUboot(partition)
 	c.Assert(u, NotNil)
 
-	err = u.(*uboot).setBootVar(bootloaderRootfsVar, "b")
+	err = setBootVar(bootloaderRootfsVar, "b")
 	c.Assert(err, IsNil)
 
 	env, err = uenv.Open(bootloaderUbootFwEnvFile)