Re: [zones-discuss] codereview for 6914152 (zonecfg)
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)
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)
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)
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)
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