Commit 4dd09f62 authored by Vladimir Sementsov-Ogievskiy's avatar Vladimir Sementsov-Ogievskiy Committed by John Snow
Browse files

qcow2-bitmap: move bitmap reopen-rw code to qcow2_reopen_commit 



The only reason I can imagine for this strange code at the very-end of
bdrv_reopen_commit is the fact that bs->read_only updated after
calling drv->bdrv_reopen_commit in bdrv_reopen_commit. And in the same
time, prior to previous commit, qcow2_reopen_bitmaps_rw did a wrong
check for being writable, when actually it only need writable file
child not self.

So, as it's fixed, let's move things to correct place.

Signed-off-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: default avatarJohn Snow <jsnow@redhat.com>
Acked-by: default avatarMax Reitz <mreitz@redhat.com>
Message-id: 20190927122355.7344-10-vsementsov@virtuozzo.com
Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
parent f6333cbf

block.c

+0 −19
Original line number Diff line number Diff line
@@ -3935,16 +3935,12 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state) 
    BlockDriver *drv;

    BlockDriverState *bs;

    BdrvChild *child;

    bool old_can_write, new_can_write;



    assert(reopen_state != NULL);

    bs = reopen_state->bs;

    drv = bs->drv;

    assert(drv != NULL);



    old_can_write =

        !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE);



    /* If there are any driver level actions to take */

    if (drv->bdrv_reopen_commit) {

        drv->bdrv_reopen_commit(reopen_state);
@@ -3988,21 +3984,6 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state) 
    }



    bdrv_refresh_limits(bs, NULL);



    new_can_write =

        !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE);

    if (!old_can_write && new_can_write && drv->bdrv_reopen_bitmaps_rw) {

        Error *local_err = NULL;

        if (drv->bdrv_reopen_bitmaps_rw(bs, &local_err) < 0) {

            /* This is not fatal, bitmaps just left read-only, so all following

             * writes will fail. User can remove read-only bitmaps to unblock

             * writes.

             */

            error_reportf_err(local_err,

                              "%s: Failed to make dirty bitmaps writable: ",

                              bdrv_get_node_name(bs));

        }

    }

}



/*

block/qcow2.c

+14 −1
Original line number Diff line number Diff line
@@ -1835,6 +1835,20 @@ fail: 
static void qcow2_reopen_commit(BDRVReopenState *state)

{

    qcow2_update_options_commit(state->bs, state->opaque);

    if (state->flags & BDRV_O_RDWR) {

        Error *local_err = NULL;



        if (qcow2_reopen_bitmaps_rw(state->bs, &local_err) < 0) {

            /*

             * This is not fatal, bitmaps just left read-only, so all following

             * writes will fail. User can remove read-only bitmaps to unblock

             * writes or retry reopen.

             */

            error_reportf_err(local_err,

                              "%s: Failed to make dirty bitmaps writable: ",

                              bdrv_get_node_name(state->bs));

        }

    }

    g_free(state->opaque);

}
@@ -5406,7 +5420,6 @@ BlockDriver bdrv_qcow2 = { 
    .bdrv_detach_aio_context  = qcow2_detach_aio_context,

    .bdrv_attach_aio_context  = qcow2_attach_aio_context,



    .bdrv_reopen_bitmaps_rw = qcow2_reopen_bitmaps_rw,

    .bdrv_co_can_store_new_dirty_bitmap = qcow2_co_can_store_new_dirty_bitmap,

    .bdrv_co_remove_persistent_dirty_bitmap =

            qcow2_co_remove_persistent_dirty_bitmap,

include/block/block_int.h

+0 −6
Original line number Diff line number Diff line
@@ -547,12 +547,6 @@ struct BlockDriver { 
                             uint64_t parent_perm, uint64_t parent_shared,

                             uint64_t *nperm, uint64_t *nshared);



    /**

     * Bitmaps should be marked as 'IN_USE' in the image on reopening image

     * as rw. This handler should realize it. It also should unset readonly

     * field of BlockDirtyBitmap's in case of success.

     */

    int (*bdrv_reopen_bitmaps_rw)(BlockDriverState *bs, Error **errp);

    bool (*bdrv_co_can_store_new_dirty_bitmap)(BlockDriverState *bs,

                                               const char *name,

                                               uint32_t granularity,