| Message ID | 20200430133007.170335-3-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vmdk: Fix zero cluster handling | expand |
On 4/30/20 8:30 AM, Kevin Wolf wrote: > m_data must be contain valid data even for zero clusters when no cluster s/be // > was allocated in the image file. Without this, zero writes segfault with > images that have zeroed_grain=on. > > For zero writes, we don't want to allocate a cluster in the image file > even in compressed files. > > Fixes: 524089bce43fd1cd3daaca979872451efa2cf7c6 Nearly 4 years ago, and claims to fix a different regression. Wow, we aren't doing too well with vmdk. > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block/vmdk.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com>
On 4/30/20 8:30 AM, Kevin Wolf wrote: > m_data must be contain valid data even for zero clusters when no cluster > was allocated in the image file. Without this, zero writes segfault with > images that have zeroed_grain=on. zero_grained=on ? > > For zero writes, we don't want to allocate a cluster in the image file > even in compressed files. > > Fixes: 524089bce43fd1cd3daaca979872451efa2cf7c6 > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block/vmdk.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-)
Am 30.04.2020 um 16:19 hat Eric Blake geschrieben: > On 4/30/20 8:30 AM, Kevin Wolf wrote: > > m_data must be contain valid data even for zero clusters when no cluster > > was allocated in the image file. Without this, zero writes segfault with > > images that have zeroed_grain=on. > > zero_grained=on ? No, zeroed_grain is the actual name of the option. I don't really know what a grain is in VMDK terminology, but about the only thing that felt healthy about the code I touched was that it has whole-grain buffers. :-) Kevin
diff --git a/block/vmdk.c b/block/vmdk.c index 5b09275578..bdd7d2dcf1 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1572,6 +1572,12 @@ static int get_cluster_offset(BlockDriverState *bs, extent->l2_cache_counts[min_index] = 1; found: l2_index = ((offset >> 9) / extent->cluster_sectors) % extent->l2_size; + if (m_data) { + m_data->l1_index = l1_index; + m_data->l2_index = l2_index; + m_data->l2_offset = l2_offset; + m_data->l2_cache_entry = ((uint32_t *)l2_table) + l2_index; + } if (extent->sesparse) { cluster_sector = le64_to_cpu(((uint64_t *)l2_table)[l2_index]); @@ -1631,10 +1637,6 @@ static int get_cluster_offset(BlockDriverState *bs, } if (m_data) { m_data->new_allocation = true; - m_data->l1_index = l1_index; - m_data->l2_index = l2_index; - m_data->l2_offset = l2_offset; - m_data->l2_cache_entry = ((uint32_t *)l2_table) + l2_index; } } *cluster_offset = cluster_sector << BDRV_SECTOR_BITS; @@ -1990,7 +1992,7 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset, error_report("Could not write to allocated cluster" " for streamOptimized"); return -EIO; - } else { + } else if (!zeroed) { /* allocate */ ret = get_cluster_offset(bs, extent, &m_data, offset, true, &cluster_offset, 0, 0);
m_data must be contain valid data even for zero clusters when no cluster was allocated in the image file. Without this, zero writes segfault with images that have zeroed_grain=on. For zero writes, we don't want to allocate a cluster in the image file even in compressed files. Fixes: 524089bce43fd1cd3daaca979872451efa2cf7c6 Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/vmdk.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)