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 <eblake@redhat.com>
This commit is contained in:
Eric Blake 2014-04-11 22:08:07 -06:00
parent 6752bc2add
commit 74430fe364
4 changed files with 40 additions and 44 deletions

View File

@ -15343,7 +15343,6 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
top_canon = disk->src.path; top_canon = disk->src.path;
top_meta = disk->backingChain; top_meta = disk->backingChain;
} else if (!(top_canon = virStorageFileChainLookup(disk->backingChain, } else if (!(top_canon = virStorageFileChainLookup(disk->backingChain,
disk->src.path,
top, &top_meta, top, &top_meta,
&top_parent))) { &top_parent))) {
goto endjob; goto endjob;
@ -15356,7 +15355,7 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
} }
if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)) if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW))
base_canon = top_meta->backingStore; base_canon = top_meta->backingStore;
else if (!(base_canon = virStorageFileChainLookup(top_meta, top_canon, else if (!(base_canon = virStorageFileChainLookup(top_meta,
base, NULL, NULL))) base, NULL, NULL)))
goto endjob; goto endjob;

View File

@ -1529,21 +1529,21 @@ int virStorageFileGetSCSIKey(const char *path,
} }
#endif #endif
/* Given a CHAIN that starts at the named file START, return a string /* Given a CHAIN, look for the backing file NAME within the chain and
* pointing to either START or within CHAIN that gives the preferred * return its canonical name. Pass NULL for NAME to find the base of
* name for the backing file NAME within that chain. Pass NULL for * the chain. If META is not NULL, set *META to the point in the
* NAME to find the base of the chain. If META is not NULL, set *META * chain that describes NAME (or to NULL if the backing element is not
* to the point in the chain that describes NAME (or to NULL if the * a file). If PARENT is not NULL, set *PARENT to the preferred name
* backing element is not a file). If PARENT is not NULL, set *PARENT * of the parent (or to NULL if NAME matches the start of the chain).
* to the preferred name of the parent (or to NULL if NAME matches * Since the results point within CHAIN, they must not be
* START). Since the results point within CHAIN, they must not be
* independently freed. Reports an error and returns NULL if NAME is * independently freed. Reports an error and returns NULL if NAME is
* not found. */ * not found. */
const char * const char *
virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start, virStorageFileChainLookup(virStorageFileMetadataPtr chain,
const char *name, virStorageFileMetadataPtr *meta, const char *name, virStorageFileMetadataPtr *meta,
const char **parent) const char **parent)
{ {
const char *start = chain->canonPath;
virStorageFileMetadataPtr owner; virStorageFileMetadataPtr owner;
const char *tmp; const char *tmp;

View File

@ -303,11 +303,10 @@ int virStorageFileChainGetBroken(virStorageFileMetadataPtr chain,
char **broken_file); char **broken_file);
const char *virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *virStorageFileChainLookup(virStorageFileMetadataPtr chain,
const char *start,
const char *name, const char *name,
virStorageFileMetadataPtr *meta, virStorageFileMetadataPtr *meta,
const char **parent) const char **parent)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); ATTRIBUTE_NONNULL(1);
void virStorageFileFreeMetadata(virStorageFileMetadataPtr meta); void virStorageFileFreeMetadata(virStorageFileMetadataPtr meta);

View File

@ -381,7 +381,6 @@ testStorageChain(const void *args)
struct testLookupData struct testLookupData
{ {
virStorageFileMetadataPtr chain; virStorageFileMetadataPtr chain;
const char *start;
const char *name; const char *name;
const char *expResult; const char *expResult;
virStorageFileMetadataPtr expMeta; virStorageFileMetadataPtr expMeta;
@ -401,8 +400,8 @@ testStorageLookup(const void *args)
* as the same string may be duplicated into more than one field, * as the same string may be duplicated into more than one field,
* we rely on STREQ rather than pointer equality. Test twice to * we rely on STREQ rather than pointer equality. Test twice to
* ensure optional parameters don't cause NULL deref. */ * ensure optional parameters don't cause NULL deref. */
actualResult = virStorageFileChainLookup(data->chain, data->start, actualResult = virStorageFileChainLookup(data->chain, data->name,
data->name, NULL, NULL); NULL, NULL);
if (!data->expResult) { if (!data->expResult) {
if (!virGetLastError()) { if (!virGetLastError()) {
@ -423,9 +422,8 @@ testStorageLookup(const void *args)
ret = -1; ret = -1;
} }
actualResult = virStorageFileChainLookup(data->chain, data->start, actualResult = virStorageFileChainLookup(data->chain, data->name,
data->name, &actualMeta, &actualMeta, &actualParent);
&actualParent);
if (!data->expResult) if (!data->expResult)
virResetLastError(); virResetLastError();
if (STRNEQ_NULLABLE(data->expResult, actualResult)) { if (STRNEQ_NULLABLE(data->expResult, actualResult)) {
@ -454,7 +452,6 @@ mymain(void)
virCommandPtr cmd = NULL; virCommandPtr cmd = NULL;
struct testChainData data; struct testChainData data;
virStorageFileMetadataPtr chain = NULL; virStorageFileMetadataPtr chain = NULL;
const char *start = NULL;
/* Prep some files with qemu-img; if that is not found on PATH, or /* 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. */ * if it lacks support for qcow2 and qed, skip this test. */
@ -849,8 +846,7 @@ mymain(void)
if (virCommandRun(cmd, NULL) < 0) if (virCommandRun(cmd, NULL) < 0)
ret = -1; ret = -1;
/* Test behavior of chain lookups, absolute backing */ /* Test behavior of chain lookups, absolute backing from relative start */
start = "wrap";
chain = virStorageFileGetMetadata("wrap", VIR_STORAGE_FILE_QCOW2, chain = virStorageFileGetMetadata("wrap", VIR_STORAGE_FILE_QCOW2,
-1, -1, false); -1, -1, false);
if (!chain) { if (!chain) {
@ -860,7 +856,7 @@ mymain(void)
#define TEST_LOOKUP(id, name, result, meta, parent) \ #define TEST_LOOKUP(id, name, result, meta, parent) \
do { \ do { \
struct testLookupData data2 = { chain, start, name, \ struct testLookupData data2 = { chain, name, \
result, meta, parent, }; \ result, meta, parent, }; \
if (virtTestRun("Chain lookup " #id, \ if (virtTestRun("Chain lookup " #id, \
testStorageLookup, &data2) < 0) \ testStorageLookup, &data2) < 0) \
@ -868,10 +864,12 @@ mymain(void)
} while (0) } while (0)
TEST_LOOKUP(0, "bogus", NULL, NULL, NULL); TEST_LOOKUP(0, "bogus", NULL, NULL, NULL);
TEST_LOOKUP(1, "wrap", start, chain, NULL); TEST_LOOKUP(1, "wrap", chain->canonPath, chain, NULL);
TEST_LOOKUP(2, abswrap, start, chain, NULL); TEST_LOOKUP(2, abswrap, chain->canonPath, chain, NULL);
TEST_LOOKUP(3, "qcow2", chain->backingStore, chain->backingMeta, start); TEST_LOOKUP(3, "qcow2", chain->backingStore, chain->backingMeta,
TEST_LOOKUP(4, absqcow2, chain->backingStore, chain->backingMeta, start); chain->canonPath);
TEST_LOOKUP(4, absqcow2, chain->backingStore, chain->backingMeta,
chain->canonPath);
TEST_LOOKUP(5, "raw", chain->backingMeta->backingStore, TEST_LOOKUP(5, "raw", chain->backingMeta->backingStore,
chain->backingMeta->backingMeta, chain->backingStore); chain->backingMeta->backingMeta, chain->backingStore);
TEST_LOOKUP(6, absraw, chain->backingMeta->backingStore, TEST_LOOKUP(6, absraw, chain->backingMeta->backingStore,
@ -892,8 +890,7 @@ mymain(void)
if (virCommandRun(cmd, NULL) < 0) if (virCommandRun(cmd, NULL) < 0)
ret = -1; ret = -1;
/* Test behavior of chain lookups, relative backing */ /* Test behavior of chain lookups, relative backing from absolute start */
start = abswrap;
virStorageFileFreeMetadata(chain); virStorageFileFreeMetadata(chain);
chain = virStorageFileGetMetadata(abswrap, VIR_STORAGE_FILE_QCOW2, chain = virStorageFileGetMetadata(abswrap, VIR_STORAGE_FILE_QCOW2,
-1, -1, false); -1, -1, false);
@ -903,10 +900,12 @@ mymain(void)
} }
TEST_LOOKUP(8, "bogus", NULL, NULL, NULL); TEST_LOOKUP(8, "bogus", NULL, NULL, NULL);
TEST_LOOKUP(9, "wrap", start, chain, NULL); TEST_LOOKUP(9, "wrap", chain->canonPath, chain, NULL);
TEST_LOOKUP(10, abswrap, start, chain, NULL); TEST_LOOKUP(10, abswrap, chain->canonPath, chain, NULL);
TEST_LOOKUP(11, "qcow2", chain->backingStore, chain->backingMeta, start); TEST_LOOKUP(11, "qcow2", chain->backingStore, chain->backingMeta,
TEST_LOOKUP(12, absqcow2, chain->backingStore, chain->backingMeta, start); chain->canonPath);
TEST_LOOKUP(12, absqcow2, chain->backingStore, chain->backingMeta,
chain->canonPath);
TEST_LOOKUP(13, "raw", chain->backingMeta->backingStore, TEST_LOOKUP(13, "raw", chain->backingMeta->backingStore,
chain->backingMeta->backingMeta, chain->backingStore); chain->backingMeta->backingMeta, chain->backingStore);
TEST_LOOKUP(14, absraw, chain->backingMeta->backingStore, TEST_LOOKUP(14, absraw, chain->backingMeta->backingStore,
@ -922,9 +921,8 @@ mymain(void)
ret = -1; ret = -1;
/* Test behavior of chain lookups, relative backing */ /* Test behavior of chain lookups, relative backing */
start = "sub/link2";
virStorageFileFreeMetadata(chain); virStorageFileFreeMetadata(chain);
chain = virStorageFileGetMetadata(start, VIR_STORAGE_FILE_QCOW2, chain = virStorageFileGetMetadata("sub/link2", VIR_STORAGE_FILE_QCOW2,
-1, -1, false); -1, -1, false);
if (!chain) { if (!chain) {
ret = -1; ret = -1;
@ -932,15 +930,15 @@ mymain(void)
} }
TEST_LOOKUP(16, "bogus", NULL, NULL, NULL); TEST_LOOKUP(16, "bogus", NULL, NULL, NULL);
TEST_LOOKUP(17, "sub/link2", start, chain, NULL); TEST_LOOKUP(17, "sub/link2", chain->canonPath, chain, NULL);
TEST_LOOKUP(18, "wrap", start, chain, NULL); TEST_LOOKUP(18, "wrap", chain->canonPath, chain, NULL);
TEST_LOOKUP(19, abswrap, start, chain, NULL); TEST_LOOKUP(19, abswrap, chain->canonPath, chain, NULL);
TEST_LOOKUP(20, "../qcow2", chain->backingStore, chain->backingMeta, start); TEST_LOOKUP(20, "../qcow2", chain->backingStore, chain->backingMeta,
/* FIXME: 21 is questionable, since there is no 'sub/qcow2' and chain->canonPath);
* since relative backing files should be looked up in the context TEST_LOOKUP(21, "qcow2", chain->backingStore, chain->backingMeta,
* of the directory containing the parent. */ chain->canonPath);
TEST_LOOKUP(21, "qcow2", chain->backingStore, chain->backingMeta, start); TEST_LOOKUP(22, absqcow2, chain->backingStore, chain->backingMeta,
TEST_LOOKUP(22, absqcow2, chain->backingStore, chain->backingMeta, start); chain->canonPath);
TEST_LOOKUP(23, "raw", chain->backingMeta->backingStore, TEST_LOOKUP(23, "raw", chain->backingMeta->backingStore,
chain->backingMeta->backingMeta, chain->backingStore); chain->backingMeta->backingMeta, chain->backingStore);
TEST_LOOKUP(24, absraw, chain->backingMeta->backingStore, TEST_LOOKUP(24, absraw, chain->backingMeta->backingStore,