[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

     if (!defined $device) {
 	$device = $self->config("host_block_devices/[$devindex]", undef);
     }
-    if ($size) {
+    if ($blocks) {
 	sysopen(BLK, "$device", O_RDONLY) or return undef;
-	return undef unless sysseek(BLK, 0, SEEK_END) == $size * 1024;
+	my $size = sysseek(BLK, 0, SEEK_END);
 	close(BLK);
+	return undef unless $size == $blocks * 1024;
     }
     return $device;
 }


-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]