ronantakizawa/codereview-bench huggingface dataset:
Repository: containerd/containerd
File: integration/client/export_test.go (Go)

Code before:
				if isImageInArchive(ctx, t, client, dstFile, defaultPlatformManifest) {
					t.Fatal("archive shouldn't contain manifest for the default platform")
				}
			},
		},
	} {
		t.Run(tc.name, func(t *testing.T) {
			t.Parallel()
			ctx, cancel := testContext(t)
			defer cancel()

			namespace := uuid.New().String()
			client, err := newClient(t, address, WithDefaultNamespace(namespace))
			if err != nil {
				t.Fatal(err)
			}
			defer client.Close()

			ctx = namespaces.WithNamespace(ctx, namespace)

			img := tc.prepare(ctx, t, client)

			dstFile, err := os.CreateTemp(t.TempDir(), "export-test")
			if err != nil {
				t.Fatal(err)
			}
			defer func() {
				dstFile.Close()
				os.Remove(dstFile.Name())
			}()

			tc.check(ctx, t, client, dstFile, img)
		})
	}
}

func isImageInArchive(ctx context.Context, t *testing.T, client *Client, dstFile *os.File, mfst ocispec.Descriptor) bool {
	dstFile.Seek(0, 0)
	tr := tar.NewReader(dstFile)

	var blobs []string
	for {
		h, err := tr.Next()
		if err != nil {
			if err == io.EOF {
				break
			}
			t.Fatal(err)
		}

		digest := strings.TrimPrefix(h.Name, "blobs/sha256/")

Review comment (suggestion):
(the actual line is too far outside the diff for GitHub to let me comment)

Since switching to `t.TempDir` we do not need an explicit `defer` for cleanup.  Please remove the `defer` lines 257-260.

Code after:

				// Check if archive contains only one manifest for the default platform
				if isImageInArchive(ctx, t, client, dstFile, defaultPlatformManifest) {
					t.Fatal("archive shouldn't contain manifest for the default platform")
				}
			},
		},
	} {
		t.Run(tc.name, func(t *testing.T) {
			t.Parallel()
			ctx, cancel := testContext(t)
			defer cancel()

			namespace := uuid.New().String()
			client, err := newClient(t, address, WithDefaultNamespace(namespace))
			require.NoError(t, err)
			t.Cleanup(func() {
				client.Close()
			})

			ctx = namespaces.WithNamespace(ctx, namespace)

			img := tc.prepare(ctx, t, client)

			dstFile, err := os.Create(filepath.Join(t.TempDir(), "export-test"))
			require.NoError(t, err)
			t.Cleanup(func() {
				dstFile.Close()
			})

			tc.check(ctx, t, client, dstFile, img)
		})
	}
}

func isImageInArchive(ctx context.Context, t *testing.T, client *Client, dstFile *os.File, mfst ocispec.Descriptor) bool {
	dstFile.Seek(0, 0)
	tr := tar.NewReader(dstFile)

	var blobs []string
	for {
		h, err := tr.Next()
		if err != nil {
			if err == io.EOF {
				break
			}
			t.Fatal(err)
		}

		digest := strings.TrimPrefix(h.Name, "blobs/sha256/")
		if digest != h.Name && digest != "" {
Repository: containerd/containerd
File: plugins/snapshots/devmapper/config_test.go (Go)

Code before:
   Unless required by applicable law or agreed to in writing, software
   distributed under the License is distributed on an "AS IS" BASIS,
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
*/

package devmapper

import (
	"os"
	"path/filepath"
	"testing"

	"github.com/pelletier/go-toml/v2"
	"github.com/stretchr/testify/assert"
)

func TestLoadConfig(t *testing.T) {
	expected := Config{
		RootPath:      "/tmp",
		PoolName:      "test",
		BaseImageSize: "128Mb",
	}

	file, err := os.Create(filepath.Join(t.TempDir(), "devmapper-config-"))
	assert.NoError(t, err)
	t.Cleanup(func() {
		file.Close()
	})

	encoder := toml.NewEncoder(file)
	err = encoder.Encode(&expected)
	assert.NoError(t, err)

	loaded, err := LoadConfig(file.Name())
	assert.NoError(t, err)

	assert.Equal(t, loaded.RootPath, expected.RootPath)
	assert.Equal(t, loaded.PoolName, expected.PoolName)
	assert.Equal(t, loaded.BaseImageSize, expected.BaseImageSize)
	assert.True(t, loaded.BaseImageSizeBytes == 128*1024*1024)
}

func TestLoadConfigInvalidPath(t *testing.T) {
	_, err := LoadConfig("")
	assert.Equal(t, os.ErrNotExist, err)

	_, err = LoadConfig("/dev/null")
	assert.NotNil(t, err)
}

Review comment (suggestion):
```suggestion
	file, err := os.CreateTemp(t.TempDir(), "devmapper-config-")
```

Code after:
   Unless required by applicable law or agreed to in writing, software
   distributed under the License is distributed on an "AS IS" BASIS,
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
*/

package devmapper

import (
	"os"
	"testing"

	"github.com/pelletier/go-toml/v2"
	"github.com/stretchr/testify/assert"
)

func TestLoadConfig(t *testing.T) {
	expected := Config{
		RootPath:      "/tmp",
		PoolName:      "test",
		BaseImageSize: "128Mb",
	}

	file, err := os.CreateTemp(t.TempDir(), "devmapper-config-")
	assert.NoError(t, err)
	t.Cleanup(func() {
		file.Close()
	})

	encoder := toml.NewEncoder(file)
	err = encoder.Encode(&expected)
	assert.NoError(t, err)

	loaded, err := LoadConfig(file.Name())
	assert.NoError(t, err)

	assert.Equal(t, loaded.RootPath, expected.RootPath)
	assert.Equal(t, loaded.PoolName, expected.PoolName)
	assert.Equal(t, loaded.BaseImageSize, expected.BaseImageSize)
	assert.True(t, loaded.BaseImageSizeBytes == 128*1024*1024)
}

func TestLoadConfigInvalidPath(t *testing.T) {
	_, err := LoadConfig("")
	assert.Equal(t, os.ErrNotExist, err)

	_, err = LoadConfig("/dev/null")
	assert.NotNil(t, err)
}
Repository: containerd/containerd
File: internal/cri/server/podsandbox/helpers_test.go (Go)

Code before:
				}
			}
			for _, kv := range test.kv {
				oci.WithEnv([]string{kv[0] + "=" + kv[1]})(context.Background(), nil, nil, &spec)
			}
			assert.Equal(t, test.expected, spec.Process.Env)
		})
	}
}

func TestEnsureRemoveAllNotExist(t *testing.T) {
	// should never return an error for a non-existent path
	if err := ensureRemoveAll(context.Background(), "/non/existent/path"); err != nil {
		t.Fatal(err)
	}
}

func TestEnsureRemoveAllWithDir(t *testing.T) {
	dir := t.TempDir()
	if err := ensureRemoveAll(context.Background(), dir); err != nil {
		t.Fatal(err)
	}
}

func TestEnsureRemoveAllWithFile(t *testing.T) {
	tmp, err := os.CreateTemp("", "test-ensure-removeall-with-dir") //nolint:usetesting
	require.NoError(t, err)
	tmp.Close()

	err = ensureRemoveAll(context.Background(), tmp.Name())
	require.NoError(t, err)
}


Review comment (suggestion):
```suggestion
	tmp, err := os.CreateTemp(t.TempDir(), "test-ensure-removeall-with-dir") //nolint:usetesting
```

Code after:
				}
			}
			for _, kv := range test.kv {
				oci.WithEnv([]string{kv[0] + "=" + kv[1]})(context.Background(), nil, nil, &spec)
			}
			assert.Equal(t, test.expected, spec.Process.Env)
		})
	}
}

func TestEnsureRemoveAllNotExist(t *testing.T) {
	// should never return an error for a non-existent path
	if err := ensureRemoveAll(context.Background(), "/non/existent/path"); err != nil {
		t.Fatal(err)
	}
}

func TestEnsureRemoveAllWithDir(t *testing.T) {
	dir := t.TempDir()
	if err := ensureRemoveAll(context.Background(), dir); err != nil {
		t.Fatal(err)
	}
}

func TestEnsureRemoveAllWithFile(t *testing.T) {
	tmp, err := os.CreateTemp(t.TempDir(), "test-ensure-removeall-with-dir")
	require.NoError(t, err)
	tmp.Close()

	err = ensureRemoveAll(context.Background(), tmp.Name())
	require.NoError(t, err)
}
Repository: containerd/containerd
File: internal/cri/server/helpers_test.go (Go)

Code before:
				}
			}
			for _, kv := range test.kv {
				oci.WithEnv([]string{kv[0] + "=" + kv[1]})(context.Background(), nil, nil, &spec)
			}
			assert.Equal(t, test.expected, spec.Process.Env)
		})
	}
}

func TestEnsureRemoveAllNotExist(t *testing.T) {
	// should never return an error for a non-existent path
	if err := ensureRemoveAll(context.Background(), "/non/existent/path"); err != nil {
		t.Fatal(err)
	}
}

func TestEnsureRemoveAllWithDir(t *testing.T) {
	dir := t.TempDir()
	if err := ensureRemoveAll(context.Background(), dir); err != nil {
		t.Fatal(err)
	}
}

func TestEnsureRemoveAllWithFile(t *testing.T) {
	tmp, err := os.CreateTemp("", "test-ensure-removeall-with-dir") //nolint:usetesting
	require.NoError(t, err)
	tmp.Close()

	err = ensureRemoveAll(context.Background(), tmp.Name())
	require.NoError(t, err)
}

// Helper function for setting up an environment to test PID namespace targeting.
func addContainer(c *criService, containerID, sandboxID string, PID uint32, createdAt, startedAt, finishedAt int64) error {
	meta := containerstore.Metadata{
		ID:        containerID,
		SandboxID: sandboxID,
	}
	status := containerstore.Status{
		Pid:        PID,
		CreatedAt:  createdAt,
		StartedAt:  startedAt,
		FinishedAt: finishedAt,
	}
	container, err := containerstore.NewContainer(meta,
		containerstore.WithFakeStatus(status),
	)
	if err != nil {
		return err
	}

Review comment (suggestion):
```suggestion
	tmp, err := os.CreateTemp(t.TempDir(), "test-ensure-removeall-with-dir") //nolint:usetesting
```

Code after:
				}
			}
			for _, kv := range test.kv {
				oci.WithEnv([]string{kv[0] + "=" + kv[1]})(context.Background(), nil, nil, &spec)
			}
			assert.Equal(t, test.expected, spec.Process.Env)
		})
	}
}

func TestEnsureRemoveAllNotExist(t *testing.T) {
	// should never return an error for a non-existent path
	if err := ensureRemoveAll(context.Background(), "/non/existent/path"); err != nil {
		t.Fatal(err)
	}
}

func TestEnsureRemoveAllWithDir(t *testing.T) {
	dir := t.TempDir()
	if err := ensureRemoveAll(context.Background(), dir); err != nil {
		t.Fatal(err)
	}
}

func TestEnsureRemoveAllWithFile(t *testing.T) {
	tmp, err := os.CreateTemp(t.TempDir(), "test-ensure-removeall-with-dir")
	require.NoError(t, err)
	tmp.Close()

	err = ensureRemoveAll(context.Background(), tmp.Name())
	require.NoError(t, err)
}

// Helper function for setting up an environment to test PID namespace targeting.
func addContainer(c *criService, containerID, sandboxID string, PID uint32, createdAt, startedAt, finishedAt int64) error {
	meta := containerstore.Metadata{
		ID:        containerID,
		SandboxID: sandboxID,
	}
	status := containerstore.Status{
		Pid:        PID,
		CreatedAt:  createdAt,
		StartedAt:  startedAt,
		FinishedAt: finishedAt,
	}
	container, err := containerstore.NewContainer(meta,
		containerstore.WithFakeStatus(status),
	)
	if err != nil {
		return err
	}
Repository: containerd/containerd
File: plugins/diff/erofs/differ_linux.go (Go)

Code before:
		native = true
	} else if _, err := images.DiffCompression(ctx, desc.MediaType); err != nil {
		return emptyDesc, fmt.Errorf("currently unsupported media type: %s", desc.MediaType)
	}

	var config diff.ApplyConfig
	for _, o := range opts {
		if err := o(ctx, desc, &config); err != nil {
			return emptyDesc, fmt.Errorf("failed to apply config opt: %w", err)
		}
	}

	layer, err := erofsutils.MountsToLayer(mounts)
	if err != nil {
		return emptyDesc, err
	}

	ra, err := s.store.ReaderAt(ctx, desc)
	if err != nil {
		return emptyDesc, fmt.Errorf("failed to get reader from content store: %w", err)
	}
	defer ra.Close()

	layerBlobPath := path.Join(layer, "layer.erofs")
	if native {
		// TODO: prefer to use hardlink as an optimization here
		f, err := os.Create(layerBlobPath)
		if err != nil {
			return emptyDesc, err
		}
		_, err = io.Copy(f, content.NewReader(ra))
		if err != nil {
			return emptyDesc, err
		}
		return desc, nil
	}

	processor := diff.NewProcessorChain(desc.MediaType, content.NewReader(ra))
	for {
		if processor, err = diff.GetProcessor(ctx, processor, config.ProcessorPayloads); err != nil {
			return emptyDesc, fmt.Errorf("failed to get stream processor for %s: %w", desc.MediaType, err)
		}
		if processor.MediaType() == ocispec.MediaTypeImageLayer {
			break
		}
	}
	defer processor.Close()

	digester := digest.Canonical.Digester()
	rc := &readCounter{
		r: io.TeeReader(processor, digester.Hash()),

Review comment (performance):
`io.Copy` in this case will usually do a `copy_file_range` which is better in this when reflinking is supported. We can also optimize this for the mac case to clone. I think we can remove this comment

Code after:
		native = true
	} else if _, err := images.DiffCompression(ctx, desc.MediaType); err != nil {
		return emptyDesc, fmt.Errorf("currently unsupported media type: %s", desc.MediaType)
	}

	var config diff.ApplyConfig
	for _, o := range opts {
		if err := o(ctx, desc, &config); err != nil {
			return emptyDesc, fmt.Errorf("failed to apply config opt: %w", err)
		}
	}

	layer, err := erofsutils.MountsToLayer(mounts)
	if err != nil {
		return emptyDesc, err
	}

	ra, err := s.store.ReaderAt(ctx, desc)
	if err != nil {
		return emptyDesc, fmt.Errorf("failed to get reader from content store: %w", err)
	}
	defer ra.Close()

	layerBlobPath := path.Join(layer, "layer.erofs")
	if native {
		f, err := os.Create(layerBlobPath)
		if err != nil {
			return emptyDesc, err
		}
		_, err = io.Copy(f, content.NewReader(ra))
		f.Close()
		if err != nil {
			return emptyDesc, err
		}
		return desc, nil
	}

	processor := diff.NewProcessorChain(desc.MediaType, content.NewReader(ra))
	for {
		if processor, err = diff.GetProcessor(ctx, processor, config.ProcessorPayloads); err != nil {
			return emptyDesc, fmt.Errorf("failed to get stream processor for %s: %w", desc.MediaType, err)
		}
		if processor.MediaType() == ocispec.MediaTypeImageLayer {
			break
		}
	}
	defer processor.Close()

	digester := digest.Canonical.Digester()
	rc := &readCounter{
		r: io.TeeReader(processor, digester.Hash()),
Repository: containerd/containerd
File: plugins/diff/erofs/differ_linux.go (Go)

Code before:

	var config diff.ApplyConfig
	for _, o := range opts {
		if err := o(ctx, desc, &config); err != nil {
			return emptyDesc, fmt.Errorf("failed to apply config opt: %w", err)
		}
	}

	layer, err := erofsutils.MountsToLayer(mounts)
	if err != nil {
		return emptyDesc, err
	}

	ra, err := s.store.ReaderAt(ctx, desc)
	if err != nil {
		return emptyDesc, fmt.Errorf("failed to get reader from content store: %w", err)
	}
	defer ra.Close()

	layerBlobPath := path.Join(layer, "layer.erofs")
	if native {
		// TODO: prefer to use hardlink as an optimization here
		f, err := os.Create(layerBlobPath)
		if err != nil {
			return emptyDesc, err
		}
		_, err = io.Copy(f, content.NewReader(ra))
		if err != nil {
			return emptyDesc, err
		}
		return desc, nil
	}

	processor := diff.NewProcessorChain(desc.MediaType, content.NewReader(ra))
	for {
		if processor, err = diff.GetProcessor(ctx, processor, config.ProcessorPayloads); err != nil {
			return emptyDesc, fmt.Errorf("failed to get stream processor for %s: %w", desc.MediaType, err)
		}
		if processor.MediaType() == ocispec.MediaTypeImageLayer {
			break
		}
	}
	defer processor.Close()

	digester := digest.Canonical.Digester()
	rc := &readCounter{
		r: io.TeeReader(processor, digester.Hash()),
	}

	err = erofsutils.ConvertTarErofs(ctx, rc, layerBlobPath, s.mkfsExtraOpts)
	if err != nil {

Review comment (suggestion):
`f` should be closed unconditionally after this call

Code after:

	var config diff.ApplyConfig
	for _, o := range opts {
		if err := o(ctx, desc, &config); err != nil {
			return emptyDesc, fmt.Errorf("failed to apply config opt: %w", err)
		}
	}

	layer, err := erofsutils.MountsToLayer(mounts)
	if err != nil {
		return emptyDesc, err
	}

	ra, err := s.store.ReaderAt(ctx, desc)
	if err != nil {
		return emptyDesc, fmt.Errorf("failed to get reader from content store: %w", err)
	}
	defer ra.Close()

	layerBlobPath := path.Join(layer, "layer.erofs")
	if native {
		f, err := os.Create(layerBlobPath)
		if err != nil {
			return emptyDesc, err
		}
		_, err = io.Copy(f, content.NewReader(ra))
		f.Close()
		if err != nil {
			return emptyDesc, err
		}
		return desc, nil
	}

	processor := diff.NewProcessorChain(desc.MediaType, content.NewReader(ra))
	for {
		if processor, err = diff.GetProcessor(ctx, processor, config.ProcessorPayloads); err != nil {
			return emptyDesc, fmt.Errorf("failed to get stream processor for %s: %w", desc.MediaType, err)
		}
		if processor.MediaType() == ocispec.MediaTypeImageLayer {
			break
		}
	}
	defer processor.Close()

	digester := digest.Canonical.Digester()
	rc := &readCounter{
		r: io.TeeReader(processor, digester.Hash()),
	}

	err = erofsutils.ConvertTarErofs(ctx, rc, layerBlobPath, s.mkfsExtraOpts)
	if err != nil {
Repository: containerd/containerd
File: plugins/transfer/plugin.go (Go)

Code before:
	MaxConcurrentDownloads int `toml:"max_concurrent_downloads"`

	// MaxConcurrentUploadedLayers is the max concurrent uploads for push
	MaxConcurrentUploadedLayers int `toml:"max_concurrent_uploaded_layers"`

	// UnpackConfiguration is used to read config from toml
	UnpackConfiguration []unpackConfiguration `toml:"unpack_config,omitempty"`

	// RegistryConfigPath is a path to the root directory containing registry-specific configurations
	RegistryConfigPath string `toml:"config_path"`
}

type unpackConfiguration struct {
	// Platform is the target unpack platform to match
	Platform string `toml:"platform"`

	// Snapshotter is the snapshotter to use to unpack
	Snapshotter string `toml:"snapshotter"`

	// Differ is the diff plugin to be used for apply
	Differ string `toml:"differ"`

	// ConfigType is the config types for this unpack configuration
	ConfigType string `toml:"config_type"`

	// ConfigTypes are the allowed layer types for this unpack configuration
	LayerTypes []string `toml:"layer_types"`
}

func defaultConfig() *transferConfig {
	return &transferConfig{
		MaxConcurrentDownloads:      3,
		MaxConcurrentUploadedLayers: 3,
	}
}


Review comment (suggestion):
```suggestion
	// LayerTypes are the allowed layer types for this unpack configuration
```

Code after:
	MaxConcurrentDownloads int `toml:"max_concurrent_downloads"`

	// MaxConcurrentUploadedLayers is the max concurrent uploads for push
	MaxConcurrentUploadedLayers int `toml:"max_concurrent_uploaded_layers"`

	// UnpackConfiguration is used to read config from toml
	UnpackConfiguration []unpackConfiguration `toml:"unpack_config,omitempty"`

	// RegistryConfigPath is a path to the root directory containing registry-specific configurations
	RegistryConfigPath string `toml:"config_path"`
}

type unpackConfiguration struct {
	// Platform is the target unpack platform to match
	Platform string `toml:"platform"`

	// Snapshotter is the snapshotter to use to unpack
	Snapshotter string `toml:"snapshotter"`

	// Differ is the diff plugin to be used for apply
	Differ string `toml:"differ"`

	// ConfigType is the config types for this unpack configuration
	ConfigType string `toml:"config_type"`

	// LayerTypes are the allowed layer types for this unpack configuration
	LayerTypes []string `toml:"layer_types"`
}

func defaultConfig() *transferConfig {
	return &transferConfig{
		MaxConcurrentDownloads:      3,
		MaxConcurrentUploadedLayers: 3,
	}
}
Repository: containerd/containerd
File: pkg/oci/spec_opts_nonlinux.go (Go)

Code before:
       http://www.apache.org/licenses/LICENSE-2.0

   Unless required by applicable law or agreed to in writing, software
   distributed under the License is distributed on an "AS IS" BASIS,
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
*/

package oci

import (
	"context"

	"github.com/containerd/containerd/v2/core/containers"
)

// WithAllCurrentCapabilities propagates the effective capabilities of the caller process to the container process.
// The capability set may differ from WithAllKnownCapabilities when running in a container.
var WithAllCurrentCapabilities = func(ctx context.Context, client Client, c *containers.Container, s *Spec) error {
	return WithCapabilities(nil)(ctx, client, c, s)
}

// WithAllKnownCapabilities sets all the known linux capabilities for the container process
var WithAllKnownCapabilities = func(ctx context.Context, client Client, c *containers.Container, s *Spec) error {
	return WithCapabilities(nil)(ctx, client, c, s)
}


Review comment (suggestion):
Wondering even if these should just be a no-op, or if we should create stubs for `cap.Current()` and `cap.Known()`, but keeping that for a future exercise for now.

Code after:
       http://www.apache.org/licenses/LICENSE-2.0

   Unless required by applicable law or agreed to in writing, software
   distributed under the License is distributed on an "AS IS" BASIS,
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
*/

package oci

import (
	"context"

	"github.com/containerd/containerd/v2/core/containers"
)

// WithAllCurrentCapabilities propagates the effective capabilities of the caller process to the container process.
// The capability set may differ from [WithAllKnownCapabilities] when running in a container.
func WithAllCurrentCapabilities(ctx context.Context, client Client, c *containers.Container, s *Spec) error {
	return WithCapabilities(nil)(ctx, client, c, s)
}

// WithAllKnownCapabilities sets all the known linux capabilities for the container process.
func WithAllKnownCapabilities(ctx context.Context, client Client, c *containers.Container, s *Spec) error {
	return WithCapabilities(nil)(ctx, client, c, s)
}
Repository: containerd/containerd
File: core/mount/mount_linux.go (Go)

Code before:
		}
	}
	return
}

func hasDirectIO(opts []string) (bool, []string) {
	for idx, opt := range opts {
		if opt == "direct-io" {
			return true, append(opts[:idx], opts[idx+1:]...)
		}
	}
	return false, opts
}

// compactLowerdirOption updates overlay lowdir option and returns the common
// dir among all the lowdirs.
func compactLowerdirOption(opts []string) (string, []string) {
	idx, dirs := findOverlayLowerdirs(opts)
	if idx == -1 || len(dirs) == 1 {
		// no need to compact if there is only one lowerdir
		return "", opts
	}

	// find out common dir
	commondir, err := getCommonDirectory(dirs)
	if err != nil {
		return "", opts
	}
	commondir = commondir + "/"

	newdirs := make([]string, 0, len(dirs))
	for _, dir := range dirs {
		if len(dir) <= len(commondir) {
			return "", opts
		}
		newdirs = append(newdirs, dir[len(commondir):])
	}

	newopts := copyOptions(opts)
	newopts = append(newopts[:idx], newopts[idx+1:]...)
	newopts = append(newopts, fmt.Sprintf("lowerdir=%s", strings.Join(newdirs, ":")))
	return commondir, newopts
}

// findOverlayLowerdirs returns the index of lowerdir in mount's options and
// all the lowerdir target.
func findOverlayLowerdirs(opts []string) (int, []string) {
	var (
		idx    = -1
		prefix = "lowerdir="
	)

Review comment (suggestion):
This seems like an unrelated change to the patchset. I'd leave this out, for a different PR. This way, we can backport this if needed to other containerd releases, without unnecesary changes

Code after:
}

// compactLowerdirOption updates overlay lowdir option and returns the common
// dir among all the lowdirs.
func compactLowerdirOption(opts []string) (string, []string) {
	idx, dirs := findOverlayLowerdirs(opts)
	if idx == -1 || len(dirs) == 1 {
		// no need to compact if there is only one lowerdir
		return "", opts
	}

	// find out common dir
	commondir := longestCommonPrefix(dirs)
	if commondir == "" {
		return "", opts
	}

	// NOTE: the snapshot id is based on digits.
	// in order to avoid to get snapshots/x, should be back to parent dir.
	// however, there is assumption that the common dir is ${root}/io.containerd.v1.overlayfs/snapshots.
	commondir = path.Dir(commondir)
	if commondir == "/" || commondir == "." {
		return "", opts
	}
	commondir = commondir + "/"

	newdirs := make([]string, 0, len(dirs))
	for _, dir := range dirs {
		if len(dir) <= len(commondir) {
			return "", opts
		}
		newdirs = append(newdirs, dir[len(commondir):])
	}

	newopts := copyOptions(opts)
	newopts = append(newopts[:idx], newopts[idx+1:]...)
	newopts = append(newopts, fmt.Sprintf("lowerdir=%s", strings.Join(newdirs, ":")))
	return commondir, newopts
}

// findOverlayLowerdirs returns the index of lowerdir in mount's options and
// all the lowerdir target.
func findOverlayLowerdirs(opts []string) (int, []string) {
	var (
		idx    = -1
		prefix = "lowerdir="
	)

	for i, opt := range opts {
		if strings.HasPrefix(opt, prefix) {
			idx = i