lvm2-commits August 2015

lvm2-commits@lists.fedorahosted.org

7 participants
580 discussions

dev-prajnoha-pv-ext-flag-used - dmstats: check for zero in _nr_areas_from_step() (Coverity)
by Peter Rajnoha 21 Aug '15

21 Aug '15

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=69fa16048a61b6cd… Commit: 69fa16048a61b6cd0b8fe9c311dd4e9eea532207 Parent: b01e9651b06a6362339d4c3d5d8da43896dae50f Author: Bryn M. Reeves <bmr(a)redhat.com> AuthorDate: Mon Aug 17 18:31:11 2015 +0100 Committer: Bryn M. Reeves <bmr(a)redhat.com> CommitterDate: Mon Aug 17 18:37:16 2015 +0100 dmstats: check for zero in _nr_areas_from_step() (Coverity) --- tools/dmsetup.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/tools/dmsetup.c b/tools/dmsetup.c index 8bcf4d5..7769fde 100644 --- a/tools/dmsetup.c +++ b/tools/dmsetup.c @@ -4434,6 +4434,10 @@ static int _size_from_string(char *argptr, uint64_t *size, const char *name) */ static uint64_t _nr_areas_from_step(uint64_t len, int64_t step) { + /* Default is one area. */ + if (!step || !len) + return 1; + /* --areas */ if (step < 0) return (uint64_t)(-step);

1 0

dev-prajnoha-pv-ext-flag-used - vgremove: fix locking when lvmlockd global lock is removed
by Peter Rajnoha 21 Aug '15

21 Aug '15

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=1aa7fa354e6ed3e4… Commit: 1aa7fa354e6ed3e468dc7966f5df0009627b2b97 Parent: 5383697c784de5af5438bb09ea8ebb57b4a47418 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Mon Aug 10 13:04:11 2015 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Mon Aug 10 13:04:11 2015 -0500 vgremove: fix locking when lvmlockd global lock is removed When vgremove is used to remove multiple VGs in one command, e.g. vgremove foo bar, the first VG (foo) that is removed may have held the sanlock global lock. In this case, do not continue removing further VGs (bar) without the global lock. --- lib/commands/toolcontext.h | 1 + lib/locking/lvmlockd.c | 28 +++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletions(-) diff --git a/lib/commands/toolcontext.h b/lib/commands/toolcontext.h index ec2242d..0440163 100644 --- a/lib/commands/toolcontext.h +++ b/lib/commands/toolcontext.h @@ -133,6 +133,7 @@ struct cmd_context { unsigned lockd_gl_disable:1; unsigned lockd_vg_disable:1; unsigned lockd_lv_disable:1; + unsigned lockd_gl_removed:1; unsigned lockd_vg_default_sh:1; unsigned lockd_vg_enforce_sh:1; diff --git a/lib/locking/lvmlockd.c b/lib/locking/lvmlockd.c index 7f14a86..5918de3 100644 --- a/lib/locking/lvmlockd.c +++ b/lib/locking/lvmlockd.c @@ -739,6 +739,19 @@ static int _free_vg_sanlock(struct cmd_context *cmd, struct volume_group *vg) if (!_lvmlockd_connected) return 0; + /* + * vgremove originally held the global lock, but lost it because the + * vgremove command is removing multiple VGs, and removed the VG + * holding the global lock before attempting to remove this VG. + * To avoid this situation, the user should remove the VG holding + * the global lock in a command by itself, or as the last arg in a + * vgremove command that removes multiple VGs. + */ + if (cmd->lockd_gl_removed) { + log_error("Global lock failed: global lock was lost by removing a previous VG."); + return 0; + } + if (!vg->lock_args || !strlen(vg->lock_args)) { /* Shouldn't happen in general, but maybe in some error cases? */ log_debug("_free_vg_sanlock %s no lock_args", vg->name); @@ -773,8 +786,21 @@ static int _free_vg_sanlock(struct cmd_context *cmd, struct volume_group *vg) goto out; } - if (lockd_flags & LD_RF_WARN_GL_REMOVED) + /* + * If the global lock was been removed by removing this VG, then: + * + * Print a warning indicating that the global lock should be enabled + * in another remaining sanlock VG. + * + * Do not allow any more VGs to be removed by this command, e.g. + * if a command removes two sanlock VGs, like vgremove foo bar, + * and the global lock existed in foo, do not continue to remove + * VG bar without the global lock. See the corresponding check above. + */ + if (lockd_flags & LD_RF_WARN_GL_REMOVED) { log_warn("VG %s held the sanlock global lock, enable global lock in another VG.", vg->name); + cmd->lockd_gl_removed = 1; + } /* * The usleep delay gives sanlock time to close the lock lv,

1 0

dev-prajnoha-pv-ext-flag-used - libdm: add dm_stats_get_{current_}area_offset()
by Peter Rajnoha 21 Aug '15

21 Aug '15

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=00ed52365916753d… Commit: 00ed52365916753d0bd6eb0de09c46ad782fec2e Parent: 77fae3d8522ffb1077dcb0e2e9ebfba86cf19daf Author: Bryn M. Reeves <bmr(a)redhat.com> AuthorDate: Fri Aug 14 20:30:05 2015 +0100 Committer: Bryn M. Reeves <bmr(a)redhat.com> CommitterDate: Fri Aug 14 22:03:37 2015 +0100 libdm: add dm_stats_get_{current_}area_offset() Add a method to retrieve the offset of an area within the containing region (rather than the offset within the containing device returned by dm_stats_get_area_start()). Although users of the library can calculate this themselves it is better to provide this through a method call to avoid users making assumptions about the structure of regions and areas. --- libdm/.exported_symbols.DM_1_02_105 | 2 ++ libdm/libdevmapper.h | 17 ++++++++++++++--- libdm/libdm-stats.c | 15 +++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/libdm/.exported_symbols.DM_1_02_105 b/libdm/.exported_symbols.DM_1_02_105 index 9b701f1..4707164 100644 --- a/libdm/.exported_symbols.DM_1_02_105 +++ b/libdm/.exported_symbols.DM_1_02_105 @@ -1 +1,3 @@ dm_report_is_empty +dm_stats_get_area_offset +dm_stats_get_current_area_offset diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h index 303ed67..bfc90da 100644 --- a/libdm/libdevmapper.h +++ b/libdm/libdevmapper.h @@ -705,19 +705,27 @@ uint64_t dm_stats_get_region_area_len(const struct dm_stats *dms, uint64_t *area_len, uint64_t region_id); /* - * Area properties: start and length. + * Area properties: start, offset and length. * * The area length is always equal to the area length of the region * that contains it and is obtained from dm_stats_get_region_area_len(). * - * The start offset of an area is a function of the area_id and the - * containing region's start and area length. + * The start of an area is a function of the area_id and the containing + * region's start and area length: it gives the absolute offset into the + * containing device of the beginning of the area. + * + * The offset expresses the area's relative offset into the current + * region. I.e. the area start minus the start offset of the containing + * region. * * All values are returned in units of 512b sectors. */ uint64_t dm_stats_get_area_start(const struct dm_stats *dms, uint64_t *start, uint64_t region_id, uint64_t area_id); +uint64_t dm_stats_get_area_offset(const struct dm_stats *dms, uint64_t *offset, + uint64_t region_id, uint64_t area_id); + /* * Retrieve program_id and aux_data for a specific region. Only valid * following a call to dm_stats_list(). The returned pointer does not @@ -876,6 +884,9 @@ uint64_t dm_stats_get_current_region_area_len(const struct dm_stats *dms, uint64_t dm_stats_get_current_area_start(const struct dm_stats *dms, uint64_t *start); +uint64_t dm_stats_get_current_area_offset(const struct dm_stats *dms, + uint64_t *offset); + uint64_t dm_stats_get_current_area_len(const struct dm_stats *dms, uint64_t *start); diff --git a/libdm/libdm-stats.c b/libdm/libdm-stats.c index 51aba79..400b12d 100644 --- a/libdm/libdm-stats.c +++ b/libdm/libdm-stats.c @@ -1335,6 +1335,14 @@ uint64_t dm_stats_get_area_start(const struct dm_stats *dms, uint64_t *start, return 1; } +uint64_t dm_stats_get_area_offset(const struct dm_stats *dms, uint64_t *offset, + uint64_t region_id, uint64_t area_id) +{ + if (!dms || !dms->regions) + return_0; + *offset = dms->regions[region_id].step * area_id; +} + uint64_t dm_stats_get_current_area_start(const struct dm_stats *dms, uint64_t *start) { @@ -1342,6 +1350,13 @@ uint64_t dm_stats_get_current_area_start(const struct dm_stats *dms, dms->cur_region, dms->cur_area); } +uint64_t dm_stats_get_current_area_offset(const struct dm_stats *dms, + uint64_t *offset) +{ + return dm_stats_get_area_offset(dms, offset, + dms->cur_region, dms->cur_area); +} + uint64_t dm_stats_get_current_area_len(const struct dm_stats *dms, uint64_t *len) {

1 0

dev-prajnoha-pv-ext-flag-used - dmsetup: use timerfd for interval timing if available
by Peter Rajnoha 21 Aug '15

21 Aug '15

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=a8b9e2eccdfccd63… Commit: a8b9e2eccdfccd63bc8816cea66f157b0257e351 Parent: 99f55abc56bf1ce6b6f0e7d9cae67e347ace16b2 Author: Bryn M. Reeves <bmr(a)redhat.com> AuthorDate: Sat Aug 8 23:59:06 2015 +0100 Committer: Bryn M. Reeves <bmr(a)redhat.com> CommitterDate: Wed Aug 12 15:09:57 2015 +0100 dmsetup: use timerfd for interval timing if available If the Linux timerfd interface to POSIX timers is available at compile time use it for all report interval timekeeping. This gives more accurate interval timing when the per-interval processing time is less than the configured interval and simplifies the timestamp bookkeeping required to keep accurate time. For systems without timerfd support fall back to the simple usleep based timer. --- configure | 2 +- configure.in | 2 +- lib/misc/configure.h.in | 3 + tools/dmsetup.c | 325 +++++++++++++++++++++++++++++++++++++++++------ 4 files changed, 290 insertions(+), 42 deletions(-) diff --git a/configure b/configure index b17e5a3..630f12a 100755 --- a/configure +++ b/configure @@ -5734,7 +5734,7 @@ fi done -for ac_header in termios.h sys/statvfs.h +for ac_header in termios.h sys/statvfs.h sys/timerfd.h do : as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh` ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default" diff --git a/configure.in b/configure.in index 243b634..c3e55e9 100644 --- a/configure.in +++ b/configure.in @@ -103,7 +103,7 @@ AC_CHECK_HEADERS([assert.h ctype.h dirent.h errno.h fcntl.h float.h \ sys/time.h sys/types.h sys/utsname.h sys/wait.h time.h \ unistd.h], , [AC_MSG_ERROR(bailing out)]) -AC_CHECK_HEADERS(termios.h sys/statvfs.h) +AC_CHECK_HEADERS(termios.h sys/statvfs.h sys/timerfd.h) case "$host_os" in linux*) diff --git a/lib/misc/configure.h.in b/lib/misc/configure.h.in index 750c75c..0adb7b1 100644 --- a/lib/misc/configure.h.in +++ b/lib/misc/configure.h.in @@ -516,6 +516,9 @@ /* Define to 1 if you have the <sys/time.h> header file. */ #undef HAVE_SYS_TIME_H +/* Define to 1 if you have the <sys/timerfd.h> header file. */ +#undef HAVE_SYS_TIMERFD_H + /* Define to 1 if you have the <sys/types.h> header file. */ #undef HAVE_SYS_TYPES_H diff --git a/tools/dmsetup.c b/tools/dmsetup.c index ed3cbd1..9876bd2 100644 --- a/tools/dmsetup.c +++ b/tools/dmsetup.c @@ -15,7 +15,6 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ - #include "tool.h" #include "dm-logging.h" @@ -49,6 +48,10 @@ # include <sys/ioctl.h> #endif +#if HAVE_SYS_TIMERFD_H +# include <sys/timerfd.h> +#endif + #if HAVE_TERMIOS_H # include <termios.h> #endif @@ -208,9 +211,15 @@ static uint64_t _disp_factor = 512; /* display sizes in sectors */ static char _disp_units = 's'; /* report timekeeping */ -static struct dm_timestamp *_ts_start = NULL, *_ts_end = NULL; -static uint64_t _last_interval = 0; /* approx. measured interval in nsecs */ +static struct dm_timestamp *_cycle_timestamp = NULL; static uint64_t _interval = 0; /* configured interval in nsecs */ +static uint64_t _new_interval = 0; /* flag top-of-interval */ +static uint64_t _last_interval = 0; /* approx. measured interval in nsecs */ +static double _mean_interval = 0.0; /* mean sample interval. */ +static int _timer_fd = -1; /* timerfd file descriptor. */ + +/* Invalid fd value used to signal end-of-reporting. */ +#define TIMER_STOPPED -2 #define NSEC_PER_USEC UINT64_C(1000) #define NSEC_PER_MSEC UINT64_C(1000000) @@ -456,9 +465,266 @@ static void _destroy_split_name(struct dm_split_name *split_name) dm_free(split_name); } +/* + * Stats clock: + * + * Use either Linux timerfds or usleep to implement the reporting + * interval wait. + * + * _start_timer() - Start the timer running. + * _do_timer_wait() - Wait until the beginning of the next interval. + * + * _update_interval_times() - Update timestamps and interval estimate. + */ + +#if HAVE_SYS_TIMERFD_H +static int _start_timerfd_timer(void) +{ + struct itimerspec interval_timer; + time_t secs; + long nsecs; + + log_debug("Using timerfd for interval timekeeping."); + + /* timer running? */ + if (_timer_fd != -1) + return 1; + + memset(&interval_timer, 0, sizeof(interval_timer)); + + /* Use CLOCK_MONOTONIC to avoid warp on RTC adjustments. */ + if ((_timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC)) < 0) { + log_error("Could not create timer: %s", strerror(errno)); + return 0; + } + + secs = (time_t) _interval / NSEC_PER_SEC; + nsecs = (long) _interval % NSEC_PER_SEC; + + /* Must set interval and value to create an armed periodic timer. */ + interval_timer.it_interval.tv_sec = secs; + interval_timer.it_interval.tv_nsec = nsecs; + interval_timer.it_value.tv_sec = secs; + interval_timer.it_value.tv_nsec = nsecs; + + log_debug("Setting interval timer to: "FMTu64"s "FMTu64"ns", secs, nsecs); + if (timerfd_settime(_timer_fd, 0, &interval_timer, NULL)) { + log_error("Could not set interval timer: %s", strerror(errno)); + return 0; + } + return 1; +} + +static int _do_timerfd_wait(void) +{ + uint64_t expired; + + if (_timer_fd < 0) + return 0; + + /* read on timerfd returns a uint64_t in host byte order. */ + if (read(_timer_fd, &expired, sizeof(expired)) < 0) { + /* EBADF from invalid timerfd or EINVAL from too small buffer. */ + log_error("Interval timer wait failed: %s", + strerror(errno)); + return 0; + } + + /* FIXME: attempt to rebase clock? */ + if (expired > 1) + log_warn("WARNING: Try increasing --interval ("FMTu64 + " missed timer events).", expired - 1); + + /* Signal that a new interval has begun. */ + _new_interval = 1; + + /* Final interval? */ + if (_count == 2) { + if (close(_timer_fd)) + stack; + /* Tell _update_interval_times() to shut down. */ + _timer_fd = TIMER_STOPPED; + } + + return 1; +} + +static int _start_timer(void) +{ + return _start_timerfd_timer(); +} + +static int _do_timer_wait(void) +{ + return _do_timerfd_wait(); +} + +#else /* !HAVE_SYS_TIMERFD_H */ +static int _start_usleep_timer(void) +{ + log_debug("Using usleep for interval timekeeping."); + return 1; +} + +static int _do_usleep_wait(void) +{ + static struct dm_timestamp *_last_sleep, *_now = NULL; + uint64_t this_interval; + int64_t delta_t; + + /* + * Report clock: compensate for time spent in userspace and stats + * message ioctls by keeping track of the last wake time and + * adjusting the sleep interval accordingly. + */ + if (!_last_sleep && !_now) { + if (!(_last_sleep = dm_timestamp_alloc())) + goto_out; + if (!(_now = dm_timestamp_alloc())) + goto_out; + dm_timestamp_get(_now); + this_interval = _interval; + log_error("Using "FMTu64" as first interval.", this_interval); + } else { + dm_timestamp_get(_now); + delta_t = dm_timestamp_delta(_now, _last_sleep); + log_debug("Interval timer delta_t: "FMTi64, delta_t); + + /* FIXME: usleep timer drift over large counts. */ + + /* adjust for time spent populating and reporting */ + this_interval = 2 * _interval - delta_t; + log_debug("Using "FMTu64" as interval.", this_interval); + } + + /* Signal that a new interval has begun. */ + _new_interval = 1; + dm_timestamp_copy(_last_sleep, _now); + + if (usleep(this_interval / NSEC_PER_USEC)) { + if (errno == EINTR) + log_error("Report interval interrupted by signal."); + if (errno == EINVAL) + log_error("Report interval too short."); + goto out; + } + + if(_count == 2) { + dm_timestamp_destroy(_last_sleep); + dm_timestamp_destroy(_now); + } + + return 1; +out: + return 0; +} + +static int _start_timer(void) +{ + return _start_usleep_timer(); +} + +static int _do_timer_wait(void) +{ + return _do_usleep_wait(); +} + +#endif /* HAVE_SYS_TIMERFD_H */ + +static int _update_interval_times(void) +{ + static struct dm_timestamp *this_timestamp = NULL; + uint64_t delta_t; + int r = 0; + + /* + * Current timestamp. If _new_interval is set this is used as + * the new cycle start timestamp. + */ + if (!this_timestamp) { + if (!(this_timestamp = dm_timestamp_alloc())) + return_0; + } + + /* + * Take cycle timstamp as close as possible to ioctl return. + * + * FIXME: use per-region timestamp deltas for interval estimate. + */ + if (!dm_timestamp_get(this_timestamp)) + goto_out; + + /* + * Stats clock: maintain a single timestamp taken just after the + * call to dm_stats_populate() and take a delta between the current + * and last value to determine the sampling interval. + * + * A new interval is started when the _new_interval flag is set + * on return from _do_report_wait(). + * + * The first interval is treated as a special case: since the + * time since the last clear of the counters is unknown (no + * previous timestamp exists) the duration is assumed to be the + * configured value. + */ + if (_cycle_timestamp) + /* Current delta_t: time from start of cycle to now. */ + delta_t = dm_timestamp_delta(this_timestamp, _cycle_timestamp); + else { + _cycle_timestamp = dm_timestamp_alloc(); + if (!_cycle_timestamp) { + log_error("Could not allocate timestamp object."); + goto out; + } + + /* Pretend we have the configured interval. */ + delta_t = _interval; + _mean_interval = (double) delta_t; + + /* start the first cycle */ + log_debug("Beginning first interval"); + _new_interval = 1; + } + + log_debug("Checking for interval end, dt="FMTu64, delta_t); + if (_new_interval) { + /* Update timestamp and interval and clear _new_interval */ + dm_timestamp_copy(_cycle_timestamp, this_timestamp); + _last_interval = delta_t; + _new_interval = 0; + + /* Track mean interval estimate. */ + _mean_interval = ((double) delta_t + _mean_interval) / 2.0; + + /* + * Log interval duration, mean interval, the last interval + * error and the difference between the mean and configured + * intervals. + */ + log_debug("Ending interval, I="FMTu64" Imean=%f, Ierr="FMTi64 + ", Emean=%.2f", _last_interval, _mean_interval, + ((int64_t)_last_interval - (int64_t)_interval), + _mean_interval - (double) _interval); + } + + r = 1; + +out: + if (!r || _timer_fd == TIMER_STOPPED) { + /* The _cycle_timestamp has not yet been allocated if we + * fail to obtain this_timestamp on the first interval. + */ + if (_cycle_timestamp) + dm_timestamp_destroy(_cycle_timestamp); + dm_timestamp_destroy(this_timestamp); + } + return r; +} + static int _display_info_cols(struct dm_task *dmt, struct dm_info *info) { struct dmsetup_report_obj obj; + int r = 0; if (!info->exists) { @@ -497,13 +763,17 @@ static int _display_info_cols(struct dm_task *dmt, struct dm_info *info) if (!(obj.stats = dm_stats_create(DM_STATS_PROGRAM_ID))) goto_out; - /* FIXME: use a single timestamp to measure _last_interval. */ - dm_stats_set_sampling_interval_ns(obj.stats, _last_interval); - dm_stats_bind_devno(obj.stats, info->major, info->minor); - if (!(dm_stats_populate(obj.stats, DM_STATS_PROGRAM_ID, DM_STATS_REGIONS_ALL))) { + + if (!(dm_stats_populate(obj.stats, DM_STATS_PROGRAM_ID, DM_STATS_REGIONS_ALL))) goto out; - } + + /* Update timestamps and handle end-of-interval accounting. */ + _update_interval_times(); + + log_debug("Setting interval, I="FMTu64, _last_interval); + /* use measured approximation for calculations */ + dm_stats_set_sampling_interval_ns(obj.stats, _last_interval); } /* @@ -5416,26 +5686,7 @@ static int _perform_command_for_all_repeatable_args(CMD_ARGS) static int _do_report_wait(void) { - if (!dm_timestamp_get(_ts_start)) - goto_out; - - /* FIXME: compensate for interval drift from time spent reporting. */ - if (usleep((useconds_t) (_interval / NSEC_PER_USEC))) { - if (errno == EINTR) - log_error("Report interval interrupted by signal."); - if (errno == EINVAL) - log_error("Report interval too short."); - goto out; - } - - if (!dm_timestamp_get(_ts_end)) - goto_out; - - _last_interval = dm_timestamp_delta(_ts_end, _ts_start); - - return 1; -out: - return 0; + return _do_timer_wait(); } int main(int argc, char **argv) @@ -5553,14 +5804,11 @@ unknown: argc--, argv++; } - _ts_start = dm_timestamp_alloc(); - _ts_end = dm_timestamp_alloc(); - if (!_ts_start || !_ts_end) { - log_error("Could not allocate timestamp objects."); - goto out; - } - /* Pretend we have the configured interval for the first iteration. */ - _last_interval = _interval; + /* Start interval timer. */ + if (_count > 1) + if (!_start_timer()) + goto_out; + doit: multiple_devices = (cmd->repeatable_cmd && argc != 2 && (argc != 1 || (!_switches[UUID_ARG] && !_switches[MAJOR_ARG]))); @@ -5581,19 +5829,16 @@ doit: goto_out; } } - } while (--_count); out: + if (_report) dm_report_free(_report); if (_dtree) dm_tree_free(_dtree); - dm_timestamp_destroy(_ts_start); - dm_timestamp_destroy(_ts_end); - dm_free(_table); if (_initial_timestamp)

1 0

dev-prajnoha-pv-ext-flag-used - dmstats: fix new area count for 'create --areasize'
by Peter Rajnoha 21 Aug '15

21 Aug '15

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=9d5cd4ca14a94d83… Commit: 9d5cd4ca14a94d83bf7b3c0b73896abc4ff8926c Parent: 0b487802a041a0848e927e3dfc3d71618b66ca42 Author: Bryn M. Reeves <bmr(a)redhat.com> AuthorDate: Sat Aug 15 00:32:59 2015 +0100 Committer: Bryn M. Reeves <bmr(a)redhat.com> CommitterDate: Sat Aug 15 00:42:51 2015 +0100 dmstats: fix new area count for 'create --areasize' Commit f10ad95 introduced a regression in the calculation of the number of areas in a region created with the --areasize switch: vg_hex-lv_home: Created new region with 0 area(s) as region ID 1 vg_hex-lv_swap: Created new region with 0 area(s) as region ID 1 Fis this by using the correct region size when calculating the value. --- tools/dmsetup.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/dmsetup.c b/tools/dmsetup.c index 9e72226..ee1e1fe 100644 --- a/tools/dmsetup.c +++ b/tools/dmsetup.c @@ -4456,7 +4456,7 @@ static int _do_stats_create_regions(struct dm_stats *dms, const char *program_id, const char *aux_data) { - uint64_t this_start, this_len, region_id = UINT64_C(0); + uint64_t this_start = 0, this_len = 0, region_id = UINT64_C(0); char *target_type, *params; /* unused */ struct dm_task *dmt; struct dm_info info; @@ -4506,7 +4506,7 @@ static int _do_stats_create_regions(struct dm_stats *dms, * whole-device region). */ this_start = (segments) ? segment_start : start; - this_len = (segments) ? segment_len : len; + this_len = (segments) ? segment_len : this_len; if (!dm_stats_create_region(dms, &region_id, this_start, this_len, step, program_id, aux_data)) {

1 0

dev-prajnoha-pv-ext-flag-used - dmsetup: make sure subcommand is initialised (Coverity)
by Peter Rajnoha 21 Aug '15

21 Aug '15

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=b01e9651b06a6362… Commit: b01e9651b06a6362339d4c3d5d8da43896dae50f Parent: 8967776713e1be2d584bc8a9713eaaaf467d8ac7 Author: Bryn M. Reeves <bmr(a)redhat.com> AuthorDate: Mon Aug 17 18:28:32 2015 +0100 Committer: Bryn M. Reeves <bmr(a)redhat.com> CommitterDate: Mon Aug 17 18:37:16 2015 +0100 dmsetup: make sure subcommand is initialised (Coverity) --- tools/dmsetup.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tools/dmsetup.c b/tools/dmsetup.c index dc002b0..8bcf4d5 100644 --- a/tools/dmsetup.c +++ b/tools/dmsetup.c @@ -5964,7 +5964,8 @@ unknown: if (cmd->has_subcommands) { subcommand = argv[0]; argc--, argv++; - } + } else + subcommand = (char *) ""; if (_switches[COLS_ARG] && !_report_init(cmd, subcommand)) goto out;

1 0

dev-prajnoha-pv-ext-flag-used - libdm: check for zero in _nr_areas() (Coverity)
by Peter Rajnoha 21 Aug '15

21 Aug '15

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=074b5de7715b76ad… Commit: 074b5de7715b76ad63147f84edb5d9e1a05870e9 Parent: 69fa16048a61b6cd0b8fe9c311dd4e9eea532207 Author: Bryn M. Reeves <bmr(a)redhat.com> AuthorDate: Mon Aug 17 18:33:22 2015 +0100 Committer: Bryn M. Reeves <bmr(a)redhat.com> CommitterDate: Mon Aug 17 18:37:16 2015 +0100 libdm: check for zero in _nr_areas() (Coverity) --- libdm/libdm-stats.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/libdm/libdm-stats.c b/libdm/libdm-stats.c index b7000cc..ba213b7 100644 --- a/libdm/libdm-stats.c +++ b/libdm/libdm-stats.c @@ -504,6 +504,9 @@ out: static uint64_t _nr_areas(uint64_t len, uint64_t step) { + /* Default is one area. */ + if (!len || !step) + return 1; /* * drivers/md/dm-stats.c::message_stats_create() * A region may be sub-divided into areas with their own counters.

1 0

dev-prajnoha-pv-ext-flag-used - tests: add pv-ext-flags.sh test
by Peter Rajnoha 21 Aug '15

21 Aug '15

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=8056a6eaa7d3df9e… Commit: 8056a6eaa7d3df9eeb51d0a5a06ac4c9417e90c8 Parent: 5ff2a9ad73b0fafb4f507d64a8992edd0e41cd13 Author: Peter Rajnoha <prajnoha(a)redhat.com> AuthorDate: Tue Apr 7 13:50:54 2015 +0200 Committer: Peter Rajnoha <prajnoha(a)redhat.com> CommitterDate: Fri Aug 21 11:15:53 2015 +0200 tests: add pv-ext-flags.sh test Testing PV extension flags. Currently, there's only one PV extension flag present - the PV_EXT_USED flag (marking PV as used in a VG). --- test/shell/pv-ext-flags.sh | 160 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 160 insertions(+), 0 deletions(-) diff --git a/test/shell/pv-ext-flags.sh b/test/shell/pv-ext-flags.sh new file mode 100644 index 0000000..8d653df --- /dev/null +++ b/test/shell/pv-ext-flags.sh @@ -0,0 +1,160 @@ +#!/bin/sh +# Copyright (C) 2015 Red Hat, Inc. All rights reserved. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions +# of the GNU General Public License v.2. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software Foundation, +# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +. lib/inittest + +aux prepare_devs 2 + +# PV_EXT_USED flag +MARKED_AS_USED_MSG="marked as used" + +###################################### +### CHECK PV WITH 0 METADATA AREAS ### +###################################### + +pvcreate -ff -y --metadatacopies 0 "$dev1" +pvcreate -ff -y --metadatacopies 1 "$dev2" + +# $dev1 and $dev2 not in any VG - pv_in_use field should be blank +check pv_field "$dev1" pv_in_use "" +check pv_field "$dev2" pv_in_use "" + +# $dev1 and $dev now in a VG - pv_in_use should display "used" +vgcreate $vg1 "$dev1" "$dev2" +check pv_field "$dev1" pv_in_use "used" +check pv_field "$dev2" pv_in_use "used" + +# disable $dev2 and dev1 with 0 MDAs remains, but still +# marked as used, so pvcreate/vgcreate/pvremove should fail +aux disable_dev "$dev2" +pvscan --cache + +check pv_field "$dev1" pv_in_use "used" +not pvcreate "$dev1" 2>err +grep "$MARKED_AS_USED_MSG" err +not pvchange -u "$dev1" 2>err +grep "$MARKED_AS_USED_MSG" err +not vgcreate $vg2 "$dev1" 2>err +grep "$MARKED_AS_USED" err +not pvremove "$dev1" 2>err +grep "$MARKED_AS_USED_MSG" err + +# save PV signature from dev1 for reuse later on in this +# test so we don't need to initialize all the VG stuff again +dd if="$dev1" of=dev1_backup bs=1M + +# pvcreate and pvremove can be forced even if the PV is marked as used +pvremove -ff "$dev1" +dd if=dev1_backup of="$dev1" bs=1M +pvcreate -ff "$dev1" +dd if=dev1_backup of="$dev1" bs=1M + +# prepare a VG with $dev1 and $dev both having 1 MDA +aux enable_dev "$dev2" +vgremove -ff $vg1 +pvcreate --metadatacopies 1 "$dev1" +vgcreate $vg1 "$dev1" "$dev2" + +# disable $dev1, then repair the VG - $dev1 is removed from VG +aux disable_dev "$dev1" +vgreduce --removemissing $vg1 +# now, enable $dev1, automatic repair will happen on pvs call +# (or any other lvm command that does vg_read with repair inside) +aux enable_dev "$dev1" + +# FIXME: once persistent cache does not cause races with timestamps +# causing LVM tools to not see the VG inconsistency and once +# VG repair is always done, delete this line which removes +# persistent .cache as a workaround +rm -f $TESTDIR/etc/.cache + +vgck $vg1 +# check $dev1 does not contain the PV_EXT_FLAG anymore - it +# should be removed as part of the repaid during vg_read since +# $dev1 is not part of $vg1 anymore +check pv_field "$dev1" pv_in_use "" + +############################################# +### CHECK PV WITH DISABLED METADATA AREAS ### +############################################# + +pvcreate -ff -y --metadatacopies 1 "$dev1" +pvcreate -ff -y --metadatacopies 1 "$dev2" + +# $dev1 and $dev2 not in any VG - pv_in_use field should be blank +check pv_field "$dev1" pv_in_use "" +check pv_field "$dev2" pv_in_use "" + +# $dev1 and $dev now in a VG - pv_in_use should display "used" +vgcreate $vg1 "$dev1" "$dev2" +check pv_field "$dev1" pv_in_use "used" +check pv_field "$dev2" pv_in_use "used" + +pvchange --metadataignore y "$dev1" +aux disable_dev "$dev2" +pvscan --cache + +check pv_field "$dev1" pv_in_use "used" +not pvcreate "$dev1" 2>err +grep "$MARKED_AS_USED_MSG" err +not pvchange -u "$dev1" 2>err +grep "$MARKED_AS_USED_MSG" err +not vgcreate $vg2 "$dev1" 2>err +grep "$MARKED_AS_USED" err +not pvremove "$dev1" 2>err +grep "$MARKED_AS_USED_MSG" err + +# save PV signature from dev1 for reuse later on in this +# test so we don't need to initialize all the VG stuff again +dd if="$dev1" of=dev1_backup bs=1M + +# pvcreate and pvremove can be forced even if the PV is marked as used +pvremove -ff "$dev1" +dd if=dev1_backup of="$dev1" bs=1M +pvcreate -ff "$dev1" +dd if=dev1_backup of="$dev1" bs=1M + +# prepare a VG with $dev1 and $dev both having 1 MDA +aux enable_dev "$dev2" +vgremove -ff $vg1 +pvcreate --metadatacopies 1 "$dev1" +vgcreate $vg1 "$dev1" "$dev2" + +# disable $dev1, then repair the VG - $dev1 is removed from VG +aux disable_dev "$dev1" +vgreduce --removemissing $vg1 +# now, enable $dev1, automatic repair will happen on pvs call +# (or any other lvm command that does vg_read with repair inside) +aux enable_dev "$dev1" + +# FIXME: once persistent cache does not cause races with timestamps +# causing LVM tools to not see the VG inconsistency and once +# VG repair is always done, delete this line which removes +# persistent .cache as a workaround +rm -f $TESTDIR/etc/.cache + +vgck $vg1 +# check $dev1 does not contain the PV_EXT_FLAG anymore - it +# should be removed as part of the repaid during vg_read since +# $dev1 is not part of $vg1 anymore +check pv_field "$dev1" pv_in_use "" + +########################### +# OTHER PV-RELATED CHECKS # +########################### + +# vgcfgrestore should also set PV_EXT_FLAG on PVs where VG is restored +vgcfgbackup -f vg_backup $vg1 +check pv_field "$dev2" pv_in_use "used" +vgremove -ff $vg1 +check pv_field "$dev2" pv_in_use "" +vgcfgrestore -f vg_backup $vg1 +check pv_field "$dev2" pv_in_use "used"

1 0

dev-prajnoha-pv-ext-flag-used - backup: backup_restore_vg: register PVs that need writing via vg->pvs_to_write list
by Peter Rajnoha 21 Aug '15

21 Aug '15

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=5ff2a9ad73b0fafb… Commit: 5ff2a9ad73b0fafb4f507d64a8992edd0e41cd13 Parent: 921b994187f9c5e0dca2a6963f090a3aa9ae9d09 Author: Peter Rajnoha <prajnoha(a)redhat.com> AuthorDate: Thu Mar 26 14:20:46 2015 +0100 Committer: Peter Rajnoha <prajnoha(a)redhat.com> CommitterDate: Fri Aug 21 11:15:53 2015 +0200 backup: backup_restore_vg: register PVs that need writing via vg->pvs_to_write list The backup_restore_vg is used directly for restoring the VG from backup. It's also used to do the VG conversions from one metadata format to another which means vgconvert calls backup_restore_vg too. When restoring VG from backup, we need to rewrite/write PV headers as PVs may have been orphans before and now they're becoming part of some VG - we need to write the PV_EXT_USED flag at least. When using the backup_restore_vg for vgconvert, we need to write completely new PV header in different format. Avoid the special "pv_write" call and handling that was used before this patch in vgconvert (vgconvert_single function to be more precise) and reuse existing internal interface to register PV header for writing (or rewriting) via vg->pvs_to_write list instead like we do it elsewhere in the code. This patch also resolves a problem in which PV headers with target format were written in the vgconvert_single fn as orphans and VG metadata were added later on - this was a tiny hack actually. We can't do this now - we need to write the PV as belonging to a VG because otherwise the PV_EXT_USED flag won't be written properly (if the PV header is written as orphan, the PV_EXT_USED is set to 0, of course, even though metadata are attached later). So this patch removes this tiny inconsistency which was passing just fine before because we didn't have any relation to the VG in PV header before. Now we have the PV_EXT_USED flag which says the "PV is used in some VG". --- lib/format_text/archiver.c | 126 ++++++++++++++++++++++++++++++++++++++------ lib/format_text/archiver.h | 3 +- tools/vgconvert.c | 89 +++++-------------------------- 3 files changed, 125 insertions(+), 93 deletions(-) diff --git a/lib/format_text/archiver.c b/lib/format_text/archiver.c index fa7ce88..5d34f3b 100644 --- a/lib/format_text/archiver.c +++ b/lib/format_text/archiver.c @@ -319,28 +319,110 @@ struct volume_group *backup_read_vg(struct cmd_context *cmd, return vg; } +static int _restore_vg_should_write_pv(struct physical_volume *pv, struct pvcreate_params *pp) +{ + struct lvmcache_info *info; + + if (pp) + return 1; + + if (!(info = lvmcache_info_from_pvid(pv->dev->pvid, 0))) { + log_error("Failed to find cached info for PV %s.", pv_dev_name(pv)); + return -1; + } + + /* + * We're restoring a VG and if the PV_EXT_USED + * flag is not set yet in PV, we need to set it now! + * This may happen if we have plain PVs without a VG + * and we're restoring former VG from backup on top + * of these PVs. + */ + if (!(lvmcache_ext_flags(info) & PV_EXT_USED)) + return 1; + + return 0; +} + /* ORPHAN and VG locks held before calling this */ -int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, int drop_lvmetad) +int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, + struct pvcreate_params *pp, int drop_lvmetad) { - struct pv_list *pvl; + struct dm_list new_pvs; + struct pv_list *pvl, *new_pvl; + struct physical_volume *existing_pv, *pv; + struct dm_list *pvs = &vg->pvs;; struct format_instance *fid; struct format_instance_ctx fic; + int should_write_pv; struct pv_to_write *pvw; - uint32_t tmp; + uint32_t tmp_extent_size; /* * FIXME: Check that the PVs referenced in the backup are * not members of other existing VGs. */ + /* Prepare new PVs if needed. */ + if (pp) { + dm_list_init(&new_pvs); + + dm_list_iterate_items(pvl, &vg->pvs) { + existing_pv = pvl->pv; + + pp->rp.id = existing_pv->id; + pp->rp.idp = &pp->rp.id; + pp->rp.pe_start = pv_pe_start(existing_pv); + pp->rp.extent_count = pv_pe_count(existing_pv); + pp->rp.extent_size = pv_pe_size(existing_pv); + /* pe_end = pv_pe_count(existing_pv) * pv_pe_size(existing_pv) + pe_start - 1 */ + + if (!(pv = pv_create(cmd, pv_dev(existing_pv), + 0, 0, 0, + pp->labelsector, + pp->pvmetadatacopies, + pp->pvmetadatasize, + 0, &pp->rp))) { + log_error("Failed to setup physical volume \"%s\".", + pv_dev_name(existing_pv)); + return 0; + } + pv->vg_name = vg->name; + pv->vgid = vg->id; + + if (!(new_pvl = dm_pool_zalloc(vg->vgmem, sizeof(*new_pvl)))) { + log_error("Failed to allocate PV list item for \"%s\".", + pv_dev_name(pvl->pv)); + return 0; + } + new_pvl->pv = pv; + dm_list_add(&new_pvs, &new_pvl->list); + + log_verbose("Set up physical volume for \"%s\" with %" PRIu64 + " available sectors.", pv_dev_name(pv), pv_size(pv)); + } + + pvs = &new_pvs; + } + /* Attempt to write out using currently active format */ fic.type = FMT_INSTANCE_AUX_MDAS; fic.context.vg_ref.vg_name = vg->name; fic.context.vg_ref.vg_id = NULL; if (!(fid = cmd->fmt->ops->create_instance(cmd->fmt, &fic))) { - log_error("Failed to allocate format instance"); + log_error("Failed to allocate format instance."); return 0; } + + if (pp) { + log_verbose("Deleting existing metadata for VG %s.", vg->name); + if (!vg_remove_mdas(vg)) { + cmd->fmt->ops->destroy_instance(fid); + log_error("Removal of existing metadata for VG %s failed.", vg->name); + return 0; + } + } + vg_set_fid(vg, fid); /* @@ -352,24 +434,36 @@ int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, int drop return 0; } - /* Add any metadata areas on the PVs */ - dm_list_iterate_items(pvl, &vg->pvs) { - if (!(pvw = dm_pool_zalloc(vg->vgmem, sizeof(*pvw)))) { - log_error("pv_to_write allocation for '%s' failed", pv_dev_name(pvl->pv)); - return 0; + dm_list_iterate_items(pvl, pvs) { + if ((should_write_pv = _restore_vg_should_write_pv(pvl->pv, pp)) < 0) + return_0; + + if (should_write_pv) { + if (!(pvw = dm_pool_zalloc(vg->vgmem, sizeof(*pvw)))) { + log_error("Failed to allocate structure for scheduled " + "writing of PV '%s'.", pv_dev_name(pvl->pv)); + return 0; + } + + if (pp) { + pvw->new_pv = 1; + pvw->pp = pp; + } + + pvw->pv = pvl->pv; + dm_list_add(&vg->pvs_to_write, &pvw->list); } - pvw->pv = pvl->pv; - dm_list_add(&vg->pvs_to_write, &pvw->list); - tmp = vg->extent_size; + /* Add any metadata areas on the PV. */ + tmp_extent_size = vg->extent_size; vg->extent_size = 0; if (!vg->fid->fmt->ops->pv_setup(vg->fid->fmt, pvl->pv, vg)) { - vg->extent_size = tmp; - log_error("Format-specific setup for %s failed", + vg->extent_size = tmp_extent_size; + log_error("Format-specific setup for %s failed.", pv_dev_name(pvl->pv)); return 0; } - vg->extent_size = tmp; + vg->extent_size = tmp_extent_size; } if (!vg_write(vg)) @@ -423,7 +517,7 @@ int backup_restore_from_file(struct cmd_context *cmd, const char *vg_name, missing_pvs = vg_missing_pv_count(vg); if (missing_pvs == 0) - r = backup_restore_vg(cmd, vg, 1); + r = backup_restore_vg(cmd, vg, NULL, 1); else log_error("Cannot restore Volume Group %s with %i PVs " "marked as missing.", vg->name, missing_pvs); diff --git a/lib/format_text/archiver.h b/lib/format_text/archiver.h index bc756d5..a24721e 100644 --- a/lib/format_text/archiver.h +++ b/lib/format_text/archiver.h @@ -51,7 +51,8 @@ int backup_remove(struct cmd_context *cmd, const char *vg_name); struct volume_group *backup_read_vg(struct cmd_context *cmd, const char *vg_name, const char *file); -int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, int drop_lvmetad); +int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, + struct pvcreate_params *pp, int drop_lvmetad); int backup_restore_from_file(struct cmd_context *cmd, const char *vg_name, const char *file, int force); int backup_restore(struct cmd_context *cmd, const char *vg_name, int force); diff --git a/tools/vgconvert.c b/tools/vgconvert.c index c4bb37a..f34db28 100644 --- a/tools/vgconvert.c +++ b/tools/vgconvert.c @@ -19,14 +19,9 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name, struct volume_group *vg, struct processing_handle *handle __attribute__((unused))) { - struct physical_volume *pv, *existing_pv; - struct pvcreate_restorable_params rp; + struct pvcreate_params pp; struct logical_volume *lv; struct lv_list *lvl; - int pvmetadatacopies = 0; - uint64_t pvmetadatasize = 0; - struct pv_list *pvl; - int change_made = 0; struct lvinfo info; int active = 0; @@ -39,21 +34,22 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name, return ECMD_FAILED; } + pvcreate_params_set_defaults(&pp); + if (cmd->fmt->features & FMT_MDAS) { if (arg_sign_value(cmd, metadatasize_ARG, SIGN_NONE) == SIGN_MINUS) { log_error("Metadata size may not be negative"); return EINVALID_CMD_LINE; } - pvmetadatasize = arg_uint64_value(cmd, metadatasize_ARG, - UINT64_C(0)); - if (!pvmetadatasize) - pvmetadatasize = + pp.pvmetadatasize = arg_uint64_value(cmd, metadatasize_ARG, UINT64_C(0)); + if (!pp.pvmetadatasize) + pp.pvmetadatasize = find_config_tree_int(cmd, metadata_pvmetadatasize_CFG, NULL); - pvmetadatacopies = arg_int_value(cmd, pvmetadatacopies_ARG, -1); - if (pvmetadatacopies < 0) - pvmetadatacopies = + pp.pvmetadatacopies = arg_int_value(cmd, pvmetadatacopies_ARG, -1); + if (pp.pvmetadatacopies < 0) + pp.pvmetadatacopies = find_config_tree_int(cmd, metadata_pvmetadatacopies_CFG, NULL); } @@ -63,9 +59,11 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name, return EINVALID_CMD_LINE; } - rp.ba_size = arg_uint64_value(cmd, bootloaderareasize_ARG, UINT64_C(0)); + pp.rp.ba_size = arg_uint64_value(cmd, bootloaderareasize_ARG, UINT64_C(0)); } + pp.labelsector = arg_int64_value(cmd, labelsector_ARG, DEFAULT_LABELSECTOR); + if (!vg_check_new_extent_size(cmd->fmt, vg->extent_size)) return_ECMD_FAILED; @@ -124,67 +122,6 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name, if (active) return_ECMD_FAILED; - dm_list_iterate_items(pvl, &vg->pvs) { - existing_pv = pvl->pv; - - rp.id = existing_pv->id; - rp.idp = &rp.id; - rp.pe_start = pv_pe_start(existing_pv); - rp.extent_count = pv_pe_count(existing_pv); - rp.extent_size = pv_pe_size(existing_pv); - - /* pe_end = pv_pe_count(existing_pv) * pv_pe_size(existing_pv) + pe_start - 1; */ - - if (!(pv = pv_create(cmd, pv_dev(existing_pv), - 0, 0, 0, - arg_int64_value(cmd, labelsector_ARG, - DEFAULT_LABELSECTOR), - pvmetadatacopies, pvmetadatasize, 0, &rp))) { - log_error("Failed to setup physical volume \"%s\"", - pv_dev_name(existing_pv)); - if (change_made) - log_error("Use pvcreate and vgcfgrestore to " - "repair from archived metadata."); - return ECMD_FAILED; - } - - /* Need to revert manually if it fails after this point */ - change_made = 1; - - log_verbose("Set up physical volume for \"%s\" with %" PRIu64 - " available sectors", pv_dev_name(pv), pv_size(pv)); - - /* Wipe existing label first */ - if (!label_remove(pv_dev(pv))) { - log_error("Failed to wipe existing label on %s", - pv_dev_name(pv)); - log_error("Use pvcreate and vgcfgrestore to repair " - "from archived metadata."); - return ECMD_FAILED; - } - - log_very_verbose("Writing physical volume data to disk \"%s\"", - pv_dev_name(pv)); - if (!(pv_write(cmd, pv, 0))) { - log_error("Failed to write physical volume \"%s\"", - pv_dev_name(pv)); - log_error("Use pvcreate and vgcfgrestore to repair " - "from archived metadata."); - return ECMD_FAILED; - } - log_verbose("Physical volume \"%s\" successfully created", - pv_dev_name(pv)); - } - - log_verbose("Deleting existing metadata for VG %s", vg_name); - if (!vg_remove_mdas(vg)) { - log_error("Removal of existing metadata for %s failed.", - vg_name); - log_error("Use pvcreate and vgcfgrestore to repair " - "from archived metadata."); - return ECMD_FAILED; - } - /* FIXME Cache the label format change so we don't have to skip this */ if (test_mode()) { log_verbose("Test mode: Skipping metadata writing for VG %s in" @@ -194,7 +131,7 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name, log_verbose("Writing metadata for VG %s using format %s", vg_name, cmd->fmt->name); - if (!backup_restore_vg(cmd, vg, 0)) { + if (!backup_restore_vg(cmd, vg, &pp, 0)) { log_error("Conversion failed for volume group %s.", vg_name); log_error("Use pvcreate and vgcfgrestore to repair from " "archived metadata.");

1 0

dev-prajnoha-pv-ext-flag-used - metadata: _vg_read: check if PV_EXT_USED flag is set correctly for non-orphan PVs and do a repair if needed
by Peter Rajnoha 21 Aug '15

21 Aug '15

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=921b994187f9c5e0… Commit: 921b994187f9c5e0dca2a6963f090a3aa9ae9d09 Parent: 12752adfa9fe864cd32a43c0f5f43886c3f58588 Author: Peter Rajnoha <prajnoha(a)redhat.com> AuthorDate: Thu Mar 19 07:53:22 2015 +0100 Committer: Peter Rajnoha <prajnoha(a)redhat.com> CommitterDate: Fri Aug 21 11:15:53 2015 +0200 metadata: _vg_read: check if PV_EXT_USED flag is set correctly for non-orphan PVs and do a repair if needed The same check as we already do for orphan PVs, just the other way round now: if the PV is surely part of some VG and any PV the VG contains does not have the PV_EXT_USED flag set, repair it. For example - /dev/sda here is in VG vg and it's incorrectly not marked as used by PV_EXT_USED flag: pvs --binary -o+pv_in_use WARNING: Volume Group vg is not consistent. WARNING: Repairing Physical Volume /dev/sda that is in Volume Group vg but not marked as used. PV VG Fmt Attr PSize PFree ExtVsn InUse InUse /dev/sda vg lvm2 a-- 124.00m 124.00m 2 1 1 --- lib/metadata/metadata.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 56 insertions(+), 1 deletions(-) diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 8670fd6..02aebac 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -3535,6 +3535,52 @@ static int _check_reappeared_pv(struct volume_group *correct_vg, return rv; } +static int _check_or_repair_pv_ext(struct cmd_context *cmd, + struct physical_volume *pv, + int repair, int *inconsistent_pvs) +{ + struct lvmcache_info *info; + uint32_t ext_version, ext_flags; + + /* Missing PV - nothing to do. */ + if (!pv->dev) + return 1; + + if (!(info = lvmcache_info_from_pvid(pv->dev->pvid, 0))) { + log_error("Failed to find cached info for PV %s.", pv_dev_name(pv)); + return 0; + } + + ext_version = lvmcache_ext_version(info); + if (ext_version < 2) { + *inconsistent_pvs = 0; + return 1; + } + + ext_flags = lvmcache_ext_flags(info); + if (!(ext_flags & PV_EXT_USED)) { + if (!repair) { + *inconsistent_pvs = 1; + return 1; + } + + log_warn("WARNING: Repairing Physical Volume %s that is " + "in Volume Group %s but not marked as used.", + pv_dev_name(pv), pv->vg->name); + + /* pv write will set correct ext_flags */ + if (!pv_write(cmd, pv, 1)) { + *inconsistent_pvs = 1; + log_error("Failed to repair physical volume \"%s\".", + pv_dev_name(pv)); + return 0; + } + } + + *inconsistent_pvs = 0; + return 1; +} + static int _repair_inconsistent_vg(struct volume_group *vg) { unsigned saved_handles_missing_pvs = vg->cmd->handles_missing_pvs; @@ -3793,6 +3839,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, inconsistent_pvs = 1; break; } + if (lvmcache_mda_count(info)) { if (!lvmcache_fid_add_mdas_pv(info, fid)) { release_vg(correct_vg); @@ -4069,7 +4116,15 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, return NULL; } - *consistent = 1; + /* We have the VG now finally, check if PV ext info is in sycn with VG metadata. */ + dm_list_iterate_items(pvl, &correct_vg->pvs) { + if (!_check_or_repair_pv_ext(cmd, pvl->pv, *consistent, &inconsistent_pvs)) { + release_vg(correct_vg); + return_NULL; + } + } + + *consistent = !inconsistent_pvs; return correct_vg; }

1 0

← Newer
1
...
5
6
7
8
9
10
11
...
58
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

lvm2-commits August 2015