From 7010768c5e9d023a7908a649e69b5627e9eba24c Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 9 Apr 2014 15:36:30 -0600 Subject: [PATCH] conf: provide details on network backing store So far, my work has been merely preserving the status quo of backing file analysis. But this patch starts to tread in the territory of making the backing chain code more powerful - we will eventually support network storage containing non-raw formats. Here, we expose metadata information about a network backing store, even if that information is still hardcoded to a raw format for now. * src/util/virstoragefile.c (virStorageFileGetMetadataRecurse): Also populate struct for non-file backing. (virStorageFileGetMetadata, virStorageFileGetMetadatainternal): Recognize non-file top image. (virFindBackingFile): Add comment. (virStorageFileChainGetBroken): Adjust comment, ensure output is set. * tests/virstoragetest.c (mymain): Update test to reflect it. Signed-off-by: Eric Blake --- src/util/virstoragefile.c | 73 ++++++++++++++++++++++++++++----------- tests/virstoragetest.c | 17 ++++++--- 2 files changed, 65 insertions(+), 25 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index f5fe8adde6..2349607ac7 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -569,6 +569,14 @@ static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) virFindBackingFile(const char *start, const char *path, char **directory, char **canonical) { + /* FIXME - when we eventually allow non-raw network devices, we + * must ensure that we handle backing files the same way as qemu. + * For a qcow2 top file of gluster://server/vol/img, qemu treats + * the relative backing file 'rel' as meaning + * 'gluster://server/vol/rel', while the backing file '/abs' is + * used as a local file. But we cannot canonicalize network + * devices via canonicalize_file_name(), because they are not part + * of the local file system. */ char *combined = NULL; int ret = -1; @@ -874,6 +882,12 @@ virStorageFileGetMetadataInternal(const char *path, meta->backingStoreRaw, path); } + } else { + if (VIR_STRDUP(meta->backingStoreRaw, backing) < 0) { + VIR_FREE(backing); + goto cleanup; + } + backingFormat = VIR_STORAGE_FILE_RAW; } VIR_FREE(backing); meta->backingStoreFormat = backingFormat; @@ -1132,18 +1146,32 @@ virStorageFileGetMetadataRecurse(const char *path, const char *canonPath, if (virHashAddEntry(cycle, canonPath, (void *)1) < 0) return -1; - if ((fd = virFileOpenAs(canonPath, O_RDONLY, 0, uid, gid, 0)) < 0) { - virReportSystemError(-fd, _("Failed to open file '%s'"), path); - return -1; - } + if (virBackingStoreIsFile(path)) { + if ((fd = virFileOpenAs(canonPath, O_RDONLY, 0, uid, gid, 0)) < 0) { + virReportSystemError(-fd, _("Failed to open file '%s'"), path); + return -1; + } - ret = virStorageFileGetMetadataFromFDInternal(path, canonPath, directory, - fd, format, meta); + ret = virStorageFileGetMetadataFromFDInternal(path, canonPath, + directory, + fd, format, meta); - if (VIR_CLOSE(fd) < 0) - VIR_WARN("could not close file %s", path); + if (VIR_CLOSE(fd) < 0) + VIR_WARN("could not close file %s", path); + } else { + /* FIXME: when the proper storage drivers are compiled in, it + * would be nice to read metadata from the network storage to + * allow for non-raw images. */ + if (VIR_STRDUP(meta->path, path) < 0) + return -1; + if (VIR_STRDUP(meta->canonPath, path) < 0) + return -1; + meta->type = VIR_STORAGE_TYPE_NETWORK; + meta->format = VIR_STORAGE_FILE_RAW; + ret = 0; + } - if (ret == 0 && meta->backingStoreIsFile) { + if (ret == 0 && meta->backingStore) { virStorageFileMetadataPtr backing; if (meta->backingStoreFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) @@ -1207,12 +1235,16 @@ virStorageFileGetMetadata(const char *path, int format, if (!cycle) return NULL; - if (!(canonPath = canonicalize_file_name(path))) { - virReportSystemError(errno, _("unable to resolve '%s'"), path); - goto cleanup; - } - if (!(directory = mdir_name(path))) { - virReportOOMError(); + if (virBackingStoreIsFile(path)) { + if (!(canonPath = canonicalize_file_name(path))) { + virReportSystemError(errno, _("unable to resolve '%s'"), path); + goto cleanup; + } + if (!(directory = mdir_name(path))) { + virReportOOMError(); + goto cleanup; + } + } else if (VIR_STRDUP(canonPath, path) < 0) { goto cleanup; } if (VIR_ALLOC(meta) < 0) @@ -1240,7 +1272,8 @@ virStorageFileGetMetadata(const char *path, int format, * * If CHAIN is broken, set *brokenFile to the broken file name, * otherwise set it to NULL. Caller MUST free *brokenFile after use. - * Return 0 on success, negative on error. + * Return 0 on success (including when brokenFile is set), negative on + * error (allocation failure). */ int virStorageFileChainGetBroken(virStorageFileMetadataPtr chain, @@ -1249,15 +1282,15 @@ virStorageFileChainGetBroken(virStorageFileMetadataPtr chain, virStorageFileMetadataPtr tmp; int ret = -1; + *brokenFile = NULL; + if (!chain) return 0; - *brokenFile = NULL; - tmp = chain; while (tmp) { - /* Break if no backing store, backing store is not file, or - * other problem such as infinite loop */ + /* Break when we hit end of chain; report error if we detected + * a missing backing file, infinite loop, or other error */ if (!tmp->backingStoreRaw) break; if (!tmp->backingStore) { diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index b3fcd328cf..3155912ca5 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -624,17 +624,24 @@ mymain(void) if (virCommandRun(cmd, NULL) < 0) ret = -1; qcow2.expBackingStore = "nbd:example.org:6000"; - qcow2.expBackingStoreRaw = NULL; + qcow2.expBackingStoreRaw = "nbd:example.org:6000"; qcow2.expBackingDirRel = NULL; qcow2.expBackingDirAbs = NULL; qcow2.expBackingFormat = VIR_STORAGE_FILE_RAW; /* Qcow2 file with backing protocol instead of file */ + testFileData nbd = { + .pathRel = "nbd:example.org:6000", + .pathAbs = "nbd:example.org:6000", + .canonPath = "nbd:example.org:6000", + .type = VIR_STORAGE_TYPE_NETWORK, + .format = VIR_STORAGE_FILE_RAW, + }; TEST_CHAIN(11, "qcow2", absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2), EXP_PASS, - (&qcow2), ALLOW_PROBE | EXP_PASS, - (&qcow2), EXP_PASS, - (&qcow2), ALLOW_PROBE | EXP_PASS); + (&qcow2, &nbd), EXP_PASS, + (&qcow2, &nbd), ALLOW_PROBE | EXP_PASS, + (&qcow2, &nbd), EXP_PASS, + (&qcow2, &nbd), ALLOW_PROBE | EXP_PASS); /* qed file */ testFileData qed = { -- GitLab