Re: [Qemu-devel] [PATCH v2] qemu-img fails to delete last snapshot

[Kevin Wolf]

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