summaryrefslogtreecommitdiff
path: root/block/qcow2-snapshot.c
AgeCommit message (Collapse)AuthorFilesLines
2018-03-09qcow2: Check snapshot L1 table in qcow2_snapshot_delete()Alberto Garcia1-0/+7
This function deletes a snapshot from disk, removing its entry from the snapshot table, freeing its L1 table and decreasing the refcounts of all clusters. The L1 table offset and size are however not validated. If we use invalid values in this function we'll probably corrupt the image even more, so we should return an error instead. We now have a function to take care of this, so let's use it. Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-09qcow2: Check snapshot L1 table in qcow2_snapshot_goto()Alberto Garcia1-0/+9
This function copies a snapshot's L1 table into the active one without validating it first. We now have a function to take care of this, so let's use it. Cc: Eric Blake <eblake@redhat.com> Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-09qcow2: Check L1 table offset in qcow2_snapshot_load_tmp()Alberto Garcia1-3/+5
This function checks that the size of a snapshot's L1 table is not too large, but it doesn't validate the offset. We now have a function to take care of this, so let's use it. Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-02qcow2: Replace align_offset() with ROUND_UP()Alberto Garcia1-5/+5
The align_offset() function is equivalent to the ROUND_UP() macro so there's no need to use the former. The ROUND_UP() name is also a bit more explicit. This patch uses ROUND_UP() instead of the slower QEMU_ALIGN_UP() because align_offset() already requires that the second parameter is a power of two. Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20180215131008.5153-1-berto@igalia.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-05-11qcow2: Discard/zero clusters by byte countEric Blake1-4/+3
Passing a byte offset, but sector count, when we ultimately want to operate on cluster granularity, is madness. Clean up the external interfaces to take both offset and count as bytes, while still keeping the assertion added previously that the caller must align the values to a cluster. Then rename things to make sure backports don't get confused by changed units: instead of qcow2_discard_clusters() and qcow2_zero_clusters(), we now have qcow2_cluster_discard() and qcow2_cluster_zeroize(). The internal functions still operate on clusters at a time, and return an int for number of cleared clusters; but on an image with 2M clusters, a single L2 table holds 256k entries that each represent a 2M cluster, totalling well over INT_MAX bytes if we ever had a request for that many bytes at once. All our callers currently limit themselves to 32-bit bytes (and therefore fewer clusters), but by making this function 64-bit clean, we have one less place to clean up if we later improve the block layer to support 64-bit bytes through all operations (with the block layer auto-fragmenting on behalf of more-limited drivers), rather than the current state where some interfaces are artificially limited to INT_MAX at a time. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 20170507000552.20847-13-eblake@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2016-07-05block: Convert bdrv_pwrite(v/_sync) to BdrvChildKevin Wolf1-7/+7
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
2016-07-05block: Convert bdrv_pread(v) to BdrvChildKevin Wolf1-6/+6
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
2016-05-19qemu-common: stop including qemu/bswap.h from qemu-common.hPaolo Bonzini1-0/+1
Move it to the actual users. There are still a few includes of qemu/bswap.h in headers; removing them is left for future work. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-03-22util: move declarations out of qemu-common.hVeronia Bahaa1-1/+1
Move declarations out of qemu-common.h for functions declared in utils/ files: e.g. include/qemu/path.h for utils/path.c. Move inline functions out of qemu-common.h and into new files (e.g. include/qemu/bcd.h) Signed-off-by: Veronia Bahaa <veroniabahaa@gmail.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-03-22include/qemu/osdep.h: Don't include qapi/error.hMarkus Armbruster1-0/+1
Commit 57cb38b included qapi/error.h into qemu/osdep.h to get the Error typedef. Since then, we've moved to include qemu/osdep.h everywhere. Its file comment explains: "To avoid getting into possible circular include dependencies, this file should not include any other QEMU headers, with the exceptions of config-host.h, compiler.h, os-posix.h and os-win32.h, all of which are doing a similar job to this file and are under similar constraints." qapi/error.h doesn't do a similar job, and it doesn't adhere to similar constraints: it includes qapi-types.h. That's in excess of 100KiB of crap most .c files don't actually need. Add the typedef to qemu/typedefs.h, and include that instead of qapi/error.h. Include qapi/error.h in .c files that need it and don't get it now. Include qapi-types.h in qom/object.h for uint16List. Update scripts/clean-includes accordingly. Update it further to match reality: replace config.h by config-target.h, add sysemu/os-posix.h, sysemu/os-win32.h. Update the list of includes in the qemu/osdep.h comment quoted above similarly. This reduces the number of objects depending on qapi/error.h from "all of them" to less than a third. Unfortunately, the number depending on qapi-types.h shrinks only a little. More work is needed for that one. Signed-off-by: Markus Armbruster <armbru@redhat.com> [Fix compilation without the spice devel packages. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-20block: Clean up includesPeter Maydell1-0/+1
Clean up includes so that osdep.h is included first and headers which it implies are not included manually. This commit was created with scripts/clean-includes. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-10-16block: Convert bs->file to BdrvChildKevin Wolf1-14/+16
This patch removes the temporary duplication between bs->file and bs->file_child by converting everything to BdrvChild. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-09-14qcow2: Rename BDRVQcowState to BDRVQcow2StateKevin Wolf1-10/+10
BDRVQcowState is already used by qcow1, and gdb is always confused which one to use. Rename the qcow2 one so they can be distinguished. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com>
2015-06-22qerror: Move #include out of qerror.hMarkus Armbruster1-0/+1
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
2015-04-28savevm: create snapshot failed when id_str already existsYi Wang1-4/+2
The command "virsh create" will fail in such condition: vm has two disks: vda and vdb. vda has snapshot s1 with id "1", vdb doesn't have s1 but has snapshot s2 with id "1". When we want to run command "virsh create s1", del_existing_snapshots() only deletes s1 in vda, and bdrv_snapshot_create() tries to create vdb's snapshot s1 with id "1", but id "1" alreay exists in vdb with name "s2"! The simplest way is call find_new_snapshot_id() unconditionally. Signed-off-by: Yi Wang <up2wing@gmail.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-03-12qcow2: fix the macro QCOW_MAX_L1_SIZE's useWen Congyang1-1/+1
QCOW_MAX_L1_SIZE's unit is byte, and l1_size's unit is l1 table entry size(8 bytes). Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> Message-id: 54FFB0F1.5010307@cn.fujitsu.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-11-03qcow2: Allow "full" discardMax Reitz1-1/+1
Normally, discarded sectors should read back as zero. However, there are cases in which a sector (or rather cluster) should be discarded as if they were never written in the first place, that is, reading them should fall through to the backing file again. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-id: 1414159063-25977-2-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-08-20block: Use g_new() & friends where that makes obvious senseMarkus Armbruster1-4/+4
g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer, for two reasons. One, it catches multiplication overflowing size_t. Two, it returns T * rather than void *, which lets the compiler catch more type errors. Patch created with Coccinelle, with two manual changes on top: * Add const to bdrv_iterate_format() to keep the types straight * Convert the allocation in bdrv_drop_intermediate(), which Coccinelle inexplicably misses Coccinelle semantic patch: @@ type T; @@ -g_malloc(sizeof(T)) +g_new(T, 1) @@ type T; @@ -g_try_malloc(sizeof(T)) +g_try_new(T, 1) @@ type T; @@ -g_malloc0(sizeof(T)) +g_new0(T, 1) @@ type T; @@ -g_try_malloc0(sizeof(T)) +g_try_new0(T, 1) @@ type T; expression n; @@ -g_malloc(sizeof(T) * (n)) +g_new(T, n) @@ type T; expression n; @@ -g_try_malloc(sizeof(T) * (n)) +g_try_new(T, n) @@ type T; expression n; @@ -g_malloc0(sizeof(T) * (n)) +g_new0(T, n) @@ type T; expression n; @@ -g_try_malloc0(sizeof(T) * (n)) +g_try_new0(T, n) @@ type T; expression p, n; @@ -g_realloc(p, sizeof(T) * (n)) +g_renew(T, p, n) @@ type T; expression p, n; @@ -g_try_realloc(p, sizeof(T) * (n)) +g_try_renew(T, p, n) Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-08-15qcow2: Handle failure for potentially large allocationsKevin Wolf1-5/+18
Some code in the block layer makes potentially huge allocations. Failure is not completely unexpected there, so avoid aborting qemu and handle out-of-memory situations gracefully. This patch addresses the allocations in the qcow2 block driver. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-04-01qcow2: Limit snapshot table sizeKevin Wolf1-1/+14
Even with a limit of 64k snapshots, each snapshot could have a filename and an ID with up to 64k, which would still lead to pretty large allocations, which could potentially lead to qemu aborting. Limit the total size of the snapshot table to an average of 1k per entry when the limit of 64k snapshots is fully used. This should be plenty for any reasonable user. This also fixes potential integer overflows of s->snapshot_size. Suggested-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-04-01qcow2: Check maximum L1 size in qcow2_snapshot_load_tmp() (CVE-2014-0143)Kevin Wolf1-0/+4
This avoids an unbounded allocation. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-04-01qcow2: Fix L1 allocation size in qcow2_snapshot_load_tmp() (CVE-2014-0145)Kevin Wolf1-1/+1
For the L1 table to loaded for an internal snapshot, the code allocated only enough memory to hold the currently active L1 table. If the snapshot's L1 table is actually larger than the current one, this leads to a buffer overflow. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-04-01qcow2: Validate snapshot table offset/size (CVE-2014-0144)Kevin Wolf1-25/+4
This avoid unbounded memory allocation and fixes a potential buffer overflow on 32 bit hosts. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-02-14block: Don't throw away errno via error_setgJeff Cody1-3/+5
There are a handful of places in the block layer where a failure path has a valid -errno value, yet error_setg() is used. Those instances should instead use error_setg_errno(), to preserve as much error information as possible. This patch replaces those instances with error_setg_errno(), so that errno is passed up the stack in the error message. Reported-By: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2013-12-04snapshot: distinguish id and name in load_tmpWenchao Xia1-2/+8
Since later this function will be used so improve it. The only caller of it now is qemu-img, and it is not impacted by introduce function bdrv_snapshot_load_tmp_by_id_or_name() that call bdrv_snapshot_load_tmp() twice to keep old search logic. bdrv_snapshot_load_tmp_by_id_or_name() return int to let caller know the errno, and errno will be used later. Also fix a typo in comments of bdrv_snapshot_delete(). Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2013-10-11qcow2: Use negated overflow check maskMax Reitz1-7/+5
In qcow2_check_metadata_overlap and qcow2_pre_write_overlap_check, change the parameter signifying the checks to perform from its current positive form to a negative one, i.e., it will no longer explicitly specify every check to perform but rather a mask of checks not to perform. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2013-10-11qcow2: Use better type for numerical snapshot IDMax Reitz1-2/+3
When trying to find a new snapshot ID, the existing ones are converted to integers using strtoul. This function returns an unsigned long, therefore its result should be saved in an unsigned long as well. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2013-10-11qcow2: Fix snapshot restoration in snapshot_createMax Reitz1-0/+1
If the new snapshot table could not be written in qcow2_snapshot_create, the old snapshot table has to be restored in memory and the new one released. This should include restoration of the old snapshot count as well, which is added by this patch. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2013-10-11qcow2: Assert against snapshot name/ID overflowMax Reitz1-0/+1
qcow2_write_snapshots relies on the length of every snapshot ID and name fitting into an unsigned 16 bit integer. This is currently ensured by QEMU through generally only allowing 128 byte IDs and 256 byte names. However, if this should change in the future, the length written to the image file should not be silently truncated (though the name itself would be written completely). Since this is currently not an issue but might require attention due to internal QEMU changes in the future, an assert ensuring sanity is enough for now. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2013-10-11qcow2: Free allocated snapshot table on errorMax Reitz1-0/+4
If an error occurs during qcow2_write_snapshots, the newly allocated snapshot table clusters are leaked and should thus be freed. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2013-10-11qcow2: Always use error path on writing snapshotsMax Reitz1-3/+4
qcow2_write_snapshots does contain a fail label and there is no reason not to use it on some errors; therefore, we should always jump there on error. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2013-09-20qcow2: Correct snapshots size for overlap checkMax Reitz1-1/+1
Using s->snapshots_size instead of snapshots_size for the metadata overlap check in qcow2_write_snapshots leads to the detection of an overlap with the main qcow2 image header when deleting the last snapshot, since s->snapshots_size has not yet been updated and is therefore non-zero. However, the offset returned by qcow2_alloc_clusters will be zero since snapshots_size is zero. Therefore, an overlap is detected albeit no such will occur. This patch fixes this by replacing s->snapshots_size by snapshots_size when calling qcow2_pre_write_overlap_check. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2013-09-12snapshot: distinguish id and name in snapshot deleteWenchao Xia1-16/+39
Snapshot creation actually already distinguish id and name since it take a structured parameter *sn, but delete can't. Later an accurate delete is needed in qmp_transaction abort and blockdev-snapshot-delete-sync, so change its prototype. Also *errp is added to tip error, but return value is kepted to let caller check what kind of error happens. Existing caller for it are savevm, delvm and qemu-img, they are not impacted by introducing a new function bdrv_snapshot_delete_by_id_or_name(), which check the return value and do the operation again. Before this patch: For qcow2, it search id first then name to find the one to delete. For rbd, it search name. For sheepdog, it does nothing. After this patch: For qcow2, logic is the same by call it twice in caller. For rbd, it always fails in delete with id, but still search for name in second try, no change to user. Some code for *errp is based on Pavel's patch. Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2013-09-12qcow2: Discard VM state in active L1 after creating snapshotKevin Wolf1-0/+7
During savevm, the VM state is written to the active L1 of the image and then a snapshot is taken. After that, the VM state isn't needed any more in the active L1 and should be discarded. This is implemented by this patch. The impact of not discarding the VM state is that a snapshot can never become smaller than any previous snapshot (because it would be padded with old VM state), and more importantly that future savevm operations cause unnecessary COWs (with associated flushes), which makes subsequent snapshots much slower. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2013-08-30qcow2: Employ metadata overlap checksMax Reitz1-0/+22
The pre-write overlap check function is now called before most of the qcow2 writes (aborting it on collision or other error). Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2013-06-24qcow2: Add refcount update reason to all callersKevin Wolf1-2/+4
This adds a refcount update reason to all callers of update_refcounts(), so that a follow-up patch can use this information to decide whether clusters that reach a refcount of 0 should be discarded in the image file. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2013-03-15qcow2: flush in qcow2_update_snapshot_refcount()Stefan Hajnoczi1-5/+0
Users of qcow2_update_snapshot_refcount() do not flush consistently. qcow2_snapshot_create() flushes but qcow2_snapshot_goto() and qcow2_snapshot_delete() do not. Solve this by moving the bdrv_flush() into qcow2_update_snapshot_refcount(). Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2013-03-15qcow2: flush refcount cache correctly in qcow2_write_snapshots()Stefan Hajnoczi1-1/+4
Since qcow2 metadata is cached we need to flush the caches, not just the underlying file. Use bdrv_flush(bs) instead of bdrv_flush(bs->file). Also add the error return path when bdrv_flush() fails and move the flush after checking for qcow2_alloc_clusters() failure so that the qcow2_alloc_clusters() error return value takes precedence. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2012-12-19block: move include files to include/block/Paolo Bonzini1-1/+1
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2012-07-09qcow2: fix #ifdef'd qcow2_check_refcounts() callersStefan Hajnoczi1-3/+3
The DEBUG_ALLOC qcow2.h macro enables additional consistency checks throughout the code. This makes it easier to spot corruptions that are introduced during development. Since consistency check is an expensive operation the DEBUG_ALLOC macro is used to compile checks out in normal builds and qcow2_check_refcounts() calls missed the addition of a new function argument. Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2012-05-02qcow2: fix the return value -ENOENT -> -EEXISTZhi Yong Wu1-1/+1
Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2012-04-20qcow2: Save disk size in snapshot headerKevin Wolf1-0/+16
This allows that different snapshots of an image can have different sizes, which is a requirement for enabling image resizing even with images that have internal snapshots. We don't do the actual support for it now, but make sure that the additional field is present and not completely ignored in all version 3 images. When trying to load a snapshot of different size, it returns an error. Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2011-12-15qcow2: Allow >4 GB VM stateKevin Wolf1-2/+32
This is a compatible extension to the snapshot header format that allows saving a 64 bit VM state size. Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2011-12-05qcow2: Fix error path in qcow2_snapshot_load_tmpKevin Wolf1-12/+22
If the bdrv_read() of the snapshot's L1 table fails, return the right error code and make sure that the old L1 table is still loaded and we don't break the BlockDriverState completely. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
2011-12-05qcow2: Fix order in qcow2_snapshot_deleteKevin Wolf1-15/+33
First the snapshot must be deleted and only then the refcounts can be decreased. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
2011-12-05qcow2: Fix order of refcount updates in qcow2_snapshot_gotoKevin Wolf1-17/+44
The refcount updates must be moved so that in the worst case we can get cluster leaks, but refcounts may never be too low. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
2011-12-05qcow2: Return real error in qcow2_snapshot_gotoKevin Wolf1-11/+40
Besides fixing the return code, this adds some comments that make clear how the code works and that it potentially breaks images if we fail in the wrong place. Actually fixing this is left for the next patch. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
2011-12-05qcow2: Rework qcow2_snapshot_create error handlingKevin Wolf1-14/+41
Increase refcounts only after allocating a new L1 table has succeeded in order to make leaks less likely. If writing the snapshot table fails, revert in-memory state to be consistent with that on disk. While at it, make it return the real error codes instead of -1. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
2011-12-05qcow2: Cleanups and memleak fix in qcow2_snapshot_createKevin Wolf1-15/+11
sn->id_str could be leaked before this. The rest of this patch changes comments, fixes coding style or removes checks that are unnecessary with g_malloc. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
2011-12-05qcow2: Update snapshot table information at onceKevin Wolf1-12/+10
Failing in the middle wouldn't help with the integrity of the image, so doing everything in a single request seems better. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>