make backup class not singleton and move restore ability there

src/include/bootloader/grub2/misc.rb

@@ -267,9 +267,10 @@ def grub_updateMBR
           "Creating backup of boot sectors of %1",
           disks_to_rewrite
         )
-        Builtins.foreach(disks_to_rewrite) do |device|
-          ::Bootloader::BootRecordBackup.create_for(device)
+        backups = disks_to_rewrite.map do |d|
+          ::Bootloader::BootRecordBackup.new(d)
         end
+        backups.each(&:create)
       end
       ret = true
       # if the bootloader stage 1 is not installed in the MBR, but

src/include/bootloader/routines/misc.rb

@@ -352,32 +352,13 @@ def getMbrsToRewrite
     # @param [String] device string device to rewrite MBR to
     # @return [Boolean] true on success
     def restoreMBR(device)
-      device_file = Builtins.mergestring(Builtins.splitstring(device, "/"), "_")
-      if Ops.less_or_equal(
-          SCR.Read(
-            path(".target.size"),
-            Builtins.sformat(
-              "/var/lib/YaST2/backup_boot_sectors/%1",
-              device_file
-            )
-          ),
-          0
-        )
+      backup = ::Bootloader::BootRecordBackup.new(device)
+      begin
+        backup.restore
+      rescue ::Bootloader::BootRecordBackup::Missing
         Report.Error("Can't restore MBR. No saved MBR found")
         return false
       end
-      # added fix 446 -> 440 for Vista booting problem bnc #396444
-      ret = Convert.to_integer(
-        SCR.Execute(
-          path(".target.bash"),
-          Builtins.sformat(
-            "/bin/dd of=%1 if=/var/lib/YaST2/backup_boot_sectors/%2 bs=440 count=1",
-            device,
-            device_file
-          )
-        )
-      )
-      ret == 0
     end
 
     # Get map of swap partitions

src/lib/bootloader/boot_record_backup.rb

@@ -3,92 +3,128 @@
 Yast.import "BootCommon"
 
 module Bootloader
-  # Responsibility of class is to manage backups of MBR, respective PBR of disk,
+  # Responsibility of class is to manage backup of MBR, respective PBR of disk,
   # respective partition.
   class BootRecordBackup
-    class << self
-      include Yast::Logger
-      BASH_PATH = Yast::Path.new(".target.bash")
-      BASH_OUTPUT_PATH = Yast::Path.new(".target.bash_output")
-      MAIN_BACKUP_DIR = "/var/lib/YaST2/backup_boot_sectors/"
-      KEPT_BACKUPS = 10
-
-      # Creates backup of MBR or PBR of given device.
-      # Backup is stored in /var/lib/YaST2/backup_boot_sectors, in logs
-      # directory and if it is MBR of primary disk, then also in /boot/backup_mbr
-      def create_for(device)
-        device_file = device.tr("/", "_")
-        device_file_path = MAIN_BACKUP_DIR + device_file
-
-        Yast::SCR.Execute(BASH_PATH, "mkdir -p #{MAIN_BACKUP_DIR}")
-
-        # check if file exists
-        if Yast::SCR.Read(Yast::Path.new(".target.size"), device_file_path) > 0
-          # move it so we do not overwrite it
-          change_date = formated_file_ctime(device_file_path)
-          Yast::SCR.Execute(
-            BASH_PATH,
-            Yast::Builtins.sformat("/bin/mv %1 %1-%2", device_file_path, change_date)
-          )
+    include Yast::Logger
+    BASH_PATH = Yast::Path.new(".target.bash")
+    BASH_OUTPUT_PATH = Yast::Path.new(".target.bash_output")
+    TARGET_SIZE = Yast::Path.new(".target.size")
+    MAIN_BACKUP_DIR = "/var/lib/YaST2/backup_boot_sectors/"
+    KEPT_BACKUPS = 10
 
-          cleanup_backups(device_file)
-        end
+    attr_reader :device
 
-        copy_br(device, device_file_path)
+    # Create backup handling class for given device
+    # @param device[String] expect kernel name of device like "/dev/sda"
+    def initialize(device)
+      @device = device
+    end
 
-        # save MBR to yast2 log directory
-        logs_path = "/var/log/YaST2/" + device_file
-        copy_br(device, logs_path)
+    # Creates backup of MBR or PBR of given device.
+    # Backup is stored in /var/lib/YaST2/backup_boot_sectors, in logs
+    # directory and if it is MBR of primary disk, then also in /boot/backup_mbr
+    def create
+      Yast::SCR.Execute(BASH_PATH, "mkdir -p #{MAIN_BACKUP_DIR}")
 
-        if device == Yast::BootCommon.mbrDisk
-          copy_br(device, "/boot/backup_mbr")
+      # check if file exists
+      if device_backup_exist?
+        # move it so we do not overwrite it
+        change_date = formated_file_ctime(device_file_path)
+        Yast::SCR.Execute(
+          BASH_PATH,
+          Yast::Builtins.sformat("/bin/mv %1 %1-%2", device_file_path, change_date)
+        )
 
-          # save thinkpad MBR
-          if Yast::BootCommon.ThinkPadMBR(device)
-            device_file_path_thinkpad = device_file_path + "thinkpadMBR"
-            log.info("Backup thinkpad MBR")
-            Yast::SCR.Execute(
-              BASH_PATH,
-              "cp #{device_file_path} #{device_file_path_thinkpad}",
-            )
-          end
-        end
+        cleanup_backups(device_file)
       end
 
-    private
-      # Get last change time of file
-      # @param [String] filename string name of file
-      # @return [String] last change date as YYYY-MM-DD-HH-MM-SS
-      def formated_file_ctime(filename)
-        stat = Yast::SCR.Read(Yast::Path.new(".target.stat"), filename)
-        ctime = stat["ctime"] or raise "Cannot get modification time of file #{filename}"
-        time = DateTime.strptime(ctime.to_s, "%s")
+      copy_br(device, device_file_path)
 
-        time.strftime("%Y-%m-%d-%H-%M-%S")
-      end
+      # save MBR to yast2 log directory
+      logs_path = "/var/log/YaST2/" + device_file
+      copy_br(device, logs_path)
 
-      def copy_br(device, target_path)
-        Yast::SCR.Execute(
-          BASH_PATH,
-          "/bin/dd if=#{device} of=#{target_path} bs=512 count=1 2>&1"
-        )
-      end
+      if device == Yast::BootCommon.mbrDisk
+        copy_br(device, "/boot/backup_mbr")
 
-      def cleanup_backups(device_file)
-        files = Yast::SCR.Read(Yast::Path.new(".target.dir"), MAIN_BACKUP_DIR)
-        # clean only backups for this device
-        files.select! do |c|
-          c =~ /#{Regexp.escape(device_file)}-\d{4}(?:-\d{2}){5}/
-        end
-        # and sort so we can benefit from its ascending order
-        files.sort!
-        files.drop(KEPT_BACKUPS).each do |file_name|
+        # save thinkpad MBR
+        if Yast::BootCommon.ThinkPadMBR(device)
+          device_file_path_thinkpad = device_file_path + "thinkpadMBR"
+          log.info("Backup thinkpad MBR")
           Yast::SCR.Execute(
-            Yast::Path.new(".target.remove"),
-            MAIN_BACKUP_DIR + file_name
+            BASH_PATH,
+            "cp #{device_file_path} #{device_file_path_thinkpad}",
           )
         end
       end
     end
+
+    # Exception used to indicate that backup missing, so any action with it is
+    # not possible
+    class Missing < RuntimeError
+      def initialize
+        super "Backup for boot record missing."
+      end
+    end
+
+    # Restore backup
+    # @raise [::Bootloader::BootRecordBackup::Missing] if backup missing
+    # @return true if copy is successful
+    def restore
+      raise Missing.new unless device_backup_exist?
+
+      # Copy only 440 bytes for Vista booting problem bnc #396444
+      # and also to not destroy partition table
+      copy_br(device_file_path, device, bs: 440) == 0
+    end
+
+  private
+
+    def device_file
+      @device_file ||= @device.tr("/", "_")
+    end
+
+    def device_file_path
+      @device_file_path ||= MAIN_BACKUP_DIR + device_file
+    end
+
+    def device_backup_exist?
+      Yast::SCR.Read(TARGET_SIZE, device_file_path) > 0
+    end
+
+    # Get last change time of file
+    # @param [String] filename string name of file
+    # @return [String] last change date as YYYY-MM-DD-HH-MM-SS
+    def formated_file_ctime(filename)
+      stat = Yast::SCR.Read(Yast::Path.new(".target.stat"), filename)
+      ctime = stat["ctime"] or raise "Cannot get modification time of file #{filename}"
+      time = DateTime.strptime(ctime.to_s, "%s")
+
+      time.strftime("%Y-%m-%d-%H-%M-%S")
+    end
+
+    def copy_br(device, target_path, bs: 512)
+      Yast::SCR.Execute(
+        BASH_PATH,
+        "/bin/dd if=#{device} of=#{target_path} bs=#{bs} count=1 2>&1"
+      )
+    end
+
+    def cleanup_backups(device_file)
+      files = Yast::SCR.Read(Yast::Path.new(".target.dir"), MAIN_BACKUP_DIR)
+      # clean only backups for this device
+      files.select! do |c|
+        c =~ /#{Regexp.escape(device_file)}-\d{4}(?:-\d{2}){5}/
+      end
+      # and sort so we can benefit from its ascending order
+      files.sort!
+      files.drop(KEPT_BACKUPS).each do |file_name|
+        Yast::SCR.Execute(
+          Yast::Path.new(".target.remove"),
+          MAIN_BACKUP_DIR + file_name
+        )
+      end
+    end
   end
 end

test/boot_record_backup_test.rb

@@ -5,12 +5,37 @@
 require "bootloader/boot_record_backup"
 
 describe Bootloader::BootRecordBackup do
-  subject { Bootloader::BootRecordBackup }
-  describe ".create_for" do
-    BASH_PATH = Yast::Path.new(".target.bash")
-    SIZE_PATH = Yast::Path.new(".target.size")
-    DIR_PATH = Yast::Path.new(".target.dir")
-    STAT_PATH = Yast::Path.new(".target.stat")
+  BASH_PATH = Yast::Path.new(".target.bash")
+  SIZE_PATH = Yast::Path.new(".target.size")
+  DIR_PATH = Yast::Path.new(".target.dir")
+  STAT_PATH = Yast::Path.new(".target.stat")
+
+  subject { Bootloader::BootRecordBackup.new("/dev/sda") }
+
+  describe "#restore" do
+    it "returns true if backup is successfully restored" do
+      allow(Yast::SCR).to receive(:Read).with(SIZE_PATH, anything).and_return(10)
+      expect(Yast::SCR).to receive(:Execute).with(BASH_PATH, /bin\/dd.* if=\/var\/lib\/YaST2\/backup_boot_sectors/).
+        and_return(0)
+
+      expect(subject.restore).to be true
+    end
+
+    it "returns false if copying backup failed" do
+      allow(Yast::SCR).to receive(:Read).with(SIZE_PATH, anything).and_return(10)
+      expect(Yast::SCR).to receive(:Execute).with(BASH_PATH, /bin\/dd.* if=\/var\/lib\/YaST2\/backup_boot_sectors/).
+        and_return(1)
+
+      expect(subject.restore).to be false
+    end
+
+    it "raise ::Bootloader::BootRecordBackup::Missing exception if there is not backup for device BR" do
+      allow(Yast::SCR).to receive(:Read).with(SIZE_PATH, anything).and_return(0)
+      expect{subject.restore}.to raise_error(::Bootloader::BootRecordBackup::Missing)
+    end
+  end
+
+  describe "#create" do
     before do
       allow(Yast::SCR).to receive(:Execute).with(BASH_PATH, /mkdir/)
       allow(Yast::SCR).to receive(:Execute).with(BASH_PATH, /bin\/dd/)
@@ -21,17 +46,17 @@
 
     it "store backup of device first 512 bytes to /var/log/YaST2" do
       expect(Yast::SCR).to receive(:Execute).with(BASH_PATH, /bin\/dd.* of=\/var\/log\/YaST2/)
-      subject.create_for("/dev/sda")
+      subject.create
     end
 
     it "store backup of device first 512 bytes to /var/lib/YaST2/backup_boot_sectors" do
       expect(Yast::SCR).to receive(:Execute).with(BASH_PATH, /bin\/dd.* of=\/var\/lib\/YaST2\/backup_boot_sectors/)
-      subject.create_for("/dev/sda")
+      subject.create
     end
 
     it "creates /var/lib/YaST2/backup_boot_sectors if it do not exists" do
       expect(Yast::SCR).to receive(:Execute).with(BASH_PATH, /mkdir.* \/var\/lib\/YaST2\/backup_boot_sectors/)
-      subject.create_for("/dev/sda")
+      subject.create
     end
 
     it "move old backup in backup_boot_sectors to copy with timestamp" do
@@ -40,7 +65,7 @@
       allow(Yast::SCR).to receive(:Read).with(STAT_PATH, anything).and_return({"ctime" => 200})
       expect(Yast::SCR).to receive(:Execute).with(BASH_PATH, /bin\/mv .*backup_boot_sectors.*\s+.*backup_boot_sectors/)
 
-      subject.create_for("/dev/sda")
+      subject.create
     end
 
     it "keep only ten backups in backup_boot_sectors" do
@@ -52,23 +77,22 @@
       allow(Yast::SCR).to receive(:Execute).with(BASH_PATH, /bin\/mv .*backup_boot_sectors.*\s+.*backup_boot_sectors/)
       expect(Yast::SCR).to receive(:Execute).with(Yast::Path.new(".target.remove"), /.*backup_boot_sectors.*/)
 
-      subject.create_for("/dev/sda")
+      subject.create
     end
 
-
     it "store backup of device first 512 bytes to /boot/backup_mbr if it is MBR of primary disk" do
       allow(Yast::BootCommon).to receive(:mbrDisk).and_return("/dev/sda")
       expect(Yast::SCR).to receive(:Execute).with(BASH_PATH, /bin\/dd.* of=\/boot\/backup_mbr/)
 
-      subject.create_for("/dev/sda")
+      subject.create
     end
 
     it "copy backup of device also to backup_boot_sectors with thinkpadMBR suffix if it is primary disk and contain thinkpad boot code" do
       allow(Yast::BootCommon).to receive(:mbrDisk).and_return("/dev/sda")
       allow(Yast::BootCommon).to receive(:ThinkPadMBR).and_return(true)
 
       expect(Yast::SCR).to receive(:Execute).with(BASH_PATH, /\Acp.* \/var\/lib\/YaST2\/backup_boot_sectors.*thinkpadMBR/)
-      subject.create_for("/dev/sda")
+      subject.create
     end
   end
 end