linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] btrfs-progs: cleanup btrfs-rescue output msgs
@ 2014-05-15  1:29 Gui Hecheng
  2014-05-15  1:29 ` [PATCH 2/3] btrfs-progs: add missing help option for rescue super-recover Gui Hecheng
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Gui Hecheng @ 2014-05-15  1:29 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Gui Hecheng

Use enum defined error codes to represent different kinds of errs
for super-recover and chunk-recover.
---
 chunk-recover.c | 35 ++++++++++++++++++++++++++---
 cmds-rescue.c   | 36 ++++++++++++++++++++----------
 super-recover.c | 69 ++++++++++++++++++++++++++++-----------------------------
 3 files changed, 90 insertions(+), 50 deletions(-)

diff --git a/chunk-recover.c b/chunk-recover.c
index 613d715..84fd8b7 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -82,6 +82,27 @@ struct device_scan {
 	int fd;
 };
 
+enum chunk_recover_err_code {
+	cr_notused,
+	ERR_CR_NO_RECOVER,
+	ERR_CR_FAILED_TO_RECOVER,
+	ERR_CR_ABORTED_TO_RECOVER,
+};
+
+char *chunk_recover_err_str(int err)
+{
+	switch (err) {
+	case ERR_CR_NO_RECOVER:
+		return "All chunks are good, no need to recover\n";
+	case ERR_CR_FAILED_TO_RECOVER:
+		return "Failed to recover the chunk tree\n";
+	case ERR_CR_ABORTED_TO_RECOVER:
+		return "Aborted to recover bad superblocks\n";
+	default:
+		return "Unkown recover result\n";
+	}
+}
+
 static struct extent_record *btrfs_new_extent_record(struct extent_buffer *eb)
 {
 	struct extent_record *rec;
@@ -2092,12 +2113,14 @@ int btrfs_recover_chunk_tree(char *path, int verbose, int yes)
 	ret = recover_prepare(&rc, path);
 	if (ret) {
 		fprintf(stderr, "recover prepare error\n");
+		ret = ERR_CR_FAILED_TO_RECOVER;
 		return ret;
 	}
 
 	ret = scan_devices(&rc);
 	if (ret) {
 		fprintf(stderr, "scan chunk headers error\n");
+		ret = ERR_CR_FAILED_TO_RECOVER;
 		goto fail_rc;
 	}
 
@@ -2105,6 +2128,7 @@ int btrfs_recover_chunk_tree(char *path, int verbose, int yes)
 	    cache_tree_empty(&rc.bg.tree) &&
 	    cache_tree_empty(&rc.devext.tree)) {
 		fprintf(stderr, "no recoverable chunk\n");
+		ret = ERR_CR_NO_RECOVER;
 		goto fail_rc;
 	}
 
@@ -2117,8 +2141,10 @@ int btrfs_recover_chunk_tree(char *path, int verbose, int yes)
 		if (!list_empty(&rc.bg.block_groups) ||
 		    !list_empty(&rc.devext.no_chunk_orphans)) {
 			ret = btrfs_recover_chunks(&rc);
-			if (ret)
+			if (ret) {
+				ret = ERR_CR_FAILED_TO_RECOVER;
 				goto fail_rc;
+			}
 		}
 		/*
 		 * If the chunk is healthy, its block group item and device
@@ -2129,32 +2155,35 @@ int btrfs_recover_chunk_tree(char *path, int verbose, int yes)
 		 */
 	} else {
 		fprintf(stderr, "Check chunks successfully with no orphans\n");
+		ret = ERR_CR_NO_RECOVER;
 		goto fail_rc;
 	}
 
 	root = open_ctree_with_broken_chunk(&rc);
 	if (IS_ERR(root)) {
 		fprintf(stderr, "open with broken chunk error\n");
-		ret = PTR_ERR(root);
+		ret = ERR_CR_FAILED_TO_RECOVER;
 		goto fail_rc;
 	}
 
 	ret = check_all_chunks_by_metadata(&rc, root);
 	if (ret) {
 		fprintf(stderr, "The chunks in memory can not match the metadata of the fs. Repair failed.\n");
+		ret = ERR_CR_FAILED_TO_RECOVER;
 		goto fail_close_ctree;
 	}
 
 	ret = btrfs_rebuild_ordered_data_chunk_stripes(&rc, root);
 	if (ret) {
 		fprintf(stderr, "Failed to rebuild ordered chunk stripes.\n");
+		ret = ERR_CR_FAILED_TO_RECOVER;
 		goto fail_close_ctree;
 	}
 
 	if (!rc.yes) {
 		ret = ask_user("We are going to rebuild the chunk tree on disk, it might destroy the old metadata on the disk, Are you sure?");
 		if (!ret) {
-			ret = 1;
+			ret = ERR_CR_ABORTED_TO_RECOVER;
 			goto fail_close_ctree;
 		}
 	}
diff --git a/cmds-rescue.c b/cmds-rescue.c
index f20a206..7e0687e 100644
--- a/cmds-rescue.c
+++ b/cmds-rescue.c
@@ -30,6 +30,9 @@ static const char * const rescue_cmd_group_usage[] = {
 int btrfs_recover_chunk_tree(char *path, int verbose, int yes);
 int btrfs_recover_superblocks(char *path, int verbose, int yes);
 
+char *chunk_recover_err_str(int err);
+char *super_recover_err_str(int err);
+
 const char * const cmd_chunk_recover_usage[] = {
 	"btrfs rescue chunk-recover [options] <device>",
 	"Recover the chunk tree by scanning the devices one by one.",
@@ -49,6 +52,13 @@ const char * const cmd_super_recover_usage[] = {
 	NULL
 };
 
+/*
+ * return codes:
+ *   0 : Chunk recover successfully
+ *   1 : All chunks are good, no need to recover
+ *   2 : Fail to recover the chunk tree
+ *   3 : Abort to recover the on-disk chunk tree
+ */
 int cmd_chunk_recover(int argc, char *argv[])
 {
 	int ret = 0;
@@ -90,23 +100,20 @@ int cmd_chunk_recover(int argc, char *argv[])
 	}
 
 	ret = btrfs_recover_chunk_tree(file, verbose, yes);
-	if (!ret) {
-		fprintf(stdout, "Recover the chunk tree successfully.\n");
-	} else if (ret > 0) {
-		ret = 0;
-		fprintf(stdout, "Abort to rebuild the on-disk chunk tree.\n");
-	} else {
-		fprintf(stdout, "Fail to recover the chunk tree.\n");
-	}
+	if (!ret)
+		printf("Chunk recover successfully\n");
+	else if (ret > 0)
+		fprintf(stderr, "%s", chunk_recover_err_str(ret));
+
 	return ret;
 }
 
 /*
  * return codes:
- *   0 : All superblocks are valid, no need to recover
- *   1 : Usage or syntax error
- *   2 : Recover all bad superblocks successfully
- *   3 : Fail to Recover bad supeblocks
+ *   0 : Super recover successfully
+ *   1 : All superblocks are good, no need to recover
+ *   2 : Usage or syntax error
+ *   3 : Fail to recover bad supeblocks
  *   4 : Abort to recover bad superblocks
  */
 int cmd_super_recover(int argc, char **argv)
@@ -146,6 +153,11 @@ int cmd_super_recover(int argc, char **argv)
 		return 1;
 	}
 	ret = btrfs_recover_superblocks(dname, verbose, yes);
+	if (!ret)
+		printf("Super recover successfully\n");
+	else if (ret > 0)
+		fprintf(stderr, "%s", super_recover_err_str(ret));
+
 	return ret;
 }
 
diff --git a/super-recover.c b/super-recover.c
index cd6ba37..72ad884 100644
--- a/super-recover.c
+++ b/super-recover.c
@@ -56,6 +56,30 @@ struct super_block_record {
 	u64 bytenr;
 };
 
+enum super_recover_err_code {
+	sr_notused,
+	ERR_SR_NO_RECOVER,
+	ERR_SR_USAGE_OR_SYNTAX,
+	ERR_SR_FAILED_TO_RECOVER,
+	ERR_SR_ABORTED_TO_RECOVER,
+};
+
+char *super_recover_err_str(int err)
+{
+	switch (err) {
+	case ERR_SR_NO_RECOVER:
+		return "All superblocks are good, no need to recover\n";
+	case ERR_SR_USAGE_OR_SYNTAX:
+		return "Usage or syntax error\n";
+	case ERR_SR_FAILED_TO_RECOVER:
+		return "Failed to recover the chunk tree\n";
+	case ERR_SR_ABORTED_TO_RECOVER:
+		return "Aborted to recover the on-disk chunk tree\n";
+	default:
+		return "Unkown recover result\n";
+	}
+}
+
 static
 void init_recover_superblock(struct btrfs_recover_superblock *recover)
 {
@@ -253,30 +277,6 @@ static void print_all_supers(struct btrfs_recover_superblock *recover)
 	printf("\n");
 }
 
-static void recover_err_str(int ret)
-{
-	switch (ret) {
-	case 0:
-		printf("All supers are valid, no need to recover\n");
-		break;
-	case 1:
-		printf("Usage or syntax errors\n");
-		break;
-	case 2:
-		printf("Recovered bad superblocks successful\n");
-		break;
-	case 3:
-		printf("Failed to recover bad superblocks\n");
-		break;
-	case 4:
-		printf("Aborted to recover bad superblocks\n");
-		break;
-	default:
-		printf("Unknown recover result\n");
-		break;
-	}
-}
-
 int btrfs_recover_superblocks(const char *dname,
 			int verbose, int yes)
 {
@@ -288,14 +288,14 @@ int btrfs_recover_superblocks(const char *dname,
 	fd = open(dname, O_RDONLY);
 	if (fd < 0) {
 		fprintf(stderr, "open %s error\n", dname);
-		return 1;
+		return ERR_SR_USAGE_OR_SYNTAX;
 	}
 	init_recover_superblock(&recover);
 
 	ret = btrfs_scan_fs_devices(fd, dname, &recover.fs_devices, 0, 0);
 	close(fd);
 	if (ret) {
-		ret = 1;
+		ret = ERR_SR_USAGE_OR_SYNTAX;
 		goto no_recover;
 	}
 
@@ -304,7 +304,7 @@ int btrfs_recover_superblocks(const char *dname,
 
 	ret = read_fs_supers(&recover);
 	if (ret) {
-		ret = 1;
+		ret = ERR_SR_USAGE_OR_SYNTAX;
 		goto no_recover;
 	}
 	if (verbose) {
@@ -312,13 +312,15 @@ int btrfs_recover_superblocks(const char *dname,
 		print_all_supers(&recover);
 	}
 
-	if (list_empty(&recover.bad_supers))
+	if (list_empty(&recover.bad_supers)) {
+		ret = ERR_SR_NO_RECOVER;
 		goto no_recover;
+	}
 
 	if (!yes) {
 		ret = ask_user("Make sure this is a btrfs disk otherwise the tool will destroy other fs, Are you sure?");
 		if (!ret) {
-			ret = 4;
+			ret = ERR_SR_ABORTED_TO_RECOVER;
 			goto no_recover;
 		}
 	}
@@ -326,20 +328,17 @@ int btrfs_recover_superblocks(const char *dname,
 	root = open_ctree(record->device_name, record->bytenr,
 			  OPEN_CTREE_RECOVER_SUPER | OPEN_CTREE_WRITES);
 	if (!root) {
-		ret = 3;
+		ret = ERR_SR_FAILED_TO_RECOVER;
 		goto no_recover;
 	}
 	/* reset super_bytenr in order that we will rewite all supers */
 	root->fs_info->super_bytenr = BTRFS_SUPER_INFO_OFFSET;
 	ret = write_all_supers(root);
-	if (!ret)
-		ret = 2;
-	else
-		ret = 3;
+	if (ret)
+		ret = ERR_SR_FAILED_TO_RECOVER;
 
 	close_ctree(root);
 no_recover:
-	recover_err_str(ret);
 	free_recover_superblock(&recover);
 	return ret;
 }
-- 
1.8.1.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/3] btrfs-progs: add missing help option for rescue super-recover
  2014-05-15  1:29 [PATCH 1/3] btrfs-progs: cleanup btrfs-rescue output msgs Gui Hecheng
@ 2014-05-15  1:29 ` Gui Hecheng
  2014-05-16 16:37   ` David Sterba
  2014-05-15  1:29 ` [PATCH 3/3] btrfs-progs: use check_argc_exact to check arg number of btrfs-rescue Gui Hecheng
  2014-05-28 16:24 ` [PATCH 1/3] btrfs-progs: cleanup btrfs-rescue output msgs David Sterba
  2 siblings, 1 reply; 8+ messages in thread
From: Gui Hecheng @ 2014-05-15  1:29 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Gui Hecheng

Add '-h' option for help for super-recover,
update the manpage at the same time.
---
 Documentation/btrfs-rescue.txt | 2 ++
 cmds-rescue.c                  | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/btrfs-rescue.txt b/Documentation/btrfs-rescue.txt
index f66ca85..f64d78b 100644
--- a/Documentation/btrfs-rescue.txt
+++ b/Documentation/btrfs-rescue.txt
@@ -39,6 +39,8 @@ Recover bad superblocks from good copies.
 assume an answer of 'yes' to all questions.
 -v::::
 verbose mode.
+-h::::
+help.
 
 EXIT STATUS
 -----------
diff --git a/cmds-rescue.c b/cmds-rescue.c
index 7e0687e..9491d0c 100644
--- a/cmds-rescue.c
+++ b/cmds-rescue.c
@@ -49,6 +49,7 @@ const char * const cmd_super_recover_usage[] = {
 	"",
 	"-y	Assume an answer of `yes' to all questions",
 	"-v	Verbose mode",
+	"-h	Help",
 	NULL
 };
 
@@ -124,7 +125,7 @@ int cmd_super_recover(int argc, char **argv)
 	char *dname;
 
 	while (1) {
-		int c = getopt(argc, argv, "vy");
+		int c = getopt(argc, argv, "vyh");
 		if (c < 0)
 			break;
 		switch (c) {
@@ -134,6 +135,7 @@ int cmd_super_recover(int argc, char **argv)
 		case 'y':
 			yes = 1;
 			break;
+		case 'h':
 		default:
 			usage(cmd_super_recover_usage);
 		}
-- 
1.8.1.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/3] btrfs-progs: use check_argc_exact to check arg number of btrfs-rescue
  2014-05-15  1:29 [PATCH 1/3] btrfs-progs: cleanup btrfs-rescue output msgs Gui Hecheng
  2014-05-15  1:29 ` [PATCH 2/3] btrfs-progs: add missing help option for rescue super-recover Gui Hecheng
@ 2014-05-15  1:29 ` Gui Hecheng
  2014-05-28 16:24 ` [PATCH 1/3] btrfs-progs: cleanup btrfs-rescue output msgs David Sterba
  2 siblings, 0 replies; 8+ messages in thread
From: Gui Hecheng @ 2014-05-15  1:29 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Gui Hecheng

The btrfs-rescue accepts exactly one arg for both
chunk-recover & super-recover, use check_argc_exact clearly.
---
 cmds-rescue.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cmds-rescue.c b/cmds-rescue.c
index 9491d0c..3629141 100644
--- a/cmds-rescue.c
+++ b/cmds-rescue.c
@@ -85,7 +85,7 @@ int cmd_chunk_recover(int argc, char *argv[])
 	}
 
 	argc = argc - optind;
-	if (argc == 0)
+	if (check_argc_exact(argc, 1))
 		usage(cmd_chunk_recover_usage);
 
 	file = argv[optind];
@@ -141,7 +141,7 @@ int cmd_super_recover(int argc, char **argv)
 		}
 	}
 	argc = argc - optind;
-	if (argc != 1)
+	if (check_argc_exact(argc, 1))
 		usage(cmd_super_recover_usage);
 
 	dname = argv[optind];
-- 
1.8.1.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] btrfs-progs: add missing help option for rescue super-recover
  2014-05-15  1:29 ` [PATCH 2/3] btrfs-progs: add missing help option for rescue super-recover Gui Hecheng
@ 2014-05-16 16:37   ` David Sterba
  2014-05-16 16:41     ` David Sterba
  0 siblings, 1 reply; 8+ messages in thread
From: David Sterba @ 2014-05-16 16:37 UTC (permalink / raw)
  To: Gui Hecheng; +Cc: linux-btrfs

On Thu, May 15, 2014 at 09:29:08AM +0800, Gui Hecheng wrote:
> Add '-h' option for help for super-recover,
> update the manpage at the same time.

We don't have the short option for help, a few patches have been already
rejected to change that.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] btrfs-progs: add missing help option for rescue super-recover
  2014-05-16 16:37   ` David Sterba
@ 2014-05-16 16:41     ` David Sterba
  2014-05-19  1:35       ` Gui Hecheng
  0 siblings, 1 reply; 8+ messages in thread
From: David Sterba @ 2014-05-16 16:41 UTC (permalink / raw)
  To: dsterba, Gui Hecheng, linux-btrfs

On Fri, May 16, 2014 at 06:37:27PM +0200, David Sterba wrote:
> On Thu, May 15, 2014 at 09:29:08AM +0800, Gui Hecheng wrote:
> > Add '-h' option for help for super-recover,
> > update the manpage at the same time.
> 
> We don't have the short option for help, a few patches have been already
> rejected to change that.

Reference: https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg30759.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] btrfs-progs: add missing help option for rescue super-recover
  2014-05-16 16:41     ` David Sterba
@ 2014-05-19  1:35       ` Gui Hecheng
  0 siblings, 0 replies; 8+ messages in thread
From: Gui Hecheng @ 2014-05-19  1:35 UTC (permalink / raw)
  To: dsterba; +Cc: linux-btrfs

On Fri, 2014-05-16 at 18:41 +0200, David Sterba wrote:
> On Fri, May 16, 2014 at 06:37:27PM +0200, David Sterba wrote:
> > On Thu, May 15, 2014 at 09:29:08AM +0800, Gui Hecheng wrote:
> > > Add '-h' option for help for super-recover,
> > > update the manpage at the same time.
> > 
> > We don't have the short option for help, a few patches have been already
> > rejected to change that.
> 
> Reference: https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg30759.html

Yes, I get it. Thanks David.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] btrfs-progs: cleanup btrfs-rescue output msgs
  2014-05-15  1:29 [PATCH 1/3] btrfs-progs: cleanup btrfs-rescue output msgs Gui Hecheng
  2014-05-15  1:29 ` [PATCH 2/3] btrfs-progs: add missing help option for rescue super-recover Gui Hecheng
  2014-05-15  1:29 ` [PATCH 3/3] btrfs-progs: use check_argc_exact to check arg number of btrfs-rescue Gui Hecheng
@ 2014-05-28 16:24 ` David Sterba
  2014-05-29  1:52   ` Gui Hecheng
  2 siblings, 1 reply; 8+ messages in thread
From: David Sterba @ 2014-05-28 16:24 UTC (permalink / raw)
  To: Gui Hecheng; +Cc: linux-btrfs

On Thu, May 15, 2014 at 09:29:07AM +0800, Gui Hecheng wrote:
> Use enum defined error codes to represent different kinds of errs
> for super-recover and chunk-recover.

I think this change hides the low-level errors (like ENOMEM) that can
possibly result into "recovery not possible", though it can be restarted
and could work fine.

The human readable error messages are good, but should also reflect if
the error was fatal or not and say "why".

Examples:

> @@ -2092,12 +2113,14 @@ int btrfs_recover_chunk_tree(char *path, int verbose, int yes)
>  	ret = recover_prepare(&rc, path);
>  	if (ret) {
>  		fprintf(stderr, "recover prepare error\n");
> +		ret = ERR_CR_FAILED_TO_RECOVER;

eg. recover_prepare can fail if it does not find the path or due to ENOMEM

>  		return ret;
>  	}
>  
>  	ret = scan_devices(&rc);
>  	if (ret) {
>  		fprintf(stderr, "scan chunk headers error\n");
> +		ret = ERR_CR_FAILED_TO_RECOVER;

device open fails, or ENOMEM

>  		goto fail_rc;
>  	}

So, somehow wrap both values into one and convert into the enhanced
messages.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] btrfs-progs: cleanup btrfs-rescue output msgs
  2014-05-28 16:24 ` [PATCH 1/3] btrfs-progs: cleanup btrfs-rescue output msgs David Sterba
@ 2014-05-29  1:52   ` Gui Hecheng
  0 siblings, 0 replies; 8+ messages in thread
From: Gui Hecheng @ 2014-05-29  1:52 UTC (permalink / raw)
  To: dsterba; +Cc: linux-btrfs

On Wed, 2014-05-28 at 18:24 +0200, David Sterba wrote:
> On Thu, May 15, 2014 at 09:29:07AM +0800, Gui Hecheng wrote:
> > Use enum defined error codes to represent different kinds of errs
> > for super-recover and chunk-recover.
> 
> I think this change hides the low-level errors (like ENOMEM) that can
> possibly result into "recovery not possible", though it can be restarted
> and could work fine.
> 
> The human readable error messages are good, but should also reflect if
> the error was fatal or not and say "why".
> 
> Examples:
> 
> > @@ -2092,12 +2113,14 @@ int btrfs_recover_chunk_tree(char *path, int verbose, int yes)
> >  	ret = recover_prepare(&rc, path);
> >  	if (ret) {
> >  		fprintf(stderr, "recover prepare error\n");
> > +		ret = ERR_CR_FAILED_TO_RECOVER;
> 
> eg. recover_prepare can fail if it does not find the path or due to ENOMEM
> 
> >  		return ret;
> >  	}
> >  
> >  	ret = scan_devices(&rc);
> >  	if (ret) {
> >  		fprintf(stderr, "scan chunk headers error\n");
> > +		ret = ERR_CR_FAILED_TO_RECOVER;
> 
> device open fails, or ENOMEM
> 
> >  		goto fail_rc;
> >  	}
> 
> So, somehow wrap both values into one and convert into the enhanced
> messages.

Hi David,
Thanks for your advice. I'll rework it soon and resend.

Something to confirm below:

o Actually I've kept almost all the "fprintf"s in the original place as
*low level* messages(except an "PTR_ERR", I'll add it back).
But it seems that the original "fprintf"s do hide low level errors, and
I'll try to enhance them in the original "fprintf"s.

o What I've added are just some *user level* messages which will show
along with the low level messages, bug not replace them.
I would like to just keep this part.

What do you think?

-Gui



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-05-29  1:57 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-15  1:29 [PATCH 1/3] btrfs-progs: cleanup btrfs-rescue output msgs Gui Hecheng
2014-05-15  1:29 ` [PATCH 2/3] btrfs-progs: add missing help option for rescue super-recover Gui Hecheng
2014-05-16 16:37   ` David Sterba
2014-05-16 16:41     ` David Sterba
2014-05-19  1:35       ` Gui Hecheng
2014-05-15  1:29 ` [PATCH 3/3] btrfs-progs: use check_argc_exact to check arg number of btrfs-rescue Gui Hecheng
2014-05-28 16:24 ` [PATCH 1/3] btrfs-progs: cleanup btrfs-rescue output msgs David Sterba
2014-05-29  1:52   ` Gui Hecheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).