[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/8] block: add drive-mirror command
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 5/8] block: add drive-mirror command |
Date: |
Mon, 16 Apr 2012 09:29:38 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1 |
Il 14/04/2012 01:08, Eric Blake ha scritto:
> On 04/13/2012 10:23 AM, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>
>> + flags = bs->open_flags | BDRV_O_RDWR;
>> + source = bs->backing_hd;
>
>> + /* create new image w/backing file */
>> + if (full && mode != NEW_IMAGE_MODE_EXISTING) {
>> + assert(format && drv);
>> + bdrv_get_geometry(bs, &size);
>> + size *= 512;
>> + ret = bdrv_img_create(target, format,
>> + NULL, NULL, NULL, size, flags);
>
> This side of the branch forces the size (which makes sense, since there
> is no backing file to probe it from),...
>
>> + } else {
>> + switch (mode) {
>> + case NEW_IMAGE_MODE_EXISTING:
>> + ret = 0;
>> + break;
>> + case NEW_IMAGE_MODE_ABSOLUTE_PATHS:
>> + ret = bdrv_img_create(target, format,
>> + source->filename,
>> + source->drv->format_name,
>> + NULL, -1, flags);
>
> ...but this side passes -1 (which I assume means to probe the backing
> file for its size, and reuse that size). But is this always right, or
> shouldn't this side of the branch _also_ be calling bdrv_get_geometry
> and passing in an explicit size?
This could be an idea---and for blockdev_snapshot_sync too, where it
would avoid a useless probe and opening the same file twice. It's not a
bigger problem than for snapshots, especially since now we open the file
for read only. It could be a small problem in case the source backing
file is smaller than the source itself. I don't know if that's possible.
> Should we be using BDRV_O_NO_BACKING
> to avoid potential problems in temporarily reopening a file that we
> already have open due to source?
Note that bdrv_img_create does not use BDRV_O_NO_BACKING and does not
open the backing file other than for probing. mirror_start does use
BDRV_O_NO_BACKING.
> Am I correct that bdrv_img_create will fail if I attempt to use a driver
> that doesn't support backing files (think raw) but include a request to
> set the backing file? (Put another way, I'm wondering whether libvirt
> can trust qemu to do error detection, or whether libvirt must filter out:
> virDomainBlockRebase(, _COPY | _SHALLOW | _COPY_RAW)
> as reasonable on an existing raw image, but as an error if the existing
> image already uses a backing file.
QEMU will fail here, if I understand what you mean:
if (base_filename) {
if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE,
base_filename)) {
error_report("Backing file not supported for file format '%s'",
fmt);
ret = -EINVAL;
goto out;
}
}
> Just to make sure: if I pass in an existing file, then it is expected
> that the data in that image is either identical to the data of the
> backing file (if full==false) or is conceptually uninitialized (if
> fall=true). We document that there might be rare cases of wanting to
> alter contents - I'm assuming that one such case would be to pass in an
> existing file with a larger size than the source, such that when we then
> do disk-reopen, we've achieved a poor-man's block_resize. Does the code
> react well to the existing destination having a different size than the
> source?
It doesn't care. It will also allow a smaller destination as long as
the "missing" sectors are unallocated in the source.
Paolo
- [Qemu-devel] [RFC PATCH 0/8] job-based mirroring implementation, Paolo Bonzini, 2012/04/13
- [Qemu-devel] [PATCH 2/8] block: allow interrupting a co_sleep_ns, Paolo Bonzini, 2012/04/13
- [Qemu-devel] [PATCH 1/8] block: introduce new dirty bitmap functionality, Paolo Bonzini, 2012/04/13
- [Qemu-devel] [PATCH 3/8] block: allow doing I/O in a job after cancellation, Paolo Bonzini, 2012/04/13
- [Qemu-devel] [PATCH 5/8] block: add drive-mirror command, Paolo Bonzini, 2012/04/13
- [Qemu-devel] [PATCH 6/8] block: add the drive-reopen command, Paolo Bonzini, 2012/04/13
- [Qemu-devel] [PATCH 4/8] block: add mirror job, Paolo Bonzini, 2012/04/13
- [Qemu-devel] [PATCH 8/8] docs: add mirroring to live block operations, Paolo Bonzini, 2012/04/13