Re: [Qemu-block] [for 3.1? PATCH] qcow2: Assert that refcount block offsets fit in the refcount table

2018-11-14 Thread Kevin Wolf
Am 14.11.2018 um 08:10 hat Alberto Garcia geschrieben: > On Tue 13 Nov 2018 06:06:54 PM CET, Eric Blake wrote: > > >> Refcount table entries have a field to store the offset of the > >> refcount block. The rest of the bits of the entry are currently > >> reserved. > >> > >> The offset is always

[Qemu-block] [PATCH v2 04/13] block: Removed unused sector-based blocking I/O

2018-11-14 Thread Eric Blake
We are gradually moving away from sector-based interfaces, towards byte-based. Now that all callers of blocking I/O have been converted to use our preferred byte-based bdrv_p{read,write}(), we can delete the unused bdrv_{read,write}(). Note that the old byte-based code checked that callers

[Qemu-block] [PATCH v2 07/13] blklogwrites: Audit for read/write 64-bit cleanness

2018-11-14 Thread Eric Blake
Nothing in blk_log_writes_co_do_log() is inherently limited by a 32-bit type; document this by updating the refresh_limits callback to document that this driver is 64-bit clean. Signed-off-by: Eric Blake --- block/blklogwrites.c | 1 + 1 file changed, 1 insertion(+) diff --git

[Qemu-block] [PATCH v2 08/13] crypto: Audit for read/write 64-bit cleanness

2018-11-14 Thread Eric Blake
The crypto read/write functions do their own fragmentation (because everything has to go through a bounce buffer); while we could advertise BLOCK_CRYPTO_MAX_IO_SIZE as our max_transfer (and let the block layer do our fragmentation for us), I'm instead choosing to document that this driver is

[Qemu-block] [PATCH v2 10/13] file-posix: Audit for read/write 64-bit cleanness

2018-11-14 Thread Eric Blake
Any use of aio is inherently limited by size_t aio_nbytes in struct aiocb. read() is similarly limited to size_t bytes, although in practice, the ssize_t return means any read attempt on a 32-bit platform for more than 2G will likely return a short read (if that much memory was even available to

[Qemu-block] [PATCH v2 11/13] qcow2: Audit for read/write 64-bit cleanness

2018-11-14 Thread Eric Blake
The qcow2 read/write functions do their own fragmentation (because of cluster remapping); while we could advertise s->cluster_size and let the block layer do fragmentation for us, that would NOT solve the issue of the block layer handing us a length less than a cluster but at an offset which

[Qemu-block] [PATCH v2 06/13] blkdebug: Audit for read/write 64-bit cleanness

2018-11-14 Thread Eric Blake
Since the block layer is never supposed to hand us an offset + bytes that would exceed off_t, we can assert this in rule_check(). With that in place, there is nothing else in the pread, pwrite, or pwrite_zeroes code paths that can't handle inputs larger than 2G (even if the block layer currently

[Qemu-block] [PATCH v2 13/13] block: Enforce non-zero bl.max_transfer

2018-11-14 Thread Eric Blake
The raw format driver and the filter drivers default to picking up the same limits as what they wrap, and I've audited that they are otherwise simple enough in their passthrough to be 64-bit clean; it's not worth changing their .bdrv_refresh_limits to advertise anything different. Previous

[Qemu-block] [PATCH v2 09/13] RFC: crypto: Rely on block layer for fragmentation

2018-11-14 Thread Eric Blake
No need to reimplement fragmentation to BLOCK_CRYPTO_MAX_IO_SIZE ourselves when we can ask the block layer to do it for us. Signed-off-by: Eric Blake --- Question - is this patch for 'crypto' acceptable, or should we stick with just the previous one that marks things as 64-bit clean? ---

[Qemu-block] [PATCH v2 03/13] vvfat: Switch to byte-based calls

2018-11-14 Thread Eric Blake
We are gradually moving away from sector-based interfaces, towards byte-based. Make the change for the last few sector-based calls into the block layer from the vvfat driver. Ideally, the vvfat driver should switch to doing everything byte-based, but that's a more invasive change that requires a

[Qemu-block] [PATCH v2 05/13] block: Switch to 64-bit bl.max_transfer

2018-11-14 Thread Eric Blake
This change has no semantic impact: all drivers either leave the value at 0 (no inherent 32-bit limit is still translated into fragmentation below 2G; see the previous patch for that audit), or set it to a value less than 2G. However, switching to a larger type and enforcing the 2G cap at the

[Qemu-block] [PATCH v2 02/13] vdi: Switch to byte-based calls

2018-11-14 Thread Eric Blake
We are gradually moving away from sector-based interfaces, towards byte-based. Make the change for the last few sector-based calls into the block layer from the vdi driver. Ideally, the vdi driver should switch to doing everything byte-based, but that's a more invasive change that requires a bit

[Qemu-block] [PATCH v2 01/13] qcow2: Prefer byte-based calls into bs->file

2018-11-14 Thread Eric Blake
We had only a few sector-based stragglers left; convert them to use our preferred byte-based accesses. Signed-off-by: Eric Blake Reviewed-by: Alberto Garcia --- v2: rebased to threaded decompression handling [moved from a different series] v5: commit message tweak v2: indentation fix ---

[Qemu-block] [PATCH v2 for-4.0 00/13] block: byte-based blocking read/write

2018-11-14 Thread Eric Blake
Based-on: <20181114210548.1098207-1-ebl...@redhat.com> [file-posix: Better checks of 64-bit copy_range] Based-on: <20181101182738.70462-1-vsement...@virtuozzo.com> [0/7 qcow2 decompress in threads] - more specifically, on Kevin's block-next branch Also available at

Re: [Qemu-block] [Qemu-devel] [PATCH 1/1] virtio-blk: rename iov to out_iov in virtio_blk_handle_request()

2018-11-14 Thread Dongli Zhang
Ping? Thanks! Dongli Zhang On 11/07/2018 12:09 AM, Dongli Zhang wrote: > In virtio_blk_handle_request(), in_iov is used for input header while iov > is used for output header. Rename iov to out_iov to pair output header's > name with in_iov to avoid confusing people when reading source code. >

[Qemu-block] [PATCH] vvfat: Fix memory leak

2018-11-14 Thread Kevin Wolf
Don't leak 'cluster' in the mapping == NULL case. Found by Coverity (CID 1055918). Fixes: 8d9401c2791ee2d2805b741b1ee3006041edcd3e Signed-off-by: Kevin Wolf --- block/vvfat.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index

Re: [Qemu-block] [Qemu-devel] [PATCH] vvfat: Fix memory leak

2018-11-14 Thread Philippe Mathieu-Daudé
On 14/11/18 13:55, Kevin Wolf wrote: Don't leak 'cluster' in the mapping == NULL case. Found by Coverity (CID 1055918). Fixes: 8d9401c2791ee2d2805b741b1ee3006041edcd3e Signed-off-by: Kevin Wolf Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- block/vvfat.c | 6

Re: [Qemu-block] [PATCH v5 2/3] file-posix: Drop s->lock_fd

2018-11-14 Thread Alberto Garcia
On Thu 11 Oct 2018 09:21:34 AM CEST, Fam Zheng wrote: > The lock_fd field is not strictly necessary because transferring locked > bytes from old fd to the new one shouldn't fail anyway. This spares the > user one fd per image. > > Signed-off-by: Fam Zheng > Reviewed-by: Max Reitz One of my

Re: [Qemu-block] [PATCH for 3.1 v3 0/3] minor qcow2 compression improvements

2018-11-14 Thread Kevin Wolf
Am 14.11.2018 um 00:03 hat Eric Blake geschrieben: > As the added iotests shows, we have a (corner case) data corruption > that is user triggerable, therefore, this is still appropriate for > inclusion in 3.1. Thanks, applied to the block branch. Kevin

Re: [Qemu-block] [for 3.1? PATCH] qcow2: Assert that refcount block offsets fit in the refcount table

2018-11-14 Thread Alberto Garcia
On Wed 14 Nov 2018 11:49:34 AM CET, Kevin Wolf wrote: >> > Reviewed-by: Eric Blake >> >> Yes, for 3.1, shall I resend it with the updated subject message? > > Honestly, I don't see why an additional assertion should qualify as a > fix? If it changes the behaviour, it's a bug. We can leave it

Re: [Qemu-block] [PATCH 1/1] virtio-blk: rename iov to out_iov in virtio_blk_handle_request()

2018-11-14 Thread Stefan Hajnoczi
On Wed, Nov 07, 2018 at 12:09:16AM +0800, Dongli Zhang wrote: > In virtio_blk_handle_request(), in_iov is used for input header while iov > is used for output header. Rename iov to out_iov to pair output header's > name with in_iov to avoid confusing people when reading source code. > >

[Qemu-block] [PATCH v2 12/13] block: Document need for audit of read/write 64-bit cleanness

2018-11-14 Thread Eric Blake
At this time, any block driver that has not been audited for 64-bit cleanness, but which uses byte-based callbacks, should explicitly document that the driver wants the block layer to cap things at 2G. This patch has no semantic change. And it shows that the things I'm not interested in auditing

Re: [Qemu-block] [Qemu-devel] [PATCH v11 07/31] iotests.py: Add node_info()

2018-11-14 Thread Eric Blake
On 10/5/18 6:39 PM, Max Reitz wrote: This function queries a node; since we cannot do that right now, it executes query-named-block-nodes and returns the matching node's object. Signed-off-by: Max Reitz --- tests/qemu-iotests/iotests.py | 7 +++ 1 file changed, 7 insertions(+)

Re: [Qemu-block] [PATCH] nvme: fix oob access issue(CVE-2018-16847)

2018-11-14 Thread Li Qiang
Paolo Bonzini 于2018年11月14日周三 下午11:44写道: > On 14/11/2018 02:38, Li Qiang wrote: > > > > > > Paolo Bonzini mailto:pbonz...@redhat.com>> 于2018 > > 年11月14日周三 上午2:27写道: > > > > On 13/11/2018 11:17, Kevin Wolf wrote: > > > Am 13.11.2018 um 02:45 hat Li Qiang geschrieben: > > >> Ping

Re: [Qemu-block] [Qemu-devel] [PATCH v11 01/31] block: Use bdrv_refresh_filename() to pull

2018-11-14 Thread Eric Blake
On 10/5/18 6:39 PM, Max Reitz wrote: Before this patch, bdrv_refresh_filename() is used in a pushing manner: Whenever the BDS graph is modified, the parents of the modified edges are supposed to be updated (recursively upwards). However, that is nonviable, considering that we want child changes

Re: [Qemu-block] [PATCH] nvme: fix oob access issue(CVE-2018-16847)

2018-11-14 Thread Paolo Bonzini
On 14/11/2018 02:38, Li Qiang wrote: > > > Paolo Bonzini mailto:pbonz...@redhat.com>> 于2018 > 年11月14日周三 上午2:27写道: > > On 13/11/2018 11:17, Kevin Wolf wrote: > > Am 13.11.2018 um 02:45 hat Li Qiang geschrieben: > >> Ping what't the status of this patch. > >> > >> I see

Re: [Qemu-block] [Qemu-devel] [PATCH for 3.1 v3 0/3] minor qcow2 compression improvements

2018-11-14 Thread Eric Blake
On 11/13/18 5:03 PM, Eric Blake wrote: As the added iotests shows, we have a (corner case) data corruption that is user triggerable, therefore, this is still appropriate for inclusion in 3.1. v6 was here: https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg08497.html Wow, I can't count.

Re: [Qemu-block] [PATCH] vvfat: Fix memory leak

2018-11-14 Thread Liam Merwick
On 14/11/2018 12:55, Kevin Wolf wrote: Don't leak 'cluster' in the mapping == NULL case. Found by Coverity (CID 1055918). Fixes: 8d9401c2791ee2d2805b741b1ee3006041edcd3e Signed-off-by: Kevin Wolf Reviewed-by: Liam Merwick Thanks. --- block/vvfat.c | 6 +++--- 1 file changed, 3

[Qemu-block] [PATCH v2] qcow2: Assert that refcount block offsets fit in the refcount table

2018-11-14 Thread Alberto Garcia
Refcount table entries have a field to store the offset of the refcount block. The rest of the bits of the entry are currently reserved. The offset is always taken from the entry using REFT_OFFSET_MASK to ensure that we only use the bits that belong to that field. While that mask is used every

Re: [Qemu-block] KVM Forum block no[td]es

2018-11-14 Thread Max Reitz
On 13.11.18 16:12, Alberto Garcia wrote: > On Sun 11 Nov 2018 11:25:00 PM CET, Max Reitz wrote: > >> Permission system >> = >> >> GRAPH_MOD >> - >> >> We need some way for the commit job to prevent graph changes on its >> chain while it is running. Our current blocker

Re: [Qemu-block] [PATCH] block/nvme: call blk_drain in NVMe reset code to avoid lockups

2018-11-14 Thread Igor Druzhinin
On 06/11/2018 12:16, Igor Druzhinin wrote: > When blk_flush called in NVMe reset path S/C queues are already freed > which means that re-entering AIO handling loop having some IO requests > unfinished will lockup or crash as their SG structures being potentially > reused. Call blk_drain before

Re: [Qemu-block] KVM Forum block no[td]es

2018-11-14 Thread John Snow
On 11/12/18 10:25 AM, Max Reitz wrote: > On 12.11.18 00:36, Nir Soffer wrote: >> On Mon, Nov 12, 2018 at 12:25 AM Max Reitz > > wrote: >> >> This is what I’ve taken from two or three BoF-like get-togethers on >> blocky things.  Amendments are more than welcome,

Re: [Qemu-block] [Qemu-devel] [PATCH v2 03/11] block: Filtered children access functions

2018-11-14 Thread Max Reitz
On 12.11.18 23:17, Eric Blake wrote: > On 8/9/18 5:31 PM, Max Reitz wrote: >> What bs->file and bs->backing mean depends on the node.  For filter >> nodes, both signify a node that will eventually receive all R/W >> accesses.  For format nodes, bs->file contains metadata and data, and >>

Re: [Qemu-block] [Qemu-devel] [PATCH v2 04/11] block: Storage child access function

2018-11-14 Thread Max Reitz
On 12.11.18 23:32, Eric Blake wrote: > On 8/9/18 5:31 PM, Max Reitz wrote: >> For completeness' sake, add a function for accessing a node's storage >> child, too.  For filters, this is there filtered child; for non-filters, > > s/there/their/ > >> this is bs->file. >> >> Some places are

Re: [Qemu-block] [Qemu-devel] [PATCH v2 07/11] block: Leave BDS.backing_file constant

2018-11-14 Thread Max Reitz
On 13.11.18 00:08, Eric Blake wrote: > On 8/9/18 5:31 PM, Max Reitz wrote: >> Parts of the block layer treat BDS.backing_file as if it were whatever >> the image header says (i.e., if it is a relative path, it is relative to >> the overlay), other parts treat it like a cache for >>

[Qemu-block] [PATCH for-3.1?] file-posix: Better checks of 64-bit copy_range

2018-11-14 Thread Eric Blake
file-posix.c was taking a 64-bit bytes in raw_co_copy_range_to(), passing it through a 32-bit parameter of paio_submit_co_full(), then widening it back to size_t when assigning into acb->aio_nbytes. Looking at io.c, I can't quickly tell if bdrv_co_copy_range_internal() is fragmenting things to