From 9b02bdbce0c661b78eb1ad42423771a71d27ffc2 Mon Sep 17 00:00:00 2001 From: Alasdair Kergon Date: Mon, 3 Oct 2005 21:10:41 +0000 Subject: [PATCH] Refuse to run pvcreate/pvremove on devices we can't open exclusively. --- WHATS_NEW | 1 + lib/device/dev-io.c | 43 ++++++++++++++++++++++++++++++++++++++----- lib/device/device.h | 6 ++++-- tools/pvcreate.c | 14 ++++++++++++-- tools/pvremove.c | 6 ++++++ 5 files changed, 61 insertions(+), 9 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index 62dad093b..8f355e433 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,6 @@ Version 2.01.15 - ================================= + Refuse to run pvcreate/pvremove on devices we can't open exclusively. Use ORPHAN lock definition throughout. Validate chunksize in lvcreate. Reduce chunksize limit to 512k. diff --git a/lib/device/dev-io.c b/lib/device/dev-io.c index 836336e6a..40e29c94a 100644 --- a/lib/device/dev-io.c +++ b/lib/device/dev-io.c @@ -320,15 +320,22 @@ int dev_open_flags(struct device *dev, int flags, int direct, int quiet) { struct stat buf; const char *name; + int need_excl = 0, need_rw = 0; + + if ((flags & O_ACCMODE) == O_RDWR) + need_rw = 1; + + if ((flags & O_EXCL)) + need_excl = 1; if (dev->fd >= 0) { - if ((dev->flags & DEV_OPENED_RW) || - ((flags & O_ACCMODE) != O_RDWR)) { + if (((dev->flags & DEV_OPENED_RW) || !need_rw) && + ((dev->flags & DEV_OPENED_EXCL) || !need_excl)) { dev->open_count++; return 1; } - if (dev->open_count) { + if (dev->open_count && !need_excl) { /* FIXME Ensure we never get here */ log_debug("WARNING: %s already opened read-only", dev_name(dev)); @@ -397,11 +404,16 @@ int dev_open_flags(struct device *dev, int flags, int direct, int quiet) dev->open_count++; dev->flags &= ~DEV_ACCESSED_W; - if ((flags & O_ACCMODE) == O_RDWR) + if (need_rw) dev->flags |= DEV_OPENED_RW; else dev->flags &= ~DEV_OPENED_RW; + if (need_excl) + dev->flags |= DEV_OPENED_EXCL; + else + dev->flags &= ~DEV_OPENED_EXCL; + if (!(dev->flags & DEV_REGULAR) && ((fstat(dev->fd, &buf) < 0) || (buf.st_rdev != dev->dev))) { log_error("%s: fstat failed: Has device name changed?", name); @@ -420,8 +432,9 @@ int dev_open_flags(struct device *dev, int flags, int direct, int quiet) list_add(&_open_devices, &dev->open_list); - log_debug("Opened %s %s%s", dev_name(dev), + log_debug("Opened %s %s%s%s", dev_name(dev), dev->flags & DEV_OPENED_RW ? "RW" : "RO", + dev->flags & DEV_OPENED_EXCL ? " O_EXCL" : "", dev->flags & DEV_O_DIRECT ? " O_DIRECT" : ""); return 1; @@ -445,6 +458,21 @@ int dev_open(struct device *dev) return dev_open_flags(dev, flags, 1, 0); } +int dev_test_excl(struct device *dev) +{ + int flags; + int r; + + flags = vg_write_lock_held() ? O_RDWR : O_RDONLY; + flags |= O_EXCL; + + r = dev_open_flags(dev, flags, 1, 1); + if (r) + dev_close_immediate(dev); + + return r; +} + static void _close(struct device *dev) { if (close(dev->fd)) @@ -479,6 +507,11 @@ static int _dev_close(struct device *dev, int immediate) if (dev->open_count > 0) dev->open_count--; + if (immediate && dev->open_count) { + log_debug("%s: Immediate close attempt while still referenced"); + dev->open_count = 0; + } + /* FIXME lookup device in cache to get vgname and see if it's locked? */ if (immediate || (dev->open_count < 1 && !vgs_locked())) _close(dev); diff --git a/lib/device/device.h b/lib/device/device.h index afe29e368..f00ec7b51 100644 --- a/lib/device/device.h +++ b/lib/device/device.h @@ -23,8 +23,9 @@ #define DEV_REGULAR 0x00000002 /* Regular file? */ #define DEV_ALLOCED 0x00000004 /* dbg_malloc used */ #define DEV_OPENED_RW 0x00000008 /* Opened RW */ -#define DEV_O_DIRECT 0x00000010 /* Use O_DIRECT */ -#define DEV_O_DIRECT_TESTED 0x00000020 /* DEV_O_DIRECT is reliable */ +#define DEV_OPENED_EXCL 0x00000010 /* Opened EXCL */ +#define DEV_O_DIRECT 0x00000020 /* Use O_DIRECT */ +#define DEV_O_DIRECT_TESTED 0x00000040 /* DEV_O_DIRECT is reliable */ /* * All devices in LVM will be represented by one of these. @@ -70,6 +71,7 @@ int dev_open_flags(struct device *dev, int flags, int direct, int quiet); int dev_close(struct device *dev); int dev_close_immediate(struct device *dev); void dev_close_all(void); +int dev_test_excl(struct device *dev); static inline int dev_fd(struct device *dev) { diff --git a/tools/pvcreate.c b/tools/pvcreate.c index a0b7807a0..75995dd30 100644 --- a/tools/pvcreate.c +++ b/tools/pvcreate.c @@ -82,6 +82,12 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name) return 0; } + if (!dev_test_excl(dev)) { + log_error("Can't open %s exclusively. Mounted filesystem?", + name); + return 0; + } + /* Wipe superblock? */ if (dev_is_md(dev, &md_superblock) && ((!arg_count(cmd, uuidstr_ARG) && @@ -218,8 +224,12 @@ static int pvcreate_single(struct cmd_context *cmd, const char *pv_name, log_error("%s not opened: device not zeroed", pv_name); goto error; } - - dev_zero(dev, UINT64_C(0), (size_t) 2048); + + if (!dev_zero(dev, UINT64_C(0), (size_t) 2048)) { + log_error("%s not wiped: aborting", pv_name); + dev_close(dev); + goto error; + } dev_close(dev); } diff --git a/tools/pvremove.c b/tools/pvremove.c index 6fd9daa08..1fce587a3 100644 --- a/tools/pvremove.c +++ b/tools/pvremove.c @@ -86,6 +86,12 @@ static int pvremove_single(struct cmd_context *cmd, const char *pv_name, goto error; } + if (!dev_test_excl(dev)) { + log_error("Can't open %s exclusively. Mounted filesystem?", + dev_name(dev)); + return 0; + } + /* Wipe existing label(s) */ if (!label_remove(dev)) { log_error("Failed to wipe existing label(s) on %s", pv_name);