Tom, Thanks for your comments. Responses are inline.
Steve Tom Whitten wrote: > Steve Peng writes: > >> Please code review the following changes which address the service >> import performance issue: >> >> 6351623 Initial manifest-import is slow >> >> It is under http://cr.opensolaris.org/~stevep/6351623 >> >> The fix is to copy the repository to the fast tmpfs, import the >> manifests and once the import is completed copy the repository >> back to /etc/svc. During the verification cycle on both sparc >> and amd64 systems, I see ~85 percent performance improvement. >> >> Any comment/suggestion is greatly appreciated. >> >> Thanks >> >> Steve >> >> >> _______________________________________________ >> smf-discuss mailing list >> smf-discuss at opensolaris.org >> > > Thanks for doing this, Steve. It's really going to speed up our initial > boot time. I have a few comments below. > > tom > > usr/src/lib/libscf/common/lowlevel.c: > usr/src/lib/libscf/common/mapfile-vers: > usr/src/lib/libscf/inc/libscf.h: > usr/src/cmd/svc/svcadm/svcadm.c: > usr/src/cmd/svc/configd/client.c: > All the above are OK > > usr/src/lib/libscf/inc/libscf_priv.h: > - I think the comment at line 295 "Switch client" gives the wrong > impresion. I think that it is the repository that is being > switched -- not the client. > > So it is a client which does repository switch. How about "Repository switch client"? > - line 300 > David bustos taught me that it is probably not a good idea to use > boolean_t type. The X/Open and POSIX definitions are different > from the SunOS definition. He recommended using int instead. > Sounds good. I will make change. > usr/src/cmd/svc/configd/configd.h: > - line 682 > boolean_t > Ditto. > usr/src/cmd/svc/configd/backend.c: > - lines 1102 to 1205 > I encourage you to not use the REP_PROTOCOL_FAIL_UNKNOWN return > value. That does not provide much informatin to the client. If > none of the existing error codes are appropriate, perhaps we > should create a new one. > Ah! I thought that one too when I implemented changes and decided to use what had been used in the backup client. I am guessing the reason we use that in the current repository backup client is that the error message will be sent to the console from the underlying filesystem not from the smf framework if the operation fails. Of course we can print out additional smf specific error message to the console but I think the message from filesystem may be sufficient enough. I am ok to create a new error message but that will mean we may need to obsolete the one used in the backup client as well to be in sync. > usr/src/cmd/svc/milestone/manifest-import: > - line 473 > Should probably use one of the SMF_EXIT_* codes defined in > /lib/svc/share/smf_include.sh. > Correct. I will change it. > usr/src/common/svc/repcache_protocol.h: > - line 356 > Need to document the new request type. > Which line were you referring to? I did not see anything but '*'. > - lines 814 and 815 > Since these arrays represent file names, they should probably be > dimensioned to MAXPATHLEN. > > I see the point. Will change that.
