Liane Praza wrote: > Tony Nguyen wrote: >> Here's the latest webrev with Tom's suggested changes. >> >> http://cr.opensolaris.org/~tonyn/6834760/ >> >> I don't have an incremental webrev since the added code is simple and >> I accidentally deleted the old version. Hope you guys don't mind. > > No worries. > > I'm a little confused by the messaging in svcadm.c. I don't see the > value of 2 messages in the EPERM case -- one from restarter_setup() and > one from its caller set_inst_enabled(). It seems to be different than > the verbose/non-verbose scheme elsewhere which prints out a 'permission > denied' error in the non-verbose case, with a specific property group in > the verbose one. I think you should do the same here, and just have > restarter_setup() print the error messages. >
Tom has similar comment here. I debated whether to show the pg/prop for all cases last night. I agree that printing both messages in all cases aren't necessary and inconsistent. I'll change the code to print pg/prop only in verbose mode. > Also, we also shouldn't ever get EROFS on a non-persistent property > group creation, should we? Oh. Ugh. I see that you're following local > convention of "shouldn't happen, but it can". Descriptive -- I wonder > why it's necessary. But, not your fault and can probably be left alone > for the scope of this fix. > Yes, convention. I'll add the "shouldn't happen..." comment to make it clear. Since I'll need a webrev for the RTI, I'm including both the final and incremental versions here in case you guys want to have look. http://cr.opensolaris.org/~tonyn/6834760-inc/ http://cr.opensolaris.org/~tonyn/6834760/ I'll proceed with another run of the SMF testsuite and file the RTI, unless I get additional comments. thanks, tony
