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);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
