[PATCH 1/5] Avoid to consider lvm snapshots when scanning devices.

[PATCH 1/5] Avoid to consider lvm snapshots when scanning devices.

[Goffredo Baroncelli]

LVM snapshots create a problem to the btrfs devices management.
BTRFS assumes that each device haw an unique 'device UUID'.
A LVM snapshot breaks this assumption.

This patch skips LVM snapshots during the device scan phase.
If you need to consider a LVM snapshot you have to set the
environmental variable BTRFS_SKIP_LVM_SNAPSHOT to "no".

To check if a device is a LVM snapshot, it is checked the
'udev' device property 'DM_UDEV_LOW_PRIORITY_FLAG' .
If it is set to 1, the device has to be skipped.

As conseguence, btrfs now depends by libudev.

Programmatically you can control this behavior with the functions:
- btrfs_scan_set_skip_lvm_snapshot(int new_value)
- int btrfs_scan_get_skip_lvm_snapshot( )

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 Makefile |   4 +--
 utils.c  | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 utils.h  |   9 +++++-
 3 files changed, 117 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 4cae30c..9464361 100644
--- a/Makefile
+++ b/Makefile
@@ -26,7 +26,7 @@ TESTS = fsck-tests.sh convert-tests.sh
 INSTALL = install
 prefix ?= /usr/local
 bindir = $(prefix)/bin
-lib_LIBS = -luuid -lblkid -lm -lz -llzo2 -L.
+lib_LIBS = -luuid -lblkid -lm -lz -ludev -llzo2 -L.
 libdir ?= $(prefix)/lib
 incdir = $(prefix)/include/btrfs
 LIBS = $(lib_LIBS) $(libs_static)
@@ -99,7 +99,7 @@ lib_links = libbtrfs.so.0 libbtrfs.so
 headers = $(libbtrfs_headers)
 
 # make C=1 to enable sparse
-check_defs := .cc-defines.h 
+check_defs := .cc-defines.h
 ifdef C
 	#
 	# We're trying to use sparse against glibc headers which go wild
diff --git a/utils.c b/utils.c
index 2a92416..9887f8b 100644
--- a/utils.c
+++ b/utils.c
@@ -29,6 +29,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <uuid/uuid.h>
+#include <libudev.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <mntent.h>
@@ -52,6 +53,13 @@
 #define BLKDISCARD	_IO(0x12,119)
 #endif
 
+/*
+ * This variable controls if the lvm snapshot have to be skipped or not.
+ * Access this variable only via the btrfs_scan_[sg]et_skip_lvm_snapshot()
+ * functions
+ */
+static int __scan_device_skip_lvm_snapshot = -1;
+
 static int btrfs_scan_done = 0;
 
 static char argv0_buf[ARGV0_BUF_SIZE] = "btrfs";
@@ -1593,6 +1601,9 @@ int btrfs_scan_block_devices(int run_ioctl)
 	char fullpath[110];
 	int scans = 0;
 	int special;
+	int skip_snapshot;
+
+	skip_snapshot = btrfs_scan_get_skip_lvm_snapshot();
 
 scan_again:
 	proc_partitions = fopen("/proc/partitions","r");
@@ -1642,6 +1653,9 @@ scan_again:
 			continue;
 		}
 
+		if (skip_snapshot && is_low_priority_device(fullpath))
+			continue;
+
 		fd = open(fullpath, O_RDONLY);
 		if (fd < 0) {
 			if (errno != ENOMEDIUM)
@@ -2182,6 +2196,30 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
 	return 0;
 }
 
+int btrfs_scan_get_skip_lvm_snapshot( )
+{
+	const char *value;
+
+	if (__scan_device_skip_lvm_snapshot != -1 )
+		return __scan_device_skip_lvm_snapshot;
+
+	value = getenv(BTRFS_SKIP_LVM_SNAPSHOT_ENV_NAME);
+	if (value && !strcasecmp(value, "NO"))
+		__scan_device_skip_lvm_snapshot = 0;
+	else  if (value && !strcasecmp(value, "YES"))
+		__scan_device_skip_lvm_snapshot = 1;
+	else
+		__scan_device_skip_lvm_snapshot =
+			BTRFS_SKIP_LVM_SNAPSHOT_DEFAULT;
+
+	return __scan_device_skip_lvm_snapshot;
+}
+
+void btrfs_scan_set_skip_lvm_snapshot(int new_value)
+{
+	__scan_device_skip_lvm_snapshot = !!new_value;
+}
+
 int btrfs_scan_lblkid()
 {
 	int fd = -1;
@@ -2192,6 +2230,9 @@ int btrfs_scan_lblkid()
 	blkid_dev dev = NULL;
 	blkid_cache cache = NULL;
 	char path[PATH_MAX];
+	int skip_snapshot;
+
+	skip_snapshot = btrfs_scan_get_skip_lvm_snapshot();
 
 	if (btrfs_scan_done)
 		return 0;
@@ -2210,6 +2251,9 @@ int btrfs_scan_lblkid()
 		/* if we are here its definitely a btrfs disk*/
 		strncpy(path, blkid_dev_devname(dev), PATH_MAX);
 
+		if (skip_snapshot && is_low_priority_device(path))
+			continue;
+
 		fd = open(path, O_RDONLY);
 		if (fd < 0) {
 			printf("ERROR: could not open %s\n", path);
@@ -2450,3 +2494,66 @@ int find_next_key(struct btrfs_path *path, struct btrfs_key *key)
 	}
 	return 1;
 }
+
+/*
+ * This function return 1 if the device (path) is consdered "LOW_PRIORITY" by
+ * LVM2 library. These device are typically the nsapshot.
+ * This function return < 0 in case of error; 0 otherwise.
+ */
+int is_low_priority_device(const char *path)
+{
+
+	struct udev *udev=NULL;
+	struct udev_device *dev;
+	struct udev_enumerate *enumerate=NULL;
+	struct udev_list_entry *devices;
+	struct udev_list_entry *node, *list;
+	int ret=-1;
+	const char *value, *syspath;
+	char *rpath=NULL;
+
+	rpath = realpath(path, NULL);
+	if (!rpath) {
+		fprintf(stderr, "ERROR: not enough memory\n");
+		ret=-2;
+		goto exit;
+	}
+
+	/* Create the udev object */
+	udev = udev_new();
+	if (!udev) {
+		fprintf(stderr, "ERROR: Can't create udev\n");
+		ret=-1;
+		goto exit;
+	}
+
+	enumerate = udev_enumerate_new(udev);
+	udev_enumerate_add_match_subsystem(enumerate, "block");
+	udev_enumerate_add_match_property(enumerate, "DEVNAME", rpath);
+	udev_enumerate_scan_devices(enumerate);
+
+	devices = udev_enumerate_get_list_entry(enumerate);
+	syspath = udev_list_entry_get_name(devices);
+
+	dev = udev_device_new_from_syspath(udev, syspath);
+
+	list = udev_device_get_properties_list_entry (dev);
+
+	ret = 0;
+	node = udev_list_entry_get_by_name(list, "DM_UDEV_LOW_PRIORITY_FLAG");
+	if (node) {
+		value = udev_list_entry_get_value(node);
+		if (value && !strcmp(value, "1"))
+			ret = 1;
+	}
+
+exit:
+	free(rpath);
+
+	/* Free the enumerator object */
+	udev_enumerate_unref(enumerate);
+
+	udev_unref(udev);
+
+	return ret;
+}
diff --git a/utils.h b/utils.h
index 289e86b..9855f09 100644
--- a/utils.h
+++ b/utils.h
@@ -128,7 +128,7 @@ int csum_tree_block(struct btrfs_root *root, struct extent_buffer *buf,
 			   int verify);
 int ask_user(char *question);
 int lookup_ino_rootid(int fd, u64 *rootid);
-int btrfs_scan_lblkid(void);
+int btrfs_scan_lblkid();
 int get_btrfs_mount(const char *dev, char *mp, size_t mp_size);
 int find_mount_root(const char *path, char **mount_root);
 int get_device_info(int fd, u64 devid,
@@ -161,4 +161,11 @@ static inline u64 btrfs_min_dev_size(u32 leafsize)
 
 int find_next_key(struct btrfs_path *path, struct btrfs_key *key);
 
+#define BTRFS_SKIP_LVM_SNAPSHOT_DEFAULT 1
+#define BTRFS_SKIP_LVM_SNAPSHOT_ENV_NAME "BTRFS_SKIP_LVM_SNAPSHOT"
+
+int is_low_priority_device(const char *path);
+void btrfs_scan_set_skip_lvm_snapshot(int new_value);
+int btrfs_scan_get_skip_lvm_snapshot( );
+
 #endif
-- 
2.1.3

[PATCH V2][BTRFS-PROGS] Don't use LVM snapshot device

[Goffredo Baroncelli]

LVM snapshots are a problem for the btrfs devices management.
BTRFS assumes that each device have an unique 'device UUID'. 
A LVM snapshot breaks this assumption.

This causes a lot of problems if some btrfs device are snapshotted:
- the set of devices for a btrfs multi-volume filesystem may be mixed
(i.e. some NON snapshotted device with some snapshotted devices)
- /proc/mount may returns a wrong device. 

In the mailing list some posts reported these incidents.

This patch allows btrfs to skip LVM snapshot during the device scan 
phase.

But if you need to consider a LVM snapshot you can set the
environment variable BTRFS_SKIP_LVM_SNAPSHOT to "no". In this case
the old behavior is applied.

To check if a device is a LVM snapshot, it is checked the 'udev'
device property 'DM_UDEV_LOW_PRIORITY_FLAG' . If it is set to 1,
the device has to be skipped.

As consequence, btrfs now depends also by the libudev.

The last patch is a check to ensure that the devices list 
doesn't contain two devices with the same dev.uuid. This check is 
performed *ONLY* when btrfs collects the information about all devices (
i.e. btrfs device scan <device> is not this case).

This patch addresses the following cases:
- scanning performed by udev rules
- "btrfs device scan" performed in the initramdisk
- btrfs device scan <device>
- other programs which call btrfs_scan_lblkid()

BR
G.Baroncelli

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Changelog
V2: moved some chunk from patch#5 to patch#2
V1: first issue

[PATCH 1/5] Avoid to consider lvm snapshots when scanning devices.

[Goffredo Baroncelli]

LVM snapshots create a problem to the btrfs devices management.
BTRFS assumes that each device haw an unique 'device UUID'.
A LVM snapshot breaks this assumption.

This patch skips LVM snapshots during the device scan phase.
If you need to consider a LVM snapshot you have to set the
environmental variable BTRFS_SKIP_LVM_SNAPSHOT to "no".

To check if a device is a LVM snapshot, it is checked the
'udev' device property 'DM_UDEV_LOW_PRIORITY_FLAG' .
If it is set to 1, the device has to be skipped.

As conseguence, btrfs now depends by libudev.

Programmatically you can control this behavior with the functions:
- btrfs_scan_set_skip_lvm_snapshot(int new_value)
- int btrfs_scan_get_skip_lvm_snapshot( )

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 Makefile |   4 +--
 utils.c  | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 utils.h  |   9 +++++-
 3 files changed, 117 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 4cae30c..9464361 100644
--- a/Makefile
+++ b/Makefile
@@ -26,7 +26,7 @@ TESTS = fsck-tests.sh convert-tests.sh
 INSTALL = install
 prefix ?= /usr/local
 bindir = $(prefix)/bin
-lib_LIBS = -luuid -lblkid -lm -lz -llzo2 -L.
+lib_LIBS = -luuid -lblkid -lm -lz -ludev -llzo2 -L.
 libdir ?= $(prefix)/lib
 incdir = $(prefix)/include/btrfs
 LIBS = $(lib_LIBS) $(libs_static)
@@ -99,7 +99,7 @@ lib_links = libbtrfs.so.0 libbtrfs.so
 headers = $(libbtrfs_headers)
 
 # make C=1 to enable sparse
-check_defs := .cc-defines.h 
+check_defs := .cc-defines.h
 ifdef C
 	#
 	# We're trying to use sparse against glibc headers which go wild
diff --git a/utils.c b/utils.c
index 2a92416..9887f8b 100644
--- a/utils.c
+++ b/utils.c
@@ -29,6 +29,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <uuid/uuid.h>
+#include <libudev.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <mntent.h>
@@ -52,6 +53,13 @@
 #define BLKDISCARD	_IO(0x12,119)
 #endif
 
+/*
+ * This variable controls if the lvm snapshot have to be skipped or not.
+ * Access this variable only via the btrfs_scan_[sg]et_skip_lvm_snapshot()
+ * functions
+ */
+static int __scan_device_skip_lvm_snapshot = -1;
+
 static int btrfs_scan_done = 0;
 
 static char argv0_buf[ARGV0_BUF_SIZE] = "btrfs";
@@ -1593,6 +1601,9 @@ int btrfs_scan_block_devices(int run_ioctl)
 	char fullpath[110];
 	int scans = 0;
 	int special;
+	int skip_snapshot;
+
+	skip_snapshot = btrfs_scan_get_skip_lvm_snapshot();
 
 scan_again:
 	proc_partitions = fopen("/proc/partitions","r");
@@ -1642,6 +1653,9 @@ scan_again:
 			continue;
 		}
 
+		if (skip_snapshot && is_low_priority_device(fullpath))
+			continue;
+
 		fd = open(fullpath, O_RDONLY);
 		if (fd < 0) {
 			if (errno != ENOMEDIUM)
@@ -2182,6 +2196,30 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
 	return 0;
 }
 
+int btrfs_scan_get_skip_lvm_snapshot( )
+{
+	const char *value;
+
+	if (__scan_device_skip_lvm_snapshot != -1 )
+		return __scan_device_skip_lvm_snapshot;
+
+	value = getenv(BTRFS_SKIP_LVM_SNAPSHOT_ENV_NAME);
+	if (value && !strcasecmp(value, "NO"))
+		__scan_device_skip_lvm_snapshot = 0;
+	else  if (value && !strcasecmp(value, "YES"))
+		__scan_device_skip_lvm_snapshot = 1;
+	else
+		__scan_device_skip_lvm_snapshot =
+			BTRFS_SKIP_LVM_SNAPSHOT_DEFAULT;
+
+	return __scan_device_skip_lvm_snapshot;
+}
+
+void btrfs_scan_set_skip_lvm_snapshot(int new_value)
+{
+	__scan_device_skip_lvm_snapshot = !!new_value;
+}
+
 int btrfs_scan_lblkid()
 {
 	int fd = -1;
@@ -2192,6 +2230,9 @@ int btrfs_scan_lblkid()
 	blkid_dev dev = NULL;
 	blkid_cache cache = NULL;
 	char path[PATH_MAX];
+	int skip_snapshot;
+
+	skip_snapshot = btrfs_scan_get_skip_lvm_snapshot();
 
 	if (btrfs_scan_done)
 		return 0;
@@ -2210,6 +2251,9 @@ int btrfs_scan_lblkid()
 		/* if we are here its definitely a btrfs disk*/
 		strncpy(path, blkid_dev_devname(dev), PATH_MAX);
 
+		if (skip_snapshot && is_low_priority_device(path))
+			continue;
+
 		fd = open(path, O_RDONLY);
 		if (fd < 0) {
 			printf("ERROR: could not open %s\n", path);
@@ -2450,3 +2494,66 @@ int find_next_key(struct btrfs_path *path, struct btrfs_key *key)
 	}
 	return 1;
 }
+
+/*
+ * This function return 1 if the device (path) is consdered "LOW_PRIORITY" by
+ * LVM2 library. These device are typically the nsapshot.
+ * This function return < 0 in case of error; 0 otherwise.
+ */
+int is_low_priority_device(const char *path)
+{
+
+	struct udev *udev=NULL;
+	struct udev_device *dev;
+	struct udev_enumerate *enumerate=NULL;
+	struct udev_list_entry *devices;
+	struct udev_list_entry *node, *list;
+	int ret=-1;
+	const char *value, *syspath;
+	char *rpath=NULL;
+
+	rpath = realpath(path, NULL);
+	if (!rpath) {
+		fprintf(stderr, "ERROR: not enough memory\n");
+		ret=-2;
+		goto exit;
+	}
+
+	/* Create the udev object */
+	udev = udev_new();
+	if (!udev) {
+		fprintf(stderr, "ERROR: Can't create udev\n");
+		ret=-1;
+		goto exit;
+	}
+
+	enumerate = udev_enumerate_new(udev);
+	udev_enumerate_add_match_subsystem(enumerate, "block");
+	udev_enumerate_add_match_property(enumerate, "DEVNAME", rpath);
+	udev_enumerate_scan_devices(enumerate);
+
+	devices = udev_enumerate_get_list_entry(enumerate);
+	syspath = udev_list_entry_get_name(devices);
+
+	dev = udev_device_new_from_syspath(udev, syspath);
+
+	list = udev_device_get_properties_list_entry (dev);
+
+	ret = 0;
+	node = udev_list_entry_get_by_name(list, "DM_UDEV_LOW_PRIORITY_FLAG");
+	if (node) {
+		value = udev_list_entry_get_value(node);
+		if (value && !strcmp(value, "1"))
+			ret = 1;
+	}
+
+exit:
+	free(rpath);
+
+	/* Free the enumerator object */
+	udev_enumerate_unref(enumerate);
+
+	udev_unref(udev);
+
+	return ret;
+}
diff --git a/utils.h b/utils.h
index 289e86b..9855f09 100644
--- a/utils.h
+++ b/utils.h
@@ -128,7 +128,7 @@ int csum_tree_block(struct btrfs_root *root, struct extent_buffer *buf,
 			   int verify);
 int ask_user(char *question);
 int lookup_ino_rootid(int fd, u64 *rootid);
-int btrfs_scan_lblkid(void);
+int btrfs_scan_lblkid();
 int get_btrfs_mount(const char *dev, char *mp, size_t mp_size);
 int find_mount_root(const char *path, char **mount_root);
 int get_device_info(int fd, u64 devid,
@@ -161,4 +161,11 @@ static inline u64 btrfs_min_dev_size(u32 leafsize)
 
 int find_next_key(struct btrfs_path *path, struct btrfs_key *key);
 
+#define BTRFS_SKIP_LVM_SNAPSHOT_DEFAULT 1
+#define BTRFS_SKIP_LVM_SNAPSHOT_ENV_NAME "BTRFS_SKIP_LVM_SNAPSHOT"
+
+int is_low_priority_device(const char *path);
+void btrfs_scan_set_skip_lvm_snapshot(int new_value);
+int btrfs_scan_get_skip_lvm_snapshot( );
+
 #endif
-- 
2.1.3

[PATCH 2/5] 'btrfs device scan' skips lvm snapshots.

[Goffredo Baroncelli]

LVM snapshots create a problem to the btrfs devices management.
BTRFS assumes that each device has an unique 'device UUID'.
A LVM snapshot breaks this assumption.

With this patch, 'btrfs device scan' skips LVM snapshot.
If you need to consider a LVM snapshot you have to pass the '-s' switch
ot set the environment variable BTRFS_SKIP_LVM_SNAPSHOT to "no".

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 cmds-device.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index 6cd41e1..f6c76e7 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -197,9 +197,10 @@ static int cmd_rm_dev(int argc, char **argv)
 }
 
 static const char * const cmd_scan_dev_usage[] = {
-	"btrfs device scan [(-d|--all-devices)|<device> [<device>...]]",
+	"btrfs device scan [-s][(-d|--all-devices)|<device> [<device>...]]",
 	"Scan devices for a btrfs filesystem",
 	" -d|--all-devices (deprecated)",
+	" -s                               don't skip lvm snapshot\n",
 	NULL
 };
 
@@ -209,6 +210,7 @@ static int cmd_scan_dev(int argc, char **argv)
 	int devstart = 1;
 	int all = 0;
 	int ret = 0;
+	int skip_snapshot = btrfs_scan_get_skip_lvm_snapshot();
 
 	optind = 1;
 	while (1) {
@@ -217,7 +219,7 @@ static int cmd_scan_dev(int argc, char **argv)
 			{ "all-devices", no_argument, NULL, 'd'},
 			{ 0, 0, 0, 0 },
 		};
-		int c = getopt_long(argc, argv, "d", long_options,
+		int c = getopt_long(argc, argv, "sd", long_options,
 				    &long_index);
 		if (c < 0)
 			break;
@@ -225,6 +227,10 @@ static int cmd_scan_dev(int argc, char **argv)
 		case 'd':
 			all = 1;
 			break;
+		case 's':
+			skip_snapshot = 0;
+			devstart++;
+			break;
 		default:
 			usage(cmd_scan_dev_usage);
 		}
@@ -233,7 +239,7 @@ static int cmd_scan_dev(int argc, char **argv)
 	if (all && check_argc_max(argc, 2))
 		usage(cmd_scan_dev_usage);
 
-	if (all || argc == 1) {
+	if (all || argc == devstart) {
 		printf("Scanning for Btrfs filesystems\n");
 		ret = btrfs_scan_lblkid();
 		if (ret)
@@ -261,6 +267,11 @@ static int cmd_scan_dev(int argc, char **argv)
 			ret = 1;
 			goto out;
 		}
+		if (skip_snapshot && is_low_priority_device(path)) {
+			fprintf(stderr, "WARNING: skip device '%s' because it is a snapshot\n",
+				argv[i]);
+			continue;
+		}
 		printf("Scanning for Btrfs filesystems in '%s'\n", path);
 		if (btrfs_register_one_device(path) != 0) {
 			ret = 1;
-- 
2.1.3

[PATCH 3/5] Update 'btrfs device scan' man page.

[Goffredo Baroncelli]

This patch documents the '-s' switch and the environment variable
BTRFS_SKIP_LVM_SNAPSHOT.

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 Documentation/btrfs-device.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/btrfs-device.txt b/Documentation/btrfs-device.txt
index 46c54b9..ce5d69d 100644
--- a/Documentation/btrfs-device.txt
+++ b/Documentation/btrfs-device.txt
@@ -77,7 +77,7 @@ force overwrite of existing filesystem on the given disk(s)
 *delete* <dev> [<dev>...] <path>::
 Remove device(s) from a filesystem identified by <path>.
 
-*scan* [(--all-devices|-d)|<device> [<device>...]]::
+*scan* [-s][(--all-devices|-d)|<device> [<device>...]]::
 Scan devices for a btrfs filesystem.
 +
 If one or more devices are passed, these are scanned for a btrfs filesystem. 
@@ -85,6 +85,9 @@ If no devices are passed, btrfs uses block devices containing btrfs
 filesystem as listed by blkid.
 Finally, if '--all-devices' or '-d' is passed, all the devices under /dev are 
 scanned.
+By default, the lvm snapshots are skipped. If '-s' is passed, these aren't
+skipped anymore. See also BTRFS_SKIP_LVM_SNAPSHOT environmental variable
+in the main btrfs man page.
 
 *ready* <device>::
 Check device to see if it has all of it's devices in cache for mounting.
@@ -114,3 +117,4 @@ SEE ALSO
 `mkfs.btrfs`(8),
 `btrfs-replace`(8),
 `btrfs-balance`(8)
+`btrfs` (8)
-- 
2.1.3

[PATCH 4/5] Add reference to BTRFS_SKIP_LVM_SNAPSHOT environment variable.

[Goffredo Baroncelli]

Add reference to BTRFS_SKIP_LVM_SNAPSHOT environment viarble in
the btrfs(8) documentation.

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 Documentation/btrfs.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/btrfs.txt b/Documentation/btrfs.txt
index 3bdc6b4..889b97c 100644
--- a/Documentation/btrfs.txt
+++ b/Documentation/btrfs.txt
@@ -96,6 +96,14 @@ AVAILABILITY
 Please refer to the btrfs wiki http://btrfs.wiki.kernel.org for
 further details.
 
+ENVIRONMENT VARIABLES
+---------------------
+BTRFS_SKIP_LVM_SNAPSHOT::
+	Controls if the *lvm snapshots* have to be
+	ignored. Default "yes"; if you need to consider them, set it to "no".
+	Pay attention that *btrfs* may be confused if two devices have
+	the same dev uuid. This is the reason to skip a lvm snapshot.
+
 SEE ALSO
 --------
 `mkfs.btrfs`(8), `ionice`(1),
-- 
2.1.3

[PATCH 5/5] Abort in case of device uuid conflict.

[Goffredo Baroncelli]

Add a check to ensure that the all devices have differents device uuid.
In case of conflict the program ends.
Pay attention that it is still possible to insert two device with
the same dev uuid via the "btrfs device scan <device>" command.

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 volumes.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/volumes.c b/volumes.c
index 5b007fc..6ff4b4a 100644
--- a/volumes.c
+++ b/volumes.c
@@ -17,6 +17,7 @@
  */
 #define _XOPEN_SOURCE 600
 #define __USE_XOPEN2K
+#define _BSD_SOURCE		/* for minor/major macro */
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/types.h>
@@ -139,7 +140,31 @@ static int device_list_add(const char *path,
 		list_add(&device->dev_list, &fs_devices->devices);
 		device->fs_devices = fs_devices;
 	} else if (!device->name || strcmp(device->name, path)) {
-		char *name = strdup(path);
+		char *name;
+		struct stat statbuf1;
+
+		/*
+		 * check if the old device and the new device are differents
+		 * if so abort because we can't say which one is the right one
+		 */
+		if (device->name && !stat(device->name, &statbuf1)) {
+			struct stat statbuf2;
+
+			if (stat(path, &statbuf2))
+				return -ENOENT;
+
+			if (major(statbuf1.st_rdev) != major(statbuf2.st_rdev) ||
+				minor(statbuf1.st_rdev) != minor(statbuf2.st_rdev)) {
+				fprintf(stderr, "ERROR: two devices (%s and %s) have the same dev uuid !!\n",
+					device->name, path);
+				fprintf(stderr, "ERROR: remove one of the device and retry\n");
+				fprintf(stderr, "ERROR: the program will abort now\n");
+
+				exit(100);
+			}
+		}
+
+		name = strdup(path);
                 if (!name)
                         return -ENOMEM;
                 kfree(device->name);
-- 
2.1.3

Re: [PATCH V2][BTRFS-PROGS] Don't use LVM snapshot device

[Duncan]

Goffredo Baroncelli posted on Thu, 04 Dec 2014 19:39:37 +0100 as
excerpted:

> To check if a device is a LVM snapshot, it is checked the 'udev'
> device property 'DM_UDEV_LOW_PRIORITY_FLAG' . If it is set to 1,
> the device has to be skipped.
> 
> As consequence, btrfs now depends also by the libudev.

Not being a coder I gotta ask...

How does this patch deal with mdev (busybox) or static dev instead of 
udev?  Does it gracefully degrade to legacy LVM-agnostic behavior?

Meanwhile, just requiring libudev is certain to bring some pretty zealous 
opposition from the anti-systemd/anti-udev camp.  I like the goal, but 
that better be an optional dep unless we're deliberately stepping into 
that debate, because that's exactly what we'd be doing.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

Re: [PATCH V2][BTRFS-PROGS] Don't use LVM snapshot device

[Goffredo Baroncelli]

On 12/05/2014 08:26 AM, Duncan wrote:
> Goffredo Baroncelli posted on Thu, 04 Dec 2014 19:39:37 +0100 as
> excerpted:
> 
>> To check if a device is a LVM snapshot, it is checked the 'udev'
>> device property 'DM_UDEV_LOW_PRIORITY_FLAG' . If it is set to 1,
>> the device has to be skipped.
>>
>> As consequence, btrfs now depends also by the libudev.
> 
> Not being a coder I gotta ask...
> 
> How does this patch deal with mdev (busybox) or static dev instead of 
> udev?  Does it gracefully degrade to legacy LVM-agnostic behavior?

My patch no; of course we can put some #ifdef to make it an option.

[dropped the part related to systemd ....]

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 1/5] Avoid to consider lvm snapshots when scanning devices.

[Qu Wenruo]

-------- Original Message --------
Subject: [PATCH 1/5] Avoid to consider lvm snapshots when scanning devices.
From: Goffredo Baroncelli <kreijack@gmail.com>
To: <linux-btrfs@vger.kernel.org>
Date: 2014年12月05日 02:39
> LVM snapshots create a problem to the btrfs devices management.
> BTRFS assumes that each device haw an unique 'device UUID'.
> A LVM snapshot breaks this assumption.
>
> This patch skips LVM snapshots during the device scan phase.
> If you need to consider a LVM snapshot you have to set the
> environmental variable BTRFS_SKIP_LVM_SNAPSHOT to "no".
IMHO, it is better only to skip LVM snapshot if and only if the snapshot 
contains a btrfs with
conflicting UUID.

Since LVM is such a flexible block level volume management, it is 
possible that some one
did a snapshot of a btrfs fs, and then reformat the original one to 
other fs.
In that case, the LVM snapshot skip seems overkilled.

Also, personally, I prefer there will be some option like -i to allow 
user to choose which device is
used when conflicting uuid is detected. This seems to be the best case 
and user can have the full control
on device scan. This also makes the environment variant not needed.

LVM snapshot skip (only when conflicting) is better to be the fallback 
behavior if -i is not given.

Thanks,
Qu
>
> To check if a device is a LVM snapshot, it is checked the
> 'udev' device property 'DM_UDEV_LOW_PRIORITY_FLAG' .
> If it is set to 1, the device has to be skipped.
>
> As conseguence, btrfs now depends by libudev.
>
> Programmatically you can control this behavior with the functions:
> - btrfs_scan_set_skip_lvm_snapshot(int new_value)
> - int btrfs_scan_get_skip_lvm_snapshot( )
>
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
> ---
>   Makefile |   4 +--
>   utils.c  | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   utils.h  |   9 +++++-
>   3 files changed, 117 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 4cae30c..9464361 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -26,7 +26,7 @@ TESTS = fsck-tests.sh convert-tests.sh
>   INSTALL = install
>   prefix ?= /usr/local
>   bindir = $(prefix)/bin
> -lib_LIBS = -luuid -lblkid -lm -lz -llzo2 -L.
> +lib_LIBS = -luuid -lblkid -lm -lz -ludev -llzo2 -L.
>   libdir ?= $(prefix)/lib
>   incdir = $(prefix)/include/btrfs
>   LIBS = $(lib_LIBS) $(libs_static)
> @@ -99,7 +99,7 @@ lib_links = libbtrfs.so.0 libbtrfs.so
>   headers = $(libbtrfs_headers)
>   
>   # make C=1 to enable sparse
> -check_defs := .cc-defines.h
> +check_defs := .cc-defines.h
>   ifdef C
>   	#
>   	# We're trying to use sparse against glibc headers which go wild
> diff --git a/utils.c b/utils.c
> index 2a92416..9887f8b 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -29,6 +29,7 @@
>   #include <sys/types.h>
>   #include <sys/stat.h>
>   #include <uuid/uuid.h>
> +#include <libudev.h>
>   #include <fcntl.h>
>   #include <unistd.h>
>   #include <mntent.h>
> @@ -52,6 +53,13 @@
>   #define BLKDISCARD	_IO(0x12,119)
>   #endif
>   
> +/*
> + * This variable controls if the lvm snapshot have to be skipped or not.
> + * Access this variable only via the btrfs_scan_[sg]et_skip_lvm_snapshot()
> + * functions
> + */
> +static int __scan_device_skip_lvm_snapshot = -1;
> +
>   static int btrfs_scan_done = 0;
>   
>   static char argv0_buf[ARGV0_BUF_SIZE] = "btrfs";
> @@ -1593,6 +1601,9 @@ int btrfs_scan_block_devices(int run_ioctl)
>   	char fullpath[110];
>   	int scans = 0;
>   	int special;
> +	int skip_snapshot;
> +
> +	skip_snapshot = btrfs_scan_get_skip_lvm_snapshot();
>   
>   scan_again:
>   	proc_partitions = fopen("/proc/partitions","r");
> @@ -1642,6 +1653,9 @@ scan_again:
>   			continue;
>   		}
>   
> +		if (skip_snapshot && is_low_priority_device(fullpath))
> +			continue;
> +
>   		fd = open(fullpath, O_RDONLY);
>   		if (fd < 0) {
>   			if (errno != ENOMEDIUM)
> @@ -2182,6 +2196,30 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
>   	return 0;
>   }
>   
> +int btrfs_scan_get_skip_lvm_snapshot( )
> +{
> +	const char *value;
> +
> +	if (__scan_device_skip_lvm_snapshot != -1 )
> +		return __scan_device_skip_lvm_snapshot;
> +
> +	value = getenv(BTRFS_SKIP_LVM_SNAPSHOT_ENV_NAME);
> +	if (value && !strcasecmp(value, "NO"))
> +		__scan_device_skip_lvm_snapshot = 0;
> +	else  if (value && !strcasecmp(value, "YES"))
> +		__scan_device_skip_lvm_snapshot = 1;
> +	else
> +		__scan_device_skip_lvm_snapshot =
> +			BTRFS_SKIP_LVM_SNAPSHOT_DEFAULT;
> +
> +	return __scan_device_skip_lvm_snapshot;
> +}
> +
> +void btrfs_scan_set_skip_lvm_snapshot(int new_value)
> +{
> +	__scan_device_skip_lvm_snapshot = !!new_value;
> +}
> +
>   int btrfs_scan_lblkid()
>   {
>   	int fd = -1;
> @@ -2192,6 +2230,9 @@ int btrfs_scan_lblkid()
>   	blkid_dev dev = NULL;
>   	blkid_cache cache = NULL;
>   	char path[PATH_MAX];
> +	int skip_snapshot;
> +
> +	skip_snapshot = btrfs_scan_get_skip_lvm_snapshot();
>   
>   	if (btrfs_scan_done)
>   		return 0;
> @@ -2210,6 +2251,9 @@ int btrfs_scan_lblkid()
>   		/* if we are here its definitely a btrfs disk*/
>   		strncpy(path, blkid_dev_devname(dev), PATH_MAX);
>   
> +		if (skip_snapshot && is_low_priority_device(path))
> +			continue;
> +
>   		fd = open(path, O_RDONLY);
>   		if (fd < 0) {
>   			printf("ERROR: could not open %s\n", path);
> @@ -2450,3 +2494,66 @@ int find_next_key(struct btrfs_path *path, struct btrfs_key *key)
>   	}
>   	return 1;
>   }
> +
> +/*
> + * This function return 1 if the device (path) is consdered "LOW_PRIORITY" by
> + * LVM2 library. These device are typically the nsapshot.
> + * This function return < 0 in case of error; 0 otherwise.
> + */
> +int is_low_priority_device(const char *path)
> +{
> +
> +	struct udev *udev=NULL;
> +	struct udev_device *dev;
> +	struct udev_enumerate *enumerate=NULL;
> +	struct udev_list_entry *devices;
> +	struct udev_list_entry *node, *list;
> +	int ret=-1;
> +	const char *value, *syspath;
> +	char *rpath=NULL;
> +
> +	rpath = realpath(path, NULL);
> +	if (!rpath) {
> +		fprintf(stderr, "ERROR: not enough memory\n");
> +		ret=-2;
> +		goto exit;
> +	}
> +
> +	/* Create the udev object */
> +	udev = udev_new();
> +	if (!udev) {
> +		fprintf(stderr, "ERROR: Can't create udev\n");
> +		ret=-1;
> +		goto exit;
> +	}
> +
> +	enumerate = udev_enumerate_new(udev);
> +	udev_enumerate_add_match_subsystem(enumerate, "block");
> +	udev_enumerate_add_match_property(enumerate, "DEVNAME", rpath);
> +	udev_enumerate_scan_devices(enumerate);
> +
> +	devices = udev_enumerate_get_list_entry(enumerate);
> +	syspath = udev_list_entry_get_name(devices);
> +
> +	dev = udev_device_new_from_syspath(udev, syspath);
> +
> +	list = udev_device_get_properties_list_entry (dev);
> +
> +	ret = 0;
> +	node = udev_list_entry_get_by_name(list, "DM_UDEV_LOW_PRIORITY_FLAG");
> +	if (node) {
> +		value = udev_list_entry_get_value(node);
> +		if (value && !strcmp(value, "1"))
> +			ret = 1;
> +	}
> +
> +exit:
> +	free(rpath);
> +
> +	/* Free the enumerator object */
> +	udev_enumerate_unref(enumerate);
> +
> +	udev_unref(udev);
> +
> +	return ret;
> +}
> diff --git a/utils.h b/utils.h
> index 289e86b..9855f09 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -128,7 +128,7 @@ int csum_tree_block(struct btrfs_root *root, struct extent_buffer *buf,
>   			   int verify);
>   int ask_user(char *question);
>   int lookup_ino_rootid(int fd, u64 *rootid);
> -int btrfs_scan_lblkid(void);
> +int btrfs_scan_lblkid();
>   int get_btrfs_mount(const char *dev, char *mp, size_t mp_size);
>   int find_mount_root(const char *path, char **mount_root);
>   int get_device_info(int fd, u64 devid,
> @@ -161,4 +161,11 @@ static inline u64 btrfs_min_dev_size(u32 leafsize)
>   
>   int find_next_key(struct btrfs_path *path, struct btrfs_key *key);
>   
> +#define BTRFS_SKIP_LVM_SNAPSHOT_DEFAULT 1
> +#define BTRFS_SKIP_LVM_SNAPSHOT_ENV_NAME "BTRFS_SKIP_LVM_SNAPSHOT"
> +
> +int is_low_priority_device(const char *path);
> +void btrfs_scan_set_skip_lvm_snapshot(int new_value);
> +int btrfs_scan_get_skip_lvm_snapshot( );
> +
>   #endif

Re: [PATCH 1/5] Avoid to consider lvm snapshots when scanning devices.

[Goffredo Baroncelli]

On 12/08/2014 03:02 AM, Qu Wenruo wrote:
> 
> -------- Original Message --------
> Subject: [PATCH 1/5] Avoid to consider lvm snapshots when scanning devices.
> From: Goffredo Baroncelli <kreijack@gmail.com>
> To: <linux-btrfs@vger.kernel.org>
> Date: 2014年12月05日 02:39
>> LVM snapshots create a problem to the btrfs devices management.
>> BTRFS assumes that each device haw an unique 'device UUID'.
>> A LVM snapshot breaks this assumption.
>>
>> This patch skips LVM snapshots during the device scan phase.
>> If you need to consider a LVM snapshot you have to set the
>> environmental variable BTRFS_SKIP_LVM_SNAPSHOT to "no".
> IMHO, it is better only to skip LVM snapshot if and only if the snapshot contains a btrfs with
> conflicting UUID.

Hi Qu,

Currently the "scan phase" in btrfs is done 1 device at time.
(udev finds a new device and starts "btrfs dev scan <device>"),
and I haven't changed that. This means that btrfs[-prog] doesn't 
know which devices are already registered, or not. And if even 
it would know this information, you have to consider the case
where the snapshot appears before the real target. So 
btrfs[-prog] is not in position to perform this analysis [see
below my other comment]

> Since LVM is such a flexible block level volume management, it is possible that some one
> did a snapshot of a btrfs fs, and then reformat the original one to other fs.
> In that case, the LVM snapshot skip seems overkilled.
> 
> Also, personally, I prefer there will be some option like -i to allow user to choose which device is
> used when conflicting uuid is detected. This seems to be the best case and user can have the full control
> on device scan. This also makes the environment variant not needed.
> 
> LVM snapshot skip (only when conflicting) is better to be the fallback behavior if -i is not given.

I understood your reasons, but I don't find any solution 
compatible with the current btrfs device registration model
(asynchronously when the device appears).

In another patch set, I proposed a mount.btrfs helper which is
in position to perform these analysis and to pick the "right"
device (even with the user suggestion).

Today the lvm-snapshot and btrfs behave very poor: it is not
predictable which device is pick (the original or the snapshot). 
These patch *avoid* most problems skipping the snapshots, which
to me seems a reasonable default.
For the other case the user is still able to mount any disks
[combination] passing them directly via command line (
mount /dev/sdX -o device=/dev/sdY,device=/dev/sdz...  );

Anyway I think for these kind of setup (btrfs on lvm-snapshot), 
passing the disks explicitly is the only solution; in fact your 
suggestion about the '-i' switch is not very different.

> Thanks,
> Qu

BR
G.Baroncelli
>>
>> To check if a device is a LVM snapshot, it is checked the
>> 'udev' device property 'DM_UDEV_LOW_PRIORITY_FLAG' .
>> If it is set to 1, the device has to be skipped.
>>
>> As conseguence, btrfs now depends by libudev.
>>
>> Programmatically you can control this behavior with the functions:
>> - btrfs_scan_set_skip_lvm_snapshot(int new_value)
>> - int btrfs_scan_get_skip_lvm_snapshot( )
>>
>> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
>> ---
>>   Makefile |   4 +--
>>   utils.c  | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   utils.h  |   9 +++++-
>>   3 files changed, 117 insertions(+), 3 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 4cae30c..9464361 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -26,7 +26,7 @@ TESTS = fsck-tests.sh convert-tests.sh
>>   INSTALL = install
>>   prefix ?= /usr/local
>>   bindir = $(prefix)/bin
>> -lib_LIBS = -luuid -lblkid -lm -lz -llzo2 -L.
>> +lib_LIBS = -luuid -lblkid -lm -lz -ludev -llzo2 -L.
>>   libdir ?= $(prefix)/lib
>>   incdir = $(prefix)/include/btrfs
>>   LIBS = $(lib_LIBS) $(libs_static)
>> @@ -99,7 +99,7 @@ lib_links = libbtrfs.so.0 libbtrfs.so
>>   headers = $(libbtrfs_headers)
>>     # make C=1 to enable sparse
>> -check_defs := .cc-defines.h
>> +check_defs := .cc-defines.h
>>   ifdef C
>>       #
>>       # We're trying to use sparse against glibc headers which go wild
>> diff --git a/utils.c b/utils.c
>> index 2a92416..9887f8b 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -29,6 +29,7 @@
>>   #include <sys/types.h>
>>   #include <sys/stat.h>
>>   #include <uuid/uuid.h>
>> +#include <libudev.h>
>>   #include <fcntl.h>
>>   #include <unistd.h>
>>   #include <mntent.h>
>> @@ -52,6 +53,13 @@
>>   #define BLKDISCARD    _IO(0x12,119)
>>   #endif
>>   +/*
>> + * This variable controls if the lvm snapshot have to be skipped or not.
>> + * Access this variable only via the btrfs_scan_[sg]et_skip_lvm_snapshot()
>> + * functions
>> + */
>> +static int __scan_device_skip_lvm_snapshot = -1;
>> +
>>   static int btrfs_scan_done = 0;
>>     static char argv0_buf[ARGV0_BUF_SIZE] = "btrfs";
>> @@ -1593,6 +1601,9 @@ int btrfs_scan_block_devices(int run_ioctl)
>>       char fullpath[110];
>>       int scans = 0;
>>       int special;
>> +    int skip_snapshot;
>> +
>> +    skip_snapshot = btrfs_scan_get_skip_lvm_snapshot();
>>     scan_again:
>>       proc_partitions = fopen("/proc/partitions","r");
>> @@ -1642,6 +1653,9 @@ scan_again:
>>               continue;
>>           }
>>   +        if (skip_snapshot && is_low_priority_device(fullpath))
>> +            continue;
>> +
>>           fd = open(fullpath, O_RDONLY);
>>           if (fd < 0) {
>>               if (errno != ENOMEDIUM)
>> @@ -2182,6 +2196,30 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
>>       return 0;
>>   }
>>   +int btrfs_scan_get_skip_lvm_snapshot( )
>> +{
>> +    const char *value;
>> +
>> +    if (__scan_device_skip_lvm_snapshot != -1 )
>> +        return __scan_device_skip_lvm_snapshot;
>> +
>> +    value = getenv(BTRFS_SKIP_LVM_SNAPSHOT_ENV_NAME);
>> +    if (value && !strcasecmp(value, "NO"))
>> +        __scan_device_skip_lvm_snapshot = 0;
>> +    else  if (value && !strcasecmp(value, "YES"))
>> +        __scan_device_skip_lvm_snapshot = 1;
>> +    else
>> +        __scan_device_skip_lvm_snapshot =
>> +            BTRFS_SKIP_LVM_SNAPSHOT_DEFAULT;
>> +
>> +    return __scan_device_skip_lvm_snapshot;
>> +}
>> +
>> +void btrfs_scan_set_skip_lvm_snapshot(int new_value)
>> +{
>> +    __scan_device_skip_lvm_snapshot = !!new_value;
>> +}
>> +
>>   int btrfs_scan_lblkid()
>>   {
>>       int fd = -1;
>> @@ -2192,6 +2230,9 @@ int btrfs_scan_lblkid()
>>       blkid_dev dev = NULL;
>>       blkid_cache cache = NULL;
>>       char path[PATH_MAX];
>> +    int skip_snapshot;
>> +
>> +    skip_snapshot = btrfs_scan_get_skip_lvm_snapshot();
>>         if (btrfs_scan_done)
>>           return 0;
>> @@ -2210,6 +2251,9 @@ int btrfs_scan_lblkid()
>>           /* if we are here its definitely a btrfs disk*/
>>           strncpy(path, blkid_dev_devname(dev), PATH_MAX);
>>   +        if (skip_snapshot && is_low_priority_device(path))
>> +            continue;
>> +
>>           fd = open(path, O_RDONLY);
>>           if (fd < 0) {
>>               printf("ERROR: could not open %s\n", path);
>> @@ -2450,3 +2494,66 @@ int find_next_key(struct btrfs_path *path, struct btrfs_key *key)
>>       }
>>       return 1;
>>   }
>> +
>> +/*
>> + * This function return 1 if the device (path) is consdered "LOW_PRIORITY" by
>> + * LVM2 library. These device are typically the nsapshot.
>> + * This function return < 0 in case of error; 0 otherwise.
>> + */
>> +int is_low_priority_device(const char *path)
>> +{
>> +
>> +    struct udev *udev=NULL;
>> +    struct udev_device *dev;
>> +    struct udev_enumerate *enumerate=NULL;
>> +    struct udev_list_entry *devices;
>> +    struct udev_list_entry *node, *list;
>> +    int ret=-1;
>> +    const char *value, *syspath;
>> +    char *rpath=NULL;
>> +
>> +    rpath = realpath(path, NULL);
>> +    if (!rpath) {
>> +        fprintf(stderr, "ERROR: not enough memory\n");
>> +        ret=-2;
>> +        goto exit;
>> +    }
>> +
>> +    /* Create the udev object */
>> +    udev = udev_new();
>> +    if (!udev) {
>> +        fprintf(stderr, "ERROR: Can't create udev\n");
>> +        ret=-1;
>> +        goto exit;
>> +    }
>> +
>> +    enumerate = udev_enumerate_new(udev);
>> +    udev_enumerate_add_match_subsystem(enumerate, "block");
>> +    udev_enumerate_add_match_property(enumerate, "DEVNAME", rpath);
>> +    udev_enumerate_scan_devices(enumerate);
>> +
>> +    devices = udev_enumerate_get_list_entry(enumerate);
>> +    syspath = udev_list_entry_get_name(devices);
>> +
>> +    dev = udev_device_new_from_syspath(udev, syspath);
>> +
>> +    list = udev_device_get_properties_list_entry (dev);
>> +
>> +    ret = 0;
>> +    node = udev_list_entry_get_by_name(list, "DM_UDEV_LOW_PRIORITY_FLAG");
>> +    if (node) {
>> +        value = udev_list_entry_get_value(node);
>> +        if (value && !strcmp(value, "1"))
>> +            ret = 1;
>> +    }
>> +
>> +exit:
>> +    free(rpath);
>> +
>> +    /* Free the enumerator object */
>> +    udev_enumerate_unref(enumerate);
>> +
>> +    udev_unref(udev);
>> +
>> +    return ret;
>> +}
>> diff --git a/utils.h b/utils.h
>> index 289e86b..9855f09 100644
>> --- a/utils.h
>> +++ b/utils.h
>> @@ -128,7 +128,7 @@ int csum_tree_block(struct btrfs_root *root, struct extent_buffer *buf,
>>                  int verify);
>>   int ask_user(char *question);
>>   int lookup_ino_rootid(int fd, u64 *rootid);
>> -int btrfs_scan_lblkid(void);
>> +int btrfs_scan_lblkid();
>>   int get_btrfs_mount(const char *dev, char *mp, size_t mp_size);
>>   int find_mount_root(const char *path, char **mount_root);
>>   int get_device_info(int fd, u64 devid,
>> @@ -161,4 +161,11 @@ static inline u64 btrfs_min_dev_size(u32 leafsize)
>>     int find_next_key(struct btrfs_path *path, struct btrfs_key *key);
>>   +#define BTRFS_SKIP_LVM_SNAPSHOT_DEFAULT 1
>> +#define BTRFS_SKIP_LVM_SNAPSHOT_ENV_NAME "BTRFS_SKIP_LVM_SNAPSHOT"
>> +
>> +int is_low_priority_device(const char *path);
>> +void btrfs_scan_set_skip_lvm_snapshot(int new_value);
>> +int btrfs_scan_get_skip_lvm_snapshot( );
>> +
>>   #endif
> 
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH V2][BTRFS-PROGS] Don't use LVM snapshot device

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/4/2014 1:39 PM, Goffredo Baroncelli wrote:
> LVM snapshots are a problem for the btrfs devices management. BTRFS
> assumes that each device have an unique 'device UUID'. A LVM
> snapshot breaks this assumption.
> 
> This causes a lot of problems if some btrfs device are
> snapshotted: - the set of devices for a btrfs multi-volume
> filesystem may be mixed (i.e. some NON snapshotted device with some
> snapshotted devices) - /proc/mount may returns a wrong device.
> 
> In the mailing list some posts reported these incidents.
> 
> This patch allows btrfs to skip LVM snapshot during the device scan
>  phase.
> 
> But if you need to consider a LVM snapshot you can set the 
> environment variable BTRFS_SKIP_LVM_SNAPSHOT to "no". In this case 
> the old behavior is applied.
> 
> To check if a device is a LVM snapshot, it is checked the 'udev' 
> device property 'DM_UDEV_LOW_PRIORITY_FLAG' . If it is set to 1, 
> the device has to be skipped.
> 
> As consequence, btrfs now depends also by the libudev.

Rather than modify btrfs device scan to link to libudev and ignore the
caller when commanded to scan a snapshot, wouldn't it be
simpler/better to just fix the udev rule to not *call* btrfs device
scan on the snapshot?


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)

iQEcBAEBAgAGBQJUhcQyAAoJENRVrw2cjl5R1YAIAJlCine4apKnM+01Tw6JpofZ
447+FVizi5SjdgkcjcREyU5zu1pa7ioOTdExF1v1irN1xMUrRBL/RJcRjjnjkvjB
dP8JU0x52MEvQABzQP9ANWJnkMqUJ0j+ryPn+3B7wLP/RtAnIn2P9Vh1EhiLkZ9N
TdxZIPtPROWPTFBl9ONTBghOHjWYEtcDMkuTS6ZhwLh5c1LE8d3A9c68ez++oSGz
TbS51ITFZCEUyF7E/r/xWHhrYagoRM+xdYqVACpi5eY8rFKl3oH4R96gBK8hNdiN
AIOilSsNFscXiflORMAaRquW/7tUolfNt+3TfzTYmaVnK4Hv5h0wiJjiKJhNgDY=
=HlmL
-----END PGP SIGNATURE-----

Re: [PATCH V2][BTRFS-PROGS] Don't use LVM snapshot device

[Goffredo Baroncelli]

On 12/08/2014 04:30 PM, Phillip Susi wrote:
> On 12/4/2014 1:39 PM, Goffredo Baroncelli wrote:
[...]
>> To check if a device is a LVM snapshot, it is checked the 'udev' 
>> device property 'DM_UDEV_LOW_PRIORITY_FLAG' . If it is set to 1, 
>> the device has to be skipped.
> 
>> As consequence, btrfs now depends also by the libudev.
> 
> Rather than modify btrfs device scan to link to libudev and ignore the
> caller when commanded to scan a snapshot, wouldn't it be
> simpler/better to just fix the udev rule to not *call* btrfs device
> scan on the snapshot?

I like this approach, but as I wrote before, it seems that 
initramfs executes a "btrfs dev scan".... (see my previoue email 
'Re: PROBLEM: #89121 BTRFS mixes up mounted devices with their snapshots' 
date 2014/12/03 9:34):

$ grep -r "btrfs dev.*scan" /usr/share/initramfs-tools/
/usr/share/initramfs-tools/scripts/local-premount/btrfs:	/sbin/btrfs device scan 2> /dev/null

(this is from a debian). 
However it has to be pointed out that
fedora doesn't seems to do the same...

I have to investigate a bit....

> 
> 
> --
> 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
> 

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH V2][BTRFS-PROGS] Don't use LVM snapshot device

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/8/2014 12:36 PM, Goffredo Baroncelli wrote:
> I like this approach, but as I wrote before, it seems that 
> initramfs executes a "btrfs dev scan".... (see my previoue email 
> 'Re: PROBLEM: #89121 BTRFS mixes up mounted devices with their
> snapshots' date 2014/12/03 9:34):
> 
> $ grep -r "btrfs dev.*scan" /usr/share/initramfs-tools/ 
> /usr/share/initramfs-tools/scripts/local-premount/btrfs:
> /sbin/btrfs device scan 2> /dev/null
> 
> (this is from a debian). However it has to be pointed out that 
> fedora doesn't seems to do the same...

Need to fix that initramfs script then.  On the other hand, if one
*does* run a scan with no arguments, then it probably is a good idea
to ignore snapshots.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)

iQEcBAEBAgAGBQJUhesjAAoJENRVrw2cjl5RPRAIALK5ERJqRJLSsa6kBSIP8bWe
WP531ew49I0Tkc0o3YOCqq07tb4kZ5rsLsPaLE+s3adCe5/wYzQOox4x6ucak1gK
0igazFx9TYM65YRtFzIUAnj/CPN4WwIInwoAac4w2qwCKB56WUbSU60lEsOmFfRr
6m9EUYkBtMRiWfW2jjuj8iLnBW6glexAqTpW1eKWPfF0AGoUXc8AQboNwceFnHi3
vcjmQM6mhL5zH+FJ0Z/meTk/PwVdjEVJQIEcbMpvggAJeqxsm90GHVIsn8C7B80i
GcX8GHe+Gw3WJMsaW49slKa+MOjWt2SumN/lrKFYPVwQUguhvg0hC1UG5m3cJFo=
=64dH
-----END PGP SIGNATURE-----

Re: [PATCH V2][BTRFS-PROGS] Don't use LVM snapshot device

[Robert White]

On 12/08/2014 09:36 AM, Goffredo Baroncelli wrote:
> I like this approach, but as I wrote before, it seems that
> initramfs executes a "btrfs dev scan".... (see my previoue email
> 'Re: PROBLEM: #89121 BTRFS mixes up mounted devices with their snapshots'
> date 2014/12/03 9:34):

Roll your own for now. I haven't started doing any significant btrfs 
specific work on it but the initramfs builder in my project 
http://underdog.sourceforge.net might get you past your problem pretty 
easily for now. It would be easy to white/black list which devices you 
want to submit to scan or mount.

It is plumbed up to look at each storage region one-by-one so you could 
assemble your file system that way.

(Note that the eventual point of the project isn't really the initramfs 
stuff but that's what I needed more/first.)

It's fairly well documented and I use it for some non-trivially complex 
systems but its not (yet) so complex that it's hard to design hooks for it.

Re: [PATCH 1/5] Avoid to consider lvm snapshots when scanning devices.

[Qu Wenruo]

Hi Goffredo

On 12/08/2014 03:02 AM, Qu Wenruo wrote:
>> -------- Original Message --------
>> Subject: [PATCH 1/5] Avoid to consider lvm snapshots when scanning devices.
>> From: Goffredo Baroncelli <kreijack@gmail.com>
>> To: <linux-btrfs@vger.kernel.org>
>> Date: 2014年12月05日 02:39
>>> LVM snapshots create a problem to the btrfs devices management.
>>> BTRFS assumes that each device haw an unique 'device UUID'.
>>> A LVM snapshot breaks this assumption.
>>>
>>> This patch skips LVM snapshots during the device scan phase.
>>> If you need to consider a LVM snapshot you have to set the
>>> environmental variable BTRFS_SKIP_LVM_SNAPSHOT to "no".
>> IMHO, it is better only to skip LVM snapshot if and only if the snapshot contains a btrfs with
>> conflicting UUID.
> Hi Qu,
>
> Currently the "scan phase" in btrfs is done 1 device at time.
> (udev finds a new device and starts "btrfs dev scan <device>"),
> and I haven't changed that. This means that btrfs[-prog] doesn't
> know which devices are already registered, or not. And if even
> it would know this information, you have to consider the case
> where the snapshot appears before the real target. So
> btrfs[-prog] is not in position to perform this analysis [see
> below my other comment]
>
>> Since LVM is such a flexible block level volume management, it is possible that some one
>> did a snapshot of a btrfs fs, and then reformat the original one to other fs.
>> In that case, the LVM snapshot skip seems overkilled.
>>
>> Also, personally, I prefer there will be some option like -i to allow user to choose which device is
>> used when conflicting uuid is detected. This seems to be the best case and user can have the full control
>> on device scan. This also makes the environment variant not needed.
>>
>> LVM snapshot skip (only when conflicting) is better to be the fallback behavior if -i is not given.
> I understood your reasons, but I don't find any solution
> compatible with the current btrfs device registration model
> (asynchronously when the device appears).
>
> In another patch set, I proposed a mount.btrfs helper which is
> in position to perform these analysis and to pick the "right"
> device (even with the user suggestion).
>
> Today the lvm-snapshot and btrfs behave very poor: it is not
> predictable which device is pick (the original or the snapshot).
> These patch *avoid* most problems skipping the snapshots, which
> to me seems a reasonable default.
> For the other case the user is still able to mount any disks
> [combination] passing them directly via command line (
> mount /dev/sdX -o device=/dev/sdY,device=/dev/sdz...  );
>
> Anyway I think for these kind of setup (btrfs on lvm-snapshot),
> passing the disks explicitly is the only solution; in fact your
> suggestion about the '-i' switch is not very different.
>
>> Thanks,
>> Qu
> BR
> G.Baroncelli
Your explains sounds quite reasonable, it's good enough before any 
better solutions.

Thanks,
Qu
>>> To check if a device is a LVM snapshot, it is checked the
>>> 'udev' device property 'DM_UDEV_LOW_PRIORITY_FLAG' .
>>> If it is set to 1, the device has to be skipped.
>>>
>>> As conseguence, btrfs now depends by libudev.
>>>
>>> Programmatically you can control this behavior with the functions:
>>> - btrfs_scan_set_skip_lvm_snapshot(int new_value)
>>> - int btrfs_scan_get_skip_lvm_snapshot( )
>>>
>>> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
>>> ---
>>>    Makefile |   4 +--
>>>    utils.c  | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>    utils.h  |   9 +++++-
>>>    3 files changed, 117 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 4cae30c..9464361 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -26,7 +26,7 @@ TESTS = fsck-tests.sh convert-tests.sh
>>>    INSTALL = install
>>>    prefix ?= /usr/local
>>>    bindir = $(prefix)/bin
>>> -lib_LIBS = -luuid -lblkid -lm -lz -llzo2 -L.
>>> +lib_LIBS = -luuid -lblkid -lm -lz -ludev -llzo2 -L.
>>>    libdir ?= $(prefix)/lib
>>>    incdir = $(prefix)/include/btrfs
>>>    LIBS = $(lib_LIBS) $(libs_static)
>>> @@ -99,7 +99,7 @@ lib_links = libbtrfs.so.0 libbtrfs.so
>>>    headers = $(libbtrfs_headers)
>>>      # make C=1 to enable sparse
>>> -check_defs := .cc-defines.h
>>> +check_defs := .cc-defines.h
>>>    ifdef C
>>>        #
>>>        # We're trying to use sparse against glibc headers which go wild
>>> diff --git a/utils.c b/utils.c
>>> index 2a92416..9887f8b 100644
>>> --- a/utils.c
>>> +++ b/utils.c
>>> @@ -29,6 +29,7 @@
>>>    #include <sys/types.h>
>>>    #include <sys/stat.h>
>>>    #include <uuid/uuid.h>
>>> +#include <libudev.h>
>>>    #include <fcntl.h>
>>>    #include <unistd.h>
>>>    #include <mntent.h>
>>> @@ -52,6 +53,13 @@
>>>    #define BLKDISCARD    _IO(0x12,119)
>>>    #endif
>>>    +/*
>>> + * This variable controls if the lvm snapshot have to be skipped or not.
>>> + * Access this variable only via the btrfs_scan_[sg]et_skip_lvm_snapshot()
>>> + * functions
>>> + */
>>> +static int __scan_device_skip_lvm_snapshot = -1;
>>> +
>>>    static int btrfs_scan_done = 0;
>>>      static char argv0_buf[ARGV0_BUF_SIZE] = "btrfs";
>>> @@ -1593,6 +1601,9 @@ int btrfs_scan_block_devices(int run_ioctl)
>>>        char fullpath[110];
>>>        int scans = 0;
>>>        int special;
>>> +    int skip_snapshot;
>>> +
>>> +    skip_snapshot = btrfs_scan_get_skip_lvm_snapshot();
>>>      scan_again:
>>>        proc_partitions = fopen("/proc/partitions","r");
>>> @@ -1642,6 +1653,9 @@ scan_again:
>>>                continue;
>>>            }
>>>    +        if (skip_snapshot && is_low_priority_device(fullpath))
>>> +            continue;
>>> +
>>>            fd = open(fullpath, O_RDONLY);
>>>            if (fd < 0) {
>>>                if (errno != ENOMEDIUM)
>>> @@ -2182,6 +2196,30 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
>>>        return 0;
>>>    }
>>>    +int btrfs_scan_get_skip_lvm_snapshot( )
>>> +{
>>> +    const char *value;
>>> +
>>> +    if (__scan_device_skip_lvm_snapshot != -1 )
>>> +        return __scan_device_skip_lvm_snapshot;
>>> +
>>> +    value = getenv(BTRFS_SKIP_LVM_SNAPSHOT_ENV_NAME);
>>> +    if (value && !strcasecmp(value, "NO"))
>>> +        __scan_device_skip_lvm_snapshot = 0;
>>> +    else  if (value && !strcasecmp(value, "YES"))
>>> +        __scan_device_skip_lvm_snapshot = 1;
>>> +    else
>>> +        __scan_device_skip_lvm_snapshot =
>>> +            BTRFS_SKIP_LVM_SNAPSHOT_DEFAULT;
>>> +
>>> +    return __scan_device_skip_lvm_snapshot;
>>> +}
>>> +
>>> +void btrfs_scan_set_skip_lvm_snapshot(int new_value)
>>> +{
>>> +    __scan_device_skip_lvm_snapshot = !!new_value;
>>> +}
>>> +
>>>    int btrfs_scan_lblkid()
>>>    {
>>>        int fd = -1;
>>> @@ -2192,6 +2230,9 @@ int btrfs_scan_lblkid()
>>>        blkid_dev dev = NULL;
>>>        blkid_cache cache = NULL;
>>>        char path[PATH_MAX];
>>> +    int skip_snapshot;
>>> +
>>> +    skip_snapshot = btrfs_scan_get_skip_lvm_snapshot();
>>>          if (btrfs_scan_done)
>>>            return 0;
>>> @@ -2210,6 +2251,9 @@ int btrfs_scan_lblkid()
>>>            /* if we are here its definitely a btrfs disk*/
>>>            strncpy(path, blkid_dev_devname(dev), PATH_MAX);
>>>    +        if (skip_snapshot && is_low_priority_device(path))
>>> +            continue;
>>> +
>>>            fd = open(path, O_RDONLY);
>>>            if (fd < 0) {
>>>                printf("ERROR: could not open %s\n", path);
>>> @@ -2450,3 +2494,66 @@ int find_next_key(struct btrfs_path *path, struct btrfs_key *key)
>>>        }
>>>        return 1;
>>>    }
>>> +
>>> +/*
>>> + * This function return 1 if the device (path) is consdered "LOW_PRIORITY" by
>>> + * LVM2 library. These device are typically the nsapshot.
>>> + * This function return < 0 in case of error; 0 otherwise.
>>> + */
>>> +int is_low_priority_device(const char *path)
>>> +{
>>> +
>>> +    struct udev *udev=NULL;
>>> +    struct udev_device *dev;
>>> +    struct udev_enumerate *enumerate=NULL;
>>> +    struct udev_list_entry *devices;
>>> +    struct udev_list_entry *node, *list;
>>> +    int ret=-1;
>>> +    const char *value, *syspath;
>>> +    char *rpath=NULL;
>>> +
>>> +    rpath = realpath(path, NULL);
>>> +    if (!rpath) {
>>> +        fprintf(stderr, "ERROR: not enough memory\n");
>>> +        ret=-2;
>>> +        goto exit;
>>> +    }
>>> +
>>> +    /* Create the udev object */
>>> +    udev = udev_new();
>>> +    if (!udev) {
>>> +        fprintf(stderr, "ERROR: Can't create udev\n");
>>> +        ret=-1;
>>> +        goto exit;
>>> +    }
>>> +
>>> +    enumerate = udev_enumerate_new(udev);
>>> +    udev_enumerate_add_match_subsystem(enumerate, "block");
>>> +    udev_enumerate_add_match_property(enumerate, "DEVNAME", rpath);
>>> +    udev_enumerate_scan_devices(enumerate);
>>> +
>>> +    devices = udev_enumerate_get_list_entry(enumerate);
>>> +    syspath = udev_list_entry_get_name(devices);
>>> +
>>> +    dev = udev_device_new_from_syspath(udev, syspath);
>>> +
>>> +    list = udev_device_get_properties_list_entry (dev);
>>> +
>>> +    ret = 0;
>>> +    node = udev_list_entry_get_by_name(list, "DM_UDEV_LOW_PRIORITY_FLAG");
>>> +    if (node) {
>>> +        value = udev_list_entry_get_value(node);
>>> +        if (value && !strcmp(value, "1"))
>>> +            ret = 1;
>>> +    }
>>> +
>>> +exit:
>>> +    free(rpath);
>>> +
>>> +    /* Free the enumerator object */
>>> +    udev_enumerate_unref(enumerate);
>>> +
>>> +    udev_unref(udev);
>>> +
>>> +    return ret;
>>> +}
>>> diff --git a/utils.h b/utils.h
>>> index 289e86b..9855f09 100644
>>> --- a/utils.h
>>> +++ b/utils.h
>>> @@ -128,7 +128,7 @@ int csum_tree_block(struct btrfs_root *root, struct extent_buffer *buf,
>>>                   int verify);
>>>    int ask_user(char *question);
>>>    int lookup_ino_rootid(int fd, u64 *rootid);
>>> -int btrfs_scan_lblkid(void);
>>> +int btrfs_scan_lblkid();
>>>    int get_btrfs_mount(const char *dev, char *mp, size_t mp_size);
>>>    int find_mount_root(const char *path, char **mount_root);
>>>    int get_device_info(int fd, u64 devid,
>>> @@ -161,4 +161,11 @@ static inline u64 btrfs_min_dev_size(u32 leafsize)
>>>      int find_next_key(struct btrfs_path *path, struct btrfs_key *key);
>>>    +#define BTRFS_SKIP_LVM_SNAPSHOT_DEFAULT 1
>>> +#define BTRFS_SKIP_LVM_SNAPSHOT_ENV_NAME "BTRFS_SKIP_LVM_SNAPSHOT"
>>> +
>>> +int is_low_priority_device(const char *path);
>>> +void btrfs_scan_set_skip_lvm_snapshot(int new_value);
>>> +int btrfs_scan_get_skip_lvm_snapshot( );
>>> +
>>>    #endif
>>
>

Re: [PATCH 1/5] Avoid to consider lvm snapshots when scanning devices.

[David Sterba]

On Mon, Dec 08, 2014 at 03:58:09PM +0100, Goffredo Baroncelli wrote:
> In another patch set, I proposed a mount.btrfs helper which is
> in position to perform these analysis and to pick the "right"
> device (even with the user suggestion).
> 
> Today the lvm-snapshot and btrfs behave very poor: it is not
> predictable which device is pick (the original or the snapshot). 
> These patch *avoid* most problems skipping the snapshots, which
> to me seems a reasonable default.
> For the other case the user is still able to mount any disks
> [combination] passing them directly via command line (
> mount /dev/sdX -o device=/dev/sdY,device=/dev/sdz...  );

Beware that passing 'device' does not mean that btrfs will use that
device to assemble the filesystem. It only says to scan the device the
same way any preceding 'btrfs dev scan' would do.

super.c:
 822                 case Opt_device:
 823                         device_name = match_strdup(&args[0]);
 824                         if (!device_name) {
 825                                 error = -ENOMEM;
 826                                 goto out;
 827                         }
 828                         error = btrfs_scan_one_device(device_name,
 829                                         flags, holder, fs_devices);
 830                         kfree(device_name);
 831                         if (error)
 832                                 goto out;
 833                         break;

> Anyway I think for these kind of setup (btrfs on lvm-snapshot), 
> passing the disks explicitly is the only solution; [...]

... for which you'd need another mount option and update the code that
selects the devices accordingly.

Re: [PATCH 1/5] Avoid to consider lvm snapshots when scanning devices.

[Goffredo Baroncelli]

On 12/09/2014 11:27 AM, David Sterba wrote:
>> > Today the lvm-snapshot and btrfs behave very poor: it is not
>> > predictable which device is pick (the original or the snapshot). 
>> > These patch *avoid* most problems skipping the snapshots, which
>> > to me seems a reasonable default.
>> > For the other case the user is still able to mount any disks
>> > [combination] passing them directly via command line (
>> > mount /dev/sdX -o device=/dev/sdY,device=/dev/sdz...  );

> Beware that passing 'device' does not mean that btrfs will use that
> device to assemble the filesystem. It only says to scan the device the
> same way any preceding 'btrfs dev scan' would do.

I thought a bit about your sentence, but I was unable to understand
the difference. Could you describe a case where it is different ?

I have quite clear that "btrfs scan <dev>" and "mount -o device=<dev>"
do the same thing: these fill a table with the devices information 
grouped by fsid. Then the kernel uses this table as hint to pick 
the devices for a filesystem. So except some strange case 
(like device "hot" removed) this shouldn't make any difference... 
Or not ?

The point is that when a btrfs scan is ran asynchronously,
a snapshot "may" hide the origin volume. Where the word
"may" means that it is not predictable. Passing the device solve only
this point: it becomes predictable which device is used.


 
-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH V2][BTRFS-PROGS] Don't use LVM snapshot device

[Anand Jain]

> This patch allows btrfs to skip LVM snapshot during the device scan
> phase.

  Its better to generalize the problem and fix it. The fix here is very
  specific to LVM use case. This does not work in cases where device is
  cloned using dd (device is unmounted).

  As mentioned we need to depend on the device wwn as provided by the
  device target driver.

Thanks, Anand

Re: [PATCH V2][BTRFS-PROGS] Don't use LVM snapshot device

[Goffredo Baroncelli]

On 12/10/2014 08:52 AM, Anand Jain wrote:
> 
> 
>> This patch allows btrfs to skip LVM snapshot during the device scan
>> phase.
> 
>  Its better to generalize the problem and fix it. The fix here is very
>  specific to LVM use case. This does not work in cases where device is
>  cloned using dd (device is unmounted).

See patch #5; this aborts btrfs[progs] when two devices have the same
dev.uuid and fsid.

Unfortunately this patch doesn't work with "btrfs dev scan"; this
because each device is discovered/registered alone. See my patches about 
mount.btrfs for an alternative approach.

> As mentioned we need to depend on the device wwn as provided by the
> device target driver.
> 
> Thanks, Anand
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5