summaryrefslogtreecommitdiffstats
path: root/migration/block-dirty-bitmap.c
Commit message (Collapse)AuthorAgeFilesLines
* migration: block-dirty-bitmap: add missing qemu_mutex_lock_iothreadEmanuele Giuseppe Esposito2021-10-051-1/+4
| | | | | | | | | | | | | | init_dirty_bitmap_migration assumes the iothread lock (BQL) to be held, but instead it isn't. Instead of adding the lock to qemu_savevm_state_setup(), follow the same pattern as the other ->save_setup callbacks and lock+unlock inside dirty_bitmap_save_setup(). Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20211005080751.3797161-2-eesposit@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* migration/block-dirty-bitmap: make incoming disabled bitmaps busyVladimir Sementsov-Ogievskiy2021-03-241-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Incoming enabled bitmaps are busy, because we do bdrv_dirty_bitmap_create_successor() for them. But disabled bitmaps being migrated are not marked busy, and user can remove them during the incoming migration. Then we may crash in cancel_incoming_locked() when try to remove the bitmap that was already removed by user, like this: #0 qemu_mutex_lock_impl (mutex=0x5593d88c50d1, file=0x559680554b20 "../block/dirty-bitmap.c", line=64) at ../util/qemu-thread-posix.c:77 #1 bdrv_dirty_bitmaps_lock (bs=0x5593d88c0ee9) at ../block/dirty-bitmap.c:64 #2 bdrv_release_dirty_bitmap (bitmap=0x5596810e9570) at ../block/dirty-bitmap.c:362 #3 cancel_incoming_locked (s=0x559680be8208 <dbm_state+40>) at ../migration/block-dirty-bitmap.c:918 #4 dirty_bitmap_load (f=0x559681d02b10, opaque=0x559680be81e0 <dbm_state>, version_id=1) at ../migration/block-dirty-bitmap.c:1194 #5 vmstate_load (f=0x559681d02b10, se=0x559680fb5810) at ../migration/savevm.c:908 #6 qemu_loadvm_section_part_end (f=0x559681d02b10, mis=0x559680fb4a30) at ../migration/savevm.c:2473 #7 qemu_loadvm_state_main (f=0x559681d02b10, mis=0x559680fb4a30) at ../migration/savevm.c:2626 #8 postcopy_ram_listen_thread (opaque=0x0) at ../migration/savevm.c:1871 #9 qemu_thread_start (args=0x5596817ccd10) at ../util/qemu-thread-posix.c:521 #10 start_thread () at /lib64/libpthread.so.0 #11 clone () at /lib64/libc.so.6 Note bs pointer taken from bitmap: it's definitely bad aligned. That's because we are in use after free, bitmap is already freed. So, let's make disabled bitmaps (being migrated) busy during incoming migration. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20210322094906.5079-2-vsementsov@virtuozzo.com>
* migration: dirty-bitmap: Allow control of bitmap persistencePeter Krempa2021-02-121-3/+26
| | | | | | | | | | | | | | | | | | | Bitmap's source persistence is transported over the migration stream and the destination mirrors it. In some cases the destination might want to persist bitmaps which are not persistent on the source (e.g. the result of merging bitmaps from a number of layers on the source when migrating into a squashed image) but currently it would need to create another set of persistent bitmaps and merge them. This patch adds a 'transform' property to the alias map which allows overriding the persistence of migrated bitmaps both on the source and destination sides. Signed-off-by: Peter Krempa <pkrempa@redhat.com> Message-Id: <b20afb675917b86f6359ac3591166ac6d4233573.1613150869.git.pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: grammar tweaks, drop dead conditional] Signed-off-by: Eric Blake <eblake@redhat.com>
* migration: dirty-bitmap: Use struct for alias map inner membersPeter Krempa2021-02-121-12/+21
| | | | | | | | | | | | | Currently the alias mapping hash stores just strings of the target objects internally. In further patches we'll be adding another member which will need to be stored in the map so pass a copy of the whole BitmapMigrationBitmapAlias QAPI struct into the map. Signed-off-by: Peter Krempa <pkrempa@redhat.com> Message-Id: <fc5f27e1fe16cb75e08a248c2d938de3997b9bfb.1613150869.git.pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: adjust long lines] Signed-off-by: Eric Blake <eblake@redhat.com>
* migration/block-dirty-bitmap: fix uninitialized variable warningChen Qun2020-10-261-12/+9Star
| | | | | | | | | | | | | | | | | | A default value is provided for the variable 'bitmap_name' to avoid a compiler warning. The compiler showed the warning: migration/block-dirty-bitmap.c:1090:13: warning: ‘bitmap_name’ may be used uninitialized in this function [-Wmaybe-uninitialized] g_strlcpy(s->bitmap_name, bitmap_name, sizeof(s->bitmap_name)); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> Message-Id: <20201014114430.1898684-1-kuhn.chenqun@huawei.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> [eblake: commit message grammar tweaks] Signed-off-by: Eric Blake <eblake@redhat.com>
* migration/block-dirty-bitmap: fix larger granularity bitmapsStefan Reiter2020-10-261-2/+3
| | | | | | | | | | | | | | | | sectors_per_chunk is a 64 bit integer, but the calculation is done in 32 bits, leading to an overflow for coarse bitmap granularities. If that results in the value 0, it leads to a hang where no progress is made but send_bitmap_bits is constantly called with nr_sectors being 0. Signed-off-by: Stefan Reiter <s.reiter@proxmox.com> Message-Id: <20201021144456.1072-1-s.reiter@proxmox.com> Fixes: b35ebdf07 migration: add postcopy migration of dirty bitmaps Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: Use correct type for 8ULL, use () to avoid overflow] Signed-off-by: Eric Blake <eblake@redhat.com>
* block: Use bdrv_filter_(bs|child) where obviousMax Reitz2020-09-071-7/+1Star
| | | | | | | | | | | | | | | | | | | | | Places that use patterns like if (bs->drv->is_filter && bs->file) { ... something about bs->file->bs ... } should be BlockDriverState *filtered = bdrv_filter_bs(bs); if (filtered) { ... something about @filtered ... } instead. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
* migration: Add block-bitmap-mapping parameterMax Reitz2020-08-211-54/+358
| | | | | | | | | | | | | | | | | | | This migration parameter allows mapping block node names and bitmap names to aliases for the purpose of block dirty bitmap migration. This way, management tools can use different node and bitmap names on the source and destination and pass the mapping of how bitmaps are to be transferred to qemu (on the source, the destination, or even both with arbitrary aliases in the migration stream). While touching this code, fix a bug where bitmap names longer than 255 bytes would fail an assertion in qemu_put_counted_string(). Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Message-Id: <20200820150725.68687-2-mreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* migration: Fix typos in bitmap migration commentsEric Blake2020-07-271-2/+2
| | | | | | | | Noticed while reviewing the file for newer patches. Fixes: b35ebdf076 Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20200727203206.134996-1-eblake@redhat.com>
* migration/block-dirty-bitmap: cancel migration on shutdownVladimir Sementsov-Ogievskiy2020-07-271-0/+16
| | | | | | | | | | | | | | | | | If target is turned off prior to postcopy finished, target crashes because busy bitmaps are found at shutdown. Canceling incoming migration helps, as it removes all unfinished (and therefore busy) bitmaps. Similarly on source we crash in bdrv_close_all which asserts that all bdrv states are removed, because bdrv states involved into dirty bitmap migration are referenced by it. So, we need to cancel outgoing migration as well. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200727194236.19551-17-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* migration/block-dirty-bitmap: relax error handling in incoming partVladimir Sementsov-Ogievskiy2020-07-271-37/+127
| | | | | | | | | | | | | | | | | | | | Bitmaps data is not critical, and we should not fail the migration (or use postcopy recovering) because of dirty-bitmaps migration failure. Instead we should just lose unfinished bitmaps. Still we have to report io stream violation errors, as they affect the whole migration stream. While touching this, tighten code that was previously blindly calling malloc on a size read from the migration stream, as a corrupted stream (perhaps from a malicious user) should not be able to convince us to allocate an inordinate amount of memory. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200727194236.19551-16-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: typo fixes, enhance commit message] Signed-off-by: Eric Blake <eblake@redhat.com>
* migration/block-dirty-bitmap: keep bitmap state for all bitmapsVladimir Sementsov-Ogievskiy2020-07-271-21/+43
| | | | | | | | | | | | | | | | | | | Keep bitmap state for disabled bitmaps too. Keep the state until the end of the process. It's needed for the following commit to implement bitmap postcopy canceling. To clean-up the new list the following logic is used: We need two events to consider bitmap migration finished: 1. chunk with DIRTY_BITMAP_MIG_FLAG_COMPLETE flag should be received 2. dirty_bitmap_mig_before_vm_start should be called These two events may come in any order, so we understand which one is last, and on the last of them we remove bitmap migration state from the list. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200727194236.19551-15-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* migration/block-dirty-bitmap: simplify dirty_bitmap_load_completeVladimir Sementsov-Ogievskiy2020-07-271-21/+4Star
| | | | | | | | | | | | | | | bdrv_enable_dirty_bitmap_locked() call does nothing, as if we are in postcopy, bitmap successor must be enabled, and reclaim operation will enable the bitmap. So, actually we need just call _reclaim_ in both if branches, and making differences only to add an assertion seems not really good. The logic becomes simple: on load complete we do reclaim and that's all. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200727194236.19551-14-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* migration/block-dirty-bitmap: rename finish_lock to just lockVladimir Sementsov-Ogievskiy2020-07-271-6/+6
| | | | | | | | | finish_lock is bad name, as lock used not only on process end. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200727194236.19551-13-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* migration/block-dirty-bitmap: refactor state global variablesVladimir Sementsov-Ogievskiy2020-07-271-80/+99
| | | | | | | | | | Move all state variables into one global struct. Reduce global variable usage, utilizing opaque pointer where possible. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Message-Id: <20200727194236.19551-12-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* migration/block-dirty-bitmap: move mutex init to dirty_bitmap_mig_initVladimir Sementsov-Ogievskiy2020-07-271-5/+1Star
| | | | | | | | | | No reasons to keep two public init functions. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Message-Id: <20200727194236.19551-11-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* migration/block-dirty-bitmap: rename dirty_bitmap_mig_cleanupVladimir Sementsov-Ogievskiy2020-07-271-4/+4
| | | | | | | | | | | Rename dirty_bitmap_mig_cleanup to dirty_bitmap_do_save_cleanup, to stress that it is on save part. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200727194236.19551-10-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* migration/block-dirty-bitmap: rename state structure typesVladimir Sementsov-Ogievskiy2020-07-271-33/+37
| | | | | | | | | | Rename types to be symmetrical for load/save part and shorter. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200727194236.19551-9-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* migration/block-dirty-bitmap: fix dirty_bitmap_mig_before_vm_startVladimir Sementsov-Ogievskiy2020-07-271-1/+1
| | | | | | | | | | | | | | Using the _locked version of bdrv_enable_dirty_bitmap to bypass locking is wrong as we do not already own the mutex. Moreover, the adjacent call to bdrv_dirty_bitmap_enable_successor grabs the mutex. Fixes: 58f72b965e9e1q Cc: qemu-stable@nongnu.org # v3.0 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200727194236.19551-8-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* migration/block-dirty-bitmap: fix add_bitmaps_to_listVladimir Sementsov-Ogievskiy2020-07-171-1/+5
| | | | | | | | | | | | | | We shouldn't fail when finding an unnamed bitmap in a unnamed node or node with auto-generated node name, as bitmap migration ignores such bitmaps in the first place. Fixes: 82640edb88faa Fixes: 4ff5cc121b089 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200626130658.76498-1-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: commit message grammar tweaks] Signed-off-by: Eric Blake <eblake@redhat.com>
* migration: forbid bitmap migration by generated node-nameVladimir Sementsov-Ogievskiy2020-05-281-0/+7
| | | | | | | | | | | | | | | It actually never worked with libvirt, as auto-generated names are different on source and destination. It's unsafe and useless to migrate by auto-generated node-names, so let's forbid it. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200521220648.3255-7-vsementsov@virtuozzo.com> [eblake: shorter subject line] Signed-off-by: Eric Blake <eblake@redhat.com>
* migration: add_bitmaps_to_list: check disk name onceVladimir Sementsov-Ogievskiy2020-05-281-6/+11
| | | | | | | | | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200521220648.3255-6-vsementsov@virtuozzo.com> [eblake: shorter subject line] Signed-off-by: Eric Blake <eblake@redhat.com>
* migration: fix bitmaps pre-blockdev migration with mirror jobVladimir Sementsov-Ogievskiy2020-05-281-1/+44
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Important thing for bitmap migration is to select destination block node to obtain the migrated bitmap. Prepatch, on source we use bdrv_get_device_or_node_name() to identify the node, and on target we do bdrv_lookup_bs. bdrv_get_device_or_node_name() returns blk name only for direct children of blk. So, bitmaps of direct children of blks are migrated by blk name and others - by node name. Old libvirt is unprepared to bitmap migration by node-name, node-names are mostly auto-generated. So actually only migration by blk name works for it. Newer libvirt will use new interface (which will be added soon) to specify node-mapping for bitmaps migration explicitly. Still, let's improve the current behavior a bit. Now, consider classic libvirt migrations assisted by mirror block job: mirror block job inserts filter, so our source is not a direct child of blk, and bitmaps are migrated by node-names. And this just doesn't work with auto-generated node names. Let's fix it by using blk-name even if some implicit filters are inserted. Note2: we, of course, can't skip filters and use blk name to migrate bitmaps in filtered node by blk name for this blk if these filters have named bitmaps which should be migrated. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1652424 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200521220648.3255-4-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: comment typo fix, shorter subject line] Signed-off-by: Eric Blake <eblake@redhat.com>
* migration: refactor init_dirty_bitmap_migrationVladimir Sementsov-Ogievskiy2020-05-281-40/+49
| | | | | | | | | | | Split out handling one bs, it is needed for the following commit, which will handle BlockBackends separately. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200521220648.3255-2-vsementsov@virtuozzo.com> [eblake: shorter subject line] Signed-off-by: Eric Blake <eblake@redhat.com>
* block/dirty-bitmap: refactor bdrv_dirty_bitmap_nextVladimir Sementsov-Ogievskiy2019-10-171-3/+1Star
| | | | | | | | | | | bdrv_dirty_bitmap_next is always used in same pattern. So, split it into _next and _first, instead of combining two functions into one and add FOR_EACH_DIRTY_BITMAP macro. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 20190916141911.5255-5-vsementsov@virtuozzo.com Signed-off-by: John Snow <jsnow@redhat.com>
* block/dirty-bitmap: add bs linkVladimir Sementsov-Ogievskiy2019-10-171-4/+3Star
| | | | | | | | | | | Add bs field to BdrvDirtyBitmap structure. Drop BlockDriverState parameter from bitmap APIs where possible. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 20190916141911.5255-3-vsementsov@virtuozzo.com [Rebased on top of block-copy. --js] Signed-off-by: John Snow <jsnow@redhat.com>
* migration: register_savevm_live doesn't need devDr. David Alan Gilbert2019-09-121-1/+1
| | | | | | | | | | | | Commit 78dd48df3 removed the last caller of register_savevm_live for an instantiable device (rather than a single system wide device); so trim out the parameter. Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Message-Id: <20190822115433.12070-1-dgilbert@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
* qapi: implement block-dirty-bitmap-remove transaction actionJohn Snow2019-08-161-1/+1
| | | | | | | | | | | | | | It is used to do transactional movement of the bitmap (which is possible in conjunction with merge command). Transactional bitmap movement is needed in scenarios with external snapshot, when we don't want to leave copy of the bitmap in the base image. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 20190708220502.12977-3-jsnow@redhat.com [Edited "since" version to 4.2 --js] Signed-off-by: John Snow <jsnow@redhat.com>
* sysemu: Split sysemu/runstate.h off sysemu/sysemu.hMarkus Armbruster2019-08-161-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | sysemu/sysemu.h is a rather unfocused dumping ground for stuff related to the system-emulator. Evidence: * It's included widely: in my "build everything" tree, changing sysemu/sysemu.h still triggers a recompile of some 1100 out of 6600 objects (not counting tests and objects that don't depend on qemu/osdep.h, down from 5400 due to the previous two commits). * It pulls in more than a dozen additional headers. Split stuff related to run state management into its own header sysemu/runstate.h. Touching sysemu/sysemu.h now recompiles some 850 objects. qemu/uuid.h also drops from 1100 to 850, and qapi/qapi-types-run-state.h from 4400 to 4200. Touching new sysemu/runstate.h recompiles some 500 objects. Since I'm touching MAINTAINERS to add sysemu/runstate.h anyway, also add qemu/main-loop.h. Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20190812052359.30071-30-armbru@redhat.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> [Unbreak OS-X build]
* migration/dirty-bitmaps: change bitmap enumeration methodJohn Snow2019-05-291-10/+4Star
| | | | | | | | | | | | | | | | | | | | | | | Shift from looking at every root BDS to *every* BDS. This will migrate bitmaps that are attached to blockdev created nodes instead of just ones attached to emulated storage devices. Note that this will not migrate anonymous or internal-use bitmaps, as those are defined as having no name. This will also fix the Coverity issues Peter Maydell has been asking about for the past several releases, as well as fixing a real bug. Reported-by: Peter Maydell <peter.maydell@linaro.org> Reported-by: Coverity 😅 Reported-by: aihua liang <aliang@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: John Snow <jsnow@redhat.com> Message-id: 20190514201926.10407-1-jsnow@redhat.com Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1652490 Fixes: Coverity CID 1390625 CC: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com>
* bitmaps: Fix typo in function nameEric Blake2019-03-121-2/+2
| | | | | | | | | Commit a88b179f introduced the ability to set and query bitmap persistence, but with an atypical spelling. Signed-off-by: Eric Blake <eblake@redhat.com> Message-id: 20190308205845.25734-1-eblake@redhat.com Signed-off-by: John Snow <jsnow@redhat.com>
* block/dirty-bitmaps: add block_dirty_bitmap_check functionJohn Snow2019-03-121-9/+4Star
| | | | | | | | | | | | | | | | | | | | | | Instead of checking against busy, inconsistent, or read only directly, use a check function with permissions bits that let us streamline the checks without reproducing them in many places. Included in this patch are permissions changes that simply add the inconsistent check to existing permissions call spots, without addressing existing bugs. In general, this means that busy+readonly checks become BDRV_BITMAP_DEFAULT, which checks against all three conditions. busy-only checks become BDRV_BITMAP_ALLOW_RO. Notably, remove allows inconsistent bitmaps, so it doesn't follow the pattern. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-id: 20190301191545.8728-4-jsnow@redhat.com Signed-off-by: John Snow <jsnow@redhat.com>
* block/dirty-bitmaps: unify qmp_locked and user_locked callsJohn Snow2019-03-121-3/+3
| | | | | | | | | | | | | These mean the same thing now. Unify them and rename the merged call bdrv_dirty_bitmap_busy to indicate semantically what we are describing, as well as help disambiguate from the various _locked and _unlocked versions of bitmap helpers that refer to mutex locks. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-id: 20190223000614.13894-8-jsnow@redhat.com Signed-off-by: John Snow <jsnow@redhat.com>
* block/dirty-bitmaps: rename frozen predicate helperJohn Snow2019-03-121-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | "Frozen" was a good description a long time ago, but it isn't adequate now. Rename the frozen predicate to has_successor to make the semantics of the predicate more clear to outside callers. In the process, remove some calls to frozen() that no longer semantically make sense. For bdrv_enable_dirty_bitmap_locked and bdrv_disable_dirty_bitmap_locked, it doesn't make sense to prohibit QEMU internals from performing this action when we only wished to prohibit QMP users from issuing these commands. All of the QMP API commands for bitmap manipulation already check against user_locked() to prohibit these actions. Several other assertions really want to check that the bitmap isn't in-use by another operation -- use the bitmap_user_locked function for this instead, which presently also checks for has_successor. This leaves some redundant checks of has_successor through different helpers that are addressed in forthcoming patches. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-id: 20190223000614.13894-3-jsnow@redhat.com Signed-off-by: John Snow <jsnow@redhat.com>
* qemu/queue.h: leave head structs anonymous unless necessaryPaolo Bonzini2019-01-111-1/+1
| | | | | | | | | | | | Most list head structs need not be given a name. In most cases the name is given just in case one is going to use QTAILQ_LAST, QTAILQ_PREV or reverse iteration, but this does not apply to lists of other kinds, and even for QTAILQ in practice this is only rarely needed. In addition, we will soon reimplement those macros completely so that they do not need a name for the head struct. So clean up everything, not giving a name except in the rare case where it is necessary. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* dirty-bitmaps: clean-up bitmaps loading and migration logicVladimir Sementsov-Ogievskiy2018-10-291-2/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch aims to bring the following behavior: 1. We don't load bitmaps, when started in inactive mode. It's the case of incoming migration. In this case we wait for bitmaps migration through migration channel (if 'dirty-bitmaps' capability is enabled) or for invalidation (to load bitmaps from the image). 2. We don't remove persistent bitmaps on inactivation. Instead, we only remove bitmaps after storing. This is the only way to restore bitmaps, if we decided to resume source after [failed] migration with 'dirty-bitmaps' capability enabled (which means, that bitmaps were not stored). 3. We load bitmaps on open and any invalidation, it's ok for all cases: - normal open - migration target invalidation with dirty-bitmaps capability (bitmaps are migrating through migration channel, the are not stored, so they should have IN_USE flag set and will be skipped when loading. However, it would fail if bitmaps are read-only[1]) - migration target invalidation without dirty-bitmaps capability (normal load of the bitmaps, if migrated with shared storage) - source invalidation with dirty-bitmaps capability (skip because IN_USE) - source invalidation without dirty-bitmaps capability (bitmaps were dropped, reload them) [1]: to accurately handle this, migration of read-only bitmaps is explicitly forbidden in this patch. New mechanism for not storing bitmaps when migrate with dirty-bitmaps capability is introduced: migration filed in BdrvDirtyBitmap. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: John Snow <jsnow@redhat.com>
* block/dirty-bitmaps: add user_locked status checkerJohn Snow2018-10-291-8/+2Star
| | | | | | | | | | | | | | | | | Instead of both frozen and qmp_locked checks, wrap it into one check. frozen implies the bitmap is split in two (for backup), and shouldn't be modified. qmp_locked implies it's being used by another operation, like being exported over NBD. In both cases it means we shouldn't allow the user to modify it in any meaningful way. Replace any usages where we check both frozen and qmp_locked with the new check. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 20181002230218.13949-2-jsnow@redhat.com [w/edits Suggested-By: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>] Signed-off-by: John Snow <jsnow@redhat.com>
* dirty-bitmap: fix double lock on bitmap enablingVladimir Sementsov-Ogievskiy2018-07-041-2/+2
| | | | | | | | | | | | | Bitmap lock/unlock were added to bdrv_enable_dirty_bitmap in 8b1402ce80d, but some places were not updated correspondingly, which leads to trying to take this lock twice, which is dead-lock. Fix this. Actually, iotest 199 (about dirty bitmap postcopy migration) is broken now, and this fixes it. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-id: 20180625165745.25259-3-vsementsov@virtuozzo.com Signed-off-by: John Snow <jsnow@redhat.com>
* migration/block-dirty-bitmap: fix dirty_bitmap_loadVladimir Sementsov-Ogievskiy2018-06-151-0/+3
| | | | | | | | | | | | | | | dirty_bitmap_load_header return code is obtained but not handled. Fix this. Bug was introduced in b35ebdf076d697bc "migration: add postcopy migration of dirty bitmaps" with the whole function. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180530112424.204835-1-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
* migration: use local path for local headersMichael S. Tsirkin2018-06-011-1/+1
| | | | | | | | | When pulling in headers that are in the same directory as the C file (as opposed to one in include/), we should use its relative path, without a directory. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>
* migration/block-dirty-bitmap: fix memory leak in dirty_bitmap_load_bitsVladimir Sementsov-Ogievskiy2018-05-041-0/+1
| | | | | | | | | | | | | Release buf on error path too. Bug was introduced in b35ebdf076d697bc "migration: add postcopy migration of dirty bitmaps" with the whole function. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180427142002.21930-3-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com>
* migration: add postcopy migration of dirty bitmapsVladimir Sementsov-Ogievskiy2018-03-131-0/+746
Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated. If destination qemu is already containing a dirty bitmap with the same name as a migrated bitmap (for the same node), then, if their granularities are the same the migration will be done, otherwise the error will be generated. If destination qemu doesn't contain such bitmap it will be created. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com [Changed '+' to '*' as per list discussion. --js] Signed-off-by: John Snow <jsnow@redhat.com>