[pve-devel] [PATCH qemu-server 2/2] update disk size before local disk migration

[Stefan Reiter]

>>>>    sub update_disksize {
>>>> +    my ($drive, $volid_hash, $log_fn) = @_;
>>>
>>> this is not a very nice interface.. maybe the logging/printing could be
>>> moved to the call site(s), by returning
>>>
>>> wantarray ? ($drive, $old, $new) : $drive;
>>>
>>> ?
>>>
>>
>> I did that to avoid writing the whole format_size shenanigans at each
>> call site.
> 
> but those shenanigans are new anyway - previously it only printed
> 
> "update disk 'scsi0' information."
> 

Yes, but as a user this tells me very little about what actually 
happened. "Updated information" is one of those filler-logs that I think 
I trained myself to ignore, but maybe that's just me ;)

Just to give a reason why I changed it at all..

> so, we could either
> A keep the output simple (no sizes)
> B include unformatted sizes in output
> C include formatted sizes in output
> 
> B & C still don't need to use a log_fn as argument ;) for C, you could
> either return the formatted sizes, or do the formatting at the one new
> call site and keep the existing users (which are just 'qm rescan', and
> 'qmrestore') like they are.
> 
>>
>> Maybe
>>
>> my $log_msg = "size of disk updated [...]";
>> wantarray ? ($drive, $log_msg) : $drive;
>>
>> would be ok too? Removes the $log_fn but keeps the call sites to a
>> single print.
> 
> better than the current version, but less flexible/future-proof than
> returning the sizes ;)
> 

I'll send a v2 that returns the formatted sizes and prints them at both 
call sites, don't see the point in leaving current code as is when we 
could improve it basically for free.