[Yum-devel] [PATCH] Fix locking issue. BZ 865601

James Antill james at fedoraproject.org
Mon Dec 17 14:22:10 UTC 2012


On Mon, 2012-12-17 at 04:17 -0500, Zdenek Pavlas wrote:
> >  LockError() in utils should certainly deal with non-zero errno (esp.
> > EPERM, as that's thrown from a few paths) ... if both it and yummain
> > should just treat all non-zero errno's as exit 1, I'm not sure. It
> > seems sane at first thought.
> 
> Missed that in yummain, we already have:
> 
>         except Errors.LockError, e:
>             if e.errno in (errno.EPERM, errno.EACCES, errno.ENOSPC):
>                 logger.critical(_("Can't create lock file; exiting"))
>                 return 1
> 
> Adding EINVAL to the list would indeed fix BZ 865601, but:
> 
> - This IF should be duplicated in YumUtilBase.waitForLock() (it isn't).

 Yeh, they should be the same code whatever it is. Probably commented
pointing to each other, so they don't get out of sync again.

> - Converting OSError to LockError seems redundant, and passing our
>   own PID as locker PID only to make sure it passes the "pid valid"
>   check is plain ugly.

 The main difference is that OSError doesn't say what went wrong to the
caller, just why, whereas LockError() does. In general we want to avoid
having generic exceptions escape to the API user.

 The more I think about it though the more I don't mind just doing:

    if e.errno:
        # can't get lock error...
        sys.exit(1)

...in the code that catches LockError().

> IMO it would make more sense to catch EEXISTS in _lock(), re-raising
> everything else.  Updated patch:

 This breaks the EPERM raises in the yum code path.



More information about the Yum-devel mailing list