Nicholas Solter wrote: > Thorsten, > > Thanks for the review. See below. > > Thorsten Frueauf wrote: >> Hi Nick et al, >> >> here are my comments: >> >> - generic: In the explanation for the start option you describe that >> any pkg.depotd daemons owned by the user running the command are >> getting >> stopped. Why is that? Shouldn't that be only the case for the specific >> port (either default or the specified one)? >> The reason why I ask - moving forward, it might be required that users >> have two repos - one for i386, one for SPARC. They need to run on >> different >> ports. I would be annoyed when starting the i386 repo would stop the >> sparc >> repo ;) > > I've fixed the explanations, as the actual behavior is to stop only the > repo on the specified port. > >> >> - generic: Currently you handle start and restart identical. I would >> recommend >> to have different behaviour. Start should really only start the repo >> on the >> default or configured port. If a repo is already running, report an >> error. >> Restart on the other side should stop the repo, then start again. >> > > Sound reasonable. So now start creates the directory and starts the > depot. Restart stops the depot and then starts it (but doesn't create > the directory). The start action now always checks for another depot > running on that port (but only for processes owned by that user, as > there's no way to check for processes owned by other users, unless > you're running with extra privileges).
One clarification: you can check for the existence of the processes, but you can't run pfiles on them. Thanks, Nick > >> >> - generic: if you run >> $ ksh -n depotctl.ksh >> depotctl.ksh: warning: line 205: `...` obsolete, use $(...) >> depotctl.ksh: warning: line 221: `...` obsolete, use $(...) >> depotctl.ksh: warning: line 223: `...` obsolete, use $(...) >> depotctl.ksh: warning: line 243: `...` obsolete, use $(...) >> depotctl.ksh: warning: line 250: `...` obsolete, use $(...) >> depotctl.ksh: warning: line 323: `...` obsolete, use $(...) >> >> > > Interesting. I didn't know that. I've made the changes. > >> - line 240/241, although the comment (and comments and manpage above) >> says it >> will stop all/any repos started as the user, line 250ff makes it clear >> that you only stop if the default or specified port matches. Which >> is the >> behaviour I like, but is not what gets documented. >> > > The comments here were correct, but the earlier comments were wrong. > >> - line 243, recommend to define PGREP with the full path to pgrep and >> use it >> here > > Done (for pfiles also). > >> >> - line 250, recommend to look for /port:/ instead /port/ >> btw. you could also parse the CMD output from ps for the -p option from >> the pkg.depotd process. > > Changed it to /port:/ > > I actually tried using the ps output first, but it turns out that it > doesn't work because the cmd is truncated at 80 characters, and so can > cut off the -p argument. > >> >> - line 265 and other lines that use $REPO_DIR, you might want to use >> quotes, >> in case it contains a white space. >> > > Fixed. > > Thanks, > Nick > >> Greets >> Thorsten >> >> Nicholas Solter wrote: >>> Hi folks, >>> >>> Here's the depotctl script I've been promising for a while. It can be >>> used to manage an IPS repository, employing the same >>> port-based-on-UID scheme that Jonathan implemented for sending >>> packages as part of the build. Generally you would run this script >>> first to set up your repository, then run a build to send packages to >>> the repository. >>> >>> http://cr.opensolaris.org/~nsolter/depotctl/webrev/ >>> >>> Thanks, >>> Nick >> > > _______________________________________________ > ha-clusters-discuss mailing list > ha-clusters-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/ha-clusters-discuss
