On Mon, Jul 15, 2013 at 10:45:24AM +0200, Michele Tartara wrote: > On Sat, Jul 13, 2013 at 11:40 AM, Michele Tartara <[email protected]>wrote: > > > > > Il giorno 12/lug/2013 20:44, "Iustin Pop" <[email protected]> ha scritto: > > > > > > > > On Fri, Jul 12, 2013 at 06:16:20PM +0200, Michele Tartara wrote: > > > > The haskell library functions only allow to change file ownership using > > > > uid/gid. A function for doing that with explicit names is added by this > > > > commit. > > > > > > > > Signed-off-by: Michele Tartara <[email protected]> > > > > > > Hi Michele, > > > > Hi Iustin, > > > > > > > > > +-- | Set the owner and the group of a file (given as names, not > > numeric id). > > > > +setOwnerAndGroupFromNames :: FilePath -> String -> String -> IO () > > > > +setOwnerAndGroupFromNames filename username groupname = do > > > > + let nameUid = fmap userID (getUserEntryForName username) > > > > + nameGid = fmap groupID (getGroupEntryForName groupname) > > > > + uid <- nameUid > > > > + gid <- nameGid > > > > + setOwnerAndGroup filename uid gid > > > > > > Do you really need this to be generic? For ganeti-specific users, you > > > could/should use instead Runtime.hs/getEnts and the users/group maps it > > > returns, so that all name resolving is in a single place. Especially at > > > runtime, after the daemons have started, we should try to avoid user > > > lookups as depending on the nsswitch configuration this could have > > > arbitrary delays. > > > > > > iustin > > > > Of course, doing is for Ganeti users only is more than enough, but I > > didn't know we had uids and gods cached already. > > > > Thanks for pointing it out, I'll modify the function to use them. > > > > Thanks, > > Michele > > > Interdiff: > diff --git a/src/Ganeti/Utils.hs b/src/Ganeti/Utils.hs > index 92b5ed8..17b469a 100644 > --- a/src/Ganeti/Utils.hs > +++ b/src/Ganeti/Utils.hs > @@ -62,16 +62,17 @@ module Ganeti.Utils > import Data.Char (toUpper, isAlphaNum, isDigit, isSpace) > import Data.Function (on) > import Data.List > +import qualified Data.Map as M > import Control.Monad (foldM) > > import Debug.Trace > > import Ganeti.BasicTypes > import qualified Ganeti.Constants as C > +import Ganeti.Runtime > import System.IO > import System.Exit > import System.Posix.Files > -import System.Posix.User > import System.Time > > -- * Debug functions > @@ -436,10 +437,12 @@ recombineEithers lefts rights trail = > show t > > -- | Set the owner and the group of a file (given as names, not numeric > id). > -setOwnerAndGroupFromNames :: FilePath -> String -> String -> IO () > -setOwnerAndGroupFromNames filename username groupname = do > - let nameUid = fmap userID (getUserEntryForName username) > - nameGid = fmap groupID (getGroupEntryForName groupname) > - uid <- nameUid > - gid <- nameGid > +setOwnerAndGroupFromNames :: FilePath -> GanetiDaemon -> GanetiGroup -> IO > () > +setOwnerAndGroupFromNames filename daemon dGroup = do > + runtimeEnts <- getEnts > + ents <- exitIfBad "Can't find required user/groups" runtimeEnts > + -- note: we use directly ! as lookup failues shouldn't happen, due > + -- to the map construction > + let uid = (fst ents) M.! daemon > + let gid = (snd ents) M.! dGroup > setOwnerAndGroup filename uid gid
LGTM ([email protected]). Would you mind adding a todo here to rework this such that we only read the runtimeEnts once per daemon startup? (I've looked and it wouldn't be directly doable, but at least a todo to resolve and either cache or pass via arguments the ents would be useful; no need for resend). thanks! iustin
