From df2e0dc75101a5700475f87ac22f1acf4938b1f1 Mon Sep 17 00:00:00 2001 From: Alasdair Kergon Date: Mon, 15 Oct 2001 18:39:40 +0000 Subject: [PATCH] More vgcreate error trapping --- lib/format1/format1.c | 35 ++++++------ lib/metadata/metadata.c | 124 +++++++++++++++++++++++++++------------- lib/metadata/metadata.h | 3 + tools/pvcreate.c | 45 +++++++-------- tools/toollib.c | 11 ---- tools/toollib.h | 2 - tools/vgcreate.c | 40 +++++++++++-- 7 files changed, 157 insertions(+), 103 deletions(-) diff --git a/lib/format1/format1.c b/lib/format1/format1.c index 2c53df913..1f3afe729 100644 --- a/lib/format1/format1.c +++ b/lib/format1/format1.c @@ -37,27 +37,21 @@ static struct volume_group *_build_vg(struct pool *mem, struct list_head *pvs) struct volume_group *vg = pool_alloc(mem, sizeof(*vg)); struct disk_list *dl; - if (list_empty(pvs)) { - stack; - return NULL; - } + if (!vg) + goto bad; + + if (list_empty(pvs)) + goto bad; dl = list_entry(pvs->next, struct disk_list, list); - if (!vg) { - stack; - return NULL; - } - memset(vg, 0, sizeof(*vg)); INIT_LIST_HEAD(&vg->pvs); INIT_LIST_HEAD(&vg->lvs); - if (!_check_vgs(pvs)) { - stack; - return NULL; - } + if (!_check_vgs(pvs)) + goto bad; if (!import_vg(mem, vg, dl)) goto bad; @@ -130,6 +124,7 @@ static struct disk_list *_flatten_pv(struct pool *mem, struct volume_group *vg, !export_lvs(dl, vg, pv, prefix) || !calculate_layout(dl)) { stack; + pool_free(mem, dl); return NULL; } @@ -239,6 +234,7 @@ static struct list_head *_get_pvs(struct io_space *is) if (!(results = pool_alloc(is->mem, sizeof(*results)))) { stack; + pool_destroy(mem); return NULL; } @@ -259,8 +255,8 @@ static struct list_head *_get_pvs(struct io_space *is) return results; bad: - pool_destroy(mem); pool_free(mem, results); + pool_destroy(mem); return NULL; } @@ -359,7 +355,7 @@ static int _pv_write(struct io_space *is, struct physical_volume *pv) if (!(dl = pool_alloc(mem, sizeof(*dl)))) { stack; - return 0; + goto bad; } dl->mem = mem; dl->dev = pv->dev; @@ -386,11 +382,11 @@ static int _pv_write(struct io_space *is, struct physical_volume *pv) int _vg_setup(struct io_space *is, struct volume_group *vg) { /* just check max_pv and max_lv */ - if (vg->max_lv > MAX_LV) - vg->max_lv = MAX_LV; + if (vg->max_lv >= MAX_LV) + vg->max_lv = MAX_LV - 1; - if (vg->max_pv > MAX_PV) - vg->max_pv = MAX_PV; + if (vg->max_pv >= MAX_PV) + vg->max_pv = MAX_PV - 1; return 1; } @@ -398,6 +394,7 @@ int _vg_setup(struct io_space *is, struct volume_group *vg) void _destroy(struct io_space *ios) { dbg_free(ios->prefix); + pool_destroy(ios->mem); dbg_free(ios); } diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 2c2eadf34..d2642e4c1 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -13,30 +13,44 @@ #include int _add_pv_to_vg(struct io_space *ios, struct volume_group *vg, - const char *name) + const char *pv_name) { - struct pv_list *pvl = pool_alloc(ios->mem, sizeof(*pvl)); - struct physical_volume *pv = &pvl->pv; + struct pv_list *pvl; + struct physical_volume *pv; - if (!pv) { - log_error("pv_list allocation for '%s' failed", name); + log_verbose("Adding physical volume '%s' to volume group '%s'", + pv_name, vg->name); + + if (!(pvl = pool_alloc(ios->mem, sizeof (*pvl)))) { + log_error("pv_list allocation for '%s' failed", pv_name); return 0; } - memset(pv, 0, sizeof(*pv)); - - if (!(pv->dev = dev_cache_get(name, ios->filter))) { - log_error("Physical volume '%s' not found.", name); + if (!(pv = ios->pv_read(ios, pv_name))) { + log_error("Failed to read existing physical volume '%s'", + pv_name); return 0; } + if (*pv->vg_name) { + log_error("Physical volume '%s' is already in volume group " + "'%s'", pv_name, pv->vg_name); + return 0; + } + + /* FIXME For LVM2, set on PV creation instead of here? */ + pv->status |= ALLOCATED_PV; + + /* FIXME check this */ + pv->exported = NULL; + if (!(pv->vg_name = pool_strdup(ios->mem, vg->name))) { - log_error("vg->name allocation failed for '%s'", name); + log_error("vg->name allocation failed for '%s'", pv_name); return 0; } - pv->exported = NULL; - pv->status = ACTIVE; + /* FIXME Tie this to activation or not? */ + pv->status |= ACTIVE; if (!dev_get_size(pv->dev, &pv->size)) { stack; @@ -55,91 +69,110 @@ int _add_pv_to_vg(struct io_space *ios, struct volume_group *vg, pv->pe_allocated = 0; if (!ios->pv_setup(ios, pv, vg)) { - log_error("Format specific setup of physical volume '%s' " - "failed.", name); + log_debug("Format specific setup of physical volume '%s' " + "failed.", pv_name); return 0; } + if (find_pv_in_vg(vg, pv_name)) { + log_error("Physical volume '%s' listed more than once.", + pv_name); + return 0; + } + + if (vg->pv_count == vg->max_pv) { + log_error("No space for '%s' - volume group '%s' " + "holds max %d physical volume(s).", pv_name, + vg->name, vg->max_pv); + return 0; + } + + memcpy(&pvl->pv, pv, sizeof (struct physical_volume)); + list_add(&pvl->list, &vg->pvs); vg->pv_count++; return 1; } -struct volume_group *vg_create(struct io_space *ios, const char *name, +struct volume_group *vg_create(struct io_space *ios, const char *vg_name, uint64_t extent_size, int max_pv, int max_lv, int pv_count, char **pv_names) { int i; struct volume_group *vg; - if (!(vg = pool_alloc(ios->mem, sizeof(*vg)))) { + if (!(vg = pool_alloc(ios->mem, sizeof (*vg)))) { stack; return NULL; } /* is this vg name already in use ? */ - if (ios->vg_read(ios, name)) { - log_err("A volume group called '%s' already exists.", name); + if (ios->vg_read(ios, vg_name)) { + log_err("A volume group called '%s' already exists.", vg_name); goto bad; } if (!id_create(&vg->id)) { - log_err("Couldn't create uuid for volume group '%s'.", name); + log_err("Couldn't create uuid for volume group '%s'.", vg_name); goto bad; } - if (!(vg->name = pool_strdup(ios->mem, name))) { + /* Strip prefix if present */ + if (!strncmp(vg_name, ios->prefix, strlen(ios->prefix))) + vg_name += strlen(ios->prefix); + + if (!(vg->name = pool_strdup(ios->mem, vg_name))) { stack; goto bad; } - vg->status = (ACTIVE | EXTENDABLE_VG | LVM_READ | LVM_WRITE); + vg->status = (ACTIVE | EXTENDABLE_VG | LVM_READ | LVM_WRITE); - vg->extent_size = extent_size; - vg->extent_count = 0; - vg->free_count = 0; + vg->extent_size = extent_size; + vg->extent_count = 0; + vg->free_count = 0; - vg->max_lv = max_lv; - vg->max_pv = max_pv; + vg->max_lv = max_lv; + vg->max_pv = max_pv; - vg->pv_count = 0; + vg->pv_count = 0; INIT_LIST_HEAD(&vg->pvs); - vg->lv_count = 0; + vg->lv_count = 0; INIT_LIST_HEAD(&vg->lvs); if (!ios->vg_setup(ios, vg)) { - log_err("Format specific setup of volume group '%s' failed.", - name); + log_error("Format specific setup of volume group '%s' failed.", + vg_name); goto bad; } /* attach the pv's */ for (i = 0; i < pv_count; i++) if (!_add_pv_to_vg(ios, vg, pv_names[i])) { - log_err("Unable to add physical volume '%s' to " - "volume group '%s'.", pv_names[i], name); + log_error("Unable to add physical volume '%s' to " + "volume group '%s'.", pv_names[i], vg_name); goto bad; } return vg; - bad: + bad: pool_free(ios->mem, vg); return NULL; } struct physical_volume *pv_create(struct io_space *ios, const char *name) { - struct physical_volume *pv = pool_alloc(ios->mem, sizeof(*pv)); + struct physical_volume *pv = pool_alloc(ios->mem, sizeof (*pv)); if (!pv) { stack; return NULL; } - id_create(&pv->id); + id_create(&pv->id); if (!(pv->dev = dev_cache_get(name, ios->filter))) { log_err("Couldn't find device '%s'", name); goto bad; @@ -152,23 +185,32 @@ struct physical_volume *pv_create(struct io_space *ios, const char *name) *pv->vg_name = 0; pv->exported = NULL; - pv->status = 0; + pv->status = 0; if (!dev_get_size(pv->dev, &pv->size)) { log_err("Couldn't get size of device '%s'", name); goto bad; } - pv->pe_size = 0; - pv->pe_start = 0; - pv->pe_count = 0; - pv->pe_allocated = 0; + pv->pe_size = 0; + pv->pe_start = 0; + pv->pe_count = 0; + pv->pe_allocated = 0; return pv; - bad: + bad: pool_free(ios->mem, pv); return NULL; } +struct list_head *find_pv_in_vg(struct volume_group *vg, const char *pv_name) +{ + struct list_head *pvh; + list_for_each(pvh, &vg->pvs) { + if (!strcmp(list_entry(pvh, struct pv_list, list)->pv.dev->name, + pv_name)) return pvh; + } + return NULL; +} diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h index cd4bbde05..dcc015a20 100644 --- a/lib/metadata/metadata.h +++ b/lib/metadata/metadata.h @@ -239,4 +239,7 @@ struct volume_group *vg_find(const char *lv_name); /* Find an LV within a given VG */ struct logical_volume *lv_find(struct volume_group *vg, const char *lv_name); +/* Find a PV within a given VG */ +struct list_head *find_pv_in_vg(struct volume_group *vg, const char *pv_name); + #endif diff --git a/tools/pvcreate.c b/tools/pvcreate.c index e5c29b539..58573a8f1 100644 --- a/tools/pvcreate.c +++ b/tools/pvcreate.c @@ -13,7 +13,7 @@ const char _really_init[] = * See if we may pvcreate on this device. * 0 indicates we may not. */ -static int _check(const char *name) +static int pvcreate_check(const char *name) { struct physical_volume *pv; @@ -36,6 +36,11 @@ static int _check(const char *name) return 0; } + if (pv->status & ACTIVE) { + log_error("Can't create on active physical volume %s", name); + return 0; + } + /* we must have -ff to overwrite a non orphan */ if (arg_count(force_ARG) < 2) { log_error("Can't initialize physical volume %s of " @@ -50,42 +55,38 @@ static int _check(const char *name) return 0; } - if (pv->status & ACTIVE) { - log_error("Can't create on active physical volume %s", name); - return 0; + if (arg_count(force_ARG)) { + log_print("WARNING: Forcing physical volume creation on " + "%s%s%s", name, + pv->vg_name[0] ? " of volume group " : "", + pv->vg_name[0] ? pv->vg_name : ""); } return 1; } -static void pvcreate_single(const char *name) +static void pvcreate_single(const char *pv_name) { struct physical_volume *pv; - if (arg_count(force_ARG)) { - log_print("WARNING: forcing physical volume creation on %s", - name); + if (!pvcreate_check(pv_name)) + return; - if (pv->vg_name[0]) - log_print(" of volume group %s", pv->vg_name); - log_print(" "); - } - - if (!(pv = pv_create(ios, name))) { - log_err("Failed to setup physical volume %s", name); + if (!(pv = pv_create(ios, pv_name))) { + log_err("Failed to setup physical volume %s", pv_name); return; } + log_verbose("Set up physical volume for %s with %llu sectors", - name, pv->size); + pv_name, pv->size); - - log_verbose("Writing physical volume data to disk %s", name); + log_verbose("Writing physical volume data to disk %s", pv_name); if (!(ios->pv_write(ios, pv))) { - log_error("Failed to write physical volume %s", name); + log_error("Failed to write physical volume %s", pv_name); return; } - log_print("Physical volume %s successfully created", name); + log_print("Physical volume %s successfully created", pv_name); } int pvcreate(int argc, char **argv) @@ -103,10 +104,6 @@ int pvcreate(int argc, char **argv) } for (i = 0; i < argc; i++) { - - if (!_check(argv[i])) - continue; - pvcreate_single(argv[i]); pool_empty(ios->mem); } diff --git a/tools/toollib.c b/tools/toollib.c index b96a64c11..a5f2adc52 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -166,14 +166,3 @@ int is_valid_chars(char *n) return 1; } -struct list_head *find_pv_in_vg(struct volume_group *vg, const char *pv_name) -{ - struct list_head *pvh; - list_for_each(pvh, &vg->pvs) { - if (!strcmp(list_entry(pvh, struct pv_list, list)->pv.dev->name, - pv_name)) return pvh; - } - - return NULL; - -} diff --git a/tools/toollib.h b/tools/toollib.h index b3aeca4ab..b44394317 100644 --- a/tools/toollib.h +++ b/tools/toollib.h @@ -34,6 +34,4 @@ int process_each_pv(int argc, char **argv, struct volume_group *vg, int is_valid_chars(char *n); -struct list_head *find_pv_in_vg(struct volume_group *vg, const char *pv_name); - #endif diff --git a/tools/vgcreate.c b/tools/vgcreate.c index a4093abd3..5ff143bd2 100644 --- a/tools/vgcreate.c +++ b/tools/vgcreate.c @@ -6,9 +6,9 @@ #include "tools.h" -/* FIXME Config file? */ -#define DEFAULT_PV 128 -#define DEFAULT_LV 128 +/* FIXME From config file? */ +#define DEFAULT_PV 255 +#define DEFAULT_LV 255 #define DEFAULT_EXTENT 8192 int vgcreate(int argc, char **argv) @@ -33,13 +33,41 @@ int vgcreate(int argc, char **argv) max_pv = arg_int_value(maxphysicalvolumes_ARG, DEFAULT_PV); extent_size = arg_int_value(physicalextentsize_ARG, DEFAULT_EXTENT); + if (max_lv < 1) { + log_error("maxlogicalvolumes too low"); + return EINVALID_CMD_LINE; + } + + if (max_pv < 1) { + log_error("maxphysicalvolumes too low"); + return EINVALID_CMD_LINE; + } + + /* Strip prefix if present */ + if (!strncmp(vg_name, ios->prefix, strlen(ios->prefix))) + vg_name += strlen(ios->prefix); + + if (!is_valid_chars(vg_name)) { + log_error("New volume group name '%s' has invalid characters", + vg_name); + return ECMD_FAILED; + } + /* create the new vg */ - if (!vg_create(ios, vg_name, extent_size, max_pv, max_lv, - argc - 1, argv + 1)) + if (!(vg = vg_create(ios, vg_name, extent_size, max_pv, max_lv, + argc - 1, argv + 1))) return ECMD_FAILED; + if (max_lv != vg->max_lv) + log_error("Warning: Setting maxlogicalvolumes to %d", + vg->max_lv); + + if (max_pv != vg->max_pv) + log_error("Warning: Setting maxphysicalvolumes to %d", + vg->max_pv); + /* store vg on disk(s) */ - if (ios->vg_write(ios, vg)) + if (!ios->vg_write(ios, vg)) return ECMD_FAILED; /* FIXME Create /dev/vg */