From ee0f9930783e1ef42be52ffd61ee6945348a9331 Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Thu, 21 Oct 2021 09:31:23 -0700 Subject: [PATCH 1/2] Pass disk handle for FormatWritableLayerVhd on RS5 On RS5 the HcsFormatWritableLayerVhd call expects to receive a disk handle. On 19h1+ you can pass a vhd handle and internally they will do the same work we're doing in this change to grab a disk handle to perform the format. Signed-off-by: Daniel Canter --- computestorage/format.go | 54 ++++++++++++++++++- .../hcsshim/computestorage/format.go | 54 ++++++++++++++++++- 2 files changed, 104 insertions(+), 4 deletions(-) diff --git a/computestorage/format.go b/computestorage/format.go index 83c0fa33f0..7c0d9e4294 100644 --- a/computestorage/format.go +++ b/computestorage/format.go @@ -2,13 +2,41 @@ package computestorage import ( "context" + "os" + "syscall" + "github.com/Microsoft/go-winio/vhd" "github.com/Microsoft/hcsshim/internal/oc" + "github.com/Microsoft/hcsshim/osversion" "github.com/pkg/errors" "go.opencensus.io/trace" "golang.org/x/sys/windows" ) +func openDisk(path string) (windows.Handle, error) { + u16, err := windows.UTF16PtrFromString(path) + if err != nil { + return 0, err + } + h, err := windows.CreateFile( + u16, + windows.GENERIC_READ|windows.GENERIC_WRITE, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE, + nil, + windows.OPEN_EXISTING, + windows.FILE_ATTRIBUTE_NORMAL|windows.FILE_FLAG_NO_BUFFERING, + 0, + ) + if err != nil { + return 0, &os.PathError{ + Op: "CreateFile", + Path: path, + Err: err, + } + } + return h, nil +} + // FormatWritableLayerVhd formats a virtual disk for use as a writable container layer. // // If the VHD is not mounted it will be temporarily mounted. @@ -18,9 +46,31 @@ func FormatWritableLayerVhd(ctx context.Context, vhdHandle windows.Handle) (err defer span.End() defer func() { oc.SetSpanStatus(span, err) }() - err = hcsFormatWritableLayerVhd(vhdHandle) + h := vhdHandle + // On RS5 HcsFormatWritableLayerVhd expects to receive a disk handle instead of a vhd handle. + if osversion.Build() < osversion.V19H1 { + if err := vhd.AttachVirtualDisk(syscall.Handle(vhdHandle), vhd.AttachVirtualDiskFlagNone, &vhd.AttachVirtualDiskParameters{Version: 1}); err != nil { + return err + } + defer func() { + if detachErr := vhd.DetachVirtualDisk(syscall.Handle(vhdHandle)); err != nil { + err = detachErr + } + }() + diskPath, err := vhd.GetVirtualDiskPhysicalPath(syscall.Handle(vhdHandle)) + if err != nil { + return err + } + diskHandle, err := openDisk(diskPath) + if err != nil { + return err + } + defer windows.CloseHandle(diskHandle) // nolint: errcheck + h = diskHandle + } + err = hcsFormatWritableLayerVhd(h) if err != nil { return errors.Wrap(err, "failed to format writable layer vhd") } - return nil + return } diff --git a/test/vendor/github.com/Microsoft/hcsshim/computestorage/format.go b/test/vendor/github.com/Microsoft/hcsshim/computestorage/format.go index 83c0fa33f0..7c0d9e4294 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/computestorage/format.go +++ b/test/vendor/github.com/Microsoft/hcsshim/computestorage/format.go @@ -2,13 +2,41 @@ package computestorage import ( "context" + "os" + "syscall" + "github.com/Microsoft/go-winio/vhd" "github.com/Microsoft/hcsshim/internal/oc" + "github.com/Microsoft/hcsshim/osversion" "github.com/pkg/errors" "go.opencensus.io/trace" "golang.org/x/sys/windows" ) +func openDisk(path string) (windows.Handle, error) { + u16, err := windows.UTF16PtrFromString(path) + if err != nil { + return 0, err + } + h, err := windows.CreateFile( + u16, + windows.GENERIC_READ|windows.GENERIC_WRITE, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE, + nil, + windows.OPEN_EXISTING, + windows.FILE_ATTRIBUTE_NORMAL|windows.FILE_FLAG_NO_BUFFERING, + 0, + ) + if err != nil { + return 0, &os.PathError{ + Op: "CreateFile", + Path: path, + Err: err, + } + } + return h, nil +} + // FormatWritableLayerVhd formats a virtual disk for use as a writable container layer. // // If the VHD is not mounted it will be temporarily mounted. @@ -18,9 +46,31 @@ func FormatWritableLayerVhd(ctx context.Context, vhdHandle windows.Handle) (err defer span.End() defer func() { oc.SetSpanStatus(span, err) }() - err = hcsFormatWritableLayerVhd(vhdHandle) + h := vhdHandle + // On RS5 HcsFormatWritableLayerVhd expects to receive a disk handle instead of a vhd handle. + if osversion.Build() < osversion.V19H1 { + if err := vhd.AttachVirtualDisk(syscall.Handle(vhdHandle), vhd.AttachVirtualDiskFlagNone, &vhd.AttachVirtualDiskParameters{Version: 1}); err != nil { + return err + } + defer func() { + if detachErr := vhd.DetachVirtualDisk(syscall.Handle(vhdHandle)); err != nil { + err = detachErr + } + }() + diskPath, err := vhd.GetVirtualDiskPhysicalPath(syscall.Handle(vhdHandle)) + if err != nil { + return err + } + diskHandle, err := openDisk(diskPath) + if err != nil { + return err + } + defer windows.CloseHandle(diskHandle) // nolint: errcheck + h = diskHandle + } + err = hcsFormatWritableLayerVhd(h) if err != nil { return errors.Wrap(err, "failed to format writable layer vhd") } - return nil + return } From 44b7ee9cb2a94d046f6990f0108516a66b0f9967 Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Thu, 21 Oct 2021 16:34:27 -0700 Subject: [PATCH 2/2] PR feedback #1 Fix DetachVHD error check and assignment. Signed-off-by: Daniel Canter --- computestorage/format.go | 2 +- .../github.com/Microsoft/hcsshim/computestorage/format.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/computestorage/format.go b/computestorage/format.go index 7c0d9e4294..61d8d5a634 100644 --- a/computestorage/format.go +++ b/computestorage/format.go @@ -53,7 +53,7 @@ func FormatWritableLayerVhd(ctx context.Context, vhdHandle windows.Handle) (err return err } defer func() { - if detachErr := vhd.DetachVirtualDisk(syscall.Handle(vhdHandle)); err != nil { + if detachErr := vhd.DetachVirtualDisk(syscall.Handle(vhdHandle)); err == nil && detachErr != nil { err = detachErr } }() diff --git a/test/vendor/github.com/Microsoft/hcsshim/computestorage/format.go b/test/vendor/github.com/Microsoft/hcsshim/computestorage/format.go index 7c0d9e4294..61d8d5a634 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/computestorage/format.go +++ b/test/vendor/github.com/Microsoft/hcsshim/computestorage/format.go @@ -53,7 +53,7 @@ func FormatWritableLayerVhd(ctx context.Context, vhdHandle windows.Handle) (err return err } defer func() { - if detachErr := vhd.DetachVirtualDisk(syscall.Handle(vhdHandle)); err != nil { + if detachErr := vhd.DetachVirtualDisk(syscall.Handle(vhdHandle)); err == nil && detachErr != nil { err = detachErr } }()