[libvirt] [PATCH] doc: Be more specific about semantics of _REUSE_EXT flag

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140707/16d361fd/attachment-0001.sig>