Hey Dave, * Dave Miner (dminer at opensolaris.org) wrote: > 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.
I'll add that information. > packages/utils/iotrace.desktop > > 5: let's put this /usr (like /usr/sbin) so it's not taking up ramdisk space > (however little) Done. > packages/utils/live-io-tracing > > 32: let's let stderr go to the service log, rather than the console Ok, what are you propsing? Just removing the stderr redirect? Or do I need to do more than that to get stderr to show up in the service log? > packages/utils/live-io-tracing.xml > > 35: you should always provide a comment explaining any service dependencies Done. > 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. I'm open to suggestions. This is my first foray into smf so I basically copied something I found which is probably not the most optimal example. At a minimum, is changing the instance name on line 47 from 'live-media' to 'default' sufficient or is there more that can/should be done? > 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. Yep, see my comment above. I've changed the timeout to 60 and removed the erroneous comment. > 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. Yep, I'll send it back out once I'm done. Thanks a ton Dave. Glenn
