Glenn Lagasse wrote:
> I've got fixes for:
> 
> 817 LiveCD should handle existing ZFS filesystems on target hard disk
> http://defect.opensolaris.org/bz/show_bug.cgi?id=817
> 
> and
> 
> 818 iotracing needs some usability improvements
> http://defect.opensolaris.org/bz/show_bug.cgi?id=818
> 
> The webrev is located at:
> 
> http://cr.opensolaris.org/~glagasse/817_818/
> 

Sorry for the delay in reviewing.

Neither bug report explains why you created the new service to start I/O 
tracing.

packages/utils/iotrace.desktop

5: let's put this /usr (like /usr/sbin) so it's not taking up ramdisk 
space (however little)

packages/utils/live-io-tracing

32: let's let stderr go to the service log, rather than the console

packages/utils/live-io-tracing.xml

35: you should always provide a comment explaining any service dependencies

47: this could just be a default instance rather than live-media, but I 
guess I don't care much.  We use the live-media instance for the other 
services because they're replacing the default, which isn't an issue here.

50: the comment looks like a cut & paste error from some other manifest, 
since you're not doing fsck.  I don't think an infinite timeout here is 
appropriate, though; seems like 1 minute or something is more than 
enough for dtrace to get started.

packages/utils/prototype

33: see comment earlier about moving to /usr/sbin

Obviously, since Ginnie's moved this to slim_source the updated webrev 
should be against that.

Dave

Reply via email to