[PATCH 0/1] check if lockfile is writable (bug 606 patch V2)

[PATCH 0/1] check if lockfile is writable (bug 606 patch V2)

[Yu Ke]

I've sent a patch for bug 606 before, which add sanity check for $DL_DIR.

After digging more, the root cause is hidden in bb.utils.lockfile(), so it
is more general to add check there. hence comes out the V2 patch for bug606.
please discard my V1 patch.

Pull URL: git://git.pokylinux.org/poky-contrib.git
  Branch: kyu3/bug606-v2
  Browse: http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=kyu3/bug606-v2

Thanks,
    Yu Ke <ke.yu@intel.com>
---


Yu Ke (1):
  bb.utils: check if lock file is writable, to fix bug 606

 bitbake/lib/bb/utils.py |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

[PATCH 1/1] bb.utils: check if lock file is writable, to fix bug 606

[Yu Ke]

Bug 606 report that if $DL_DIR is read-only, do_fetch will
simply hang without any error message.

The root cause is that: bb.fetch.go()->bb.utils.lockfile()
will try to lock file ${DL_DIR}/xxxxx.lock. Since ${DL_DIR}
is read-only, it will cause IOError exception. Although
lockfile() can catch the exception, currently code simply
ignore all the exception and continue the loop. it make
sense if the exception is caused by locking contention,
but in the read-only $DL_DIR case, it cause endless waiting
unfortunately.

So this patch add read-only check for lockfile to avoid the
silent hang.

Fix [BUGID #606]

Signed-off-by: Yu Ke <ke.yu@intel.com>
---
 bitbake/lib/bb/utils.py |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/bitbake/lib/bb/utils.py b/bitbake/lib/bb/utils.py
index f468faf..e02969c 100644
--- a/bitbake/lib/bb/utils.py
+++ b/bitbake/lib/bb/utils.py
@@ -401,6 +401,10 @@ def lockfile(name):
         bb.msg.error(bb.msg.domain.Util, "Error, lockfile path does not exist!: %s" % path)
         sys.exit(1)
 
+    if not os.access(path, os.W_OK):
+        bb.msg.error(bb.msg.domain.Util, "Error, lockfile path is not writable!: %s" % path)
+        sys.exit(1)
+
     while True:
         # If we leave the lockfiles lying around there is no problem
         # but we should clean up after ourselves. This gives potential
-- 
1.7.0.4

Re: [PATCH 0/1] check if lockfile is writable (bug 606 patch V2)

[Saul Wold]

On 12/29/2010 01:54 AM, Yu Ke wrote:
> I've sent a patch for bug 606 before, which add sanity check for $DL_DIR.
>
> After digging more, the root cause is hidden in bb.utils.lockfile(), so it
> is more general to add check there. hence comes out the V2 patch for bug606.
> please discard my V1 patch.
>
Ke,

I still wonder about this.  If the DL_DIR is read-only, since it is a 
shared environment (as example), when a fetch occurs, it may be for 
different reasons.

A couple of examples are:
1) an upstream fetch was missed (world build, but different hardware)
2) a developer changed the version information

In cases like this we really do want to do the fetch, so how can we 
handle that issue correctly, and not just fail and force the user to 
re-download all the packages (which may not be possible).

One option I can think of is to add a "WRITABLE_DL_DIR" (losy name but 
you get the idea) or something like that which if the DL_DIR is 
read-only could be written to for the lock file and ultimately the fetch 
of the missing upstream package.

Thoughts?

Sau!

> Pull URL: git://git.pokylinux.org/poky-contrib.git
>    Branch: kyu3/bug606-v2
>    Browse: http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=kyu3/bug606-v2
>
> Thanks,
>      Yu Ke<ke.yu@intel.com>
> ---
>
>
> Yu Ke (1):
>    bb.utils: check if lock file is writable, to fix bug 606
>
>   bitbake/lib/bb/utils.py |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)
>
> _______________________________________________
> poky mailing list
> poky@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/poky
>

Re: [PATCH 0/1] check if lockfile is writable (bug 606 patch V2)

[Richard Purdie]

On Wed, 2010-12-29 at 10:01 -0800, Saul Wold wrote:
> I still wonder about this.  If the DL_DIR is read-only, since it is a 
> shared environment (as example), when a fetch occurs, it may be for 
> different reasons.
> 
> A couple of examples are:
> 1) an upstream fetch was missed (world build, but different hardware)
> 2) a developer changed the version information
> 
> In cases like this we really do want to do the fetch, so how can we 
> handle that issue correctly, and not just fail and force the user to 
> re-download all the packages (which may not be possible).
> 
> One option I can think of is to add a "WRITABLE_DL_DIR" (losy name but 
> you get the idea) or something like that which if the DL_DIR is 
> read-only could be written to for the lock file and ultimately the fetch 
> of the missing upstream package.

DL_DIR *must* be writeable. We don't support the use case where it is
not. We handle readonly directory sources as part of the mirror
handling. It might be not a lot is written to DL_DIR, that is just fine.

I therefore agree with Ke's patch which errors if the directory is
writeable.

Cheers,

Richard

Re: [PATCH 0/1] check if lockfile is writable (bug 606 patch V2)

[Yu Ke]

On Dec 29, 20:26, Richard Purdie wrote:
> On Wed, 2010-12-29 at 10:01 -0800, Saul Wold wrote:
> > I still wonder about this.  If the DL_DIR is read-only, since it is a 
> > shared environment (as example), when a fetch occurs, it may be for 
> > different reasons.
> > 
> > A couple of examples are:
> > 1) an upstream fetch was missed (world build, but different hardware)
> > 2) a developer changed the version information
> > 
> > In cases like this we really do want to do the fetch, so how can we 
> > handle that issue correctly, and not just fail and force the user to 
> > re-download all the packages (which may not be possible).
> > 
> > One option I can think of is to add a "WRITABLE_DL_DIR" (losy name but 
> > you get the idea) or something like that which if the DL_DIR is 
> > read-only could be written to for the lock file and ultimately the fetch 
> > of the missing upstream package.
> 
> DL_DIR *must* be writeable. We don't support the use case where it is
> not. We handle readonly directory sources as part of the mirror
> handling. It might be not a lot is written to DL_DIR, that is just fine.

Yes, so this patch does not intended to support read-only DL_DIR, it just provides more verbal information to use instead of silently hang, so that user can know what is wrong and fix it accordingly.

Regards
Ke

> 
> I therefore agree with Ke's patch which errors if the directory is
> writeable.
> 
> Cheers,
> 
> Richard
> 
> 
> 
> _______________________________________________
> poky mailing list
> poky@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/poky

Re: [PATCH 0/1] check if lockfile is writable (bug 606 patch V2)

[Richard Purdie]

On Wed, 2010-12-29 at 17:54 +0800, Yu Ke wrote:
> I've sent a patch for bug 606 before, which add sanity check for $DL_DIR.
> 
> After digging more, the root cause is hidden in bb.utils.lockfile(), so it
> is more general to add check there. hence comes out the V2 patch for bug606.
> please discard my V1 patch.
> 
> Pull URL: git://git.pokylinux.org/poky-contrib.git
>   Branch: kyu3/bug606-v2
>   Browse: http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=kyu3/bug606-v2
> 
> Thanks,
>     Yu Ke <ke.yu@intel.com>

Merged into master, thanks.

Richard