On Thu, Feb 14, 2013 at 9:40 AM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 13.02.2013 22:17, schrieb Blue Swirl:
>> On Wed, Feb 13, 2013 at 1:22 PM, Kevin Wolf <kw...@redhat.com> wrote:
>>> Look only for clusters that start at a given physical offset.
>>>
>>> Signed-off-by: Kevin Wolf <kw...@redhat.com>
>>> ---
>>> block/qcow2-cluster.c | 26 ++++++++++++++++++--------
>>> 1 files changed, 18 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
>>> index 5ce2c88..90fe36c 100644
>>> --- a/block/qcow2-cluster.c
>>> +++ b/block/qcow2-cluster.c
>>> @@ -827,8 +827,6 @@ static int handle_dependencies(BlockDriverState *bs,
>>> uint64_t guest_offset,
>>> * the length of the area that can be written to.
>>> *
>>> * -errno: in error cases
>>> - *
>>> - * TODO Make non-zero host_offset behave like describe above
>>> */
>>> static int handle_copied(BlockDriverState *bs, uint64_t guest_offset,
>>> uint64_t *host_offset, uint64_t *bytes, QCowL2Meta **m)
>>> @@ -843,7 +841,6 @@ static int handle_copied(BlockDriverState *bs, uint64_t
>>> guest_offset,
>>>
>>> trace_qcow2_handle_copied(qemu_coroutine_self(), guest_offset,
>>> *host_offset,
>>> *bytes);
>>> - assert(*host_offset == 0);
>>>
>>> /*
>>> * Calculate the number of clusters to look for. We stop at L2 table
>>> @@ -867,6 +864,15 @@ static int handle_copied(BlockDriverState *bs,
>>> uint64_t guest_offset,
>>> if (qcow2_get_cluster_type(cluster_offset) == QCOW2_CLUSTER_NORMAL
>>> && (cluster_offset & QCOW_OFLAG_COPIED))
>>> {
>>> + /* If a specific host_offset is required, check it */
>>> + if (*host_offset != 0
>>> + && (cluster_offset & L2E_OFFSET_MASK) != *host_offset)
>>> + {
>>
>> Braces should cuddle with the previous line.
>
> Can we get rid of this rule for multiline ifs? Having the
> second/third/... line of the condition and the first line of the then
> branch with no clear separation severely hurts readability in my opinion.
Perhaps the problem is that the condition is long, it could be
rewritten in this style:
bool has_host_offset = *host_offset != 0;
bool offset_matches = (cluster_offset & L2E_OFFSET_MASK) != *host_offset;
if (has_host_offset && offset_matches) {
>
>>
>>> + *bytes = 0;
>>> + ret = 0;
>>> + goto out;
>>> + }
>>> +
>>> /* We keep all QCOW_OFLAG_COPIED clusters */
>>> keep_clusters =
>>> count_contiguous_clusters(nb_clusters, s->cluster_size,
>
> Kevin
