[PATCH 0/2] btrfs-progs: btrfslabel source code consolidation

[PATCH 0/2] btrfs-progs: btrfslabel source code consolidation

[Jeff Liu]

Hello,

As per David's suggestions, we should remove btrfslabel source code and move the
related functions to cmds-filesystems or utils.

I prefer to move them into utils since there already has a check_label() function
which can be used for the pre-checkup against the input label. 

This patch set is based on my previous v5 patches of "add get/set label support
against a mounted filesystem":
http://permalink.gmane.org/gmane.comp.file-systems.btrfs/21713


Thanks,
-Jeff

[PATCH 1/2] btrfs-progs: refactor check_label()

[Jeff Liu]

Refactor check_label().

Make it be static at first, this is a preparation step since we'll remove
btrfslabel.[c|h] and move those functions at them to utils.[c|h], we can do
pre-checking against the input label string with it.

Also, fix the input lable length verfication from BTRFS_LABEL_SIZE to
BTRFS_LABEL_SIZE - 1.

Signed-off-by: Jie Liu <jeff.liu@oracle.com>
CC: David Sterba <dsterba@suse.cz>
CC: Gene Czarcinski <gene@czarc.net>
---
 utils.c |    8 ++++++--
 utils.h |    1 -
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/utils.c b/utils.c
index d59bca3..034da8f 100644
--- a/utils.c
+++ b/utils.c
@@ -1122,17 +1122,21 @@ char *pretty_sizes(u64 size)
       -1    if the label is too long
       -2    if the label contains an invalid character
  */
-int check_label(char *input)
+static int check_label(char *input)
 {
        int i;
        int len = strlen(input);
 
-       if (len > BTRFS_LABEL_SIZE) {
+       if (len > BTRFS_LABEL_SIZE - 1) {
+		fprintf(stderr, "ERROR: Label %s is too long (max %d)\n",
+			input, BTRFS_LABEL_SIZE - 1);
                return -1;
        }
 
        for (i = 0; i < len; i++) {
                if (input[i] == '/' || input[i] == '\\') {
+			fprintf(stderr, "ERROR: Label %s contains invalid "
+				"characters\n", input);
                        return -2;
                }
        }
diff --git a/utils.h b/utils.h
index 8750f28..a0b782b 100644
--- a/utils.h
+++ b/utils.h
@@ -42,7 +42,6 @@ int check_mounted_where(int fd, const char *file, char *where, int size,
 int btrfs_device_already_in_root(struct btrfs_root *root, int fd,
 				 int super_offset);
 char *pretty_sizes(u64 size);
-int check_label(char *input);
 int get_mountpt(char *dev, char *mntpt, size_t size);
 
 int btrfs_scan_block_devices(int run_ioctl);
-- 
1.7.9.5

Re: [PATCH 1/2] btrfs-progs: refactor check_label()

[David Sterba]

On Tue, Jan 29, 2013 at 02:24:12PM +0800, Jeff Liu wrote:
> --- a/utils.c
> +++ b/utils.c
> @@ -1122,17 +1122,21 @@ char *pretty_sizes(u64 size)
>        -1    if the label is too long
>        -2    if the label contains an invalid character
>   */
> -int check_label(char *input)
> +static int check_label(char *input)
>  {
>         int i;
>         int len = strlen(input);
>  
> -       if (len > BTRFS_LABEL_SIZE) {
> +       if (len > BTRFS_LABEL_SIZE - 1) {
> +		fprintf(stderr, "ERROR: Label %s is too long (max %d)\n",
> +			input, BTRFS_LABEL_SIZE - 1);
>                 return -1;
>         }
>  
>         for (i = 0; i < len; i++) {
>                 if (input[i] == '/' || input[i] == '\\') {
> +			fprintf(stderr, "ERROR: Label %s contains invalid "
> +				"characters\n", input);
>                         return -2;
>                 }

Plase drop this check, see
http://repo.or.cz/w/btrfs-progs-unstable/devel.git/commit/79e0e445fc2365e47fc7f060d5a4445d37e184b8
(also function comment and maybe the callers)

"btrfs-progs: kill check for /'s in labels

This patch kills a check in mkfs's label stuff which doesn't allow
labels that have /'s in them.  This causes problems for Anaconda which
try to label volumes with their mountpoints."
(mkfs.c)

thanks,
david

Re: [PATCH 1/2] btrfs-progs: refactor check_label()

[Jeff Liu]

On 01/29/2013 11:19 PM, David Sterba wrote:
> On Tue, Jan 29, 2013 at 02:24:12PM +0800, Jeff Liu wrote:
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -1122,17 +1122,21 @@ char *pretty_sizes(u64 size)
>>        -1    if the label is too long
>>        -2    if the label contains an invalid character
>>   */
>> -int check_label(char *input)
>> +static int check_label(char *input)
>>  {
>>         int i;
>>         int len = strlen(input);
>>  
>> -       if (len > BTRFS_LABEL_SIZE) {
>> +       if (len > BTRFS_LABEL_SIZE - 1) {
>> +		fprintf(stderr, "ERROR: Label %s is too long (max %d)\n",
>> +			input, BTRFS_LABEL_SIZE - 1);
>>                 return -1;
>>         }
>>  
>>         for (i = 0; i < len; i++) {
>>                 if (input[i] == '/' || input[i] == '\\') {
>> +			fprintf(stderr, "ERROR: Label %s contains invalid "
>> +				"characters\n", input);
>>                         return -2;
>>                 }
> 
> Plase drop this check, see
> http://repo.or.cz/w/btrfs-progs-unstable/devel.git/commit/79e0e445fc2365e47fc7f060d5a4445d37e184b8
> (also function comment and maybe the callers)
> 
> "btrfs-progs: kill check for /'s in labels
> 
> This patch kills a check in mkfs's label stuff which doesn't allow
> labels that have /'s in them.  This causes problems for Anaconda which
> try to label volumes with their mountpoints."
> (mkfs.c)
Ok, so looks we can safely clean this routine out of the code base since
there is no other users call it if am not missing anything.

Thanks,
-Jeff

[PATCH 2/2] btrfs-progs: remove btrfslabel.[c|h]

[Jeff Liu]

Clean btrfslabel.[c|h] out of the source tree and move those related
functions to utils.[c|h].

Signed-off-by: Jie Liu <jeff.liu@oracle.com>
CC: David Sterba <dsterba@suse.cz>
CC: Gene Czarcinski <gene@czarc.net>
---
 Makefile          |    4 +-
 btrfslabel.c      |  178 -----------------------------------------------------
 btrfslabel.h      |    5 --
 cmds-filesystem.c |    1 -
 utils.c           |  129 ++++++++++++++++++++++++++++++++++++++
 utils.h           |    2 +
 6 files changed, 133 insertions(+), 186 deletions(-)
 delete mode 100644 btrfslabel.c
 delete mode 100644 btrfslabel.h

diff --git a/Makefile b/Makefile
index 4894903..e54b21e 100644
--- a/Makefile
+++ b/Makefile
@@ -4,8 +4,8 @@ CFLAGS = -g -O1
 objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \
 	  root-tree.o dir-item.o file-item.o inode-item.o \
 	  inode-map.o crc32c.o rbtree.o extent-cache.o extent_io.o \
-	  volumes.o utils.o btrfs-list.o btrfslabel.o repair.o \
-	  send-stream.o send-utils.o qgroup.o
+	  volumes.o utils.o btrfs-list.o repair.o send-stream.o \
+	  send-utils.o qgroup.o
 cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
 	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
 	       cmds-quota.o cmds-qgroup.o
diff --git a/btrfslabel.c b/btrfslabel.c
deleted file mode 100644
index 2826050..0000000
--- a/btrfslabel.c
+++ /dev/null
@@ -1,178 +0,0 @@
-/*
- * Copyright (C) 2008 Morey Roof.   All rights reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public
- * License v2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public
- * License along with this program; if not, write to the
- * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
- * Boston, MA 021110-1307, USA.
- */
-
-#define _GNU_SOURCE
-
-#ifndef __CHECKER__
-#include <sys/ioctl.h>
-#include <sys/mount.h>
-#include "ioctl.h"
-#endif /* __CHECKER__ */
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <dirent.h>
-#include <fcntl.h>
-#include <unistd.h>
-#include <linux/fs.h>
-#include <linux/limits.h>
-#include <ctype.h>
-#include "kerncompat.h"
-#include "ctree.h"
-#include "utils.h"
-#include "version.h"
-#include "disk-io.h"
-#include "transaction.h"
-
-#define MOUNTED                        1
-#define UNMOUNTED                      2
-#define GET_LABEL                      3
-#define SET_LABEL                      4
-
-static int set_label_unmounted(const char *dev, const char *label)
-{
-	struct btrfs_trans_handle *trans;
-	struct btrfs_root *root;
-	int ret;
-
-	ret = check_mounted(dev);
-	if (ret < 0) {
-	       fprintf(stderr, "FATAL: error checking %s mount status\n", dev);
-	       return -1;
-	}
-	if (ret > 0) {
-		fprintf(stderr, "ERROR: dev %s is mounted, use mount point\n",
-			dev);
-		return -1;
-	}
-
-	if (strlen(label) > BTRFS_LABEL_SIZE - 1) {
-		fprintf(stderr, "ERROR: Label %s is too long (max %d)\n",
-			label, BTRFS_LABEL_SIZE - 1);
-		return -1;
-	}
-
-	/* Open the super_block at the default location
-	 * and as read-write.
-	 */
-	root = open_ctree(dev, 0, 1);
-	if (!root) /* errors are printed by open_ctree() */
-		return -1;
-
-	trans = btrfs_start_transaction(root, 1);
-	snprintf(root->fs_info->super_copy.label, BTRFS_LABEL_SIZE, "%s",
-		 label);
-	btrfs_commit_transaction(trans, root);
-
-	/* Now we close it since we are done. */
-	close_ctree(root);
-	return 0;
-}
-
-static int set_label_mounted(const char *mount_path, const char *label)
-{
-	int fd;
-
-	fd = open(mount_path, O_RDONLY | O_NOATIME);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: unable access to '%s'\n", mount_path);
-		return -1;
-	}
-
-	if (ioctl(fd, BTRFS_IOC_SET_FSLABEL, label) < 0) {
-		fprintf(stderr, "ERROR: unable to set label %s\n",
-			strerror(errno));
-		close(fd);
-		return -1;
-	}
-
-	return 0;
-}
-
-static int get_label_unmounted(const char *dev)
-{
-	struct btrfs_root *root;
-	int ret;
-
-	ret = check_mounted(dev);
-	if (ret < 0) {
-	       fprintf(stderr, "FATAL: error checking %s mount status\n", dev);
-	       return -1;
-	}
-	if (ret > 0) {
-		fprintf(stderr, "ERROR: dev %s is mounted, use mount point\n",
-			dev);
-		return -1;
-	}
-
-	/* Open the super_block at the default location
-	 * and as read-only.
-	 */
-	root = open_ctree(dev, 0, 0);
-	if(!root)
-		return -1;
-
-	fprintf(stdout, "%s\n", root->fs_info->super_copy.label);
-
-	/* Now we close it since we are done. */
-	close_ctree(root);
-	return 0;
-}
-
-/*
- * If a partition is mounted, try to get the filesystem label via its
- * mounted path rather than device.  Return the corresponding error
- * the user specified the device path.
- */
-static int get_label_mounted(const char *mount_path)
-{
-	char label[BTRFS_LABEL_SIZE];
-	int fd;
-
-	fd = open(mount_path, O_RDONLY | O_NOATIME);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: unable access to '%s'\n", mount_path);
-		return -1;
-	}
-
-	memset(label, '\0', sizeof(label));
-	if (ioctl(fd, BTRFS_IOC_GET_FSLABEL, label) < 0) {
-		fprintf(stderr, "ERROR: unable get label %s\n", strerror(errno));
-		close(fd);
-		return -1;
-	}
-
-	fprintf(stdout, "%s\n", label);
-	return 0;
-}
-
-int get_label(const char *btrfs_dev)
-{
-	return is_existing_blk_or_reg_file(btrfs_dev) ?
-		get_label_unmounted(btrfs_dev) :
-		get_label_mounted(btrfs_dev);
-}
-
-int set_label(char *btrfs_dev, char *label)
-{
-	return is_existing_blk_or_reg_file(btrfs_dev) ?
-		set_label_unmounted(btrfs_dev, label) :
-		set_label_mounted(btrfs_dev, label);
-}
diff --git a/btrfslabel.h b/btrfslabel.h
deleted file mode 100644
index abf43ad..0000000
--- a/btrfslabel.h
+++ /dev/null
@@ -1,5 +0,0 @@
-/* btrflabel.h */
-
-
-int get_label(char *btrfs_dev);
-int set_label(char *btrfs_dev, char *nLabel);
\ No newline at end of file
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 5770d8b..3752703 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -32,7 +32,6 @@
 #include "version.h"
 
 #include "commands.h"
-#include "btrfslabel.h"
 
 static const char * const filesystem_cmd_group_usage[] = {
 	"btrfs filesystem [<group>] <command> [<args>]",
diff --git a/utils.c b/utils.c
index 034da8f..d65bc35 100644
--- a/utils.c
+++ b/utils.c
@@ -16,6 +16,7 @@
  * Boston, MA 021110-1307, USA.
  */
 
+#define _GNU_SOURCE
 #define _XOPEN_SOURCE 600
 #define __USE_XOPEN2K
 #include <stdio.h>
@@ -1144,6 +1145,134 @@ static int check_label(char *input)
        return 0;
 }
 
+static int set_label_unmounted(const char *dev, const char *label)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_root *root;
+	int ret;
+
+	ret = check_mounted(dev);
+	if (ret < 0) {
+	       fprintf(stderr, "FATAL: error checking %s mount status\n", dev);
+	       return -1;
+	}
+	if (ret > 0) {
+		fprintf(stderr, "ERROR: dev %s is mounted, use mount point\n",
+			dev);
+		return -1;
+	}
+
+	/* Open the super_block at the default location
+	 * and as read-write.
+	 */
+	root = open_ctree(dev, 0, 1);
+	if (!root) /* errors are printed by open_ctree() */
+		return -1;
+
+	trans = btrfs_start_transaction(root, 1);
+	snprintf(root->fs_info->super_copy.label, BTRFS_LABEL_SIZE, "%s",
+		 label);
+	btrfs_commit_transaction(trans, root);
+
+	/* Now we close it since we are done. */
+	close_ctree(root);
+	return 0;
+}
+
+static int set_label_mounted(const char *mount_path, const char *label)
+{
+	int fd;
+
+	fd = open(mount_path, O_RDONLY | O_NOATIME);
+	if (fd < 0) {
+		fprintf(stderr, "ERROR: unable access to '%s'\n", mount_path);
+		return -1;
+	}
+
+	if (ioctl(fd, BTRFS_IOC_SET_FSLABEL, label) < 0) {
+		fprintf(stderr, "ERROR: unable to set label %s\n",
+			strerror(errno));
+		close(fd);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int get_label_unmounted(const char *dev)
+{
+	struct btrfs_root *root;
+	int ret;
+
+	ret = check_mounted(dev);
+	if (ret < 0) {
+	       fprintf(stderr, "FATAL: error checking %s mount status\n", dev);
+	       return -1;
+	}
+	if (ret > 0) {
+		fprintf(stderr, "ERROR: dev %s is mounted, use mount point\n",
+			dev);
+		return -1;
+	}
+
+	/* Open the super_block at the default location
+	 * and as read-only.
+	 */
+	root = open_ctree(dev, 0, 0);
+	if(!root)
+		return -1;
+
+	fprintf(stdout, "%s\n", root->fs_info->super_copy.label);
+
+	/* Now we close it since we are done. */
+	close_ctree(root);
+	return 0;
+}
+
+/*
+ * If a partition is mounted, try to get the filesystem label via its
+ * mounted path rather than device.  Return the corresponding error
+ * the user specified the device path.
+ */
+static int get_label_mounted(const char *mount_path)
+{
+	char label[BTRFS_LABEL_SIZE];
+	int fd;
+
+	fd = open(mount_path, O_RDONLY | O_NOATIME);
+	if (fd < 0) {
+		fprintf(stderr, "ERROR: unable access to '%s'\n", mount_path);
+		return -1;
+	}
+
+	memset(label, '\0', sizeof(label));
+	if (ioctl(fd, BTRFS_IOC_GET_FSLABEL, label) < 0) {
+		fprintf(stderr, "ERROR: unable get label %s\n", strerror(errno));
+		close(fd);
+		return -1;
+	}
+
+	fprintf(stdout, "%s\n", label);
+	return 0;
+}
+
+int get_label(const char *btrfs_dev)
+{
+	return is_existing_blk_or_reg_file(btrfs_dev) ?
+		get_label_unmounted(btrfs_dev) :
+		get_label_mounted(btrfs_dev);
+}
+
+int set_label(const char *btrfs_dev, const char *label)
+{
+	if (check_label(label))
+		return -1;
+
+	return is_existing_blk_or_reg_file(btrfs_dev) ?
+		set_label_unmounted(btrfs_dev, label) :
+		set_label_mounted(btrfs_dev, label);
+}
+
 int btrfs_scan_block_devices(int run_ioctl)
 {
 
diff --git a/utils.h b/utils.h
index a0b782b..632b01c 100644
--- a/utils.h
+++ b/utils.h
@@ -46,4 +46,6 @@ int get_mountpt(char *dev, char *mntpt, size_t size);
 
 int btrfs_scan_block_devices(int run_ioctl);
 int is_existing_blk_or_reg_file(const char* filename);
+int get_label(const char *btrfs_dev);
+int set_label(const char *btrfs_dev, const char *label);
 #endif
-- 
1.7.9.5

Re: [PATCH 2/2] btrfs-progs: remove btrfslabel.[c|h]

[Stefan Behrens]

On Tue, 29 Jan 2013 14:24:13 +0800, Jeff Liu wrote:
> Clean btrfslabel.[c|h] out of the source tree and move those related
> functions to utils.[c|h].
> 
> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
> CC: David Sterba <dsterba@suse.cz>
> CC: Gene Czarcinski <gene@czarc.net>
> ---
>  Makefile          |    4 +-
>  btrfslabel.c      |  178 -----------------------------------------------------
>  btrfslabel.h      |    5 --
>  cmds-filesystem.c |    1 -
>  utils.c           |  129 ++++++++++++++++++++++++++++++++++++++
>  utils.h           |    2 +
>  6 files changed, 133 insertions(+), 186 deletions(-)
>  delete mode 100644 btrfslabel.c
>  delete mode 100644 btrfslabel.h
> 
> diff --git a/Makefile b/Makefile
> index 4894903..e54b21e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -4,8 +4,8 @@ CFLAGS = -g -O1
>  objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \
>  	  root-tree.o dir-item.o file-item.o inode-item.o \
>  	  inode-map.o crc32c.o rbtree.o extent-cache.o extent_io.o \
> -	  volumes.o utils.o btrfs-list.o btrfslabel.o repair.o \
> -	  send-stream.o send-utils.o qgroup.o
> +	  volumes.o utils.o btrfs-list.o repair.o send-stream.o \
> +	  send-utils.o qgroup.o
>  cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
>  	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
>  	       cmds-quota.o cmds-qgroup.o
> diff --git a/btrfslabel.c b/btrfslabel.c
> deleted file mode 100644
> index 2826050..0000000
> --- a/btrfslabel.c
> +++ /dev/null
> @@ -1,178 +0,0 @@
> -/*
> - * Copyright (C) 2008 Morey Roof.   All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public
> - * License v2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public
> - * License along with this program; if not, write to the
> - * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
> - * Boston, MA 021110-1307, USA.
> - */
> -
> -#define _GNU_SOURCE
> -
> -#ifndef __CHECKER__
> -#include <sys/ioctl.h>
> -#include <sys/mount.h>
> -#include "ioctl.h"
> -#endif /* __CHECKER__ */
> -
> -#include <stdio.h>
> -#include <stdlib.h>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <dirent.h>
> -#include <fcntl.h>
> -#include <unistd.h>
> -#include <linux/fs.h>
> -#include <linux/limits.h>
> -#include <ctype.h>
> -#include "kerncompat.h"
> -#include "ctree.h"
> -#include "utils.h"
> -#include "version.h"
> -#include "disk-io.h"
> -#include "transaction.h"
> -
> -#define MOUNTED                        1
> -#define UNMOUNTED                      2
> -#define GET_LABEL                      3
> -#define SET_LABEL                      4
> -
> -static int set_label_unmounted(const char *dev, const char *label)
> -{
> -	struct btrfs_trans_handle *trans;
> -	struct btrfs_root *root;
> -	int ret;
> -
> -	ret = check_mounted(dev);
> -	if (ret < 0) {
> -	       fprintf(stderr, "FATAL: error checking %s mount status\n", dev);
> -	       return -1;
> -	}
> -	if (ret > 0) {
> -		fprintf(stderr, "ERROR: dev %s is mounted, use mount point\n",
> -			dev);
> -		return -1;
> -	}
> -
> -	if (strlen(label) > BTRFS_LABEL_SIZE - 1) {
> -		fprintf(stderr, "ERROR: Label %s is too long (max %d)\n",
> -			label, BTRFS_LABEL_SIZE - 1);
> -		return -1;
> -	}
> -
> -	/* Open the super_block at the default location
> -	 * and as read-write.
> -	 */
> -	root = open_ctree(dev, 0, 1);
> -	if (!root) /* errors are printed by open_ctree() */
> -		return -1;
> -
> -	trans = btrfs_start_transaction(root, 1);
> -	snprintf(root->fs_info->super_copy.label, BTRFS_LABEL_SIZE, "%s",
> -		 label);
> -	btrfs_commit_transaction(trans, root);
> -
> -	/* Now we close it since we are done. */
> -	close_ctree(root);
> -	return 0;
> -}
> -
> -static int set_label_mounted(const char *mount_path, const char *label)
> -{
> -	int fd;
> -
> -	fd = open(mount_path, O_RDONLY | O_NOATIME);
> -	if (fd < 0) {
> -		fprintf(stderr, "ERROR: unable access to '%s'\n", mount_path);
> -		return -1;
> -	}
> -
> -	if (ioctl(fd, BTRFS_IOC_SET_FSLABEL, label) < 0) {
> -		fprintf(stderr, "ERROR: unable to set label %s\n",
> -			strerror(errno));
> -		close(fd);
> -		return -1;
> -	}
> -
> -	return 0;
> -}
> -
> -static int get_label_unmounted(const char *dev)
> -{
> -	struct btrfs_root *root;
> -	int ret;
> -
> -	ret = check_mounted(dev);
> -	if (ret < 0) {
> -	       fprintf(stderr, "FATAL: error checking %s mount status\n", dev);
> -	       return -1;
> -	}
> -	if (ret > 0) {
> -		fprintf(stderr, "ERROR: dev %s is mounted, use mount point\n",
> -			dev);
> -		return -1;
> -	}
> -
> -	/* Open the super_block at the default location
> -	 * and as read-only.
> -	 */
> -	root = open_ctree(dev, 0, 0);
> -	if(!root)
> -		return -1;
> -
> -	fprintf(stdout, "%s\n", root->fs_info->super_copy.label);
> -
> -	/* Now we close it since we are done. */
> -	close_ctree(root);
> -	return 0;
> -}
> -
> -/*
> - * If a partition is mounted, try to get the filesystem label via its
> - * mounted path rather than device.  Return the corresponding error
> - * the user specified the device path.
> - */
> -static int get_label_mounted(const char *mount_path)
> -{
> -	char label[BTRFS_LABEL_SIZE];
> -	int fd;
> -
> -	fd = open(mount_path, O_RDONLY | O_NOATIME);
> -	if (fd < 0) {
> -		fprintf(stderr, "ERROR: unable access to '%s'\n", mount_path);
> -		return -1;
> -	}
> -
> -	memset(label, '\0', sizeof(label));
> -	if (ioctl(fd, BTRFS_IOC_GET_FSLABEL, label) < 0) {
> -		fprintf(stderr, "ERROR: unable get label %s\n", strerror(errno));
> -		close(fd);
> -		return -1;
> -	}
> -
> -	fprintf(stdout, "%s\n", label);
> -	return 0;
> -}
> -
> -int get_label(const char *btrfs_dev)
> -{
> -	return is_existing_blk_or_reg_file(btrfs_dev) ?
> -		get_label_unmounted(btrfs_dev) :
> -		get_label_mounted(btrfs_dev);
> -}
> -
> -int set_label(char *btrfs_dev, char *label)
> -{
> -	return is_existing_blk_or_reg_file(btrfs_dev) ?
> -		set_label_unmounted(btrfs_dev, label) :
> -		set_label_mounted(btrfs_dev, label);
> -}
> diff --git a/btrfslabel.h b/btrfslabel.h
> deleted file mode 100644
> index abf43ad..0000000
> --- a/btrfslabel.h
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -/* btrflabel.h */
> -
> -
> -int get_label(char *btrfs_dev);
> -int set_label(char *btrfs_dev, char *nLabel);
> \ No newline at end of file
> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
> index 5770d8b..3752703 100644
> --- a/cmds-filesystem.c
> +++ b/cmds-filesystem.c
> @@ -32,7 +32,6 @@
>  #include "version.h"
>  
>  #include "commands.h"
> -#include "btrfslabel.h"
>  
>  static const char * const filesystem_cmd_group_usage[] = {
>  	"btrfs filesystem [<group>] <command> [<args>]",
> diff --git a/utils.c b/utils.c
> index 034da8f..d65bc35 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -16,6 +16,7 @@
>   * Boston, MA 021110-1307, USA.
>   */
>  
> +#define _GNU_SOURCE
>  #define _XOPEN_SOURCE 600
>  #define __USE_XOPEN2K
>  #include <stdio.h>
> @@ -1144,6 +1145,134 @@ static int check_label(char *input)
>         return 0;
>  }
>  
> +static int set_label_unmounted(const char *dev, const char *label)
> +{
> +	struct btrfs_trans_handle *trans;
> +	struct btrfs_root *root;
> +	int ret;
> +
> +	ret = check_mounted(dev);
> +	if (ret < 0) {
> +	       fprintf(stderr, "FATAL: error checking %s mount status\n", dev);
> +	       return -1;
> +	}
> +	if (ret > 0) {
> +		fprintf(stderr, "ERROR: dev %s is mounted, use mount point\n",
> +			dev);
> +		return -1;
> +	}
> +
> +	/* Open the super_block at the default location
> +	 * and as read-write.
> +	 */
> +	root = open_ctree(dev, 0, 1);
> +	if (!root) /* errors are printed by open_ctree() */
> +		return -1;
> +
> +	trans = btrfs_start_transaction(root, 1);
> +	snprintf(root->fs_info->super_copy.label, BTRFS_LABEL_SIZE, "%s",
> +		 label);
> +	btrfs_commit_transaction(trans, root);
> +
> +	/* Now we close it since we are done. */
> +	close_ctree(root);
> +	return 0;
> +}
> +
> +static int set_label_mounted(const char *mount_path, const char *label)
> +{
> +	int fd;
> +
> +	fd = open(mount_path, O_RDONLY | O_NOATIME);
> +	if (fd < 0) {
> +		fprintf(stderr, "ERROR: unable access to '%s'\n", mount_path);
> +		return -1;
> +	}
> +
> +	if (ioctl(fd, BTRFS_IOC_SET_FSLABEL, label) < 0) {
> +		fprintf(stderr, "ERROR: unable to set label %s\n",
> +			strerror(errno));
> +		close(fd);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int get_label_unmounted(const char *dev)
> +{
> +	struct btrfs_root *root;
> +	int ret;
> +
> +	ret = check_mounted(dev);
> +	if (ret < 0) {
> +	       fprintf(stderr, "FATAL: error checking %s mount status\n", dev);
> +	       return -1;
> +	}
> +	if (ret > 0) {
> +		fprintf(stderr, "ERROR: dev %s is mounted, use mount point\n",
> +			dev);
> +		return -1;
> +	}
> +
> +	/* Open the super_block at the default location
> +	 * and as read-only.
> +	 */
> +	root = open_ctree(dev, 0, 0);
> +	if(!root)
> +		return -1;
> +
> +	fprintf(stdout, "%s\n", root->fs_info->super_copy.label);
> +
> +	/* Now we close it since we are done. */
> +	close_ctree(root);
> +	return 0;
> +}
> +
> +/*
> + * If a partition is mounted, try to get the filesystem label via its
> + * mounted path rather than device.  Return the corresponding error
> + * the user specified the device path.
> + */
> +static int get_label_mounted(const char *mount_path)
> +{
> +	char label[BTRFS_LABEL_SIZE];
> +	int fd;
> +
> +	fd = open(mount_path, O_RDONLY | O_NOATIME);
> +	if (fd < 0) {
> +		fprintf(stderr, "ERROR: unable access to '%s'\n", mount_path);
> +		return -1;
> +	}
> +
> +	memset(label, '\0', sizeof(label));
> +	if (ioctl(fd, BTRFS_IOC_GET_FSLABEL, label) < 0) {
> +		fprintf(stderr, "ERROR: unable get label %s\n", strerror(errno));
> +		close(fd);
> +		return -1;
> +	}
> +
> +	fprintf(stdout, "%s\n", label);
> +	return 0;
> +}
> +
> +int get_label(const char *btrfs_dev)
> +{
> +	return is_existing_blk_or_reg_file(btrfs_dev) ?
> +		get_label_unmounted(btrfs_dev) :
> +		get_label_mounted(btrfs_dev);
> +}
> +
> +int set_label(const char *btrfs_dev, const char *label)
> +{
> +	if (check_label(label))
> +		return -1;
> +
> +	return is_existing_blk_or_reg_file(btrfs_dev) ?
> +		set_label_unmounted(btrfs_dev, label) :
> +		set_label_mounted(btrfs_dev, label);
> +}
> +
>  int btrfs_scan_block_devices(int run_ioctl)
>  {
>  
> diff --git a/utils.h b/utils.h
> index a0b782b..632b01c 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -46,4 +46,6 @@ int get_mountpt(char *dev, char *mntpt, size_t size);
>  
>  int btrfs_scan_block_devices(int run_ioctl);
>  int is_existing_blk_or_reg_file(const char* filename);
> +int get_label(const char *btrfs_dev);
> +int set_label(const char *btrfs_dev, const char *label);
>  #endif
> 

Jeff, You moved everything except for the copyright text and finally
deleted the file with the copyright text.
Shouldn't you add this text to util.c as well?

Something like this (the Oracle text was already present in utils.c, and
the Morey Roof text is taken from utils.c, the license was equal in both
files):
/*
 * Copyright (C) 2007 Oracle.  All rights reserved.
 * Copyright (C) 2008 Morey Roof.   All rights reserved.
 *
 * This program is free software; you can redistribute it and/or
...

Re: [PATCH 2/2] btrfs-progs: remove btrfslabel.[c|h]

[Jeff Liu]

On 01/29/2013 06:26 PM, Stefan Behrens wrote:
> On Tue, 29 Jan 2013 14:24:13 +0800, Jeff Liu wrote:
>> Clean btrfslabel.[c|h] out of the source tree and move those related
>> functions to utils.[c|h].
>>
>> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
>> CC: David Sterba <dsterba@suse.cz>
>> CC: Gene Czarcinski <gene@czarc.net>
>> ---
>>  Makefile          |    4 +-
>>  btrfslabel.c      |  178 -----------------------------------------------------
>>  btrfslabel.h      |    5 --
>>  cmds-filesystem.c |    1 -
>>  utils.c           |  129 ++++++++++++++++++++++++++++++++++++++
>>  utils.h           |    2 +
>>  6 files changed, 133 insertions(+), 186 deletions(-)
>>  delete mode 100644 btrfslabel.c
>>  delete mode 100644 btrfslabel.h
>>
>> diff --git a/Makefile b/Makefile
>> index 4894903..e54b21e 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -4,8 +4,8 @@ CFLAGS = -g -O1
>>  objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \
>>  	  root-tree.o dir-item.o file-item.o inode-item.o \
>>  	  inode-map.o crc32c.o rbtree.o extent-cache.o extent_io.o \
>> -	  volumes.o utils.o btrfs-list.o btrfslabel.o repair.o \
>> -	  send-stream.o send-utils.o qgroup.o
>> +	  volumes.o utils.o btrfs-list.o repair.o send-stream.o \
>> +	  send-utils.o qgroup.o
>>  cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
>>  	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
>>  	       cmds-quota.o cmds-qgroup.o
>> diff --git a/btrfslabel.c b/btrfslabel.c
>> deleted file mode 100644
>> index 2826050..0000000
>> --- a/btrfslabel.c
>> +++ /dev/null
>> @@ -1,178 +0,0 @@
>> -/*
>> - * Copyright (C) 2008 Morey Roof.   All rights reserved.
>> - *
>> - * This program is free software; you can redistribute it and/or
>> - * modify it under the terms of the GNU General Public
>> - * License v2 as published by the Free Software Foundation.
>> - *
>> - * This program is distributed in the hope that it will be useful,
>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> - * General Public License for more details.
>> - *
>> - * You should have received a copy of the GNU General Public
>> - * License along with this program; if not, write to the
>> - * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
>> - * Boston, MA 021110-1307, USA.
>> - */
>> -
>> -#define _GNU_SOURCE
>> -
>> -#ifndef __CHECKER__
>> -#include <sys/ioctl.h>
>> -#include <sys/mount.h>
>> -#include "ioctl.h"
>> -#endif /* __CHECKER__ */
>> -
>> -#include <stdio.h>
>> -#include <stdlib.h>
>> -#include <sys/types.h>
>> -#include <sys/stat.h>
>> -#include <dirent.h>
>> -#include <fcntl.h>
>> -#include <unistd.h>
>> -#include <linux/fs.h>
>> -#include <linux/limits.h>
>> -#include <ctype.h>
>> -#include "kerncompat.h"
>> -#include "ctree.h"
>> -#include "utils.h"
>> -#include "version.h"
>> -#include "disk-io.h"
>> -#include "transaction.h"
>> -
>> -#define MOUNTED                        1
>> -#define UNMOUNTED                      2
>> -#define GET_LABEL                      3
>> -#define SET_LABEL                      4
>> -
>> -static int set_label_unmounted(const char *dev, const char *label)
>> -{
>> -	struct btrfs_trans_handle *trans;
>> -	struct btrfs_root *root;
>> -	int ret;
>> -
>> -	ret = check_mounted(dev);
>> -	if (ret < 0) {
>> -	       fprintf(stderr, "FATAL: error checking %s mount status\n", dev);
>> -	       return -1;
>> -	}
>> -	if (ret > 0) {
>> -		fprintf(stderr, "ERROR: dev %s is mounted, use mount point\n",
>> -			dev);
>> -		return -1;
>> -	}
>> -
>> -	if (strlen(label) > BTRFS_LABEL_SIZE - 1) {
>> -		fprintf(stderr, "ERROR: Label %s is too long (max %d)\n",
>> -			label, BTRFS_LABEL_SIZE - 1);
>> -		return -1;
>> -	}
>> -
>> -	/* Open the super_block at the default location
>> -	 * and as read-write.
>> -	 */
>> -	root = open_ctree(dev, 0, 1);
>> -	if (!root) /* errors are printed by open_ctree() */
>> -		return -1;
>> -
>> -	trans = btrfs_start_transaction(root, 1);
>> -	snprintf(root->fs_info->super_copy.label, BTRFS_LABEL_SIZE, "%s",
>> -		 label);
>> -	btrfs_commit_transaction(trans, root);
>> -
>> -	/* Now we close it since we are done. */
>> -	close_ctree(root);
>> -	return 0;
>> -}
>> -
>> -static int set_label_mounted(const char *mount_path, const char *label)
>> -{
>> -	int fd;
>> -
>> -	fd = open(mount_path, O_RDONLY | O_NOATIME);
>> -	if (fd < 0) {
>> -		fprintf(stderr, "ERROR: unable access to '%s'\n", mount_path);
>> -		return -1;
>> -	}
>> -
>> -	if (ioctl(fd, BTRFS_IOC_SET_FSLABEL, label) < 0) {
>> -		fprintf(stderr, "ERROR: unable to set label %s\n",
>> -			strerror(errno));
>> -		close(fd);
>> -		return -1;
>> -	}
>> -
>> -	return 0;
>> -}
>> -
>> -static int get_label_unmounted(const char *dev)
>> -{
>> -	struct btrfs_root *root;
>> -	int ret;
>> -
>> -	ret = check_mounted(dev);
>> -	if (ret < 0) {
>> -	       fprintf(stderr, "FATAL: error checking %s mount status\n", dev);
>> -	       return -1;
>> -	}
>> -	if (ret > 0) {
>> -		fprintf(stderr, "ERROR: dev %s is mounted, use mount point\n",
>> -			dev);
>> -		return -1;
>> -	}
>> -
>> -	/* Open the super_block at the default location
>> -	 * and as read-only.
>> -	 */
>> -	root = open_ctree(dev, 0, 0);
>> -	if(!root)
>> -		return -1;
>> -
>> -	fprintf(stdout, "%s\n", root->fs_info->super_copy.label);
>> -
>> -	/* Now we close it since we are done. */
>> -	close_ctree(root);
>> -	return 0;
>> -}
>> -
>> -/*
>> - * If a partition is mounted, try to get the filesystem label via its
>> - * mounted path rather than device.  Return the corresponding error
>> - * the user specified the device path.
>> - */
>> -static int get_label_mounted(const char *mount_path)
>> -{
>> -	char label[BTRFS_LABEL_SIZE];
>> -	int fd;
>> -
>> -	fd = open(mount_path, O_RDONLY | O_NOATIME);
>> -	if (fd < 0) {
>> -		fprintf(stderr, "ERROR: unable access to '%s'\n", mount_path);
>> -		return -1;
>> -	}
>> -
>> -	memset(label, '\0', sizeof(label));
>> -	if (ioctl(fd, BTRFS_IOC_GET_FSLABEL, label) < 0) {
>> -		fprintf(stderr, "ERROR: unable get label %s\n", strerror(errno));
>> -		close(fd);
>> -		return -1;
>> -	}
>> -
>> -	fprintf(stdout, "%s\n", label);
>> -	return 0;
>> -}
>> -
>> -int get_label(const char *btrfs_dev)
>> -{
>> -	return is_existing_blk_or_reg_file(btrfs_dev) ?
>> -		get_label_unmounted(btrfs_dev) :
>> -		get_label_mounted(btrfs_dev);
>> -}
>> -
>> -int set_label(char *btrfs_dev, char *label)
>> -{
>> -	return is_existing_blk_or_reg_file(btrfs_dev) ?
>> -		set_label_unmounted(btrfs_dev, label) :
>> -		set_label_mounted(btrfs_dev, label);
>> -}
>> diff --git a/btrfslabel.h b/btrfslabel.h
>> deleted file mode 100644
>> index abf43ad..0000000
>> --- a/btrfslabel.h
>> +++ /dev/null
>> @@ -1,5 +0,0 @@
>> -/* btrflabel.h */
>> -
>> -
>> -int get_label(char *btrfs_dev);
>> -int set_label(char *btrfs_dev, char *nLabel);
>> \ No newline at end of file
>> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
>> index 5770d8b..3752703 100644
>> --- a/cmds-filesystem.c
>> +++ b/cmds-filesystem.c
>> @@ -32,7 +32,6 @@
>>  #include "version.h"
>>  
>>  #include "commands.h"
>> -#include "btrfslabel.h"
>>  
>>  static const char * const filesystem_cmd_group_usage[] = {
>>  	"btrfs filesystem [<group>] <command> [<args>]",
>> diff --git a/utils.c b/utils.c
>> index 034da8f..d65bc35 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -16,6 +16,7 @@
>>   * Boston, MA 021110-1307, USA.
>>   */
>>  
>> +#define _GNU_SOURCE
>>  #define _XOPEN_SOURCE 600
>>  #define __USE_XOPEN2K
>>  #include <stdio.h>
>> @@ -1144,6 +1145,134 @@ static int check_label(char *input)
>>         return 0;
>>  }
>>  
>> +static int set_label_unmounted(const char *dev, const char *label)
>> +{
>> +	struct btrfs_trans_handle *trans;
>> +	struct btrfs_root *root;
>> +	int ret;
>> +
>> +	ret = check_mounted(dev);
>> +	if (ret < 0) {
>> +	       fprintf(stderr, "FATAL: error checking %s mount status\n", dev);
>> +	       return -1;
>> +	}
>> +	if (ret > 0) {
>> +		fprintf(stderr, "ERROR: dev %s is mounted, use mount point\n",
>> +			dev);
>> +		return -1;
>> +	}
>> +
>> +	/* Open the super_block at the default location
>> +	 * and as read-write.
>> +	 */
>> +	root = open_ctree(dev, 0, 1);
>> +	if (!root) /* errors are printed by open_ctree() */
>> +		return -1;
>> +
>> +	trans = btrfs_start_transaction(root, 1);
>> +	snprintf(root->fs_info->super_copy.label, BTRFS_LABEL_SIZE, "%s",
>> +		 label);
>> +	btrfs_commit_transaction(trans, root);
>> +
>> +	/* Now we close it since we are done. */
>> +	close_ctree(root);
>> +	return 0;
>> +}
>> +
>> +static int set_label_mounted(const char *mount_path, const char *label)
>> +{
>> +	int fd;
>> +
>> +	fd = open(mount_path, O_RDONLY | O_NOATIME);
>> +	if (fd < 0) {
>> +		fprintf(stderr, "ERROR: unable access to '%s'\n", mount_path);
>> +		return -1;
>> +	}
>> +
>> +	if (ioctl(fd, BTRFS_IOC_SET_FSLABEL, label) < 0) {
>> +		fprintf(stderr, "ERROR: unable to set label %s\n",
>> +			strerror(errno));
>> +		close(fd);
>> +		return -1;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int get_label_unmounted(const char *dev)
>> +{
>> +	struct btrfs_root *root;
>> +	int ret;
>> +
>> +	ret = check_mounted(dev);
>> +	if (ret < 0) {
>> +	       fprintf(stderr, "FATAL: error checking %s mount status\n", dev);
>> +	       return -1;
>> +	}
>> +	if (ret > 0) {
>> +		fprintf(stderr, "ERROR: dev %s is mounted, use mount point\n",
>> +			dev);
>> +		return -1;
>> +	}
>> +
>> +	/* Open the super_block at the default location
>> +	 * and as read-only.
>> +	 */
>> +	root = open_ctree(dev, 0, 0);
>> +	if(!root)
>> +		return -1;
>> +
>> +	fprintf(stdout, "%s\n", root->fs_info->super_copy.label);
>> +
>> +	/* Now we close it since we are done. */
>> +	close_ctree(root);
>> +	return 0;
>> +}
>> +
>> +/*
>> + * If a partition is mounted, try to get the filesystem label via its
>> + * mounted path rather than device.  Return the corresponding error
>> + * the user specified the device path.
>> + */
>> +static int get_label_mounted(const char *mount_path)
>> +{
>> +	char label[BTRFS_LABEL_SIZE];
>> +	int fd;
>> +
>> +	fd = open(mount_path, O_RDONLY | O_NOATIME);
>> +	if (fd < 0) {
>> +		fprintf(stderr, "ERROR: unable access to '%s'\n", mount_path);
>> +		return -1;
>> +	}
>> +
>> +	memset(label, '\0', sizeof(label));
>> +	if (ioctl(fd, BTRFS_IOC_GET_FSLABEL, label) < 0) {
>> +		fprintf(stderr, "ERROR: unable get label %s\n", strerror(errno));
>> +		close(fd);
>> +		return -1;
>> +	}
>> +
>> +	fprintf(stdout, "%s\n", label);
>> +	return 0;
>> +}
>> +
>> +int get_label(const char *btrfs_dev)
>> +{
>> +	return is_existing_blk_or_reg_file(btrfs_dev) ?
>> +		get_label_unmounted(btrfs_dev) :
>> +		get_label_mounted(btrfs_dev);
>> +}
>> +
>> +int set_label(const char *btrfs_dev, const char *label)
>> +{
>> +	if (check_label(label))
>> +		return -1;
>> +
>> +	return is_existing_blk_or_reg_file(btrfs_dev) ?
>> +		set_label_unmounted(btrfs_dev, label) :
>> +		set_label_mounted(btrfs_dev, label);
>> +}
>> +
>>  int btrfs_scan_block_devices(int run_ioctl)
>>  {
>>  
>> diff --git a/utils.h b/utils.h
>> index a0b782b..632b01c 100644
>> --- a/utils.h
>> +++ b/utils.h
>> @@ -46,4 +46,6 @@ int get_mountpt(char *dev, char *mntpt, size_t size);
>>  
>>  int btrfs_scan_block_devices(int run_ioctl);
>>  int is_existing_blk_or_reg_file(const char* filename);
>> +int get_label(const char *btrfs_dev);
>> +int set_label(const char *btrfs_dev, const char *label);
>>  #endif
>>
> 
> Jeff, You moved everything except for the copyright text and finally
> deleted the file with the copyright text.
> Shouldn't you add this text to util.c as well?
Sure, thanks for your reminder. :)

-Jeff
> 
> Something like this (the Oracle text was already present in utils.c, and
> the Morey Roof text is taken from utils.c, the license was equal in both
> files):
> /*
>  * Copyright (C) 2007 Oracle.  All rights reserved.
>  * Copyright (C) 2008 Morey Roof.   All rights reserved.
>  *
>  * This program is free software; you can redistribute it and/or
> ...
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>