On 05/09/2014 09:02 AM, Mike Day wrote: > 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. > > Signed-off-by: Mike Day <ncm...@ncultra.org> > --- > > There are two specific problems in the curent code. First is a lack of
s/curent/current/ > parenthesis in the calculation of a memmove 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 > > 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. This information is actually quite useful in understanding the patch, and I would have included it prior to the --- for inclusion in git, rather than in the reviewer-only commentary that gets stripped during 'git am'. > > block/qcow2-snapshot.c | 25 +++++++++++++++---------- > 1 file changed, 15 insertions(+), 10 deletions(-) I'd suggest posting a v2 with a better commit message; but the code itself seems fine, so you can add: Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
