Merge pull request #26842 from jstarks/fix_save

Windows: Back up files mutated during layer import

daemon/graphdriver/windows/windows.go

@@ -31,7 +31,6 @@ import (
 	"github.com/docker/docker/pkg/longpath"
 	"github.com/docker/docker/pkg/reexec"
 	"github.com/docker/go-units"
-	"github.com/vbatts/tar-split/tar/storage"
 )
 
 // filterDriver is an HCSShim driver type for the Windows Filter driver.
@@ -41,6 +40,15 @@ var (
 	vmcomputedll            = syscall.NewLazyDLL("vmcompute.dll")
 	hcsExpandSandboxSize    = vmcomputedll.NewProc("ExpandSandboxSize")
 	hcsSandboxSizeSupported = hcsExpandSandboxSize.Find() == nil
+
+	// mutatedFiles is a list of files that are mutated by the import process
+	// and must be backed up and restored.
+	mutatedFiles = map[string]string{
+		"UtilityVM/Files/EFI/Microsoft/Boot/BCD":      "bcd.bak",
+		"UtilityVM/Files/EFI/Microsoft/Boot/BCD.LOG":  "bcd.log.bak",
+		"UtilityVM/Files/EFI/Microsoft/Boot/BCD.LOG1": "bcd.log1.bak",
+		"UtilityVM/Files/EFI/Microsoft/Boot/BCD.LOG2": "bcd.log2.bak",
+	}
 )
 
 // init registers the windows graph drivers to the register.
@@ -532,7 +540,48 @@ func (d *Driver) exportLayer(id string, parentLayerPaths []string) (archive.Arch
 	return archive, nil
 }
 
-func writeLayerFromTar(r archive.Reader, w hcsshim.LayerWriter) (int64, error) {
+// writeBackupStreamFromTarAndSaveMutatedFiles reads data from a tar stream and
+// writes it to a backup stream, and also saves any files that will be mutated
+// by the import layer process to a backup location.
+func writeBackupStreamFromTarAndSaveMutatedFiles(buf *bufio.Writer, w io.Writer, t *tar.Reader, hdr *tar.Header, root string) (nextHdr *tar.Header, err error) {
+	var bcdBackup *os.File
+	var bcdBackupWriter *winio.BackupFileWriter
+	if backupPath, ok := mutatedFiles[hdr.Name]; ok {
+		bcdBackup, err = os.Create(filepath.Join(root, backupPath))
+		if err != nil {
+			return nil, err
+		}
+		defer func() {
+			cerr := bcdBackup.Close()
+			if err == nil {
+				err = cerr
+			}
+		}()
+
+		bcdBackupWriter = winio.NewBackupFileWriter(bcdBackup, false)
+		defer func() {
+			cerr := bcdBackupWriter.Close()
+			if err == nil {
+				err = cerr
+			}
+		}()
+
+		buf.Reset(io.MultiWriter(w, bcdBackupWriter))
+	} else {
+		buf.Reset(w)
+	}
+
+	defer func() {
+		ferr := buf.Flush()
+		if err == nil {
+			err = ferr
+		}
+	}()
+
+	return backuptar.WriteBackupStreamFromTarFile(buf, t, hdr)
+}
+
+func writeLayerFromTar(r archive.Reader, w hcsshim.LayerWriter, root string) (int64, error) {
 	t := tar.NewReader(r)
 	hdr, err := t.Next()
 	totalSize := int64(0)
@@ -566,13 +615,7 @@ func writeLayerFromTar(r archive.Reader, w hcsshim.LayerWriter) (int64, error) {
 			if err != nil {
 				return 0, err
 			}
-			buf.Reset(w)
+			hdr, err = writeBackupStreamFromTarAndSaveMutatedFiles(buf, w, t, hdr, root)
-
-			hdr, err = backuptar.WriteBackupStreamFromTarFile(buf, t, hdr)
-			ferr := buf.Flush()
-			if ferr != nil {
-				err = ferr
-			}
 			totalSize += size
 		}
 	}
@@ -582,46 +625,6 @@ func writeLayerFromTar(r archive.Reader, w hcsshim.LayerWriter) (int64, error) {
 	return totalSize, nil
 }
 
-func addAceToSddlDacl(sddl, ace string) (string, bool) {
-	daclStart := strings.Index(sddl, "D:")
-	if daclStart < 0 {
-		return sddl, false
-	}
-
-	dacl := sddl[daclStart:]
-	daclEnd := strings.Index(dacl, "S:")
-	if daclEnd < 0 {
-		daclEnd = len(dacl)
-	}
-	dacl = dacl[:daclEnd]
-
-	if strings.Contains(dacl, ace) {
-		return sddl, true
-	}
-
-	i := 2
-	for i+1 < len(dacl) {
-		if dacl[i] != '(' {
-			return sddl, false
-		}
-
-		if dacl[i+1] == 'A' {
-			break
-		}
-
-		i += 2
-		for p := 1; i < len(dacl) && p > 0; i++ {
-			if dacl[i] == '(' {
-				p++
-			} else if dacl[i] == ')' {
-				p--
-			}
-		}
-	}
-
-	return sddl[:daclStart+i] + ace + sddl[daclStart+i:], true
-}
-
 // importLayer adds a new layer to the tag and graph store based on the given data.
 func (d *Driver) importLayer(id string, layerData archive.Reader, parentLayerPaths []string) (size int64, err error) {
 	cmd := reexec.Command(append([]string{"docker-windows-write-layer", d.info.HomeDir, id}, parentLayerPaths...)...)
@@ -663,7 +666,7 @@ func writeLayer() {
 			return err
 		}
 
-		size, err := writeLayerFromTar(os.Stdin, w)
+		size, err := writeLayerFromTar(os.Stdin, w, filepath.Join(home, id))
 		if err != nil {
 			return err
 		}
@@ -743,6 +746,10 @@ type fileGetCloserWithBackupPrivileges struct {
 }
 
 func (fg *fileGetCloserWithBackupPrivileges) Get(filename string) (io.ReadCloser, error) {
+	if backupPath, ok := mutatedFiles[filename]; ok {
+		return os.Open(filepath.Join(fg.path, backupPath))
+	}
+
 	var f *os.File
 	// Open the file while holding the Windows backup privilege. This ensures that the
 	// file can be opened even if the caller does not actually have access to it according
@@ -767,16 +774,6 @@ func (fg *fileGetCloserWithBackupPrivileges) Close() error {
 	return nil
 }
 
-type fileGetDestroyCloser struct {
-	storage.FileGetter
-	path string
-}
-
-func (f *fileGetDestroyCloser) Close() error {
-	// TODO: activate layers and release here?
-	return os.RemoveAll(f.path)
-}
-
 // DiffGetter returns a FileGetCloser that can read files from the directory that
 // contains files for the layer differences. Used for direct access for tar-split.
 func (d *Driver) DiffGetter(id string) (graphdriver.FileGetCloser, error) {

daemon/graphdriver/windows/windows_windows_test.go

@@ -1,18 +0,0 @@
-package windows
-
-import "testing"
-
-func TestAddAceToSddlDacl(t *testing.T) {
-	cases := [][3]string{
-		{"D:", "(A;;;)", "D:(A;;;)"},
-		{"D:(A;;;)", "(A;;;)", "D:(A;;;)"},
-		{"O:D:(A;;;stuff)", "(A;;;new)", "O:D:(A;;;new)(A;;;stuff)"},
-		{"O:D:(D;;;no)(A;;;stuff)", "(A;;;new)", "O:D:(D;;;no)(A;;;new)(A;;;stuff)"},
-	}
-
-	for _, c := range cases {
-		if newSddl, worked := addAceToSddlDacl(c[0], c[1]); !worked || newSddl != c[2] {
-			t.Errorf("%s + %s == %s, expected %s (%v)", c[0], c[1], newSddl, c[2], worked)
-		}
-	}
-}