On 07/15/2013 09:31 PM, Martin Kletzander wrote:
s/doesn't/don't/ in $SUBJ
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
s/exist/exists/; s/the/The/
arguments don't have so much meannings for F_OK, so give 0 for them.
s/meannings/meaning/
---
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
@@ -580,6 +580,13 @@ virFindBackingFile(const char *start, bool start_is_dir, const char *path,
goto cleanup;
}
+ if (virFileAccessibleAs(combined, F_OK, 0, 0) < 0) {
+ virReportSystemError(errno,
+ _("Backing file '%s' does not exist"),
+ combined);
+ goto cleanup;
+ }
+
Pre-existing, but this nad other errors will be shadowed by the error in
qemuDomainBlockCommit. The existence of the file is already checked by:
if (!(*canonical = canonicalize_file_name(combined))) {
this ^^ line, so no need to call virFileAccessibleAs().
Calling virFileAccessibleAs() here is for more clear error message.
virReportSystemError(errno,
_("Can't canonicalize path '%s'"), path);
@@ -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;
And then you report one more error here, even though the function may
fail because of something else. I'd guess that's why it was a warning.
No idea why it skipped the errors, though.
The errors should be fixed, but this patch just adds more confusion to
the error reporting.
no, VIR_ERROR just adds one more error log item, the actual raised error
is still there not being shadowed.
Guannan
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list