Use new dev_open_readonly fn to prevent opening devices for read-write when not necessary.

Before, we used vg_write_lock_held call to determnine the way a device is
opened. Unfortunately, this opened many devices in RW mode when it was not
really necessary. With the OPTIONS+="watch" rule used in the udev rules,
this could fire numerous events while closing such devices (and it caused
useless scans from within udev rules in return).

A common bug we hit with this was with the lvremove command which was unable
to remove the LV since it was being opened from within the udev rules. This
patch should minimize such situations (at least with respect to LVM handling
of devices).

Though there's still a possibility someone will open a device 'outside' in
parallel and fire the event based on the watch rule when closing a device
once opened for RW.

9 files changed, 17 insertions, 26 deletions

diff --git a/lib/device/dev-io.c b/lib/device/dev-io.c
index 16aad38c..9b486a98 100644
--- a/lib/device/dev-io.c
+++ b/lib/device/dev-io.c
@@ -431,9 +431,8 @@ int dev_open_flags(struct device *dev, int flags, int direct, int quiet)
 		}
 
 		if (dev->open_count && !need_excl) {
-			/* FIXME Ensure we never get here */
-			log_error(INTERNAL_ERROR "%s already opened read-only",
-				 dev_name(dev));
+			log_debug("%s already opened read-only. Upgrading "
+				  "to read-write.", dev_name(dev));
 			dev->open_count++;
 		}
 
@@ -540,20 +539,12 @@ int dev_open_flags(struct device *dev, int flags, int direct, int quiet)
 
 int dev_open_quiet(struct device *dev)
 {
-	int flags;
-
-	flags = vg_write_lock_held() ? O_RDWR : O_RDONLY;
-
-	return dev_open_flags(dev, flags, 1, 1);
+	return dev_open_flags(dev, O_RDWR, 1, 1);
 }
 
 int dev_open(struct device *dev)
 {
-	int flags;
-
-	flags = vg_write_lock_held() ? O_RDWR : O_RDONLY;
-
-	return dev_open_flags(dev, flags, 1, 0);
+	return dev_open_flags(dev, O_RDWR, 1, 0);
 }
 
 int dev_open_readonly(struct device *dev)
diff --git a/lib/device/dev-luks.c b/lib/device/dev-luks.c
index 6337992e..10aae300 100644
--- a/lib/device/dev-luks.c
+++ b/lib/device/dev-luks.c
@@ -23,7 +23,7 @@ int dev_is_luks(struct device *dev, uint64_t *signature)
 	char buf[LUKS_SIGNATURE_SIZE];
 	int ret = -1;
 
-	if (!dev_open(dev)) {
+	if (!dev_open_readonly(dev)) {
 		stack;
 		return -1;
 	}
diff --git a/lib/device/dev-md.c b/lib/device/dev-md.c
index 89b93410..0d522f4d 100644
--- a/lib/device/dev-md.c
+++ b/lib/device/dev-md.c
@@ -94,7 +94,7 @@ int dev_is_md(struct device *dev, uint64_t *sb)
 	if (size < MD_RESERVED_SECTORS * 2)
 		return 0;
 
-	if (!dev_open(dev)) {
+	if (!dev_open_readonly(dev)) {
 		stack;
 		return -1;
 	}
diff --git a/lib/device/dev-swap.c b/lib/device/dev-swap.c
index 287eafd7..f742bfe1 100644
--- a/lib/device/dev-swap.c
+++ b/lib/device/dev-swap.c
@@ -50,7 +50,7 @@ int dev_is_swap(struct device *dev, uint64_t *signature)
 		return -1;
 	}
 
-	if (!dev_open(dev)) {
+	if (!dev_open_readonly(dev)) {
 		stack;
 		return -1;
 	}
diff --git a/lib/format1/disk-rep.c b/lib/format1/disk-rep.c
index 071b39dd..c3aeac65 100644
--- a/lib/format1/disk-rep.c
+++ b/lib/format1/disk-rep.c
@@ -415,7 +415,7 @@ struct disk_list *read_disk(const struct format_type *fmt, struct device *dev,
 {
 	struct disk_list *dl;
 
-	if (!dev_open(dev))
+	if (!dev_open_readonly(dev))
 		return_NULL;
 
 	dl = __read_disk(fmt, dev, mem, vg_name);
diff --git a/lib/format_pool/disk_rep.c b/lib/format_pool/disk_rep.c
index ca8bfd73..589ffbf2 100644
--- a/lib/format_pool/disk_rep.c
+++ b/lib/format_pool/disk_rep.c
@@ -355,7 +355,7 @@ struct pool_list *read_pool_disk(const struct format_type *fmt,
 {
 	struct pool_list *pl;
 
-	if (!dev_open(dev))
+	if (!dev_open_readonly(dev))
 		return_NULL;
 
 	if (!(pl = dm_pool_zalloc(mem, sizeof(*pl)))) {
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 7e799152..383cae14 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -184,7 +184,7 @@ static int _pv_analyze_mda_raw (const struct format_type * fmt,
 		  PRIu64, mdac->area.start, mdac->area.size);
 	area = &mdac->area;
 
-	if (!dev_open(area->dev))
+	if (!dev_open_readonly(area->dev))
 		return_0;
 
 	if (!(mdah = raw_read_mda_header(fmt, area)))
@@ -462,7 +462,7 @@ static int _raw_holds_vgname(struct format_instance *fid,
 	int noprecommit = 0;
 	struct mda_header *mdah;
 
-	if (!dev_open(dev_area->dev))
+	if (!dev_open_readonly(dev_area->dev))
 		return_0;
 
 	if (!(mdah = raw_read_mda_header(fid->fmt, dev_area)))
@@ -533,7 +533,7 @@ static struct volume_group *_vg_read_raw(struct format_instance *fid,
 	struct mda_context *mdac = (struct mda_context *) mda->metadata_locn;
 	struct volume_group *vg;
 
-	if (!dev_open(mdac->area.dev))
+	if (!dev_open_readonly(mdac->area.dev))
 		return_NULL;
 
 	vg = _vg_read_raw_area(fid, vgname, &mdac->area, 0);
@@ -551,7 +551,7 @@ static struct volume_group *_vg_read_precommit_raw(struct format_instance *fid,
 	struct mda_context *mdac = (struct mda_context *) mda->metadata_locn;
 	struct volume_group *vg;
 
-	if (!dev_open(mdac->area.dev))
+	if (!dev_open_readonly(mdac->area.dev))
 		return_NULL;
 
 	vg = _vg_read_raw_area(fid, vgname, &mdac->area, 1);
@@ -1218,7 +1218,7 @@ static int _scan_raw(const struct format_type *fmt, const char *vgname __attribu
 
 	dm_list_iterate_items(rl, raw_list) {
 		/* FIXME We're reading mdah twice here... */
-		if (!dev_open(rl->dev_area.dev)) {
+		if (!dev_open_readonly(rl->dev_area.dev)) {
 			stack;
 			continue;
 		}
diff --git a/lib/format_text/text_label.c b/lib/format_text/text_label.c
index 675dbe84..11f93e20 100644
--- a/lib/format_text/text_label.c
+++ b/lib/format_text/text_label.c
@@ -302,7 +302,7 @@ static int _text_read(struct labeller *l, struct device *dev, void *buf,
 
 	dm_list_iterate_items(mda, &info->mdas) {
 		mdac = (struct mda_context *) mda->metadata_locn;
-		if (!dev_open(mdac->area.dev)) {
+		if (!dev_open_readonly(mdac->area.dev)) {
 			mda_set_ignored(mda, 1);
 			stack;
 			continue;
diff --git a/lib/label/label.c b/lib/label/label.c
index c622812f..f1efecfe 100644
--- a/lib/label/label.c
+++ b/lib/label/label.c
@@ -272,7 +272,7 @@ int label_read(struct device *dev, struct label **result,
 		return 1;
 	}
 
-	if (!dev_open(dev)) {
+	if (!dev_open_readonly(dev)) {
 		stack;
 
 		if ((info = info_from_pvid(dev->pvid, 0)))
@@ -352,7 +352,7 @@ int label_verify(struct device *dev)
 	struct lvmcache_info *info;
 	int r = 0;
 
-	if (!dev_open(dev)) {
+	if (!dev_open_readonly(dev)) {
 		if ((info = info_from_pvid(dev->pvid, 0)))
 			lvmcache_update_vgname_and_id(info, info->fmt->orphan_vg_name,
 						      info->fmt->orphan_vg_name,