[lxc-devel] [lxd/master] Make ZFS backend more reliable
stgraber on Github
lxc-bot at linuxcontainers.org
Mon Sep 30 03:07:15 UTC 2019
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/20190929/cdfeddba/attachment.bin>
-------------- next part --------------
From c0c812a7632a999570c9d443e28cbb176d4febe9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at 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 <stgraber at 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?= <stgraber at 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 <stgraber at 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 at 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 at 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 at 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?= <stgraber at 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 <stgraber at 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
More information about the lxc-devel
mailing list