[lxc-devel] [lxd/master] Alternative approach to operation cancelation

stgraber on Github lxc-bot at linuxcontainers.org
Tue Jun 27 06:39:56 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170627/4c3be468/attachment.bin>
-------------- next part --------------
From 48f751afeb9cff9b5cf37abcd5bdeb9b31730e0f Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Wed, 14 Jun 2017 10:50:46 +0200
Subject: [PATCH 1/4] Cleanup.

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/images.go | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/lxd/images.go b/lxd/images.go
index 4fcbe60e5..18f491843 100644
--- a/lxd/images.go
+++ b/lxd/images.go
@@ -720,35 +720,25 @@ func imagesPost(d *Daemon, r *http.Request) Response {
 		// Setup the cleanup function
 		defer cleanup(builddir, post)
 
-		if !imageUpload {
+		if imageUpload {
+			/* Processing image upload */
+			info, err = getImgPostInfo(d, r, builddir, post)
+		} else {
 			if req.Source.Type == "image" {
 				/* Processing image copy from remote */
 				info, err = imgPostRemoteInfo(d, req, op)
-				if err != nil {
-					return err
-				}
 			} else if req.Source.Type == "url" {
 				/* Processing image copy from URL */
 				info, err = imgPostURLInfo(d, req, op)
-				if err != nil {
-					return err
-				}
 			} else {
 				/* Processing image creation from container */
 				imagePublishLock.Lock()
 				info, err = imgPostContInfo(d, r, req, builddir)
-				if err != nil {
-					imagePublishLock.Unlock()
-					return err
-				}
 				imagePublishLock.Unlock()
 			}
-		} else {
-			/* Processing image upload */
-			info, err = getImgPostInfo(d, r, builddir, post)
-			if err != nil {
-				return err
-			}
+		}
+		if err != nil {
+			return err
 		}
 
 		// Apply any provided alias

From 2566bba45fcd737249764f31fda09a0b1ea0c6f9 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Thu, 15 Jun 2017 15:01:42 +0200
Subject: [PATCH 2/4] Add logic to cancel HTTP request.

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 client/interfaces.go           |  4 ++++
 client/simplestreams_images.go |  8 ++++----
 client/util.go                 |  8 +++++---
 lxd/daemon_images.go           |  5 ++++-
 lxd/images.go                  |  1 +
 lxd/operations.go              | 28 +++++++++++++++++++++++-----
 shared/operation/operation.go  | 31 +++++++++++++++++++++++++++++++
 test/main.sh                   |  1 +
 test/suites/image.sh           | 24 ++++++++++++++++++++++++
 9 files changed, 97 insertions(+), 13 deletions(-)
 create mode 100644 shared/operation/operation.go

diff --git a/client/interfaces.go b/client/interfaces.go
index dd9c67827..42e36394f 100644
--- a/client/interfaces.go
+++ b/client/interfaces.go
@@ -6,6 +6,7 @@ import (
 	"github.com/gorilla/websocket"
 
 	"github.com/lxc/lxd/shared/api"
+	"github.com/lxc/lxd/shared/operation"
 )
 
 // The Server type represents a generic read-only server.
@@ -194,6 +195,9 @@ type ImageFileRequest struct {
 
 	// Progress handler (called whenever some progress is made)
 	ProgressHandler func(progress ProgressData)
+
+	// The cancellable operation that's handling the request
+	Operation operation.CancellableOperation
 }
 
 // The ImageFileResponse struct is used as the response for image downloads
diff --git a/client/simplestreams_images.go b/client/simplestreams_images.go
index d84f8ed09..ef803105f 100644
--- a/client/simplestreams_images.go
+++ b/client/simplestreams_images.go
@@ -63,11 +63,11 @@ func (r *ProtocolSimpleStreams) GetImageFile(fingerprint string, req ImageFileRe
 		// Try over http
 		url := fmt.Sprintf("http://%s/%s", strings.TrimPrefix(r.httpHost, "https://"), meta.Path)
 
-		size, err := downloadFileSha256(r.http, r.httpUserAgent, req.ProgressHandler, "metadata", url, meta.Sha256, req.MetaFile)
+		size, err := downloadFileSha256(req.Operation, r.http, r.httpUserAgent, req.ProgressHandler, "metadata", url, meta.Sha256, req.MetaFile)
 		if err != nil {
 			// Try over https
 			url = fmt.Sprintf("%s/%s", r.httpHost, meta.Path)
-			size, err = downloadFileSha256(r.http, r.httpUserAgent, req.ProgressHandler, "metadata", url, meta.Sha256, req.MetaFile)
+			size, err = downloadFileSha256(req.Operation, r.http, r.httpUserAgent, req.ProgressHandler, "metadata", url, meta.Sha256, req.MetaFile)
 			if err != nil {
 				return nil, err
 			}
@@ -84,11 +84,11 @@ func (r *ProtocolSimpleStreams) GetImageFile(fingerprint string, req ImageFileRe
 		// Try over http
 		url := fmt.Sprintf("http://%s/%s", strings.TrimPrefix(r.httpHost, "https://"), rootfs.Path)
 
-		size, err := downloadFileSha256(r.http, r.httpUserAgent, req.ProgressHandler, "rootfs", url, rootfs.Sha256, req.RootfsFile)
+		size, err := downloadFileSha256(req.Operation, r.http, r.httpUserAgent, req.ProgressHandler, "rootfs", url, rootfs.Sha256, req.RootfsFile)
 		if err != nil {
 			// Try over https
 			url = fmt.Sprintf("%s/%s", r.httpHost, rootfs.Path)
-			size, err = downloadFileSha256(r.http, r.httpUserAgent, req.ProgressHandler, "rootfs", url, rootfs.Sha256, req.RootfsFile)
+			size, err = downloadFileSha256(req.Operation, r.http, r.httpUserAgent, req.ProgressHandler, "rootfs", url, rootfs.Sha256, req.RootfsFile)
 			if err != nil {
 				return nil, err
 			}
diff --git a/client/util.go b/client/util.go
index 973f1d562..2b35e606b 100644
--- a/client/util.go
+++ b/client/util.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/lxc/lxd/shared"
 	"github.com/lxc/lxd/shared/ioprogress"
+	"github.com/lxc/lxd/shared/operation"
 )
 
 func tlsHTTPClient(tlsClientCert string, tlsClientKey string, tlsCA string, tlsServerCert string, proxy func(req *http.Request) (*url.URL, error)) (*http.Client, error) {
@@ -81,7 +82,7 @@ func unixHTTPClient(path string) (*http.Client, error) {
 	return &client, nil
 }
 
-func downloadFileSha256(httpClient *http.Client, useragent string, progress func(progress ProgressData), filename string, url string, hash string, target io.WriteSeeker) (int64, error) {
+func downloadFileSha256(op operation.CancellableOperation, httpClient *http.Client, useragent string, progress func(progress ProgressData), filename string, url string, hash string, target io.WriteSeeker) (int64, error) {
 	// Always seek to the beginning
 	target.Seek(0, 0)
 
@@ -95,12 +96,13 @@ func downloadFileSha256(httpClient *http.Client, useragent string, progress func
 		req.Header.Set("User-Agent", useragent)
 	}
 
-	// Start the request
-	r, err := httpClient.Do(req)
+	// Perform the request
+	r, err, doneCh := operation.CancellableDownload(op, httpClient, req)
 	if err != nil {
 		return -1, err
 	}
 	defer r.Body.Close()
+	defer close(doneCh)
 
 	if r.StatusCode != http.StatusOK {
 		return -1, fmt.Errorf("Unable to fetch %s: %s", url, r.Status)
diff --git a/lxd/daemon_images.go b/lxd/daemon_images.go
index af4fa94cf..9eace2371 100644
--- a/lxd/daemon_images.go
+++ b/lxd/daemon_images.go
@@ -19,6 +19,7 @@ import (
 	"github.com/lxc/lxd/shared/api"
 	"github.com/lxc/lxd/shared/ioprogress"
 	"github.com/lxc/lxd/shared/logger"
+	cancellable_op "github.com/lxc/lxd/shared/operation"
 	"github.com/lxc/lxd/shared/version"
 
 	log "gopkg.in/inconshreveable/log15.v2"
@@ -392,6 +393,7 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 			MetaFile:        io.WriteSeeker(dest),
 			RootfsFile:      io.WriteSeeker(destRootfs),
 			ProgressHandler: progress,
+			Operation:       op,
 		}
 
 		if secret != "" {
@@ -425,7 +427,8 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 		req.Header.Set("User-Agent", version.UserAgent)
 
 		// Make the request
-		raw, err := httpClient.Do(req)
+		raw, err, doneCh := cancellable_op.CancellableDownload(op, httpClient, req)
+		defer close(doneCh)
 		if err != nil {
 			return nil, err
 		}
diff --git a/lxd/images.go b/lxd/images.go
index 18f491843..4afb59ea9 100644
--- a/lxd/images.go
+++ b/lxd/images.go
@@ -715,6 +715,7 @@ func imagesPost(d *Daemon, r *http.Request) Response {
 
 	// Begin background operation
 	run := func(op *operation) error {
+		var err error
 		var info *api.Image
 
 		// Setup the cleanup function
diff --git a/lxd/operations.go b/lxd/operations.go
index 0794bc5d0..b3953187d 100644
--- a/lxd/operations.go
+++ b/lxd/operations.go
@@ -54,7 +54,8 @@ type operation struct {
 	onConnect func(*operation, *http.Request, http.ResponseWriter) error
 
 	// Channels used for error reporting and state tracking of background actions
-	chanDone chan error
+	chanDone   chan error
+	chanCancel chan error
 
 	// Locking for concurent access to the operation
 	lock sync.Mutex
@@ -153,17 +154,21 @@ func (op *operation) Cancel() (chan error, error) {
 		return nil, fmt.Errorf("Only running operations can be cancelled")
 	}
 
+	op.lock.Lock()
 	if !op.mayCancel() {
+		op.lock.Unlock()
 		return nil, fmt.Errorf("This operation can't be cancelled")
 	}
 
-	chanCancel := make(chan error, 1)
-
-	op.lock.Lock()
 	oldStatus := op.status
 	op.status = api.Cancelling
+	if op.chanCancel != nil {
+		// operationClassToken doesn't have the channel set
+		close(op.chanCancel)
+	}
 	op.lock.Unlock()
 
+	chanCancel := make(chan error, 1)
 	if op.onCancel != nil {
 		go func(op *operation, oldStatus api.StatusCode, chanCancel chan error) {
 			err := op.onCancel(op)
@@ -244,7 +249,20 @@ func (op *operation) Connect(r *http.Request, w http.ResponseWriter) (chan error
 }
 
 func (op *operation) mayCancel() bool {
-	return op.onCancel != nil || op.class == operationClassToken
+	return op.chanCancel != nil || op.class == operationClassToken
+}
+
+// Toggle whether the operation is cancellable. If `true` is passed, the
+// channel to cancel the operation is returned, otherwise nil.
+func (op *operation) Cancellable(flag bool) chan error {
+	var ch chan error
+	if flag {
+		ch = make(chan error)
+	}
+	op.lock.Lock()
+	op.chanCancel = ch
+	op.lock.Unlock()
+	return ch
 }
 
 func (op *operation) Render() (string, *api.Operation, error) {
diff --git a/shared/operation/operation.go b/shared/operation/operation.go
new file mode 100644
index 000000000..b0ee2c0d9
--- /dev/null
+++ b/shared/operation/operation.go
@@ -0,0 +1,31 @@
+package operation
+
+import (
+	"net/http"
+)
+
+// An operation that can be canceled.
+type CancellableOperation interface {
+
+	// Toggle whether the operation is cancellable
+	Cancellable(flag bool) chan error
+}
+
+func CancellableDownload(op CancellableOperation, client *http.Client, req *http.Request) (*http.Response, error, chan bool) {
+	chDone := make(chan bool)
+
+	go func() {
+		chCancel := op.Cancellable(true)
+		select {
+		case <-chCancel:
+			if transport, ok := client.Transport.(*http.Transport); ok {
+				transport.CancelRequest(req)
+			}
+		case <-chDone:
+		}
+		op.Cancellable(false)
+	}()
+
+	resp, err := client.Do(req)
+	return resp, err, chDone
+}
diff --git a/test/main.sh b/test/main.sh
index 6d00232d6..965d3b614 100755
--- a/test/main.sh
+++ b/test/main.sh
@@ -600,6 +600,7 @@ run_test test_basic_usage "basic usage"
 run_test test_security "security features"
 run_test test_image_expiry "image expiry"
 run_test test_image_list_all_aliases "image list all aliases"
+run_test test_image_copy_cancel "cancel image copy operation"
 run_test test_image_auto_update "image auto-update"
 run_test test_image_prefer_cached "image prefer cached"
 run_test test_concurrent_exec "concurrent exec"
diff --git a/test/suites/image.sh b/test/suites/image.sh
index 7b5c19101..4c11182ff 100644
--- a/test/suites/image.sh
+++ b/test/suites/image.sh
@@ -48,5 +48,29 @@ test_image_list_all_aliases() {
     # both aliases are listed if the "aliases" column is included in output
     lxc image list -c L | grep -q testimage
     lxc image list -c L | grep -q zzz
+}
 
+test_image_copy_cancel() {
+    # shellcheck disable=2039,2153,2155
+    local operation_id=$(
+        curl -s --unix-socket "${LXD_DIR}"/unix.socket lxd/1.0/images -d '
+             {
+                "auto_update": false,
+                "public": false,
+                "source": {
+                        "certificate": "",
+                        "fingerprint": "x",
+                        "mode": "pull",
+                        "protocol": "simplestreams",
+                        "server": "https://cloud-images.ubuntu.com/releases",
+                        "type": "image"
+                        }
+             }' | jq -r .metadata.id)
+    # while curl -s --unix-socket "${LXD_DIR}"/unix.socket "lxd/1.0/operations/$operation_id" | jq -r .metadata.may_cancel | grep -q false; do
+    #     sleep 0.1
+    # done
+    # cancel the operation and expect a success response
+    curl -s --unix-socket "${LXD_DIR}"/unix.socket -X DELETE \
+         "lxd/1.0/operations/$operation_id" | grep 200
+    test "$(lxc image list --format=csv)" = ""
 }

From 229c4917df06d937d4e7e5e00001fbf9bd01fae3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 27 Jun 2017 02:22:36 -0400
Subject: [PATCH 3/4] Tweak wording and concept a bit
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 client/interfaces.go           |  6 ++---
 client/simplestreams_images.go |  8 +++----
 client/util.go                 |  6 ++---
 lxd/daemon_images.go           | 12 +++++++---
 lxd/operations.go              | 45 ++++++++++++++++++++-----------------
 shared/cancel/canceler.go      | 51 ++++++++++++++++++++++++++++++++++++++++++
 shared/operation/operation.go  | 31 -------------------------
 7 files changed, 94 insertions(+), 65 deletions(-)
 create mode 100644 shared/cancel/canceler.go
 delete mode 100644 shared/operation/operation.go

diff --git a/client/interfaces.go b/client/interfaces.go
index 42e36394f..410c4f726 100644
--- a/client/interfaces.go
+++ b/client/interfaces.go
@@ -6,7 +6,7 @@ import (
 	"github.com/gorilla/websocket"
 
 	"github.com/lxc/lxd/shared/api"
-	"github.com/lxc/lxd/shared/operation"
+	"github.com/lxc/lxd/shared/cancel"
 )
 
 // The Server type represents a generic read-only server.
@@ -196,8 +196,8 @@ type ImageFileRequest struct {
 	// Progress handler (called whenever some progress is made)
 	ProgressHandler func(progress ProgressData)
 
-	// The cancellable operation that's handling the request
-	Operation operation.CancellableOperation
+	// A canceler that can be used to interrupt some part of the image download request
+	Canceler *cancel.Canceler
 }
 
 // The ImageFileResponse struct is used as the response for image downloads
diff --git a/client/simplestreams_images.go b/client/simplestreams_images.go
index ef803105f..fd82d9912 100644
--- a/client/simplestreams_images.go
+++ b/client/simplestreams_images.go
@@ -63,11 +63,11 @@ func (r *ProtocolSimpleStreams) GetImageFile(fingerprint string, req ImageFileRe
 		// Try over http
 		url := fmt.Sprintf("http://%s/%s", strings.TrimPrefix(r.httpHost, "https://"), meta.Path)
 
-		size, err := downloadFileSha256(req.Operation, r.http, r.httpUserAgent, req.ProgressHandler, "metadata", url, meta.Sha256, req.MetaFile)
+		size, err := downloadFileSha256(r.http, r.httpUserAgent, req.ProgressHandler, req.Canceler, "metadata", url, meta.Sha256, req.MetaFile)
 		if err != nil {
 			// Try over https
 			url = fmt.Sprintf("%s/%s", r.httpHost, meta.Path)
-			size, err = downloadFileSha256(req.Operation, r.http, r.httpUserAgent, req.ProgressHandler, "metadata", url, meta.Sha256, req.MetaFile)
+			size, err = downloadFileSha256(r.http, r.httpUserAgent, req.ProgressHandler, req.Canceler, "metadata", url, meta.Sha256, req.MetaFile)
 			if err != nil {
 				return nil, err
 			}
@@ -84,11 +84,11 @@ func (r *ProtocolSimpleStreams) GetImageFile(fingerprint string, req ImageFileRe
 		// Try over http
 		url := fmt.Sprintf("http://%s/%s", strings.TrimPrefix(r.httpHost, "https://"), rootfs.Path)
 
-		size, err := downloadFileSha256(req.Operation, r.http, r.httpUserAgent, req.ProgressHandler, "rootfs", url, rootfs.Sha256, req.RootfsFile)
+		size, err := downloadFileSha256(r.http, r.httpUserAgent, req.ProgressHandler, req.Canceler, "rootfs", url, rootfs.Sha256, req.RootfsFile)
 		if err != nil {
 			// Try over https
 			url = fmt.Sprintf("%s/%s", r.httpHost, rootfs.Path)
-			size, err = downloadFileSha256(req.Operation, r.http, r.httpUserAgent, req.ProgressHandler, "rootfs", url, rootfs.Sha256, req.RootfsFile)
+			size, err = downloadFileSha256(r.http, r.httpUserAgent, req.ProgressHandler, req.Canceler, "rootfs", url, rootfs.Sha256, req.RootfsFile)
 			if err != nil {
 				return nil, err
 			}
diff --git a/client/util.go b/client/util.go
index 2b35e606b..1a166787d 100644
--- a/client/util.go
+++ b/client/util.go
@@ -9,8 +9,8 @@ import (
 	"net/url"
 
 	"github.com/lxc/lxd/shared"
+	"github.com/lxc/lxd/shared/cancel"
 	"github.com/lxc/lxd/shared/ioprogress"
-	"github.com/lxc/lxd/shared/operation"
 )
 
 func tlsHTTPClient(tlsClientCert string, tlsClientKey string, tlsCA string, tlsServerCert string, proxy func(req *http.Request) (*url.URL, error)) (*http.Client, error) {
@@ -82,7 +82,7 @@ func unixHTTPClient(path string) (*http.Client, error) {
 	return &client, nil
 }
 
-func downloadFileSha256(op operation.CancellableOperation, httpClient *http.Client, useragent string, progress func(progress ProgressData), filename string, url string, hash string, target io.WriteSeeker) (int64, error) {
+func downloadFileSha256(httpClient *http.Client, useragent string, progress func(progress ProgressData), canceler *cancel.Canceler, filename string, url string, hash string, target io.WriteSeeker) (int64, error) {
 	// Always seek to the beginning
 	target.Seek(0, 0)
 
@@ -97,7 +97,7 @@ func downloadFileSha256(op operation.CancellableOperation, httpClient *http.Clie
 	}
 
 	// Perform the request
-	r, err, doneCh := operation.CancellableDownload(op, httpClient, req)
+	r, err, doneCh := cancel.CancelableDownload(canceler, httpClient, req)
 	if err != nil {
 		return -1, err
 	}
diff --git a/lxd/daemon_images.go b/lxd/daemon_images.go
index 9eace2371..3931de54a 100644
--- a/lxd/daemon_images.go
+++ b/lxd/daemon_images.go
@@ -17,9 +17,9 @@ import (
 	"github.com/lxc/lxd/client"
 	"github.com/lxc/lxd/shared"
 	"github.com/lxc/lxd/shared/api"
+	"github.com/lxc/lxd/shared/cancel"
 	"github.com/lxc/lxd/shared/ioprogress"
 	"github.com/lxc/lxd/shared/logger"
-	cancellable_op "github.com/lxc/lxd/shared/operation"
 	"github.com/lxc/lxd/shared/version"
 
 	log "gopkg.in/inconshreveable/log15.v2"
@@ -354,6 +354,12 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 		}
 	}
 
+	var canceler *cancel.Canceler
+	if op != nil {
+		canceler = &cancel.Canceler{}
+		op.canceler = canceler
+	}
+
 	if protocol == "lxd" || protocol == "simplestreams" {
 		// Create the target files
 		dest, err := os.Create(destName)
@@ -393,7 +399,7 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 			MetaFile:        io.WriteSeeker(dest),
 			RootfsFile:      io.WriteSeeker(destRootfs),
 			ProgressHandler: progress,
-			Operation:       op,
+			Canceler:        canceler,
 		}
 
 		if secret != "" {
@@ -427,7 +433,7 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 		req.Header.Set("User-Agent", version.UserAgent)
 
 		// Make the request
-		raw, err, doneCh := cancellable_op.CancellableDownload(op, httpClient, req)
+		raw, err, doneCh := cancel.CancelableDownload(canceler, httpClient, req)
 		defer close(doneCh)
 		if err != nil {
 			return nil, err
diff --git a/lxd/operations.go b/lxd/operations.go
index b3953187d..043181d81 100644
--- a/lxd/operations.go
+++ b/lxd/operations.go
@@ -13,6 +13,7 @@ import (
 
 	"github.com/lxc/lxd/shared"
 	"github.com/lxc/lxd/shared/api"
+	"github.com/lxc/lxd/shared/cancel"
 	"github.com/lxc/lxd/shared/logger"
 	"github.com/lxc/lxd/shared/version"
 )
@@ -47,6 +48,7 @@ type operation struct {
 	metadata  map[string]interface{}
 	err       string
 	readonly  bool
+	canceler  *cancel.Canceler
 
 	// Those functions are called at various points in the operation lifecycle
 	onRun     func(*operation) error
@@ -54,8 +56,7 @@ type operation struct {
 	onConnect func(*operation, *http.Request, http.ResponseWriter) error
 
 	// Channels used for error reporting and state tracking of background actions
-	chanDone   chan error
-	chanCancel chan error
+	chanDone chan error
 
 	// Locking for concurent access to the operation
 	lock sync.Mutex
@@ -154,21 +155,17 @@ func (op *operation) Cancel() (chan error, error) {
 		return nil, fmt.Errorf("Only running operations can be cancelled")
 	}
 
-	op.lock.Lock()
 	if !op.mayCancel() {
-		op.lock.Unlock()
 		return nil, fmt.Errorf("This operation can't be cancelled")
 	}
 
+	chanCancel := make(chan error, 1)
+
+	op.lock.Lock()
 	oldStatus := op.status
 	op.status = api.Cancelling
-	if op.chanCancel != nil {
-		// operationClassToken doesn't have the channel set
-		close(op.chanCancel)
-	}
 	op.lock.Unlock()
 
-	chanCancel := make(chan error, 1)
 	if op.onCancel != nil {
 		go func(op *operation, oldStatus api.StatusCode, chanCancel chan error) {
 			err := op.onCancel(op)
@@ -200,6 +197,13 @@ func (op *operation) Cancel() (chan error, error) {
 	_, md, _ := op.Render()
 	eventSend("operation", md)
 
+	if op.canceler != nil {
+		err := op.canceler.Cancel()
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	if op.onCancel == nil {
 		op.lock.Lock()
 		op.status = api.Cancelled
@@ -249,20 +253,19 @@ func (op *operation) Connect(r *http.Request, w http.ResponseWriter) (chan error
 }
 
 func (op *operation) mayCancel() bool {
-	return op.chanCancel != nil || op.class == operationClassToken
-}
+	if op.class == operationClassToken {
+		return true
+	}
 
-// Toggle whether the operation is cancellable. If `true` is passed, the
-// channel to cancel the operation is returned, otherwise nil.
-func (op *operation) Cancellable(flag bool) chan error {
-	var ch chan error
-	if flag {
-		ch = make(chan error)
+	if op.onCancel != nil {
+		return true
 	}
-	op.lock.Lock()
-	op.chanCancel = ch
-	op.lock.Unlock()
-	return ch
+
+	if op.canceler != nil && op.canceler.Cancelable() {
+		return true
+	}
+
+	return false
 }
 
 func (op *operation) Render() (string, *api.Operation, error) {
diff --git a/shared/cancel/canceler.go b/shared/cancel/canceler.go
new file mode 100644
index 000000000..7e8685197
--- /dev/null
+++ b/shared/cancel/canceler.go
@@ -0,0 +1,51 @@
+package cancel
+
+import (
+	"fmt"
+	"net/http"
+)
+
+// A struct to track canceleation
+type Canceler struct {
+	chCancel chan bool
+}
+
+func (c *Canceler) Cancelable() bool {
+	return c.chCancel != nil
+}
+
+func (c *Canceler) Cancel() error {
+	if c.chCancel == nil {
+		return fmt.Errorf("This operation cannot be canceled at this time")
+	}
+
+	close(c.chCancel)
+	c.chCancel = nil
+	return nil
+}
+
+func CancelableDownload(c *Canceler, client *http.Client, req *http.Request) (*http.Response, error, chan bool) {
+	chDone := make(chan bool)
+
+	go func() {
+		chCancel := make(chan bool)
+		if c != nil {
+			c.chCancel = chCancel
+		}
+
+		select {
+		case <-c.chCancel:
+			if transport, ok := client.Transport.(*http.Transport); ok {
+				transport.CancelRequest(req)
+			}
+		case <-chDone:
+		}
+
+		if c != nil {
+			c.chCancel = nil
+		}
+	}()
+
+	resp, err := client.Do(req)
+	return resp, err, chDone
+}
diff --git a/shared/operation/operation.go b/shared/operation/operation.go
deleted file mode 100644
index b0ee2c0d9..000000000
--- a/shared/operation/operation.go
+++ /dev/null
@@ -1,31 +0,0 @@
-package operation
-
-import (
-	"net/http"
-)
-
-// An operation that can be canceled.
-type CancellableOperation interface {
-
-	// Toggle whether the operation is cancellable
-	Cancellable(flag bool) chan error
-}
-
-func CancellableDownload(op CancellableOperation, client *http.Client, req *http.Request) (*http.Response, error, chan bool) {
-	chDone := make(chan bool)
-
-	go func() {
-		chCancel := op.Cancellable(true)
-		select {
-		case <-chCancel:
-			if transport, ok := client.Transport.(*http.Transport); ok {
-				transport.CancelRequest(req)
-			}
-		case <-chDone:
-		}
-		op.Cancellable(false)
-	}()
-
-	resp, err := client.Do(req)
-	return resp, err, chDone
-}

From 24492c158bf9cef18bd0e718004adb57a23d1b81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 27 Jun 2017 02:29:08 -0400
Subject: [PATCH 4/4] tests: Don't finger public remotes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The API endpoint for GetServer doesn't exist on most of those so the new
client package doesn't allow it.

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 test/suites/remote.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/suites/remote.sh b/test/suites/remote.sh
index fd9a92446..d53ee123e 100644
--- a/test/suites/remote.sh
+++ b/test/suites/remote.sh
@@ -17,7 +17,6 @@ test_remote_url() {
 
   for url in ${urls}; do
     lxc_remote remote add test "${url}"
-    lxc_remote finger test:
     lxc_remote remote remove test
   done
 }


More information about the lxc-devel mailing list