All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: Christian Barry <christian.barry@usp.br>,
	qemu-devel@nongnu.org, Eric Blake <eblake@redhat.com>,
	John Snow <jsnow@redhat.com>,
	qemu-block@nongnu.org, Hanna Reitz <hreitz@redhat.com>,
	David de Barros Tadokoro <davidbtadokoro@usp.br>,
	Eduardo Augusto Cavalcanti <eduardoaugustoabc@ime.usp.br>
Subject: Re: [PATCH] block: qcow2-bitmap: fix persistent bitmap corruption on ENOSPC
Date: Tue, 16 Jun 2026 12:03:42 +0200	[thread overview]
Message-ID: <ajEffqTgNxUEncoj@redhat.com> (raw)
In-Reply-To: <c2a0d86c-5989-40ca-9a7c-265b15e6b1c6@yandex-team.ru>

Am 16.06.2026 um 11:06 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 10.06.26 05:33, Christian Barry пишет:
> > From: Christian Barry <christian.barry@usp.br>
> > 
> > Changed qcow2_co_remove_persistent_dirty_bitmap() to treat free_bitmap_clusters()
> > failure as fatal and roll back the header update through a new helper
> > function rollback_ext_header_and_dir_helper(), which was also applied
> > to update_ext_header_and_dir(), which had internal rollback logic.
> > Changed bitmap_list_store() to zero the cluster tail after write,
> > guaranteeing that qemu-img won't parse into stale bytes.
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3516
> 
> Hi!
> 
> First, seem like two different fixes merged into one commit, please if
> possible, make them separate two commits.
> 
> Next, please, make commit messages more descriptive, about what the
> issue we solve (link is good as additional information, but the commit
> message itself should be complete).
> 
> For example, I don't understand free_bitmap_clusters makes the image
> unreadable. And we we fix it in one place, leaving other calls to
> free_bitmap_clusters() unchanged. Could we fix free_bitmap_clusters()
> itself? See also my comments below

I haven't looked at the patch yet, but this sounds like apart from
splitting the patches into one logical change per patch and improving
the commit message, the patch series should also include a test case
(probably qemu-iotests) that demonstrates the problem and how the patch
changes behaviour.

Kevin



      reply	other threads:[~2026-06-16 10:04 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-10  2:33 [PATCH] block: qcow2-bitmap: fix persistent bitmap corruption on ENOSPC Christian Barry
2026-06-16  9:06 ` Vladimir Sementsov-Ogievskiy
2026-06-16 10:03   ` Kevin Wolf [this message]

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=ajEffqTgNxUEncoj@redhat.com \
    --to=kwolf@redhat.com \
    --cc=christian.barry@usp.br \
    --cc=davidbtadokoro@usp.br \
    --cc=eblake@redhat.com \
    --cc=eduardoaugustoabc@ime.usp.br \
    --cc=hreitz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@yandex-team.ru \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.