Re: [zones-discuss] codereview for 6914152 (zonecfg)

2010-02-19 Thread Jordan Vaughan

On 02/19/10 10:32 AM, Frank Batschulat (Home) wrote:

On Fri, 19 Feb 2010 15:39:21 +0100, Jerry Jelinek  
wrote:


On 02/19/10 06:53, Frank Batschulat (Home) wrote:

May I request 2 code reviewers for the changes for:

6914152 zonecfg fails when less(1M) is missing
http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6914152

http://cr.opensolaris.org/~batschul/zpager/


This looks fine to me.  One nit:

911&  5192 The error says ""Could not stat PAGER".  This error
  message might be useful to a developer
  but isn't that useful for a sysadmin.  Can you print
  something more meaningful like "PAGER %s does not exist"


Thanks Jerry, that is indeed a valid concern, I changed it to be:


PAGER /usr/bin/nonsense does not exist (No such file or directory).


I included the real error string in case of permission errors where the
file does indeed exist and I am now dropping the mysterious "stat" part.

updated webrev:

http://cr.opensolaris.org/~batschul/zpager/

cheers
frankB
___
zones-discuss mailing list
zones-discuss@opensolaris.org


Hi Frank,

This looks fine to me.

Jordan
___
zones-discuss mailing list
zones-discuss@opensolaris.org


Re: [zones-discuss] codereview for 6914152 (zonecfg)

2010-02-19 Thread Jerry Jelinek

On 02/19/10 11:32, Frank Batschulat (Home) wrote:

Thanks Jerry, that is indeed a valid concern, I changed it to be:


PAGER /usr/bin/nonsense does not exist (No such file or directory).


I included the real error string in case of permission errors where the
file does indeed exist and I am now dropping the mysterious "stat" part.

updated webrev:

http://cr.opensolaris.org/~batschul/zpager/


LGTM.

Thanks,
Jerry


___
zones-discuss mailing list
zones-discuss@opensolaris.org


Re: [zones-discuss] codereview for 6914152 (zonecfg)

2010-02-19 Thread Frank Batschulat (Home)
On Fri, 19 Feb 2010 15:39:21 +0100, Jerry Jelinek  
wrote:

> On 02/19/10 06:53, Frank Batschulat (Home) wrote:
>> May I request 2 code reviewers for the changes for:
>>
>> 6914152 zonecfg fails when less(1M) is missing
>> http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6914152
>>
>> http://cr.opensolaris.org/~batschul/zpager/
>
> This looks fine to me.  One nit:
>
> 911 & 5192 The error says ""Could not stat PAGER".  This error
>  message might be useful to a developer
>  but isn't that useful for a sysadmin.  Can you print
>  something more meaningful like "PAGER %s does not exist"

Thanks Jerry, that is indeed a valid concern, I changed it to be:


PAGER /usr/bin/nonsense does not exist (No such file or directory).


I included the real error string in case of permission errors where the
file does indeed exist and I am now dropping the mysterious "stat" part.

updated webrev:

http://cr.opensolaris.org/~batschul/zpager/

cheers
frankB
___
zones-discuss mailing list
zones-discuss@opensolaris.org


Re: [zones-discuss] codereview for 6914152 (zonecfg)

2010-02-19 Thread Jerry Jelinek

On 02/19/10 06:53, Frank Batschulat (Home) wrote:

May I request 2 code reviewers for the changes for:

6914152 zonecfg fails when less(1M) is missing
http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6914152

http://cr.opensolaris.org/~batschul/zpager/


Frank,

This looks fine to me.  One nit:

911 & 5192 The error says ""Could not stat PAGER".  This error
message might be useful to a developer
but isn't that useful for a sysadmin.  Can you print
something more meaningful like "PAGER %s does not exist"
or something along those lines.

Thanks,
Jerry

___
zones-discuss mailing list
zones-discuss@opensolaris.org


[zones-discuss] codereview for 6914152 (zonecfg)

2010-02-19 Thread Frank Batschulat (Home)
May I request 2 code reviewers for the changes for:

6914152 zonecfg fails when less(1M) is missing
http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6914152

http://cr.opensolaris.org/~batschul/zpager/

thanks!
frankB

1) old failure when $PAGER was bogus:

osoldev.root./export/home/batschul/tmp.=> setenv PAGER foobar

osoldev.root./export/home/batschul/tmp.=> zonecfg -z zone2
zonecfg:zone2> info
sh: line 1: foobar: not found

zonecfg:zone2> help
sh: line 1: foobar: not found

zonecfg:zone2> help add
usage:
add 
(global scope)
add  
(resource scope)
Add specified resource to configuration.

sh: line 1: foobar: not found

2) new failure mode when $PAGER is bogus:

osoldev.root./export/home/batschul.=> setenv PAGER nonsense

osoldev.root./export/home/batschul.=> zonecfg -z zone2
zonecfg:zone2> info
Could not stat PAGER nonsense: No such file or directory
zonename: zone2
zonepath: /tank/zones/zone2
brand: ipkg
autoboot: false
bootargs:
pool:
limitpriv:
scheduling-class:
ip-type: shared
hostid:

zonecfg:zone2> help
Could not stat PAGER nonsense: No such file or directory
Commands:

add 
(global scope)
add  
(resource scope)
Add specified resource to configuration.
.

___
zones-discuss mailing list
zones-discuss@opensolaris.org