partitioning: fix race condition on loop device allocation

[alex3d]

Description

Find and take loop device in one shot to prevent race condition

How Has This Been Tested?

• Build image

Checklist:

• I have performed a self-review of my own code

[igorpecovnik]

I know initial hack was not the best, but will this change survive building of lets say 50 images in parallel on one build host?

[alex3d]

From losetup man:

The loop device setup is not an atomic operation when used with --find, and losetup does not protect this operation by any lock. The number of attempts is internally restricted to a maximum of 16. It is recommended to use for example flock1 to avoid a collision in heavily parallel use cases.

So I think this is more or less equivalent to the current solution (I was wrong that it is truly atomic).
I presume parallel builds are executed in docker containers started from different git working directories, so there is no common file to flock.
I could add a few attempts in loop to increase chances, if you think 16 re-tries is not enough.
The good thing with proposed solution is that it will allocate new loop device on demand when all are used (and it is a little bit shorter of course)

[alex3d]

It looks like #6568 will be also fixed by this PR

[AaronNGray]

@igorpecovnik This patch works and should really have been in the last release : |

[SteeManMI]

Since this has been preventing me from building on any ubuntu platform, I've tested and have been using this fix for a while. It works well.

[SteeManMI]

I realize this fix may not be perfect, but the number of people reporting that they can't build due to the broken existing code on Ubuntu hosts with snaps taking up all the pre-allocated loop devices is a big problem. This fixes the basic build case. Will it have issues in the pararllel build environment? maybe, but the trade off is for many users of the build framework being able to build vs the one case of the armbian infrastructure building in parallel failing. I personally vote for the users builds working. Thus why I approved and merged this in.

[AaronNGray]

Na its a great fix, worked first time and removes a rather dodgy loop structure.

[szdayisi]

All my 11 loops are occupied by snapd, so I made some change to lib/functions/image/partitioning.sh.

    FIND_LOOP_CYCLES=1
    losetup -f      #try best to get a free loop dev,  it will create a one if all existed loop devs are busy
    while : ; do
            LOOP=$(find /dev/loop* -type b | sort -R | head -1)
            LOOP_COMPARE=$(losetup -l --noheadings --raw --output=NAME | grep $LOOP || true)
            [[ -z $LOOP_COMPARE ]] && break
            [[ $FIND_LOOP_CYCLES -gt 100 ]] && exit_with_error "Unable to find free loop device"
            FIND_LOOP_CYCLES=$(( FIND_LOOP_CYCLES + 1 ))
            sleep 0.01
    done

[igorpecovnik]

On host machine try adding max_loop=xx to the kernel CMDLINE. This is mine:

GRUB_CMDLINE_LINUX_DEFAULT=" console=tty1 splash=verbose max_loop=96 i915.force_probe=* resume=UUID=bef2d749-9136-481e
-9934-d9038c6bd0b3 resume_offset=12032000"

[AaronNGray]

There's a fix higher up if you read this to thread

…

On Fri, 26 Jul 2024, 08:52 Igor, ***@***.***> wrote: On host machine try adding max_loop=xx to the kernel CMDLINE. This is mine: GRUB_CMDLINE_LINUX_DEFAULT=" console=tty1 splash=verbose *max_loop=96* i915.force_probe=* resume=UUID=bef2d749-9136-481e -9934-d9038c6bd0b3 resume_offset=12032000" — Reply to this email directly, view it on GitHub <#6729 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAU4E27L76SPLVFOZYYOS3ZOH537AVCNFSM6AAAAABJFCIO56VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJSGE3TMNJQGU> . You are receiving this because you commented.Message ID: ***@***.***>