On Tue, Jun 25, 2019 at 04:57:53PM +0200, Max Reitz wrote:
> On 25.06.19 16:47, Stefano Garzarella wrote:
> > On Tue, Jun 25, 2019 at 04:02:04PM +0200, Max Reitz wrote:
> >> On 09.05.19 16:59, Stefano Garzarella wrote:
> >>> RBD APIs don't allow us to write more than the size set with
> >>> rbd_create() or rbd_resize().
> >>> In order to support growing images (eg. qcow2), we resize the
> >>> image before write operations that exceed the current size.
> >>>
> >>> Signed-off-by: Stefano Garzarella <sgarz...@redhat.com>
> >>> ---
> >>> v3:
> >>> - add 'image_size' field in the BDRVRBDState to keep track of the
> >>> current size of the RBD image [Jason, Kevin]
> >>> ---
> >>> block/rbd.c | 42 +++++++++++++++++++++++++++++++++++++++---
> >>> 1 file changed, 39 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/block/rbd.c b/block/rbd.c
> >>> index 0c549c9935..b0355a2ce0 100644
> >>> --- a/block/rbd.c
> >>> +++ b/block/rbd.c
> >>
> >> [...]
> >>
> >>> @@ -833,6 +842,22 @@ static void qemu_rbd_close(BlockDriverState *bs)
> >>> rados_shutdown(s->cluster);
> >>> }
> >>>
> >>> +/* Resize the RBD image and update the 'image_size' with the current
> >>> size */
> >>> +static int qemu_rbd_resize(BlockDriverState *bs, uint64_t size)
> >>> +{
> >>> + BDRVRBDState *s = bs->opaque;
> >>> + int r;
> >>> +
> >>> + r = rbd_resize(s->image, size);
> >>> + if (r < 0) {
> >>> + return r;
> >>> + }
> >>> +
> >>> + s->image_size = size;
> >>
> >> I think this should update bs->total_sectors, too. In fact, I’m
> >> wondering why you don’t just use bs->total_sectors (or bdrv_getlength(),
> >> which returns bs->total_sectors * 512) instead of adding this new field?
> >>
> >
> > Hi Max,
> > thanks for taking a look!
> >
> > I used bs->total_sectors in the v2, but Jason pointed out a possible
> > issue with this, so I proposed to add a variable in the BDRVRBDState to
> > track the latest resize and Kevin acked [1].
> >
> > IIUC what Kevin said on his comment, the 'bs->total_sectors' should be
> > updated by bdrv_co_write_req_finish(), for this reason I didn't update
> > it.
> >
> > [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg615195.html
>
> Ah, right! Yeah, sure, now that I think about it, the block layer must
> have general code for successful writes beyond EOF... (Read: Now that
> I’m pointed towards it...)
Yeah, do you mean for example to call .bdrv_co_truncate() (or a new
callback implemented only in the driver that need to be resized like
rbd) in the bdrv_driver_pwritev() if we will write beyond EOF?
>
> OK then; thanks for the patch, applied to my block branch:
Thanks to take this!
Stefano
