External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 05:35:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

blockcommit: require base below top

Browse Source 
The block commit code looks for an explicit base file relative
to the discovered top file; so for a chain of:
  base <- snap1 <- snap2 <- snap3
and a command of:
  virsh blockcommit $dom vda --base snap2 --top snap1
we got a sane message (here from libvirt 1.0.5):
error: invalid argument: could not find base 'snap2' below 'snap1' in chain for 'vda'

Meanwhile, recent refactoring has slightly reduced the quality of the
libvirt error messages, by losing the phrase 'below xyz':
error: invalid argument: could not find image 'snap2' in chain for 'snap3'

But we had a one-off, where we were not excluding the top file
itself in searching for the base; thankfully qemu still reports
the error, but the quality is worse:
  virsh blockcommit $dom vda --base snap2 --top snap2
error: internal error unable to execute QEMU command 'block-commit': Base '/snap2' not found

Fix the one-off in blockcommit by changing the semantics of name
lookup - if a starting point is specified, then the result must
be below that point, rather than including that point.  The only
other call to chain lookup was blockpull code, which was already
forcing the lookup to omit the active layer and only needs a
tweak to use the new semantics.

This also fixes the bug exposed in the testsuite, where when doing
a lookup pinned to an intermediate point in the chain, we were
unable to return the name of the parent also in the chain.

* src/util/virstoragefile.c (virStorageFileChainLookup): Change
semantics for non-NULL startFrom.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Adjust caller,
to keep existing semantics.
* tests/virstoragetest.c (mymain): Adjust to expose new semantics.

Signed-off-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Eric Blake 2014-06-11 16:22:57 -06:00
parent b10a0e9198
commit 3e3c6ff10f
3 changed files with 38 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/qemu/qemu_driver.c
View File

@ -15094,8 +15094,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
if (base &&
(virStorageFileParseChainIndex(disk->dst, base, &baseIndex) < 0 ||
!(baseSource = virStorageFileChainLookup(disk->src,
disk->src->backingStore,
!(baseSource = virStorageFileChainLookup(disk->src, disk->src,
base, baseIndex, NULL))))
goto endjob;

33
src/util/virstoragefile.c
View File

@ -1331,15 +1331,16 @@ virStorageFileParseChainIndex(const char *diskTarget,
return ret;
}
/* Given a @chain, look for the backing store @name within the chain starting
* from @startFrom or @chain if @startFrom is NULL and return that location
* within the chain. @chain must always point to the top of the chain. Pass
* NULL for @name and 0 for @idx to find the base of the chain. Pass nonzero
* @idx to find the backing source according to its position in the backing
* chain. 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.
/* Given a @chain, look for the backing store @name that is a backing file
* of @startFrom (or any member of @chain if @startFrom is NULL) and return
* that location within the chain. @chain must always point to the top of
* the chain. Pass NULL for @name and 0 for @idx to find the base of the
* chain. Pass nonzero @idx to find the backing source according to its
* position in the backing chain. 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.
*/
virStorageSourcePtr
virStorageFileChainLookup(virStorageSourcePtr chain,
@ -1360,10 +1361,11 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
i = 0;
if (startFrom) {
while (chain && chain != startFrom) {
while (chain && chain != startFrom->backingStore) {
chain = chain->backingStore;
i++;
}
*parent = startFrom->path;
}
while (chain) {
@ -1403,9 +1405,14 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
_("could not find backing store %u in chain for '%s'"),
idx, start);
} else if (name) {
virReportError(VIR_ERR_INVALID_ARG,
_("could not find image '%s' in chain for '%s'"),
name, start);
if (startFrom)
virReportError(VIR_ERR_INVALID_ARG,
_("could not find image '%s' beneath '%s' in "
"chain for '%s'"), name, startFrom->path, start);
else
virReportError(VIR_ERR_INVALID_ARG,
_("could not find image '%s' in chain for '%s'"),
name, start);
} else {
virReportError(VIR_ERR_INVALID_ARG,
_("could not find base image in chain for '%s'"),

36
tests/virstoragetest.c
View File

@ -942,31 +942,31 @@ mymain(void)
TEST_LOOKUP(0, NULL, "bogus", NULL, NULL, NULL);
TEST_LOOKUP(1, chain, "bogus", NULL, NULL, NULL);
TEST_LOOKUP(2, NULL, "wrap", chain->path, chain, NULL);
TEST_LOOKUP(3, chain, "wrap", chain->path, chain, NULL);
TEST_LOOKUP(3, chain, "wrap", NULL, NULL, NULL);
TEST_LOOKUP(4, chain2, "wrap", NULL, NULL, NULL);
TEST_LOOKUP(5, NULL, abswrap, chain->path, chain, NULL);
TEST_LOOKUP(6, chain, abswrap, chain->path, chain, NULL);
TEST_LOOKUP(6, chain, abswrap, NULL, NULL, NULL);
TEST_LOOKUP(7, chain2, abswrap, NULL, NULL, NULL);
TEST_LOOKUP(8, NULL, "qcow2", chain2->path, chain2, chain->path);
TEST_LOOKUP(9, chain, "qcow2", chain2->path, chain2, chain->path);
TEST_LOOKUP(10, chain2, "qcow2", chain2->path, chain2, NULL);
TEST_LOOKUP(10, chain2, "qcow2", NULL, NULL, NULL);
TEST_LOOKUP(11, chain3, "qcow2", NULL, NULL, NULL);
TEST_LOOKUP(12, NULL, absqcow2, chain2->path, chain2, chain->path);
TEST_LOOKUP(13, chain, absqcow2, chain2->path, chain2, chain->path);
TEST_LOOKUP(14, chain2, absqcow2, chain2->path, chain2, NULL);
TEST_LOOKUP(14, chain2, absqcow2, NULL, NULL, NULL);
TEST_LOOKUP(15, chain3, absqcow2, NULL, NULL, NULL);
TEST_LOOKUP(16, NULL, "raw", chain3->path, chain3, chain2->path);
TEST_LOOKUP(17, chain, "raw", chain3->path, chain3, chain2->path);
TEST_LOOKUP(18, chain2, "raw", chain3->path, chain3, chain2->path);
TEST_LOOKUP(19, chain3, "raw", chain3->path, chain3, NULL);
TEST_LOOKUP(19, chain3, "raw", NULL, NULL, NULL);
TEST_LOOKUP(20, NULL, absraw, chain3->path, chain3, chain2->path);
TEST_LOOKUP(21, chain, absraw, chain3->path, chain3, chain2->path);
TEST_LOOKUP(22, chain2, absraw, chain3->path, chain3, chain2->path);
TEST_LOOKUP(23, chain3, absraw, chain3->path, chain3, NULL);
TEST_LOOKUP(23, chain3, absraw, NULL, NULL, NULL);
TEST_LOOKUP(24, NULL, NULL, chain3->path, chain3, chain2->path);
TEST_LOOKUP(25, chain, NULL, chain3->path, chain3, chain2->path);
TEST_LOOKUP(26, chain2, NULL, chain3->path, chain3, chain2->path);
TEST_LOOKUP(27, chain3, NULL, chain3->path, chain3, NULL);
TEST_LOOKUP(27, chain3, NULL, NULL, NULL, NULL);
/* Rewrite wrap and qcow2 back to 3-deep chain, relative backing */
virCommandFree(cmd);
@ -995,31 +995,31 @@ mymain(void)
TEST_LOOKUP(28, NULL, "bogus", NULL, NULL, NULL);
TEST_LOOKUP(29, chain, "bogus", NULL, NULL, NULL);
TEST_LOOKUP(30, NULL, "wrap", chain->path, chain, NULL);
TEST_LOOKUP(31, chain, "wrap", chain->path, chain, NULL);
TEST_LOOKUP(31, chain, "wrap", NULL, NULL, NULL);
TEST_LOOKUP(32, chain2, "wrap", NULL, NULL, NULL);
TEST_LOOKUP(33, NULL, abswrap, chain->path, chain, NULL);
TEST_LOOKUP(34, chain, abswrap, chain->path, chain, NULL);
TEST_LOOKUP(34, chain, abswrap, NULL, NULL, NULL);
TEST_LOOKUP(35, chain2, abswrap, NULL, NULL, NULL);
TEST_LOOKUP(36, NULL, "qcow2", chain2->path, chain2, chain->path);
TEST_LOOKUP(37, chain, "qcow2", chain2->path, chain2, chain->path);
TEST_LOOKUP(38, chain2, "qcow2", chain2->path, chain2, NULL);
TEST_LOOKUP(38, chain2, "qcow2", NULL, NULL, NULL);
TEST_LOOKUP(39, chain3, "qcow2", NULL, NULL, NULL);
TEST_LOOKUP(40, NULL, absqcow2, chain2->path, chain2, chain->path);
TEST_LOOKUP(41, chain, absqcow2, chain2->path, chain2, chain->path);
TEST_LOOKUP(42, chain2, absqcow2, chain2->path, chain2, NULL);
TEST_LOOKUP(42, chain2, absqcow2, NULL, NULL, NULL);
TEST_LOOKUP(43, chain3, absqcow2, NULL, NULL, NULL);
TEST_LOOKUP(44, NULL, "raw", chain3->path, chain3, chain2->path);
TEST_LOOKUP(45, chain, "raw", chain3->path, chain3, chain2->path);
TEST_LOOKUP(46, chain2, "raw", chain3->path, chain3, chain2->path);
TEST_LOOKUP(47, chain3, "raw", chain3->path, chain3, NULL);
TEST_LOOKUP(47, chain3, "raw", NULL, NULL, NULL);
TEST_LOOKUP(48, NULL, absraw, chain3->path, chain3, chain2->path);
TEST_LOOKUP(49, chain, absraw, chain3->path, chain3, chain2->path);
TEST_LOOKUP(50, chain2, absraw, chain3->path, chain3, chain2->path);
TEST_LOOKUP(51, chain3, absraw, chain3->path, chain3, NULL);
TEST_LOOKUP(51, chain3, absraw, NULL, NULL, NULL);
TEST_LOOKUP(52, NULL, NULL, chain3->path, chain3, chain2->path);
TEST_LOOKUP(53, chain, NULL, chain3->path, chain3, chain2->path);
TEST_LOOKUP(54, chain2, NULL, chain3->path, chain3, chain2->path);
TEST_LOOKUP(55, chain3, NULL, chain3->path, chain3, NULL);
TEST_LOOKUP(55, chain3, NULL, NULL, NULL, NULL);
/* Use link to wrap with cross-directory relative backing */
virCommandFree(cmd);
@ -1054,15 +1054,14 @@ mymain(void)
TEST_LOOKUP_TARGET(67, "vda", NULL, "vda[-1]", 0, NULL, NULL, NULL);
TEST_LOOKUP_TARGET(68, "vda", NULL, "vda[1][1]", 0, NULL, NULL, NULL);
TEST_LOOKUP_TARGET(69, "vda", NULL, "wrap", 0, chain->path, chain, NULL);
TEST_LOOKUP_TARGET(70, "vda", chain, "wrap", 0, chain->path, chain, NULL);
TEST_LOOKUP_TARGET(70, "vda", chain, "wrap", 0, NULL, NULL, NULL);
TEST_LOOKUP_TARGET(71, "vda", chain2, "wrap", 0, NULL, NULL, NULL);
TEST_LOOKUP_TARGET(72, "vda", NULL, "vda[0]", 0, NULL, NULL, NULL);
TEST_LOOKUP_TARGET(73, "vda", NULL, "vda[1]", 1, chain2->path, chain2,
chain->path);
TEST_LOOKUP_TARGET(74, "vda", chain, "vda[1]", 1, chain2->path, chain2,
chain->path);
TEST_LOOKUP_TARGET(75, "vda", chain2, "vda[1]", 1, chain2->path, chain2,
NULL);
TEST_LOOKUP_TARGET(75, "vda", chain2, "vda[1]", 1, NULL, NULL, NULL);
TEST_LOOKUP_TARGET(76, "vda", chain3, "vda[1]", 1, NULL, NULL, NULL);
TEST_LOOKUP_TARGET(77, "vda", NULL, "vda[2]", 2, chain3->path, chain3,
chain2->path);
@ -1070,8 +1069,7 @@ mymain(void)
chain2->path);
TEST_LOOKUP_TARGET(79, "vda", chain2, "vda[2]", 2, chain3->path, chain3,
chain2->path);
TEST_LOOKUP_TARGET(80, "vda", chain3, "vda[2]", 2, chain3->path, chain3,
NULL);
TEST_LOOKUP_TARGET(80, "vda", chain3, "vda[2]", 2, NULL, NULL, NULL);
TEST_LOOKUP_TARGET(81, "vda", NULL, "vda[3]", 3, NULL, NULL, NULL);
cleanup: