Commit f8aa905a authored by Jeff Cody's avatar Jeff Cody Committed by Max Reitz
Browse files

block: set device_list.tqe_prev to NULL on BDS removal 



This fixes a regression introduced with commit 3f09bfbc.  Multiple
bugs arise in conjunction with live snapshots and mirroring operations
(which include active layer commit).

After a live snapshot occurs, the active layer and the base layer both
have a non-NULL tqe_prev field in the device_list, although the base
node's tqe_prev field points to a NULL entry.  This non-NULL tqe_prev
field occurs after the bdrv_append() in the external snapshot calls
change_parent_backing_link().

In change_parent_backing_link(), when the previous active layer is
removed from device_list, the device_list.tqe_prev pointer is not
set to NULL.

The operating scheme in the block layer is to indicate that a BDS belongs
in the bdrv_states device_list iff the device_list.tqe_prev pointer
is non-NULL.

This patch does two things:

1.) Introduces a new block layer helper bdrv_device_remove() to remove a
    BDS from the device_list, and
2.) uses that new API, which also fixes the regression once used in
    change_parent_backing_link().

Signed-off-by: default avatarJeff Cody <jcody@redhat.com>
Message-id: 0cd51e11c0666c04ddb7c05293fe94afeb551e89.1454376655.git.jcody@redhat.com
Reviewed-by: default avatarMax Reitz <mreitz@redhat.com>
Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
parent c7fc50d3

block.c

+14 −10
Original line number Diff line number Diff line
@@ -2242,21 +2242,25 @@ void bdrv_close_all(void) 
    }

}



/* Note that bs->device_list.tqe_prev is initially null,

 * and gets set to non-null by QTAILQ_INSERT_TAIL().  Establish

 * the useful invariant "bs in bdrv_states iff bs->tqe_prev" by

 * resetting it to null on remove.  */

void bdrv_device_remove(BlockDriverState *bs)

{

    QTAILQ_REMOVE(&bdrv_states, bs, device_list);

    bs->device_list.tqe_prev = NULL;

}



/* make a BlockDriverState anonymous by removing from bdrv_state and

 * graph_bdrv_state list.

   Also, NULL terminate the device_name to prevent double remove */

void bdrv_make_anon(BlockDriverState *bs)

{

    /*

     * Take care to remove bs from bdrv_states only when it's actually

     * in it.  Note that bs->device_list.tqe_prev is initially null,

     * and gets set to non-null by QTAILQ_INSERT_TAIL().  Establish

     * the useful invariant "bs in bdrv_states iff bs->tqe_prev" by

     * resetting it to null on remove.

     */

    /* Take care to remove bs from bdrv_states only when it's actually

     * in it. */

    if (bs->device_list.tqe_prev) {

        QTAILQ_REMOVE(&bdrv_states, bs, device_list);

        bs->device_list.tqe_prev = NULL;

        bdrv_device_remove(bs);

    }

    if (bs->node_name[0] != '\0') {

        QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list);
@@ -2297,7 +2301,7 @@ static void change_parent_backing_link(BlockDriverState *from, 
        if (!to->device_list.tqe_prev) {

            QTAILQ_INSERT_BEFORE(from, to, device_list);

        }

        QTAILQ_REMOVE(&bdrv_states, from, device_list);

        bdrv_device_remove(from);

    }

}

blockdev.c

+1 −2
Original line number Diff line number Diff line
@@ -2408,8 +2408,7 @@ void qmp_x_blockdev_remove_medium(const char *device, Error **errp) 


    /* This follows the convention established by bdrv_make_anon() */

    if (bs->device_list.tqe_prev) {

        QTAILQ_REMOVE(&bdrv_states, bs, device_list);

        bs->device_list.tqe_prev = NULL;

        bdrv_device_remove(bs);

    }



    blk_remove_bs(blk);

include/block/block.h

+1 −0
Original line number Diff line number Diff line
@@ -199,6 +199,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, 
int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);

BlockDriverState *bdrv_new_root(void);

BlockDriverState *bdrv_new(void);

void bdrv_device_remove(BlockDriverState *bs);

void bdrv_make_anon(BlockDriverState *bs);

void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);

void bdrv_replace_in_backing_chain(BlockDriverState *old,