Move guts of pvcreate into library area - prep for vgcreate.

Preparation for allowing vgcreate on devices which are not currently PVs. Move necessary pvcreate code from tools subdir into library so we can call this higher-level pvcreate code from the vgcreate path in the library. This will also get us a little closer to where we should be with libLVM, though there is some cleanup to do (pvcreate_single and pv_create should be merged into a single function exported by the libLVM).

---
 lib/metadata/metadata-exported.h |   18 +++
 lib/metadata/metadata.c          |  196 +++++++++++++++++++++++++++++++++++
 tools/pvcreate.c                 |  213 +-------------------------------------
 3 files changed, 215 insertions(+), 212 deletions(-)

index d49eaba..829a894 100644
@@ -353,6 +353,24 @@ vg_t *vg_lock_and_read(struct cmd_context *cmd, const char *vg_name,
 		       uint32_t lock_flags, uint32_t status_flags,
 		       uint32_t misc_flags);
 
+struct pvcreate_params {
+	int zero;
+	uint64_t size;
+	int pvmetadatacopies;
+	uint64_t pvmetadatasize;
+	int64_t labelsector;
+	struct id id; /* FIXME: redundant */
+	struct id *idp; /* 0 if no --uuid option */
+	uint64_t pe_start;
+	uint32_t extent_count;
+	uint32_t extent_size;
+	const char *restorefile; /* 0 if no --restorefile option */
+	force_t force;
+	unsigned yes;
+};
+
+int pvcreate_single(struct cmd_context *cmd, const char *pv_name,
+		    void *handle);
 /* pe_start and pe_end relate to any existing data so that new metadata
 * areas can avoid overlap */
 pv_t *pv_create(const struct cmd_context *cmd,

1.5.5.1

---
 lib/metadata/metadata.c |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

index 265b619..09c345f 100644
@@ -993,10 +993,28 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name,
 int pvcreate_single(struct cmd_context *cmd, const char *pv_name,
 		    void *handle)
 {
-	struct pvcreate_params *pp = (struct pvcreate_params *) handle;
+	struct pvcreate_params *pp;
 	void *pv;
 	struct device *dev;
 	struct dm_list mdas;
+	struct pvcreate_params default_pp = { 0, /* zero Y/N */
+					      0, /* size */
+					      DEFAULT_PVMETADATACOPIES,
+					      DEFAULT_PVMETADATASIZE,
+					      DEFAULT_LABELSECTOR,
+					      { { 0 } }, /* id */
+					      0, /* idp */
+					      0, /* pe_start */
+					      0, /* extent_count */
+					      0, /* extent_size */
+					      0, /* restorefile */
+					      PROMPT, /* force option */
+					      0 /* yes */};
+
+	if (!handle)
+		pp = &default_pp;
+	else
+		pp = (struct pvcreate_params *) handle;
 
 	if (pp->idp) {
 		if ((dev = device_from_pvid(cmd, pp->idp)) &&

1.5.5.1

---
 lib/metadata/metadata-exported.h |    4 ++--
 lib/metadata/metadata.c          |   12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

index 829a894..c77ebdd 100644
@@ -369,8 +369,8 @@ struct pvcreate_params {
 	unsigned yes;
 };
 
-int pvcreate_single(struct cmd_context *cmd, const char *pv_name,
-		    void *handle);
+pv_t *pvcreate_single(struct cmd_context *cmd, const char *pv_name,
+		      void *handle);
 /* pe_start and pe_end relate to any existing data so that new metadata
 * areas can avoid overlap */
 pv_t *pv_create(const struct cmd_context *cmd,

index 09c345f..4b231d3 100644
@@ -990,8 +990,8 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name,
 	return 1;
 }
 
-int pvcreate_single(struct cmd_context *cmd, const char *pv_name,
-		    void *handle)
+pv_t *pvcreate_single(struct cmd_context *cmd, const char *pv_name,
+		      void *handle)
 {
 	struct pvcreate_params *pp;
 	void *pv;

@@ -1021,13 +1021,13 @@ int pvcreate_single(struct cmd_context *cmd, const char *pv_name,
 		    (dev != dev_cache_get(pv_name, cmd->filter))) {
 			log_error("uuid %s already in use on \"%s\"",
 				  pp->idp->uuid, dev_name(dev));
-			return 0;
+			return NULL;
 		}
 	}
 
 	if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) {
 		log_error("Can't get lock for orphan PVs");
-		return 0;
+		return NULL;
 	}
 
 	if (!pvcreate_check(cmd, pv_name, pp))

@@ -1086,11 +1086,11 @@ int pvcreate_single(struct cmd_context *cmd, const char *pv_name,
 	log_print("Physical volume \"%s\" successfully created", pv_name);
 
 	unlock_vg(cmd, VG_ORPHANS);
-	return 1;
+	return pv;
 
       error:
 	unlock_vg(cmd, VG_ORPHANS);
-	return 0;
+	return NULL;
 }
 
 /* FIXME: liblvm todo - make into function that returns handle */

1.5.5.1

Note that this changes the behavior of vgcreate and vgextend when non-PV devices are given. Previously this would result in vgextend or vgcreate printing an error message and exiting. Now these tools will prompt the user.

With this update, the user may opt to skip pvcreate, provided the default values of pvcreate are adequate. If the user wants some non-default value in the PV (e.g. 0 metadatacopies, etc), pvcreate must be used prior to vgcreate/vgextend.

Update man pages to clarify the new behavior.

Modify pvcreate_single() to first check whether the VG_ORPHANS lock is held before trying to obtain it. This is necessary for the vgexten code path, which must obtain VG_ORPHANS before obtaining the appropriate VG lock, and befroe calling vg_extend(). The other paths calling pvcreate_single() are pvcreate() and vgcreate(), and neither of these paths hold VG_ORPHANS. Without this modification vgextend() will hang trying to obtain VG_ORPHANS which it already holds.

---
 WHATS_NEW               |    1 +
 lib/metadata/metadata.c |   32 ++++++++++++++++++++++++++++----
 man/vgcreate.8.in       |   15 +++++++++------
 man/vgextend.8.in       |    6 ++++++
 4 files changed, 44 insertions(+), 10 deletions(-)

index 459533a..03ec238 100644
@@ -1,5 +1,6 @@
 Version 2.02.44 - 
 ====================================
+  Update vgcreate and vgextend to allow uninitialized devices as input.
   Don't skip updating pvid hash when lvmcache_info struct got swapped.
   Add tinfo to termcap search path for pld-linux.
   Fix startup race in clvmd.

index 4b231d3..4e375f5 100644
@@ -422,9 +422,22 @@ int vg_extend(struct volume_group *vg, int pv_count, char **pv_names)
 	/* attach each pv */
 	for (i = 0; i < pv_count; i++) {
 		if (!(pv = pv_by_path(vg->fid->fmt->cmd, pv_names[i]))) {
-			log_error("%s not identified as an existing "
-				  "physical volume", pv_names[i]);
-			goto bad;
+			if (yes_no_prompt("%s not identified as an existing "
+					  "physical volume.\nInitialize "
+					  "device and add to volume "
+					  "group %s? [y/n]: ", pv_names[i],
+					  vg->name) == 'n') {
+				goto bad;
+			} else {
+				pv = pvcreate_single(vg->cmd, pv_names[i],
+						     NULL);
+				if (!pv) {
+					log_error("Failed to setup physical "
+						  "volume \"%s\"",
+						  pv_names[i]);
+					goto bad;
+				}
+			}
 		}
 		
 		if (!add_pv_to_vg(vg, pv_names[i], pv))

@@ -990,6 +1003,16 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name,
 	return 1;
 }
 
+/*
+ * pvcreate_single() - initialize a device with PV label and metadata
+ *
+ * Locking: Ok (but not required) for caller to hold VG_ORPHAN lock
+ * before calling
+ *
+ * Parameters:
+ * - pv_name: device path to initialize
+ * - handle: options to pass to pv_create; NULL indicates use defaults
+ */
 pv_t *pvcreate_single(struct cmd_context *cmd, const char *pv_name,
 		      void *handle)
 {

@@ -1025,7 +1048,8 @@ pv_t *pvcreate_single(struct cmd_context *cmd, const char *pv_name,
 		}
 	}
 
-	if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) {
+	if (!vgname_is_locked(VG_ORPHANS) &&
+	    (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE))) {
 		log_error("Can't get lock for orphan PVs");
 		return NULL;
 	}

index a53197e..0e8baa2 100644
@@ -21,16 +21,19 @@ vgcreate \- create a volume group
 .RB [ \-t | \-\-test ]
 .RB [ \-v | \-\-verbose ]
 .RB [ \-\-version ]
-.I VolumeGroupName PhysicalVolumePath
-.RI [ PhysicalVolumePath ...]
+.I VolumeGroupName PhysicalDevicePath
+.RI [ PhysicalDevicePath ...]
 .SH DESCRIPTION
 .B vgcreate
 creates a new volume group called
 .I VolumeGroupName
-using the block special device
-.IR PhysicalVolumePath
-previously configured for LVM with
-.BR pvcreate (8).
+using the block special device \fIPhysicalDevicePath\fP.
+If \fIPhysicalDevicePath\fP was not previously configured for LVM with
+\fBpvcreate (8)\fP, a prompt will be given to confirm initialization of
+the device.  If confirmed, the device will be initialized with the same
+default values used with \fBpvcreate\fP.  If non-default 
+\fPpvcreate\fP values are are desired, \fBpvcreate\fP should be explicitly
+called prior to calling \fBvgcreate\fP.
 .SH OPTIONS
 See \fBlvm\fP for common options.
 .TP

index 682cc5a..1fee769 100644
@@ -11,6 +11,12 @@ VolumeGroupName PhysicalDevicePath [PhysicalDevicePath...]
 vgextend allows you to add one or more initialized physical volumes ( see
 .B pvcreate(8)
 ) to an existing volume group to extend it in size.
+If \fIPhysicalDevicePath\fP was not previously configured for LVM with
+\fBpvcreate (8)\fP, a prompt will be given to confirm initialization of
+the device.  If confirmed, the device will be initialized with the same
+default values used with \fBpvcreate\fP.  If non-default 
+\fPpvcreate\fP values are are desired, \fBpvcreate\fP should be explicitly
+called prior to calling \fBvgextend\fP.
 .SH OPTIONS
 See \fBlvm\fP for common options.
 .SH Examples

1.5.5.1

I thought it would clutter the commands unnecessarily for options that were seldom used. I will work on a patch for adding the parameters though since it sounds like that is what you would like.

So you don't think there is danger in just switching to the new behavior (used to be an error to vgcreate/vgextend with non-initialized disk, now will initialized the disk) without a transition period? The case I was thinking of was a user that inadvertently specified the wrong disk. The pvcreate code will catch at least some of the cases but perhaps not others. It is now a destructive operation whereas before it was not.

Right. I was just wondering about the transition period and whether we would deprecate pvcreate in the future. If not, then why not just leave vgextend/vgcreate without the pvcreate options and make the less-likely user pay the penalty?

Note that this changes the behavior of vgcreate and vgextend when non-PV devices are given. Previously this would result in vgextend or vgcreate printing an error message and exiting. Now these tools will implicitly pvcreate on any devices given on the commandline that are not already PVs. All devices given on the commandline are first checked if they are an existing PV before attempting to initialize, so there should be little/no risk with this change.

With this update, the user may opt to skip pvcreate, provided the default values of pvcreate are adequate. If the user wants some non-default value in the PV (e.g. 0 metadatacopies, etc), pvcreate must be used prior to vgcreate/vgextend.

Update man pages to clarify the new behavior.

Add pvcreate_devices() helper function inside toollib which loops through a set of device names and runs pvcreate on each devices if it is not already a PV.

---
 WHATS_NEW                        |    1 +
 lib/metadata/metadata-exported.h |    1 +
 lib/metadata/metadata.c          |    7 +++++++
 lib/metadata/metadata.h          |    1 -
 man/vgcreate.8.in                |   14 ++++++++------
 man/vgextend.8.in                |    5 +++++
 tools/toollib.c                  |   23 +++++++++++++++++++++++
 tools/toollib.h                  |    2 ++
 tools/vgcreate.c                 |    6 ++++++
 tools/vgextend.c                 |    6 ++++++
 10 files changed, 59 insertions(+), 7 deletions(-)

index 459533a..03ec238 100644
@@ -1,5 +1,6 @@
 Version 2.02.44 - 
 ====================================
+  Update vgcreate and vgextend to allow uninitialized devices as input.
   Don't skip updating pvid hash when lvmcache_info struct got swapped.
   Add tinfo to termcap search path for pld-linux.
   Fix startup race in clvmd.

index c77ebdd..71dfe36 100644
@@ -588,6 +588,7 @@ uint32_t pv_pe_size(const pv_t *pv);
 uint64_t pv_pe_start(const pv_t *pv);
 uint32_t pv_pe_count(const pv_t *pv);
 uint32_t pv_pe_alloc_count(const pv_t *pv);
+pv_t *pv_by_path(struct cmd_context *cmd, const char *pv_name);
 
 int vg_missing_pv_count(const vg_t *vg);
 uint32_t vg_status(const vg_t *vg);

index 4b231d3..8afa322 100644
@@ -990,6 +990,13 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name,
 	return 1;
 }
 
+/*
+ * pvcreate_single() - initialize a device with PV label and metadata
+ *
+ * Parameters:
+ * - pv_name: device path to initialize
+ * - handle: options to pass to pv_create; NULL indicates use defaults
+ */
 pv_t *pvcreate_single(struct cmd_context *cmd, const char *pv_name,
 		      void *handle)
 {

index 3d9fd8d..7ce2c62 100644
@@ -343,7 +343,6 @@ struct id pv_id(const pv_t *pv);
 const struct format_type *pv_format_type(const pv_t *pv);
 struct id pv_vgid(const pv_t *pv);
 
-pv_t *pv_by_path(struct cmd_context *cmd, const char *pv_name);
 int add_pv_to_vg(struct volume_group *vg, const char *pv_name,
 		 struct physical_volume *pv);
 

index a53197e..54276dd 100644
@@ -21,16 +21,18 @@ vgcreate \- create a volume group
 .RB [ \-t | \-\-test ]
 .RB [ \-v | \-\-verbose ]
 .RB [ \-\-version ]
-.I VolumeGroupName PhysicalVolumePath
-.RI [ PhysicalVolumePath ...]
+.I VolumeGroupName PhysicalDevicePath
+.RI [ PhysicalDevicePath ...]
 .SH DESCRIPTION
 .B vgcreate
 creates a new volume group called
 .I VolumeGroupName
-using the block special device
-.IR PhysicalVolumePath
-previously configured for LVM with
-.BR pvcreate (8).
+using the block special device \fIPhysicalDevicePath\fP.
+If \fIPhysicalDevicePath\fP was not previously configured for LVM with
+\fBpvcreate (8)\fP, the device will be initialized with the same
+default values used with \fBpvcreate\fP.  If non-default 
+\fPpvcreate\fP values are are desired, \fBpvcreate\fP should be explicitly
+called prior to calling \fBvgcreate\fP.
 .SH OPTIONS
 See \fBlvm\fP for common options.
 .TP

index 682cc5a..3ba44a7 100644
@@ -11,6 +11,11 @@ VolumeGroupName PhysicalDevicePath [PhysicalDevicePath...]
 vgextend allows you to add one or more initialized physical volumes ( see
 .B pvcreate(8)
 ) to an existing volume group to extend it in size.
+If \fIPhysicalDevicePath\fP was not previously configured for LVM with
+\fBpvcreate (8)\fP, the device will be initialized with the same
+default values used with \fBpvcreate\fP.  If non-default 
+\fPpvcreate\fP values are are desired, \fBpvcreate\fP should be explicitly
+called prior to calling \fBvgcreate\fP.
 .SH OPTIONS
 See \fBlvm\fP for common options.
 .SH Examples

index ef4ee0e..2f0713e 100644
@@ -1288,3 +1288,26 @@ int fill_vg_create_params(struct cmd_context *cmd,
 
 	return 0;
 }
+
+/*
+ * Run pvcreate on a set of devices if they are not currently initialized.
+ */
+int pvcreate_devices(struct cmd_context *cmd, int pv_count, char **pv_names)
+{
+	int i;
+	pv_t *pv;
+
+	for (i = 0; i < pv_count; i++) {
+		if (!(pv = pv_by_path(cmd, pv_names[i]))) {
+			log_verbose("Running pvcreate on %s", pv_names[i]);
+			if (!pvcreate_single(cmd, pv_names[i], NULL)) {
+				log_error("Failed to setup physical "
+					  "volume \"%s\"",
+					  pv_names[i]);
+				return 0;
+			}
+		}
+	}
+
+	return 1;
+}

index f002d60..72d13a3 100644
@@ -100,4 +100,6 @@ int is_reserved_lvname(const char *name);
 int fill_vg_create_params(struct cmd_context *cmd,
 			  char *vg_name, struct vgcreate_params *vp_new,
 			  struct vgcreate_params *vp_def);
+int pvcreate_devices(struct cmd_context *cmd, int pv_count, char **pv_names);
+
 #endif

index 62c957a..d579ea9 100644
@@ -46,6 +46,12 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
 	if (validate_vg_create_params(cmd, &vp_new))
 	    return EINVALID_CMD_LINE;
 
+	/*
+	 * Check whether any devices need initialized
+	 */
+	if (!pvcreate_devices(cmd, argc - 1, argv + 1))
+		return ECMD_FAILED;
+
 	/* Create the new VG */
 	if (!(vg = vg_create(cmd, vp_new.vg_name, vp_new.extent_size,
 			     vp_new.max_pv, vp_new.max_lv, vp_new.alloc,

index 089c44e..154fbf6 100644
@@ -35,6 +35,12 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv)
 	argc--;
 	argv++;
 
+	/*
+	 * Check whether any devices need initialized
+	 */
+	if (!pvcreate_devices(cmd, argc, argv))
+		return ECMD_FAILED;
+
 	if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) {
 		log_error("Can't get lock for orphan PVs");
 		return ECMD_FAILED;

1.5.5.1

Can that hook instead into vg_extend() to replace the error message? Current explicit PV functionality needs to become handled implicitly throughout the code, rather than "do all the PV things" then "do all the VG things".

Currently:
vgcreate = { create empty vg; vgextend; }

int vg_extend(struct volume_group *vg, int pv_count, char **pv_names) {
...

        /* attach each pv */
        for (i = 0; i < pv_count; i++) {
                if (!(pv = pv_by_path(vg->fid->fmt->cmd, pv_names[i]))) {
                        log_error("%s not identified as an existing "
                                  "physical volume", pv_names[i]);
                        goto bad;
                }

Alasdair

agk at redhat.com

I tried that in my first patch and ran into an issue with VG_ORPHAN lock.
http://www.redhat.com/archives/lvm-devel/2008-November/msg00075.html

vgextend() obtains VG_ORPHAN, then the lock on the vg before calling vg_extend(). So if I put pvcreate_devices() logic in there, that calls down into pvcreate_single(), which also tries to obtain / release VG_ORPHAN. I could put logic into pvcreate_single() to check to see if lock is already obtained (original patch has that) and not try to reaquire / release VG_ORPHAN (original patch missed release part), but that looked more hacky than the solution of first calling the logic at the higher level in the tool. Would you prefer this first approach and detecting whether VG_ORPHAN was obtained?

Makes sense.

So you have two options:
1. an extra parameter on the function to say whether or not the lock is already held
2. require the lock always to be held before calling that function.

Think of VG_ORPHAN as if it was a real volume group - pvcreate is the same as vgextend on that volume group and should be calling into the same code and should already hold the VG lock.

But then you see this:

                unlock_vg(cmd, VG_ORPHANS);

                persistent_filter_wipe(cmd->filter);
                lvmcache_destroy(cmd, 1);

                init_md_filtering(0);
                if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) {

and need to worry about *why* the code drops the lock there.

It's probably just for accounting reasons - the cache knows something about held locks. IOW (I think) no locks can be held when lvmcache_destroy is called. Check that (look at other instances where it's called) then look for some workaround - a less-drastic destroy function that preserves the state, perhaps, or splitting the function.

Also note that VG_ORPHANS is set to disappear, replaced by ORPHAN_VG_NAME(fmt).

Alasdair

agk at redhat.com

Look at each of the pvcreate command line options and try to offer them as part of vgcreate and vgextend. They will only be used for devices that are not already PVs and inside the VG.

vgcreate
[--addtag Tag]
- will continue to add a VG tag not a PV tag (because the command name starts with vg)
If there's a need for both in future we can add --addpvtag and --addvgtag.

pvcreate args:
Add to both vgcreate & vgextend:
[-f[f]|--force [--force]]
[-y|--yes]
- trivial

[--labelsector sector] [--metadatasize MetadataSize[kKmMgGtTpPeE]]
- unambiguous

[--metadatacopies #copies]
- ambiguous: Does it mean number of copies on each disk or number of copies in total in the VG?
Solution: add --pvmetadatacopies to pvcreate, vgcreate & vgextend In future we can add --vgmetadatacopies to vgcreate & vgextend. --metadatacopies will be the same as --pvmetadatacopies in pv* commands and --vgmetadatacopies in vg* commands. Code it ready to work like that, i.e. change the existing code to use pvmetadatacopies_ARG everywhere internally, and use _merge_synonym() based on the first 2 chars of the command (hard-coded).

[-Z|--zero {y|n}]
- potentially ambiguous: Does it mean zero the VG itself?
But it's already ambiguous and the only way to find out what it does is to read the man page definition. Zeroing the VG would have no impact or use within LVM, so we can leave it doing just what it does today to new PVs.

Don't add to vgcreate & vgextend - leave these as pv-only for now: (We can add them in an extended way eventually.)
[--setphysicalvolumesize PhysicalVolumeSize[kKmMgGtTpPeE]
- main use is for testing so leave as pvcreate only

[--restorefile file]
[-u|--uuid uuid]
- only required in recovery situations

After these changes, the only necessary use of pvcreate will be in recovery situations. In future, we'll sort out those remaining recovery options, then require a vgname (defaulting to an explicit per-metadata-type orphan vg) always to be given with pvcreate so that every PV belongs to a VG and make pvcreate a synonym for vgcreate.

Alasdair

agk at redhat.com

Why global mode - why not just set 'force' and '-y' for the API call? I think this was similar to Thomas's argument - he always envisioned '-ff' and '-y' on things like pvcreate.

To be specific, in pvcreate we prompt if:
1) the device is already a PV
2) we find an MD superblock
(and of course the user has not given the '-ff' and '-y')

So if we don't want the yes_no_prompt() in the library, either the library calls do not do these checks at all, or we duplicate the code in the library. Otherwise, we leave the yes_no_prompt() code in the library and just set the options for the API call.

Note that we currently have yes_no_prompt() in the library for lvremove and vgremove code paths. The pvcreate/vgremove/lvremove paths might also be a good area to talk about the error codes/handling and what the API should look like.

To clarify: I mentioned two possible ways to proceed there.

1) no prompting in the library (which was my original intention) => library calls are split at appropriate places
In your example above, this would mean separate calls for the validation functions (which anaconda would also call) prior to the call that actually makes the change (which would not repeat the validation).

2) prompting within the library, with a guaranteed way for a caller to avoid it (e.g. a global setting that defaults to returning with an error whenever there would otherwise have been a prompt).
In the pvcreate example, it would avoid returning with an error because when the equivalent of -ff and -y are given there would not be a prompt.

Alasdair

agk at redhat.com

I don't see how this is possible. Are you suggesting we split the validation into a separate API call(s), and let an app decide to call it or not?

If you're not saying let the app decide, then if we split the validation and the create APIs, the only way I can think of ensuring the app has called the validation before calling the create is to somehow store a "passed_validation" flag somewhere (maybe within struct cmd_context). Seems kinda messy though since the "passed_validation" would depend on the create parameters.

Maybe there is another way but I don't see it.

This might be the only reasonable way to proceed. This would be the equivalent of a "--no" option without prompting I guess.

There are two sorts of validation.

Optional validation and compulsory validation.

Compulsory validation covers anything that is required for internal LVM consistency. This must be built into the library calls and impossible to bypass.

Optional validation covers everything else: the client of the library is free to bypass it if it believes it knows what it is doing and doesn't require it. Roughly, this is what you get today with -ff -y.

Two current examples from pvcreate:

Compulsory
- The device must not be in use currently (ie we can open it exclusively)

Optional
- PV must not belong to an existing VG (ie we can destroy a VG if want to)

Alasdair

agk at redhat.com

Split the validation into optional validation and compulsory validation. Offer separate library call(s) for the optional validation, but build the compulsory validation into the main call(s).

Alasdair

agk at redhat.com

So looking at pvcreate, most of pvcreate_check() is optional and depending on error handling could be done as one piece or broken into multiple functions. The few bits of compulsory validation move out of there into the actual pvcreate call (which is performed via a vgextend() interface now).

Alasdair

agk at redhat.com

Ok, I did not understand that we are going to allow an APP the option to bypass what was in pvcreate_check() (except the exclusive device open check).