ronantakizawa/codereview-bench huggingface dataset:
Repository: moby/moby
File: registry/registry.go (Go)

Code before:
// Headers returns request modifiers with a User-Agent and metaHeaders
func Headers(userAgent string, metaHeaders http.Header) []transport.RequestModifier {
	modifiers := []transport.RequestModifier{}
	if userAgent != "" {
		modifiers = append(modifiers, transport.NewHeaderRequestModifier(http.Header{
			"User-Agent": []string{userAgent},
		}))
	}
	if metaHeaders != nil {
		modifiers = append(modifiers, transport.NewHeaderRequestModifier(metaHeaders))
	}
	return modifiers
}

// newTransport returns a new HTTP transport. If tlsConfig is nil, it uses the
// default TLS configuration.
func newTransport(tlsConfig *tls.Config) http.RoundTripper {
	if tlsConfig == nil {
		tlsConfig = tlsconfig.ServerDefault()
	}

	direct := &net.Dialer{
		Timeout:   30 * time.Second,
		KeepAlive: 30 * time.Second,
	}

	return otelhttp.NewTransport(
		&http.Transport{
			Proxy:               http.ProxyFromEnvironment,
			DialContext:         direct.DialContext,
			TLSHandshakeTimeout: 10 * time.Second,
			TLSClientConfig:     tlsConfig,
			// TODO(dmcgowan): Call close idle connections when complete and use keep alive
			DisableKeepAlives: true,
		},
		otelhttp.WithSpanNameFormatter(func(operation string, r *http.Request) string {
			return tracing.Name("registry", operation)
		}),
	)
}


Review comment (refactor):
I saw there's also a utility for this, not sure if that's better? (possibly it allows not having to import `otelhttp`, but leave that to to the utility); https://github.com/moby/moby/blob/ad6929339acd64ebae3fd55657d0dcf22895da86/vendor/github.com/containerd/containerd/tracing/tracing.go#L51-L59

Code after:
func Headers(userAgent string, metaHeaders http.Header) []transport.RequestModifier {
	modifiers := []transport.RequestModifier{}
	if userAgent != "" {
		modifiers = append(modifiers, transport.NewHeaderRequestModifier(http.Header{
			"User-Agent": []string{userAgent},
		}))
	}
	if metaHeaders != nil {
		modifiers = append(modifiers, transport.NewHeaderRequestModifier(metaHeaders))
	}
	return modifiers
}

// newTransport returns a new HTTP transport. If tlsConfig is nil, it uses the
// default TLS configuration.
func newTransport(tlsConfig *tls.Config) http.RoundTripper {
	if tlsConfig == nil {
		tlsConfig = tlsconfig.ServerDefault()
	}

	direct := &net.Dialer{
		Timeout:   30 * time.Second,
		KeepAlive: 30 * time.Second,
	}

	return otelhttp.NewTransport(
		&http.Transport{
			Proxy:               http.ProxyFromEnvironment,
			DialContext:         direct.DialContext,
			TLSHandshakeTimeout: 10 * time.Second,
			TLSClientConfig:     tlsConfig,
			// TODO(dmcgowan): Call close idle connections when complete and use keep alive
			DisableKeepAlives: true,
		},
	)
}
Repository: moby/moby
File: builder/builder-next/worker/containerdworker.go (Go)

Code before:
	"github.com/moby/buildkit/client"
	bkexporter "github.com/moby/buildkit/exporter"
	"github.com/moby/buildkit/session"
	"github.com/moby/buildkit/worker/base"
)

// ContainerdWorker is a local worker instance with dedicated snapshotter, cache, and so on.
type ContainerdWorker struct {
	*base.Worker
	callbacks exporter.BuildkitCallbacks
}

// NewContainerdWorker instantiates a local worker.
func NewContainerdWorker(ctx context.Context, wo base.WorkerOpt, callbacks exporter.BuildkitCallbacks) (*ContainerdWorker, error) {
	bw, err := base.NewWorker(ctx, wo)
	if err != nil {
		return nil, err
	}
	return &ContainerdWorker{Worker: bw, callbacks: callbacks}, nil
}

// Exporter returns exporter by name
func (w *ContainerdWorker) Exporter(name string, sm *session.Manager) (bkexporter.Exporter, error) {
	switch name {
	case exporter.Moby:
		exp, err := w.Worker.Exporter(client.ExporterImage, sm)
		if err != nil {
			return nil, err
		}
		return exporter.NewWrapper(exp, w.callbacks)
	default:
		return w.Worker.Exporter(name, sm)
	}
}


Review comment (bug):
Note: I'm not returning an error here the same way the graph driver worker doesn't return an error

* https://github.com/moby/moby/blob/f5af46d4d5db591da5ce330c9936baecd1df5d9c/builder/builder-next/worker/worker.go#L121-L129

Code after:
	"github.com/containerd/log"
	"github.com/docker/docker/builder/builder-next/exporter"
	"github.com/moby/buildkit/client"
	bkexporter "github.com/moby/buildkit/exporter"
	"github.com/moby/buildkit/session"
	"github.com/moby/buildkit/source/http"
	"github.com/moby/buildkit/worker/base"
)

// ContainerdWorker is a local worker instance with dedicated snapshotter, cache, and so on.
type ContainerdWorker struct {
	*base.Worker
	callbacks exporter.BuildkitCallbacks
}

// NewContainerdWorker instantiates a local worker.
func NewContainerdWorker(ctx context.Context, wo base.WorkerOpt, callbacks exporter.BuildkitCallbacks, rt nethttp.RoundTripper) (*ContainerdWorker, error) {
	bw, err := base.NewWorker(ctx, wo)
	if err != nil {
		return nil, err
	}
	hs, err := http.NewSource(http.Opt{
		CacheAccessor: bw.CacheManager(),
		Transport:     rt,
	})
	if err == nil {
		bw.SourceManager.Register(hs)
	} else {
		log.G(ctx).Warnf("Could not register builder http source: %s", err)
	}

	return &ContainerdWorker{Worker: bw, callbacks: callbacks}, nil
}

// Exporter returns exporter by name
func (w *ContainerdWorker) Exporter(name string, sm *session.Manager) (bkexporter.Exporter, error) {
	switch name {
	case exporter.Moby:
		exp, err := w.Worker.Exporter(client.ExporterImage, sm)
		if err != nil {
			return nil, err
		}
		return exporter.NewWrapper(exp, w.callbacks)
	default:
		return w.Worker.Exporter(name, sm)
	}
}
Repository: moby/moby
File: pkg/sysinfo/sysinfo_linux_test.go (Go)

Code before:
	}
}

func TestCgroupEnabled(t *testing.T) {
	cgroupDir := t.TempDir()

	if cgroupEnabled(cgroupDir, "test") {
		t.Fatal("cgroupEnabled should be false")
	}

	err := os.WriteFile(filepath.Join(cgroupDir, "test"), []byte{}, 0o644)
	if err != nil {
		t.Fatal(err)
	}

	if !cgroupEnabled(cgroupDir, "test") {
		t.Fatal("cgroupEnabled should be true")
	}
}

// TestNew verifies that sysInfo is initialized with the expected values.
func TestNew(t *testing.T) {
	sysInfo := New()
	if sysInfo == nil {
		t.Fatal("sysInfo should not be nil")
	}
	if expected := seccomp.IsEnabled(); sysInfo.Seccomp != expected {
		t.Errorf("got Seccomp %v, wanted %v", sysInfo.Seccomp, expected)
	}
	if expected := apparmorSupported(); sysInfo.AppArmor != expected {
		t.Errorf("got AppArmor %v, wanted %v", sysInfo.AppArmor, expected)
	}
	if expected := cgroupnsSupported(); sysInfo.CgroupNamespaces != expected {
		t.Errorf("got CgroupNamespaces %v, wanted %v", sysInfo.AppArmor, expected)
	}
}

func TestNumCPU(t *testing.T) {
	cpuNumbers := NumCPU()
	if cpuNumbers <= 0 {
		t.Fatal("CPU returned must be greater than zero")
	}
}


Review comment (suggestion):
Starting to consider if we should create a local copy  of this instead  of  depending on  containerd's seccomp package, because it's really minimal. We could even consider moving it into our `profiles/seccomp` package, but perhaps that's not needed  if we only use it here.


(this is the only package where we import containerd's pkg/seccomp)

Code after:
	}
}

func TestCgroupEnabled(t *testing.T) {
	cgroupDir := t.TempDir()

	if cgroupEnabled(cgroupDir, "test") {
		t.Fatal("cgroupEnabled should be false")
	}

	if err := os.WriteFile(filepath.Join(cgroupDir, "test"), []byte{}, 0o644); err != nil {
		t.Fatal(err)
	}

	if !cgroupEnabled(cgroupDir, "test") {
		t.Fatal("cgroupEnabled should be true")
	}
}

// TestNew verifies that sysInfo is initialized with the expected values.
func TestNew(t *testing.T) {
	sysInfo := New()
	if sysInfo == nil {
		t.Fatal("sysInfo should not be nil")
	}
	if expected := seccomp.IsEnabled(); sysInfo.Seccomp != expected {
		t.Errorf("got Seccomp %v, wanted %v", sysInfo.Seccomp, expected)
	}
	if expected := apparmorSupported(); sysInfo.AppArmor != expected {
		t.Errorf("got AppArmor %v, wanted %v", sysInfo.AppArmor, expected)
	}
	if expected := cgroupnsSupported(); sysInfo.CgroupNamespaces != expected {
		t.Errorf("got CgroupNamespaces %v, wanted %v", sysInfo.AppArmor, expected)
	}
}

func TestNumCPU(t *testing.T) {
	if cpuNumbers := NumCPU(); cpuNumbers <= 0 {
		t.Fatal("CPU returned must be greater than zero")
	}
}
Repository: moby/moby
File: integration/container/create_test.go (Go)

Code before:

func TestCreateByImageID(t *testing.T) {
	ctx := setupTest(t)
	apiClient := testEnv.APIClient()

	img, _, err := apiClient.ImageInspectWithRaw(ctx, "busybox")
	assert.NilError(t, err)

	imgIDWithAlgorithm := img.ID
	imgID, _ := strings.CutPrefix(imgIDWithAlgorithm, "sha256:")
	imgShortID := stringid.TruncateID(imgID)

	testCases := []struct {
		doc             string
		image           string
		expectedErrType func(error) bool
		expectedErr     string
	}{
		{
			doc:   "image ID with algorithm",
			image: imgIDWithAlgorithm,
		},
		{
			doc:   "image ID without algorithm",
			image: imgID,
		},
		{
			doc:   "image short-ID",
			image: imgShortID,
		},
		{
			doc:             "image with ID and algorithm as tag",
			image:           "busybox:" + imgIDWithAlgorithm,
			expectedErrType: errdefs.IsInvalidParameter,
			expectedErr:     "Error response from daemon: invalid reference format",
		},
		{
			doc:             "image with ID as tag",
			image:           "busybox:" + imgID,
			expectedErrType: errdefs.IsNotFound,
			expectedErr:     "Error response from daemon: No such image: busybox:" + imgID,
		},
	}

	for _, tc := range testCases {
		t.Run(tc.doc, func(t *testing.T) {
			t.Parallel()
			ctx := testutil.StartSpan(ctx, t)
			resp, err := apiClient.ContainerCreate(ctx,
				&container.Config{Image: tc.image},
				&container.HostConfig{},

Review comment (suggestion):
Looks like this  is  testing the same  as https://github.com/moby/moby/blob/58125b6df64fb34bdee5f073febcbda9f3b02fbf/integration-cli/docker_cli_create_test.go#L306-L312


Let me remove that  test, and add a comment here that  it's related  to https://github.com/moby/moby/issues/20972

Code after:

func TestCreateByImageID(t *testing.T) {
	ctx := setupTest(t)
	apiClient := testEnv.APIClient()

	img, _, err := apiClient.ImageInspectWithRaw(ctx, "busybox")
	assert.NilError(t, err)

	imgIDWithAlgorithm := img.ID
	imgID, _ := strings.CutPrefix(imgIDWithAlgorithm, "sha256:")
	imgShortID := stringid.TruncateID(imgID)

	testCases := []struct {
		doc             string
		image           string
		expectedErrType func(error) bool
		expectedErr     string
	}{
		{
			doc:   "image ID with algorithm",
			image: imgIDWithAlgorithm,
		},
		{
			// test case for https://github.com/moby/moby/issues/20972
			doc:   "image ID without algorithm",
			image: imgID,
		},
		{
			doc:   "image short-ID",
			image: imgShortID,
		},
		{
			doc:             "image with ID and algorithm as tag",
			image:           "busybox:" + imgIDWithAlgorithm,
			expectedErrType: errdefs.IsInvalidParameter,
			expectedErr:     "Error response from daemon: invalid reference format",
		},
		{
			doc:             "image with ID as tag",
			image:           "busybox:" + imgID,
			expectedErrType: errdefs.IsNotFound,
			expectedErr:     "Error response from daemon: No such image: busybox:" + imgID,
		},
	}

	for _, tc := range testCases {
		t.Run(tc.doc, func(t *testing.T) {
			t.Parallel()
			ctx := testutil.StartSpan(ctx, t)
			resp, err := apiClient.ContainerCreate(ctx,
				&container.Config{Image: tc.image},
Repository: moby/moby
File: runconfig/config_test.go (Go)

Code before:
			isolation:   "invalid",
			invalid:     true,
			expectedErr: `Invalid isolation: "invalid"`,
		},
		{
			isolation:   "process",
			invalid:     runtime.GOOS != "windows",
			expectedErr: `Invalid isolation: "process"`,
		},
		{
			isolation:   "hyperv",
			invalid:     runtime.GOOS != "windows",
			expectedErr: `Invalid isolation: "hyperv"`,
		},
	}
	for _, tc := range tests {
		t.Run("isolation="+tc.isolation, func(t *testing.T) {
			// TODO(thaJeztah): consider using fixtures for the JSON requests so that we don't depend on current implementations.
			b, err := json.Marshal(container.CreateRequest{
				HostConfig: &container.HostConfig{
					Isolation: container.Isolation(tc.isolation),
				},
			})
			assert.NilError(t, err)
			if err != nil {
				t.Fatalf("failed to marshal: %v", err)
			}
			_, _, _, err = decodeContainerConfig(bytes.NewReader(b), sysinfo.New())
			if tc.invalid {
				assert.Check(t, is.ErrorContains(err, tc.expectedErr))
			} else {
				assert.NilError(t, err)
			}
		})
	}
}


Review comment (suggestion):
This can be removed as we already have `assert.NilError` above

Code after:
		{
			isolation:   "hyperv",
			invalid:     runtime.GOOS != "windows",
			expectedErr: `invalid isolation (hyperv):`,
		},
	}
	for _, tc := range tests {
		t.Run("isolation="+tc.isolation, func(t *testing.T) {
			// TODO(thaJeztah): consider using fixtures for the JSON requests so that we don't depend on current implementations.
			b, err := json.Marshal(container.CreateRequest{
				HostConfig: &container.HostConfig{
					Isolation: container.Isolation(tc.isolation),
				},
			})
			assert.NilError(t, err)

			_, _, _, err = decodeContainerConfig(bytes.NewReader(b), sysinfo.New())
			if tc.invalid {
				assert.Check(t, is.ErrorContains(err, tc.expectedErr))
				assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter))
			} else {
				assert.NilError(t, err)
			}
		})
	}
}

func TestDecodeContainerConfigPrivileged(t *testing.T) {
	requestJSON, err := json.Marshal(container.CreateRequest{HostConfig: &container.HostConfig{Privileged: true}})
	assert.NilError(t, err)

	_, _, _, err = decodeContainerConfig(bytes.NewReader(requestJSON), sysinfo.New())
	if runtime.GOOS == "windows" {
		const expected = "invalid option: privileged mode is not supported for Windows containers"
		assert.Check(t, is.Error(err, expected))
		assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter))
	} else {
		assert.NilError(t, err)
	}
}
Repository: moby/moby
File: daemon/links/links_test.go (Go)

Code before:

func TestLinkNaming(t *testing.T) {
	ports := make(nat.PortSet)
	ports[newPortNoError("tcp", "6379")] = struct{}{}

	link := NewLink("172.0.17.3", "172.0.17.2", "/db/docker-1", nil, ports)

	rawEnv := link.ToEnv()
	env := make(map[string]string, len(rawEnv))
	for _, e := range rawEnv {
		parts := strings.Split(e, "=")
		if len(parts) != 2 {
			t.FailNow()
		}
		env[parts[0]] = parts[1]
	}

	value, ok := env["DOCKER_1_PORT"]

	if !ok {
		t.Fatal("DOCKER_1_PORT not found in env")
	}

	if value != "tcp://172.0.17.2:6379" {
		t.Fatalf("Expected 172.0.17.2:6379, got %s", env["DOCKER_1_PORT"])
	}
}

func TestLinkNew(t *testing.T) {
	ports := make(nat.PortSet)
	ports[newPortNoError("tcp", "6379")] = struct{}{}

	link := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", nil, ports)

	if link.Name != "/db/docker" {
		t.Fail()
	}
	if link.ParentIP != "172.0.17.3" {
		t.Fail()
	}
	if link.ChildIP != "172.0.17.2" {
		t.Fail()
	}
	for _, p := range link.Ports {
		if p != newPortNoError("tcp", "6379") {
			t.Fail()
		}
	}
}

func TestLinkEnv(t *testing.T) {

Review comment (suggestion):
FWIW; this is roughly how I arrived in this package; the `Link` type is not used anywhere, and it's only used as an intermediate struct to produce the env-vars to set.

I may be rewriting some code so that the scope of the package is reduced to only generating env-vars and internalise the `Link` type.

Code after:
		"DOCKER_1_NAME=/db/docker-1",
		"DOCKER_1_PORT=tcp://172.0.17.2:6379",
		"DOCKER_1_PORT_6379_TCP=tcp://172.0.17.2:6379",
		"DOCKER_1_PORT_6379_TCP_ADDR=172.0.17.2",
		"DOCKER_1_PORT_6379_TCP_PORT=6379",
		"DOCKER_1_PORT_6379_TCP_PROTO=tcp",
	}

	actual := link.ToEnv()
	sort.Strings(actual) // order of env-vars is not relevant
	assert.DeepEqual(t, expectedEnv, actual)
}

func TestLinkNew(t *testing.T) {
	link := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", nil, nat.PortSet{
		"6379/tcp": struct{}{},
	})

	expected := &Link{
		Name:     "/db/docker",
		ParentIP: "172.0.17.3",
		ChildIP:  "172.0.17.2",
		Ports:    []nat.Port{"6379/tcp"},
	}

	assert.DeepEqual(t, expected, link)
}

func TestLinkEnv(t *testing.T) {
	link := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", []string{"PASSWORD=gordon"}, nat.PortSet{
		"6379/tcp": struct{}{},
	})

	expectedEnv := []string{
		"DOCKER_ENV_PASSWORD=gordon",
		"DOCKER_NAME=/db/docker",
		"DOCKER_PORT=tcp://172.0.17.2:6379",
		"DOCKER_PORT_6379_TCP=tcp://172.0.17.2:6379",
		"DOCKER_PORT_6379_TCP_ADDR=172.0.17.2",
		"DOCKER_PORT_6379_TCP_PORT=6379",
		"DOCKER_PORT_6379_TCP_PROTO=tcp",
	}

	actual := link.ToEnv()
	sort.Strings(actual) // order of env-vars is not relevant
	assert.DeepEqual(t, expectedEnv, actual)
}

func TestLinkMultipleEnv(t *testing.T) {
	link := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", []string{"PASSWORD=gordon"}, nat.PortSet{
		"6379/tcp": struct{}{},
Repository: moby/moby
File: pkg/sysinfo/numcpu.go (Go)

Code before:
package sysinfo // import "github.com/docker/docker/pkg/sysinfo"

import (
	"runtime"
)

// NumCPU returns the number of CPUs. It's the equivalent of [runtime.NumCPU].
// Deprecated: Use [runtime.NumCPU] instead. It will be removed in the next release.
var NumCPU = runtime.NumCPU()


Review comment (suggestion):
This needs an empty line before the `Deprecated` comment, otherwise it's not detected as "Deprecated".

It's also slightly better to create a wrapper function here; both because we don't want any code from overriding the  exported alias, but also because otherwise it shows up under `variables` on pkg.go.dev, which can be confusing.

```suggestion
// NumCPU returns the number of CPUs. It's the equivalent of [runtime.NumCPU].
//
// Deprecated: Use [runtime.NumCPU] instead. It will be removed in the next release.
func NumCPU() int {
	return runtime.NumCPU()
}
```

Code after:
package sysinfo // import "github.com/docker/docker/pkg/sysinfo"

import (
	"runtime"
)

// NumCPU returns the number of CPUs. It's the equivalent of [runtime.NumCPU].
//
// Deprecated: Use [runtime.NumCPU] instead. It will be removed in the next release.
func NumCPU() int {
	return runtime.NumCPU()
}
Repository: moby/moby
File: daemon/containerd/image_changes.go (Go)

Code before:
	if rwl == nil {
		return nil, fmt.Errorf("RWLayer is unexpectedly nil for container %s", ctr.ID)
	}
	snapshotter := i.client.SnapshotService(ctr.Driver)
	info, err := snapshotter.Stat(ctx, ctr.ID)
	if err != nil {
		return nil, err
	}

	id := stringid.GenerateRandomID()
	parentViewKey := ctr.ID + "-parent-view-" + id
	imageMounts, _ := snapshotter.View(ctx, parentViewKey, info.Parent)

	defer func() {
		if err := snapshotter.Remove(ctx, parentViewKey); err != nil {
			log.G(ctx).WithError(err).Warn("error removing the parent view snapshot")
		}
	}()

	containerRoot, err := rwl.Mount(ctr.GetMountLabel())
	if err != nil {
		return nil, err
	}
	defer func() {
		err := rwl.Unmount()
		if err != nil {
			log.G(ctx).WithError(err).WithField("container", ctr.ID).Warn("Failed to unmount container RWLayer after export")
		}
	}()
	var changes []archive.Change
	err = mount.WithReadonlyTempMount(ctx, imageMounts, func(imageRoot string) error {
		changes, err = archive.ChangesDirs(containerRoot, imageRoot)
		return err
	})
	return changes, err
}


Review comment (suggestion):
Really nit (no need to change unless you're  making other changes);

```suggestion
		if err := rwl.Unmount(); err != nil {
```


Code after:
	if rwl == nil {
		return nil, fmt.Errorf("RWLayer is unexpectedly nil for container %s", ctr.ID)
	}
	snapshotter := i.client.SnapshotService(ctr.Driver)
	info, err := snapshotter.Stat(ctx, ctr.ID)
	if err != nil {
		return nil, err
	}

	id := stringid.GenerateRandomID()
	parentViewKey := ctr.ID + "-parent-view-" + id
	imageMounts, _ := snapshotter.View(ctx, parentViewKey, info.Parent)

	defer func() {
		if err := snapshotter.Remove(ctx, parentViewKey); err != nil {
			log.G(ctx).WithError(err).Warn("error removing the parent view snapshot")
		}
	}()

	containerRoot, err := rwl.Mount(ctr.GetMountLabel())
	if err != nil {
		return nil, err
	}
	defer func() {
		if err := rwl.Unmount(); err != nil {
			log.G(ctx).WithFields(log.Fields{"error": err, "container": ctr.ID}).Warn("Failed to unmount container RWLayer after export")
		}
	}()
	var changes []archive.Change
	err = mount.WithReadonlyTempMount(ctx, imageMounts, func(imageRoot string) error {
		changes, err = archive.ChangesDirs(containerRoot, imageRoot)
		return err
	})
	return changes, err
}
Repository: moby/moby
File: daemon/export.go (Go)

Code before:
		return fmt.Errorf("Error exporting container %s: %v", name, err)
	}

	return nil
}

func (daemon *Daemon) containerExport(ctx context.Context, ctr *container.Container, out io.Writer) error {
	rwlayer := ctr.RWLayer
	if rwlayer == nil {
		return fmt.Errorf("container %s has no rootfs", ctr.ID)
	}

	select {
	case <-ctx.Done():
		return ctx.Err()
	default:
	}

	basefs, err := rwlayer.Mount(ctr.GetMountLabel())
	if err != nil {
		return err
	}
	defer func() {
		err := rwlayer.Unmount()
		if err != nil {
			log.G(ctx).WithError(err).WithField("container", ctr.ID).Warn("Failed to unmount container RWLayer after export")
		}
	}()

	archv, err := chrootarchive.Tar(basefs, &archive.TarOptions{
		Compression: archive.Uncompressed,
		IDMap:       daemon.idMapping,
	}, basefs)
	if err != nil {
		return err
	}

	select {
	case <-ctx.Done():
		return ctx.Err()
	default:
	}
	// Stream the entire contents of the container (basically a volatile snapshot)
	_, err = io.Copy(out, archv)
	if err != nil {
		return err
	}

	daemon.LogContainerEvent(ctr, events.ActionExport)
	return nil
}

Review comment (bug):
Perhaps use `WithFields` here;


```suggestion
			log.G(ctx).WithFields(log.Fields{"error": err, "container": ctr.ID}).Warn("Failed to unmount container RWLayer after export")
```

Code after:
		return fmt.Errorf("Error exporting container %s: %v", name, err)
	}

	return nil
}

func (daemon *Daemon) containerExport(ctx context.Context, ctr *container.Container, out io.Writer) error {
	rwl := ctr.RWLayer
	if rwl == nil {
		return fmt.Errorf("container %s has no rootfs", ctr.ID)
	}

	if err := ctx.Err(); err != nil {
		return err
	}

	basefs, err := rwl.Mount(ctr.GetMountLabel())
	if err != nil {
		return err
	}
	defer func() {
		if err := rwl.Unmount(); err != nil {
			log.G(ctx).WithFields(log.Fields{"error": err, "container": ctr.ID}).Warn("Failed to unmount container RWLayer after export")
		}
	}()

	archv, err := chrootarchive.Tar(basefs, &archive.TarOptions{
		Compression: archive.Uncompressed,
		IDMap:       daemon.idMapping,
	}, basefs)
	if err != nil {
		return err
	}

	if err := ctx.Err(); err != nil {
		return err
	}
	// Stream the entire contents of the container (basically a volatile snapshot)
	if _, err := io.Copy(out, archv); err != nil {
		return err
	}

	daemon.LogContainerEvent(ctr, events.ActionExport)
	return nil
}