fix(delete_internal_snap): deletes internally created snap (#148)

Signed-off-by: Vishnu Itta <vitta@mayadata.io>

cmd/Makefile.am

@@ -1,8 +1,8 @@
-SUBDIRS  = zfs zpool zdb zhack zinject zstreamdump ztest uzfs_test zpios
+SUBDIRS  = zfs zpool zdb zhack zinject zstreamdump ztest zpios
 SUBDIRS += mount_zfs fsck_zfs zvol_id vdev_id arcstat dbufstat zed
 SUBDIRS += arc_summary raidz_test zgenhostid
 
 if ENABLE_UZFS
-SUBDIRS += zrepl
+SUBDIRS += zrepl uzfs_test
 endif
 

cmd/uzfs_test/Makefile.am

@@ -22,6 +22,7 @@ uzfs_test_LDADD = \
 	$(top_builddir)/lib/libnvpair/libnvpair.la \
 	$(top_builddir)/lib/libuutil/libuutil.la \
 	$(top_builddir)/lib/libzpool/libzpool.la \
+	$(top_builddir)/lib/libzrepl/libzrepl.la \
 	$(top_builddir)/lib/libzfs/libzfs.la \
 	$(top_builddir)/lib/libzfs_core/libzfs_core.la
 

cmd/uzfs_test/uzfs_test_rebuilding.c

@@ -21,8 +21,10 @@
 
 #include <sys/zfs_context.h>
 #include <sys/spa.h>
+#include <sys/dsl_destroy.h>
 #include <sys/uzfs_zvol.h>
 #include <uzfs_mgmt.h>
+#include <data_conn.h>
 #include <uzfs_io.h>
 #include <zrepl_mgmt.h>
 #include <uzfs_zap.h>
@@ -217,18 +219,24 @@ fetch_modified_data(void *arg)
 	off_t offset, end;
 	size_t len;
 	int max_count = 4;
+	zvol_state_t *snap_zv = NULL;
+	char *snap_name;
+	int rc;
 
 	printf("fetching modified data\n");
 	md.io_num = repl_data->base_io;
 
 	len = r_data->zvol->zv_volsize / max_count;
 
+	uzfs_zvol_create_internal_snapshot(repl_data->zvol, &snap_zv,
+	    md.io_num);
+
 	for (offset = 0; offset < r_data->zvol->zv_volsize; ) {
 		end = offset + len;
 		if (end > r_data->zvol->zv_volsize)
 			len = r_data->zvol->zv_volsize - offset;
 
-		err = uzfs_get_io_diff(repl_data->zvol, &md,
+		err = uzfs_get_io_diff(repl_data->zvol, &md, snap_zv,
 		    uzfs_test_meta_diff_traverse_cb, offset, len,
 		    r_data);
 		if (err)
@@ -239,6 +247,16 @@ fetch_modified_data(void *arg)
 			break;
 	}
 
+	if (snap_zv != NULL) {
+		snap_name = kmem_asprintf("%s", snap_zv->zv_name);
+		uzfs_close_dataset(snap_zv);
+		rc = dsl_destroy_snapshot(snap_name, B_FALSE);
+		if (rc != 0)
+			LOG_ERR("snap destroy failed %s %d", snap_name, rc);
+		strfree(snap_name);
+		snap_zv = NULL;
+	}
+
 	if (err) {
 		printf("error(%d)... while fetching modified data\n", err);
 		exit(1);

include/data_conn.h

@@ -73,7 +73,9 @@ void uzfs_update_ionum_interval(zvol_info_t *zinfo, uint32_t timeout);
 void uzfs_zvol_timer_thread(void);
 
 void signal_fds_related_to_zinfo(zvol_info_t *zinfo);
-
+int
+uzfs_zvol_create_internal_snapshot(zvol_state_t *zv, zvol_state_t **snap_zv,
+    uint64_t io_num);
 #ifdef __cplusplus
 }
 #endif

include/uzfs_mgmt.h

@@ -48,6 +48,10 @@ extern int uzfs_hold_dataset(zvol_state_t *zv);
 extern void uzfs_rele_dataset(zvol_state_t *zv);
 
 int uzfs_pool_create(const char *name, char *path, spa_t **spa);
+int
+get_snapshot_zv(zvol_state_t *zv, char *snap_name, zvol_state_t **snap_zv,
+    boolean_t fail_exists, boolean_t fail_notexists);
+
 #ifdef __cplusplus
 }
 #endif

include/uzfs_rebuilding.h

@@ -42,7 +42,8 @@ int compare_blk_metadata(blk_metadata_t *first_md, blk_metadata_t *second_md);
  * API to access data whose metadata is higer than base_metadata
  */
 int uzfs_get_io_diff(zvol_state_t *zv, blk_metadata_t *base_metadata,
-    uzfs_get_io_diff_cb_t *cb_func, off_t offset, size_t len, void *arg);
+    zvol_state_t *snap_zv, uzfs_get_io_diff_cb_t *cb_func, off_t offset,
+    size_t len, void *arg);
 
 /*
  * uzfs_get_nonoverlapping_ondisk_blks will check on_disk metadata with
@@ -60,10 +61,6 @@ uzfs_zvol_create_snaprebuild_clone(zvol_state_t *zv,
 int
 uzfs_zvol_destroy_snaprebuild_clone(zvol_state_t *zv,
     zvol_state_t *snap_zv);
-int
-uzfs_zvol_create_internal_snapshot(zvol_state_t *zv, zvol_state_t **snap_zv,
-    uint64_t io_num, char **snap);
-
 #ifdef __cplusplus
 }
 #endif

lib/libzpool/uzfs_mgmt.c

@@ -526,6 +526,71 @@ uzfs_zvol_create_minors(spa_t *spa, const char *name)
 	kmem_free(pool_name, MAXNAMELEN);
 }
 
+int
+get_snapshot_zv(zvol_state_t *zv, char *snap_name, zvol_state_t **snap_zv,
+    boolean_t fail_exists, boolean_t fail_notexists)
+{
+	char *dataset;
+	int ret = 0;
+
+	dataset = kmem_asprintf("%s@%s", strchr(zv->zv_name, '/') + 1,
+	    snap_name);
+
+	ret = uzfs_open_dataset(zv->zv_spa, dataset, snap_zv);
+	if (ret == ENOENT) {
+		if (fail_notexists) {
+			LOG_ERR("fail on unavailable snapshot %s",
+			    dataset);
+			strfree(dataset);
+			ret = SET_ERROR(ENOENT);
+			return (ret);
+		}
+		ret = dmu_objset_snapshot_one(zv->zv_name, snap_name);
+		if (ret) {
+			LOG_ERR("Failed to create snapshot %s: %d",
+			    dataset, ret);
+			strfree(dataset);
+			return (ret);
+		}
+
+		ret = uzfs_open_dataset(zv->zv_spa, dataset, snap_zv);
+		if (ret == 0) {
+			ret = uzfs_hold_dataset(*snap_zv);
+			if (ret != 0) {
+				LOG_ERR("Failed to hold snapshot: %s %d",
+				    dataset, ret);
+				uzfs_close_dataset(*snap_zv);
+				*snap_zv = NULL;
+			}
+		}
+		else
+			LOG_ERR("Failed to open snapshot: %d", ret);
+	} else if (ret == 0) {
+		if (fail_exists) {
+			LOG_ERR("fail on already available snapshot %s",
+			    dataset);
+			uzfs_close_dataset(*snap_zv);
+			*snap_zv = NULL;
+			ret = SET_ERROR(EEXIST);
+		} else {
+			LOG_INFO("holding already available snapshot %s",
+			    dataset);
+			ret = uzfs_hold_dataset(*snap_zv);
+			if (ret != 0) {
+				LOG_ERR("Failed to hold already existing "
+				    "snapshot %s: %d", dataset, ret);
+				uzfs_close_dataset(*snap_zv);
+				*snap_zv = NULL;
+			}
+		}
+	} else {
+		LOG_ERR("Failed to open snapshot: %d", ret);
+	}
+
+	strfree(dataset);
+	return (ret);
+}
+
 /* uZFS Zvol destroy call back function */
 int
 uzfs_zvol_destroy_cb(const char *ds_name, void *arg)

lib/libzpool/uzfs_rebuilding.c

@@ -70,104 +70,6 @@ iszero(blk_metadata_t *md)
 			last_md = NULL;					\
 		} while (0)
 
-int
-get_snapshot_zv(zvol_state_t *zv, char *snap_name, zvol_state_t **snap_zv,
-    boolean_t fail_exists, boolean_t fail_notexists)
-{
-	char *dataset;
-	int ret = 0;
-
-	dataset = kmem_asprintf("%s@%s", strchr(zv->zv_name, '/') + 1,
-	    snap_name);
-
-	ret = uzfs_open_dataset(zv->zv_spa, dataset, snap_zv);
-	if (ret == ENOENT) {
-		if (fail_notexists) {
-			LOG_ERR("fail on unavailable snapshot %s",
-			    dataset);
-			strfree(dataset);
-			ret = SET_ERROR(ENOENT);
-			return (ret);
-		}
-		ret = dmu_objset_snapshot_one(zv->zv_name, snap_name);
-		if (ret) {
-			LOG_ERR("Failed to create snapshot %s: %d",
-			    dataset, ret);
-			strfree(dataset);
-			return (ret);
-		}
-
-		ret = uzfs_open_dataset(zv->zv_spa, dataset, snap_zv);
-		if (ret == 0) {
-			ret = uzfs_hold_dataset(*snap_zv);
-			if (ret != 0) {
-				LOG_ERR("Failed to hold snapshot: %s %d",
-				    dataset, ret);
-				uzfs_close_dataset(*snap_zv);
-				*snap_zv = NULL;
-			}
-		}
-		else
-			LOG_ERR("Failed to open snapshot: %d", ret);
-	} else if (ret == 0) {
-		if (fail_exists) {
-			LOG_ERR("fail on already available snapshot %s",
-			    dataset);
-			uzfs_close_dataset(*snap_zv);
-			*snap_zv = NULL;
-			ret = SET_ERROR(EEXIST);
-		} else {
-			LOG_INFO("holding already available snapshot %s",
-			    dataset);
-			ret = uzfs_hold_dataset(*snap_zv);
-			if (ret != 0) {
-				LOG_ERR("Failed to hold already existing "
-				    "snapshot %s: %d", dataset, ret);
-				uzfs_close_dataset(*snap_zv);
-				*snap_zv = NULL;
-			}
-		}
-	} else {
-		LOG_ERR("Failed to open snapshot: %d", ret);
-	}
-
-	strfree(dataset);
-	return (ret);
-}
-
-/*
- * Creates internal snapshot on given zv using current time as part of snapname.
- * If snap already exists, waits and attempts to create again.
- * Returns zv of snapshot in snap_zv.
- */
-int
-uzfs_zvol_create_internal_snapshot(zvol_state_t *zv, zvol_state_t **snap_zv,
-    uint64_t io_num, char **snap)
-{
-	int ret = 0;
-	char *snap_name;
-	time_t now;
-again:
-	now = time(NULL);
-	snap_name = kmem_asprintf("%s%llu.%llu", IO_DIFF_SNAPNAME, io_num, now);
-	ret = get_snapshot_zv(zv, snap_name, snap_zv, B_TRUE, B_FALSE);
-	if (ret == EEXIST) {
-		strfree(snap_name);
-		sleep(1);
-		LOG_INFO("Waiting to create internal snapshot");
-		goto again;
-	}
-	if (ret != 0) {
-		LOG_ERR("Failed to get info about %s@%s",
-		    zv->zv_name, snap_name);
-		strfree(snap_name);
-	} else {
-		*snap = snap_name;
-	}
-	return (ret);
-}
-
-
 void
 destroy_snapshot_zv(zvol_state_t *zv, char *snap_name)
 {
@@ -179,15 +81,15 @@ destroy_snapshot_zv(zvol_state_t *zv, char *snap_name)
 }
 
 int
-uzfs_get_io_diff(zvol_state_t *zv, blk_metadata_t *low,
+uzfs_get_io_diff(zvol_state_t *zv, blk_metadata_t *low, zvol_state_t *snap,
     uzfs_get_io_diff_cb_t *func, off_t lun_offset, size_t lun_len, void *arg)
 {
 	uint64_t blocksize = zv->zv_volmetablocksize;
 	uint64_t metadata_read_chunk_size = 10 * blocksize;
 	uint64_t metaobjectsize = (zv->zv_volsize / zv->zv_metavolblocksize) *
 	    zv->zv_volmetadatasize;
 	uint64_t metadatasize = zv->zv_volmetadatasize;
-	char *buf, *snap_name = NULL;
+	char *buf;
 	uint64_t i, read;
 	uint64_t offset, len, end;
 	int ret = 0;
@@ -197,7 +99,7 @@ uzfs_get_io_diff(zvol_state_t *zv, blk_metadata_t *low,
 	zvol_state_t *snap_zv;
 	metaobj_blk_offset_t snap_metablk;
 
-	if (!func || (lun_offset + lun_len) > zv->zv_volsize)
+	if (!func || (lun_offset + lun_len) > zv->zv_volsize || snap == NULL)
 		return (EINVAL);
 
 	get_zv_metaobj_block_details(&snap_metablk, zv, lun_offset, lun_len);
@@ -207,8 +109,7 @@ uzfs_get_io_diff(zvol_state_t *zv, blk_metadata_t *low,
 	if (end > metaobjectsize)
 		end = metaobjectsize;
 
-	ret = uzfs_zvol_create_internal_snapshot(zv, &snap_zv,
-	    low->io_num, &snap_name);
+	snap_zv = snap;
 
 	if (ret != 0) {
 		LOG_ERR("Failed to create snapshot for vol %s io_num %lu",
@@ -292,17 +193,8 @@ uzfs_get_io_diff(zvol_state_t *zv, blk_metadata_t *low,
 		}
 	}
 
-	uzfs_close_dataset(snap_zv);
-
-	/*
-	 * TODO: if we failed to destroy snapshot here then
-	 * this should be handled separately from application.
-	 */
-	if (end == metaobjectsize)
-		destroy_snapshot_zv(zv, snap_name);
 
 	umem_free(buf, metadata_read_chunk_size);
-	strfree(snap_name);
 	return (ret);
 }