From mboxrd@z Thu Jan 1 00:00:00 1970 From: Petr Rockai Date: Fri, 28 Jan 2011 17:31:33 +0100 Subject: [PATCH 09/17] Add pv_initialise fn to format_handler interface. In-Reply-To: <1295867048-21558-10-git-send-email-prajnoha@redhat.com> (Peter Rajnoha's message of "Mon, 24 Jan 2011 12:04:00 +0100") References: <1295867048-21558-1-git-send-email-prajnoha@redhat.com> <1295867048-21558-10-git-send-email-prajnoha@redhat.com> Message-ID: <87oc71ouqi.fsf@twilight.int.mornfall.net.> List-Id: To: lvm-devel@redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Peter Rajnoha writes: > This is a split-off from the original pv_setup code, a simple refactor. > It makes the code easier to read and cleans up the interface a bit. So > now we will call pv_initialise when we initialise a PV only, creating a > brand new PV. It will also initialise a new format_instance to use later on. OK, although the parameter lists are huge -- calls to these functions are plain unreadable. Please consider doing something about it... (this is a widespread problem in LVM, I know, but that's no excuse to keep not fixing it). Yours, Petr > Signed-off-by: Peter Rajnoha Reviewed-by: Petr Rockai > --- > lib/format1/format1.c | 35 ++++++++++++++------ > lib/format_pool/format_pool.c | 14 ++++++++ > lib/format_text/format-text.c | 71 +++++++++++++++++++++++++++++++++++++++++ > lib/metadata/metadata.h | 13 +++++++ > 4 files changed, 123 insertions(+), 10 deletions(-) > > diff --git a/lib/format1/format1.c b/lib/format1/format1.c > index 4f66cdc..cb17ef5 100644 > --- a/lib/format1/format1.c > +++ b/lib/format1/format1.c > @@ -361,16 +361,15 @@ static int _format1_pv_read(const struct format_type *fmt, const char *pv_name, > return r; > } > > -static int _format1_pv_setup(const struct format_type *fmt, > - uint64_t pe_start, uint32_t extent_count, > - uint32_t extent_size, > - unsigned long data_alignment __attribute__((unused)), > - unsigned long data_alignment_offset __attribute__((unused)), > - int pvmetadatacopies __attribute__((unused)), > - uint64_t pvmetadatasize __attribute__((unused)), > - unsigned metadataignore __attribute__((unused)), > - struct dm_list *mdas __attribute__((unused)), > - struct physical_volume *pv, struct volume_group *vg __attribute__((unused))) > +static int _format1_pv_initialise(const struct format_type * fmt, > + int64_t label_sector __attribute__((unused)), > + uint64_t pe_start, > + int pe_start_locked __attribute__((unused)), > + uint32_t extent_count, > + uint32_t extent_size, > + unsigned long data_alignment __attribute__((unused)), > + unsigned long data_alignment_offset __attribute__((unused)), > + struct physical_volume * pv) > { > if (pv->size > MAX_PV_SIZE) > pv->size--; > @@ -400,6 +399,21 @@ static int _format1_pv_setup(const struct format_type *fmt, > return 1; > } > > +static int _format1_pv_setup(const struct format_type *fmt, > + uint64_t pe_start, uint32_t extent_count, > + uint32_t extent_size, > + unsigned long data_alignment __attribute__((unused)), > + unsigned long data_alignment_offset __attribute__((unused)), > + int pvmetadatacopies __attribute__((unused)), > + uint64_t pvmetadatasize __attribute__((unused)), > + unsigned metadataignore __attribute__((unused)), > + struct dm_list *mdas __attribute__((unused)), > + struct physical_volume *pv, > + struct volume_group *vg __attribute__((unused))) > +{ > + return _format1_pv_initialise(fmt, -1, 0, 0, 0, vg->extent_size, 0, 0, pv); > +} > + > static int _format1_lv_setup(struct format_instance *fid, struct logical_volume *lv) > { > uint64_t max_size = UINT_MAX; > @@ -563,6 +577,7 @@ static void _format1_destroy(struct format_type *fmt) > > static struct format_handler _format1_ops = { > .pv_read = _format1_pv_read, > + .pv_initialise = _format1_pv_initialise, > .pv_setup = _format1_pv_setup, > .pv_write = _format1_pv_write, > .lv_setup = _format1_lv_setup, > diff --git a/lib/format_pool/format_pool.c b/lib/format_pool/format_pool.c > index 24ccfc2..83f474a 100644 > --- a/lib/format_pool/format_pool.c > +++ b/lib/format_pool/format_pool.c > @@ -188,6 +188,19 @@ out: > return NULL; > } > > +static int _pool_pv_initialise(const struct format_type *fmt __attribute__((unused)), > + int64_t label_sector __attribute__((unused)), > + uint64_t pe_start __attribute__((unused)), > + int pe_start_locked __attribute__((unused)), > + uint32_t extent_count __attribute__((unused)), > + uint32_t extent_size __attribute__((unused)), > + unsigned long data_alignment __attribute__((unused)), > + unsigned long data_alignment_offset __attribute__((unused)), > + struct physical_volume *pv __attribute__((unused))) > +{ > + return 1; > +} > + > static int _pool_pv_setup(const struct format_type *fmt __attribute__((unused)), > uint64_t pe_start __attribute__((unused)), > uint32_t extent_count __attribute__((unused)), > @@ -295,6 +308,7 @@ static void _pool_destroy(struct format_type *fmt) > /* *INDENT-OFF* */ > static struct format_handler _format_pool_ops = { > .pv_read = _pool_pv_read, > + .pv_initialise = _pool_pv_initialise, > .pv_setup = _pool_pv_setup, > .create_instance = _pool_create_instance, > .destroy_instance = _pool_destroy_instance, > diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c > index 99f691f..c6adedc 100644 > --- a/lib/format_text/format-text.c > +++ b/lib/format_text/format-text.c > @@ -1688,6 +1688,76 @@ static int _text_pv_read(const struct format_type *fmt, const char *pv_name, > return 1; > } > > +static int _text_pv_initialise(const struct format_type *fmt, > + const int64_t label_sector, > + uint64_t pe_start, > + int pe_start_locked, > + uint32_t extent_count, > + uint32_t extent_size, > + unsigned long data_alignment, > + unsigned long data_alignment_offset, > + struct physical_volume *pv) > +{ > + struct text_fid_pv_context *fid_pv_tc; > + > + /* > + * Try to keep the value of PE start set to a firm value if requested. > + * This is usefull when restoring existing PE start value (backups etc.). > + */ > + if (pe_start_locked) > + pv->pe_start = pe_start; > + > + if (!data_alignment) > + data_alignment = find_config_tree_int(pv->fmt->cmd, > + "devices/data_alignment", > + 0) * 2; > + > + if (set_pe_align(pv, data_alignment) != data_alignment && > + data_alignment) { > + log_error("%s: invalid data alignment of " > + "%lu sectors (requested %lu sectors)", > + pv_dev_name(pv), pv->pe_align, data_alignment); > + return 0; > + } > + > + if (set_pe_align_offset(pv, data_alignment_offset) != data_alignment_offset && > + data_alignment_offset) { > + log_error("%s: invalid data alignment offset of " > + "%lu sectors (requested %lu sectors)", > + pv_dev_name(pv), pv->pe_align_offset, data_alignment_offset); > + return 0; > + } > + > + if (pv->pe_align < pv->pe_align_offset) { > + log_error("%s: pe_align (%lu sectors) must not be less " > + "than pe_align_offset (%lu sectors)", > + pv_dev_name(pv), pv->pe_align, pv->pe_align_offset); > + return 0; > + } > + > + if (!pe_start_locked && pv->pe_start < pv->pe_align) > + pv->pe_start = pv->pe_align; > + > + if (extent_size) > + pv->pe_size = extent_size; > + > + if (extent_count) > + pv->pe_count = extent_count; > + > + if ((pv->pe_start + pv->pe_count * pv->pe_size - 1) > (pv->size << SECTOR_SHIFT)) { > + log_error("Physical extents end beyond end of device %s.", > + pv_dev_name(pv)); > + return 0; > + } > + > + if (label_sector != -1) { > + fid_pv_tc = (struct text_fid_pv_context *) pv->fid->private; > + fid_pv_tc->label_sector = label_sector; > + } > + > + return 1; > +} > + > static void _text_destroy_instance(struct format_instance *fid __attribute__((unused))) > { > } > @@ -2377,6 +2447,7 @@ void *create_text_context(struct cmd_context *cmd, const char *path, > static struct format_handler _text_handler = { > .scan = _text_scan, > .pv_read = _text_pv_read, > + .pv_initialise = _text_pv_initialise, > .pv_setup = _text_pv_setup, > .pv_add_metadata_area = _text_pv_add_metadata_area, > .pv_remove_metadata_area = _text_pv_remove_metadata_area, > diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h > index ed72551..ae8dd9a 100644 > --- a/lib/metadata/metadata.h > +++ b/lib/metadata/metadata.h > @@ -244,6 +244,19 @@ struct format_handler { > struct physical_volume * pv, int scan_label_only); > > /* > + * Initialise a new PV. > + */ > + int (*pv_initialise) (const struct format_type * fmt, > + int64_t label_sector, > + uint64_t pe_start, > + int pe_start_locked, > + uint32_t extent_count, > + uint32_t extent_size, > + unsigned long data_alignment, > + unsigned long data_alignment_offset, > + struct physical_volume * pv); > + > + /* > * Tweak an already filled out a pv ready for importing into a > * vg. eg. pe_count is format specific. > */