On 30.01.2016 06:17, Jeff Cody wrote:
> In change_parent_backing_link(), we only inserted the new
> BlockDriverState entry into the device_list if the tqe_prev pointer was
> NULL. However, we must also allow insertion when the BDS pointed
> to by the tqe_prev pointer is NULL as well.
>
> This fixes a bug with external snapshots, and live active layer commits.
>
> 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.
>
> Once the active commit is finished, bdrv_replace_in_backing_chain() is
> called to set the base node as the new active node, and remove the
> node that was the prior active layer from the device_list.
>
> If we only check against the tqe_prev pointer field and not the entity
> it is pointing to, then we fail to insert base image into the device
> list. The previous active layer is still removed from the device_list,
> leaving an empty device_list queue.
>
> With an empty device_list queue, odd behavior occurs - such as not
> allowing any more live snapshots.
>
> This commit fixes this issue, by checking for a NULL tqe_prev entity
> in the devices_list.
>
> Signed-off-by: Jeff Cody <jc...@redhat.com>
> ---
> block.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block.c b/block.c
> index 5709d3d..0b8526b 100644
> --- a/block.c
> +++ b/block.c
> @@ -2272,7 +2272,7 @@ static void change_parent_backing_link(BlockDriverState
> *from,
> }
> if (from->blk) {
> blk_set_bs(from->blk, to);
> - if (!to->device_list.tqe_prev) {
> + if (!to->device_list.tqe_prev || !*to->device_list.tqe_prev) {I'm not sure this is the right fix; bdrv_make_anon() clearly states that we do want device_list.tqe_prev to be NULL if and only if the BDS is not part of the device list. So this should not be happening. > QTAILQ_INSERT_BEFORE(from, to, device_list); > } > QTAILQ_REMOVE(&bdrv_states, from, device_list); Inserting a from->device_list.tqe_prev = NULL; here makes the iotest happy and looks like the better fix to me. Max >
signature.asc
Description: OpenPGP digital signature
