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

[libvirt] [PATCHv5 06/23] blockjob: enhance xml to track mirrors across libvirtd restart



In order to track a block copy job across libvirtd restarts, we
need to save internal XML that tracks the name of the file
holding the mirror.  Displaying this name in dumpxml might also
be useful to the user, even if we don't yet have a way to (re-)
start a domain with mirroring enabled up front.  This is done
with a new <mirror> sub-element to <disk>, as in:

    <disk type='file' device='disk'>
      <driver name='qemu' type='raw'/>
      <source file='/var/lib/libvirt/images/original.img'/>
      <mirror file='/var/lib/libvirt/images/copy.img' format='qcow2' ready='yes'/>
      ...
    </disk>

For now, the element is output-only, in live domains; it is ignored
when defining a domain or hot-plugging a disk (since those contexts
use VIR_DOMAIN_XML_INACTIVE in parsing).  The 'ready' attribute appears
when libvirt knows that the job has changed from the initial pulling
phase over to the mirroring phase, although absence of the attribute
is not a sure indicator of the current phase.  If we come up with a way
to make qemu start with mirroring enabled, we can relax the xml
restriction, and allow <mirror> (but not attribute 'ready') on input.
Testing active-only XML meant tweaking the testsuite slightly, but it
was worth it.

* docs/schemas/domaincommon.rng (diskspec): Add diskMirror.
* docs/formatdomain.html.in (elementsDisks): Document it.
* src/conf/domain_conf.h (_virDomainDiskDef): New members.
* src/conf/domain_conf.c (virDomainDiskDefFree): Clean them.
(virDomainDiskDefParseXML): Parse them, but only internally.
(virDomainDiskDefFormat): Output them.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: New test file.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror.xml: Likewise.
* tests/qemuxml2xmltest.c (testInfo): Alter members.
(testCompareXMLToXMLHelper): Allow more test control.
(mymain): Run new test.
---

was 7/18 in v4
v5: allow but ignore <mirror> on inactive, and add tests

 docs/formatdomain.html.in                          |   13 ++++
 docs/schemas/domaincommon.rng                      |   24 +++++++-
 src/conf/domain_conf.c                             |   62 +++++++++++++++++---
 src/conf/domain_conf.h                             |    4 +
 .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml  |   42 +++++++++++++
 .../qemuxml2xmlout-disk-mirror.xml                 |   40 +++++++++++++
 tests/qemuxml2xmltest.c                            |   42 ++++++++------
 7 files changed, 198 insertions(+), 29 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index f8ec6ff..a99281d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1296,6 +1296,19 @@
         </table>
         <span class="since">Since 0.9.7</span>
         </dd>
+      <dt><code>mirror</code></dt>
+      <dd>
+        This element is present if the hypervisor has started a block
+        copy operation (via the <code>virDomainBlockCopy</code> API),
+        where the mirror location in attribute <code>file</code> will
+        eventually have the same contents as the source, and with the
+        file format in attribute <code>format</code> (which might
+        differ from the format of the source).  If
+        attribute <code>ready</code> is present, then it is known the
+        disk is ready to pivot; otherwise, the disk is probably still
+        copying.  For now, this element only valid in output; it is
+        rejected on input.  <span class="since">Since 0.9.12</span>
+      </dd>
       <dt><code>target</code></dt>
       <dd>The <code>target</code> element controls the bus / device
         under which the disk is exposed to the guest
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 0cc04af..44f0e8c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -772,6 +772,9 @@
         <ref name="driver"/>
       </optional>
       <optional>
+        <ref name='diskMirror'/>
+      </optional>
+      <optional>
         <ref name="diskAuth"/>
       </optional>
       <ref name="target"/>
@@ -1013,9 +1016,7 @@
     </element>
   </define>
   <!--
-      Disk may use a special driver for access. Currently this is
-      only defined for Xen for tap/aio and file, but will certainly be
-      extended in the future, and libvirt doesn't look for specific values.
+      Disk may use a special driver for access.
     -->
   <define name="driver">
     <element name="driver">
@@ -3024,6 +3025,23 @@
       <empty/>
     </element>
   </define>
+  <define name='diskMirror'>
+    <element name='mirror'>
+      <attribute name='file'>
+        <ref name='absFilePath'/>
+      </attribute>
+      <optional>
+        <attribute name='format'>
+          <ref name="genericName"/>
+        </attribute>
+      </optional>
+      <optional>
+        <attribute name='ready'>
+          <value>yes</value>
+        </attribute>
+      </optional>
+    </element>
+  </define>
   <define name="diskAuth">
     <element name="auth">
       <attribute name="username">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a9c5cbc..fe5c92c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -933,6 +933,8 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
     VIR_FREE(def->dst);
     VIR_FREE(def->driverName);
     VIR_FREE(def->driverType);
+    VIR_FREE(def->mirror);
+    VIR_FREE(def->mirrorFormat);
     VIR_FREE(def->auth.username);
     if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
         VIR_FREE(def->auth.secret.usage);
@@ -3318,6 +3320,9 @@ virDomainDiskDefParseXML(virCapsPtr caps,
     char *ioeventfd = NULL;
     char *event_idx = NULL;
     char *copy_on_read = NULL;
+    char *mirror = NULL;
+    char *mirrorFormat = NULL;
+    bool mirroring = false;
     char *devaddr = NULL;
     virStorageEncryptionPtr encryption = NULL;
     char *serial = NULL;
@@ -3353,8 +3358,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
     cur = node->children;
     while (cur != NULL) {
         if (cur->type == XML_ELEMENT_NODE) {
-            if ((source == NULL && hosts == NULL) &&
-                (xmlStrEqual(cur->name, BAD_CAST "source"))) {
+            if (source == NULL && hosts == NULL &&
+                xmlStrEqual(cur->name, BAD_CAST "source")) {

                 sourceNode = cur;

@@ -3431,8 +3436,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
                    those broken apps */
                 if (source && STREQ(source, ""))
                     VIR_FREE(source);
-            } else if ((target == NULL) &&
-                       (xmlStrEqual(cur->name, BAD_CAST "target"))) {
+            } else if (target == NULL &&
+                       xmlStrEqual(cur->name, BAD_CAST "target")) {
                 target = virXMLPropString(cur, "dev");
                 bus = virXMLPropString(cur, "bus");
                 tray = virXMLPropString(cur, "tray");
@@ -3442,8 +3447,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
                 if (target &&
                     STRPREFIX(target, "ioemu:"))
                     memmove(target, target+6, strlen(target)-5);
-            } else if ((driverName == NULL) &&
-                       (xmlStrEqual(cur->name, BAD_CAST "driver"))) {
+            } else if (driverName == NULL &&
+                       xmlStrEqual(cur->name, BAD_CAST "driver")) {
                 driverName = virXMLPropString(cur, "name");
                 driverType = virXMLPropString(cur, "type");
                 cachetag = virXMLPropString(cur, "cache");
@@ -3453,6 +3458,22 @@ virDomainDiskDefParseXML(virCapsPtr caps,
                 ioeventfd = virXMLPropString(cur, "ioeventfd");
                 event_idx = virXMLPropString(cur, "event_idx");
                 copy_on_read = virXMLPropString(cur, "copy_on_read");
+            } else if (mirror == NULL &&
+                       xmlStrEqual(cur->name, BAD_CAST "mirror") &&
+                       !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+                char *ready;
+                mirror = virXMLPropString(cur, "file");
+                if (!mirror) {
+                    virDomainReportError(VIR_ERR_XML_ERROR, "%s",
+                                         _("mirror requires file name"));
+                    goto error;
+                }
+                mirrorFormat = virXMLPropString(cur, "format");
+                ready = virXMLPropString(cur, "ready");
+                if (ready) {
+                    mirroring = true;
+                    VIR_FREE(ready);
+                }
             } else if (xmlStrEqual(cur->name, BAD_CAST "auth")) {
                 authUsername = virXMLPropString(cur, "username");
                 if (authUsername == NULL) {
@@ -3577,8 +3598,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
                                                            cur);
                 if (encryption == NULL)
                     goto error;
-            } else if ((serial == NULL) &&
-                       (xmlStrEqual(cur->name, BAD_CAST "serial"))) {
+            } else if (serial == NULL &&
+                       xmlStrEqual(cur->name, BAD_CAST "serial")) {
                 serial = (char *)xmlNodeGetContent(cur);
             } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) {
                 /* boot is parsed as part of virDomainDeviceInfoParseXML */
@@ -3867,6 +3888,11 @@ virDomainDiskDefParseXML(virCapsPtr caps,
     driverName = NULL;
     def->driverType = driverType;
     driverType = NULL;
+    def->mirror = mirror;
+    mirror = NULL;
+    def->mirrorFormat = mirrorFormat;
+    mirrorFormat = NULL;
+    def->mirroring = mirroring;
     def->encryption = encryption;
     encryption = NULL;
     def->serial = serial;
@@ -3882,6 +3908,12 @@ virDomainDiskDefParseXML(virCapsPtr caps,
         !(def->driverName = strdup(caps->defaultDiskDriverName)))
         goto no_memory;

+
+    if (def->mirror && !def->mirrorFormat &&
+        caps->defaultDiskDriverType &&
+        !(def->mirrorFormat = strdup(caps->defaultDiskDriverType)))
+        goto no_memory;
+
     if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE
         && virDomainDiskDefAssignAddress(caps, def) < 0)
         goto error;
@@ -3907,6 +3939,8 @@ cleanup:
     VIR_FREE(authUsage);
     VIR_FREE(driverType);
     VIR_FREE(driverName);
+    VIR_FREE(mirror);
+    VIR_FREE(mirrorFormat);
     VIR_FREE(cachetag);
     VIR_FREE(error_policy);
     VIR_FREE(rerror_policy);
@@ -10829,6 +10863,18 @@ virDomainDiskDefFormat(virBufferPtr buf,
         }
     }

+    /* For now, mirroring is currently output-only: we only output it
+     * for live domains, therefore we ignore it on input except for
+     * the internal parse on libvirtd restart.  */
+    if (def->mirror && !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+        virBufferEscapeString(buf, "      <mirror file='%s'", def->mirror);
+        if (def->mirrorFormat)
+            virBufferAsprintf(buf, " format='%s'", def->mirrorFormat);
+        if (def->mirroring)
+            virBufferAddLit(buf, " ready='yes'");
+        virBufferAddLit(buf, "/>\n");
+    }
+
     virBufferAsprintf(buf, "      <target dev='%s' bus='%s'",
                       def->dst, bus);
     if ((def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0eed60e..5aa8fc1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -563,6 +563,10 @@ struct _virDomainDiskDef {
     char *driverName;
     char *driverType;

+    char *mirror;
+    char *mirrorFormat;
+    bool mirroring;
+
     virDomainBlockIoTuneInfo blkdeviotune;

     char *serial;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
new file mode 100644
index 0000000..0c95724
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
@@ -0,0 +1,42 @@
+<domain type='qemu' id='1'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <mirror file='/dev/HostVG/QEMUGuest1Copy' ready='yes'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <disk type='block' device='cdrom'>
+      <source dev='/dev/HostVG/QEMUGuest2'/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/data.img'/>
+      <mirror file='/tmp/copy.img' format='qcow2'/>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/logs.img'/>
+      <target dev='vdb' bus='virtio'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror.xml
new file mode 100644
index 0000000..00111be
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror.xml
@@ -0,0 +1,40 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <disk type='block' device='cdrom'>
+      <source dev='/dev/HostVG/QEMUGuest2'/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/data.img'/>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/logs.img'/>
+      <target dev='vdb' bus='virtio'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 42e4d9d..9bca066 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -54,10 +54,16 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool live)
     return ret;
 }

+enum {
+    WHEN_INACTIVE = 1,
+    WHEN_ACTIVE = 2,
+    WHEN_EITHER = 3,
+};
+
 struct testInfo {
     const char *name;
-    int different;
-    bool inactive_only;
+    bool different;
+    int when;
 };

 static int
@@ -74,17 +80,15 @@ testCompareXMLToXMLHelper(const void *data)
                     abs_srcdir, info->name) < 0)
         goto cleanup;

-    if (info->different) {
-        ret = testCompareXMLToXMLFiles(xml_in, xml_out, false);
-    } else {
-        ret = testCompareXMLToXMLFiles(xml_in, xml_in, false);
+    if (info->when & WHEN_INACTIVE) {
+        ret = testCompareXMLToXMLFiles(xml_in,
+                                       info->different ? xml_out : xml_in,
+                                       false);
     }
-    if (!info->inactive_only) {
-        if (info->different) {
-            ret = testCompareXMLToXMLFiles(xml_in, xml_out, true);
-        } else {
-            ret = testCompareXMLToXMLFiles(xml_in, xml_in, true);
-        }
+    if (info->when & WHEN_ACTIVE) {
+        ret = testCompareXMLToXMLFiles(xml_in,
+                                       info->different ? xml_out : xml_in,
+                                       true);
     }

 cleanup:
@@ -102,19 +106,19 @@ mymain(void)
     if ((driver.caps = testQemuCapsInit()) == NULL)
         return EXIT_FAILURE;

-# define DO_TEST_FULL(name, is_different, inactive)                     \
+# define DO_TEST_FULL(name, is_different, when)                         \
     do {                                                                \
-        const struct testInfo info = {name, is_different, inactive};    \
+        const struct testInfo info = {name, is_different, when};        \
         if (virtTestRun("QEMU XML-2-XML " name,                         \
                         1, testCompareXMLToXMLHelper, &info) < 0)       \
             ret = -1;                                                   \
     } while (0)

 # define DO_TEST(name) \
-    DO_TEST_FULL(name, 0, false)
+    DO_TEST_FULL(name, false, WHEN_EITHER)

 # define DO_TEST_DIFFERENT(name) \
-    DO_TEST_FULL(name, 1, false)
+    DO_TEST_FULL(name, true, WHEN_EITHER)

     /* Unset or set all envvars here that are copied in qemudBuildCommandLine
      * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
@@ -151,6 +155,8 @@ mymain(void)
     DO_TEST("disk-scsi-device");
     DO_TEST("disk-scsi-vscsi");
     DO_TEST("disk-scsi-virtio-scsi");
+    DO_TEST_FULL("disk-mirror", false, WHEN_ACTIVE);
+    DO_TEST_FULL("disk-mirror", true, WHEN_INACTIVE);
     DO_TEST("graphics-listen-network");
     DO_TEST("graphics-vnc");
     DO_TEST("graphics-vnc-sasl");
@@ -208,8 +214,8 @@ mymain(void)
     DO_TEST("usb-redir");
     DO_TEST("blkdeviotune");

-    DO_TEST_FULL("seclabel-dynamic-baselabel", false, true);
-    DO_TEST_FULL("seclabel-dynamic-override", false, true);
+    DO_TEST_FULL("seclabel-dynamic-baselabel", false, WHEN_INACTIVE);
+    DO_TEST_FULL("seclabel-dynamic-override", false, WHEN_INACTIVE);
     DO_TEST("seclabel-static");
     DO_TEST("seclabel-none");

-- 
1.7.7.6


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