[libvirt] [TCK PATCHv4] block devices: allow specification of size for safety

[Daniel P. Berrange]

On Fri, May 07, 2010 at 04:17:57PM +0200, Jim Meyering wrote:
> Eric Blake wrote:
> > I was getting failures of domain/103-blockdev-save-restore.t when
> > connecting as qemu:///session, since my uid could stat /dev/sdb but
> > not open it.  That test now skips for unprivileged users, as well as
> > adds a layer of sanity checking against expected size to avoid
> > trashing the wrong device.
> >
> > * conf/default.cfg (host_block_devices): Document optional size.
> > * lib/Sys/Virt/TCK.pm (get_host_block_device): If optional size is
> > supplied, skip a device that does not match.  Also, avoid devices
> > that can't be opened.
> > ---
> >
> > Thanks again to Jim and Daniel for the helpful feedback.
> > Here's the version I actually pushed, based on your feedback.
> 
> > diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm
> ...
> 
> Normally I wouldn't go into such detail, but you did say
> you're new to Perl, so...
> 
> > -    return $self->config("host_block_devices/[$devindex]", undef);
> > +    my $device = ($self->config("host_block_devices/[$devindex]/path", undef)
> > +		  || $self->config("host_block_devices/[$devindex]", undef));
> > +    return undef unless $device;
> 
> Personally I avoid the above ordering of constructs, since it places
> the unlikely event "return" first, and putting it all on one line
> makes it harder to see there's a conditional.

I actually like this style, because it reads like I think it :-)

  "return unless the device is defined"

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|