Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=32ae07cef1a30064…
Commit: 32ae07cef1a30064d3269b5e10f2c54f6486c446
Parent: 784867d5bd47fce992b6b6e964d9088a652fc0b9
Author: Peter Rajnoha <prajnoha(a)redhat.com>
AuthorDate: Mon Mar 25 14:30:40 2013 +0100
Committer: Peter Rajnoha <prajnoha(a)redhat.com>
CommitterDate: Mon Mar 25 15:08:26 2013 +0100
pv_write: clean up non-orphan format1 PV write
...to not pollute the common and format-independent code in the
abstraction layer above.
The format1 pv_write has common code for writing metadata and
PV header by calling the "write_disks" fn and when rewriting
the header itself only (e.g. just for the purpose of changing
the PV UUID) during the pvchange operation, we had to tweak
this functionality for the format1 case and we had to assign
the PV the orphan state temporarily.
This patch removes the need for this format1 tweak and it calls
the write_disks with appropriate flag indicating whether this is
a PV write call or a VG write call, allowing for metatada update
for the latter one.
Also, a side effect of the former tweak was that it effectively
invalidated the cache (even for the non-format1 PVs) as we
assigned it the orphan state temporarily just for the format1
PV write to pass.
Also, that tweak made it difficult to directly detect whether
a PV was part of a VG or not because the state was incorrect.
Also, it's not necessary to backup and restore some PV fields
when doing a PV write:
orig_pe_size = pv_pe_size(pv);
orig_pe_start = pv_pe_start(pv);
orig_pe_count = pv_pe_count(pv);
...
pv_write(pv)
...
pv->pe_size = orig_pe_size;
pv->pe_start = orig_pe_start;
pv->pe_count = orig_pe_count;
...this is already done by the layer below itself (the _format1_pv_write fn).
So let's have this cleaned up so we don't need to be bothered
about any 'format1 special case for pv_write' anymore.
---
WHATS_NEW | 1 +
lib/format1/disk-rep.c | 14 +++++++-------
lib/format1/disk-rep.h | 3 ++-
lib/format1/format1.c | 4 ++--
tools/pvchange.c | 32 ++++----------------------------
5 files changed, 16 insertions(+), 38 deletions(-)
diff --git a/WHATS_NEW b/WHATS_NEW
index a35c09b..61a2558 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
Version 2.02.99 -
===================================
+ Clean up format1 PV write to remove a need for an orphan VG for it to pass.
Fix vgextend to not allow a PV with 0 MDAs to be used while already in a VG.
Move update_pool_params() from /tools to /lib for better reuse.
Give precedence to EMC power2 devices with duplicate PVIDs.
diff --git a/lib/format1/disk-rep.c b/lib/format1/disk-rep.c
index ce6cda1..19776e0 100644
--- a/lib/format1/disk-rep.c
+++ b/lib/format1/disk-rep.c
@@ -674,7 +674,7 @@ static int _write_pvd(struct disk_list *data)
* assumes the device has been opened.
*/
static int __write_all_pvd(const struct format_type *fmt __attribute__((unused)),
- struct disk_list *data)
+ struct disk_list *data, int write_vg_metadata)
{
const char *pv_name = dev_name(data->dev);
@@ -685,9 +685,9 @@ static int __write_all_pvd(const struct format_type *fmt __attribute__((unused))
/* vgcache_add(data->pvd.vg_name, data->vgd.vg_uuid, data->dev, fmt); */
/*
- * Stop here for orphan pv's.
+ * Stop here for orphan PVs or if VG metadata write not requested.
*/
- if (data->pvd.vg_name[0] == '\0') {
+ if ((data->pvd.vg_name[0] == '\0') || !write_vg_metadata) {
/* if (!test_mode())
vgcache_add(data->pvd.vg_name, NULL, data->dev, fmt); */
return 1;
@@ -723,14 +723,14 @@ static int __write_all_pvd(const struct format_type *fmt __attribute__((unused))
/*
* opens the device and hands to the above fn.
*/
-static int _write_all_pvd(const struct format_type *fmt, struct disk_list *data)
+static int _write_all_pvd(const struct format_type *fmt, struct disk_list *data, int write_vg_metadata)
{
int r;
if (!dev_open(data->dev))
return_0;
- r = __write_all_pvd(fmt, data);
+ r = __write_all_pvd(fmt, data, write_vg_metadata);
if (!dev_close(data->dev))
stack;
@@ -743,12 +743,12 @@ static int _write_all_pvd(const struct format_type *fmt, struct disk_list *data)
* little sanity checking, so make sure correct
* data is passed to here.
*/
-int write_disks(const struct format_type *fmt, struct dm_list *pvs)
+int write_disks(const struct format_type *fmt, struct dm_list *pvs, int write_vg_metadata)
{
struct disk_list *dl;
dm_list_iterate_items(dl, pvs) {
- if (!(_write_all_pvd(fmt, dl)))
+ if (!(_write_all_pvd(fmt, dl, write_vg_metadata)))
return_0;
log_very_verbose("Successfully wrote data to %s",
diff --git a/lib/format1/disk-rep.h b/lib/format1/disk-rep.h
index 0d54438..52ee303 100644
--- a/lib/format1/disk-rep.h
+++ b/lib/format1/disk-rep.h
@@ -197,7 +197,8 @@ int read_pvs_in_vg(const struct format_type *fmt, const char *vg_name,
struct dev_filter *filter,
struct dm_pool *mem, struct dm_list *results);
-int write_disks(const struct format_type *fmt, struct dm_list *pvds);
+int write_disks(const struct format_type *fmt, struct dm_list *pvds,
+ int write_vg_metadata);
/*
* Functions to translate to between disk and in
diff --git a/lib/format1/format1.c b/lib/format1/format1.c
index f3945bc..f47ed95 100644
--- a/lib/format1/format1.c
+++ b/lib/format1/format1.c
@@ -299,7 +299,7 @@ static int _format1_vg_write(struct format_instance *fid, struct volume_group *v
r = (_flatten_vg(fid, mem, vg, &pvds, fid->fmt->cmd->dev_dir,
fid->fmt->cmd->filter) &&
- write_disks(fid->fmt, &pvds));
+ write_disks(fid->fmt, &pvds, 1));
lvmcache_update_vg(vg, 0);
dm_pool_destroy(mem);
@@ -458,7 +458,7 @@ static int _format1_pv_write(const struct format_type *fmt, struct physical_volu
dl->pvd.pe_on_disk.base = LVM1_PE_ALIGN << SECTOR_SHIFT;
dm_list_add(&pvs, &dl->list);
- if (!write_disks(fmt, &pvs))
+ if (!write_disks(fmt, &pvs, 0))
goto_bad;
goto out;
diff --git a/tools/pvchange.c b/tools/pvchange.c
index c282ec0..1458e92 100644
--- a/tools/pvchange.c
+++ b/tools/pvchange.c
@@ -19,13 +19,7 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg,
struct physical_volume *pv,
void *handle __attribute__((unused)))
{
- uint32_t orig_pe_alloc_count;
- /* FIXME Next three only required for format1. */
- uint32_t orig_pe_count, orig_pe_size;
- uint64_t orig_pe_start;
-
const char *pv_name = pv_dev_name(pv);
- const char *orig_vg_name;
char uuid[64] __attribute__((aligned(8)));
int allocatable = 0;
@@ -129,28 +123,10 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg,
if (!id_write_format(&pv->id, uuid, sizeof(uuid)))
return 0;
log_verbose("Changing uuid of %s to %s.", pv_name, uuid);
- if (!is_orphan(pv)) {
- orig_vg_name = pv_vg_name(pv);
- orig_pe_alloc_count = pv_pe_alloc_count(pv);
-
- /* FIXME format1 pv_write doesn't preserve these. */
- orig_pe_size = pv_pe_size(pv);
- orig_pe_start = pv_pe_start(pv);
- orig_pe_count = pv_pe_count(pv);
-
- pv->vg_name = pv->fmt->orphan_vg_name;
- pv->pe_alloc_count = 0;
- if (!(pv_write(cmd, pv, 0))) {
- log_error("pv_write with new uuid failed "
- "for %s.", pv_name);
- return 0;
- }
- pv->vg_name = orig_vg_name;
- pv->pe_alloc_count = orig_pe_alloc_count;
-
- pv->pe_size = orig_pe_size;
- pv->pe_start = orig_pe_start;
- pv->pe_count = orig_pe_count;
+ if (!is_orphan(pv) && (!pv_write(cmd, pv, 1))) {
+ log_error("pv_write with new uuid failed "
+ "for %s.", pv_name);
+ return 0;
}
}
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=784867d5bd47fce9…
Commit: 784867d5bd47fce992b6b6e964d9088a652fc0b9
Parent: ea36d0501e0ede347f251f6cda20903ad13f3700
Author: Peter Rajnoha <prajnoha(a)redhat.com>
AuthorDate: Tue Mar 19 15:41:21 2013 +0100
Committer: Peter Rajnoha <prajnoha(a)redhat.com>
CommitterDate: Tue Mar 19 15:41:34 2013 +0100
WHATS_NEW: vgextend and PV with 0 MDAs
---
WHATS_NEW | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/WHATS_NEW b/WHATS_NEW
index 68a7d03..a35c09b 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
Version 2.02.99 -
===================================
+ Fix vgextend to not allow a PV with 0 MDAs to be used while already in a VG.
Move update_pool_params() from /tools to /lib for better reuse.
Give precedence to EMC power2 devices with duplicate PVIDs.
Add --validate option to lvm dumpconfig to validate current config on demand.
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=ea36d0501e0ede34…
Commit: ea36d0501e0ede347f251f6cda20903ad13f3700
Parent: 7e5e2dd4ee684137485a86c00e8ff29e7f169e14
Author: Peter Rajnoha <prajnoha(a)redhat.com>
AuthorDate: Tue Mar 19 14:17:53 2013 +0100
Committer: Peter Rajnoha <prajnoha(a)redhat.com>
CommitterDate: Tue Mar 19 14:57:36 2013 +0100
cleanup: remove unused 'pv_by_path' fn
The pv_by_path might be also dangerous to use as it does not
count with any other metadata areas but the ones found on the PV
itself. If metadata was not found on the PV referenced by the path,
it returned no PV though it might have been referenced by metadata
elsewhere (on other PVs...).
---
lib/metadata/metadata-exported.h | 3 +--
lib/metadata/metadata.c | 16 ----------------
lib/metadata/metadata.h | 1 -
3 files changed, 1 insertions(+), 19 deletions(-)
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index f218f47..df21759 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -522,8 +522,7 @@ int remove_lvs_in_vg(struct cmd_context *cmd,
/*
* free_pv_fid() must be called on every struct physical_volume allocated
- * by pv_create, pv_read, find_pv_by_name or pv_by_path to free it when
- * no longer required.
+ * by pv_create, pv_read, find_pv_by_name or to free it when no longer required.
*/
void free_pv_fid(struct physical_volume *pv);
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 775e40d..a913597 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -4524,19 +4524,3 @@ char *tags_format_and_copy(struct dm_pool *mem, const struct dm_list *tags)
}
return dm_pool_end_object(mem);
}
-
-/**
- * pv_by_path - Given a device path return a PV handle if it is a PV
- * @cmd - handle to the LVM command instance
- * @pv_name - device path to read for the PV
- *
- * Returns:
- * NULL - device path does not contain a valid PV
- * non-NULL - PV handle corresponding to device path
- *
- * FIXME: merge with find_pv_by_name ?
- */
-struct physical_volume *pv_by_path(struct cmd_context *cmd, const char *pv_name)
-{
- return _pv_read(cmd, cmd->mem, pv_name, NULL, 1, 0);
-}
diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h
index f7b3409..0c13a97 100644
--- a/lib/metadata/metadata.h
+++ b/lib/metadata/metadata.h
@@ -483,7 +483,6 @@ struct id pv_id(const struct physical_volume *pv);
const struct format_type *pv_format_type(const struct physical_volume *pv);
struct id pv_vgid(const struct physical_volume *pv);
-struct physical_volume *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, struct pvcreate_params *pp);
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=7e5e2dd4ee684137…
Commit: 7e5e2dd4ee684137485a86c00e8ff29e7f169e14
Parent: 59878d0129f63c3e8c1c4bc9715a400dc8fa808a
Author: Peter Rajnoha <prajnoha(a)redhat.com>
AuthorDate: Tue Mar 19 14:13:44 2013 +0100
Committer: Peter Rajnoha <prajnoha(a)redhat.com>
CommitterDate: Tue Mar 19 14:57:36 2013 +0100
vgextend: do not allow PV with 0 MDAs to be added while already in a VG
If extending a VG and including a PV with 0 MDAs that was already
a part of a VG, the vgextend allowed that PV to be added and we
ended up *with one PV in two VGs*!
The vgextend code used the 'pv_by_path' fn that returned a PV for
a given path. However, when the PV did not have any metadata areas,
the fn just returned a PV without any reference to existing VG.
Consequently, any checks for the existing VG failed.
[0] raw/~ # pvcreate --metadatacopies 0 /dev/sda
Physical volume "/dev/sda" successfully created
[0] raw/~ # pvcreate --metadatacopies 1 /dev/sdb
Physical volume "/dev/sdb" successfully created
[0] raw/~ # vgcreate vg1 /dev/sda /dev/sdb
Volume group "vg1" successfully created
[0] raw/~ # pvcreate --metadatacopies 1 /dev/sdc
Physical volume "/dev/sdc" successfully created
[0] raw/~ # vgcreate vg2 /dev/sdc
Volume group "vg2" successfully created
Before this patch (incorrect):
[0] raw/~ # vgextend vg2 /dev/sda
Volume group "vg2" successfully extended
With this patch (correct):
[0] raw/~ # vgextend vg2 /dev/sda
Physical volume '/dev/sda' is already in volume group 'vg1'
Unable to add physical volume '/dev/sda' to volume group 'vg2'.
---
lib/metadata/metadata.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index fdf47db..775e40d 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -629,7 +629,7 @@ static int vg_extend_single_pv(struct volume_group *vg, char *pv_name,
{
struct physical_volume *pv;
- if (!(pv = pv_by_path(vg->fid->fmt->cmd, pv_name)))
+ if (!(pv = find_pv_by_name(vg->cmd, pv_name, 1)))
stack;
if (!pv && !pp) {
log_error("%s not identified as an existing "
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=ea36d0501e0ede34…
Commit: ea36d0501e0ede347f251f6cda20903ad13f3700
Parent: 7e5e2dd4ee684137485a86c00e8ff29e7f169e14
Author: Peter Rajnoha <prajnoha(a)redhat.com>
AuthorDate: Tue Mar 19 14:17:53 2013 +0100
Committer: Peter Rajnoha <prajnoha(a)redhat.com>
CommitterDate: Tue Mar 19 14:57:36 2013 +0100
cleanup: remove unused 'pv_by_path' fn
The pv_by_path might be also dangerous to use as it does not
count with any other metadata areas but the ones found on the PV
itself. If metadata was not found on the PV referenced by the path,
it returned no PV though it might have been referenced by metadata
elsewhere (on other PVs...).
---
lib/metadata/metadata-exported.h | 3 +--
lib/metadata/metadata.c | 16 ----------------
lib/metadata/metadata.h | 1 -
3 files changed, 1 insertions(+), 19 deletions(-)
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index f218f47..df21759 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -522,8 +522,7 @@ int remove_lvs_in_vg(struct cmd_context *cmd,
/*
* free_pv_fid() must be called on every struct physical_volume allocated
- * by pv_create, pv_read, find_pv_by_name or pv_by_path to free it when
- * no longer required.
+ * by pv_create, pv_read, find_pv_by_name or to free it when no longer required.
*/
void free_pv_fid(struct physical_volume *pv);
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 775e40d..a913597 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -4524,19 +4524,3 @@ char *tags_format_and_copy(struct dm_pool *mem, const struct dm_list *tags)
}
return dm_pool_end_object(mem);
}
-
-/**
- * pv_by_path - Given a device path return a PV handle if it is a PV
- * @cmd - handle to the LVM command instance
- * @pv_name - device path to read for the PV
- *
- * Returns:
- * NULL - device path does not contain a valid PV
- * non-NULL - PV handle corresponding to device path
- *
- * FIXME: merge with find_pv_by_name ?
- */
-struct physical_volume *pv_by_path(struct cmd_context *cmd, const char *pv_name)
-{
- return _pv_read(cmd, cmd->mem, pv_name, NULL, 1, 0);
-}
diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h
index f7b3409..0c13a97 100644
--- a/lib/metadata/metadata.h
+++ b/lib/metadata/metadata.h
@@ -483,7 +483,6 @@ struct id pv_id(const struct physical_volume *pv);
const struct format_type *pv_format_type(const struct physical_volume *pv);
struct id pv_vgid(const struct physical_volume *pv);
-struct physical_volume *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, struct pvcreate_params *pp);
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=7e5e2dd4ee684137…
Commit: 7e5e2dd4ee684137485a86c00e8ff29e7f169e14
Parent: 59878d0129f63c3e8c1c4bc9715a400dc8fa808a
Author: Peter Rajnoha <prajnoha(a)redhat.com>
AuthorDate: Tue Mar 19 14:13:44 2013 +0100
Committer: Peter Rajnoha <prajnoha(a)redhat.com>
CommitterDate: Tue Mar 19 14:57:36 2013 +0100
vgextend: do not allow PV with 0 MDAs to be added while already in a VG
If extending a VG and including a PV with 0 MDAs that was already
a part of a VG, the vgextend allowed that PV to be added and we
ended up *with one PV in two VGs*!
The vgextend code used the 'pv_by_path' fn that returned a PV for
a given path. However, when the PV did not have any metadata areas,
the fn just returned a PV without any reference to existing VG.
Consequently, any checks for the existing VG failed.
[0] raw/~ # pvcreate --metadatacopies 0 /dev/sda
Physical volume "/dev/sda" successfully created
[0] raw/~ # pvcreate --metadatacopies 1 /dev/sdb
Physical volume "/dev/sdb" successfully created
[0] raw/~ # vgcreate vg1 /dev/sda /dev/sdb
Volume group "vg1" successfully created
[0] raw/~ # pvcreate --metadatacopies 1 /dev/sdc
Physical volume "/dev/sdc" successfully created
[0] raw/~ # vgcreate vg2 /dev/sdc
Volume group "vg2" successfully created
Before this patch (incorrect):
[0] raw/~ # vgextend vg2 /dev/sda
Volume group "vg2" successfully extended
With this patch (correct):
[0] raw/~ # vgextend vg2 /dev/sda
Physical volume '/dev/sda' is already in volume group 'vg1'
Unable to add physical volume '/dev/sda' to volume group 'vg2'.
---
lib/metadata/metadata.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index fdf47db..775e40d 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -629,7 +629,7 @@ static int vg_extend_single_pv(struct volume_group *vg, char *pv_name,
{
struct physical_volume *pv;
- if (!(pv = pv_by_path(vg->fid->fmt->cmd, pv_name)))
+ if (!(pv = find_pv_by_name(vg->cmd, pv_name, 1)))
stack;
if (!pv && !pp) {
log_error("%s not identified as an existing "