qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Benoît Canet" <benoit.canet@nodalink.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Max Reitz" <mreitz@redhat.com>
Subject: [Qemu-devel] [PATCH v2 0/9] qcow2: Fix image repairing
Date: Fri, 15 Aug 2014 17:16:17 +0200	[thread overview]
Message-ID: <1408115786-13640-1-git-send-email-mreitz@redhat.com> (raw)

As can be seen in the final patch of this series, there are certain
cases where the current repair implementation of qcow2 actually damages
the image further because it allocates new clusters for the refcount
structure which overlap with existing but according to the on-disk
refcounts (which are assumed to be wrong to begin with) unallocated
clusters.

This series fixes this by completely recreating the refcount structure
based on the in-memory information calculated during the check operation
if the possibility of damaging the image while repairing the refcount
structures in-place exists.


v2:
- Patch 1: Added
- Patch 2 (prev. 1): Drop coding style fixes [Benoît]; mention
  g_try_alloc() usage in the commit message
- Patch 3 (prev. 2): Drop coding style fixes [Benoît]
- Patch 5 (prev. 4): Try to rebuild (at least set *rebuild = true for
  this patch) refcount structures if the image size could not be
  increased to fit a refblock which is outside of the image
- Patch 6 (prev. 5): Add a note that alloc_clusters_imrt() returns an
  offset but takes a cluster index [Benoît]
- Patch 7 (prev. 6): Only clean up leaks after the refcount structure
  has been rebuilt if leaks should be fixed at all

I have kept patch 6 after 5 which leads to the check function not
repairing some errors after patch 5 and before 6 but instead just
emitting an error message. I believe this will not break bisects,
because tests which expect a specific output (like the qemu-iotests)
will be broken by this series anyway.

On the other hand, tests which do not expect a specific output but
simply expect a specific image to be repaired and which are still broken
by patch 5 without 6 should not have been repaired in the way qemu
repaired them before patch 5 anyway; so in my opinion, if these tests
stop at this commit (patch 5), the bisecter has marked some actually bad
commit as good.


git-backport-diff against v1:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/9:[down] 'qcow2: Fix leaks in dirty images'
002/9:[0032] [FC] 'qcow2: Factor out refcount accounting for check'
003/9:[0010] [FC] 'qcow2: Factor out refcount comparison for check'
004/9:[----] [-C] 'qcow2: Fix refcount blocks beyond image end'
005/9:[0013] [FC] 'qcow2: Do not perform potentially damaging repairs'
006/9:[0003] [FC] 'qcow2: Rebuild refcount structure during check'
007/9:[0020] [FC] 'qcow2: Clean up after refcount rebuild'
008/9:[----] [--] 'iotests: Fix test outputs'
009/9:[----] [--] 'iotests: Add test for potentially damaging repairs'

Max Reitz (9):
  qcow2: Fix leaks in dirty images
  qcow2: Factor out refcount accounting for check
  qcow2: Factor out refcount comparison for check
  qcow2: Fix refcount blocks beyond image end
  qcow2: Do not perform potentially damaging repairs
  qcow2: Rebuild refcount structure during check
  qcow2: Clean up after refcount rebuild
  iotests: Fix test outputs
  iotests: Add test for potentially damaging repairs

 block/qcow2-refcount.c     | 643 +++++++++++++++++++++++++++++++--------------
 block/qcow2.c              |   2 +-
 tests/qemu-iotests/039.out |  10 +-
 tests/qemu-iotests/060.out |  10 +-
 tests/qemu-iotests/061.out |  18 +-
 tests/qemu-iotests/101     |  98 +++++++
 tests/qemu-iotests/101.out |  46 ++++
 tests/qemu-iotests/group   |   1 +
 8 files changed, 623 insertions(+), 205 deletions(-)
 create mode 100755 tests/qemu-iotests/101
 create mode 100644 tests/qemu-iotests/101.out

-- 
2.0.3

             reply	other threads:[~2014-08-15 15:16 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-15 15:16 Max Reitz [this message]
2014-08-15 15:16 ` [Qemu-devel] [PATCH v2 1/9] qcow2: Fix leaks in dirty images Max Reitz
2014-08-22 16:09   ` Eric Blake
2014-08-15 15:16 ` [Qemu-devel] [PATCH v2 2/9] qcow2: Factor out refcount accounting for check Max Reitz
2014-08-21 18:47   ` Benoît Canet
2014-08-21 18:57   ` Benoît Canet
2014-08-21 19:13     ` Eric Blake
2014-08-21 21:16       ` Benoît Canet
2014-08-22 15:26         ` Max Reitz
2014-08-22 15:37           ` Benoît Canet
2014-08-22 15:44             ` Max Reitz
2014-08-22 15:46               ` Max Reitz
2014-08-22 15:48               ` Eric Blake
2014-08-15 15:16 ` [Qemu-devel] [PATCH v2 3/9] qcow2: Factor out refcount comparison " Max Reitz
2014-08-15 15:16 ` [Qemu-devel] [PATCH v2 4/9] qcow2: Fix refcount blocks beyond image end Max Reitz
2014-08-15 15:16 ` [Qemu-devel] [PATCH v2 5/9] qcow2: Do not perform potentially damaging repairs Max Reitz
2014-08-15 15:16 ` [Qemu-devel] [PATCH v2 6/9] qcow2: Rebuild refcount structure during check Max Reitz
2014-08-15 15:16 ` [Qemu-devel] [PATCH v2 7/9] qcow2: Clean up after refcount rebuild Max Reitz
2014-08-15 15:16 ` [Qemu-devel] [PATCH v2 8/9] iotests: Fix test outputs Max Reitz
2014-08-15 15:16 ` [Qemu-devel] [PATCH v2 9/9] iotests: Add test for potentially damaging repairs Max Reitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1408115786-13640-1-git-send-email-mreitz@redhat.com \
    --to=mreitz@redhat.com \
    --cc=benoit.canet@nodalink.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).