On Mon, Jul 15, 2013 at 9:42 PM, Iustin Pop <[email protected]> wrote: > 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). >
Ok, I will. Thanks, Michele
