[PATCH v3] Btrfs-progs: check out if the swap device

[PATCH v3] Btrfs-progs: check out if the swap device

[Tsutomu Itoh]

Currently, the following commands succeed.

 # cat /proc/swaps
 Filename                                Type            Size    Used    Priority
 /dev/sda3                               partition       8388604 0       -1
 /dev/sdc8                               partition       9765884 0       -2
 # mkfs.btrfs /dev/sdc8
 
 WARNING! - Btrfs v0.20-rc1-165-g82ac345 IS EXPERIMENTAL
 WARNING! - see http://btrfs.wiki.kernel.org before using
 
 fs created label (null) on /dev/sdc8
         nodesize 4096 leafsize 4096 sectorsize 4096 size 9.31GB
 Btrfs v0.20-rc1-165-g82ac345
 # btrfs fi sh /dev/sdc8
 Label: none  uuid: fc0bdbd0-7eed-460f-b4e9-131273b66df2
         Total devices 1 FS bytes used 28.00KB
         devid    1 size 9.31GB used 989.62MB path /dev/sdc8
 
 Btrfs v0.20-rc1-165-g82ac345
 #

But we should check out the swap device. Fixed it.

Signed-off-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
---
(this patch is based on Chris's raid56-experimental branch)
v2 -> v3:
	- Open again without O_EXCL so that the problem should not occur
	  (btrfs_register_one_device() fails if O_EXCL is on)
v1 -> v2:
	- O_EXCL is specified at open(), as a last safety check
	- Check when the swap device is a file is added
	- Translate '\nnn'(octal) to character
	Thank you Eric, David, Zach and others.
---
 mkfs.c  | 52 +++++++++++++++++++++++++++++++++++++++++++--
 utils.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 utils.h |  1 +
 3 files changed, 126 insertions(+), 2 deletions(-)

diff --git a/mkfs.c b/mkfs.c
index 2d3c2af..2210312 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1366,6 +1366,16 @@ int main(int ac, char **av)
 
 	if (source_dir == 0) {
 		file = av[optind++];
+		ret = is_swap_device(file);
+		if (ret < 0) {
+			fprintf(stderr, "error checking %s status: %s\n", file,
+				strerror(-ret));
+			exit(1);
+		}
+		if (ret == 1) {
+			fprintf(stderr, "%s is a swap device\n", file);
+			exit(1);
+		}
 		ret = check_mounted(file);
 		if (ret < 0) {
 			fprintf(stderr, "error checking %s mount status\n", file);
@@ -1376,9 +1386,23 @@ int main(int ac, char **av)
 			exit(1);
 		}
 		ac--;
+		/* check if the device is busy */
+		fd = open(file, O_RDWR|O_EXCL);
+		if (fd < 0) {
+			fprintf(stderr, "unable to open %s: %s\n", file,
+				strerror(errno));
+			exit(1);
+		}
+		close(fd);
+		/*
+		 * open again without O_EXCL so that the problem should not
+		 * occur by the following processing.
+		 * (btrfs_register_one_device() fails if O_EXCL is on)
+		 */
 		fd = open(file, O_RDWR);
 		if (fd < 0) {
-			fprintf(stderr, "unable to open %s\n", file);
+			fprintf(stderr, "unable to open %s: %s\n", file,
+				strerror(errno));
 			exit(1);
 		}
 		first_file = file;
@@ -1461,6 +1485,16 @@ int main(int ac, char **av)
 		int old_mixed = mixed;
 
 		file = av[optind++];
+		ret = is_swap_device(file);
+		if (ret < 0) {
+			fprintf(stderr, "error checking %s status: %s\n", file,
+				strerror(-ret));
+			exit(1);
+		}
+		if (ret == 1) {
+			fprintf(stderr, "%s is a swap device\n", file);
+			exit(1);
+		}
 		ret = check_mounted(file);
 		if (ret < 0) {
 			fprintf(stderr, "error checking %s mount status\n",
@@ -1471,9 +1505,23 @@ int main(int ac, char **av)
 			fprintf(stderr, "%s is mounted\n", file);
 			exit(1);
 		}
+		/* check if the device is busy */
+		fd = open(file, O_RDWR|O_EXCL);
+		if (fd < 0) {
+			fprintf(stderr, "unable to open %s: %s\n", file,
+				strerror(errno));
+			exit(1);
+		}
+		close(fd);
+		/*
+		 * open again without O_EXCL so that the problem should not
+		 * occur by the following processing.
+		 * (btrfs_register_one_device() fails if O_EXCL is on)
+		 */
 		fd = open(file, O_RDWR);
 		if (fd < 0) {
-			fprintf(stderr, "unable to open %s\n", file);
+			fprintf(stderr, "unable to open %s: %s\n", file,
+				strerror(errno));
 			exit(1);
 		}
 		ret = btrfs_device_already_in_root(root, fd,
diff --git a/utils.c b/utils.c
index f9ee812..891edcc 100644
--- a/utils.c
+++ b/utils.c
@@ -1386,3 +1386,78 @@ int get_fs_info(int fd, char *path, struct btrfs_ioctl_fs_info_args *fi_args,
 
 	return 0;
 }
+
+#define isoctal(c)	(((c) & ~7) == '0')
+
+static inline void translate(char *f, char *t)
+{
+	while (*f != '\0') {
+		if (*f == '\\' &&
+		    isoctal(f[1]) && isoctal(f[2]) && isoctal(f[3])) {
+			*t++ = 64*(f[1] & 7) + 8*(f[2] & 7) + (f[3] & 7);
+			f += 4;
+		} else
+			*t++ = *f++;
+	}
+	*t = '\0';
+	return;
+}
+
+/*
+ * Checks if the swap device.
+ * Returns 1 if swap device, < 0 on error or 0 if not swap device.
+ */
+int is_swap_device(const char *file)
+{
+	FILE	*f;
+	struct stat	st_buf;
+	dev_t	dev;
+	ino_t	ino = 0;
+	char	tmp[PATH_MAX];
+	char	buf[PATH_MAX];
+	char	*cp;
+	int	ret = 0;
+
+	if (stat(file, &st_buf) < 0)
+		return -errno;
+	if (S_ISBLK(st_buf.st_mode))
+		dev = st_buf.st_rdev;
+	else if (S_ISREG(st_buf.st_mode)) {
+		dev = st_buf.st_dev;
+		ino = st_buf.st_ino;
+	} else
+		return 0;
+
+	if ((f = fopen("/proc/swaps", "r")) == NULL)
+		return 0;
+
+	/* skip the first line */
+	if (fgets(tmp, sizeof(tmp), f) == NULL)
+		goto out;
+
+	while (fgets(tmp, sizeof(tmp), f) != NULL) {
+		if ((cp = strchr(tmp, ' ')) != NULL)
+			*cp = '\0';
+		if ((cp = strchr(tmp, '\t')) != NULL)
+			*cp = '\0';
+		translate(tmp, buf);
+		if (stat(buf, &st_buf) != 0)
+			continue;
+		if (S_ISBLK(st_buf.st_mode)) {
+			if (dev == st_buf.st_rdev) {
+				ret = 1;
+				break;
+			}
+		} else if (S_ISREG(st_buf.st_mode)) {
+			if (dev == st_buf.st_dev && ino == st_buf.st_ino) {
+				ret = 1;
+				break;
+			}
+		}
+	}
+
+out:
+	fclose(f);
+
+	return ret;
+}
diff --git a/utils.h b/utils.h
index bbcaf6a..60a0fea 100644
--- a/utils.h
+++ b/utils.h
@@ -55,6 +55,7 @@ int get_fs_info(int fd, char *path, struct btrfs_ioctl_fs_info_args *fi_args,
 		struct btrfs_ioctl_dev_info_args **di_ret);
 
 char *__strncpy__null(char *dest, const char *src, size_t n);
+int is_swap_device(const char *file);
 /* Helper to always get proper size of the destination string */
 #define strncpy_null(dest, src) __strncpy__null(dest, src, sizeof(dest))
 

Re: [PATCH v3] Btrfs-progs: check out if the swap device

[David Sterba]

On Thu, Feb 14, 2013 at 04:53:04PM +0900, Tsutomu Itoh wrote:
> Signed-off-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>

Worked for me here in these situations (ie. mkfs failed, filenames
specified given as simple path or with mixture of ./ and ../ leading to
the device):

* mounted /dev/sdx
* /dev/sdx part of a multi-device mounted fs
* /dev/sdx swap device
* file as a swap device
* swap files with funny names: \ \040 \134 \888 <space>

Tested-by: David Sterba <dsterba@suse.cz>

Re: [PATCH v3] Btrfs-progs: check out if the swap device

[Brendan Hide]

On 2013/02/14 09:53 AM, Tsutomu Itoh wrote:
> +		if (ret < 0) {
> +			fprintf(stderr, "error checking %s status: %s\n", file,
> +				strerror(-ret));
> +			exit(1);
> +		}
>
> ...
>
> +		/* check if the device is busy */
> +		fd = open(file, O_RDWR|O_EXCL);
> +		if (fd < 0) {
> +			fprintf(stderr, "unable to open %s: %s\n", file,
> +				strerror(errno));
> +			exit(1);
> +		}
This is fine and works (as tested by David) - but I'm not sure if the 
below suggestions from Zach were taken into account.

1. If the check with "open(file, O_RDWR|O_EXCL)" shows that the device 
is available, there's no point in checking if it is mounted as a swap 
device. A preliminary check using this could precede all other checks 
which should be skipped if it shows success.

2. If there's an error checking the status (for example lets say 
/proc/swaps is deprecated), we should print the informational message 
but not error out.

On 2013/02/13 11:58 AM, Zach Brown wrote:
> - First always open with O_EXCL.  If it succeeds then there's no reason
>    to check /proc/swaps at all.  (Maybe it doesn't need to try
>    check_mounted() there either?  Not sure if it's protecting against
>    accidentally mounting mounted shared storage or not.)
>
> ...
>
> - At no point is failure of any of the /proc/swaps parsing fatal.  It'd
>    carry on ignoring errors until it doesnt have work to do.  It'd only
>    ever print the nice message when it finds a match.


-- 
__________
Brendan Hide
http://swiftspirit.co.za/
http://www.webafrica.co.za/?AFF1E97

Re: [PATCH v3] Btrfs-progs: check out if the swap device

[Tsutomu Itoh]

On 2013/03/04 1:47, Brendan Hide wrote:
> On 2013/02/14 09:53 AM, Tsutomu Itoh wrote:
>> +        if (ret < 0) {
>> +            fprintf(stderr, "error checking %s status: %s\n", file,
>> +                strerror(-ret));
>> +            exit(1);
>> +        }
>>
>> ...
>>
>> +        /* check if the device is busy */
>> +        fd = open(file, O_RDWR|O_EXCL);
>> +        if (fd < 0) {
>> +            fprintf(stderr, "unable to open %s: %s\n", file,
>> +                strerror(errno));
>> +            exit(1);
>> +        }
> This is fine and works (as tested by David) - but I'm not sure if the below
  suggestions from Zach were taken into account.
>
> 1. If the check with "open(file, O_RDWR|O_EXCL)" shows that the device is
> available, there's no point in checking if it is mounted as a swap device.
> A preliminary check using this could precede all other checks which should
> be skipped if it shows success.
>
> 2. If there's an error checking the status (for example lets say /proc/swaps
> is deprecated), we should print the informational message but not error out.

If "open(file, O_RDWR|O_EXCL)" failed, we should output an appropriate message
why it failed. So, I'm testing is_swap_device() and check_mounted() first.
But is_swap_device() is not perfect, so I'm trying to open O_EXCL after all
other tests, as a last safety check.

Thanks,
Tsutomu

>
> On 2013/02/13 11:58 AM, Zach Brown wrote:
>> - First always open with O_EXCL.  If it succeeds then there's no reason
>>    to check /proc/swaps at all.  (Maybe it doesn't need to try
>>    check_mounted() there either?  Not sure if it's protecting against
>>    accidentally mounting mounted shared storage or not.)
>>
>> ...
>>
>> - At no point is failure of any of the /proc/swaps parsing fatal.  It'd
>>    carry on ignoring errors until it doesnt have work to do.  It'd only
>>    ever print the nice message when it finds a match.
>
>