On Thu, Jan 23, 2014 at 11:05:26PM +0100, Benoît Canet wrote:
> Le Thursday 23 Jan 2014 à 16:48:56 (-0500), Jeff Cody a écrit :
> > If the top image to commit is the active layer, and also larger than
> > the base image, then an I/O error will likely be returned during
> > block-commit.
> >
> > For instance, if we have a base image with a virtual size 10G, and a
> > active layer image of size 20G, then committing the snapshot via
> > 'block-commit' will likely fail.
> >
> > This will automatically attempt to resize the base image, if the
> > active layer image to be committed is larger.
> >
> > Signed-off-by: Jeff Cody <jc...@redhat.com>
> > Reviewed-by: Eric Blake <ebl...@redhat.com>
> > ---
> > block/mirror.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 41 insertions(+)
> >
> > diff --git a/block/mirror.c b/block/mirror.c
> > index 2932bab..2a33aa6 100644
> > --- a/block/mirror.c
> > +++ b/block/mirror.c
> > @@ -630,11 +630,52 @@ void commit_active_start(BlockDriverState *bs,
> > BlockDriverState *base,
> > BlockDriverCompletionFunc *cb,
> > void *opaque, Error **errp)
> > {
> > + int64_t length, base_length;
> > + int orig_base_flags;
> > +
> > + orig_base_flags = bdrv_get_flags(base);
> > +
> > if (bdrv_reopen(base, bs->open_flags, errp)) {
> > return;
> > }
> > +
> > + length = bdrv_getlength(bs);
> > + if (length < 0) {
> > + error_setg(errp, "Unable to determine length of %s", bs->filename);
> > + goto error_restore_flags;
> > + }
> > +
> > + base_length = bdrv_getlength(base);
> > + if (base_length < 0) {
> > + error_setg(errp, "Unable to determine length of %s",
> > base->filename);
> > + goto error_restore_flags;
> > + }
> > +
> > + if (length > base_length) {
> > + if (bdrv_truncate(base, length) < 0) {
> > + error_setg(errp, "Top image %s is larger than base image %s,
> > and "
> > + "resize of base image failed",
> > + bs->filename, base->filename);
> > + goto error_restore_flags;
> > + }
>
>
> > + } else if (length < 0) {
> > + goto error_restore_flags;
> > + }
> The code already test for (length < 0) ten lines above. Is this branch really
> needed ?
>
> Best regards
>
> Benoît
Thanks - no, it is not needed. I did not mean to have that there -
while harmless, it is essentially dead code and should be culled.
I'll submit a v4 to take care of that.
>
> > +
> > +
> > bdrv_ref(base);
> > mirror_start_job(bs, base, speed, 0, 0,
> > on_error, on_error, cb, opaque, errp,
> > &commit_active_job_driver, false, base);
> > + if (error_is_set(errp)) {
> > + goto error_restore_flags;
> > + }
> > +
> > + return;
> > +
> > +error_restore_flags:
> > + /* ignore error and errp for bdrv_reopen, because we want to propagate
> > + * the original error */
> > + bdrv_reopen(base, orig_base_flags, NULL);
> > + return;
> > }
> > --
> > 1.8.3.1
> >
> >
