Skip to content

Commit 3751726

Browse files
gabriel-samfiragabriel-samfira
committed
Fix go.mod, simplify boolean logic, add logging
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
1 parent 600abd1 commit 3751726

2 files changed

Lines changed: 51 additions & 41 deletions

File tree

‎mount/mount_windows.go‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,17 @@
1717
package mount
1818

1919
import (
20+
"context"
2021
"encoding/json"
2122
"errors"
2223
"fmt"
2324
"os"
2425
"path/filepath"
2526
"strings"
26-
"syscall"
2727

2828
"github.com/Microsoft/go-winio/pkg/bindfilter"
2929
"github.com/Microsoft/hcsshim"
30+
"github.com/containerd/containerd/log"
3031
"golang.org/x/sys/windows"
3132
)
3233

@@ -74,16 +75,21 @@ func (m *Mount) mount(target string) error {
7475
}
7576
defer func() {
7677
if err != nil {
77-
hcsshim.DeactivateLayer(di, layerID)
78+
if layerErr := hcsshim.DeactivateLayer(di, layerID); layerErr != nil {
79+
log.G(context.TODO()).WithError(layerErr).Error("failed to deactivate layer during mount failure cleanup")
80+
}
7881
}
7982
}()
8083

8184
if err = hcsshim.PrepareLayer(di, layerID, parentLayerPaths); err != nil {
8285
return fmt.Errorf("failed to prepare layer %s: %w", m.Source, err)
8386
}
87+
8488
defer func() {
8589
if err != nil {
86-
hcsshim.UnprepareLayer(di, layerID)
90+
if layerErr := hcsshim.UnprepareLayer(di, layerID); layerErr != nil {
91+
log.G(context.TODO()).WithError(layerErr).Error("failed to unprepare layer during mount failure cleanup")
92+
}
8793
}
8894
}()
8995

@@ -97,7 +103,9 @@ func (m *Mount) mount(target string) error {
97103
}
98104
defer func() {
99105
if err != nil {
100-
bindfilter.RemoveFileBinding(target)
106+
if bindErr := bindfilter.RemoveFileBinding(target); bindErr != nil {
107+
log.G(context.TODO()).WithError(bindErr).Error("failed to remove binding during mount failure cleanup")
108+
}
101109
}
102110
}()
103111

@@ -144,7 +152,7 @@ func Unmount(mount string, flags int) error {
144152
}
145153

146154
if err := bindfilter.RemoveFileBinding(mount); err != nil {
147-
if errno, ok := errors.Unwrap(err).(syscall.Errno); ok && errno == windows.ERROR_INVALID_PARAMETER || errno == windows.ERROR_NOT_FOUND {
155+
if errors.Is(err, windows.ERROR_INVALID_PARAMETER) || errors.Is(err, windows.ERROR_NOT_FOUND) {
148156
// not a mount point
149157
return nil
150158
}

‎snapshots/windows/windows.go‎

Lines changed: 38 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
362362
// Create the new snapshot dir
363363
snDir := s.getSnapshotDir(newSnapshot.ID)
364364
if err = os.MkdirAll(snDir, 0700); err != nil {
365-
return err
365+
return fmt.Errorf("creating snapshot dir: %w", err)
366366
}
367367

368368
if strings.Contains(key, snapshots.UnpackKeyPrefix) {
@@ -372,57 +372,59 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
372372
// snapshot if this isn't the snapshot that just houses the final sandbox.vhd
373373
// that will be mounted as the containers scratch. Currently the key for a snapshot
374374
// where a layer will be extracted to will have the string `extract-` in it.
375-
} else if len(newSnapshot.ParentIDs) == 0 {
375+
return nil
376+
}
377+
378+
if len(newSnapshot.ParentIDs) == 0 {
376379
// A parentless snapshot is just a bind-mount to a directory named
377380
// "Files". When committed, there'll be some post-processing to fill in the rest
378381
// of the metadata.
379382
filesDir := filepath.Join(snDir, "Files")
380383
if err := os.MkdirAll(filesDir, 0700); err != nil {
381-
return err
384+
return fmt.Errorf("creating Files dir: %w", err)
382385
}
383-
} else {
384-
parentLayerPaths := s.parentIDsToParentPaths(newSnapshot.ParentIDs)
386+
return nil
387+
}
385388

386-
var snapshotInfo snapshots.Info
387-
for _, o := range opts {
388-
o(&snapshotInfo)
389-
}
389+
parentLayerPaths := s.parentIDsToParentPaths(newSnapshot.ParentIDs)
390+
var snapshotInfo snapshots.Info
391+
for _, o := range opts {
392+
o(&snapshotInfo)
393+
}
390394

391-
var sizeInBytes uint64
392-
if sizeGBstr, ok := snapshotInfo.Labels[rootfsSizeInGBLabel]; ok {
393-
log.G(ctx).Warnf("%q label is deprecated, please use %q instead.", rootfsSizeInGBLabel, rootfsSizeInBytesLabel)
395+
var sizeInBytes uint64
396+
if sizeGBstr, ok := snapshotInfo.Labels[rootfsSizeInGBLabel]; ok {
397+
log.G(ctx).Warnf("%q label is deprecated, please use %q instead.", rootfsSizeInGBLabel, rootfsSizeInBytesLabel)
394398

395-
sizeInGB, err := strconv.ParseUint(sizeGBstr, 10, 32)
396-
if err != nil {
397-
return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInGBLabel, sizeGBstr, err)
398-
}
399-
sizeInBytes = sizeInGB * 1024 * 1024 * 1024
399+
sizeInGB, err := strconv.ParseUint(sizeGBstr, 10, 32)
400+
if err != nil {
401+
return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInGBLabel, sizeGBstr, err)
400402
}
403+
sizeInBytes = sizeInGB * 1024 * 1024 * 1024
404+
}
401405

402-
// Prefer the newer label in bytes over the deprecated Windows specific GB variant.
403-
if sizeBytesStr, ok := snapshotInfo.Labels[rootfsSizeInBytesLabel]; ok {
404-
sizeInBytes, err = strconv.ParseUint(sizeBytesStr, 10, 64)
405-
if err != nil {
406-
return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInBytesLabel, sizeBytesStr, err)
407-
}
406+
// Prefer the newer label in bytes over the deprecated Windows specific GB variant.
407+
if sizeBytesStr, ok := snapshotInfo.Labels[rootfsSizeInBytesLabel]; ok {
408+
sizeInBytes, err = strconv.ParseUint(sizeBytesStr, 10, 64)
409+
if err != nil {
410+
return fmt.Errorf("failed to parse label %q=%q: %w", rootfsSizeInBytesLabel, sizeBytesStr, err)
408411
}
412+
}
409413

410-
var makeUVMScratch bool
411-
if _, ok := snapshotInfo.Labels[uvmScratchLabel]; ok {
412-
makeUVMScratch = true
413-
}
414+
var makeUVMScratch bool
415+
if _, ok := snapshotInfo.Labels[uvmScratchLabel]; ok {
416+
makeUVMScratch = true
417+
}
414418

415-
// This has to be run first to avoid clashing with the containers sandbox.vhdx.
416-
if makeUVMScratch {
417-
if err = s.createUVMScratchLayer(ctx, snDir, parentLayerPaths); err != nil {
418-
return fmt.Errorf("failed to make UVM's scratch layer: %w", err)
419-
}
420-
}
421-
if err = s.createScratchLayer(ctx, snDir, parentLayerPaths, sizeInBytes); err != nil {
422-
return fmt.Errorf("failed to create scratch layer: %w", err)
419+
// This has to be run first to avoid clashing with the containers sandbox.vhdx.
420+
if makeUVMScratch {
421+
if err = s.createUVMScratchLayer(ctx, snDir, parentLayerPaths); err != nil {
422+
return fmt.Errorf("failed to make UVM's scratch layer: %w", err)
423423
}
424424
}
425-
425+
if err = s.createScratchLayer(ctx, snDir, parentLayerPaths, sizeInBytes); err != nil {
426+
return fmt.Errorf("failed to create scratch layer: %w", err)
427+
}
426428
return nil
427429
})
428430
if err != nil {

0 commit comments

Comments
 (0)