Re: [PATCH 1/1] dpkg: acquire lock when calling reprepro

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2018-08-22 15:41, Henning Schild wrote:
> My first reaction was the same, but in the other direction. I would
> prefer a bitbake lock over the flock, if possible.

That would at least involve factoring out the critical section into 
dedicated tasks - otherwise we to over-locking and serialize harmless 
(and potentially lengthy) steps needlessly.

For now, I would be infavor of fixing in-place and trying to cleanup on top.

Jan

> 
> Henning
> 
> Am Wed, 22 Aug 2018 12:48:14 +0000
> schrieb "Hombourger, Cedric" <Cedric_Hombourger@mentor.com>:
> 
>> Thanks Jan for your quick feedback. I have submitted a v2 with
>> isar.lock removed
>>
>> Cedric
>>
>> -----Original Message-----
>> From: Jan Kiszka [mailto:jan.kiszka@siemens.com]
>> Sent: Wednesday, August 22, 2018 2:20 PM
>> To: Hombourger, Cedric <Cedric_Hombourger@mentor.com>;
>> isar-users@googlegroups.com Subject: Re: [PATCH 1/1] dpkg: acquire
>> lock when calling reprepro
>>
>> On 2018-08-22 13:34, Cedric Hombourger wrote:
>>> As bitbake performs multi-threaded builds, packages may be built
>>> while others get deployed. A lock was already acquired within
>>> dpkg_runbuild but accesses to the isar-apt database were not
>>> protected from do_deploy_deb. Move the lock from the buildchroot to
>>> the isar-apt repository and acquire the (same) lock from
>>> dpkg_runbuild, repo_clean and do_deploy_deb.
>>
>> I think I've seen this issue once but forgot to dig into the details.
>> It looks consistent.
>>
>>>
>>> Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
>>> ---
>>>    meta/classes/dpkg-base.bbclass                   | 8 ++++++--
>>>    meta/recipes-devtools/buildchroot/files/build.sh | 2 +-
>>>    meta/recipes-kernel/linux/files/build-kernel.sh  | 2 +-
>>>    3 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/meta/classes/dpkg-base.bbclass
>>> b/meta/classes/dpkg-base.bbclass index a41df2c..7137968 100644
>>> --- a/meta/classes/dpkg-base.bbclass
>>> +++ b/meta/classes/dpkg-base.bbclass
>>> @@ -71,6 +71,8 @@ do_build() {
>>>                mount -t proc none ${BUILDCHROOT_DIR}/proc
>>>            fi'
>>>    
>>> +    # Create database lock file here so we own it (not root) and
>>> perform actual build
>>> +    touch ${DEPLOY_DIR_APT}/${DISTRO}/db.lock
>>>        dpkg_runbuild
>>>    
>>>        sudo umount ${BUILDROOT} 2>/dev/null || true @@ -82,7 +84,8
>>> @@ CLEANFUNCS += "repo_clean"
>>>    repo_clean() {
>>>        PACKAGES=$(cd ${S}/..; ls *.deb | sed 's/\([^_]*\).*/\1/')
>>>        if [ -n "${PACKAGES}" ]; then
>>> -        reprepro -b ${DEPLOY_DIR_APT}/${DISTRO} \
>>> +        flock ${DEPLOY_DIR_APT}/${DISTRO}/db.lock \
>>> +            reprepro -b ${DEPLOY_DIR_APT}/${DISTRO} \
>>>                     --dbdir ${DEPLOY_DIR_DB}/${DISTRO} \
>>>                     -C main -A ${DISTRO_ARCH} \
>>>                     remove ${DEBDISTRONAME} \ @@ -93,7 +96,8 @@
>>> repo_clean() {
>>>    # Install package to Isar-apt
>>>    do_deploy_deb() {
>>>        repo_clean
>>> -    reprepro -b ${DEPLOY_DIR_APT}/${DISTRO} \
>>> +    flock ${DEPLOY_DIR_APT}/${DISTRO}/db.lock \
>>> +        reprepro -b ${DEPLOY_DIR_APT}/${DISTRO} \
>>>                 --dbdir ${DEPLOY_DIR_DB}/${DISTRO} \
>>>                 -C main \
>>>                 includedeb ${DEBDISTRONAME} \
>>
>> There is also
>>
>> do_deploy_deb[lockfiles] = "${DEPLOY_DIR_APT}/isar.lock"
>>
>> in that file. I suppose that can now be dropped, can't it?
>>
>>> diff --git a/meta/recipes-devtools/buildchroot/files/build.sh
>>> b/meta/recipes-devtools/buildchroot/files/build.sh
>>> index e2dabab..e8a9c17 100644
>>> --- a/meta/recipes-devtools/buildchroot/files/build.sh
>>> +++ b/meta/recipes-devtools/buildchroot/files/build.sh
>>> @@ -50,7 +50,7 @@ install_cmd="apt-get -o
>>> Debug::pkgProblemResolver=yes --no-install-recommends -y
>>>    
>>>        # Install all build deps
>>>        mk-build-deps $set_arch -t "${install_cmd}" -i -r
>>> debian/control -) 42>/dpkg.lock
>>> +) 42>/isar-apt/db.lock
>>>    
>>>    # If autotools files have been created, update their timestamp to
>>>    # prevent them from being regenerated diff --git
>>> a/meta/recipes-kernel/linux/files/build-kernel.sh
>>> b/meta/recipes-kernel/linux/files/build-kernel.sh
>>> index 8b58e93..2ff5a1e 100644
>>> --- a/meta/recipes-kernel/linux/files/build-kernel.sh
>>> +++ b/meta/recipes-kernel/linux/files/build-kernel.sh
>>> @@ -34,7 +34,7 @@ REPACK_LINUX_IMAGE_DIR="${REPACK_DIR}/linux-image"
>>>    REPACK_LINUX_HEADERS_DIR="${REPACK_DIR}/linux-headers"
>>>    
>>>    # Lock-protected because apt and dpkg do not wait in case of
>>> contention -flock /dpkg.lock \
>>> +flock /isar-apt/db.lock \
>>>        apt-get install -y -o Debug::pkgProblemResolver=yes \
>>>            --no-install-recommends ${KBUILD_DEPENDS}
>>>    
>>>    
>>
>> Jan
>>
>> --
>> Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate
>> Competence Center Embedded Linux
>>
> 

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux