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

Re: [libvirt] [PATCH v4 2/4] storage: report error rather than warning if backing files doesn't exist



On 07/15/2013 11:31 PM, Ján Tomko wrote:
On 07/02/2013 11:35 AM, Guannan Ren wrote:
If one of backing files for disk source doesn't exist, the guest will not
be able to find and use the disk even though the disk still exists in
guest xml definition. So reporting an error make more sense.

Adding virFileAccessibleAs() to check if the backing file described in
disk meta exist in real path. If not, report error. the uid and gid
arguments don't have so much meannings for F_OK, so give 0 for them.
---
  src/util/virstoragefile.c | 23 +++++++++++++++--------
  tests/virstoragetest.c    | 16 ++++++++--------
  2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 27aa4fe..cb61e5b 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -870,14 +877,10 @@ virStorageFileGetMetadataInternal(const char *path,
                                         !!directory, backing,
                                         &meta->directory,
                                         &meta->backingStore) < 0) {
-                    /* the backing file is (currently) unavailable, treat this
-                     * file as standalone:
-                     * backingStoreRaw is kept to mark broken image chains */
-                    meta->backingStoreIsFile = false;
-                    backingFormat = VIR_STORAGE_FILE_NONE;
-                    VIR_WARN("Backing file '%s' of image '%s' is missing.",
-                             meta->backingStoreRaw, path);
-
+                    VIR_ERROR(_("Backing file '%s' of image '%s' is missing."),
+                              meta->backingStoreRaw, path);
+                    VIR_FREE(backing);
+                    goto cleanup;
                  }
              }
              VIR_FREE(backing);
This change means you won't be able to start the pool if one of the files is
missing a backing file. I've forwarded a patch [1] from/for [2] that ignores
missing files on pool start and there is a bug [3] requesting that we ignore
other files as well. I feel like this is going in the other direction.

Wouldn't it be enough to check for them on domain start-up and leave the pool
running even if one of those volumes doesn't have an existing backing file?

Jan

[1] https://www.redhat.com/archives/libvir-list/2013-July/msg00635.html
[2] https://bugzilla.redhat.com/show_bug.cgi?id=977706
[3] https://bugzilla.redhat.com/show_bug.cgi?id=710866

As what you pointed out is close to my issue, I chose to propose a patch on bug 977706 first
https://www.redhat.com/archives/libvir-list/2013-July/msg01026.html.

Guannan


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