From be7e44e4b21468c1b44037e09d6daf3c5360fb4a Mon Sep 17 00:00:00 2001 From: Hrudaya Date: Fri, 11 Aug 2023 12:58:20 +0000 Subject: [PATCH] fix(snapshot): use customize function to judge snapshot inplace of spdk native function Signed-off-by: Hrudaya --- io-engine/src/lvs/lvol_snapshot.rs | 2 +- io-engine/src/lvs/lvs_lvol.rs | 11 +++++++++-- io-engine/tests/snapshot_lvol.rs | 20 ++++++++++++++++++-- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/io-engine/src/lvs/lvol_snapshot.rs b/io-engine/src/lvs/lvol_snapshot.rs index e616706977..d7b6104726 100644 --- a/io-engine/src/lvs/lvol_snapshot.rs +++ b/io-engine/src/lvs/lvol_snapshot.rs @@ -429,7 +429,6 @@ impl SnapshotOps for Lvol { if let Some(parent_lvol) = parent { // Skip snapshots if it's parent is not matched. if curr_attr_val != parent_lvol.uuid() { - warn!("presisted parent ?curr_attr_val not matched to input parent ?parent_lvol.uuid()"); return None; } } @@ -589,6 +588,7 @@ impl SnapshotOps for Lvol { if !snapshot_lvol.is_snapshot() { continue; } + match snapshot_lvol.snapshot_descriptor(Some(self)) { Some(snapshot_descriptor) => { snapshot_list.push(snapshot_descriptor) diff --git a/io-engine/src/lvs/lvs_lvol.rs b/io-engine/src/lvs/lvs_lvol.rs index aa15d708bf..9a5d3907bc 100644 --- a/io-engine/src/lvs/lvs_lvol.rs +++ b/io-engine/src/lvs/lvs_lvol.rs @@ -21,7 +21,6 @@ use spdk_rs::libspdk::{ spdk_blob_get_num_clusters_ancestors, spdk_blob_get_xattr_value, spdk_blob_is_read_only, - spdk_blob_is_snapshot, spdk_blob_is_thin_provisioned, spdk_blob_set_xattr, spdk_blob_sync_md, @@ -48,6 +47,7 @@ use crate::{ ShareProps, SnapshotOps, SnapshotParams, + SnapshotXattrs, ToErrno, UntypedBdev, UpdateProps, @@ -716,8 +716,15 @@ impl LvsLvol for Lvol { } /// Returns a boolean indicating if the lvol is a snapshot. + /// Currently in place of SPDK native API to judge lvol as snapshot, xattr + /// is checked here. When there is only single Lvol(snapshot) present in + /// the system and there is restart of io-engine and pool import + /// happens, SPDK native API consider lvol(snapshot) as normal lvol. + /// Looks like a bug in SPDK, but all snapshot attribute are intact in + /// SPDK after io-engine restarts. fn is_snapshot(&self) -> bool { - unsafe { spdk_blob_is_snapshot(self.blob_checked()) } + Lvol::get_blob_xattr(self, SnapshotXattrs::SnapshotCreateTime.name()) + .is_some() } /// Lvol is considered as clone if its sourceuuid attribute is a valid diff --git a/io-engine/tests/snapshot_lvol.rs b/io-engine/tests/snapshot_lvol.rs index daa5f58af9..b195282769 100755 --- a/io-engine/tests/snapshot_lvol.rs +++ b/io-engine/tests/snapshot_lvol.rs @@ -451,10 +451,26 @@ async fn test_lvol_list_snapshot() { .await .expect("Failed to create a snapshot"); - let snapshot_list = lvol.list_snapshot_by_source_uuid(); + let mut snapshot_list = lvol.list_snapshot_by_source_uuid(); info!("Total number of snapshots: {}", snapshot_list.len()); assert_eq!(2, snapshot_list.len(), "Snapshot Count not matched!!"); - clean_snapshots(snapshot_list).await; + lvol.destroy() + .await + .expect("Failed to destroy the original replica"); + let snap_lvol_1 = snapshot_list.remove(0).snapshot_lvol; + let snap_lvol_2 = snapshot_list.remove(0).snapshot_lvol; + snap_lvol_1 + .destroy() + .await + .expect("Failed to destroy first snapshot"); + assert!( + snap_lvol_2.is_snapshot(), + "It is a snapshot, wrongly recognized as normal replica" + ); + snap_lvol_2 + .destroy() + .await + .expect("Failed to destroy last snapshot"); }) .await; }