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.

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_debug("%s already opened read-only. Upgrading "
-			log_error(INTERNAL_ERROR "%s already opened read-only",
+				  "to read-write.", dev_name(dev));
-				 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;
+	return dev_open_flags(dev, O_RDWR, 1, 1);
-
-	flags = vg_write_lock_held() ? O_RDWR : O_RDONLY;
-
-	return dev_open_flags(dev, flags, 1, 1);
 }
 
 int dev_open(struct device *dev)
 {
-	int flags;
+	return dev_open_flags(dev, O_RDWR, 1, 0);
-
-	flags = vg_write_lock_held() ? O_RDWR : O_RDONLY;
-
-	return dev_open_flags(dev, flags, 1, 0);
 }
 
 int dev_open_readonly(struct device *dev)

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

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

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

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

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

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

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;

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,

tools/lvmdiskscan.c

@@ -72,7 +72,7 @@ static int _check_device(struct cmd_context *cmd, struct device *dev)
 	char buffer;
 	uint64_t size;
 
-	if (!dev_open(dev)) {
+	if (!dev_open_readonly(dev)) {
 		return 0;
 	}
 	if (!dev_read(dev, UINT64_C(0), (size_t) 1, &buffer)) {