Date: Wed, 17 Feb 2010 07:05:11 +1100 From: Peter Jeremy <peterjeremy@acm.org> To: Jeremy Chadwick <freebsd@jdc.parodius.com> Cc: freebsd-stable@freebsd.org Subject: Re: ZFS tuning [was: hardware for home use large storage] Message-ID: <20100216200511.GA95812@server.vk2pj.dyndns.org> In-Reply-To: <20100216175946.GA98082@icarus.home.lan> References: <cf9b1ee01002150049o43fced71ucb5776a0a1eaf4cf@mail.gmail.com> <20100215090756.GA54764@icarus.home.lan> <20100215105000.101326yj01j0f64g@webmail.leidinger.net> <20100215122744.GA57382@icarus.home.lan> <20100215161105.14071eiflhc9le68@webmail.leidinger.net> <4B79BA9C.3020402@quip.cz> <alpine.BSF.2.00.1002152024040.74553@pragry.qngnvk.ybpny> <4B7AD0A3.9080701@barryp.org> <20100216175946.GA98082@icarus.home.lan>
next in thread | previous in thread | raw e-mail | index | archive | help
--T4sUOijqQbZv57TR
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On 2010-Feb-16 09:59:46 -0800, Jeremy Chadwick <freebsd@jdc.parodius.com> w=
rote:
>On Tue, Feb 16, 2010 at 11:06:43AM -0600, Barry Pederson wrote:
>> maybe something like this tacked on the end of the script (excuse my
>> Perl, I'm a Python guy).
>>=20
>> ----
>> #### Loader Settings #############
>> open(LOADER, '/boot/loader.conf');
>> print "\n/boot/loader.conf settings:\n";
>> while (<LOADER>){
>> chomp;
>> if (/^\s*(zfs|vfs\.zfs|vm\.kmem)/){
>> print "\t$_\n";
>> }
>> }
>> ----
>
>Major problems with the above code:
>
>1) Opens /boot/loader.conf for rw access; should be read-only
Wrong. It opens /boot/loader.conf read-only, as it should.
>2) Makes the assumption /boot/loader.conf exists
Accepted but it was offered as a proof-of-concept.
>3) Does not close the fd
This is hardly a "major problem" in a short once-through script.
>4) Excessively quotes variables for no justified reason
If you mean including the $_ inside "", this is a standard perl idiom.
>5) Makes some bad assumptions about the contents of the file (ex.
> comments with the word "zfs" in them would match)
Wrong. It only matches "zfs" as the first non-blank text on
a line - which means it can't be a comment.
So that's one real deficiency in a proof-of-concept script written by
someone who does not claim to be comfortable with perl.
>The code should really be something like what's below. This should
>be much more manageable as well (@tunables that is), although I always
>worry when using grep()...
Whilst we are picking nits, your script has the following issues:
- unnecessary trailing wildcard matches in the regexps
- "grep" misused as "map"
- "die" is probably not appropriate for embedding into another script
- No useful error message if /boot/loader.conf can't be opened.
- Doesn't correctly handle optional whitespace around "=3D"
- No heading to explain what is being reported.
- Doesn't allow for "zfs" as a top-level identifier
Overall, Barry's script makes an excellent proof-of-concept - which is
what he was offering.
--=20
Peter Jeremy
--T4sUOijqQbZv57TR
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (FreeBSD)
iEYEARECAAYFAkt6+ncACgkQ/opHv/APuIe16gCfWqYstXAOODj2df8aMU3Zjxar
qrQAoIu2f6tpKPIuXzcH845+L1iUnJSF
=uJH4
-----END PGP SIGNATURE-----
--T4sUOijqQbZv57TR--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100216200511.GA95812>