Re: [Qemu-devel] [PATCH v2 2/9] qcow2: Factor out refcount accounting for check

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

On 08/21/2014 12:57 PM, Benoît Canet wrote:
> On Fri, Aug 15, 2014 at 05:16:19PM +0200, Max Reitz wrote:
>> Put the code for calculating the reference counts during qemu-img check
>> into an own function.
>>
>> Also, do not use g_realloc() for increasing the size of the in-memory
>> refcount table, but rather g_try_realloc().

The "Also," may be a sign of doing two things in one patch that could
have been two.

>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  block/qcow2-refcount.c | 188 ++++++++++++++++++++++++++++++-------------------
>>  1 file changed, 115 insertions(+), 73 deletions(-)

But overall, this patch seems like a reasonable refactor to me,
splitting out a reusable piece.

> Another point is that I think these two extractions patches will totaly confuse
> git blame the day we will need it.

I disagree.  When refactoring to split a large function into multiple
smaller functions, where the original function calls out to a helper
function for what it used to do inline, git blame tracks things
perfectly.  Someone following blame may end up on this commit as the
source of a given line in its new location, but the blame viewer will
also show this commit is a refactor, and it should be fairly easy to
find where the line was moved from, and resume blaming even further.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]