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 :|