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
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
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
(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.
(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?
(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
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.
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