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

[Jim Meyering]

Eric Blake wrote:

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

Your "$blocks" is better than "$size", but
how about $kb_blocks instead "blocks", to distinguish from
the relatively common connotation of 512-byte blocks.

> 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);