[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v15 18/21] block: Reuse bs as backi
|
From: |
Kevin Wolf |
|
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v15 18/21] block: Reuse bs as backing hd for drive-backup sync=none |
|
Date: |
Wed, 26 Apr 2017 16:34:02 +0200 |
|
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 26.04.2017 um 15:15 hat Fam Zheng geschrieben:
> On Wed, 04/26 14:52, Kevin Wolf wrote:
> > Am 26.04.2017 um 05:34 hat Fam Zheng geschrieben:
> > > Signed-off-by: Fam Zheng <address@hidden>
> >
> > The commit message is a bit terse. :-)
> >
> > So I think this means that instead of opening the backing file of the
> > backup (which is probably the active file of the VM) a second time, we
> > instead take a reference on the existing node. Very good idea.
> >
> > In fact, my question is: How did this ever work? Did we just neglect to
> > test this? The backup backing file will use stale qcow2 metadata when we
> > continue to write to the source.
>
> Reading from the target BDS would be a problem, but I guess it's relatively
> uncommon:
>
> - In QEMU code, COW is always done manually in backup_do_cow, so no reading
> from
> target->backing in block layer.
>
> - Writing to target is done in target cluster granularity so no COW too.
>
> But it's still a fully valid point, for example it can be exported to NBD,
> etc.
Which I believe is the exact setup for fleecing.
And actually, if we were sure that we never read from the file, we
wouldn't even have to attach the backing file in the first place.
> > Unless I'm missing something, I'd propose this for qemu-stable.
>
> Yes, sounds good.
>
> >
> > Also, mirror with MIRROR_OPEN_BACKING_CHAIN probably has a similar
> > problem with opening the images in the backing chain a second time.
>
> Seems so. But if there was such a test case, image locking would have
> complained.
>
> Is that a practical use case? Mirror target image uses the active image as
> backign file? (It's a bit late in the evening, please remind me :-)
I believe storage migration uses a mode like this, but to be honest I
don't remember the details at the moment either.
But I do see that NEW_IMAGE_MODE_ABSOLUTE_PATHS enters the source as the
backing file of the target and opens it (a second instance of it) when
the mirror completes. Hopefully, by that time the source doesn't have
another user that keeps it alive (and I think op blockers should
actually ensure this), but it looks a bit fishy. Probably worth a closer
look.
Kevin
- Re: [Qemu-block] [PATCH v15 13/21] iotests: 091: Quit QEMU before checking image, (continued)
- [Qemu-block] [PATCH v15 14/21] iotests: 172: Use separate images for multiple devices, Fam Zheng, 2017/04/25
- [Qemu-block] [PATCH v15 15/21] tests: Use null-co:// instead of /dev/null as the dummy image, Fam Zheng, 2017/04/25
- [Qemu-block] [PATCH v15 16/21] file-posix: Add 'locking' option, Fam Zheng, 2017/04/25
- [Qemu-block] [PATCH v15 17/21] tests: Disable image lock in test-replication, Fam Zheng, 2017/04/25
- [Qemu-block] [PATCH v15 18/21] block: Reuse bs as backing hd for drive-backup sync=none, Fam Zheng, 2017/04/25
[Qemu-block] [PATCH v15 19/21] osdep: Add qemu_lock_fd and qemu_unlock_fd, Fam Zheng, 2017/04/25
[Qemu-block] [PATCH v15 20/21] file-posix: Add image locking to perm operations, Fam Zheng, 2017/04/25