On 07/07/2014 06:40 AM, Peter Krempa wrote:

Snapshots and block-copy have a flag that forces qemu to re-use existing file. Our docs weren't exactly clear on what the existing file should contain for this to actually work.

Re-word the docs a bit to state that the file needs to be pre-created in the desired format and the backing chain metadata needs to be set prior to handing it over to qemu.

Furthermore, in at least the case of copying to raw (but probably elsewhere), the destination file must be sized at least as large as what the guest sees, or else qemu 2.0 and earlier can abort the job on an I/O error with no indication to libvirt what the error was. https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg07377.html Libvirt should probably be taught to use newer qemu's rerror= and werror= job options so that the job sticks around rather than disappearing on error, so that it is not quite such an invisible shock of a job just disappearing because of I/O error due to insufficient sizing, but that's a much bigger patch. For now, documenting the issue is probably sufficient.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1084360 --- src/libvirt.c | 12 +++++++----- tools/virsh.pod | 12 +++++++----- 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c index b80b484..e7a6aca 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18274,10 +18274,12 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) * destination files already exist as a non-empty regular file, the * snapshot is rejected to avoid losing contents of those files. * However, if @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT, - * then the destination files must already exist and contain content - * identical to the source files (this allows a management app to - * pre-create files with relative backing file names, rather than the - * default of creating with absolute backing file names). + * then the destination files must be pre-created manually with + * the correct image format and metadata including backing store path.

No trailing dot, since your parenthetical comment is a continuation of the same sentence.

+ * (this allows a management app to pre-create files with relative backing + * file names, rather than the default of creating with absolute backing + * file names). Note that setting incorrect metadata in the pre-created + * image may lead to the VM being unable to start. * * Be aware that although libvirt prefers to report errors up front with * no other effect, some hypervisors have certain types of failures where @@ -19731,7 +19733,7 @@ virDomainBlockPull(virDomainPtr dom, const char *disk, * by adding VIR_DOMAIN_BLOCK_REBASE_COPY_RAW to force the copy to be raw * (does not make sense with the shallow flag unless the source is also raw), * or by using VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT to reuse an existing file - * with initial contents identical to the backing file of the source (this + * which was pre-created with the correct format and metadata (this

Given my observation above, I'd use: pre-created with the correct format, metadata, and sufficient size to hold the copy Oh, and it SHOULD be valid to do the following: starting from: base <- snap1 <- active use: qemu-img convert -f qcow2 -O raw snap1 flat then start a shallow copy: virsh blockcopy $dom vda flat --shallow --raw --reuse-external --finish and end up with 'flat' containing the complete guest contents visible from 'active' at the time it completes. That is, the combination of the two separate actions into 'flat' will cover all the sectors, even though the shallow copy does NOT copy all sectors. And _that_ is what my original wording was trying to convey when it said 'with initial contents identical to the backing file of the source'. Maybe a better wording would have been: reuse an existing file that contains the same guest-visible content as the backing file of the active image being copied

* allows a management app to pre-create files with relative backing file * names, rather than the default of absolute backing file names; as a * security precaution, you should generally only use reuse_ext with the diff --git a/tools/virsh.pod b/tools/virsh.pod index 1b6f3c4..39fe423 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -875,8 +875,8 @@ flattens the entire chain; but if I<--shallow> is specified, the copy shares the backing chain.

If I<--reuse-external> is specified, then I<dest> must exist and have -contents identical to the resulting backing file (that is, it must -start with contents matching the backing file I<disk> if I<--shallow> +metadata identical to the resulting backing file (that is, it must

No, it's not the metadata that must be identical, but the guest-visible contents (the metadata can be completely different, even qed instead of qcow2, as long as loading that file would give the guest the same data).

+start with metadata matching the backing file I<disk> if I<--shallow> is used, otherwise it must start empty); this option is typically used to set up a relative backing file name in the destination.

@@ -3165,7 +3165,8 @@ metadata again).

If I<--reuse-external> is specified, and the snapshot XML requests an external snapshot with a destination of an existing file, then the -destination must exist, and is reused; otherwise, a snapshot is refused +destination must exist and be pre-created with correct format and +metadata. The file is then reused; otherwise, a snapshot is refused to avoid losing contents of the existing files.

If I<--quiesce> is specified, libvirt will try to use guest agent @@ -3224,8 +3225,9 @@ results in the following XML:

If I<--reuse-external> is specified, and the domain XML or I<diskspec> option requests an external snapshot with a destination of an existing -file, then the destination must exist, and is reused; otherwise, a -snapshot is refused to avoid losing contents of the existing files. +file, then the destination must exist and be pre-created with correct +format and metadata. The file is then reused; otherwise, a snapshot

spurious double space

+is refused to avoid losing contents of the existing files.

If I<--quiesce> is specified, libvirt will try to use guest agent to freeze and unfreeze domain's mounted file systems. However,

I'm wondering if I should attempt a v2 to address the concerns I pointed out above. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org