add patch fixing resume for snapshot and hibernate with drive with iothread and a dirty bitmap
Not difficult to run into, just have a drive with iothread, take a PBS backup and then take a snapshot or hibernate. Resuming will fail with > qemu: qemu_mutex_unlock_impl: Operation not permitted because of not acquiring the correct AioContext first. Migration is not affected, because it runs in coroutine context. Reported in the community forum: https://forum.proxmox.com/threads/129899/ Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
This commit is contained in:
		
							parent
							
								
									409db0cd7b
								
							
						
					
					
						commit
						5919ec1446
					
				
							
								
								
									
										48
									
								
								debian/patches/extra/0010-migration-block-dirty-bitmap-fix-loading-bitmap-when.patch
									
									
									
									
										vendored
									
									
										Normal file
									
								
							
							
						
						
									
										48
									
								
								debian/patches/extra/0010-migration-block-dirty-bitmap-fix-loading-bitmap-when.patch
									
									
									
									
										vendored
									
									
										Normal file
									
								
							| @ -0,0 +1,48 @@ | |||||||
|  | From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: Fiona Ebner <f.ebner@proxmox.com> | ||||||
|  | Date: Fri, 28 Jul 2023 10:47:48 +0200 | ||||||
|  | Subject: [PATCH] migration/block-dirty-bitmap: fix loading bitmap when there | ||||||
|  |  is an iothread | ||||||
|  | 
 | ||||||
|  | The bdrv_create_dirty_bitmap() function (which is also called by | ||||||
|  | bdrv_dirty_bitmap_create_successor()) uses bdrv_getlength(bs). This is | ||||||
|  | a wrapper around a coroutine, and thus uses bdrv_poll_co(). Polling | ||||||
|  | tries to release the AioContext which will trigger an assert() if it | ||||||
|  | hasn't been acquired before. | ||||||
|  | 
 | ||||||
|  | The issue does not happen for migration, because there we are in a | ||||||
|  | coroutine already, so the wrapper will just call bdrv_co_getlength() | ||||||
|  | directly without polling. | ||||||
|  | 
 | ||||||
|  | Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> | ||||||
|  | ---
 | ||||||
|  |  migration/block-dirty-bitmap.c | 6 ++++++ | ||||||
|  |  1 file changed, 6 insertions(+) | ||||||
|  | 
 | ||||||
|  | diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
 | ||||||
|  | index fe73aa94b1..7eaf498439 100644
 | ||||||
|  | --- a/migration/block-dirty-bitmap.c
 | ||||||
|  | +++ b/migration/block-dirty-bitmap.c
 | ||||||
|  | @@ -805,8 +805,11 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s)
 | ||||||
|  |                       "destination", bdrv_dirty_bitmap_name(s->bitmap)); | ||||||
|  |          return -EINVAL; | ||||||
|  |      } else { | ||||||
|  | +        AioContext *ctx = bdrv_get_aio_context(s->bs);
 | ||||||
|  | +        aio_context_acquire(ctx);
 | ||||||
|  |          s->bitmap = bdrv_create_dirty_bitmap(s->bs, granularity, | ||||||
|  |                                               s->bitmap_name, &local_err); | ||||||
|  | +        aio_context_release(ctx);
 | ||||||
|  |          if (!s->bitmap) { | ||||||
|  |              error_report_err(local_err); | ||||||
|  |              return -EINVAL; | ||||||
|  | @@ -833,7 +836,10 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s)
 | ||||||
|  |   | ||||||
|  |      bdrv_disable_dirty_bitmap(s->bitmap); | ||||||
|  |      if (flags & DIRTY_BITMAP_MIG_START_FLAG_ENABLED) { | ||||||
|  | +        AioContext *ctx = bdrv_get_aio_context(s->bs);
 | ||||||
|  | +        aio_context_acquire(ctx);
 | ||||||
|  |          bdrv_dirty_bitmap_create_successor(s->bitmap, &local_err); | ||||||
|  | +        aio_context_release(ctx);
 | ||||||
|  |          if (local_err) { | ||||||
|  |              error_report_err(local_err); | ||||||
|  |              return -EINVAL; | ||||||
| @ -19,7 +19,7 @@ Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com> | |||||||
|  1 file changed, 1 insertion(+), 1 deletion(-) |  1 file changed, 1 insertion(+), 1 deletion(-) | ||||||
| 
 | 
 | ||||||
| diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
 | diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
 | ||||||
| index fe73aa94b1..a6440929fa 100644
 | index 7eaf498439..509f3df0a6 100644
 | ||||||
| --- a/migration/block-dirty-bitmap.c
 | --- a/migration/block-dirty-bitmap.c
 | ||||||
| +++ b/migration/block-dirty-bitmap.c
 | +++ b/migration/block-dirty-bitmap.c
 | ||||||
| @@ -539,7 +539,7 @@ static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs,
 | @@ -539,7 +539,7 @@ static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs,
 | ||||||
|  | |||||||
| @ -67,10 +67,10 @@ index a8dfd8fefd..fa9b0b0f10 100644 | |||||||
|       * must_precopy: |       * must_precopy: | ||||||
|       * - must be migrated in precopy or in stopped state |       * - must be migrated in precopy or in stopped state | ||||||
| diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
 | diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
 | ||||||
| index a6440929fa..69fab3275c 100644
 | index 509f3df0a6..42dc4a8d61 100644
 | ||||||
| --- a/migration/block-dirty-bitmap.c
 | --- a/migration/block-dirty-bitmap.c
 | ||||||
| +++ b/migration/block-dirty-bitmap.c
 | +++ b/migration/block-dirty-bitmap.c
 | ||||||
| @@ -1214,10 +1214,17 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
 | @@ -1220,10 +1220,17 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
 | ||||||
|  { |  { | ||||||
|      DBMSaveState *s = &((DBMState *)opaque)->save; |      DBMSaveState *s = &((DBMState *)opaque)->save; | ||||||
|      SaveBitmapState *dbms = NULL; |      SaveBitmapState *dbms = NULL; | ||||||
| @ -90,7 +90,7 @@ index a6440929fa..69fab3275c 100644 | |||||||
|          return -1; |          return -1; | ||||||
|      } |      } | ||||||
|   |   | ||||||
| @@ -1225,7 +1232,9 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
 | @@ -1231,7 +1238,9 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
 | ||||||
|          send_bitmap_start(f, s, dbms); |          send_bitmap_start(f, s, dbms); | ||||||
|      } |      } | ||||||
|      qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS); |      qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS); | ||||||
|  | |||||||
							
								
								
									
										1
									
								
								debian/patches/series
									
									
									
									
										vendored
									
									
								
							
							
						
						
									
										1
									
								
								debian/patches/series
									
									
									
									
										vendored
									
									
								
							| @ -7,6 +7,7 @@ extra/0006-lsi53c895a-disable-reentrancy-detection-for-script-R.patch | |||||||
| extra/0007-bcm2835_property-disable-reentrancy-detection-for-io.patch | extra/0007-bcm2835_property-disable-reentrancy-detection-for-io.patch | ||||||
| extra/0008-raven-disable-reentrancy-detection-for-iomem.patch | extra/0008-raven-disable-reentrancy-detection-for-iomem.patch | ||||||
| extra/0009-apic-disable-reentrancy-detection-for-apic-msi.patch | extra/0009-apic-disable-reentrancy-detection-for-apic-msi.patch | ||||||
|  | extra/0010-migration-block-dirty-bitmap-fix-loading-bitmap-when.patch | ||||||
| bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch | bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch | ||||||
| bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch | bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch | ||||||
| bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch | bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user
	 Fiona Ebner
						Fiona Ebner