[pve-devel] [PATCH 0/4] Keep dirty-bitmaps for PBS during migration

[pve-devel] [PATCH 0/4] Keep dirty-bitmaps for PBS during migration

[Stefan Reiter]

Allow dirty bitmaps to persist over a live migration, allowing incremental
backups even after a VM has been moved to another node.

Migrating the dirty-bitmaps themselves is supported natively by QEMU, only
requiring a fix for a bug leading to hangs when migrating bitmaps with a
granularity as low as we are using (see first patch).

The second and third patches are the interesting bits, implementing
"savevm"-style migration for all PBS state that is being kept in memory
(checksums).

The last patch brings it all together by enabling it in qemu-server when support
is detected (which is important, since we never want to enable "dirty-bitmaps"
capability on unpatched versions of QEMU, as that will lead to unrecoverable VM
hangs).

Compatility is maintained between all pve-qemu-kvm and qemu-server versions,
with the usual exception of not being able to migrate VMs started with a newer
QEMU version to a machine with an older one.


qemu: Stefan Reiter (2):
  migration/block-dirty-bitmap: fix larger granularity bitmaps
  PVE: Migrate dirty bitmap state via savevm

 include/migration/misc.h       |  3 ++
 migration/Makefile.objs        |  1 +
 migration/block-dirty-bitmap.c |  3 +-
 migration/pbs-state.c          | 92 ++++++++++++++++++++++++++++++++++
 pve-backup.c                   |  1 +
 qapi/block-core.json           |  9 +++-
 softmmu/vl.c                   |  1 +
 7 files changed, 108 insertions(+), 2 deletions(-)
 create mode 100644 migration/pbs-state.c

proxmox-backup-qemu: Stefan Reiter (1):
  add state serializing and loading functions

 Cargo.toml      |  1 +
 current-api.h   | 20 ++++++++++++++++++++
 src/commands.rs | 19 +++++++++++++++++++
 src/lib.rs      | 34 ++++++++++++++++++++++++++++++++++
 4 files changed, 74 insertions(+)

qemu-server: Stefan Reiter (1):
  migrate: enable dirty-bitmap migration

 PVE/QemuServer.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

-- 
2.20.1

[pve-devel] [PATCH qemu 1/4] migration/block-dirty-bitmap: fix larger granularity bitmaps

[Stefan Reiter]

sectors_per_chunk is a 64 bit integer, but the calculation would be 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.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---

See also this thread for the upstream discussion:
https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg06145.html

 migration/block-dirty-bitmap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 784330ebe1..5bf0d9fbc6 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -334,8 +334,9 @@ static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs,
         dbms->node_name = bs_name;
         dbms->bitmap = bitmap;
         dbms->total_sectors = bdrv_nb_sectors(bs);
-        dbms->sectors_per_chunk = CHUNK_SIZE * 8 *
+        dbms->sectors_per_chunk = CHUNK_SIZE * 8LLU *
             bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
+        assert(dbms->sectors_per_chunk != 0);
         if (bdrv_dirty_bitmap_enabled(bitmap)) {
             dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_ENABLED;
         }
-- 
2.20.1

[pve-devel] [PATCH qemu 2/4] PVE: Migrate dirty bitmap state via savevm

[Stefan Reiter]

QEMU provides 'savevm' registrations as a mechanism for arbitrary state
to be migrated along with a VM. Use this to send a serialized version of
dirty bitmap state data from proxmox-backup-qemu, and restore it on the
target node.

Also add a flag to query-proxmox-support so qemu-server can determine if
safe migration is possible and makes sense.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---

Depends on the updated proxmox-backup-qemu library of course.

The new query-proxmox-support property is also used to detect the fix from patch
2, so these must be applied together.

 include/migration/misc.h |  3 ++
 migration/Makefile.objs  |  1 +
 migration/pbs-state.c    | 92 ++++++++++++++++++++++++++++++++++++++++
 pve-backup.c             |  1 +
 qapi/block-core.json     |  9 +++-
 softmmu/vl.c             |  1 +
 6 files changed, 106 insertions(+), 1 deletion(-)
 create mode 100644 migration/pbs-state.c

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 34e7d75713..f83816dd3c 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -75,4 +75,7 @@ bool migration_in_incoming_postcopy(void);
 /* migration/block-dirty-bitmap.c */
 void dirty_bitmap_mig_init(void);
 
+/* migration/pbs-state.c */
+void pbs_state_mig_init(void);
+
 #endif
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 0fc619e380..20b3792599 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -9,6 +9,7 @@ common-obj-y += qjson.o
 common-obj-y += block-dirty-bitmap.o
 common-obj-y += multifd.o
 common-obj-y += multifd-zlib.o
+common-obj-y += pbs-state.o
 common-obj-$(CONFIG_ZSTD) += multifd-zstd.o
 
 common-obj-$(CONFIG_RDMA) += rdma.o
diff --git a/migration/pbs-state.c b/migration/pbs-state.c
new file mode 100644
index 0000000000..165895b488
--- /dev/null
+++ b/migration/pbs-state.c
@@ -0,0 +1,92 @@
+/*
+ * PBS (dirty-bitmap) state migration
+ */
+
+#include "qemu/osdep.h"
+#include "migration/misc.h"
+#include "qemu-file.h"
+#include "migration/vmstate.h"
+#include "migration/register.h"
+#include "proxmox-backup-qemu.h"
+
+static void pbs_state_save_pending(QEMUFile *f, void *opaque,
+                                      uint64_t max_size,
+                                      uint64_t *res_precopy_only,
+                                      uint64_t *res_compatible,
+                                      uint64_t *res_postcopy_only)
+{
+    /* we send everything in save_setup, so nothing is ever pending */
+    *res_precopy_only = 0;
+    *res_compatible = 0;
+    *res_postcopy_only = 0;
+}
+
+/* receive PBS state via f and deserialize, called on target */
+static int pbs_state_load(QEMUFile *f, void *opaque, int version_id)
+{
+    /* safe cast, we cannot migrate to target with less bits than source */
+    size_t buf_size = (size_t)qemu_get_be64(f);
+
+    uint8_t *buf = (uint8_t *)malloc(buf_size);
+    size_t read = qemu_get_buffer(f, buf, buf_size);
+
+    if (read < buf_size) {
+        fprintf(stderr, "error receiving PBS state: not enough data\n");
+        return -EIO;
+    }
+
+    proxmox_import_state(buf, buf_size);
+
+    free(buf);
+    return 0;
+}
+
+/* serialize PBS state and send to target via f, called on source */
+static int pbs_state_save_setup(QEMUFile *f, void *opaque)
+{
+    size_t buf_size;
+    uint8_t *buf = proxmox_export_state(&buf_size);
+
+    /* LV encoding */
+    qemu_put_be64(f, buf_size);
+    qemu_put_buffer(f, buf, buf_size);
+
+    proxmox_free_state_buf(buf);
+    return 0;
+}
+
+static bool pbs_state_is_active(void *opaque)
+{
+    /* always active, i.e. we do our job for every migration, since there's no
+     * harm done if we just copy an empty buffer */
+    return true;
+}
+
+static bool pbs_state_is_active_iterate(void *opaque)
+{
+    /* we don't iterate, everything is sent in save_setup */
+    return false;
+}
+
+static bool pbs_state_has_postcopy(void *opaque)
+{
+    /* PBS state can't change during a migration (since that's blocking any
+     * potential backups), so we can copy everything before the VM is stopped */
+    return false;
+}
+
+static SaveVMHandlers savevm_pbs_state_handlers = {
+    .save_setup = pbs_state_save_setup,
+    .has_postcopy = pbs_state_has_postcopy,
+    .save_live_pending = pbs_state_save_pending,
+    .is_active_iterate = pbs_state_is_active_iterate,
+    .load_state = pbs_state_load,
+    .is_active = pbs_state_is_active,
+};
+
+void pbs_state_mig_init(void)
+{
+    register_savevm_live("pbs-state", 0, 1,
+                         &savevm_pbs_state_handlers,
+                         NULL);
+}
diff --git a/pve-backup.c b/pve-backup.c
index 53cf23ed5a..1954d22069 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -1080,5 +1080,6 @@ ProxmoxSupportStatus *qmp_query_proxmox_support(Error **errp)
     ProxmoxSupportStatus *ret = g_malloc0(sizeof(*ret));
     ret->pbs_dirty_bitmap = true;
     ret->query_bitmap_info = true;
+    ret->pbs_dirty_bitmap_migration = true;
     return ret;
 }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b31ad8d989..00c9e12fcc 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -890,9 +890,16 @@
 #
 # @query-bitmap-info: True if the 'query-pbs-bitmap-info' QMP call is supported.
 #
+# @pbs-dirty-bitmap-migration: True if safe migration of dirty-bitmaps including
+#                              PBS state is supported. Enabling 'dirty-bitmaps'
+#                              migration cap if this is false/unset may lead
+#                              to crashes on migration!
+#
 ##
 { 'struct': 'ProxmoxSupportStatus',
-  'data': { 'pbs-dirty-bitmap': 'bool', 'query-bitmap-info': 'bool' } }
+  'data': { 'pbs-dirty-bitmap': 'bool',
+            'query-bitmap-info': 'bool',
+            'pbs-dirty-bitmap-migration': 'bool' } }
 
 ##
 # @query-proxmox-support:
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 16aa2186b0..88b13871fd 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -4288,6 +4288,7 @@ void qemu_init(int argc, char **argv, char **envp)
     blk_mig_init();
     ram_mig_init();
     dirty_bitmap_mig_init();
+    pbs_state_mig_init();
 
     qemu_opts_foreach(qemu_find_opts("mon"),
                       mon_init_func, NULL, &error_fatal);
-- 
2.20.1

[pve-devel] [PATCH proxmox-backup-qemu 3/4] add state serializing and loading functions

[Stefan Reiter]

For dirty-bitmap migration, QEMU also needs to move the static state of
the library to the target. proxmox_{import,export}_state provide a means
of accessing said data in a serialized fashion.

QEMU treats the state as some unknown quantity of bytes and the result
does not need to be human-readable, so we encode it with 'bincode',
which is based on serde.

Since the quantity is only known *after* serialization, we have to
allocate the buffer ourselves. This is handled by Box::leak-ing a Rust
allocated buffer and cleaning up via the explicit
proxmox_free_state_buf function.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---

I'm not sure we use 'bincode' anywhere else, but it is packaged for debian
already and seemed like a good fit. Alternatively, we could of course use our
already existing dependency on serde_json and just serialize to a JSON string.

 Cargo.toml      |  1 +
 current-api.h   | 20 ++++++++++++++++++++
 src/commands.rs | 19 +++++++++++++++++++
 src/lib.rs      | 34 ++++++++++++++++++++++++++++++++++
 4 files changed, 74 insertions(+)

diff --git a/Cargo.toml b/Cargo.toml
index d51ba93..ca93b36 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -28,3 +28,4 @@ proxmox-backup = { git = "git://git.proxmox.com/git/proxmox-backup.git", tag = "
 #proxmox-backup = { path = "../proxmox-backup" }
 serde_json = "1.0"
 tokio = { version = "0.2.9", features = [ "blocking", "fs", "io-util", "macros", "rt-threaded", "signal", "stream", "tcp", "time", "uds" ] }
+bincode = "1.0"
diff --git a/current-api.h b/current-api.h
index d77eff6..77e8c4b 100644
--- a/current-api.h
+++ b/current-api.h
@@ -263,6 +263,26 @@ void proxmox_backup_write_data_async(ProxmoxBackupHandle *handle,
                                      int *result,
                                      char **error);
 
+/**
+ * Serialize all state data into a byte buffer. Can be loaded again with
+ * proxmox_import_state. Use for migration for example.
+ *
+ * Length of the returned buffer is written to buf_size. Returned buffer must
+ * be freed with proxmox_free_state_buf.
+ */
+uint8_t *proxmox_export_state(uintptr_t *buf_size);
+
+/**
+ * Free a buffer acquired from proxmox_export_state.
+ */
+void proxmox_free_state_buf(uint8_t *buf);
+
+/**
+ * Load state serialized by proxmox_export_state. If loading fails, a message
+ * will be logged to stderr, but the function will not fail.
+ */
+void proxmox_import_state(const uint8_t *buf, uintptr_t buf_size);
+
 /**
  * Open connection to the backup server (sync)
  *
diff --git a/src/commands.rs b/src/commands.rs
index f7e0f62..7a24b7c 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -16,6 +16,9 @@ use crate::upload_queue::*;
 use lazy_static::lazy_static;
 
 lazy_static!{
+    // Note: Any state stored here that needs to be sent along with migration
+    // needs to be specified in (de)serialize_state as well!
+
     static ref PREVIOUS_CSUMS: Mutex<HashMap<String, [u8;32]>> = {
         Mutex::new(HashMap::new())
     };
@@ -35,6 +38,22 @@ pub struct ImageUploadInfo {
 }
 
 
+pub(crate) fn serialize_state() -> Vec<u8> {
+    let prev_csums = &*PREVIOUS_CSUMS.lock().unwrap();
+    let prev_crypt_digest = &*PREVIOUS_CRYPT_CONFIG_DIGEST.lock().unwrap();
+    bincode::serialize(&(prev_csums, prev_crypt_digest)).unwrap()
+}
+
+pub(crate) fn deserialize_state(data: &[u8]) -> Result<(), Error> {
+    let (prev_csums, prev_crypt_digest) = bincode::deserialize(data)?;
+    let mut prev_csums_guard = PREVIOUS_CSUMS.lock().unwrap();
+    let mut prev_crypt_digest_guard = PREVIOUS_CRYPT_CONFIG_DIGEST.lock().unwrap();
+    *prev_csums_guard = prev_csums;
+    *prev_crypt_digest_guard = prev_crypt_digest;
+    Ok(())
+}
+
+
 // Note: We alway register/upload a chunk containing zeros
 async fn register_zero_chunk(
     client: Arc<BackupWriter>,
diff --git a/src/lib.rs b/src/lib.rs
index b9ca905..8511cbf 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -967,3 +967,37 @@ pub extern "C" fn proxmox_restore_read_image_at_async(
         callback_info.send_result(result);
     });
 }
+
+/// Serialize all state data into a byte buffer. Can be loaded again with
+/// proxmox_import_state. Use for migration for example.
+///
+/// Length of the returned buffer is written to buf_size. Returned buffer must
+/// be freed with proxmox_free_state_buf.
+#[no_mangle]
+#[allow(clippy::not_unsafe_ptr_arg_deref)]
+pub extern "C" fn proxmox_export_state(buf_size: *mut usize) -> *mut u8 {
+    let data = commands::serialize_state().into_boxed_slice();
+    unsafe { *buf_size = data.len(); }
+    Box::leak(data).as_mut_ptr()
+}
+
+/// Load state serialized by proxmox_export_state. If loading fails, a message
+/// will be logged to stderr, but the function will not fail.
+#[no_mangle]
+#[allow(clippy::not_unsafe_ptr_arg_deref)]
+pub extern "C" fn proxmox_import_state(buf: *const u8, buf_size: usize) {
+    let data = unsafe { std::slice::from_raw_parts(buf, buf_size) };
+    // ignore errors, just log what happened
+    if let Err(err) = commands::deserialize_state(data) {
+        eprintln!("error deserializing PBS state - {}", err);
+    }
+}
+
+/// Free a buffer acquired from proxmox_export_state.
+#[no_mangle]
+#[allow(clippy::not_unsafe_ptr_arg_deref)]
+pub extern "C" fn proxmox_free_state_buf(buf: *mut u8) {
+    if !buf.is_null() {
+        unsafe { Box::from_raw(buf); }
+    }
+}
-- 
2.20.1

[pve-devel] [PATCH qemu-server 4/4] migrate: enable dirty-bitmap migration

[Stefan Reiter]

We query QEMU if it's safe before enabling it, as on versions without
the necessary patches it not only would be useless, but can actually
lead to hangs.

PBS state is always migrated, as it's a small amount of data anyway, so
we don't need to set a specific flag for it.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 PVE/QemuServer.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 20e284c..a282449 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4247,6 +4247,8 @@ sub qemu_volume_snapshot_delete {
 sub set_migration_caps {
     my ($vmid) = @_;
 
+    my $qemu_support = eval { mon_cmd($vmid, "query-proxmox-support") };
+
     my $cap_ref = [];
 
     my $enabled_cap = {
@@ -4254,7 +4256,8 @@ sub set_migration_caps {
 	"xbzrle" => 1,
 	"x-rdma-pin-all" => 0,
 	"zero-blocks" => 0,
-	"compress" => 0
+	"compress" => 0,
+	"dirty-bitmaps" => $qemu_support->{'pbs-dirty-bitmap-migration'} ? 1 : 0,
     };
 
     my $supported_capabilities = mon_cmd($vmid, "query-migrate-capabilities");
-- 
2.20.1

[pve-devel] applied: [PATCH proxmox-backup-qemu 3/4] add state serializing and loading functions

[Thomas Lamprecht]

On 22.10.20 17:34, Stefan Reiter wrote:
> For dirty-bitmap migration, QEMU also needs to move the static state of
> the library to the target. proxmox_{import,export}_state provide a means
> of accessing said data in a serialized fashion.
> 
> QEMU treats the state as some unknown quantity of bytes and the result
> does not need to be human-readable, so we encode it with 'bincode',
> which is based on serde.
> 
> Since the quantity is only known *after* serialization, we have to
> allocate the buffer ourselves. This is handled by Box::leak-ing a Rust
> allocated buffer and cleaning up via the explicit
> proxmox_free_state_buf function.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> 
> I'm not sure we use 'bincode' anywhere else, but it is packaged for debian
> already and seemed like a good fit. Alternatively, we could of course use our
> already existing dependency on serde_json and just serialize to a JSON string.
> 
>  Cargo.toml      |  1 +
>  current-api.h   | 20 ++++++++++++++++++++
>  src/commands.rs | 19 +++++++++++++++++++
>  src/lib.rs      | 34 ++++++++++++++++++++++++++++++++++
>  4 files changed, 74 insertions(+)
> 
>

applied, thanks!

[pve-devel] applied-series: [PATCH 0/4] Keep dirty-bitmaps for PBS during migration

[Thomas Lamprecht]

On 22.10.20 17:34, Stefan Reiter wrote:
> Allow dirty bitmaps to persist over a live migration, allowing incremental
> backups even after a VM has been moved to another node.
> 
> Migrating the dirty-bitmaps themselves is supported natively by QEMU, only
> requiring a fix for a bug leading to hangs when migrating bitmaps with a
> granularity as low as we are using (see first patch).
> 
> The second and third patches are the interesting bits, implementing
> "savevm"-style migration for all PBS state that is being kept in memory
> (checksums).
> 
> The last patch brings it all together by enabling it in qemu-server when support
> is detected (which is important, since we never want to enable "dirty-bitmaps"
> capability on unpatched versions of QEMU, as that will lead to unrecoverable VM
> hangs).
> 
> Compatility is maintained between all pve-qemu-kvm and qemu-server versions,
> with the usual exception of not being able to migrate VMs started with a newer
> QEMU version to a machine with an older one.
> 
> 
> qemu: Stefan Reiter (2):
>   migration/block-dirty-bitmap: fix larger granularity bitmaps
>   PVE: Migrate dirty bitmap state via savevm
> 
>  include/migration/misc.h       |  3 ++
>  migration/Makefile.objs        |  1 +
>  migration/block-dirty-bitmap.c |  3 +-
>  migration/pbs-state.c          | 92 ++++++++++++++++++++++++++++++++++
>  pve-backup.c                   |  1 +
>  qapi/block-core.json           |  9 +++-
>  softmmu/vl.c                   |  1 +
>  7 files changed, 108 insertions(+), 2 deletions(-)
>  create mode 100644 migration/pbs-state.c
> 
> proxmox-backup-qemu: Stefan Reiter (1):
>   add state serializing and loading functions
> 
>  Cargo.toml      |  1 +
>  current-api.h   | 20 ++++++++++++++++++++
>  src/commands.rs | 19 +++++++++++++++++++
>  src/lib.rs      | 34 ++++++++++++++++++++++++++++++++++
>  4 files changed, 74 insertions(+)
> 
> qemu-server: Stefan Reiter (1):
>   migrate: enable dirty-bitmap migration
> 
>  PVE/QemuServer.pm | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 



applied remaining parts of the series, thanks!