rename: adjust all mount points

[gahr]

Fixes #659

[yaazkal]

Thanks @gahr for the PR. We had an issue (#698) related to devfs rules that is fixed now. Maybe that could be the root cause. We would appreciate it if you could test again to check if this PR is still needed.

[gahr]

Hi @yaazkal , how is this related to devfs rules? This is about mount point mounted via the MOUNT command in Bastillefiles that are not in fstab.

[yaazkal]

Sorry @gahr that comment was intented for another PR which is now merged (multi tab in browser is bad for me). Let me take a look at your PR.

[yaazkal]

Let me ask, did you test your PR with linux jails too?

[gahr]

No, I don't have any Linux jails

[yaazkal]

Well, this needs extensive testing to not affect Midnight, Linux, or any other supported jails. Labeling the PR now to "help wanted" too.

[gahr]

@yaazkal why do you think this would be jail-type specific? This is the host's fstab, not the guest's.

[tschettervictor]

Can we go over this again? I have many jails that I do renames on, and this is a problem for them.

[tschettervictor]

Why are you removing all these lines?

[gahr]

I don't have much familiarity with this code, I just fixed it to fit my use case, so let's go through it.

My proposal is to change occurrences of ${bastille_jailsdir}/${TARGET} into ${bastille_jailsdir}/${NEWNAME} in the fstab file. I think this is hardly debatable: we are renaming the jail ${TARGET} into the jail ${NEWNAME}, so all mount points that were pointing to the former now need to point to the latter.

So why did I remove all other lines? I think the old code was trying to match a particular mount point into fstab, namely, the ..../root/.bastille mountpoint. The original bug is that it left all other custom mount points alone, unrenamed.

So, my take is to sed them all, which makes the rest of the code useless.

The linprocfs/linsysfs special case should also be covered, although I don't understand why the leading dot in the string matched in sed -i '' "s|.${bastille_jailsdir}/${TARGET}/|${bastille_jailsdir}/${NEWNAME}/|" "${FSTAB_CONFIG}".

Hope this helps clarify.

[tschettervictor]

Yes I see what you mean.
However, what happens if a jail is named the same as a directory in the path specified?

Say you have a directory mounted into a jail named test, and the jail is also named test?

What do you think would be the best way to solve this?

[tschettervictor]

This is actually a problem for the whole of the bastille project. Rename.sh and clone.sh use the approach you do to rename fstab and jail.conf files.

I suppose we could document that you should try to never name your jails in a similar manner as your directories.

[gahr]

How would that match ${bastille_jailsdir}/${TARGET} or ${bastille_jailsdir}/${NEWNAME}? You'd have to have a path, say, /usr/local/bastille/jails/foo under bastlle_jailsdir, so your mount point could be:

/host/path /usr/local/bastille/jails/TARGET/usr/local/bastille/jails/TARGET nullfs ro 0 0

In this case, yes, my approach would rename both TARGET to NEW, while probably you only want to rename the first.

I'd say it's still safer than what we're doing now...

[tschettervictor]

But just to make sure it catches the right code, I went ahead and added an intentional space in front so it will only match the jail path and never the host path.

update_fstab() {
    # Update fstab to use the new name
    FSTAB_CONFIG="${bastille_jailsdir}/${NEWNAME}/fstab"
    if [ -f "${FSTAB_CONFIG}" ]; then
        # Update fstab paths with new jail path
        sed -i '' "s| ${bastille_jailsdir}/${TARGET}/root/| ${bastille_jailsdir}/${NEWNAME}/root/|g" "${FSTAB_CONFIG}"
    fi
}

I'd recommend this change here as well.

[gahr]

LGTM, thanks!

[gahr]

Well aaaactually.. if you want the extra check to only match the second field in fstab, you should consider that fields can be separated by spaces or tabs. You version won't rename a tab-separate fstab entry.

[tschettervictor]

Good point. Reverted. Thanks.

I tested it and you are correct.

[tschettervictor]

I think we can go ahead and merge this. The recent "clone.sh" did exactly the same thing, and it is working.

[tschettervictor]

I've added PR #757 to also change the "update_fstab" function. Here it is for reference.

It indeed makes more sense to search the fstab file for all old jail paths, and simply rename them to the new jail paths.

update_fstab() {
    # Update fstab to use the new name
    FSTAB_CONFIG="${bastille_jailsdir}/${NEWNAME}/fstab"
    if [ -f "${FSTAB_CONFIG}" ]; then
        # Update fstab paths with new jail path
        sed -i '' "s|${bastille_jailsdir}/${TARGET}|${bastille_jailsdir}/${NEWNAME}|" "${FSTAB_CONFIG}"
    fi
}

[tschettervictor]

@yaazkal I just tested with a bionic Linux jail. Rename updated the mount point perfectly.

[yaazkal]

Thanks @gahr and @tschettervictor

Merging this. I'll open a new PR moving update_fstab from clone, rename and import to common to avoid this kind of issues in the future and have consistency.