On 08/10/2017 08:02 AM, Kevin Wolf wrote: > Am 09.08.2017 um 22:38 hat Eric Blake geschrieben: >> We already have a lot of bdrv_getlength() fixes in -rc2; so I think >> this is still okay for -rc3. >> >> v1 was here (with a typo'd subject line): >> https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01226.html >> >> Since v1: >> - patch 1: fix error message capitalization (Kevin, R-b kept) >> - fix locking bug in original patch 2 (Kevin) >> - split original patch 2 into two parts: signature update, and >> added error checking (Kevin) >> - check for unlikely integer overflow before bdrv_truncate (Jeff) >> >> 001/5: [FC] 'vpc: Check failure of bdrv_getlength()' >> 002/5:[down] 'qcow: Change signature of get_cluster_offset()' >> 003/5: [FC] 'qcow: Check failure of bdrv_getlength() and >> bdrv_truncate()' >> 004/5:[----] [--] 'qcow2: Drop debugging dump_refcounts()' >> 005/5:[----] [--] 'qcow2: Check failure of bdrv_getlength()' > > Looks good to me, but as the bug is far from being critical, I'd rather > apply the more complex qcow1 patches only to block-next. The vpc and > qcow2 parts seems a lot less risky, so 2.10 should be okay for them. > > What do you think?
The argument for NOT doing the qcow changes (patches 2 and 3): the only place where we are not checking for failures is part of get_cluster_offset() - but in all likelihood, if we were unable to determine or change the length of the backing file, we will have nearby problems that will ultimately cause failure soon enough. Furthermore, it's not a regression (we've had several releases with the problem), and qcow is not a good format (it's painfully slow, and we strongly recommend qcow2 instead) - so no one will be hitting any actual bugs in practice. I'll trust your judgment as maintainer, so taking just 1, 4, and 5 in 2.10 is fine. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Description: OpenPGP digital signature