From c08c564e216834ddce5e6c97ee365bf763fb0781 Mon Sep 17 00:00:00 2001 From: Peter Rajnoha Date: Sat, 28 May 2011 09:48:14 +0000 Subject: [PATCH] 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 | 17 ++++------------- lib/device/dev-luks.c | 2 +- lib/device/dev-md.c | 2 +- lib/device/dev-swap.c | 2 +- lib/format1/disk-rep.c | 2 +- lib/format_pool/disk_rep.c | 2 +- lib/format_text/format-text.c | 10 +++++----- lib/format_text/text_label.c | 2 +- lib/label/label.c | 4 ++-- tools/lvmdiskscan.c | 2 +- 10 files changed, 18 insertions(+), 27 deletions(-) diff --git a/lib/device/dev-io.c b/lib/device/dev-io.c index 16aad38c4..9b486a987 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 6337992e5..10aae300a 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 89b93410e..0d522f4d5 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 287eafd72..f742bfe1a 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 071b39dd8..c3aeac657 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 ca8bfd736..589ffbf25 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 7e7991526..383cae146 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 675dbe84e..11f93e209 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 c622812ff..f1efecfe0 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, diff --git a/tools/lvmdiskscan.c b/tools/lvmdiskscan.c index 9127c1561..a35667e00 100644 --- a/tools/lvmdiskscan.c +++ b/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)) {