Bug 1110212

Summary: The error info is not correct when do blockcommit with --base and --top point to same source
Product: Red Hat Enterprise Linux 7 Reporter: Shanzhi Yu <shyu>
Component: libvirtAssignee: Eric Blake <eblake>
Status: CLOSED ERRATA QA Contact: Virtualization Bugs <virt-bugs>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.0CC: dyuan, eblake, mzhan, pkrempa, rbalakri, xuzhang, yanyang
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: libvirt-1.2.7-1.el7 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-03-05 07:37:44 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Shanzhi Yu 2014-06-17 09:09:26 UTC
Description of problem:

The error info is not correct when do blockcommit with --base and --top point to same source

Version-Release number of selected component (if applicable):

libvirt-1.1.1-29.el7.x86_64

How reproducible:

100%

Steps to Reproduce:

1.Prepare an running guest
# virsh list 
 Id    Name                           State
----------------------------------------------------
 3     rhel6                          running
# virsh domblklist rhel6
Target     Source
------------------------------------------------
vda        /var/lib/libvirt/images/base.img

2.Create three external disk snapshot
# for i in s1 s2 s3;do virsh snapshot-create-as rhel6 $i --disk-only;done
Domain snapshot s1 created
Domain snapshot s2 created
Domain snapshot s3 created

3. do blockcommit from base.s2 to base.s2

# virsh blockcommit rhel6 vda --base /var/lib/libvirt/images/base.s2 --top /var/lib/libvirt/images/base.s2
error: internal error: unable to execute QEMU command 'block-commit': Base '/var/lib/libvirt/images/base.s2' not found

# ll /var/lib/libvirt/images/base.s2
-rw-------. 1 qemu qemu 197120 Jun 17 12:58 /var/lib/libvirt/images/base.s2


Actual results:


Expected results:

Libvirt should give a proper error info 

Additional info:

Comment 1 Eric Blake 2014-06-17 11:46:06 UTC
Solved upstream with:

commit 3e3c6ff10fdb0bb086d61629d75bcad9169152b9
Author: Eric Blake <eblake>
Date:   Wed Jun 11 16:22:57 2014 -0600

    blockcommit: require base below top
    
    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>

Comment 3 Yang Yang 2014-12-09 14:12:20 UTC
Attepmted to verify it with the following component
libvirt-1.2.8-10.el7.x86_64
qemu-kvm-rhev-2.1.2-16.el7.x86_64
kernel-3.10.0-212.el7.x86_64

Steps:
1. create 4 external disk-only snapshots
# for i in {1..4}; do virsh snapshot-create-as vm2 s$i --disk-only --diskspec hda,file=/tmp/vm2.s$i; done
Domain snapshot s1 created
Domain snapshot s2 created
Domain snapshot s3 created
Domain snapshot s4 created

# virsh domblklist vm2
Target     Source
------------------------------------------------
hda        /tmp/vm2.s4

2. blockcommit using absolute path

# virsh blockcommit vm2 hda --base /tmp/vm2.s4 --top /tmp/vm2.s4 --wait --verbose --active
error: invalid argument: could not find image '/tmp/vm2.s4' beneath '/tmp/vm2.s4' in chain for '/tmp/vm2.s4'

# virsh blockcommit vm2 hda --base /tmp/vm2.s2 --top /tmp/vm2.s2 --wait --verbose
error: invalid argument: could not find image '/tmp/vm2.s2' beneath '/tmp/vm2.s2' in chain for '/tmp/vm2.s4'

# virsh blockcommit vm2 hda --base /var/lib/libvirt/images/vm2.qcow2 --top /var/lib/libvirt/images/vm2.qcow2 --wait --verbose 
error: invalid argument: top '/var/lib/libvirt/images/vm2.qcow2' in chain for 'hda' has no backing file

# virsh blockcommit vm2 hda --base /tmp/vm2.s4 --top /tmp/vm2.s2 --wait --verbose
error: invalid argument: could not find image '/tmp/vm2.s4' beneath '/tmp/vm2.s2' in chain for '/tmp/vm2.s4'

3. blockcommit passing hda[x]

# virsh blockcommit vm2 hda --base hda[4] --top hda[4] --wait --verbose
error: invalid argument: top '/var/lib/libvirt/images/vm2.qcow2' in chain for 'hda' has no backing file

# virsh blockcommit vm2 hda --base hda --top hda --wait --verbose
error: invalid argument: could not find image 'hda' in chain for '/tmp/vm2.s4'

# virsh blockcommit vm2 hda --base /tmp/vm2.s4 --top hda[2] --wait --verbose
error: invalid argument: could not find image '/tmp/vm2.s4' beneath '/tmp/vm2.s2' in chain for '/tmp/vm2.s4'

Hi Eric,
The error shows that 'could not find backing store X in chain...' when passing hda[X] to base. However, hda[X] is a backing store in chain. Does it deserve a fix ?

# virsh blockcommit vm2 hda --base hda[2] --top hda[2] --wait --verbose
error: invalid argument: could not find backing store 2 in chain for '/tmp/vm2.s4'

# virsh blockcommit vm2 hda --base hda[1] --top hda[2] --wait --verbose
error: invalid argument: could not find backing store 1 in chain for '/tmp/vm2.s4'


# virsh blockcommit vm2 hda --base hda[1] --top /tmp/vm2.s1 --wait --verbose
error: invalid argument: could not find backing store 1 in chain for '/tmp/vm2.s4'

Comment 4 Eric Blake 2014-12-23 16:02:05 UTC
(In reply to yangyang from comment #3)
> Attepmted to verify it with the following component
> libvirt-1.2.8-10.el7.x86_64
> qemu-kvm-rhev-2.1.2-16.el7.x86_64
> kernel-3.10.0-212.el7.x86_64
> 

> # virsh blockcommit vm2 hda --base hda[4] --top hda[4] --wait --verbose
> error: invalid argument: top '/var/lib/libvirt/images/vm2.qcow2' in chain
> for 'hda' has no backing file

Through here, the messages make sense.

> 
> # virsh blockcommit vm2 hda --base hda --top hda --wait --verbose
> error: invalid argument: could not find image 'hda' in chain for
> '/tmp/vm2.s4'

This message is slightly awkward, but still tolerable (it would be better if --base hda turned into /tmp/vm2.s4 in the same way that --top hda did).

> 
> # virsh blockcommit vm2 hda --base /tmp/vm2.s4 --top hda[2] --wait --verbose
> error: invalid argument: could not find image '/tmp/vm2.s4' beneath
> '/tmp/vm2.s2' in chain for '/tmp/vm2.s4'

Fine.

> 
> Hi Eric,
> The error shows that 'could not find backing store X in chain...' when
> passing hda[X] to base. However, hda[X] is a backing store in chain. Does it
> deserve a fix ?
> 
> # virsh blockcommit vm2 hda --base hda[2] --top hda[2] --wait --verbose
> error: invalid argument: could not find backing store 2 in chain for
> '/tmp/vm2.s4'

Awkward message, but correct that the attempt failed.

> 
> # virsh blockcommit vm2 hda --base hda[1] --top hda[2] --wait --verbose
> error: invalid argument: could not find backing store 1 in chain for
> '/tmp/vm2.s4'

Awkward message, but correct that the attempt failed.

> 
> 
> # virsh blockcommit vm2 hda --base hda[1] --top /tmp/vm2.s1 --wait --verbose
> error: invalid argument: could not find backing store 1 in chain for
> '/tmp/vm2.s4'

Awkward message, but correct that the attempt failed.

So at this point, the behavior is correct in all cases (we fail when we are supposed to), and the only thing remaining is that the error message is not as high quality as it could be.  I would suggest filing that as a separate bug, since it is lower priority and should not hold up a release, while this bug appears to be validated by your testing.

Comment 5 Yang Yang 2014-12-24 02:22:37 UTC
Thanks Eric.
Filed a separate low priority bug. 
https://bugzilla.redhat.com/show_bug.cgi?id=1177062
Mark this one as verified.

Comment 7 errata-xmlrpc 2015-03-05 07:37:44 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHSA-2015-0323.html