[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] blockcopy: allow block device destination



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.

I still plan to add a virDomainBlockCopy() API that takes the
destination disk as XML, allowing full expressive capability
to copy to a network disk.  But a new API can't be backported,
while a new flag to an existing API can.  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.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Test it.

Signed-off-by: Eric Blake <eblake redhat com>
---
 include/libvirt/libvirt.h.in                       |    2 ++
 src/libvirt.c                                      |    8 ++++++--
 src/qemu/qemu_driver.c                             |   12 ++++++++----
 .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml  |    4 ++--
 tools/virsh-domain.c                               |    6 ++++++
 tools/virsh.pod                                    |    7 +++++--
 6 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 47ea695..f2a02ea 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2590,6 +2590,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 992e4f2..c4643e8 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -19881,7 +19881,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
@@ -19950,7 +19953,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 b6219ba..e74227e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15295,7 +15295,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);
@@ -15374,7 +15375,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
     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) {
@@ -15466,7 +15468,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;
@@ -15482,7 +15485,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
             format = "raw";
         flags &= ~(VIR_DOMAIN_BLOCK_REBASE_COPY |
                    VIR_DOMAIN_BLOCK_REBASE_COPY_RAW);
-        return qemuDomainBlockCopy(vm, dom->conn, path, base, format, bandwidth, flags);
+        return qemuDomainBlockCopy(vm, dom->conn, path, base, format,
+                                   bandwidth, flags);
     }

     return qemuDomainBlockJobImpl(vm, dom->conn, path, base, bandwidth, 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 f7193cb..1fbd36e 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1521,6 +1521,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);
@@ -1828,6 +1830,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 f07deec..8178868 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -873,7 +873,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
@@ -891,7 +892,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
-- 
1.7.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]