Skip to contentSkip to conversation

LVM2 ./WHATS_NEW man/pvscan.8.in tools/args.h ...

PATCH New [email protected] 3 participants 4 messages Mar 6, 2012 – Mar 15, 2012

lore
Share & tools
lore
#0 lore raw

CVSROOT: /cvs/lvm2
Module name: LVM2
Changes by: agk at sourceware.org 2012-03-06 02:30:50

Modified files:

	.              : WHATS_NEW 
	man            : pvscan.8.in 
	tools          : args.h commands.h lvchange.c lvcreate.c 

lvmcmdline.c pvscan.c tools.h
udev : 69-dm-lvm-metad.rules

Log message:
Switch pvscan --cache major:minor to --major --minor.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW.diff?cvsroot=lvm2&r1=1.2345&r2=1.2346
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/man/pvscan.8.in.diff?cvsroot=lvm2&r1=1.9&r2=1.10
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/args.h.diff?cvsroot=lvm2&r1=1.88&r2=1.89
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/commands.h.diff?cvsroot=lvm2&r1=1.172&r2=1.173
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/lvchange.c.diff?cvsroot=lvm2&r1=1.139&r2=1.140
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/lvcreate.c.diff?cvsroot=lvm2&r1=1.261&r2=1.262
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/lvmcmdline.c.diff?cvsroot=lvm2&r1=1.152&r2=1.153
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/pvscan.c.diff?cvsroot=lvm2&r1=1.60&r2=1.61
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/tools.h.diff?cvsroot=lvm2&r1=1.79&r2=1.80
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/udev/69-dm-lvm-metad.rules.diff?cvsroot=lvm2&r1=1.2&r2=1.3

WHATS_NEW +1 -0
@@ -4,6 +4,7 @@
   Print just warning on thin pool check callback path for failing check.
   Always use 64bit math with VG extent_size expression.
   Validate udev structures in _insert_udev_dir().
+  Take repeatable --major --minor with pvscan --cache instead of major:minor.
   Scan all devices for lvmetad if 'pvscan --cache' used without device list.
   Populate lvmcache from lvmetad before displaying PVs in pvscan. (2.02.94)
   Suppress incorrect -n pvscan warning now always displayed. (2.02.94)
man/pvscan.8.in +14 -2
@@ -3,7 +3,6 @@
 pvscan \- scan all disks for physical volumes
 .SH SYNOPSIS
 .B pvscan
-.RB [ \-\-cache " " [ DevicePath ... ] ]
 .RB [ \-d | \-\-debug ]
 .RB [ \-h | \-\-help ]
 .RB [ \-v | \-\-verbose ]
@@ -13,6 +12,19 @@
 .RB [ \-n | \-\-novolumegroup ]
 .RB [ \-s | \-\-short ]
 .RB [ \-u | \-\-uuid ]
+.BR
+
+.B pvscan
+.RB [ \-d | \-\-debug ]
+.RB [ \-h | \-\-help ]
+.RB \-\-cache
+.RB [ \-\-major
+.IR major
+.RB \-\-minor
+.IR minor
+|
+.RI DevicePath 
+] ...
 .SH DESCRIPTION
 .B pvscan
 scans all supported LVM block devices in the system for physical volumes.
@@ -31,7 +43,7 @@
 .BR \-u ", " \-\-uuid
 Show UUIDs (Uniform Unique Identifiers) in addition to device special names.
 .TP
-.BR \-\-cache " " [ DevicePath... ]
+.BR \-\-cache " " [ " " \-\-major " " major " " \-\-minor " " minor " " | " " DevicePath " " ] " " ...
 Scan one or more devices and instruct the lvmetad daemon to update its cached
 state accordingly.  Called internally by udev rules.
 All devices listed explicitly are processed \fBregardless\fP of any device
tools/args.h +2 -2
@@ -44,7 +44,7 @@
 arg(deltag_ARG, '\0', "deltag", tag_arg, ARG_GROUPABLE)
 arg(refresh_ARG, '\0', "refresh", NULL, 0)
 arg(mknodes_ARG, '\0', "mknodes", NULL, 0)
-arg(minor_ARG, '\0', "minor", minor_arg, 0)
+arg(minor_ARG, '\0', "minor", minor_arg, ARG_GROUPABLE)
 arg(type_ARG, '\0', "type", segtype_arg, 0)
 arg(alloc_ARG, '\0', "alloc", alloc_arg, 0)
 arg(separator_ARG, '\0', "separator", string_arg, 0)
@@ -115,7 +115,7 @@
 arg(logicalextent_ARG, 'L', "logicalextent", int_arg_with_sign, 0)
 arg(persistent_ARG, 'M', "persistent", yes_no_arg, 0)
 arg(merge_ARG, '\0', "merge", NULL, 0)
-arg(major_ARG, 'j', "major", major_arg, 0)
+arg(major_ARG, 'j', "major", major_arg, ARG_GROUPABLE)
 arg(mirrors_ARG, 'm', "mirrors", int_arg_with_sign, 0)
 arg(metadatatype_ARG, 'M', "metadatatype", metadatatype_arg, 0)
 arg(maps_ARG, 'm', "maps", NULL, 0)
tools/commands.h +3 -3
@@ -663,7 +663,7 @@
    "List all physical volumes",
    PERMITTED_READ_ONLY,
    "pvscan " "\n"
-   "\t[--cache [DevicePath...]] " "\n"
+   "\t[--cache [ DevicePath | --major major --minor minor]...\n"
    "\t[-d|--debug] " "\n"
    "\t{-e|--exported | -n|--novolumegroup} " "\n"
    "\t[-h|-?|--help]" "\n"
@@ -674,8 +674,8 @@
    "\t[-v|--verbose] " "\n"
    "\t[--version]\n",
 
-   cache_ARG, exported_ARG, ignorelockingfailure_ARG, novolumegroup_ARG,
-   partial_ARG, short_ARG, uuid_ARG)
+   cache_ARG, exported_ARG, ignorelockingfailure_ARG, major_ARG, minor_ARG,
+   novolumegroup_ARG, partial_ARG, short_ARG, uuid_ARG)
 
 xx(segtypes,
    "List available segment types",
tools/lvchange.c +8 -0
@@ -456,6 +456,14 @@
 			log_error("Minor number must be specified with -My");
 			return 0;
 		}
+		if (arg_count(cmd, major_ARG) > 1) {
+			log_error("Option -j/--major may not be repeated.");
+			return 0;
+		}
+		if (arg_count(cmd, minor_ARG) > 1) {
+			log_error("Option --minor may not be repeated.");
+			return 0;
+		}
 		if (!arg_count(cmd, major_ARG) && lv->major < 0) {
 			log_error("Major number must be specified with -My");
 			return 0;
tools/lvcreate.c +10 -0
@@ -587,6 +587,16 @@
 	if (!(lp->permission & LVM_WRITE))
 		lp->zero = 0;
 
+	if (arg_count(cmd, major_ARG) > 1) {
+		log_error("Option -j/--major may not be repeated.");
+		return 0;
+	}
+
+	if (arg_count(cmd, minor_ARG) > 1) {
+		log_error("Option --minor may not be repeated.");
+		return 0;
+	}
+
 	lp->minor = arg_int_value(cmd, minor_ARG, -1);
 	lp->major = arg_int_value(cmd, major_ARG, -1);
 
tools/lvmcmdline.c +5 -0
@@ -89,6 +89,11 @@
 	return grouped_arg_count(av, a) ? av[a].value : def;
 }
 
+int32_t grouped_arg_int_value(const struct arg_values *av, int a, const int32_t def)
+{
+	return grouped_arg_count(av, a) ? av[a].i_value : def;
+}
+
 int32_t arg_int_value(struct cmd_context *cmd, int a, const int32_t def)
 {
 	return arg_count(cmd, a) ? cmd->arg_values[a].i_value : def;
tools/pvscan.c +57 -51

148 lines not on this page · raw message

tools/tools.h +1 -0
@@ -173,6 +173,7 @@
 unsigned grouped_arg_count(const struct arg_values *av, int a);
 unsigned grouped_arg_is_set(const struct arg_values *av, int a);
 const char *grouped_arg_str_value(const struct arg_values *av, int a, const char *def);
+int32_t grouped_arg_int_value(const struct arg_values *av, int a, const int32_t def); 
 
 const char *command_name(struct cmd_context *cmd);
 
udev/69-dm-lvm-metad.rules +1 -1
@@ -20,6 +20,6 @@
 KERNEL=="dm-[0-9]*", ENV{DM_UDEV_RULES_VSN}!="?*", GOTO="lvm_end"
 
 # Only process devices already marked as a PV - this requires blkid to be called before.
-ENV{ID_FS_TYPE}=="LVM2_member|LVM1_member", RUN+="$env{DM_SBIN_PATH}/pvscan --cache $major:$minor"
+ENV{ID_FS_TYPE}=="LVM2_member|LVM1_member", RUN+="$env{DM_SBIN_PATH}/pvscan --cache --major $major --minor $minor"
 
 LABEL="lvm_end"
#1 lore raw
Quoted 13 lines
tools/args.h +1 -1
@@ -44,7 +44,7 @@
 arg(deltag_ARG, '\0', "deltag", tag_arg, ARG_GROUPABLE)
 arg(refresh_ARG, '\0', "refresh", NULL, 0)
 arg(mknodes_ARG, '\0', "mknodes", NULL, 0)
-arg(minor_ARG, '\0', "minor", minor_arg, 0)
+arg(minor_ARG, '\0', "minor", minor_arg, ARG_GROUPABLE)
 
 arg(type_ARG, '\0', "type", segtype_arg, 0)
 arg(alloc_ARG, '\0', "alloc", alloc_arg, 0)

arg(separator_ARG, '\0', "separator", string_arg, 0)

tools/args.h line 121

Quoted 8 lines
tools/args.h +1 -1
@@ -115,7 +115,7 @@
 arg(logicalextent_ARG, 'L', "logicalextent", int_arg_with_sign, 0)
 arg(persistent_ARG, 'M', "persistent", yes_no_arg, 0)
 arg(merge_ARG, '\0', "merge", NULL, 0)
-arg(major_ARG, 'j', "major", major_arg, 0)
+arg(major_ARG, 'j', "major", major_arg, ARG_GROUPABLE)
 arg(mirrors_ARG, 'm', "mirrors", int_arg_with_sign, 0)
 arg(metadatatype_ARG, 'M', "metadatatype", metadatatype_arg, 0)
 arg(maps_ARG, 'm', "maps", NULL, 0)

These two changes are responsible for RHBZ 801205, since we use arg_int_value on the major/minor _ARG in lvchange/lvcreate etc., but it won't work with groupable options.

We can probably:
- either add explicit loops to all the lvchange/lvcreate commands OR
- add a new first_grouped_arg_int_value and use that

Opinions? (I'll try the latter, even though both are very ugly.) Maybe we could contrive a way for arg_int_value to behave like first_grouped_arg_int_value on groupable options, although I am not entirely sure it's easy to extract that information at that point.

Petr

Signature · 1 line
id' Ash = Ash; id' Dust = Dust; id' _ = undefined
#2 lore raw

PATCH 1/1 Fix broken --major/--minor (RHBZ 801205)

Quoted 5 lines · Petr Rockai

These two changes are responsible for RHBZ 801205, since we use arg_int_value on the major/minor _ARG in lvchange/lvcreate etc., but it won't work with groupable options.

- add a new first_grouped_arg_int_value and use that

The patch to do the latter, including a regression test for the bug:

-------------- next part --------------
A non-text attachment was scrubbed...
Name: activate-minor.diff
Type: text/x-patch
Size: 4139 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20120315/0711f372/attachment.bin>
-------------- next part --------------

Signature · 1 line
id' Ash = Ash; id' Dust = Dust; id' _ = undefined
#3 lore raw

PATCH 1/1 Fix broken --major/--minor (RHBZ 801205)

Quoted 4 lines · Peter Rockai
Quoted 3 lines · Petr Rockai

These two changes are responsible for RHBZ 801205, since we use arg_int_value on the major/minor _ARG in lvchange/lvcreate etc., but it won't work with groupable options.

Gah.

Two options:

Have minor_ARG and minor_GROUPED_ARG as separate args (and rename other GROUPABLE args)
- forces the user to know if they're using grouped args or not
- that might be good or bad

or

Fix arg_int_value to recognise a groupable arg

        return (_cmdline.arg_props[a].flags & ARG_GROUPABLE) ? first_grouped_arg_int_value(cmd, a, def) : arg_count(cmd, a) ? cmd->arg_values[a].i_value : def;
Quoted 1 line

+int32_t first_grouped_arg_int_value(struct cmd_context *cmd, int a, const int32_t def)

The arg might not be set in the first group so it does need to loop.

{
        struct arg_value_group_list *current_group;
        struct arg_values *av;

        dm_list_iterate_items(current_group, &cmd->arg_value_groups)  {
		av = current_group->arg_values;
                if (grouped_arg_count(av, a))
                        return grouped_arg_int_value(av, a, def);
	}

        return def;
}

Alasdair