[pve-devel] [PATCH qemu-server 2/4] add function returning virtual size of disk image

[Timo Grodzinski]

Hi Wolfgang,

Thanks for your annotations!

Am 17.02.2016 um 15:01 schrieb Wolfgang Bumiller:
> Please keep our coding style in mind.
> There should be no spaces after opening and before closing parenthesis.

Do you have a perltidyrc config for your coding style?

> (Another one inline)
> 
> On Mon, Feb 15, 2016 at 02:33:47PM +0100, Timo Grodzinski wrote:
>> Signed-off-by: Timo Grodzinski <t.grodzinski at profihost.ag>
>> ---
>>  PVE/QemuServer.pm | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index a130596..6fafa59 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -6547,6 +6547,26 @@ sub clone_disk {
>>      return $disk;
>>  }
>>  
>> +sub get_image_size {
>> +    my ( $storecfg, $volid ) = @_;
>> +
>> +    my $path = PVE::Storage::path( $storecfg, $volid );
>> +
>> +    my @cmd = ( '/usr/bin/qemu-img', 'info', $path );
> 
> We try to avoid having to use '\' to make references, so please use
> +    my $cmd = ['/usr/bin/qemu-img', 'info', $path];

Why?

> 
>> +    my $size;
>> +    my $parser = sub {
>> +        my ( $line ) = @_;
>> +        if ( $line =~ m/^virtual size: .*? \((\d+) bytes\)$/ ) {
>> +            $size = $1;
>> +        }
>> +    };
>> +
>> +    eval { run_command( \@cmd, timeout => undef, outfunc => $parser ); };
> 
> With the above change this is then simply $cmd.
> 
>> +    die "unable to get image size of volume id '$volid': $@" if $@;
>> +
>> +    return $size;
>> +}
>> +
>>  # this only works if VM is running
>>  sub get_current_qemu_machine {
>>      my ($vmid) = @_;
>