qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Cc: den@openvz.org, qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 1/2] block/raw-format: switch to BDRV_BLOCK_DATA with BDRV_BLOCK_RECURSE
Date: Tue, 13 Aug 2019 16:43:48 +0200	[thread overview]
Message-ID: <fef7f4d1-b40e-6c84-3952-120a641a8061@redhat.com> (raw)
In-Reply-To: <20190813110428.GE4663@localhost.localdomain>


[-- Attachment #1.1: Type: text/plain, Size: 1825 bytes --]

On 13.08.19 13:04, Kevin Wolf wrote:
> Am 12.08.2019 um 20:11 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> BDRV_BLOCK_RAW makes generic bdrv_co_block_status to fallthrough to
>> returned file. But is it correct behavior at all? If returned file
>> itself has a backing file, we may report as totally unallocated and
>> area which actually has data in bottom backing file.
>>
>> So, mirroring of qcow2 under raw-format is broken. Which is illustrated
>> by following commit with a test. Let's make raw-format behave more
>> correctly returning BDRV_BLOCK_DATA.
>>
>> Suggested-by: Max Reitz <mreitz@redhat.com>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> After some reading, I think I came to the conclusion that RAW is the
> correct thing to do. There is indeed a problem, but this patch is trying
> to fix it in the wrong place.
> 
> In the case where the backing file contains some data, and we have a
> 'raw' node above the qcow2 overlay node, the content of the respective
> block is not defined by the queried backing file layer, so it is
> completely correct that bdrv_is_allocated() returns false,like it would
> if you queried the qcow2 layer directly.

I disagree.  The queried backing file layer is the raw node.  As I said,
in my opinion raw nodes are not filter nodes, neither in behavior (they
have an offset option), nor in how they are generally used (as a format).

The raw format does not support backing files.  Therefore, everything on
a raw node is allocated.

(That is, like, my opinion.)

>                                          If it returned true, we would
> copy everything, which isn't right either (the test cases should may add
> the qemu-img map output of the target so this becomes visible).

It is right.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  parent reply	other threads:[~2019-08-13 14:44 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-12 18:11 [Qemu-devel] [PATCH 0/2] deal with BDRV_BLOCK_RAW Vladimir Sementsov-Ogievskiy
2019-08-12 18:11 ` [Qemu-devel] [PATCH 1/2] block/raw-format: switch to BDRV_BLOCK_DATA with BDRV_BLOCK_RECURSE Vladimir Sementsov-Ogievskiy
2019-08-13 11:04   ` Kevin Wolf
2019-08-13 11:28     ` Vladimir Sementsov-Ogievskiy
2019-08-13 12:01       ` Kevin Wolf
2019-08-13 13:21         ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2019-08-13 14:46           ` Max Reitz
2019-08-13 14:43     ` Max Reitz [this message]
2019-08-13 14:56       ` [Qemu-devel] " Vladimir Sementsov-Ogievskiy
2019-08-13 15:03         ` Max Reitz
2019-08-13 15:22           ` Vladimir Sementsov-Ogievskiy
2019-08-13 16:07             ` Max Reitz
2019-08-13 15:41       ` Kevin Wolf
2019-08-13 15:54         ` Vladimir Sementsov-Ogievskiy
2019-08-13 16:08           ` Kevin Wolf
2019-08-13 16:32             ` Vladimir Sementsov-Ogievskiy
2019-08-14  6:27               ` Vladimir Sementsov-Ogievskiy
2019-08-13 16:21         ` Max Reitz
2019-08-12 18:11 ` [Qemu-devel] [PATCH 2/2] iotests: test mirroring qcow2 under raw format Vladimir Sementsov-Ogievskiy
2019-08-13  9:10   ` Kevin Wolf
2019-08-13  9:22     ` Vladimir Sementsov-Ogievskiy
2019-08-13  9:36       ` Kevin Wolf
2019-08-12 19:46 ` [Qemu-devel] [PATCH 0/2] deal with BDRV_BLOCK_RAW Max Reitz
2019-08-12 19:50   ` Max Reitz
2019-08-13  8:39     ` Vladimir Sementsov-Ogievskiy
2019-08-13  9:01       ` Vladimir Sementsov-Ogievskiy
2019-08-13  9:33         ` Vladimir Sementsov-Ogievskiy
2019-08-13 11:14           ` Vladimir Sementsov-Ogievskiy
2019-08-13 11:51             ` Kevin Wolf
2019-08-13 13:00               ` Vladimir Sementsov-Ogievskiy
2019-08-13 14:31               ` Max Reitz
2019-08-13 14:46                 ` Vladimir Sementsov-Ogievskiy
2019-08-13 14:53                   ` Max Reitz
2019-08-13 15:03                     ` Kevin Wolf
2019-08-13 15:04                       ` Max Reitz
2019-08-13 14:50                 ` Eric Blake
2019-08-13  9:34   ` Kevin Wolf
2019-08-13 14:38     ` 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=fef7f4d1-b40e-6c84-3952-120a641a8061@redhat.com \
    --to=mreitz@redhat.com \
    --cc=den@openvz.org \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.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).