Commit 3f328549 authored by Peter Maydell's avatar Peter Maydell
Browse files

Merge remote-tracking branch 'remotes/jnsnow/tags/bitmaps-pull-request' into staging 



Pull request

# gpg: Signature made Mon 29 Oct 2018 21:24:08 GMT
# gpg:                using RSA key 7DEF8106AAFC390E
# gpg: Good signature from "John Snow (John Huston) <jsnow@redhat.com>"
# Primary key fingerprint: FAEB 9711 A12C F475 812F  18F2 88A9 064D 1835 61EB
#      Subkey fingerprint: F9B7 ABDB BCAC DF95 BE76  CBD0 7DEF 8106 AAFC 390E

* remotes/jnsnow/tags/bitmaps-pull-request:
  iotests: 169: add cases for source vm resuming
  iotests: improve 169
  dirty-bitmaps: clean-up bitmaps loading and migration logic
  bitmap: Update count after a merge
  nbd: forbid use of frozen bitmaps
  block/backup: prohibit backup from using in use bitmaps
  block/dirty-bitmaps: prohibit enable/disable on locked/frozen bitmaps
  block/dirty-bitmaps: allow clear on disabled bitmaps
  block/dirty-bitmaps: fix merge permissions
  block/dirty-bitmaps: add user_locked status checker
  bloc/qcow2: drop dirty_bitmaps_loaded state variable
  block/qcow2: improve error message in qcow2_inactivate
  iotests: 169: drop deprecated 'autoload' parameter
  qapi: add transaction support for x-block-dirty-bitmap-merge
  blockdev: rename block-dirty-bitmap-clear transaction handlers
  dirty-bitmap: make it possible to restore bitmap after merge
  dirty-bitmap: rename bdrv_undo_clear_dirty_bitmap
  dirty-bitmap: switch assert-fails to errors in bdrv_merge_dirty_bitmap
  blockdev-backup: add bitmap argument

Signed-off-by: default avatarPeter Maydell <peter.maydell@linaro.org>
parents e8b38d73 3e6d88f2

block.c

+7 −4
Original line number Diff line number Diff line
@@ -4403,6 +4403,7 @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, 
    uint64_t perm, shared_perm;

    Error *local_err = NULL;

    int ret;

    BdrvDirtyBitmap *bm;



    if (!bs->drv)  {

        return;
@@ -4452,6 +4453,12 @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, 
        }

    }



    for (bm = bdrv_dirty_bitmap_next(bs, NULL); bm;

         bm = bdrv_dirty_bitmap_next(bs, bm))

    {

        bdrv_dirty_bitmap_set_migration(bm, false);

    }



    ret = refresh_total_sectors(bs, bs->total_sectors);

    if (ret < 0) {

        bs->open_flags |= BDRV_O_INACTIVE;
@@ -4566,10 +4573,6 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs, 
        }

    }



    /* At this point persistent bitmaps should be already stored by the format

     * driver */

    bdrv_release_persistent_dirty_bitmaps(bs);



    return 0;

}

block/dirty-bitmap.c

+49 −30
Original line number Diff line number Diff line
@@ -55,6 +55,10 @@ struct BdrvDirtyBitmap { 
                                   and this bitmap must remain unchanged while

                                   this flag is set. */

    bool persistent;            /* bitmap must be saved to owner disk image */

    bool migration;             /* Bitmap is selected for migration, it should

                                   not be stored on the next inactivation

                                   (persistent flag doesn't matter until next

                                   invalidation).*/

    QLIST_ENTRY(BdrvDirtyBitmap) list;

};
@@ -176,6 +180,12 @@ bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap) 
    return bitmap->successor;

}



/* Both conditions disallow user-modification via QMP. */

bool bdrv_dirty_bitmap_user_locked(BdrvDirtyBitmap *bitmap) {

    return bdrv_dirty_bitmap_frozen(bitmap) ||

           bdrv_dirty_bitmap_qmp_locked(bitmap);

}



void bdrv_dirty_bitmap_set_qmp_locked(BdrvDirtyBitmap *bitmap, bool qmp_locked)

{

    qemu_mutex_lock(bitmap->mutex);
@@ -314,7 +324,7 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs, 
        return NULL;

    }



    if (!hbitmap_merge(parent->bitmap, successor->bitmap)) {

    if (!hbitmap_merge(parent->bitmap, successor->bitmap, parent->bitmap)) {

        error_setg(errp, "Merging of parent and successor bitmap failed");

        return NULL;

    }
@@ -383,26 +393,6 @@ void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs) 
    bdrv_dirty_bitmaps_unlock(bs);

}



/**

 * Release all persistent dirty bitmaps attached to a BDS (for use in

 * bdrv_inactivate_recurse()).

 * There must not be any frozen bitmaps attached.

 * This function does not remove persistent bitmaps from the storage.

 * Called with BQL taken.

 */

void bdrv_release_persistent_dirty_bitmaps(BlockDriverState *bs)

{

    BdrvDirtyBitmap *bm, *next;



    bdrv_dirty_bitmaps_lock(bs);

    QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {

        if (bdrv_dirty_bitmap_get_persistance(bm)) {

            bdrv_release_dirty_bitmap_locked(bm);

        }

    }

    bdrv_dirty_bitmaps_unlock(bs);

}



/**

 * Remove persistent dirty bitmap from the storage if it exists.

 * Absence of bitmap is not an error, because we have the following scenario:
@@ -619,7 +609,6 @@ void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap, 


void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)

{

    assert(bdrv_dirty_bitmap_enabled(bitmap));

    assert(!bdrv_dirty_bitmap_readonly(bitmap));

    bdrv_dirty_bitmap_lock(bitmap);

    if (!out) {
@@ -633,12 +622,12 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out) 
    bdrv_dirty_bitmap_unlock(bitmap);

}



void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in)

void bdrv_restore_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *backup)

{

    HBitmap *tmp = bitmap->bitmap;

    assert(bdrv_dirty_bitmap_enabled(bitmap));

    assert(!bdrv_dirty_bitmap_readonly(bitmap));

    bitmap->bitmap = in;

    bitmap->bitmap = backup;

    hbitmap_free(tmp);

}
@@ -756,16 +745,24 @@ void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap, bool persistent) 
    qemu_mutex_unlock(bitmap->mutex);

}



/* Called with BQL taken. */

void bdrv_dirty_bitmap_set_migration(BdrvDirtyBitmap *bitmap, bool migration)

{

    qemu_mutex_lock(bitmap->mutex);

    bitmap->migration = migration;

    qemu_mutex_unlock(bitmap->mutex);

}



bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap)

{

    return bitmap->persistent;

    return bitmap->persistent && !bitmap->migration;

}



bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs)

{

    BdrvDirtyBitmap *bm;

    QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {

        if (bm->persistent && !bm->readonly) {

        if (bm->persistent && !bm->readonly && !bm->migration) {

            return true;

        }

    }
@@ -791,19 +788,41 @@ int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t offset) 
}



void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,

                             Error **errp)

                             HBitmap **backup, Error **errp)

{

    bool ret;



    /* only bitmaps from one bds are supported */

    assert(dest->mutex == src->mutex);



    qemu_mutex_lock(dest->mutex);



    assert(bdrv_dirty_bitmap_enabled(dest));

    assert(!bdrv_dirty_bitmap_readonly(dest));

    if (bdrv_dirty_bitmap_user_locked(dest)) {

        error_setg(errp, "Bitmap '%s' is currently in use by another"

        " operation and cannot be modified", dest->name);

        goto out;

    }



    if (bdrv_dirty_bitmap_readonly(dest)) {

        error_setg(errp, "Bitmap '%s' is readonly and cannot be modified",

                   dest->name);

        goto out;

    }



    if (!hbitmap_merge(dest->bitmap, src->bitmap)) {

    if (!hbitmap_can_merge(dest->bitmap, src->bitmap)) {

        error_setg(errp, "Bitmaps are incompatible and can't be merged");

        goto out;

    }



    if (backup) {

        *backup = dest->bitmap;

        dest->bitmap = hbitmap_alloc(dest->size, hbitmap_granularity(*backup));

        ret = hbitmap_merge(*backup, src->bitmap, dest->bitmap);

    } else {

        ret = hbitmap_merge(dest->bitmap, src->bitmap, dest->bitmap);

    }

    assert(ret);



out:

    qemu_mutex_unlock(dest->mutex);

}

block/qcow2-bitmap.c

+16 −0
Original line number Diff line number Diff line
@@ -1418,6 +1418,22 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) 
        g_free(tb);

    }



    QSIMPLEQ_FOREACH(bm, bm_list, entry) {

        /* For safety, we remove bitmap after storing.

         * We may be here in two cases:

         * 1. bdrv_close. It's ok to drop bitmap.

         * 2. inactivation. It means migration without 'dirty-bitmaps'

         *    capability, so bitmaps are not marked with

         *    BdrvDirtyBitmap.migration flags. It's not bad to drop them too,

         *    and reload on invalidation.

         */

        if (bm->dirty_bitmap == NULL) {

            continue;

        }



        bdrv_release_dirty_bitmap(bs, bm->dirty_bitmap);

    }



    bitmap_list_free(bm_list);

    return;

block/qcow2.c

+66 −20
Original line number Diff line number Diff line
@@ -1153,7 +1153,6 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, 
    uint64_t ext_end;

    uint64_t l1_vm_state_index;

    bool update_header = false;

    bool header_updated = false;



    ret = bdrv_pread(bs->file, 0, &header, sizeof(header));

    if (ret < 0) {
@@ -1492,23 +1491,70 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, 
        s->autoclear_features &= QCOW2_AUTOCLEAR_MASK;

    }



    if (s->dirty_bitmaps_loaded) {

        /* It's some kind of reopen. There are no known cases where we need to

         * reload bitmaps in such a situation, so it's safer to skip them.

    /* == Handle persistent dirty bitmaps ==

     *

         * Moreover, if we have some readonly bitmaps and we are reopening for

         * rw we should reopen bitmaps correspondingly.

     * We want load dirty bitmaps in three cases:

     *

     * 1. Normal open of the disk in active mode, not related to invalidation

     *    after migration.

     *

     * 2. Invalidation of the target vm after pre-copy phase of migration, if

     *    bitmaps are _not_ migrating through migration channel, i.e.

     *    'dirty-bitmaps' capability is disabled.

     *

     * 3. Invalidation of source vm after failed or canceled migration.

     *    This is a very interesting case. There are two possible types of

     *    bitmaps:

     *

     *    A. Stored on inactivation and removed. They should be loaded from the

     *       image.

     *

     *    B. Not stored: not-persistent bitmaps and bitmaps, migrated through

     *       the migration channel (with dirty-bitmaps capability).

     *

     *    On the other hand, there are two possible sub-cases:

     *

     *    3.1 disk was changed by somebody else while were inactive. In this

     *        case all in-RAM dirty bitmaps (both persistent and not) are

     *        definitely invalid. And we don't have any method to determine

     *        this.

     *

     *        Simple and safe thing is to just drop all the bitmaps of type B on

     *        inactivation. But in this case we lose bitmaps in valid 4.2 case.

     *

     *        On the other hand, resuming source vm, if disk was already changed

     *        is a bad thing anyway: not only bitmaps, the whole vm state is

     *        out of sync with disk.

     *

     *        This means, that user or management tool, who for some reason

     *        decided to resume source vm, after disk was already changed by

     *        target vm, should at least drop all dirty bitmaps by hand.

     *

     *        So, we can ignore this case for now, but TODO: "generation"

     *        extension for qcow2, to determine, that image was changed after

     *        last inactivation. And if it is changed, we will drop (or at least

     *        mark as 'invalid' all the bitmaps of type B, both persistent

     *        and not).

     *

     *    3.2 disk was _not_ changed while were inactive. Bitmaps may be saved

     *        to disk ('dirty-bitmaps' capability disabled), or not saved

     *        ('dirty-bitmaps' capability enabled), but we don't need to care

     *        of: let's load bitmaps as always: stored bitmaps will be loaded,

     *        and not stored has flag IN_USE=1 in the image and will be skipped

     *        on loading.

     *

     * One remaining possible case when we don't want load bitmaps:

     *

     * 4. Open disk in inactive mode in target vm (bitmaps are migrating or

     *    will be loaded on invalidation, no needs try loading them before)

     */

        if (bdrv_has_readonly_bitmaps(bs) &&

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

        {

            qcow2_reopen_bitmaps_rw_hint(bs, &header_updated, &local_err);

        }

    } else {

        header_updated = qcow2_load_dirty_bitmaps(bs, &local_err);

        s->dirty_bitmaps_loaded = true;

    }



    if (!(bdrv_get_flags(bs) & BDRV_O_INACTIVE)) {

        /* It's case 1, 2 or 3.2. Or 3.1 which is BUG in management layer. */

        bool header_updated = qcow2_load_dirty_bitmaps(bs, &local_err);



        update_header = update_header && !header_updated;

    }

    if (local_err != NULL) {

        error_propagate(errp, local_err);

        ret = -EINVAL;
@@ -2123,8 +2169,8 @@ static int qcow2_inactivate(BlockDriverState *bs) 
    qcow2_store_persistent_dirty_bitmaps(bs, &local_err);

    if (local_err != NULL) {

        result = -EINVAL;

        error_report_err(local_err);

        error_report("Persistent bitmaps are lost for node '%s'",

        error_reportf_err(local_err, "Lost persistent bitmaps during "

                          "inactivation of node '%s': ",

                          bdrv_get_device_or_node_name(bs));

    }

block/qcow2.h

+0 −1
Original line number Diff line number Diff line
@@ -300,7 +300,6 @@ typedef struct BDRVQcow2State { 
    uint32_t nb_bitmaps;

    uint64_t bitmap_directory_size;

    uint64_t bitmap_directory_offset;

    bool dirty_bitmaps_loaded;



    int flags;

    int qcow_version;