Re: [libvirt] [TCK PATCH] block devices: allow specification of size for safety
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.
---
Go easy on me - I'm not that fluent in perl (yet); if there's
a better way to do the sanity check, I'm all ears.
The overall approach sounds fine, from my limited perspective.
diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm
...
- return
$self->config("host_block_devices/[$devindex]", undef);
+ my $device = $self->config("host_block_devices/[$devindex]/path",
undef);
+ my $size = $self->config("host_block_devices/[$devindex]/size", 0);
+
+ if (!defined $device) {
+ $device = $self->config("host_block_devices/[$devindex]", undef);
+ }
This can be equivalently (idiomatically) written as:
$device ||= $self->config("host_block_devices/[$devindex]", undef);
I prefer that, since it eliminates one use of "$device".
+ if ($size) {
+ sysopen(BLK, "$device", O_RDONLY) or return undef;
+ return undef unless sysseek(BLK, 0, SEEK_END) == $size * 1024;
+ close(BLK);
+ }
(Note no need for double quotes around $device;
Leaving parens off of some built-ins like "close" is a personal
preference (less syntax is better), but obviously keeping in sync
with the prevailing style is more important)
This is similar, but doesn't leave BLK open upon failure or size mismatch:
if ($size) {
my $match = (sysopen (BLK, $device, O_RDONLY)
&& sysseek (BLK, 0, SEEK_END) == $size * 1024);
close BLK;
$match or return undef;
}