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