From 74430fe36456db5993e43a65558c80ef8b0c590b Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 11 Apr 2014 22:08:07 -0600 Subject: [PATCH] conf: drop redundant parameter to chain lookup The original chain lookup code had to pass in the starting name, because it was not available in the chain. But now that we have added fields to the struct, this parameter is redundant. * src/util/virstoragefile.h (virStorageFileChainLookup): Alter signature. * src/util/virstoragefile.c (virStorageFileChainLookup): Adjust handling of top of chain. * src/qemu/qemu_driver.c (qemuDomainBlockCommit): Adjust caller. * tests/virstoragetest.c (testStorageLookup, mymain): Likewise. Signed-off-by: Eric Blake --- src/qemu/qemu_driver.c | 3 +- src/util/virstoragefile.c | 18 ++++++------ src/util/virstoragefile.h | 3 +- tests/virstoragetest.c | 60 +++++++++++++++++++-------------------- 4 files changed, 40 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fed7d1cf9a..454e39fb11 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15343,7 +15343,6 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, top_canon = disk->src.path; top_meta = disk->backingChain; } else if (!(top_canon = virStorageFileChainLookup(disk->backingChain, - disk->src.path, top, &top_meta, &top_parent))) { goto endjob; @@ -15356,7 +15355,7 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, } if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)) base_canon = top_meta->backingStore; - else if (!(base_canon = virStorageFileChainLookup(top_meta, top_canon, + else if (!(base_canon = virStorageFileChainLookup(top_meta, base, NULL, NULL))) goto endjob; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 77cc8781f9..bc12cea14f 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1529,21 +1529,21 @@ int virStorageFileGetSCSIKey(const char *path, } #endif -/* Given a CHAIN that starts at the named file START, return a string - * pointing to either START or within CHAIN that gives the preferred - * name for the backing file NAME within that chain. Pass NULL for - * NAME to find the base of the chain. If META is not NULL, set *META - * to the point in the chain that describes NAME (or to NULL if the - * backing element is not a file). If PARENT is not NULL, set *PARENT - * to the preferred name of the parent (or to NULL if NAME matches - * START). Since the results point within CHAIN, they must not be +/* Given a CHAIN, look for the backing file NAME within the chain and + * return its canonical name. Pass NULL for NAME to find the base of + * the chain. If META is not NULL, set *META to the point in the + * chain that describes NAME (or to NULL if the backing element is not + * a file). If PARENT is not NULL, set *PARENT to the preferred name + * of the parent (or to NULL if NAME matches the start of the chain). + * Since the results point within CHAIN, they must not be * independently freed. Reports an error and returns NULL if NAME is * not found. */ const char * -virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start, +virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *name, virStorageFileMetadataPtr *meta, const char **parent) { + const char *start = chain->canonPath; virStorageFileMetadataPtr owner; const char *tmp; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index c747f20928..959937c528 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -303,11 +303,10 @@ int virStorageFileChainGetBroken(virStorageFileMetadataPtr chain, char **broken_file); const char *virStorageFileChainLookup(virStorageFileMetadataPtr chain, - const char *start, const char *name, virStorageFileMetadataPtr *meta, const char **parent) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + ATTRIBUTE_NONNULL(1); void virStorageFileFreeMetadata(virStorageFileMetadataPtr meta); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index de37f7fb67..5db3cde122 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -381,7 +381,6 @@ testStorageChain(const void *args) struct testLookupData { virStorageFileMetadataPtr chain; - const char *start; const char *name; const char *expResult; virStorageFileMetadataPtr expMeta; @@ -401,8 +400,8 @@ testStorageLookup(const void *args) * as the same string may be duplicated into more than one field, * we rely on STREQ rather than pointer equality. Test twice to * ensure optional parameters don't cause NULL deref. */ - actualResult = virStorageFileChainLookup(data->chain, data->start, - data->name, NULL, NULL); + actualResult = virStorageFileChainLookup(data->chain, data->name, + NULL, NULL); if (!data->expResult) { if (!virGetLastError()) { @@ -423,9 +422,8 @@ testStorageLookup(const void *args) ret = -1; } - actualResult = virStorageFileChainLookup(data->chain, data->start, - data->name, &actualMeta, - &actualParent); + actualResult = virStorageFileChainLookup(data->chain, data->name, + &actualMeta, &actualParent); if (!data->expResult) virResetLastError(); if (STRNEQ_NULLABLE(data->expResult, actualResult)) { @@ -454,7 +452,6 @@ mymain(void) virCommandPtr cmd = NULL; struct testChainData data; virStorageFileMetadataPtr chain = NULL; - const char *start = NULL; /* Prep some files with qemu-img; if that is not found on PATH, or * if it lacks support for qcow2 and qed, skip this test. */ @@ -849,8 +846,7 @@ mymain(void) if (virCommandRun(cmd, NULL) < 0) ret = -1; - /* Test behavior of chain lookups, absolute backing */ - start = "wrap"; + /* Test behavior of chain lookups, absolute backing from relative start */ chain = virStorageFileGetMetadata("wrap", VIR_STORAGE_FILE_QCOW2, -1, -1, false); if (!chain) { @@ -860,7 +856,7 @@ mymain(void) #define TEST_LOOKUP(id, name, result, meta, parent) \ do { \ - struct testLookupData data2 = { chain, start, name, \ + struct testLookupData data2 = { chain, name, \ result, meta, parent, }; \ if (virtTestRun("Chain lookup " #id, \ testStorageLookup, &data2) < 0) \ @@ -868,10 +864,12 @@ mymain(void) } while (0) TEST_LOOKUP(0, "bogus", NULL, NULL, NULL); - TEST_LOOKUP(1, "wrap", start, chain, NULL); - TEST_LOOKUP(2, abswrap, start, chain, NULL); - TEST_LOOKUP(3, "qcow2", chain->backingStore, chain->backingMeta, start); - TEST_LOOKUP(4, absqcow2, chain->backingStore, chain->backingMeta, start); + TEST_LOOKUP(1, "wrap", chain->canonPath, chain, NULL); + TEST_LOOKUP(2, abswrap, chain->canonPath, chain, NULL); + TEST_LOOKUP(3, "qcow2", chain->backingStore, chain->backingMeta, + chain->canonPath); + TEST_LOOKUP(4, absqcow2, chain->backingStore, chain->backingMeta, + chain->canonPath); TEST_LOOKUP(5, "raw", chain->backingMeta->backingStore, chain->backingMeta->backingMeta, chain->backingStore); TEST_LOOKUP(6, absraw, chain->backingMeta->backingStore, @@ -892,8 +890,7 @@ mymain(void) if (virCommandRun(cmd, NULL) < 0) ret = -1; - /* Test behavior of chain lookups, relative backing */ - start = abswrap; + /* Test behavior of chain lookups, relative backing from absolute start */ virStorageFileFreeMetadata(chain); chain = virStorageFileGetMetadata(abswrap, VIR_STORAGE_FILE_QCOW2, -1, -1, false); @@ -903,10 +900,12 @@ mymain(void) } TEST_LOOKUP(8, "bogus", NULL, NULL, NULL); - TEST_LOOKUP(9, "wrap", start, chain, NULL); - TEST_LOOKUP(10, abswrap, start, chain, NULL); - TEST_LOOKUP(11, "qcow2", chain->backingStore, chain->backingMeta, start); - TEST_LOOKUP(12, absqcow2, chain->backingStore, chain->backingMeta, start); + TEST_LOOKUP(9, "wrap", chain->canonPath, chain, NULL); + TEST_LOOKUP(10, abswrap, chain->canonPath, chain, NULL); + TEST_LOOKUP(11, "qcow2", chain->backingStore, chain->backingMeta, + chain->canonPath); + TEST_LOOKUP(12, absqcow2, chain->backingStore, chain->backingMeta, + chain->canonPath); TEST_LOOKUP(13, "raw", chain->backingMeta->backingStore, chain->backingMeta->backingMeta, chain->backingStore); TEST_LOOKUP(14, absraw, chain->backingMeta->backingStore, @@ -922,9 +921,8 @@ mymain(void) ret = -1; /* Test behavior of chain lookups, relative backing */ - start = "sub/link2"; virStorageFileFreeMetadata(chain); - chain = virStorageFileGetMetadata(start, VIR_STORAGE_FILE_QCOW2, + chain = virStorageFileGetMetadata("sub/link2", VIR_STORAGE_FILE_QCOW2, -1, -1, false); if (!chain) { ret = -1; @@ -932,15 +930,15 @@ mymain(void) } TEST_LOOKUP(16, "bogus", NULL, NULL, NULL); - TEST_LOOKUP(17, "sub/link2", start, chain, NULL); - TEST_LOOKUP(18, "wrap", start, chain, NULL); - TEST_LOOKUP(19, abswrap, start, chain, NULL); - TEST_LOOKUP(20, "../qcow2", chain->backingStore, chain->backingMeta, start); - /* FIXME: 21 is questionable, since there is no 'sub/qcow2' and - * since relative backing files should be looked up in the context - * of the directory containing the parent. */ - TEST_LOOKUP(21, "qcow2", chain->backingStore, chain->backingMeta, start); - TEST_LOOKUP(22, absqcow2, chain->backingStore, chain->backingMeta, start); + TEST_LOOKUP(17, "sub/link2", chain->canonPath, chain, NULL); + TEST_LOOKUP(18, "wrap", chain->canonPath, chain, NULL); + TEST_LOOKUP(19, abswrap, chain->canonPath, chain, NULL); + TEST_LOOKUP(20, "../qcow2", chain->backingStore, chain->backingMeta, + chain->canonPath); + TEST_LOOKUP(21, "qcow2", chain->backingStore, chain->backingMeta, + chain->canonPath); + TEST_LOOKUP(22, absqcow2, chain->backingStore, chain->backingMeta, + chain->canonPath); TEST_LOOKUP(23, "raw", chain->backingMeta->backingStore, chain->backingMeta->backingMeta, chain->backingStore); TEST_LOOKUP(24, absraw, chain->backingMeta->backingStore,