The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6260

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) ===

From c0c812a7632a999570c9d443e28cbb176d4febe9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgra...@ubuntu.com>
Date: Sun, 29 Sep 2019 22:43:14 -0400
Subject: [PATCH 1/3] lxd/storage/zfs: Fix error handling in ImageCreate
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgra...@ubuntu.com>
---
 lxd/storage_zfs.go | 65 +++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 29 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 47bf74eca9..5fb3fc1d49 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -2322,59 +2322,63 @@ func (s *storageZfs) ContainerBackupLoad(info 
backupInfo, data io.ReadSeeker, ta
 func (s *storageZfs) ImageCreate(fingerprint string, tracker 
*ioprogress.ProgressTracker) error {
        logger.Debugf("Creating ZFS storage volume for image \"%s\" on storage 
pool \"%s\"", fingerprint, s.pool.Name)
 
+       // Common variables
        poolName := s.getOnDiskPoolName()
        imageMntPoint := driver.GetImageMountPoint(s.pool.Name, fingerprint)
        fs := fmt.Sprintf("images/%s", fingerprint)
-       revert := true
-       subrevert := true
 
+       // Revert flags
+       revertDB := true
+       revertMountpoint := true
+       revertDataset := true
+
+       // Create the image volume entry
        err := s.createImageDbPoolVolume(fingerprint)
        if err != nil {
                return err
        }
+
        defer func() {
-               if !subrevert {
+               if !revertDB {
                        return
                }
+
                s.deleteImageDbPoolVolume(fingerprint)
        }()
 
-       if zfsFilesystemEntityExists(poolName, fmt.Sprintf("deleted/%s", fs)) {
-               if err := zfsPoolVolumeRename(poolName, 
fmt.Sprintf("deleted/%s", fs), fs, true); err != nil {
+       // Create mountpoint if missing
+       if !shared.PathExists(imageMntPoint) {
+               err := os.MkdirAll(imageMntPoint, 0700)
+               if err != nil {
                        return err
                }
 
                defer func() {
-                       if !revert {
+                       if !revertMountpoint {
                                return
                        }
-                       s.ImageDelete(fingerprint)
+
+                       os.RemoveAll(imageMntPoint)
                }()
+       }
 
-               // In case this is an image from an older lxd instance, wipe the
-               // mountpoint.
-               err = zfsPoolVolumeSet(poolName, fs, "mountpoint", "none")
+       // Check for deleted images
+       if zfsFilesystemEntityExists(poolName, fmt.Sprintf("deleted/%s", fs)) {
+               // Restore deleted image
+               err := zfsPoolVolumeRename(poolName, fmt.Sprintf("deleted/%s", 
fs), fs, true)
                if err != nil {
                        return err
                }
 
-               revert = false
-               subrevert = false
-
-               return nil
-       }
-
-       if !shared.PathExists(imageMntPoint) {
-               err := os.MkdirAll(imageMntPoint, 0700)
+               // In case this is an image from an older lxd instance, wipe 
the mountpoint.
+               err = zfsPoolVolumeSet(poolName, fs, "mountpoint", "none")
                if err != nil {
                        return err
                }
-               defer func() {
-                       if !subrevert {
-                               return
-                       }
-                       os.RemoveAll(imageMntPoint)
-               }()
+
+               revertDB = false
+               revertMountpoint = false
+               return nil
        }
 
        // Create temporary mountpoint directory.
@@ -2387,19 +2391,20 @@ func (s *storageZfs) ImageCreate(fingerprint string, 
tracker *ioprogress.Progres
 
        imagePath := shared.VarPath("images", fingerprint)
 
-       // Create a new storage volume on the storage pool for the image.
+       // Create a new dataset for the image
        dataset := fmt.Sprintf("%s/%s", poolName, fs)
        msg, err := zfsPoolVolumeCreate(dataset, "mountpoint=none")
        if err != nil {
                logger.Errorf("Failed to create ZFS dataset \"%s\" on storage 
pool \"%s\": %s", dataset, s.pool.Name, msg)
                return err
        }
-       subrevert = false
+
        defer func() {
-               if !revert {
+               if !revertDataset {
                        return
                }
-               s.ImageDelete(fingerprint)
+
+               zfsPoolVolumeDestroy(poolName, fs)
        }()
 
        // Set a temporary mountpoint for the image.
@@ -2441,7 +2446,9 @@ func (s *storageZfs) ImageCreate(fingerprint string, 
tracker *ioprogress.Progres
                return err
        }
 
-       revert = false
+       revertDB = false
+       revertMountpoint = false
+       revertDataset = false
 
        logger.Debugf("Created ZFS storage volume for image \"%s\" on storage 
pool \"%s\"", fingerprint, s.pool.Name)
        return nil

From 5a101e9b3c9e0fb6ad89111ee5b9f61daa0e276c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgra...@ubuntu.com>
Date: Sun, 29 Sep 2019 22:56:45 -0400
Subject: [PATCH 2/3] lxd/storage/zfs: Better handle broken images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Check for the "@readonly" snapshot and if missing, attempt to re-import
the image from scratch.

Signed-off-by: Stéphane Graber <stgra...@ubuntu.com>
---
 lxd/storage_zfs.go | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 5fb3fc1d49..b0bf9c9df9 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -863,7 +863,7 @@ func (s *storageZfs) ContainerCreateFromImage(container 
Instance, fingerprint st
                lxdStorageMapLock.Unlock()
 
                var imgerr error
-               if !zfsFilesystemEntityExists(poolName, fsImage) {
+               if !zfsFilesystemEntityExists(poolName, 
fmt.Sprintf("%s@readonly", fsImage)) {
                        imgerr = s.ImageCreate(fingerprint, tracker)
                }
 
@@ -2332,6 +2332,13 @@ func (s *storageZfs) ImageCreate(fingerprint string, 
tracker *ioprogress.Progres
        revertMountpoint := true
        revertDataset := true
 
+       // Deal with bad/partial unpacks
+       if zfsFilesystemEntityExists(poolName, fs) {
+               zfsPoolVolumeDestroy(poolName, fmt.Sprintf("%s@readonly", fs))
+               zfsPoolVolumeDestroy(poolName, fs)
+               s.deleteImageDbPoolVolume(fingerprint)
+       }
+
        // Create the image volume entry
        err := s.createImageDbPoolVolume(fingerprint)
        if err != nil {
@@ -2363,7 +2370,7 @@ func (s *storageZfs) ImageCreate(fingerprint string, 
tracker *ioprogress.Progres
        }
 
        // Check for deleted images
-       if zfsFilesystemEntityExists(poolName, fmt.Sprintf("deleted/%s", fs)) {
+       if zfsFilesystemEntityExists(poolName, fmt.Sprintf("deleted/%s", 
fmt.Sprintf("%s@readonly", fs))) {
                // Restore deleted image
                err := zfsPoolVolumeRename(poolName, fmt.Sprintf("deleted/%s", 
fs), fs, true)
                if err != nil {

From 4ae5fe257077bfffb0af43200e6263128fc3029c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgra...@ubuntu.com>
Date: Sun, 29 Sep 2019 23:03:29 -0400
Subject: [PATCH 3/3] lxd/storage/zfs: Tweak destroy logic
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This attempts to workaround some reported issues around dataset deletion
by first attempting a clean umount, then a lazy umount and also adding a
1s delay in the case where we unmounted something.

This path is however unlikely to ever be taken on a normal system as LXD
should have unmounted the container before hitting that point.

Signed-off-by: Stéphane Graber <stgra...@ubuntu.com>
---
 lxd/storage_zfs_utils.go | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 4a2d87091f..7574ecdb2f 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -252,11 +252,17 @@ func zfsPoolVolumeDestroy(pool string, path string) error 
{
        }
 
        if mountpoint != "none" && shared.IsMountPoint(mountpoint) {
-               err := unix.Unmount(mountpoint, unix.MNT_DETACH)
+               // Make sure the filesystem isn't mounted anymore
+               err := unix.Unmount(mountpoint, 0)
                if err != nil {
-                       logger.Errorf("umount failed: %s", err)
-                       return err
+                       err := unix.Unmount(mountpoint, unix.MNT_DETACH)
+                       if err != nil {
+                               return err
+                       }
                }
+
+               // Give a chance for the kernel to notice (workaround for zfs 
slowness)
+               time.Sleep(1 * time.Second)
        }
 
        // Due to open fds or kernel refs, this may fail for a bit, give it 10s
@@ -267,8 +273,7 @@ func zfsPoolVolumeDestroy(pool string, path string) error {
                fmt.Sprintf("%s/%s", pool, path))
 
        if err != nil {
-               logger.Errorf("zfs destroy failed: %v", err)
-               return errors.Wrap(err, "Failed to destroy ZFS filesystem")
+               return errors.Wrap(err, "Failed to destroy ZFS dataset")
        }
 
        return nil
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to