Am 30.07.2018 um 13:41 hat Leonid Bloch geschrieben: > On 07/30/2018 12:43 PM, Kevin Wolf wrote: > > Am 29.07.2018 um 23:27 hat Leonid Bloch geschrieben: > > > Signed-off-by: Leonid Bloch <lbl...@janustech.com> > > > --- > > > block/qcow2.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/block/qcow2.c b/block/qcow2.c > > > index ec9e6238a0..223d351e40 100644 > > > --- a/block/qcow2.c > > > +++ b/block/qcow2.c > > > @@ -3646,6 +3646,8 @@ static int coroutine_fn > > > qcow2_co_truncate(BlockDriverState *bs, int64_t offset, > > > } > > > } > > > + bs->total_sectors = offset / 512; > > > + > > > /* write updated header.size */ > > > offset = cpu_to_be64(offset); > > > ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, size), > > > > This shouldn't be necessary, bdrv_co_truncate() already updates > > bs->total_sectors after calling the block driver. > > It looks like without it the cache resize works only on the second resize.
Yes, and after reading the rest of the series, it makes sense to me because qcow2_update_option() -> read_cache_sizes() uses bs->total_sectors, so if we call that in qcow2_co_truncate(), we already need to update the value early. The comment should explain this connection because otherwise it's not obvious when you read the code. > > If this is needed by one of the following patches, we need a comment > > that explains why this seemingly superfluous assignment is actually > > necessary. > > > > Also, 512 should be BDRV_SECTOR_SIZE. > > I was surprised that it's not, but it's 512 also in two other places, > including in qcow2_co_truncate itself. So I decided to keep that. Probably > would be better if I'd repair it in the other places instead. :) Yeah, or at least not introduce new places with a literal 512. Kevin
