Skip to content

Commit

Permalink
qcow2: Repair OFLAG_COPIED when fixing leaks
Browse files Browse the repository at this point in the history
Repairing OFLAG_COPIED is usually safe because it is done after the
refcounts have been repaired.  Therefore, it we did not find anyone else
referencing a data or L2 cluster, it makes no sense to not set
OFLAG_COPIED -- and the other direction (clearing OFLAG_COPIED) is
always safe, anyway, it may just induce leaks.

Furthermore, if OFLAG_COPIED is actually consistent with a wrong (leaky)
refcount, we will decrement the refcount with -r leaks, but OFLAG_COPIED
will then be wrong.  qemu-img check should not produce images that are
more corrupted afterwards then they were before.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1527085
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509200059.31125-2-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
  • Loading branch information
XanClic committed Jun 11, 2018
1 parent dee6ddd commit 3cce51c
Showing 1 changed file with 17 additions and 8 deletions.
25 changes: 17 additions & 8 deletions block/qcow2-refcount.c
Expand Up @@ -1799,6 +1799,19 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
int ret;
uint64_t refcount;
int i, j;
bool repair;

if (fix & BDRV_FIX_ERRORS) {
/* Always repair */
repair = true;
} else if (fix & BDRV_FIX_LEAKS) {
/* Repair only if that seems safe: This function is always
* called after the refcounts have been fixed, so the refcount
* is accurate if that repair was successful */
repair = !res->check_errors && !res->corruptions && !res->leaks;
} else {
repair = false;
}

for (i = 0; i < s->l1_size; i++) {
uint64_t l1_entry = s->l1_table[i];
Expand All @@ -1818,10 +1831,8 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
if ((refcount == 1) != ((l1_entry & QCOW_OFLAG_COPIED) != 0)) {
fprintf(stderr, "%s OFLAG_COPIED L2 cluster: l1_index=%d "
"l1_entry=%" PRIx64 " refcount=%" PRIu64 "\n",
fix & BDRV_FIX_ERRORS ? "Repairing" :
"ERROR",
i, l1_entry, refcount);
if (fix & BDRV_FIX_ERRORS) {
repair ? "Repairing" : "ERROR", i, l1_entry, refcount);
if (repair) {
s->l1_table[i] = refcount == 1
? l1_entry | QCOW_OFLAG_COPIED
: l1_entry & ~QCOW_OFLAG_COPIED;
Expand Down Expand Up @@ -1862,10 +1873,8 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
if ((refcount == 1) != ((l2_entry & QCOW_OFLAG_COPIED) != 0)) {
fprintf(stderr, "%s OFLAG_COPIED data cluster: "
"l2_entry=%" PRIx64 " refcount=%" PRIu64 "\n",
fix & BDRV_FIX_ERRORS ? "Repairing" :
"ERROR",
l2_entry, refcount);
if (fix & BDRV_FIX_ERRORS) {
repair ? "Repairing" : "ERROR", l2_entry, refcount);
if (repair) {
l2_table[j] = cpu_to_be64(refcount == 1
? l2_entry | QCOW_OFLAG_COPIED
: l2_entry & ~QCOW_OFLAG_COPIED);
Expand Down

0 comments on commit 3cce51c

Please sign in to comment.