From babfc1d48c3a0f83592fa501b609fd839ff1a51b Mon Sep 17 00:00:00 2001

Message-Id: <babfc1d48c3a0f83592fa501b609fd839ff1a51b@dist-git>

From: Eric Blake <eblake@redhat.com>

Date: Tue, 24 Feb 2015 11:59:52 +0100

Subject: [PATCH] blockcopy: allow block device destination

https://bugzilla.redhat.com/show_bug.cgi?id=1196066

To date, anyone performing a block copy and pivot ends up with

the destination being treated as <disk type='file'>.  While this

works for data access for a block device, it has at least one

noticeable shortcoming: virDomainGetBlockInfo() reports allocation

differently for block devices visited as files (the size of the

device) than for block devices visited as <disk type='block'>

(the maximum sector used, as reported by qemu); and this difference

is significant when trying to manage qcow2 format on block devices

that can be grown as needed.

Of course, the more powerful virDomainBlockCopy() API can already

express the ability to set the <disk> type.  But a new API can't

be backported, while a new flag to an existing API can; and it is

also rather inconvenient to have to resort to the full power of

generating XML when just adding a flag to the older call will do

the trick.  So this patch enhances blockcopy to let the user flag

when the resulting XML after the copy must list the device as

type='block'.

* include/libvirt/libvirt.h.in (VIR_DOMAIN_BLOCK_REBASE_COPY_DEV):

New flag.

* src/libvirt.c (virDomainBlockRebase): Document it.

* tools/virsh-domain.c (opts_block_copy, blockJobImpl): Add

--blockdev option.

* tools/virsh.pod (blockcopy): Document it.

* src/qemu/qemu_driver.c (qemuDomainBlockRebase): Allow new flag.

(qemuDomainBlockCopy): Remember the flag, and make sure it is only

used on actual block devices.

* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Test it.

Signed-off-by: Eric Blake <eblake@redhat.com>

(cherry picked from commit b7e73585a8d96677695a52bafb156f26cbd48fb5)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 include/libvirt/libvirt.h.in                       |  2 ++

 src/libvirt.c                                      |  8 +++--

 src/qemu/qemu_driver.c                             | 36 ++++++++++++++--------

 .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml  |  4 +--

 tools/virsh-domain.c                               |  6 ++++

 tools/virsh.pod                                    |  7 +++--

 6 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in

index 94de8a6..e086c8f 100644

--- a/include/libvirt/libvirt.h.in

+++ b/include/libvirt/libvirt.h.in

@@ -2638,6 +2638,8 @@ typedef enum {

     VIR_DOMAIN_BLOCK_REBASE_RELATIVE  = 1 << 4, /* Keep backing chain

                                                    referenced using relative

                                                    names */

+    VIR_DOMAIN_BLOCK_REBASE_COPY_DEV  = 1 << 5, /* Treat destination as block

+                                                   device instead of file */

 } virDomainBlockRebaseFlags;

 int           virDomainBlockRebase(virDomainPtr dom, const char *disk,

diff --git a/src/libvirt.c b/src/libvirt.c

index 5315881..e1c02dc 100644

--- a/src/libvirt.c

+++ b/src/libvirt.c

@@ -19891,7 +19891,10 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,

  * 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.

+ * destination file.  By default, the copy destination will be treated as

+ * type='file', but using VIR_DOMAIN_BLOCK_REBASE_COPY_DEV treats the

+ * destination as type='block' (affecting how virDomainGetBlockInfo() will

+ * report allocation after pivoting).

  *

  * 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

@@ -19966,7 +19969,8 @@ virDomainBlockRebase(virDomainPtr dom, const char *disk,

         virCheckNonNullArgGoto(base, error);

     } else if (flags & (VIR_DOMAIN_BLOCK_REBASE_SHALLOW |

                         VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |

-                        VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)) {

+                        VIR_DOMAIN_BLOCK_REBASE_COPY_RAW |

+                        VIR_DOMAIN_BLOCK_REBASE_COPY_DEV)) {

         virReportInvalidArg(flags,

                             _("use of flags in %s requires a copy job"),

                             __FUNCTION__);

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c

index 162e039..c25c5ac 100644

--- a/src/qemu/qemu_driver.c

+++ b/src/qemu/qemu_driver.c

@@ -15781,7 +15781,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm,

     /* Preliminaries: find the disk we are editing, sanity checks */

     virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |

-                  VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT, -1);

+                  VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |

+                  VIR_DOMAIN_BLOCK_REBASE_COPY_DEV, -1);

     priv = vm->privateData;

     cfg = virQEMUDriverGetConfig(driver);

@@ -15842,25 +15843,34 @@ qemuDomainBlockCopy(virDomainObjPtr vm,

             virReportSystemError(errno, _("unable to stat for disk %s: %s"),

                                  disk->dst, dest);

             goto endjob;

-        } else if (flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) {

+        } else if (flags & (VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |

+                            VIR_DOMAIN_BLOCK_REBASE_COPY_DEV)) {

             virReportSystemError(errno,

                                  _("missing destination file for disk %s: %s"),

                                  disk->dst, dest);

             goto endjob;

         }

-    } else if (!S_ISBLK(st.st_mode) && st.st_size &&

-               !(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) {

-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,

-                       _("external destination file for disk %s already "

-                         "exists and is not a block device: %s"),

-                       disk->dst, dest);

-        goto endjob;

+    } else if (!S_ISBLK(st.st_mode)) {

+        if (st.st_size && !(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) {

+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,

+                           _("external destination file for disk %s already "

+                             "exists and is not a block device: %s"),

+                           disk->dst, dest);

+            goto endjob;

+        }

+        if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_DEV) {

+            virReportError(VIR_ERR_INVALID_ARG,

+                           _("blockdev flag requested for disk %s, but file "

+                             "'%s' is not a block device"), disk->dst, dest);

+            goto endjob;

+        }

     }

     if (VIR_ALLOC(mirror) < 0)

         goto endjob;

     /* XXX Allow non-file mirror destinations */

-    mirror->type = VIR_STORAGE_TYPE_FILE;

+    mirror->type = flags & VIR_DOMAIN_BLOCK_REBASE_COPY_DEV ?

+        VIR_STORAGE_TYPE_BLOCK : VIR_STORAGE_TYPE_FILE;

     if (format) {

         if ((mirror->format = virStorageFileFormatTypeFromString(format)) <= 0) {

@@ -15954,7 +15964,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,

                   VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |

                   VIR_DOMAIN_BLOCK_REBASE_COPY |

                   VIR_DOMAIN_BLOCK_REBASE_COPY_RAW |

-                  VIR_DOMAIN_BLOCK_REBASE_RELATIVE, -1);

+                  VIR_DOMAIN_BLOCK_REBASE_RELATIVE |

+                  VIR_DOMAIN_BLOCK_REBASE_COPY_DEV, -1);

     if (!(vm = qemuDomObjFromDomain(dom)))

         return -1;

@@ -15982,7 +15993,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,

     }

     flags &= (VIR_DOMAIN_BLOCK_REBASE_SHALLOW |

-              VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT);

+              VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |

+              VIR_DOMAIN_BLOCK_REBASE_COPY_DEV);

     ret = qemuDomainBlockCopy(vm, dom->conn, path, base, format,

                               bandwidth, flags);

     vm = NULL;

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml

index 46f2a3e..7495a45 100644

--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml

+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml

@@ -17,8 +17,8 @@

     <disk type='block' device='disk'>

       <source dev='/dev/HostVG/QEMUGuest1'/>

       <backingStore/>

-      <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' job='copy' ready='yes'>

-        <source file='/dev/HostVG/QEMUGuest1Copy'/>

+      <mirror type='block' job='copy' ready='yes'>

+        <source dev='/dev/HostVG/QEMUGuest1Copy'/>

       </mirror>

       <target dev='hda' bus='ide'/>

       <address type='drive' controller='0' bus='0' target='0' unit='0'/>

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c

index 28f5319..8f79b55 100644

--- a/tools/virsh-domain.c

+++ b/tools/virsh-domain.c

@@ -1550,6 +1550,8 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,

             flags |= VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT;

         if (vshCommandOptBool(cmd, "raw"))

             flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_RAW;

+        if (vshCommandOptBool(cmd, "blockdev"))

+            flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_DEV;

         if (vshCommandOptStringReq(ctl, cmd, "dest", &base) < 0)

             goto cleanup;

         ret = virDomainBlockRebase(dom, path, base, bandwidth, flags);

@@ -1857,6 +1859,10 @@ static const vshCmdOptDef opts_block_copy[] = {

      .type = VSH_OT_BOOL,

      .help = N_("use raw destination file")

     },

+    {.name = "blockdev",

+     .type = VSH_OT_BOOL,

+     .help = N_("copy destination is block device instead of regular file")

+    },

     {.name = "wait",

      .type = VSH_OT_BOOL,

      .help = N_("wait for job to reach mirroring phase")

diff --git a/tools/virsh.pod b/tools/virsh.pod

index c5b176a..46ef01d 100644

--- a/tools/virsh.pod

+++ b/tools/virsh.pod

@@ -959,7 +959,8 @@ unlimited. The hypervisor can choose whether to reject the value or

 convert it to the maximum value allowed.

 =item B<blockcopy> I<domain> I<path> I<dest> [I<bandwidth>] [I<--shallow>]

-[I<--reuse-external>] [I<--raw>] [I<--wait> [I<--async>] [I<--verbose>]]

+[I<--reuse-external>] [I<--raw>] [I<--blockdev>]

+[I<--wait> [I<--async>] [I<--verbose>]]

 [{I<--pivot> | I<--finish>}] [I<--timeout> B<seconds>]

 Copy a disk backing image chain to I<dest>. By default, this command

@@ -977,7 +978,9 @@ 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

 I<--reuse-external> is specified, the existing destination is probed

 for a format; and in all other cases, the destination format will

-match the source format.

+match the source format.  The destination is treated as a regular

+file unless I<--blockdev> is used to signal that it is a block

+device.

 By default, the copy job runs in the background, and consists of two

 phases.  Initially, the job must copy all data from the source, and

-- 

2.3.0