| Message ID | 1491057878-27868-6-git-send-email-ashijeetacharya@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, 04/01 20:14, Ashijeet Acharya wrote: > Set the maximum bytes allowed to get allocated at once to be not more > than the extent size boundary to handle writes at two separate extents > appropriately. > > Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> > --- > block/vmdk.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/block/vmdk.c b/block/vmdk.c > index a8babd7..9456ddd 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -1767,6 +1767,7 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset, > int64_t offset_in_cluster, n_bytes; > uint64_t cluster_offset; > uint64_t bytes_done = 0; > + uint64_t extent_size; > VmdkMetaData m_data; > uint32_t total_alloc_clusters = 0; > > @@ -1782,9 +1783,17 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset, > if (!extent) { > return -EIO; > } > + extent_size = extent->end_sector * BDRV_SECTOR_SIZE; Maybe extent_end to be more accurate? > + > offset_in_cluster = vmdk_find_offset_in_cluster(extent, offset); > - n_bytes = MIN(bytes, extent->cluster_sectors * BDRV_SECTOR_SIZE > - - offset_in_cluster); > + > + /* truncate n_bytes to first cluster because we need to perform COW */ Makes sense, but shouldn't this be squashed into patch patch 3? Because it looks like it is fixing an intermediate bug. > + if (offset_in_cluster > 0) { > + n_bytes = MIN(bytes, extent->cluster_sectors * BDRV_SECTOR_SIZE > + - offset_in_cluster); > + } else { > + n_bytes = MIN(bytes, extent_size - offset); > + } > > ret = vmdk_alloc_cluster_offset(bs, extent, &m_data, offset, > !(extent->compressed || zeroed), > -- > 2.6.2 > Fam
On Wed, Apr 19, 2017 at 6:30 PM, Fam Zheng <famz@redhat.com> wrote: > On Sat, 04/01 20:14, Ashijeet Acharya wrote: >> Set the maximum bytes allowed to get allocated at once to be not more >> than the extent size boundary to handle writes at two separate extents >> appropriately. >> >> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> >> --- >> block/vmdk.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/block/vmdk.c b/block/vmdk.c >> index a8babd7..9456ddd 100644 >> --- a/block/vmdk.c >> +++ b/block/vmdk.c >> @@ -1767,6 +1767,7 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset, >> int64_t offset_in_cluster, n_bytes; >> uint64_t cluster_offset; >> uint64_t bytes_done = 0; >> + uint64_t extent_size; >> VmdkMetaData m_data; >> uint32_t total_alloc_clusters = 0; >> >> @@ -1782,9 +1783,17 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset, >> if (!extent) { >> return -EIO; >> } >> + extent_size = extent->end_sector * BDRV_SECTOR_SIZE; > > Maybe extent_end to be more accurate? Done >> + >> offset_in_cluster = vmdk_find_offset_in_cluster(extent, offset); >> - n_bytes = MIN(bytes, extent->cluster_sectors * BDRV_SECTOR_SIZE >> - - offset_in_cluster); >> + >> + /* truncate n_bytes to first cluster because we need to perform COW */ > > Makes sense, but shouldn't this be squashed into patch patch 3? Because it looks > like it is fixing an intermediate bug. Did you mean that I should merge this whole patch into patch 3? Maybe moving it before patch 3 rather than squashing it make more sense? Ashijeet
On Fri, Apr 21, 2017 at 8:23 PM, Ashijeet Acharya <ashijeetacharya@gmail.com> wrote: > On Wed, Apr 19, 2017 at 6:30 PM, Fam Zheng <famz@redhat.com> wrote: >> On Sat, 04/01 20:14, Ashijeet Acharya wrote: >>> Set the maximum bytes allowed to get allocated at once to be not more >>> than the extent size boundary to handle writes at two separate extents >>> appropriately. >>> >>> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> >>> --- >>> block/vmdk.c | 13 +++++++++++-- >>> 1 file changed, 11 insertions(+), 2 deletions(-) >>> >>> diff --git a/block/vmdk.c b/block/vmdk.c >>> index a8babd7..9456ddd 100644 >>> --- a/block/vmdk.c >>> +++ b/block/vmdk.c >>> @@ -1767,6 +1767,7 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset, >>> int64_t offset_in_cluster, n_bytes; >>> uint64_t cluster_offset; >>> uint64_t bytes_done = 0; >>> + uint64_t extent_size; >>> VmdkMetaData m_data; >>> uint32_t total_alloc_clusters = 0; >>> >>> @@ -1782,9 +1783,17 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset, >>> if (!extent) { >>> return -EIO; >>> } >>> + extent_size = extent->end_sector * BDRV_SECTOR_SIZE; >> >> Maybe extent_end to be more accurate? > > Done > >>> + >>> offset_in_cluster = vmdk_find_offset_in_cluster(extent, offset); >>> - n_bytes = MIN(bytes, extent->cluster_sectors * BDRV_SECTOR_SIZE >>> - - offset_in_cluster); >>> + >>> + /* truncate n_bytes to first cluster because we need to perform COW */ >> >> Makes sense, but shouldn't this be squashed into patch patch 3? Because it looks >> like it is fixing an intermediate bug. > > Did you mean that I should merge this whole patch into patch 3? Maybe > moving it before patch 3 rather than squashing it make more sense? Instead I have moved it before patch 3 in v4 Ashijeet
diff --git a/block/vmdk.c b/block/vmdk.c index a8babd7..9456ddd 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1767,6 +1767,7 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset, int64_t offset_in_cluster, n_bytes; uint64_t cluster_offset; uint64_t bytes_done = 0; + uint64_t extent_size; VmdkMetaData m_data; uint32_t total_alloc_clusters = 0; @@ -1782,9 +1783,17 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset, if (!extent) { return -EIO; } + extent_size = extent->end_sector * BDRV_SECTOR_SIZE; + offset_in_cluster = vmdk_find_offset_in_cluster(extent, offset); - n_bytes = MIN(bytes, extent->cluster_sectors * BDRV_SECTOR_SIZE - - offset_in_cluster); + + /* truncate n_bytes to first cluster because we need to perform COW */ + if (offset_in_cluster > 0) { + n_bytes = MIN(bytes, extent->cluster_sectors * BDRV_SECTOR_SIZE + - offset_in_cluster); + } else { + n_bytes = MIN(bytes, extent_size - offset); + } ret = vmdk_alloc_cluster_offset(bs, extent, &m_data, offset, !(extent->compressed || zeroed),
Set the maximum bytes allowed to get allocated at once to be not more than the extent size boundary to handle writes at two separate extents appropriately. Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> --- block/vmdk.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)