All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Bryan S Rosenburg <rosnbrg@us.ibm.com>
Cc: Qemu-block <qemu-block@nongnu.org>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Leo Luan <leoluan@gmail.com>, John Snow <jsnow@redhat.com>,
	Qemu-devel <qemu-devel-bounces+rosnbrg=us.ibm.com@nongnu.org>
Subject: Re: Avoid copying unallocated clusters during full backup
Date: Mon, 20 Apr 2020 18:04:33 +0300	[thread overview]
Message-ID: <59e3fb48-b397-1e59-6f29-8e7f9148fe41@virtuozzo.com> (raw)
In-Reply-To: <OF6B6CB4FB.FFD51055-ON85258550.0049F242-85258550.004FD137@notes.na.collabserv.com>

20.04.2020 17:31, Bryan S Rosenburg wrote:
> Vladimir, thank you for outlining the current state of affairs regarding efficient backup. I'd like to describe what we know about the image-expansion problem we're having using the current (qemu 4.2.0) code, just to be sure that your work is addressing it.
> 
> In our use case, the image-expansion problem occurs only when the source disk file and the target backup file are in different file systems. Both files are qcow2 files, and as long as they both reside in the same file system, the target file winds up with roughly the same size as the source. But if the target is in another file system (we've tried a second ext4 hard disk file system, a tmpfs file system, and fuse-based file systems such as s3fs), the target ends up with a size comparable to the nominal size of the source disk.
> 
> I think the expansion is related to this comment in qemu/include/block/block.h:
> 
> /**
> * bdrv_co_copy_range:
> . . . .
> * Note: block layer doesn't emulate or fallback to a bounce buffer approach
> * because usually the caller shouldn't attempt offloaded copy any more (e.g.
> * calling copy_file_range(2)) after the first error, thus it should fall back
> * to a read+write path in the caller level.
> 
> 
> 
> The bdrv_co_copy_range() service does the right things with respect to skipping unallocated ranges in the source disk and not writing zeros to the target. But qemu gives up on using this service the first time an underlying copy_file_range() system call fails, and copy_file_range() always fails with EXDEV when the source and destination files are on different file systems. In this specific case (at least), I think that falling back to a bounce buffer approach would make sense so that we don't lose the rest of the logic in bdrv_co_copy_range. As it is, qemu falls back on a very high-level loop reading from the source and writing to the target. At this high level, reading an unallocated range from the source simply returns a buffer full of zeroes, with no indication that the range was unallocated. The zeroes are then written to the target as if they were real data.
> 
> As a quick experiment, I tried a very localized fallback when copy_file_range returns EXDEV in handle_aiocb_copy_range() in qemu/block/file-posix.c. It's not a great fix because it has to allocate and free a buffer on the spot and it does not head off future calls to copy_file_range that will also fail, but it does fix the image-expansion problem when crossing file systems. I can provide the patch if anyone wants to see it.
> 
> I just wanted to get this aspect of the problem onto the table, to make sure it gets addressed in the current rework. Maybe it's a non-issue already.
> 

Yes, the problem is that copy_range subsystem handles block-status, when generic backup copying loop doesn't. I'm not sure that adding fallback into copy-range is a correct thing to do, at least it should be optional, enabled by flag.. But you don't need it for your problem,
as it is already fixed upstream:

You need to backport my commit 2d57511a88 "block/block-copy: use block_status" (together with 3 preparing patches before it, or with the whole series (including some refactoring after the 2d57511 commit)

Hope, it will help)

-- 
Best regards,
Vladimir


  reply	other threads:[~2020-04-20 15:05 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-17 18:33 Avoid copying unallocated clusters during full backup Leo Luan
2020-04-17 20:11 ` John Snow
2020-04-17 20:24   ` Eric Blake
2020-04-17 22:57     ` Leo Luan
2020-04-18  0:34       ` John Snow
2020-04-18  1:43         ` Leo Luan
2020-04-20 10:56           ` Vladimir Sementsov-Ogievskiy
2020-04-20 14:31             ` Bryan S Rosenburg
2020-04-20 15:04               ` Vladimir Sementsov-Ogievskiy [this message]
2020-04-21 14:41                 ` Bryan S Rosenburg
2020-04-17 22:31   ` Leo Luan

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=59e3fb48-b397-1e59-6f29-8e7f9148fe41@virtuozzo.com \
    --to=vsementsov@virtuozzo.com \
    --cc=jsnow@redhat.com \
    --cc=leoluan@gmail.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel-bounces+rosnbrg=us.ibm.com@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rosnbrg@us.ibm.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 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.