diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 206f75a60155776a485c439af6a2e39f15901d26..b7781c9586e9bd41c518238bc86c0c9c3556e4e4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18537,9 +18537,8 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, if (iter(disk, path, 0, opaque) < 0) goto cleanup; - /* XXX: temporarily we need to select the second element of the backing - * chain to start as the first is the copy of the disk itself. */ - tmp = disk->src.backingStore ? disk->src.backingStore->backingStore : NULL; + + tmp = disk->src.backingStore; while (tmp && virStorageIsFile(tmp->path)) { if (!ignoreOpenFailure && tmp->backingStoreRaw && !tmp->backingStore) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 45ed8727b0fcbe6983331e814ed20c71e85839ee..bb9cb6b4ae379e91a4c989635b9a7f9e59efb243 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2239,10 +2239,10 @@ qemuDiskChainCheckBroken(virDomainDiskDefPtr disk) { char *brokenFile = NULL; - if (!virDomainDiskGetSource(disk) || !disk->src.backingStore) + if (!virDomainDiskGetSource(disk)) return 0; - if (virStorageFileChainGetBroken(disk->src.backingStore, &brokenFile) < 0) + if (virStorageFileChainGetBroken(&disk->src, &brokenFile) < 0) return -1; if (brokenFile) { @@ -2419,11 +2419,9 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, qemuDomainGetImageIds(cfg, vm, disk, &uid, &gid); - disk->src.backingStore = virStorageFileGetMetadata(src, - virDomainDiskGetFormat(disk), - uid, gid, - cfg->allowDiskFormatProbing); - if (!disk->src.backingStore) + if (virStorageFileGetMetadata(&disk->src, + uid, gid, + cfg->allowDiskFormatProbing) < 0) ret = -1; cleanup: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a610ed82e1dda5b05ca43cb0378a6d623577168a..c01da6c5d4eea6d76aa2a4205d878b8d3dd3a4d0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15123,7 +15123,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm, if ((flags & VIR_DOMAIN_BLOCK_REBASE_SHALLOW) && STREQ_NULLABLE(format, "raw") && - disk->src.backingStore->backingStore->path) { + disk->src.backingStore->path) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk '%s' has backing file, so raw shallow copy " "is not possible"), @@ -15330,8 +15330,8 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, if (!top) { top_canon = disk->src.path; - top_meta = disk->src.backingStore; - } else if (!(top_canon = virStorageFileChainLookup(disk->src.backingStore, + top_meta = &disk->src; + } else if (!(top_canon = virStorageFileChainLookup(&disk->src, top, &top_meta, &top_parent))) { goto endjob; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 1c8981537ec1363a2bdafc1fbca3b12a11664c57..32fc04a4a298e5f913888d39dfefd3fca5bc3a81 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -943,18 +943,15 @@ get_files(vahControl * ctl) for (i = 0; i < ctl->def->ndisks; i++) { virDomainDiskDefPtr disk = ctl->def->disks[i]; - const char *src = virDomainDiskGetSource(disk); - if (!src) + if (!virDomainDiskGetSource(disk)) continue; /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. */ if (!disk->src.backingStore) { bool probe = ctl->allowDiskFormatProbing; - disk->src.backingStore = virStorageFileGetMetadata(src, - virDomainDiskGetFormat(disk), - -1, -1, probe); + virStorageFileGetMetadata(&disk->src, -1, -1, probe); } /* XXX passing ignoreOpenFailure = true to get back to the behavior diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index a716b5d743d4fbdf0d0b4bde609b7b2e0c40deee..3ba9d93f14ec030c6a06c6fa504ea75cb036b721 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1110,105 +1110,112 @@ virStorageFileGetMetadataFromFD(const char *path, /* Recursive workhorse for virStorageFileGetMetadata. */ static int -virStorageFileGetMetadataRecurse(const char *path, const char *canonPath, - const char *directory, - int format, uid_t uid, gid_t gid, - bool allow_probe, virHashTablePtr cycle, - virStorageSourcePtr meta) +virStorageFileGetMetadataRecurse(virStorageSourcePtr src, + const char *canonPath, + uid_t uid, gid_t gid, + bool allow_probe, + virHashTablePtr cycle) { int fd; int ret = -1; + virStorageSourcePtr backingStore = NULL; int backingFormat; - char *backingPath = NULL; - char *backingDirectory = NULL; VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d", - path, canonPath, NULLSTR(directory), format, + src->path, canonPath, NULLSTR(src->relDir), src->format, (int)uid, (int)gid, allow_probe); if (virHashLookup(cycle, canonPath)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("backing store for %s is self-referential"), - path); + src->path); return -1; } + if (virHashAddEntry(cycle, canonPath, (void *)1) < 0) return -1; - if (virStorageIsFile(path)) { - if (VIR_STRDUP(meta->relPath, path) < 0) - return -1; - if (VIR_STRDUP(meta->path, canonPath) < 0) - return -1; - if (VIR_STRDUP(meta->relDir, directory) < 0) + if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) { + if ((fd = virFileOpenAs(src->path, O_RDONLY, 0, uid, gid, 0)) < 0) { + virReportSystemError(-fd, _("Failed to open file '%s'"), + src->path); return -1; - meta->format = format; + } - if ((fd = virFileOpenAs(canonPath, O_RDONLY, 0, uid, gid, 0)) < 0) { - virReportSystemError(-fd, _("Failed to open file '%s'"), path); + if (virStorageFileGetMetadataFromFDInternal(src, fd, + &backingFormat) < 0) { + VIR_FORCE_CLOSE(fd); return -1; } - ret = virStorageFileGetMetadataFromFDInternal(meta, fd, &backingFormat); - if (VIR_CLOSE(fd) < 0) - VIR_WARN("could not close file %s", path); + VIR_WARN("could not close file %s", src->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->relPath, path) < 0) - return -1; - if (VIR_STRDUP(meta->path, path) < 0) - return -1; - meta->type = VIR_STORAGE_TYPE_NETWORK; - meta->format = VIR_STORAGE_FILE_RAW; - ret = 0; + /* TODO: currently we call this only for local storage */ + return 0; } - if (ret == 0 && meta->backingStoreRaw) { - virStorageSourcePtr backing; - - if (virStorageIsFile(meta->backingStoreRaw)) { - if (virFindBackingFile(directory, - meta->backingStoreRaw, - &backingDirectory, - &backingPath) < 0) { - /* the backing file is (currently) unavailable, treat this - * file as standalone: - * backingStoreRaw is kept to mark broken image chains */ - VIR_WARN("Backing file '%s' of image '%s' is missing.", - meta->backingStoreRaw, path); - - return 0; - } - } else { - if (VIR_STRDUP(backingPath, meta->backingStoreRaw) < 0) - return -1; - } + /* check whether we need to go deeper */ + if (!src->backingStoreRaw) + return 0; - if (backingFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) - backingFormat = VIR_STORAGE_FILE_RAW; - else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE) - backingFormat = VIR_STORAGE_FILE_AUTO; - if (VIR_ALLOC(backing) < 0 || - virStorageFileGetMetadataRecurse(meta->backingStoreRaw, - backingPath, - backingDirectory, backingFormat, - uid, gid, allow_probe, - cycle, backing) < 0) { - /* If we failed to get backing data, mark the chain broken */ - virStorageSourceFree(backing); - } else { - meta->backingStore = backing; + if (VIR_ALLOC(backingStore) < 0) + return -1; + + if (VIR_STRDUP(backingStore->relPath, src->backingStoreRaw) < 0) + goto cleanup; + + if (virStorageIsFile(src->backingStoreRaw)) { + backingStore->type = VIR_STORAGE_TYPE_FILE; + + if (virFindBackingFile(src->relDir, + src->backingStoreRaw, + &backingStore->relDir, + &backingStore->path) < 0) { + /* the backing file is (currently) unavailable, treat this + * file as standalone: + * backingStoreRaw is kept to mark broken image chains */ + VIR_WARN("Backing file '%s' of image '%s' is missing.", + src->backingStoreRaw, src->path); + ret = 0; + goto cleanup; } + } else { + /* TODO: To satisfy the test case, copy the network URI as path. This + * will be removed later. */ + backingStore->type = VIR_STORAGE_TYPE_NETWORK; + + if (VIR_STRDUP(backingStore->path, src->backingStoreRaw) < 0) + goto cleanup; } - VIR_FREE(backingDirectory); - VIR_FREE(backingPath); + if (backingFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) + backingStore->format = VIR_STORAGE_FILE_RAW; + else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE) + backingStore->format = VIR_STORAGE_FILE_AUTO; + else + backingStore->format = backingFormat; + + if (virStorageFileGetMetadataRecurse(backingStore, + backingStore->path, + uid, gid, allow_probe, + cycle) < 0) { + /* if we fail somewhere midway, just accept and return a + * broken chain */ + ret = 0; + goto cleanup; + } + + src->backingStore = backingStore; + backingStore = NULL; + ret = 0; + + cleanup: + virStorageSourceFree(backingStore); return ret; } + /** * virStorageFileGetMetadata: * @@ -1226,51 +1233,51 @@ virStorageFileGetMetadataRecurse(const char *path, const char *canonPath, * * Caller MUST free result after use via virStorageSourceFree. */ -virStorageSourcePtr -virStorageFileGetMetadata(const char *path, int format, +int +virStorageFileGetMetadata(virStorageSourcePtr src, uid_t uid, gid_t gid, bool allow_probe) { VIR_DEBUG("path=%s format=%d uid=%d gid=%d probe=%d", - path, format, (int)uid, (int)gid, allow_probe); + src->path, src->format, (int)uid, (int)gid, allow_probe); - virHashTablePtr cycle = virHashCreate(5, NULL); - virStorageSourcePtr meta = NULL; - virStorageSourcePtr ret = NULL; + virHashTablePtr cycle = NULL; char *canonPath = NULL; - char *directory = NULL; + int ret = -1; - if (!cycle) - return NULL; + if (!(cycle = virHashCreate(5, NULL))) + return -1; - if (virStorageIsFile(path)) { - if (!(canonPath = canonicalize_file_name(path))) { - virReportSystemError(errno, _("unable to resolve '%s'"), path); + if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) { + if (!(canonPath = canonicalize_file_name(src->path))) { + virReportSystemError(errno, _("unable to resolve '%s'"), + src->path); goto cleanup; } - if (!(directory = mdir_name(path))) { + + if (!src->relPath && + VIR_STRDUP(src->relPath, src->path) < 0) + goto cleanup; + + if (!src->relDir && + !(src->relDir = mdir_name(src->path))) { virReportOOMError(); goto cleanup; } - } else if (VIR_STRDUP(canonPath, path) < 0) { + } else { + /* TODO: currently unimplemented for non-local storage */ + ret = 0; goto cleanup; } - if (VIR_ALLOC(meta) < 0) - goto cleanup; - if (format <= VIR_STORAGE_FILE_NONE) - format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW; - if (virStorageFileGetMetadataRecurse(path, canonPath, directory, format, - uid, gid, allow_probe, cycle, - meta) < 0) - goto cleanup; - ret = meta; - meta = NULL; + if (src->format <= VIR_STORAGE_FILE_NONE) + src->format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW; + + ret = virStorageFileGetMetadataRecurse(src, canonPath, uid, gid, + allow_probe, cycle); cleanup: - virStorageSourceFree(meta); VIR_FREE(canonPath); - VIR_FREE(directory); virHashFree(cycle); return ret; } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index e60befea74950a37eabd0f6e4a3b1ac6b4e5f7ce..a0adb9be1bb3a0026cb2799b284644b7b4769dbd 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -263,10 +263,9 @@ int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid); int virStorageFileProbeFormatFromBuf(const char *path, char *buf, size_t buflen); -virStorageSourcePtr virStorageFileGetMetadata(const char *path, - int format, - uid_t uid, gid_t gid, - bool allow_probe) +int virStorageFileGetMetadata(virStorageSourcePtr src, + uid_t uid, gid_t gid, + bool allow_probe) ATTRIBUTE_NONNULL(1); virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path, int fd, diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 370b8c21cf4b1d8785894812a689f22753a04a99..c02d866d808f2b6f45d3d864068b2ee74004e4ca 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -29,6 +29,7 @@ #include "virlog.h" #include "virstoragefile.h" #include "virstring.h" +#include "dirname.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -88,6 +89,44 @@ testCleanupImages(void) virFileDeleteTree(datadir); } + +static virStorageSourcePtr +testStorageFileGetMetadata(const char *path, + int format, + uid_t uid, gid_t gid, + bool allow_probe) +{ + virStorageSourcePtr ret = NULL; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + ret->type = VIR_STORAGE_TYPE_FILE; + ret->format = format; + + if (VIR_STRDUP(ret->relPath, path) < 0) + goto error; + + if (!(ret->relDir = mdir_name(path))) { + virReportOOMError(); + goto error; + } + + if (!(ret->path = canonicalize_file_name(path))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "failed to resolve '%s'", path); + goto error; + } + + if (virStorageFileGetMetadata(ret, uid, gid, allow_probe) < 0) + goto error; + + return ret; + + error: + virStorageSourceFree(ret); + return NULL; +} + static int testPrepImages(void) { @@ -272,8 +311,8 @@ testStorageChain(const void *args) char *broken = NULL; bool isAbs = !!(data->flags & ABS_START); - meta = virStorageFileGetMetadata(data->start, data->format, -1, -1, - (data->flags & ALLOW_PROBE) != 0); + meta = testStorageFileGetMetadata(data->start, data->format, -1, -1, + (data->flags & ALLOW_PROBE) != 0); if (!meta) { if (data->flags & EXP_FAIL) { virResetLastError(); @@ -825,8 +864,8 @@ mymain(void) ret = -1; /* Test behavior of chain lookups, absolute backing from relative start */ - chain = virStorageFileGetMetadata("wrap", VIR_STORAGE_FILE_QCOW2, - -1, -1, false); + chain = testStorageFileGetMetadata("wrap", VIR_STORAGE_FILE_QCOW2, + -1, -1, false); if (!chain) { ret = -1; goto cleanup; @@ -870,8 +909,8 @@ mymain(void) /* Test behavior of chain lookups, relative backing from absolute start */ virStorageSourceFree(chain); - chain = virStorageFileGetMetadata(abswrap, VIR_STORAGE_FILE_QCOW2, - -1, -1, false); + chain = testStorageFileGetMetadata(abswrap, VIR_STORAGE_FILE_QCOW2, + -1, -1, false); if (!chain) { ret = -1; goto cleanup; @@ -900,8 +939,8 @@ mymain(void) /* Test behavior of chain lookups, relative backing */ virStorageSourceFree(chain); - chain = virStorageFileGetMetadata("sub/link2", VIR_STORAGE_FILE_QCOW2, - -1, -1, false); + chain = testStorageFileGetMetadata("sub/link2", VIR_STORAGE_FILE_QCOW2, + -1, -1, false); if (!chain) { ret = -1; goto cleanup;