9119d9
From 4dedf415bbbe7edef4c530e902be1fd266ba95b0 Mon Sep 17 00:00:00 2001
9119d9
Message-Id: <4dedf415bbbe7edef4c530e902be1fd266ba95b0@dist-git>
9119d9
From: Peter Krempa <pkrempa@redhat.com>
9119d9
Date: Wed, 24 Sep 2014 11:01:29 +0200
9119d9
Subject: [PATCH] qemu: Report better errors from broken backing chains
9119d9
9119d9
https://bugzilla.redhat.com/show_bug.cgi?id=1138231
9119d9
9119d9
Request erroring out from the backing chain traveller and drop qemu's
9119d9
internal backing chain integrity tester.
9119d9
9119d9
The backing chain traveller reports errors by itself with possibly more
9119d9
detail than qemuDiskChainCheckBroken ever could.
9119d9
9119d9
We also need to make sure that we reconnect to existing qemu instances
9119d9
even at the cost of losing the backing chain info (this really should be
9119d9
stored in the XML rather than reloaded from disk, but that needs some
9119d9
work).
9119d9
9119d9
(cherry picked from commit 639a00984a1b6b328093c2e89f2b329ed4f341e4)
9119d9
9119d9
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
9119d9
---
9119d9
 src/qemu/qemu_domain.c  | 29 ++++-------------------------
9119d9
 src/qemu/qemu_domain.h  |  3 ++-
9119d9
 src/qemu/qemu_driver.c  | 11 ++++++-----
9119d9
 src/qemu/qemu_hotplug.c |  2 +-
9119d9
 src/qemu/qemu_process.c | 12 ++++++++----
9119d9
 5 files changed, 21 insertions(+), 36 deletions(-)
9119d9
9119d9
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
9119d9
index 3766ceb..ac70f62 100644
9119d9
--- a/src/qemu/qemu_domain.c
9119d9
+++ b/src/qemu/qemu_domain.c
9119d9
@@ -2515,27 +2515,6 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
9119d9
     return -1;
9119d9
 }
9119d9
 
9119d9
-static int
9119d9
-qemuDiskChainCheckBroken(virDomainDiskDefPtr disk)
9119d9
-{
9119d9
-    char *brokenFile = NULL;
9119d9
-
9119d9
-    if (!virDomainDiskGetSource(disk))
9119d9
-        return 0;
9119d9
-
9119d9
-    if (virStorageFileChainGetBroken(disk->src, &brokenFile) < 0)
9119d9
-        return -1;
9119d9
-
9119d9
-    if (brokenFile) {
9119d9
-        virReportError(VIR_ERR_INVALID_ARG,
9119d9
-                       _("Backing file '%s' of image '%s' is missing."),
9119d9
-                       brokenFile, virDomainDiskGetSource(disk));
9119d9
-        VIR_FREE(brokenFile);
9119d9
-        return -1;
9119d9
-    }
9119d9
-
9119d9
-    return 0;
9119d9
-}
9119d9
 
9119d9
 int
9119d9
 qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
9119d9
@@ -2565,8 +2544,7 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
9119d9
             virFileExists(path))
9119d9
             continue;
9119d9
 
9119d9
-        if (qemuDomainDetermineDiskChain(driver, vm, disk, false) >= 0 &&
9119d9
-            qemuDiskChainCheckBroken(disk) >= 0)
9119d9
+        if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) >= 0)
9119d9
             continue;
9119d9
 
9119d9
         if (disk->startupPolicy &&
9119d9
@@ -2711,7 +2689,8 @@ int
9119d9
 qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
9119d9
                              virDomainObjPtr vm,
9119d9
                              virDomainDiskDefPtr disk,
9119d9
-                             bool force_probe)
9119d9
+                             bool force_probe,
9119d9
+                             bool report_broken)
9119d9
 {
9119d9
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
9119d9
     int ret = 0;
9119d9
@@ -2733,7 +2712,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
9119d9
     if (virStorageFileGetMetadata(disk->src,
9119d9
                                   uid, gid,
9119d9
                                   cfg->allowDiskFormatProbing,
9119d9
-                                  false) < 0)
9119d9
+                                  report_broken) < 0)
9119d9
         ret = -1;
9119d9
 
9119d9
  cleanup:
9119d9
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
9119d9
index c1d1edf..845d3c7 100644
9119d9
--- a/src/qemu/qemu_domain.h
9119d9
+++ b/src/qemu/qemu_domain.h
9119d9
@@ -370,7 +370,8 @@ int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
9119d9
 int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
9119d9
                                  virDomainObjPtr vm,
9119d9
                                  virDomainDiskDefPtr disk,
9119d9
-                                 bool force_probe);
9119d9
+                                 bool force_probe,
9119d9
+                                 bool report_broken);
9119d9
 
9119d9
 int qemuDomainStorageFileInit(virQEMUDriverPtr driver,
9119d9
                               virDomainObjPtr vm,
9119d9
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
9119d9
index 6629946..46be72c 100644
9119d9
--- a/src/qemu/qemu_driver.c
9119d9
+++ b/src/qemu/qemu_driver.c
9119d9
@@ -6793,7 +6793,7 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
9119d9
     if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
9119d9
         goto end;
9119d9
 
9119d9
-    if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0)
9119d9
+    if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
9119d9
         goto end;
9119d9
 
9119d9
     switch (disk->device) {
9119d9
@@ -13171,7 +13171,8 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
9119d9
     for (i = 0; i < snap->def->ndisks; i++) {
9119d9
         if (snap->def->disks[i].snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
9119d9
             continue;
9119d9
-        qemuDomainDetermineDiskChain(driver, vm, vm->def->disks[i], true);
9119d9
+        ignore_value(qemuDomainDetermineDiskChain(driver, vm, vm->def->disks[i],
9119d9
+                                                  true, true));
9119d9
     }
9119d9
     if (orig_err) {
9119d9
         virSetError(orig_err);
9119d9
@@ -14980,7 +14981,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
9119d9
         oldsrc = disk->src;
9119d9
         disk->src = disk->mirror;
9119d9
 
9119d9
-        if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0)
9119d9
+        if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
9119d9
             goto cleanup;
9119d9
 
9119d9
         if (disk->mirror->format &&
9119d9
@@ -15409,7 +15410,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
9119d9
         goto endjob;
9119d9
     }
9119d9
 
9119d9
-    if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0)
9119d9
+    if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
9119d9
         goto endjob;
9119d9
 
9119d9
     if ((flags & VIR_DOMAIN_BLOCK_REBASE_SHALLOW) &&
9119d9
@@ -15647,7 +15648,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
9119d9
                        disk->dst);
9119d9
         goto endjob;
9119d9
     }
9119d9
-    if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0)
9119d9
+    if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
9119d9
         goto endjob;
9119d9
 
9119d9
     if (!top)
9119d9
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
9119d9
index a364c52..33e2dff 100644
9119d9
--- a/src/qemu/qemu_hotplug.c
9119d9
+++ b/src/qemu/qemu_hotplug.c
9119d9
@@ -779,7 +779,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
9119d9
     if (qemuSetUnprivSGIO(dev) < 0)
9119d9
         goto end;
9119d9
 
9119d9
-    if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0)
9119d9
+    if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
9119d9
         goto end;
9119d9
 
9119d9
     switch (disk->device)  {
9119d9
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
9119d9
index 5c3b3c8..9294619 100644
9119d9
--- a/src/qemu/qemu_process.c
9119d9
+++ b/src/qemu/qemu_process.c
9119d9
@@ -1089,7 +1089,8 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
9119d9
             save = disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
9119d9
             disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
9119d9
             disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
9119d9
-            qemuDomainDetermineDiskChain(driver, vm, disk, true);
9119d9
+            ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk,
9119d9
+                                                      true, true));
9119d9
         } else if (disk->mirror &&
9119d9
                    (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY ||
9119d9
                     type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)) {
9119d9
@@ -3428,9 +3429,12 @@ qemuProcessReconnect(void *opaque)
9119d9
         if (virStorageTranslateDiskSourcePool(conn, obj->def->disks[i]) < 0)
9119d9
             goto error;
9119d9
 
9119d9
-        /* XXX we should be able to restore all data from XML in the future */
9119d9
-        if (qemuDomainDetermineDiskChain(driver, obj,
9119d9
-                                         obj->def->disks[i], true) < 0)
9119d9
+        /* XXX we should be able to restore all data from XML in the future.
9119d9
+         * This should be the only place that calls qemuDomainDetermineDiskChain
9119d9
+         * with @report_broken == false to guarantee best-effort domain
9119d9
+         * reconnect */
9119d9
+        if (qemuDomainDetermineDiskChain(driver, obj, obj->def->disks[i],
9119d9
+                                         true, false) < 0)
9119d9
             goto error;
9119d9
 
9119d9
         dev.type = VIR_DOMAIN_DEVICE_DISK;
9119d9
-- 
9119d9
2.1.1
9119d9