[TA2091] Fix volume name compare issue. (#89)

Signed-off-by: satbir <[email protected]>

include/zrepl_mgmt.h

@@ -160,6 +160,7 @@ void uzfs_zvol_store_last_committed_io_no(zvol_state_t *zv,
     uint64_t io_seq);
 extern int create_and_bind(const char *port, int bind_needed,
     boolean_t nonblocking);
+int uzfs_zvol_name_compare(zvol_info_t *zv, const char *name);
 
 /*
  * API to drop refcnt on zinfo. If refcnt

lib/libzpool/zrepl_mgmt.c

@@ -154,37 +154,52 @@ uzfs_mark_offline_and_free_zinfo(zvol_info_t *zinfo)
 	(void) uzfs_zinfo_free(zinfo);
 }
 
+int
+uzfs_zvol_name_compare(zvol_info_t *zv, const char *name)
+{
+
+	char *p;
+	int pathlen, namelen;
+
+	if (name == NULL)
+		return (-1);
+
+	namelen = strlen(name);
+	pathlen = strlen(zv->name);
+
+	if (namelen > pathlen)
+		return (-1);
+	/*
+	 * iSCSI controller send volume name without any prefix
+	 * while zinfo store volume name with prefix of pool_name.
+	 * So we need to extract volume name from zinfo->name
+	 * and compare it with pass name.
+	 */
+	p = zv->name + (pathlen - namelen);
+
+	/*
+	 * Name can be in any of these formats
+	 * "vol1" or "zpool/vol1"
+	 */
+	if ((strcmp(zv->name, name) == 0) ||
+	    ((strcmp(p, name) == 0) && (*(--p) == '/'))) {
+		return (0);
+	}
+	return (-1);
+}
+
 zvol_info_t *
 uzfs_zinfo_lookup(const char *name)
 {
-	int pathlen;
-	char *p;
 	zvol_info_t *zv = NULL;
-	int namelen = ((name) ? strlen(name) : 0);
 
 	if (name == NULL)
 		return (NULL);
 
 	(void) mutex_enter(&zvol_list_mutex);
 	SLIST_FOREACH(zv, &zvol_list, zinfo_next) {
-		/*
-		 * TODO: Come up with better approach.
-		 * Since iSCSI tgt can send volname in desired format,
-		 * we have added this hack where we do calculate length
-		 * of name passed as arg, look for those many bytes in
-		 * zv->name from tail/end.
-		 */
-		pathlen = strlen(zv->name);
-		p = zv->name + (pathlen - namelen);
-
-		/*
-		 * Name can be in any of these formats
-		 * "vol1" or "zpool/vol1"
-		 */
-		if ((strcmp(zv->name, name) == 0) ||
-		    ((strcmp(p, name) == 0) && (*(--p) == '/'))) {
+		if (uzfs_zvol_name_compare(zv, name) == 0)
 			break;
-		}
 	}
 	if (zv != NULL) {
 		/* Take refcount */

lib/libzrepl/mgmt_conn.c

@@ -716,8 +716,7 @@ uzfs_zvol_rebuild_dw_replica_start(uzfs_mgmt_conn_t *conn, zvol_io_hdr_t *hdrp,
 	for (; rebuild_op_cnt > 0; rebuild_op_cnt--, mack++) {
 		LOG_INFO("zvol %s at %s:%u helping in rebuild",
 		    mack->volname, mack->ip, mack->port);
-		if (strncmp(zinfo->name, mack->dw_volname, MAXNAMELEN)
-		    != 0) {
+		if (uzfs_zvol_name_compare(zinfo, mack->dw_volname) != 0) {
 			LOG_ERR("zvol %s not matching with zinfo %s",
 			    mack->dw_volname, zinfo->name);
 ret_error:

tests/cbtest/gtest/test_uzfs.cc

@@ -1157,3 +1157,19 @@ TEST(RebuildScanner, RebuildSuccess) {
 
 	memset(&zinfo->zv->rebuild_info, 0, sizeof (zvol_rebuild_info_t));
 }
+
+/* Volume name stored in zinfo is "pool1/vol1" */
+TEST(VolumeNameCompare, VolumeNameCompareTest) {
+
+	/* Pass NULL string for compare */
+	EXPECT_EQ(-1, uzfs_zvol_name_compare(zinfo, ""));
+
+	/* Pass wrong volname but smaller string size */
+	EXPECT_EQ(-1, uzfs_zvol_name_compare(zinfo, "vol"));
+
+	/* Pass wrong volname but larger string size */
+	EXPECT_EQ(-1, uzfs_zvol_name_compare(zinfo, "vol12345678910"));
+
+	/* Pass correct volname */
+	EXPECT_EQ(0, uzfs_zvol_name_compare(zinfo, "vol1"));
+}