The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6342
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
Fixes bug in client migration code that failed to respond to remote API errors when migrating a volume to a machine where a volume of the same name exists.
During a `move` operation this would cause the local volume to be removed even though it hadn't been transferred to the far side.
During a `copy` operation this would give the illusion that the copy had been successful even though it hadn't been.
From fa082b7e2395787117fcdafc756727917e8d96fb Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 23 Oct 2019 12:05:05 +0100
Subject: [PATCH 1/2] client/util: Removes empty error slice check in
remoteOperationError
remoteOperationError currently checks for an empty slice of errors and if empty
then returns a nil error.
However all current uses of remoteOperationError are also wrapped in an `if
!success {}` statement leading to two methods of detecting if there has been an
error.
1. If success boolean is false
2. If the errors slice is empty
There has been one bug in the migration code that set success to false but did
not append to the errors slice, meaning this function did not detect an error.
Either we should remove the success boolean entirely and use an empty errors
slice to indicate no error or we should remove the emtpy errors slice check in
remoteOperationError, having both just increases the chance of a bug occurring.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
client/util.go | 5 -----
1 file changed, 5 deletions(-)
diff --git a/client/util.go b/client/util.go
index 6e13123e82..e5c468c8ac 100644
--- a/client/util.go
+++ b/client/util.go
@@ -144,11 +144,6 @@ func (nullReadWriteCloser) Write(p []byte) (int, error) {
return len(p), nil }
func (nullReadWriteCloser) Read(p []byte) (int, error) { return 0, io.EOF }
func remoteOperationError(msg string, errors map[string]error) error {
- // Check if empty
- if len(errors) == 0 {
- return nil
- }
-
// Check if all identical
var err error
for _, entry := range errors {
From f0d7aa64faf19fb87acf22944b76bc5b8aa59ebd Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 23 Oct 2019 12:03:09 +0100
Subject: [PATCH 2/2] client/lxd/storage/volumes: Fixes bug where migration
errors were ignored
This was a potentially serious bug as if you attempted to move a volume from
one machine to another and the volume already existed on the remote side then
even if it responded with an error, this error was ignored and the local volume
was subsequently deleted without having copied it to the remote machine.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
client/lxd_storage_volumes.go | 1 +
1 file changed, 1 insertion(+)
diff --git a/client/lxd_storage_volumes.go b/client/lxd_storage_volumes.go
index cd86c39fc9..b10f954ed7 100644
--- a/client/lxd_storage_volumes.go
+++ b/client/lxd_storage_volumes.go
@@ -323,6 +323,7 @@ func (r *ProtocolLXD) tryCreateStoragePoolVolume(pool
string, req api.StorageVol
path := fmt.Sprintf("/storage-pools/%s/volumes/%s",
url.PathEscape(pool), url.PathEscape(req.Type))
top, _, err := r.queryOperation("POST", path, req, "")
if err != nil {
+ errors[serverURL] = err
continue
}
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
