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?
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.
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; 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?
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.
Dave
