Re: [PATCH 12/28] config: move config section code to it's own file

[Dave Chinner <david@fromorbit.com>]

On Fri, May 09, 2025 at 11:39:30AM +0530, Nirjhar Roy wrote:
> On Thu, 2025-04-17 at 13:00 +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Move the config section parsing, checking and setup code from
> > common/config to common/config-section so that it can be included
> > directly in contexts where the rest of common/config is not needed.
> This looks okay to me. Just a couple of nits comments below.
> 
> Reviewed-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
....
> > diff --git a/common/config-sections b/common/config-sections
> > new file mode 100644
> > index 000000000..69a03375a
> > --- /dev/null
> > +++ b/common/config-sections
> > @@ -0,0 +1,390 @@
> > +##/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2000-2003,2006 Silicon Graphics, Inc.  All Rights Reserved.
> > +# Copyright (c) 2025 Red Hat, Inc.  All Rights Reserved.
> > +#
> > +# Config section parsing and setup code
> Since we want to facilitate sourcing of this file independently, should
> we at least mention in the comments some of the dependant files this
> file needs to be sourced? For example, this file uses _exit() - so
> usage of this file requires common/exit to be sourced?

The high level code should have already sourced that file.

> > +
> > +_check_device()
> > +{
> > +	local name=$1
> > +	local dev_needed=$2
> > +	local dev=$3
> > +
> > +	if [ -z "$dev" ]; then
> > +		if [ "$dev_needed" == "required" ]; then
> > +			_fatal "common/config: $name is required but not defined!"
> > +		fi
> > +		return 0
> > +	fi
> > +
> > +	if [ -b "$dev" ] || ( echo $dev | grep -qE ":|//" ); then
> > +		# block device or a network url
> > +		return 0
> > +	fi
> > +
> > +	case "$FSTYP" in
> > +	9p|fuse|tmpfs|virtiofs|afs)
> > +		# 9p, fuse, virtiofs and afs mount tags are just plain strings,
> > +		# so anything is allowed tmpfs doesn't use mount source, ignore
> > +		;;
> > +	ceph)
> > +		# ceph has two different possible syntaxes for mount devices. The
> > +		# network URL check above catches the legacy syntax. Check for the
> > +		# new-style syntax here.
> > +		if ( echo $dev | grep -qEv "=/" ); then
> > +			_fatal "common/config: $name ($dev) is not a valid ceph mount string"
> > +		fi
> > +		;;
> > +	overlay)
> > +		if [ ! -d "$dev" ]; then
> > +			_fatal "common/config: $name ($dev) is not a directory for overlay"
> > +		fi
> > +		;;
> > +	ubifs)
> > +		if [ ! -c "$dev" ]; then
> > +			_fatal "common/config: $name ($dev) is not a character device"
> > +		fi
> > +		;;
> > +	ceph-fuse)
> > +		;;
> > +	*)
> > +		_fatal "common/config: $name ($dev) is not a block device or a network filesystem"
> Nit: 80 chars limit exceed.

There are many of these in the code I moved, and there are many,
many lines that exceed 80 columns all through fstests. I don't think
this needs fixing.

Regardless, this patch is moving code from A to B, so I'm trying to
avoid mixing in formatting or bug fixes that would otherwise be
impossible to spot in the diff...

-Dave.

-- 
Dave Chinner
david@fromorbit.com