[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] qemu-img fails to delete last snapshot
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v2] qemu-img fails to delete last snapshot |
Date: |
Wed, 14 May 2014 17:15:55 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 12.05.2014 um 22:27 hat Mike Day geschrieben:
> When deleting the last snapshot, copying the resulting snapshot table
> currently fails, causing the delete operation to also fail. Fix the
> failure by skipping the copy and just writing the snapshot header and
> freeing the extra clusters.
Do you have an easy reproducer? Because I can't see the bug.
> There are two specific problems in the current code. First is a lack of
> parenthesis in the calculation of the memmove size parameter:
>
> s->nb_snapshots - snapshot_index - 1
>
> When s->nb_snapshots is 0, snapshot_index is 1.
>
> 0 - 1 - 1 = 0xfffffffe
>
> it should be:
>
> 0 - (1 - 1) = 0x00
Not really. With s->nb_snapshots == 0, there is no snapshot to delete to
start with. Therefore find_snapshot_by_id_and_name() returns -1 and we
return immediately.
> The second problem is shifting the snapshot table to the left. After
> removing the last snapshot there are no existing snapshots to be
> shifted. All that needs to be done is to write the header and
> unallocate the blocks.
When removing the last snapshot, we have:
nb_snapshots = 1
snapshot_index = 0
memmove(..., (1 - 0 - 1) * sizeof(sn));
So we're not moving anything, which is what you correctly said needs to
happen.
Kevin