[libvirt] [PATCHv2 0/2] doc: Improve snapshot/blockjob docs

Peter Krempa (2): doc: Document that snapshot name of block-backed disk isnt autogenerated doc: Be more specific about semantics of _REUSE_EXT flag docs/formatsnapshot.html.in | 8 +++++--- src/libvirt.c | 23 ++++++++++++++--------- tools/virsh.pod | 21 +++++++++++++-------- 3 files changed, 32 insertions(+), 20 deletions(-) -- 2.0.0

Libvirt generates external snapshot target file names for file backed storage but not for block backed storage. Document the limitation. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1032363 --- docs/formatsnapshot.html.in | 8 +++++--- tools/virsh.pod | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 1f9a6c6..4f7b7b2 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -164,9 +164,11 @@ attribute <code>type</code> giving the driver type (such as qcow2), of the new file created by the external snapshot of the new file. If <code>source</code> is not - given, a file name is generated that consists of the - existing file name with anything after the trailing dot - replaced by the snapshot name. Remember that with external + given and the disk is backed by a local image file (not + a block device or remote storage), a file name is + generated that consists of the existing file name + with anything after the trailing dot replaced by the + snapshot name. Remember that with external snapshots, the original file name becomes the read-only snapshot, and the new file name contains the read-write delta of all disk changes since the snapshot. diff --git a/tools/virsh.pod b/tools/virsh.pod index a5e8406..5da71c3 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3219,7 +3219,9 @@ The I<--diskspec> option can be used to control how I<--disk-only> and external checkpoints create external files. This option can occur multiple times, according to the number of <disk> elements in the domain xml. Each <diskspec> is in the -form B<disk[,snapshot=type][,driver=type][,file=name]>. To include a +form B<disk[,snapshot=type][,driver=type][,file=name]>. A I<diskspec> +must be provided for disks backed by block devices as libvirt doesn't +auto-generate file names for those. To include a literal comma in B<disk> or in B<file=name>, escape it with a second comma. A literal I<--diskspec> must precede each B<diskspec> unless all three of I<domain>, I<name>, and I<description> are also present. -- 2.0.0

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. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1084360 --- src/libvirt.c | 23 ++++++++++++++--------- tools/virsh.pod | 17 ++++++++++------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 8684298..79bcdf1 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18394,10 +18394,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 + * (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 @@ -19864,11 +19866,14 @@ 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 - * 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 - * shallow flag and a non-raw destination file). + * which was pre-created with the correct format and metadata and sufficient + * size to hold the copy. In case the VIR_DOMAIN_BLOCK_REBASE_SHALLOW flag + * is used the pre-created file has to exhibit the same guest visible contents + * as the backing file of the original image. This 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 shallow flag and a non-raw + * destination file. * * A copy job has two parts; in the first phase, the @bandwidth parameter * affects how fast the source is pulled into the destination, and the job diff --git a/tools/virsh.pod b/tools/virsh.pod index 5da71c3..1a2b01f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -876,10 +876,11 @@ 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> -is used, otherwise it must start empty); this option is typically used -to set up a relative backing file name in the destination. +sufficient space to hold the copy. If I<--shallow> is used in +conjunction with I<--reuse-external> then the pre-created image must have +guest visible contents identical to guest visible contents of the backing +file of the original image. This may be used to modify the backing file +names on the destination. The format of the destination is determined by the first match in the following list: if I<--raw> is specified, it will be raw; if @@ -3172,7 +3173,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 @@ -3233,8 +3235,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 +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, -- 2.0.0

On 07/11/2014 03:01 AM, Peter Krempa wrote:
Peter Krempa (2): doc: Document that snapshot name of block-backed disk isnt autogenerated doc: Be more specific about semantics of _REUSE_EXT flag
ACK series; you picked up on all my suggestions on v1.
docs/formatsnapshot.html.in | 8 +++++--- src/libvirt.c | 23 ++++++++++++++--------- tools/virsh.pod | 21 +++++++++++++-------- 3 files changed, 32 insertions(+), 20 deletions(-)
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 07/11/14 18:27, Eric Blake wrote:
On 07/11/2014 03:01 AM, Peter Krempa wrote:
Peter Krempa (2): doc: Document that snapshot name of block-backed disk isnt autogenerated doc: Be more specific about semantics of _REUSE_EXT flag
ACK series; you picked up on all my suggestions on v1.
Pushed; Thanks. Peter
participants (2)
-
Eric Blake
-
Peter Krempa