[Qemu-devel] [PATCH for-2.10 v2 0/5] More bdrv_getlength() fixes
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:[0002] [FC] 'vpc: Check failure of bdrv_getlength()' 002/5:[down] 'qcow: Change signature of get_cluster_offset()' 003/5:[0048] [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()' Eric Blake (5): vpc: Check failure of bdrv_getlength() qcow: Change signature of get_cluster_offset() qcow: Check failure of bdrv_getlength() and bdrv_truncate() qcow2: Drop debugging dump_refcounts() qcow2: Check failure of bdrv_getlength() block/qcow.c | 165 ++ block/qcow2.c | 26 ++--- block/vpc.c | 9 +++- 3 files changed, 110 insertions(+), 90 deletions(-) -- 2.13.4
Re: [Qemu-devel] [PATCH for-2.10 v2 0/5] More bdrv_getlength() fixes
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:[0002] [FC] 'vpc: Check failure of bdrv_getlength()' > 002/5:[down] 'qcow: Change signature of get_cluster_offset()' > 003/5:[0048] [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? Kevin
Re: [Qemu-devel] [PATCH for-2.10 v2 0/5] More bdrv_getlength() fixes
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:[0002] [FC] 'vpc: Check failure of bdrv_getlength()' >> 002/5:[down] 'qcow: Change signature of get_cluster_offset()' >> 003/5:[0048] [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 signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH for-2.10 v2 0/5] More bdrv_getlength() fixes
Am 10.08.2017 um 17:08 hat Eric Blake geschrieben: > 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:[0002] [FC] 'vpc: Check failure of bdrv_getlength()' > >> 002/5:[down] 'qcow: Change signature of get_cluster_offset()' > >> 003/5:[0048] [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. Thanks, applied the patches to block and block-next, respectively. Kevin signature.asc Description: PGP signature