David, Thanks very much for your comments. See inline:
> Cathy Zhou wrote: > > SMF: Seb, David Powell > > > > External webrev: > > > > http://cr.grommit.com/~yun/webrev_uv_09_28 > > dlmgmt.xml: > > Why are you ignoring core dumps and signals? Is dlmgmtd a launcher > for unreliable software? > You are right. I'll remove that. I thought about another problem in dlmgmt.xml the other day: Today, the 'stop' method is ':true', we choose that because trying to disable datalink-management service is usually not a right decision and would only cause problems. But that leaves a dlmgmtd deamon running, and reenabling the services will bring the service to the maintenance mode. Do you have good suggestion on this? > svc-dlmgmtd: > > Since svc-dlmgmtd and /sbin/dlmgmtd are co-packaged, I don't see > the need for testing for the existence of /sbin/dlmgmtd. > Okay. I will remove that. > If you want to guard against another running copy of dlmgmtd, I'd > first: > > Try to find some way to test other than grepping for a process > that happens to have the same name. I think fattach will fail if > another daemon has set up shop; Okay. I will add a check to see whether fattach() to the door file can succeed before the dlmgmt_daemonize() call. > either that or a failure of > dlmgmt_wait_for_child() to connect to the child would be a lot > more reliable. > > If that isn't possible, I'd add constraints to pgrep to increase > the probability you're finding dlmgmtd and not something else. > You should definitely be using "-u 0" to only match root > processes. > > Why are you redirecting output to /dev/msglog? > I'll remove that. The output should be able to be caught in the service's svc log file. > bfu.sh: > > You've copied a large chunk of code to import and enable your > service before rebooting (line 1954) -- code that already has 1 or > 2 copies already. This is the point where factoring it into a > subroutine starts to make sense. > > This is supported by the fact that the original code (and therefore > your copy) is buggy. To enumerate: > > You set SVCCFG_REPOSITORY in the shell's environment, but you > don't export it to subprocesses. If an invocation of bfu doesn't > fortuitously visit one of the other code paths in bfu that export > SVCCFG_REPOSITORY, the alternate root code will be importing into > the wrong repository. > > The same is true for enable_next_boot: it can enable the > datalink-management service in the wrong repository. > > I'm puzzled as to why we don't import the manifests as-is in the > alternate root case and only bother with enable_next_boot in the > standard path. Since, as far as I know, we don't support BFUing > a *running* alternate root, the extra steps taken in the > alternate root case are unnecessary. > > Additionally, the comments and messages contain a variety of errors > resulting from the copy/pasting: > > "This is to get them in" -> "This is to get it in" > "as they are in the seed" -> "as it is in the seed" > "cannot ensure the pre-import of datalink-management service." -> > "cannot ensure the pre-import of the datalink-management service." > "If they do not work" -> "If it does not work" > "make sure the services are enabled" -> > "make sure the service is enabled" > > On line 2480 you should be testing for $rootprefix/sbin/dlmgmtd not > $root/sbin/dlmgmtd. > I've considered all of your above comments and put the code into a common function that everyone can call. Please see the webrev in /net/aquila.prc//export/home/cathy/clearview-uv/webrev_review_1 note the files: dlmgmt.xml, dlmgmt_main.c, svc-dlmgmtd and bfu.sh. Sorry for those who have no SWAN access. I will generate an external webrev once I incorporated all the comments. Thanks - Cathy > Dave >
