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

On 05/05/2010 12:54 PM, Jim Meyering wrote: >> + 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); Thanks for the tip. >> + 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: Aargh. I noticed that too, and even have it in my editor, but I hadn't hit save before I did 'git commit'. But your alternative > > if ($size) { > my $match = (sysopen (BLK, $device, O_RDONLY) > && sysseek (BLK, 0, SEEK_END) == $size * 1024); > close BLK; > $match or return undef; > } is even shorter that what 'git diff' says was still in my editor:

diff --git i/lib/Sys/Virt/TCK.pm w/lib/Sys/Virt/TCK.pm index fc325a3..1fcfdf0 100644 --- i/lib/Sys/Virt/TCK.pm +++ w/lib/Sys/Virt/TCK.pm @@ -835,15 +835,16 @@ sub get_host_block_device { my $devindex = @_ ? shift : 0; my $device = $self->config("host_block_devices/[$devindex]/path", undef); - my $size = $self->config("host_block_devices/[$devindex]/size", 0); + my $blocks = $self->config("host_block_devices/[$devindex]/size", 0);