[LU-8530] Bad configure code snuck into ldiskfs LDISKFS_LINUX_SERIES macro

• Status: Resolved
• Project: Lustre
• Type: Bug
• Priority: Minor
• Reporter: Christopher Morrone
• Assignee: Yang Sheng
• Resolution: Incomplete
• Severity: 3

Description

This bad code was snuck into a "kernel update" patch, in commit 6c9384678f5555f87bcc7311bb5bf73b1e4bf4e4.

case x$LDISKFS_SERIES in
	x)			# not set
		;;
	*.series)		# set externally
		;;
	*) LDISKFS_SERIES=
esac
AS_IF([test -z "$LDISKFS_SERIES"], [

Not even a comment in the commit message to explain it's existence, so I'm left to guess at what the author intended. But whatever was intended, this isn't the right way to go about it.

I'll submit a patch to revert this section.

Comments

[Gerrit Updater]

Christopher J. Morrone ([e-mail]) uploaded a new patch: http://review.whamcloud.com/22086
Subject: LU-8530 config: Remove bad side-effect configuration
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: c58b7654a31d4bc82d47205d97173c288c1e2b32

[Peter Jones]

Yang Sheng

Could you please review this patch?

Thanks

Peter

[Bob Glossman (Inactive)]

as the author that put this in I confess to it not being desirable. However it was the only expeditious method I could come up with to force the correct selection of ldiskfs series on sles12 builds in our test framework. With only a small set of sles12 build servers with varied versions of sles12 and distinct and different series for sles12 vs sles12sp1 needed a quick and dirty way to impose a specific selection regardless of the underlying running sles12 version.

[Christopher Morrone]

I don't understand that explanation. The ldiskfs series detection code just determines which kernel to apply a series to from whatever kernel Lustre decides to compile against as decided in the LB_LINUX_RELEASE. I can't imagine that you would want to select a series file for a kernel that is different than the kernel that Lustre is compiling against.

[Bob Glossman (Inactive)]

LB_LINUX_RELEASE auto detects the kernel version being compiled against 100% correctly only in the case of RHEL. On SLES, and particularly on SLES12, it guesses the kernel version based oh the version running on the builder, not the version being compiled for. Needed a way to override that selection. It became an acute problem trying to build lustre for sles12sp1 on builder nodes installed with base sles12.

[Christopher Morrone]

I'm not sure that is quite right. Looking a little more closely, it works like this:

LB_LINUX_RELEASE just needs to decide if the platform is SUSE. This pretty much has to work, or lots of things will go wrong in packaging.

LDISKFS_LINUX_SERIES, if SUSE_KERNEL=yes, just uses $LINUXRELEASE to decide which series file to apply.

LINUXRELEASE is set in LB_LINUX_UTSRELEASE based off of $LINUX_OBJ. I don't think $LINUX_OBJ can be wrong, or the whole build will probably fail.

So where exactly do you see it failing?

Are you seeing the "Cannot determine Linux kernel version." error message?

[Bob Glossman (Inactive)]

in LDISKFS_LINUX_SERIES there is the following for the SUSE case:

        [LDISKFS_SERIES="3.0-sles11.series"],[
                PLEV=$(grep PATCHLEVEL /etc/SuSE-release | sed -e 's/.*= *//')
                case $PLEV in
                2) LDISKFS_SERIES="3.0-sles11.series"
                        ;;
                3|4) LDISKFS_SERIES="3.0-sles11sp3.series"
                        ;;
                esac
        ])],[LDISKFS_SERIES="3.12-sles12.series"],[
                PLEV=$(grep PATCHLEVEL /etc/SuSE-release | sed -e 's/.*= *//')
                case $PLEV in
                1) LDISKFS_SERIES="3.12-sles12sp1.series"
                        ;;
                *) LDISKFS_SERIES="3.12-sles12.series"
                        ;;
                sac

thus for recent sles11 and all sles12 it chooses the ldiskfs series based on the content of /etc/SuSE-release. This comes from the installed SLES on the builder, not the kernel version lustre is being compiled against.

[Yang Sheng]

Hi, Christopher,

Anyway, Your patch broken ability that set ldiskfs series directly. Sometime we need it in our build system.

[Christopher Morrone]

Anyway, Your patch broken ability that set ldiskfs series directly. Sometime we need it in our build system.

Fine, then do it correctly. Add a configure option. This method is not acceptable.

And frankly, I question whether it is really necessary in your build system, or whether folks just don't understand how to do it correctly. If you give me more details about the necessary situation I might be able add some additional insight.

But if you really need a configure option, the correct way to do it is to actually add an option to configure. Sneaking it in through environment variables is a recipe for an unmaintainable build system.

[Christopher Morrone]

in LDISKFS_LINUX_SERIES there is the following for the SUSE case:

Ah, I see. I don't imagine it would be too difficult to correct that. It should be obvious now, I hope, that just looking at the host distro is not going to be a correct method of identifying which patch series should be applied to the kernel against which Lustre is compiling. The configuration logic should really be looking at the kernel in question instead.

[Bob Glossman (Inactive)]

I don't claim the selection rules for SUSE are 100% correct or even the proper way to do things. Looking back at history I gather that when we abandoned using kernel version directly for RHEL and switched to using RHEL_RELEASE_NO instead we looked for a way to divorce selections in SUSE from kernel version numbers at the same time. Selecting he series based on the SLES SP version of the install offered a way to do that. I believe it was intended as a heuristic rather than a perfectly correct method. It served well for that for years and only broke down at the time of the introduction of sles12sp1. would have been much better if we could have identified a direct analog of RHLE_RELEASE_NO for SLES, but we couldn't find one.

[Christopher Morrone]

I gather that when we abandoned using kernel version directly for RHEL and switched to using RHEL_RELEASE_NO instead

I think that was a mistake too. I already opened ticket LU-8533 on that topic.

It served well for that for years and only broke down at the time of the introduction of sles12sp1.

It probably wasn't really sles12sp1 that is the problem so much as lbuild's weird build environment.

would have been much better if we could have identified a direct analog of RHLE_RELEASE_NO for SLES, but we couldn't find one.

The newer sles kernel doesn't have a newer kernel version? In other words do major releases of SLES have the same $LINUXVERSION?