Re: [PATCH V2 07/23] metadump: Introduce struct metadump_ops

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jun 06, 2023 at 02:57:50PM +0530, Chandan Babu R wrote:
> We will need two sets of functions to implement two versions of metadump. This
> commit adds the definition for 'struct metadump_ops' to hold pointers to
> version specific metadump functions.
> 
> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> ---
>  db/metadump.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/db/metadump.c b/db/metadump.c
> index 91150664..266d3413 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -40,6 +40,30 @@ static const cmdinfo_t	metadump_cmd =
>  		N_("[-a] [-e] [-g] [-m max_extent] [-w] [-o] filename"),
>  		N_("dump metadata to a file"), metadump_help };
>  
> +struct metadump_ops {
> +	/*
> +	 * Initialize Metadump. This may perform actions such as
> +	 * 1. Allocating memory for structures required for dumping the
> +	 *    metadata.
> +	 * 2. Writing a header to the beginning of the metadump file.
> +	 */
> +	int (*init)(void);
> +	/*
> +	 * Write metadata to the metadump file along with the required ancillary
> +	 * data.
> +	 */
> +	int (*write)(enum typnm type, char *data, xfs_daddr_t off,
> +		int len);

Minor nits: const char *data (since the ->write function doesn't alter
the caller's buffer); and please document that @off and @len are both in
units of 512b blocks.

With that fixed,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> +	/*
> +	 * Flush any in-memory remanents of metadata to the metadump file.
> +	 */
> +	int (*end_write)(void);
> +	/*
> +	 * Free resources allocated during metadump process.
> +	 */
> +	void (*release)(void);
> +};
> +
>  static struct metadump {
>  	int			version;
>  	bool			show_progress;
> @@ -54,6 +78,7 @@ static struct metadump {
>  	xfs_ino_t		cur_ino;
>  	/* Metadump file */
>  	FILE			*outf;
> +	struct metadump_ops	*mdops;
>  	/* header + index + buffers */
>  	struct xfs_metablock	*metablock;
>  	__be64			*block_index;
> -- 
> 2.39.1
>