On Tue, Apr 20, 2010 at 11:06, Iustin Pop <[email protected]> wrote: > On Tue, Apr 20, 2010 at 11:01:35AM +0100, Guido Trotter wrote: > > On Tue, Apr 20, 2010 at 10:59 AM, Iustin Pop <[email protected]> wrote: > > > > [+ balazs ] > > > > > On Mon, Apr 19, 2010 at 04:54:40PM +0100, Guido Trotter wrote: > > >> We can't guarantee to keep the LockedObject around, when ReleaseUid is > > >> called (because that might happen, for example, after a ganeti > > >> shutdown/restart). So we'll support releasing on just an integer uid. > > >> > > >> Signed-off-by: Guido Trotter <[email protected]> > > >> --- > > >> lib/uidpool.py | 8 ++++++-- > > >> 1 files changed, 6 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/lib/uidpool.py b/lib/uidpool.py > > >> index 1ad9435..b1e2155 100644 > > >> --- a/lib/uidpool.py > > >> +++ b/lib/uidpool.py > > >> @@ -322,9 +322,13 @@ def RequestUnusedUid(all_uids): > > >> def ReleaseUid(uid): > > >> """This should be called when the given user-id is no longer in > use. > > >> > > >> + @type uid: LockedUid or integer > > >> + @param uid: the uid to release back to the pool > > >> + > > >> """ > > >> - # Make sure we release the exclusive lock, if there is any > > >> - uid.Unlock() > > >> + if isinstance(uid, LockedUid): > > >> + # Make sure we release the exclusive lock, if there is any > > >> + uid.Unlock() > > >> try: > > >> uid_path = utils.PathJoin(constants.UIDPOOL_LOCKDIR, str(uid)) > > >> os.remove(uid_path) > > > > > > Hmm, I just realize that we rely on the str() representation to match > > > the uid number. Not optimal IMHO :), but anyway. > > > > > > > Yes, I don't like it either! :( Maybe we can improve this in a later > > patch series? > > Actually during testing of this patch series I changed the string > > representation to something which I thought was "nicer" and broke it > > already. > > Yeah, exactly, that's my point :), str is used for nice display. > > > So maybe it's something worth fixing! :) > > Indeed! > > iustin >
I would be more than happy to fix this, but I don't see what needs to be fixed here. Don't get me wrong, I'm sure it's just me not understanding the problem. Please explain. I had the following reasoning when I wrote the original code: a user-id is an integer by definition and its str() representation is expected to be the matching string of decimal digits (at least that's what I would expect). The filename should be UID_POOL_DIR + <uid>, where <uid> is the decimal representation of the user-id. BTW: Guido, I'm curious what the "nicer" string representation was in your case ;) /leczb -- Subscription settings: http://groups.google.com/group/ganeti-devel/subscribe?hl=en
