From 1bb4b5a5aee39136dead9ec3e502810ea6f15100 Mon Sep 17 00:00:00 2001
From: matt-fidd <81489167+matt-fidd@users.noreply.github.com>
Date: Wed, 7 Apr 2021 00:05:54 +0100
Subject: [PATCH] zfs get -p only outputs 3 columns if "clones" property is
 empty

get_clones_string currently returns an empty string for filesystem
snapshots which have no clones. This breaks parsable `zfs get` output as
only three columns are output, instead of 4.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Matt Fiddaman <github@m.fiddaman.uk>
Co-authored-by: matt <matt@fiddaman.net>
Closes #11837
---
 lib/libzfs/libzfs_dataset.c                          |  3 ++-
 .../functional/cli_root/zfs_get/zfs_get_001_pos.ksh  | 12 +++++++++++-
 .../cli_root/zfs_get/zfs_get_common.kshlib           | 10 ++++++++--
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c
index b654e41a53..e57bc49bac 100644
--- a/lib/libzfs/libzfs_dataset.c
+++ b/lib/libzfs/libzfs_dataset.c
@@ -32,6 +32,7 @@
  * Copyright 2017-2018 RackTop Systems.
  * Copyright (c) 2019 Datto Inc.
  * Copyright (c) 2019, loli10K <ezomori.nozomu@gmail.com>
+ * Copyright (c) 2021 Matt Fiddaman
  */
 
 #include <ctype.h>
@@ -2385,7 +2386,7 @@ get_clones_string(zfs_handle_t *zhp, char *propbuf, size_t proplen)
 	nvpair_t *pair;
 
 	value = zfs_get_clones_nvl(zhp);
-	if (value == NULL)
+	if (value == NULL || nvlist_empty(value))
 		return (-1);
 
 	propbuf[0] = '\0';
diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_001_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_001_pos.ksh
index eeae5390f0..3547fb76c6 100755
--- a/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_001_pos.ksh
+++ b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_001_pos.ksh
@@ -27,6 +27,7 @@
 
 #
 # Copyright (c) 2016 by Delphix. All rights reserved.
+# Copyright (c) 2021 Matt Fiddaman
 #
 
 . $STF_SUITE/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib
@@ -72,7 +73,8 @@ typeset all_props=("${zfs_props[@]}" \
     "${zfs_props_os[@]}" \
     "${userquota_props[@]}")
 typeset dataset=($TESTPOOL/$TESTCTR $TESTPOOL/$TESTFS $TESTPOOL/$TESTVOL \
-	$TESTPOOL/$TESTFS@$TESTSNAP $TESTPOOL/$TESTVOL@$TESTSNAP)
+	$TESTPOOL/$TESTFS@$TESTSNAP $TESTPOOL/$TESTVOL@$TESTSNAP
+	$TESTPOOL/$TESTFS@$TESTSNAP1 $TESTPOOL/$TESTCLONE)
 
 typeset bookmark_props=(creation)
 typeset bookmark=($TESTPOOL/$TESTFS#$TESTBKMARK $TESTPOOL/$TESTVOL#$TESTBKMARK)
@@ -102,6 +104,7 @@ function check_return_value
 
 			if [[ $item == $p ]]; then
 				((found += 1))
+				cols=$(echo $line | awk '{print NF}')
 				break
 			fi
 		done < $TESTDIR/$TESTFILE0
@@ -109,6 +112,9 @@ function check_return_value
 		if ((found == 0)); then
 			log_fail "'zfs get $opt $props $dst' return " \
 			    "error message.'$p' haven't been found."
+		elif [[ "$opt" == "-p" ]] && ((cols != 4)); then
+			log_fail "'zfs get $opt $props $dst' returned " \
+			    "$cols columns instead of 4."
 		fi
 	done
 
@@ -123,6 +129,10 @@ log_onexit cleanup
 create_snapshot $TESTPOOL/$TESTFS $TESTSNAP
 create_snapshot $TESTPOOL/$TESTVOL $TESTSNAP
 
+# Create second snapshot and clone it
+create_snapshot $TESTPOOL/$TESTFS $TESTSNAP1
+create_clone $TESTPOOL/$TESTFS@$TESTSNAP1 $TESTPOOL/$TESTCLONE
+
 # Create filesystem and volume's bookmark
 create_bookmark $TESTPOOL/$TESTFS $TESTSNAP $TESTBKMARK
 create_bookmark $TESTPOOL/$TESTVOL $TESTSNAP $TESTBKMARK
diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib
index d8cb9af028..9b4eecf371 100644
--- a/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib
+++ b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_common.kshlib
@@ -26,6 +26,7 @@
 
 #
 # Copyright (c) 2016 by Delphix. All rights reserved.
+# Copyright (c) 2021 Matt Fiddaman
 #
 
 . $STF_SUITE/include/libtest.shlib
@@ -87,8 +88,8 @@ function gen_option_str # $elements $prefix $separator $counter
 }
 
 #
-# Cleanup the volume snapshot, filesystem snapshot, volume bookmark, and
-# filesystem bookmark that were created for this test case.
+# Cleanup the volume snapshot, filesystem snapshots, clone, volume bookmark,
+# and filesystem bookmark that were created for this test case.
 #
 function cleanup
 {
@@ -97,6 +98,11 @@ function cleanup
 	datasetexists $TESTPOOL/$TESTFS@$TESTSNAP && \
 		destroy_snapshot $TESTPOOL/$TESTFS@$TESTSNAP
 
+	datasetexists $TESTPOOL/$TESTCLONE && \
+		destroy_clone $TESTPOOL/$TESTCLONE
+	datasetexists $TESTPOOL/$TESTFS@$TESTSNAP1 && \
+		destroy_snapshot $TESTPOOL/$TESTFS@$TESTSNAP1
+
 	bkmarkexists $TESTPOOL/$TESTVOL#$TESTBKMARK && \
 		destroy_bookmark $TESTPOOL/$TESTVOL#$TESTBKMARK
 	bkmarkexists $TESTPOOL/$TESTFS#$TESTBKMARK && \