Bug 1933050 - libvirtd crashes when storage pool contains volume with corrupt backing store
Summary: libvirtd crashes when storage pool contains volume with corrupt backing store
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux Advanced Virtualization
Classification: Red Hat
Component: libvirt
Version: 8.5
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: rc
: 8.4
Assignee: Peter Krempa
QA Contact: Meina Li
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-02-25 14:54 UTC by Peter Krempa
Modified: 2021-05-25 06:48 UTC (History)
4 users (show)

Fixed In Version: libvirt-7.0.0-7.el8
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-05-25 06:47:27 UTC
Type: Bug
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Peter Krempa 2021-02-25 14:54:09 UTC
Description of problem:


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


How reproducible:


Steps to Reproduce:
1. create image with broken backing store in a storage pool path

$ qemu-img create -f qcow2 -F qcow2 -b 'json:{' -u img.qcow2 10M

2. restart libvirtd/refresh pool
3.

Actual results:
libvirtd crashes

Comment 1 Peter Krempa 2021-03-01 15:10:17 UTC
Fixed upstream:

commit e83fbead66c50de3c030b58f48f24f79c87f1981
Author: Peter Krempa <pkrempa>
Date:   Thu Feb 25 11:22:58 2021 +0100

    storageBackendProbeTarget: Check return value of virStorageSourceNewFromBacking
    
    Commit bc3a78f61a2aaac3 errorneously removed the return value check from
    virStorageSourceNewFromBacking. In cases when we e.g. can't parse the
    backing store string this leads to a crash:
    
     #0  virStorageSourceGetActualType (def=0x0) at ../../../libvirt/src/conf/storage_source_conf.c:1014
     #1  0x00007ffff7cee4f9 in virStorageSourceIsLocalStorage (src=<optimized out>) at ../../../libvirt/src/conf/storage_source_conf.c:1026
     #2  0x00007ffff455c97c in storageBackendProbeTarget (encryption=0x7fff9c122ce8, target=0x7fff9c122c68) at ../../../libvirt/src/storage/storage_util.c:3443
     #3  virStorageBackendRefreshVolTargetUpdate (vol=0x7fff9c122c30) at ../../../libvirt/src/storage/storage_util.c:3519
     #4  0x00007ffff455cdc0 in virStorageBackendRefreshLocal (pool=0x7fff9c010ea0) at ../../../libvirt/src/storage/storage_util.c:3593
     #5  0x00007ffff454f0a1 in storagePoolRefreshImpl
        (backend=backend@entry=0x7ffff4711180 <virStorageBackendDirectory>, obj=obj@entry=0x7fff9c010ea0, stateFile=stateFile@entry=0x7fff9c111a90 "/var/run/libvirt/storage/tmp.xml") at ../../../libvirt/src/storage/storage_driver.c:103
     #6  0x00007ffff4550ea5 in storagePoolUpdateStateCallback (obj=0x7fff9c010ea0, opaque=<optimized out>) at ../../../libvirt/src/storage/storage_driver.c:165
     #7  0x00007ffff7cefef4 in virStoragePoolObjListForEachCb (payload=<optimized out>, name=<optimized out>, opaque=0x7fffc8a489c0)
        at ../../../libvirt/src/conf/virstorageobj.c:435
     #8  0x00007ffff7c03195 in virHashForEachSafe
        (table=<optimized out>, iter=iter@entry=0x7ffff7cefec0 <virStoragePoolObjListForEachCb>, opaque=opaque@entry=0x7fffc8a489c0)
        at ../../../libvirt/src/util/virhash.c:414
     #9  0x00007ffff7cf0520 in virStoragePoolObjListForEach
        (pools=<optimized out>, iter=iter@entry=0x7ffff4550e10 <storagePoolUpdateStateCallback>, opaque=opaque@entry=0x0)
        at ../../../libvirt/src/conf/virstorageobj.c:468
     #10 0x00007ffff454f43a in storagePoolUpdateAllState () at ../../../libvirt/src/storage/storage_driver.c:184
     #11 storageStateInitialize (privileged=<optimized out>, root=<optimized out>, callback=<optimized out>, opaque=<optimized out>)
        at ../../../libvirt/src/storage/storage_driver.c:315
     #12 0x00007ffff7e10c04 in virStateInitialize
        (opaque=0x555555621820, callback=0x55555557b1d0 <daemonInhibitCallback>, root=0x0, mandatory=<optimized out>, privileged=true)
        at ../../../libvirt/src/libvirt.c:656
     #13 virStateInitialize
        (privileged=<optimized out>, mandatory=mandatory@entry=false, root=root@entry=0x0, callback=callback@entry=0x55555557b1d0 <daemonInhibitCallback>, opaque=opaque@entry=0x555555621820) at ../../../libvirt/src/libvirt.c:638
     #14 0x000055555557b230 in daemonRunStateInit (opaque=0x555555621820) at ../../../libvirt/src/remote/remote_daemon.c:605
     #15 0x00007ffff7c46bb5 in virThreadHelper (data=<optimized out>) at ../../../libvirt/src/util/virthread.c:233
     #16 0x00007ffff6e453f9 in start_thread () at /lib64/libpthread.so.0
     #17 0x00007ffff766fb53 in clone () at /lib64/libc.so
    
    An invalid image can be easily created by:
    
    $ qemu-img create -f qcow2 -F qcow2 -b 'json:{' -u img.qcow2 10M

Comment 5 Meina Li 2021-03-05 02:42:15 UTC
Hi Peter,

Can you help to check if the test result is expected? 

# qemu-img create -f qcow2 -F qcow2 -b 'json:{' -u /var/lib/libvirt/images/test.img 200M
Formatting '/var/lib/libvirt/images/test.img', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=209715200 backing_file=json: backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16

# virsh pool-refresh default 
error: Failed to refresh pool default
error: internal error: cannot parse json : parse error: premature EOF
                                       
                     (right here) ------^

I think it should support refresh the pool. If not, the pool will change to be inactive after restart libvirtd service. please check again.

# qemu-img create -f qcow2 -F qcow2 -b 'json:{' -u /var/lib/libvirt/images/test.img 200M
Formatting '/var/lib/libvirt/images/test.img', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=209715200 backing_file=json: backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
# systemctl restart libvirtd
# virsh pool-list --all
 Name      State      Autostart
---------------------------------
 default   inactive   no
 images    active     yes

Comment 6 Peter Krempa 2021-03-05 09:53:16 UTC
(In reply to Meina Li from comment #5)
> Hi Peter,
> 
> Can you help to check if the test result is expected? 
> 
> # qemu-img create -f qcow2 -F qcow2 -b 'json:{' -u
> /var/lib/libvirt/images/test.img 200M
> Formatting '/var/lib/libvirt/images/test.img', fmt=qcow2 cluster_size=65536
> extended_l2=off compression_type=zlib size=209715200 backing_file=json:
> backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
> 
> # virsh pool-refresh default 
> error: Failed to refresh pool default
> error: internal error: cannot parse json : parse error: premature EOF
>                                        
>                      (right here) ------^
> 
> I think it should support refresh the pool. If not, the pool will change to
> be inactive after restart libvirtd service. please check again.

That is a different pre-existing bug. Upstream fixes it by 

commit 104db1951d3abfd8cb363b8e8070f712044bc645
Author: Peter Krempa <pkrempa>
Date:   Thu Feb 25 13:51:51 2021 +0100

    storageBackendProbeTarget: Don't fail if backing store can't be parsed
    
    When the backing store of the image can't be parsed
    virStorageSourceNewFromBacking returns -1. storageBackendProbeTarget
    then also fails which makes the pool refresh fail or even the storage
    pool becomes inactive after (re)start of libvirtd.
    
    In situations when we can't access the backing store via network we
    just report the backing store string, thus we can do the same thing for
    unparsable backing store to prevent the pool from going offline.

Comment 7 Meina Li 2021-03-08 03:22:06 UTC
(In reply to Peter Krempa from comment #6)

> 
> That is a different pre-existing bug. Upstream fixes it by 
> 
> commit 104db1951d3abfd8cb363b8e8070f712044bc645
> Author: Peter Krempa <pkrempa>
> Date:   Thu Feb 25 13:51:51 2021 +0100
> 
>     storageBackendProbeTarget: Don't fail if backing store can't be parsed
>     
>     When the backing store of the image can't be parsed
>     virStorageSourceNewFromBacking returns -1. storageBackendProbeTarget
>     then also fails which makes the pool refresh fail or even the storage
>     pool becomes inactive after (re)start of libvirtd.
>     
>     In situations when we can't access the backing store via network we
>     just report the backing store string, thus we can do the same thing for
>     unparsable backing store to prevent the pool from going offline.

Which is the pre-existing bug you mentioned? Sorry for didn't find it.

And if there's no bug track this patch, can we track this patch in this bug and reset the ITM of this bug?

Comment 8 Peter Krempa 2021-03-09 14:28:59 UTC
(In reply to Meina Li from comment #7)
> (In reply to Peter Krempa from comment #6)
> 
> > 
> > That is a different pre-existing bug. Upstream fixes it by 
> > 
> > commit 104db1951d3abfd8cb363b8e8070f712044bc645
> > Author: Peter Krempa <pkrempa>
> > Date:   Thu Feb 25 13:51:51 2021 +0100
> > 
> >     storageBackendProbeTarget: Don't fail if backing store can't be parsed
> >     
> >     When the backing store of the image can't be parsed
> >     virStorageSourceNewFromBacking returns -1. storageBackendProbeTarget
> >     then also fails which makes the pool refresh fail or even the storage
> >     pool becomes inactive after (re)start of libvirtd.
> >     
> >     In situations when we can't access the backing store via network we
> >     just report the backing store string, thus we can do the same thing for
> >     unparsable backing store to prevent the pool from going offline.
> 
> Which is the pre-existing bug you mentioned? Sorry for didn't find it.

I meant that it's a bug in the code that was existing even before this BZ for a long time. There is no specific BZ for it.


> And if there's no bug track this patch, can we track this patch in this bug
> and reset the ITM of this bug?

This bug was meant only to fix the crasher bug. I don't plan to backport the logic change to avoid failure of the pool startup to av-8.4, so it will be rebased into av-8.5

Comment 9 Meina Li 2021-03-10 06:58:52 UTC
Verified Version:
# rpm -q libvirt qemu-kvm
libvirt-7.0.0-8.module+el8.4.0+10233+8b7fd9eb.x86_64
qemu-kvm-5.2.0-10.module+el8.4.0+10217+cbdd2152.x86_64

Verified Steps:
1. Prepare a dir pool.
# virsh pool-dumpxml default 
<pool type='dir'>
  <name>default</name>
  <uuid>7932d41a-daec-4522-b405-d3f0e92b3092</uuid>
  <capacity unit='bytes'>75125227520</capacity>
  <allocation unit='bytes'>17200705536</allocation>
  <available unit='bytes'>57924521984</available>
  <source>
  </source>
  <target>
    <path>/var/lib/libvirt/images</path>
    <permissions>
      <mode>0711</mode>
      <owner>0</owner>
      <group>0</group>
      <label>system_u:object_r:virt_image_t:s0</label>
    </permissions>
  </target>
</pool>
2. Create an image with a broken backing store in the storage pool path.
# qemu-img create -f qcow2 -F qcow2 -b 'json:{' -u /var/lib/libvirt/images/test.img 200M
Formatting '/var/lib/libvirt/images/test.img', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=209715200 backing_file=json:{ backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
3. Restart the libvirtd or refresh the default pool.
1) Restart libvirtd.
# systemctl restart libvirtd
---libvirtd will not crash.
2) Or refresh the default pool.
# virsh pool-refresh default 
error: Failed to refresh pool default
error: internal error: cannot parse json {: parse error: premature EOF
                                       {
                     (right here) ------^
---libvirtd will not crash here, and the current error will be tracked in bug 1937204.

So move this bug to be verified.

Comment 11 errata-xmlrpc 2021-05-25 06:47:27 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 (virt:av bug fix and enhancement update), 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://access.redhat.com/errata/RHBA-2021:2098


Note You need to log in before you can comment on or make changes to this bug.