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

Stefan Reiter s.reiter at proxmox.com
Mon Dec 9 13:40:27 CET 2019


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



More information about the pve-devel mailing list