A network disk might actually be backed by local storage. Also the path iterator actually handles networked disks well now so remove the code that skips the labelling in dac and selinux security driver. --- src/security/security_dac.c | 3 --- src/security/security_selinux.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 015b699..9d5c25b 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -333,9 +333,6 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, if (!priv->dynamicOwnership) return 0; - if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK) - return 0; - secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); if (secdef && secdef->norelabel) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 008c58c..228e5cb 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1255,9 +1255,6 @@ virSecuritySELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr, if (!cbdata.secdef || cbdata.secdef->norelabel) return 0; - if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK) - return 0; - return virDomainDiskDefForeachPath(disk, true, virSecuritySELinuxSetSecurityFileLabel, -- 1.9.3

Introduce a common function that will take a callback to resolve links that will be used to canonicalize paths on various storage systems and add extensive tests. --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virstoragefile.h | 7 ++ tests/virstoragetest.c | 108 +++++++++++++++++++++++++ 4 files changed, 311 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f69cd1c..7671730 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1864,6 +1864,7 @@ virStorageGenerateQcowPassphrase; # util/virstoragefile.h +virStorageFileCanonicalizePath; virStorageFileChainGetBroken; virStorageFileChainLookup; virStorageFileFeatureTypeFromString; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 0792dd8..ef69bf3 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -40,6 +40,7 @@ #include "virutil.h" #include "viruri.h" #include "dirname.h" +#include "virbuffer.h" #if HAVE_SYS_SYSCALL_H # include <sys/syscall.h> #endif @@ -1928,3 +1929,197 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent) return ret; } + + +static char * +virStorageFileCanonicalizeFormatPath(char **components, + size_t ncomponents, + bool beginSlash, + bool beginDoubleSlash) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + size_t i; + char *ret = NULL; + + if (beginSlash) + virBufferAddLit(&buf, "/"); + + if (beginDoubleSlash) + virBufferAddLit(&buf, "/"); + + for (i = 0; i < ncomponents; i++) { + if (i != 0) + virBufferAddLit(&buf, "/"); + + virBufferAdd(&buf, components[i], -1); + } + + if (virBufferError(&buf) != 0) { + virReportOOMError(); + return NULL; + } + + /* if the output string is empty just return an empty string */ + if (!(ret = virBufferContentAndReset(&buf))) + ignore_value(VIR_STRDUP(ret, "")); + + return ret; +} + + +static int +virStorageFileCanonicalizeInjectSymlink(const char *path, + size_t at, + char ***components, + size_t *ncomponents) +{ + char **tmp = NULL; + char **next; + size_t ntmp = 0; + int ret = -1; + + if (!(tmp = virStringSplitCount(path, "/", 0, &ntmp))) + goto cleanup; + + /* prepend */ + for (next = tmp; *next; next++) { + if (VIR_INSERT_ELEMENT(*components, at, *ncomponents, *next) < 0) + goto cleanup; + + at++; + } + + ret = 0; + + cleanup: + virStringFreeListCount(tmp, ntmp); + return ret; +} + + +char * +virStorageFileCanonicalizePath(const char *path, + virStorageFileSimplifyPathReadlinkCallback cb, + void *cbdata) +{ + virHashTablePtr cycle = NULL; + bool beginSlash = false; + bool beginDoubleSlash = false; + char **components = NULL; + size_t ncomponents = 0; + char *linkpath = NULL; + char *currentpath = NULL; + size_t i = 0; + int rc; + char *ret = NULL; + + if (path[0] == '/') { + beginSlash = true; + + if (path[1] == '/' && path[2] != '/') + beginDoubleSlash = true; + } + + if (!(cycle = virHashCreate(10, NULL))) + goto cleanup; + + if (!(components = virStringSplitCount(path, "/", 0, &ncomponents))) + goto cleanup; + + while (i < ncomponents) { + /* skip slashes and '.'s */ + if (STREQ(components[i], "") || + STREQ(components[i], ".")) { + VIR_FREE(components[i]); + VIR_DELETE_ELEMENT(components, i, ncomponents); + continue; + } + + /* resolve changes to parent directory */ + if (STREQ(components[i], "..")) { + if (!beginSlash && + (i == 0 || STREQ(components[i - 1], ".."))) { + i++; + continue; + } + + VIR_FREE(components[i]); + VIR_DELETE_ELEMENT(components, i, ncomponents); + + if (i != 0) { + VIR_FREE(components[i - 1]); + VIR_DELETE_ELEMENT(components, i - 1, ncomponents); + i--; + } + + continue; + } + + /* check if the actual path isn't resulting into a symlink */ + if (!(currentpath = virStorageFileCanonicalizeFormatPath(components, + i + 1, + beginSlash, + beginDoubleSlash))) + goto cleanup; + + if ((rc = cb(currentpath, &linkpath, cbdata)) < 0) + goto cleanup; + + if (rc == 0) { + if (virHashLookup(cycle, currentpath)) { + virReportSystemError(ELOOP, + _("Failed to canonicalize path '%s'"), path); + goto cleanup; + } + + if (virHashAddEntry(cycle, currentpath, (void *) 1) < 0) + goto cleanup; + + if (linkpath[0] == '/') { + /* kill everything from the beginning including the actual component */ + i++; + while (i--) { + VIR_FREE(components[0]); + VIR_DELETE_ELEMENT(components, 0, ncomponents); + } + beginSlash = true; + + if (linkpath[1] == '/' && linkpath[2] != '/') + beginDoubleSlash = true; + else + beginDoubleSlash = false; + + i = 0; + } else { + VIR_FREE(components[i]); + VIR_DELETE_ELEMENT(components, i, ncomponents); + } + + if (virStorageFileCanonicalizeInjectSymlink(linkpath, + i, + &components, + &ncomponents) < 0) + goto cleanup; + + VIR_FREE(linkpath); + VIR_FREE(currentpath); + + continue; + } + + VIR_FREE(currentpath); + + i++; + } + + ret = virStorageFileCanonicalizeFormatPath(components, ncomponents, + beginSlash, beginDoubleSlash); + + cleanup: + virHashFree(cycle); + virStringFreeListCount(components, ncomponents); + VIR_FREE(linkpath); + VIR_FREE(currentpath); + + return ret; +} diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 34b3625..fd5c89e 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -325,5 +325,12 @@ void virStorageSourceFree(virStorageSourcePtr def); void virStorageSourceClearBackingStore(virStorageSourcePtr def); virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent); +typedef int +(*virStorageFileSimplifyPathReadlinkCallback)(const char *path, + char **link, + void *data); +char *virStorageFileCanonicalizePath(const char *path, + virStorageFileSimplifyPathReadlinkCallback cb, + void *cbdata); #endif /* __VIR_STORAGE_FILE_H__ */ diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index bd593b0..8111f58 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -524,12 +524,78 @@ testStorageLookup(const void *args) return ret; } + +struct testPathCanonicalizeData +{ + const char *path; + const char *expect; +}; + +static const char *testPathCanonicalizeSymlinks[][2] = +{ + {"/path/blah", "/other/path/huzah"}, + {"/path/to/relative/symlink", "../../actual/file"}, + {"/cycle", "/cycle"}, + {"/cycle2/link", "./link"}, +}; + +static int +testPathCanonicalizeReadlink(const char *path, + char **link, + void *data ATTRIBUTE_UNUSED) +{ + size_t i; + + *link = NULL; + + for (i = 0; i < ARRAY_CARDINALITY(testPathCanonicalizeSymlinks); i++) { + if (STREQ(path, testPathCanonicalizeSymlinks[i][0])) { + if (VIR_STRDUP(*link, testPathCanonicalizeSymlinks[i][1]) < 0) + return -1; + + return 0; + } + } + + return 1; +} + + +static int +testPathCanonicalize(const void *args) +{ + const struct testPathCanonicalizeData *data = args; + char *canon = NULL; + int ret = -1; + + canon = virStorageFileCanonicalizePath(data->path, + testPathCanonicalizeReadlink, + NULL); + + if (STRNEQ_NULLABLE(data->expect, canon)) { + fprintf(stderr, + "path canonicalization of '%s' failed: expected '%s' got '%s'\n", + data->path, NULLSTR(data->expect), NULLSTR(canon)); + + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(canon); + + return ret; +} + + static int mymain(void) { int ret; virCommandPtr cmd = NULL; struct testChainData data; + struct testPathCanonicalizeData data3; virStorageSourcePtr chain = NULL; /* Prep some files with qemu-img; if that is not found on PATH, or @@ -1028,6 +1094,48 @@ mymain(void) chain->backingStore->path); TEST_LOOKUP_TARGET(33, "vda", "vda[3]", 3, NULL, NULL, NULL); +#define TEST_PATH_CANONICALIZE(id, PATH, EXPECT) \ + do { \ + data3.path = PATH; \ + data3.expect = EXPECT; \ + if (virtTestRun("Path canonicalize " #id, \ + testPathCanonicalize, &data3) < 0) \ + ret = -1; \ + } while (0) + + TEST_PATH_CANONICALIZE(1, "/", "/"); + TEST_PATH_CANONICALIZE(2, "/path", "/path"); + TEST_PATH_CANONICALIZE(3, "/path/to/blah", "/path/to/blah"); + TEST_PATH_CANONICALIZE(4, "/path/", "/path"); + TEST_PATH_CANONICALIZE(5, "///////", "/"); + TEST_PATH_CANONICALIZE(6, "//", "//"); + TEST_PATH_CANONICALIZE(7, "", ""); + TEST_PATH_CANONICALIZE(8, ".", ""); + TEST_PATH_CANONICALIZE(9, "../", ".."); + TEST_PATH_CANONICALIZE(10, "../../", "../.."); + TEST_PATH_CANONICALIZE(11, "../../blah", "../../blah"); + TEST_PATH_CANONICALIZE(12, "/./././blah", "/blah"); + TEST_PATH_CANONICALIZE(13, ".././../././../blah", "../../../blah"); + TEST_PATH_CANONICALIZE(14, "/././", "/"); + TEST_PATH_CANONICALIZE(15, "./././", ""); + TEST_PATH_CANONICALIZE(16, "blah/../foo", "foo"); + TEST_PATH_CANONICALIZE(17, "foo/bar/../blah", "foo/blah"); + TEST_PATH_CANONICALIZE(18, "foo/bar/.././blah", "foo/blah"); + TEST_PATH_CANONICALIZE(19, "/path/to/foo/bar/../../../../../../../../baz", "/baz"); + TEST_PATH_CANONICALIZE(20, "path/to/foo/bar/../../../../../../../../baz", "../../../../baz"); + TEST_PATH_CANONICALIZE(21, "path/to/foo/bar", "path/to/foo/bar"); + TEST_PATH_CANONICALIZE(22, "//foo//bar", "//foo/bar"); + TEST_PATH_CANONICALIZE(23, "/bar//foo", "/bar/foo"); + TEST_PATH_CANONICALIZE(24, "//../blah", "//blah"); + + /* test paths with symlinks */ + TEST_PATH_CANONICALIZE(25, "/path/blah", "/other/path/huzah"); + TEST_PATH_CANONICALIZE(26, "/path/to/relative/symlink", "/path/actual/file"); + TEST_PATH_CANONICALIZE(27, "/path/to/relative/symlink/blah", "/path/actual/file/blah"); + TEST_PATH_CANONICALIZE(28, "/path/blah/yippee", "/other/path/huzah/yippee"); + TEST_PATH_CANONICALIZE(29, "/cycle", NULL); + TEST_PATH_CANONICALIZE(30, "/cycle2/link", NULL); + cleanup: /* Final cleanup */ virStorageSourceFree(chain); -- 1.9.3

This patch introduces a function that will allow us to resolve a relative difference between two elements of a disk backing chain. This fucntion will be used to allow relative block commit and block pull where we need to specify the new relative name of the image to qemu. This patch also adds unit tests for the function to verify that it works correctly. --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 77 ++++++++++++++++++++++++ src/util/virstoragefile.h | 4 ++ tests/virstoragetest.c | 146 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 228 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7671730..773b208 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1875,6 +1875,7 @@ virStorageFileGetLVMKey; virStorageFileGetMetadataFromBuf; virStorageFileGetMetadataFromFD; virStorageFileGetMetadataInternal; +virStorageFileGetRelativeBackingPath; virStorageFileGetSCSIKey; virStorageFileIsClusterFS; virStorageFileParseChainIndex; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index ef69bf3..ce1fc86 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2123,3 +2123,80 @@ virStorageFileCanonicalizePath(const char *path, return ret; } + + +static char * +virStorageFileRemoveLastPathComponent(const char *path) +{ + char *tmp; + char *ret; + + if (VIR_STRDUP(ret, path ? path : "") < 0) + return NULL; + + if ((tmp = strrchr(ret, '/'))) + tmp[1] = '\0'; + else + ret[0] = '\0'; + + return ret; +} + + +/* + * virStorageFileGetRelativeBackingPath: + * + * Resolve relative path to be written to the overlay of @top image when + * collapsing the backing chain between @top and @base. + * + * Returns 0 on success; 1 if backing chain isn't relative and -1 on error. + */ +int +virStorageFileGetRelativeBackingPath(virStorageSourcePtr top, + virStorageSourcePtr base, + char **relpath) +{ + virStorageSourcePtr next; + char *tmp = NULL; + char *path = NULL; + char ret = -1; + + *relpath = NULL; + + for (next = top; next; next = next->backingStore) { + if (!next->backingRelative || !next->relPath) { + ret = 1; + goto cleanup; + } + + if (!(tmp = virStorageFileRemoveLastPathComponent(path))) + goto cleanup; + + VIR_FREE(path); + + if (virAsprintf(&path, "%s%s", tmp, next->relPath) < 0) + goto cleanup; + + VIR_FREE(tmp); + + if (next == base) + break; + } + + if (next != base) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to resolve relative backing name: " + "base image is not in backing chain")); + goto cleanup; + } + + *relpath = path; + path = NULL; + + ret = 0; + + cleanup: + VIR_FREE(path); + VIR_FREE(tmp); + return ret; +} diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index fd5c89e..ff8130d 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -333,4 +333,8 @@ char *virStorageFileCanonicalizePath(const char *path, virStorageFileSimplifyPathReadlinkCallback cb, void *cbdata); +int virStorageFileGetRelativeBackingPath(virStorageSourcePtr from, + virStorageSourcePtr to, + char **relpath); + #endif /* __VIR_STORAGE_FILE_H__ */ diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 8111f58..5cea929 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -588,6 +588,104 @@ testPathCanonicalize(const void *args) return ret; } +virStorageSource backingchain[12]; + +static void +testPathRelativePrepare(void) +{ + size_t i; + + for (i = 0; i < ARRAY_CARDINALITY(backingchain); i++) { + if (i < ARRAY_CARDINALITY(backingchain) - 1) + backingchain[i].backingStore = &backingchain[i+1]; + else + backingchain[i].backingStore = NULL; + + backingchain[i].backingRelative = true; + } + + /* normal relative backing chain */ + backingchain[0].path = (char *) "/path/to/some/img"; + backingchain[0].relPath = (char *) "/path/to/some/img"; + backingchain[0].backingRelative = false; + + backingchain[1].path = (char *) "/path/to/some/asdf"; + backingchain[1].relPath = (char *) "asdf"; + + backingchain[2].path = (char *) "/path/to/some/test"; + backingchain[2].relPath = (char *) "test"; + + backingchain[3].path = (char *) "/path/to/some/blah"; + backingchain[3].relPath = (char *) "blah"; + + /* ovirt's backing chain */ + backingchain[4].path = (char *) "/path/to/volume/image1"; + backingchain[4].relPath = (char *) "/path/to/volume/image1"; + backingchain[4].backingRelative = false; + + backingchain[5].path = (char *) "/path/to/volume/image2"; + backingchain[5].relPath = (char *) "../volume/image2"; + + backingchain[6].path = (char *) "/path/to/volume/image3"; + backingchain[6].relPath = (char *) "../volume/image3"; + + backingchain[7].path = (char *) "/path/to/volume/image4"; + backingchain[7].relPath = (char *) "../volume/image4"; + + /* some arbitrarily crazy backing chains */ + backingchain[8].path = (char *) "/crazy/base/image"; + backingchain[8].relPath = (char *) "/crazy/base/image"; + backingchain[8].backingRelative = false; + + backingchain[9].path = (char *) "/crazy/base/directory/stuff/volumes/garbage/image2"; + backingchain[9].relPath = (char *) "directory/stuff/volumes/garbage/image2"; + + backingchain[10].path = (char *) "/crazy/base/directory/image3"; + backingchain[10].relPath = (char *) "../../../image3"; + + backingchain[11].path = (char *) "/crazy/base/blah/image4"; + backingchain[11].relPath = (char *) "../blah/image4"; +} + + +struct testPathRelativeBacking +{ + virStorageSourcePtr top; + virStorageSourcePtr base; + + const char *expect; +}; + +static int +testPathRelative(const void *args) +{ + const struct testPathRelativeBacking *data = args; + char *actual = NULL; + int ret = -1; + + if (virStorageFileGetRelativeBackingPath(data->top, + data->base, + &actual) < 0) { + fprintf(stderr, "relative backing path resolution failed\n"); + goto cleanup; + } + + if (STRNEQ_NULLABLE(data->expect, actual)) { + fprintf(stderr, "relative path resolution from '%s' to '%s': " + "expected '%s', got '%s'\n", + data->top->path, data->base->path, + NULLSTR(data->expect), NULLSTR(actual)); + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(actual); + + return ret; +} + static int mymain(void) @@ -596,6 +694,7 @@ mymain(void) virCommandPtr cmd = NULL; struct testChainData data; struct testPathCanonicalizeData data3; + struct testPathRelativeBacking data4; virStorageSourcePtr chain = NULL; /* Prep some files with qemu-img; if that is not found on PATH, or @@ -1136,6 +1235,53 @@ mymain(void) TEST_PATH_CANONICALIZE(29, "/cycle", NULL); TEST_PATH_CANONICALIZE(30, "/cycle2/link", NULL); +#define TEST_RELATIVE_BACKING(id, TOP, BASE, EXPECT) \ + do { \ + data4.top = &TOP; \ + data4.base = &BASE; \ + data4.expect = EXPECT; \ + if (virtTestRun("Path relative resolve " #id, \ + testPathRelative, &data4) < 0) \ + ret = -1; \ + } while (0) + + testPathRelativePrepare(); + + /* few negative tests first */ + + /* a non-relative image is in the backing chain span */ + TEST_RELATIVE_BACKING(1, backingchain[0], backingchain[1], NULL); + TEST_RELATIVE_BACKING(2, backingchain[0], backingchain[2], NULL); + TEST_RELATIVE_BACKING(3, backingchain[0], backingchain[3], NULL); + TEST_RELATIVE_BACKING(4, backingchain[1], backingchain[5], NULL); + + /* image is not in chain (specified backwards) */ + TEST_RELATIVE_BACKING(5, backingchain[2], backingchain[1], NULL); + + /* positive tests */ + TEST_RELATIVE_BACKING(6, backingchain[1], backingchain[1], "asdf"); + TEST_RELATIVE_BACKING(7, backingchain[1], backingchain[2], "test"); + TEST_RELATIVE_BACKING(8, backingchain[1], backingchain[3], "blah"); + TEST_RELATIVE_BACKING(9, backingchain[2], backingchain[2], "test"); + TEST_RELATIVE_BACKING(10, backingchain[2], backingchain[3], "blah"); + TEST_RELATIVE_BACKING(11, backingchain[3], backingchain[3], "blah"); + + /* oVirt spelling */ + TEST_RELATIVE_BACKING(12, backingchain[5], backingchain[5], "../volume/image2"); + TEST_RELATIVE_BACKING(13, backingchain[5], backingchain[6], "../volume/../volume/image3"); + TEST_RELATIVE_BACKING(14, backingchain[5], backingchain[7], "../volume/../volume/../volume/image4"); + TEST_RELATIVE_BACKING(15, backingchain[6], backingchain[6], "../volume/image3"); + TEST_RELATIVE_BACKING(16, backingchain[6], backingchain[7], "../volume/../volume/image4"); + TEST_RELATIVE_BACKING(17, backingchain[7], backingchain[7], "../volume/image4"); + + /* crazy spellings */ + TEST_RELATIVE_BACKING(17, backingchain[9], backingchain[9], "directory/stuff/volumes/garbage/image2"); + TEST_RELATIVE_BACKING(18, backingchain[9], backingchain[10], "directory/stuff/volumes/garbage/../../../image3"); + TEST_RELATIVE_BACKING(19, backingchain[9], backingchain[11], "directory/stuff/volumes/garbage/../../../../blah/image4"); + TEST_RELATIVE_BACKING(20, backingchain[10], backingchain[10], "../../../image3"); + TEST_RELATIVE_BACKING(21, backingchain[10], backingchain[11], "../../../../blah/image4"); + TEST_RELATIVE_BACKING(22, backingchain[11], backingchain[11], "../blah/image4"); + cleanup: /* Final cleanup */ virStorageSourceFree(chain); -- 1.9.3

When the test is failing but the debug output isn't enabled the resulting line would look ugly like and would not contain the actual difference. TEST: virstoragetest .................chain member 1!chain member 1!chain member 1! Store the member index in the actual checked string to hide this problem --- tests/virstoragetest.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index b7383c0..6068612 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -313,6 +313,7 @@ struct testChainData static const char testStorageChainFormat[] = + "chain member: %zu\n" "path:%s\n" "backingStoreRaw: %s\n" "capacity: %lld\n" @@ -383,7 +384,7 @@ testStorageChain(const void *args) expRelDir = isAbs ? data->files[i]->relDirAbs : data->files[i]->relDirRel; if (virAsprintf(&expect, - testStorageChainFormat, + testStorageChainFormat, i, NULLSTR(data->files[i]->path), NULLSTR(data->files[i]->expBackingStoreRaw), data->files[i]->expCapacity, @@ -393,7 +394,7 @@ testStorageChain(const void *args) data->files[i]->type, data->files[i]->format) < 0 || virAsprintf(&actual, - testStorageChainFormat, + testStorageChainFormat, i, NULLSTR(elt->path), NULLSTR(elt->backingStoreRaw), elt->capacity, @@ -407,7 +408,6 @@ testStorageChain(const void *args) goto cleanup; } if (STRNEQ(expect, actual)) { - fprintf(stderr, "chain member %zu", i); virtTestDifference(stderr, expect, actual); VIR_FREE(expect); VIR_FREE(actual); -- 1.9.3

Separately remove the now unused variable. --- tests/virstoragetest.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index dd25069..11bc4dd 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -283,7 +283,6 @@ struct _testFileData unsigned long long expCapacity; bool expEncrypted; const char *pathRel; - const char *pathAbs; const char *path; const char *relDirRel; const char *relDirAbs; @@ -730,7 +729,6 @@ mymain(void) /* Raw image, whether with right format or no specified format */ testFileData raw = { - .pathAbs = canonraw, .path = canonraw, .relDirRel = ".", .relDirAbs = datadir, @@ -749,12 +747,10 @@ mymain(void) (&raw), ALLOW_PROBE | EXP_PASS); /* Qcow2 file with relative raw backing, format provided */ - raw.pathAbs = "raw"; raw.pathRel = "raw"; testFileData qcow2 = { .expBackingStoreRaw = "raw", .expCapacity = 1024, - .pathAbs = canonqcow2, .path = canonqcow2, .relDirRel = ".", .relDirAbs = datadir, @@ -762,7 +758,6 @@ mymain(void) .format = VIR_STORAGE_FILE_QCOW2, }; testFileData qcow2_as_raw = { - .pathAbs = canonqcow2, .path = canonqcow2, .relDirRel = ".", .relDirAbs = datadir, @@ -788,7 +783,6 @@ mymain(void) ret = -1; qcow2.expBackingStoreRaw = absraw; raw.pathRel = NULL; - raw.pathAbs = absraw; raw.relDirRel = datadir; /* Qcow2 file with raw as absolute backing, backing format provided */ @@ -807,7 +801,6 @@ mymain(void) testFileData wrap = { .expBackingStoreRaw = absqcow2, .expCapacity = 1024, - .pathAbs = abswrap, .path = canonwrap, .relDirRel = ".", .relDirAbs = datadir, @@ -839,7 +832,6 @@ mymain(void) testFileData wrap_as_raw = { .expBackingStoreRaw = absqcow2, .expCapacity = 1024, - .pathAbs = abswrap, .path = canonwrap, .relDirRel = ".", .relDirAbs = datadir, @@ -894,7 +886,6 @@ mymain(void) /* Qcow2 file with backing protocol instead of file */ testFileData nbd = { - .pathAbs = "nbd:example.org:6000:exportname=blah", .path = "blah", .type = VIR_STORAGE_TYPE_NETWORK, .format = VIR_STORAGE_FILE_RAW, @@ -911,7 +902,6 @@ mymain(void) testFileData qed = { .expBackingStoreRaw = absraw, .expCapacity = 1024, - .pathAbs = absqed, .path = canonqed, .relDirRel = ".", .relDirAbs = datadir, @@ -919,7 +909,6 @@ mymain(void) .format = VIR_STORAGE_FILE_QED, }; testFileData qed_as_raw = { - .pathAbs = absqed, .path = canonqed, .relDirRel = ".", .relDirAbs = datadir, @@ -934,7 +923,6 @@ mymain(void) /* directory */ testFileData dir = { - .pathAbs = absdir, .path = canondir, .relDirRel = ".", .relDirAbs = datadir, @@ -973,7 +961,6 @@ mymain(void) .expBackingStoreRaw = "../raw", .expCapacity = 1024, .pathRel = "../sub/link1", - .pathAbs = "../sub/link1", .path = canonqcow2, .relDirRel = "sub/../sub", .relDirAbs = datadir "/sub/../sub", @@ -983,7 +970,6 @@ mymain(void) testFileData link2 = { .expBackingStoreRaw = "../sub/link1", .expCapacity = 1024, - .pathAbs = abslink2, .path = canonwrap, .relDirRel = "sub", .relDirAbs = datadir "/sub", @@ -991,7 +977,6 @@ mymain(void) .format = VIR_STORAGE_FILE_QCOW2, }; raw.pathRel = "../raw"; - raw.pathAbs = "../raw"; raw.relDirRel = "sub/../sub/.."; raw.relDirAbs = datadir "/sub/../sub/.."; TEST_CHAIN(15, "sub/link2", abslink2, VIR_STORAGE_FILE_QCOW2, -- 1.9.3

libvirt always uses an absolute path to address the top image of an image chain. Our storage test tests also the relative path which won't ever be used. Additionally it makes the test more complicated. --- tests/virstoragetest.c | 79 +++++++++++++------------------------------------- 1 file changed, 20 insertions(+), 59 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index d7786bb..0e2c9ac 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -703,17 +703,12 @@ mymain(void) #define VIR_FLATTEN_2(...) __VA_ARGS__ #define VIR_FLATTEN_1(_1) VIR_FLATTEN_2 _1 -#define TEST_CHAIN(id, relstart, absstart, format, chain1, flags1, \ - chain2, flags2, chain3, flags3, chain4, flags4) \ +#define TEST_CHAIN(id, path, format, chain1, flags1, chain2, flags2) \ do { \ - TEST_ONE_CHAIN(#id "a", relstart, format, flags1, \ + TEST_ONE_CHAIN(#id "a", path, format, flags1 | ABS_START, \ VIR_FLATTEN_1(chain1)); \ - TEST_ONE_CHAIN(#id "b", relstart, format, flags2, \ + TEST_ONE_CHAIN(#id "b", path, format, flags2 | ABS_START, \ VIR_FLATTEN_1(chain2)); \ - TEST_ONE_CHAIN(#id "c", absstart, format, flags3 | ABS_START,\ - VIR_FLATTEN_1(chain3)); \ - TEST_ONE_CHAIN(#id "d", absstart, format, flags4 | ABS_START,\ - VIR_FLATTEN_1(chain4)); \ } while (0) /* The actual tests, in several groups. */ @@ -729,14 +724,10 @@ mymain(void) .type = VIR_STORAGE_TYPE_FILE, .format = VIR_STORAGE_FILE_RAW, }; - TEST_CHAIN(1, "raw", absraw, VIR_STORAGE_FILE_RAW, - (&raw), EXP_PASS, - (&raw), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(1, absraw, VIR_STORAGE_FILE_RAW, (&raw), EXP_PASS, (&raw), ALLOW_PROBE | EXP_PASS); - TEST_CHAIN(2, "raw", absraw, VIR_STORAGE_FILE_AUTO, - (&raw), EXP_PASS, - (&raw), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(2, absraw, VIR_STORAGE_FILE_AUTO, (&raw), EXP_PASS, (&raw), ALLOW_PROBE | EXP_PASS); @@ -758,14 +749,10 @@ mymain(void) .type = VIR_STORAGE_TYPE_FILE, .format = VIR_STORAGE_FILE_RAW, }; - TEST_CHAIN(3, "qcow2", absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2, &raw), EXP_PASS, - (&qcow2, &raw), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(3, absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &raw), EXP_PASS, (&qcow2, &raw), ALLOW_PROBE | EXP_PASS); - TEST_CHAIN(4, "qcow2", absqcow2, VIR_STORAGE_FILE_AUTO, - (&qcow2_as_raw), EXP_PASS, - (&qcow2, &raw), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(4, absqcow2, VIR_STORAGE_FILE_AUTO, (&qcow2_as_raw), EXP_PASS, (&qcow2, &raw), ALLOW_PROBE | EXP_PASS); @@ -780,14 +767,10 @@ mymain(void) raw.relDirRel = datadir; /* Qcow2 file with raw as absolute backing, backing format provided */ - TEST_CHAIN(5, "qcow2", absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2, &raw), EXP_PASS, - (&qcow2, &raw), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(5, absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &raw), EXP_PASS, (&qcow2, &raw), ALLOW_PROBE | EXP_PASS); - TEST_CHAIN(6, "qcow2", absqcow2, VIR_STORAGE_FILE_AUTO, - (&qcow2_as_raw), EXP_PASS, - (&qcow2, &raw), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(6, absqcow2, VIR_STORAGE_FILE_AUTO, (&qcow2_as_raw), EXP_PASS, (&qcow2, &raw), ALLOW_PROBE | EXP_PASS); @@ -802,9 +785,7 @@ mymain(void) .format = VIR_STORAGE_FILE_QCOW2, }; qcow2.relDirRel = datadir; - TEST_CHAIN(7, "wrap", abswrap, VIR_STORAGE_FILE_QCOW2, - (&wrap, &qcow2, &raw), EXP_PASS, - (&wrap, &qcow2, &raw), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(7, abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap, &qcow2, &raw), EXP_PASS, (&wrap, &qcow2, &raw), ALLOW_PROBE | EXP_PASS); @@ -832,9 +813,7 @@ mymain(void) .type = VIR_STORAGE_TYPE_FILE, .format = VIR_STORAGE_FILE_QCOW2, }; - TEST_CHAIN(8, "wrap", abswrap, VIR_STORAGE_FILE_QCOW2, - (&wrap_as_raw, &qcow2_as_raw), EXP_PASS, - (&wrap, &qcow2, &raw), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(8, abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap_as_raw, &qcow2_as_raw), EXP_PASS, (&wrap, &qcow2, &raw), ALLOW_PROBE | EXP_PASS); @@ -849,9 +828,7 @@ mymain(void) qcow2.relDirRel = "."; /* Qcow2 file with missing backing file but specified type */ - TEST_CHAIN(9, "qcow2", absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2), EXP_WARN, - (&qcow2), ALLOW_PROBE | EXP_WARN, + TEST_CHAIN(9, absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN, (&qcow2), ALLOW_PROBE | EXP_WARN); @@ -863,9 +840,7 @@ mymain(void) ret = -1; /* Qcow2 file with missing backing file and no specified type */ - TEST_CHAIN(10, "qcow2", absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2), EXP_WARN, - (&qcow2), ALLOW_PROBE | EXP_WARN, + TEST_CHAIN(10, absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN, (&qcow2), ALLOW_PROBE | EXP_WARN); @@ -886,9 +861,7 @@ mymain(void) .relDirRel = ".", .relDirAbs = ".", }; - TEST_CHAIN(11, "qcow2", absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2, &nbd), EXP_PASS, - (&qcow2, &nbd), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(11, absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &nbd), EXP_PASS, (&qcow2, &nbd), ALLOW_PROBE | EXP_PASS); @@ -909,9 +882,7 @@ mymain(void) .type = VIR_STORAGE_TYPE_FILE, .format = VIR_STORAGE_FILE_RAW, }; - TEST_CHAIN(12, "qed", absqed, VIR_STORAGE_FILE_AUTO, - (&qed_as_raw), EXP_PASS, - (&qed, &raw), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(12, absqed, VIR_STORAGE_FILE_AUTO, (&qed_as_raw), EXP_PASS, (&qed, &raw), ALLOW_PROBE | EXP_PASS); @@ -923,14 +894,10 @@ mymain(void) .type = VIR_STORAGE_TYPE_DIR, .format = VIR_STORAGE_FILE_DIR, }; - TEST_CHAIN(13, "dir", absdir, VIR_STORAGE_FILE_AUTO, - (&dir), EXP_PASS, - (&dir), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(13, absdir, VIR_STORAGE_FILE_AUTO, (&dir), EXP_PASS, (&dir), ALLOW_PROBE | EXP_PASS); - TEST_CHAIN(14, "dir", absdir, VIR_STORAGE_FILE_DIR, - (&dir), EXP_PASS, - (&dir), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(14, absdir, VIR_STORAGE_FILE_DIR, (&dir), EXP_PASS, (&dir), ALLOW_PROBE | EXP_PASS); @@ -973,9 +940,7 @@ mymain(void) raw.pathRel = "../raw"; raw.relDirRel = "sub/../sub/.."; raw.relDirAbs = datadir "/sub/../sub/.."; - TEST_CHAIN(15, "sub/link2", abslink2, VIR_STORAGE_FILE_QCOW2, - (&link2, &link1, &raw), EXP_PASS, - (&link2, &link1, &raw), ALLOW_PROBE | EXP_PASS, + TEST_CHAIN(15, abslink2, VIR_STORAGE_FILE_QCOW2, (&link2, &link1, &raw), EXP_PASS, (&link2, &link1, &raw), ALLOW_PROBE | EXP_PASS); #endif @@ -989,9 +954,7 @@ mymain(void) qcow2.expBackingStoreRaw = "qcow2"; /* Behavior of an infinite loop chain */ - TEST_CHAIN(16, "qcow2", absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2), EXP_WARN, - (&qcow2), ALLOW_PROBE | EXP_WARN, + TEST_CHAIN(16, absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN, (&qcow2), ALLOW_PROBE | EXP_WARN); @@ -1011,9 +974,7 @@ mymain(void) qcow2.relDirRel = datadir; /* Behavior of an infinite loop chain */ - TEST_CHAIN(17, "wrap", abswrap, VIR_STORAGE_FILE_QCOW2, - (&wrap, &qcow2), EXP_WARN, - (&wrap, &qcow2), ALLOW_PROBE | EXP_WARN, + TEST_CHAIN(17, abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap, &qcow2), EXP_WARN, (&wrap, &qcow2), ALLOW_PROBE | EXP_WARN); -- 1.9.3

Store backing chain paths as non-canonical. The canonicalization step will be already taken. This will allow to avoid storing unnecessary amounts of data. --- src/util/virstoragefile.c | 33 ++++++--------------------------- tests/virstoragetest.c | 10 +++++----- 2 files changed, 11 insertions(+), 32 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 3b0a4ab..2524efa 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1044,25 +1044,17 @@ virStorageFileGetMetadataFromFD(const char *path, int *backingFormat) { - virStorageSourcePtr ret = NULL; - char *canonPath = NULL; - - if (!(canonPath = canonicalize_file_name(path))) { - virReportSystemError(errno, _("unable to resolve '%s'"), path); - goto cleanup; - } + virStorageSourcePtr ret; - if (!(ret = virStorageFileMetadataNew(canonPath, format))) - goto cleanup; + if (!(ret = virStorageFileMetadataNew(path, format))) + return NULL; if (virStorageFileGetMetadataFromFDInternal(ret, fd, backingFormat) < 0) { virStorageSourceFree(ret); - ret = NULL; + return NULL; } - cleanup: - VIR_FREE(canonPath); return ret; } @@ -1611,15 +1603,6 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent, virReportOOMError(); goto error; } - - /* XXX we don't currently need to store the canonical path but the - * change would break the test suite. Get rid of this later */ - char *tmp = ret->path; - if (!(ret->path = canonicalize_file_name(tmp))) { - ret->path = tmp; - tmp = NULL; - } - VIR_FREE(tmp); } else { ret->type = VIR_STORAGE_TYPE_NETWORK; @@ -1858,12 +1841,8 @@ virStorageSourceNewFromBackingAbsolute(const char *path) goto error; } - /* XXX we don't currently need to store the canonical path but the - * change would break the test suite. Get rid of this later */ - if (!(ret->path = canonicalize_file_name(path))) { - if (VIR_STRDUP(ret->path, path) < 0) - goto error; - } + if (VIR_STRDUP(ret->path, path) < 0) + goto error; } else { ret->type = VIR_STORAGE_TYPE_NETWORK; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 782f3a8..2511418 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -121,10 +121,8 @@ testStorageFileGetMetadata(const char *path, goto error; } - if (!(ret->path = canonicalize_file_name(path))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "failed to resolve '%s'", path); + if (VIR_STRDUP(ret->path, path) < 0) goto error; - } if (virStorageFileGetMetadata(ret, uid, gid, allow_probe) < 0) goto error; @@ -901,7 +899,7 @@ mymain(void) .expBackingStoreRaw = "../raw", .expCapacity = 1024, .pathRel = "../sub/link1", - .path = canonqcow2, + .path = datadir "/sub/../sub/link1", .relDir = datadir "/sub/../sub", .type = VIR_STORAGE_TYPE_FILE, .format = VIR_STORAGE_FILE_QCOW2, @@ -909,11 +907,13 @@ mymain(void) testFileData link2 = { .expBackingStoreRaw = "../sub/link1", .expCapacity = 1024, - .path = canonwrap, + .path = abslink2, .relDir = datadir "/sub", .type = VIR_STORAGE_TYPE_FILE, .format = VIR_STORAGE_FILE_QCOW2, }; + + raw.path = datadir "/sub/../sub/../raw"; raw.pathRel = "../raw"; raw.relDir = datadir "/sub/../sub/.."; TEST_CHAIN(15, abslink2, VIR_STORAGE_FILE_QCOW2, -- 1.9.3

This command allows to change the backing file name recorded in the metadata of a qcow (or other) image. The capability also notifies that the "block-stream" and "block-commit" commands understand the "backing-file" attribute. --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 08c3d04..05613e5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -256,6 +256,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "usb-kbd", /* 165 */ "host-pci-multidomain", "msg-timestamp", + "change-backing-file", ); @@ -1382,6 +1383,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "blockdev-snapshot-sync", QEMU_CAPS_DISK_SNAPSHOT }, { "add-fd", QEMU_CAPS_ADD_FD }, { "nbd-server-start", QEMU_CAPS_NBD_SERVER }, + { "change-backing-file", QEMU_CAPS_CHANGE_BACKING_FILE }, }; struct virQEMUCapsStringFlags virQEMUCapsEvents[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d755caa..6b4d7c4 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -206,6 +206,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_USB_KBD = 165, /* -device usb-kbd */ QEMU_CAPS_HOST_PCI_MULTIDOMAIN = 166, /* support domain > 0 in host pci address */ QEMU_CAPS_MSG_TIMESTAMP = 167, /* -msg timestamp */ + QEMU_CAPS_CHANGE_BACKING_FILE = 168, /* change name of backing file in metadata */ QEMU_CAPS_LAST, /* this must always be the last item */ }; -- 1.9.3

To allow changing the name that is recorded in the top of the current image chain used in a block pull/rebase operation, we need to specify the backing name to qemu. This is done via the "backing-file" attribute to the block-stream commad. --- src/qemu/qemu_driver.c | 8 ++++---- src/qemu/qemu_migration.c | 6 +++--- src/qemu/qemu_monitor.c | 12 +++++++----- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 15 +++++++++++++++ src/qemu/qemu_monitor_json.h | 1 + 6 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 61f2beb..88d44d3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14883,7 +14883,7 @@ qemuDomainBlockPivot(virConnectPtr conn, /* Probe the status, if needed. */ if (!disk->mirroring) { qemuDomainObjEnterMonitor(driver, vm); - rc = qemuMonitorBlockJob(priv->mon, device, NULL, 0, &info, + rc = qemuMonitorBlockJob(priv->mon, device, NULL, NULL, 0, &info, BLOCK_JOB_INFO, true); qemuDomainObjExitMonitor(driver, vm); if (rc < 0) @@ -15100,7 +15100,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorBlockJob(priv->mon, device, baseIndex ? baseSource->path : base, - bandwidth, info, mode, async); + NULL, bandwidth, info, mode, async); qemuDomainObjExitMonitor(driver, vm); if (ret < 0) goto endjob; @@ -15142,8 +15142,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, virDomainBlockJobInfo dummy; qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorBlockJob(priv->mon, device, NULL, 0, &dummy, - BLOCK_JOB_INFO, async); + ret = qemuMonitorBlockJob(priv->mon, device, NULL, NULL, 0, + &dummy, BLOCK_JOB_INFO, async); qemuDomainObjExitMonitor(driver, vm); if (ret <= 0) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 842f782..30175f2 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1308,7 +1308,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, _("canceled by client")); goto error; } - mon_ret = qemuMonitorBlockJob(priv->mon, diskAlias, NULL, 0, + mon_ret = qemuMonitorBlockJob(priv->mon, diskAlias, NULL, NULL, 0, &info, BLOCK_JOB_INFO, true); qemuDomainObjExitMonitor(driver, vm); @@ -1360,7 +1360,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, continue; if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT) == 0) { - if (qemuMonitorBlockJob(priv->mon, diskAlias, NULL, 0, + if (qemuMonitorBlockJob(priv->mon, diskAlias, NULL, NULL, 0, NULL, BLOCK_JOB_ABORT, true) < 0) { VIR_WARN("Unable to cancel block-job on '%s'", diskAlias); } @@ -1426,7 +1426,7 @@ qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) goto cleanup; - if (qemuMonitorBlockJob(priv->mon, diskAlias, NULL, 0, + if (qemuMonitorBlockJob(priv->mon, diskAlias, NULL, NULL, 0, NULL, BLOCK_JOB_ABORT, true) < 0) VIR_WARN("Unable to stop block job on %s", diskAlias); qemuDomainObjExitMonitor(driver, vm); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2c9e738..ed6368e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3354,6 +3354,7 @@ int qemuMonitorScreendump(qemuMonitorPtr mon, int qemuMonitorBlockJob(qemuMonitorPtr mon, const char *device, const char *base, + const char *backingName, unsigned long bandwidth, virDomainBlockJobInfoPtr info, qemuMonitorBlockJobCmd mode, @@ -3362,9 +3363,10 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, int ret = -1; unsigned long long speed; - VIR_DEBUG("mon=%p, device=%s, base=%s, bandwidth=%luM, info=%p, mode=%o, " - "modern=%d", mon, device, NULLSTR(base), bandwidth, info, mode, - modern); + VIR_DEBUG("mon=%p, device=%s, base=%s, backingName=%s, bandwidth=%luM, " + "info=%p, mode=%o, modern=%d", + mon, device, NULLSTR(base), NULLSTR(backingName), + bandwidth, info, mode, modern); /* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is * limited to LLONG_MAX also for unsigned values */ @@ -3378,8 +3380,8 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, speed <<= 20; if (mon->json) - ret = qemuMonitorJSONBlockJob(mon, device, base, speed, info, mode, - modern); + ret = qemuMonitorJSONBlockJob(mon, device, base, backingName, + speed, info, mode, modern); else virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("block jobs require JSON monitor")); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 71b7d3c..460239c 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -691,7 +691,8 @@ typedef enum { int qemuMonitorBlockJob(qemuMonitorPtr mon, const char *device, - const char *back, + const char *base, + const char *backingName, unsigned long bandwidth, virDomainBlockJobInfoPtr info, qemuMonitorBlockJobCmd mode, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index b4cdd6d..dfe57f1 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3766,6 +3766,7 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, const char *device, const char *base, + const char *backingName, unsigned long long speed, virDomainBlockJobInfoPtr info, qemuMonitorBlockJobCmd mode, @@ -3781,6 +3782,19 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon, _("only modern block pull supports base: %s"), base); return -1; } + + if (backingName && mode != BLOCK_JOB_PULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("backing name is supported only for block pull")); + return -1; + } + + if (backingName && !base) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("backing name requires a base image")); + return -1; + } + if (speed && mode == BLOCK_JOB_PULL && !modern) { virReportError(VIR_ERR_INTERNAL_ERROR, _("only modern block pull supports speed: %llu"), @@ -3815,6 +3829,7 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon, "s:device", device, "P:speed", speed, "S:base", base, + "S:backing-file", backingName, NULL); break; } diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 37ab325..7c9297c 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -284,6 +284,7 @@ int qemuMonitorJSONScreendump(qemuMonitorPtr mon, int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, const char *device, const char *base, + const char *backingName, unsigned long long speed, virDomainBlockJobInfoPtr info, qemuMonitorBlockJobCmd mode, -- 1.9.3

Introduce flag for the block rebase API to allow the rebase operation to leave the chain relatively addressed. Also adds a virsh switch to enable this behavior. --- include/libvirt/libvirt.h.in | 2 ++ src/libvirt.c | 5 +++++ tools/virsh-domain.c | 22 +++++++++++++++++++--- tools/virsh.pod | 4 ++++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bacdf57..44efd60 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2573,6 +2573,8 @@ typedef enum { file for a copy */ VIR_DOMAIN_BLOCK_REBASE_COPY_RAW = 1 << 2, /* Make destination file raw */ VIR_DOMAIN_BLOCK_REBASE_COPY = 1 << 3, /* Start a copy job */ + VIR_DOMAIN_BLOCK_REBASE_RELATIVE = 1 << 4, /* Keep backing chain relative + if possible */ } virDomainBlockRebaseFlags; int virDomainBlockRebase(virDomainPtr dom, const char *disk, diff --git a/src/libvirt.c b/src/libvirt.c index 21ef41f..a5f9547 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -19716,6 +19716,11 @@ virDomainBlockPull(virDomainPtr dom, const char *disk, * exists. If the job is aborted, a new one can be started later to * resume from the same point. * + * If @flags contains VIR_DOMAIN_BLOCK_REBASE_RELATIVE, the name recorded + * into the overlay of the @base image as path to the new backing file + * will be kept relative to other images in case the backing chain was + * using relative names. + * * When @flags includes VIR_DOMAIN_BLOCK_REBASE_COPY, this starts a copy, * where @base must be the name of a new file to copy the chain to. By * default, the copy will pull the entire source chain into the destination diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f26a133..e9162db 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1479,10 +1479,14 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, case VSH_CMD_BLOCK_JOB_PULL: if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) goto cleanup; - if (base) - ret = virDomainBlockRebase(dom, path, base, bandwidth, 0); - else + if (base) { + if (vshCommandOptBool(cmd, "keep-relative")) + flags |= VIR_DOMAIN_BLOCK_REBASE_RELATIVE; + + ret = virDomainBlockRebase(dom, path, base, bandwidth, flags); + } else { ret = virDomainBlockPull(dom, path, bandwidth, 0); + } break; case VSH_CMD_BLOCK_JOB_COMMIT: if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0 || @@ -2072,6 +2076,11 @@ static const vshCmdOptDef opts_block_pull[] = { .type = VSH_OT_BOOL, .help = N_("with --wait, don't wait for cancel to finish") }, + {.name = "keep-relative", + .type = VSH_OT_BOOL, + .help = N_("keep the backing chain relative if it was relatively " + "referenced if it was before") + }, {.name = NULL} }; @@ -2092,6 +2101,13 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd) bool quit = false; int abort_flags = 0; + if (vshCommandOptBool(cmd, "keep-relative") && + !vshCommandOptBool(cmd, "base")) { + vshError(ctl, "%s", _("--keep-relative is supported only with partial " + "pull operations with --base specified")); + return false; + } + if (blocking) { if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0) return false; diff --git a/tools/virsh.pod b/tools/virsh.pod index 5816c0b..84a60a7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -846,6 +846,7 @@ I<bandwidth> specifies copying bandwidth limit in MiB/s. =item B<blockpull> I<domain> I<path> [I<bandwidth>] [I<base>] [I<--wait> [I<--verbose>] [I<--timeout> B<seconds>] [I<--async>]] +[I<--keep-relative>] Populate a disk from its backing image chain. By default, this command flattens the entire chain; but if I<base> is specified, containing the @@ -865,6 +866,9 @@ is triggered, I<--async> will return control to the user as fast as possible, otherwise the command may continue to block a little while longer until the job is done cleaning up. +Using the I<--keep-relative> flag will try to keep the backing chain names +relative (if they were relative before). + I<path> specifies fully-qualified path of the disk; it corresponds to a unique target name (<target dev='name'/>) or source file (<source file='name'/>) for one of the disk devices attached to I<domain> (see -- 1.9.3

Now that we are able to select images from the backing chain via indexed access we should also convert possible network sources to qemu-compatible strings before passing them to qemu. --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2b7670e..07fd9a8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15029,6 +15029,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, virDomainDiskDefPtr disk; virStorageSourcePtr baseSource = NULL; unsigned int baseIndex = 0; + char *basePath = NULL; + char *backingPath = NULL; if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -15036,6 +15038,13 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, goto cleanup; } + if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE && !base) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE is valid only " + " with non-null base ")); + goto cleanup; + } + priv = vm->privateData; if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) { async = true; @@ -15097,10 +15106,35 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, base, baseIndex, NULL)))) goto endjob; + if (baseSource) { + if (qemuGetDriveSourceString(baseSource, NULL, &basePath) < 0) + goto endjob; + + if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE) { + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary doesn't support relative " + "block pull/rebase")); + goto endjob; + } + + if (virStorageFileGetRelativeBackingPath(disk->src->backingStore, + baseSource, + &backingPath) < 0) + goto endjob; + + + if (!backingPath) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Can't keep relative backing relationship.")); + goto endjob; + } + } + } + qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorBlockJob(priv->mon, device, - baseIndex ? baseSource->path : base, - NULL, bandwidth, info, mode, async); + ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath, + bandwidth, info, mode, async); qemuDomainObjExitMonitor(driver, vm); if (ret < 0) goto endjob; @@ -15172,6 +15206,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, } cleanup: + VIR_FREE(basePath); + VIR_FREE(backingPath); VIR_FREE(device); if (vm) virObjectUnlock(vm); @@ -15419,7 +15455,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT | VIR_DOMAIN_BLOCK_REBASE_COPY | - VIR_DOMAIN_BLOCK_REBASE_COPY_RAW, -1); + VIR_DOMAIN_BLOCK_REBASE_COPY_RAW | + VIR_DOMAIN_BLOCK_REBASE_RELATIVE, -1); if (!(vm = qemuDomObjFromDomain(dom))) return -1; -- 1.9.3