[pve-devel] [PATCH v2 0/5] improve logging for dirty bitmap PBS backups

[pve-devel] [PATCH v2 0/5] improve logging for dirty bitmap PBS backups

[Stefan Reiter]

Makes the log output more useful for dirty bitmap backups and fixes some minor
issues.

A new QMP call is introduced to avoid shoving even more data in the possibly
often called 'query-backup' and not having to do some hacky special casing for
the first query iteration.

The new QMP call is gated behind a new proxmox feature flag. The previous
behaviour of having 'dirty' in 'query-backup' is retained for compatibility with
older qemu-server versions.


v2:
* keep 'dirty' flag in query-backup
* add patch 5


qemu: Stefan Reiter (1):
  PVE: add query-pbs-bitmap-info QMP call

 monitor/hmp-cmds.c   |  28 ++++++-----
 pve-backup.c         | 117 ++++++++++++++++++++++++++++++++-----------
 qapi/block-core.json |  57 ++++++++++++++++++++-
 3 files changed, 159 insertions(+), 43 deletions(-)

qemu-server: Stefan Reiter (4):
  vzdump: improve logging output with dirty bitmaps
  vzdump: display actually uploaded chunks as 'write' speed
  vzdump: log 100% percent in case $target is 0
  vzdump: don't use dirty bitmap when VM was off

 PVE/VZDump/QemuServer.pm | 75 +++++++++++++++++++++++++++++++---------
 1 file changed, 59 insertions(+), 16 deletions(-)

-- 
2.20.1

[pve-devel] [PATCH v2 qemu 1/5] PVE: add query-pbs-bitmap-info QMP call

[Stefan Reiter]

Returns advanced information about dirty bitmaps used (or not used) for
the latest PBS backup.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 monitor/hmp-cmds.c   |  28 ++++++-----
 pve-backup.c         | 117 ++++++++++++++++++++++++++++++++-----------
 qapi/block-core.json |  57 ++++++++++++++++++++-
 3 files changed, 159 insertions(+), 43 deletions(-)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 4f692c15a2..4717fe7d2c 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -195,6 +195,7 @@ void hmp_info_mice(Monitor *mon, const QDict *qdict)
 void hmp_info_backup(Monitor *mon, const QDict *qdict)
 {
     BackupStatus *info;
+    PBSBitmapInfoList *bitmap_info;
 
     info = qmp_query_backup(NULL);
 
@@ -225,26 +226,29 @@ void hmp_info_backup(Monitor *mon, const QDict *qdict)
             // this should not happen normally
             monitor_printf(mon, "Total size: %d\n", 0);
         } else {
-            bool incremental = false;
             size_t total_or_dirty = info->total;
-            if (info->has_transferred) {
-                if (info->has_dirty && info->dirty) {
-                     if (info->dirty < info->total) {
-                        total_or_dirty = info->dirty;
-                        incremental = true;
-                    }
-                }
+            bitmap_info = qmp_query_pbs_bitmap_info(NULL);
+
+            while (bitmap_info) {
+                monitor_printf(mon, "Drive %s:\n",
+                        bitmap_info->value->drive);
+                monitor_printf(mon, "  bitmap action: %s\n",
+                        PBSBitmapAction_str(bitmap_info->value->action));
+                monitor_printf(mon, "  size: %zd\n",
+                        bitmap_info->value->size);
+                monitor_printf(mon, "  dirty: %zd\n",
+                        bitmap_info->value->dirty);
+                bitmap_info = bitmap_info->next;
             }
 
-            int per = (info->transferred * 100)/total_or_dirty;
-
-            monitor_printf(mon, "Backup mode: %s\n", incremental ? "incremental" : "full");
+            qapi_free_PBSBitmapInfoList(bitmap_info);
 
             int zero_per = (info->has_zero_bytes && info->zero_bytes) ?
                 (info->zero_bytes * 100)/info->total : 0;
             monitor_printf(mon, "Total size: %zd\n", info->total);
+            int trans_per = (info->transferred * 100)/total_or_dirty;
             monitor_printf(mon, "Transferred bytes: %zd (%d%%)\n",
-                           info->transferred, per);
+                           info->transferred, trans_per);
             monitor_printf(mon, "Zero bytes: %zd (%d%%)\n",
                            info->zero_bytes, zero_per);
 
diff --git a/pve-backup.c b/pve-backup.c
index 7c99554514..c6d8a51406 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -46,6 +46,7 @@ static struct PVEBackupState {
         size_t transferred;
         size_t reused;
         size_t zero_bytes;
+        GList *bitmap_list;
     } stat;
     int64_t speed;
     VmaWriter *vmaw;
@@ -674,7 +675,6 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
     }
 
     size_t total = 0;
-    size_t dirty = 0;
 
     l = di_list;
     while (l) {
@@ -695,18 +695,33 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
 
     uuid_generate(uuid);
 
+    qemu_mutex_lock(&backup_state.stat.lock);
+    backup_state.stat.reused = 0;
+
+    /* clear previous backup's bitmap_list */
+    if (backup_state.stat.bitmap_list) {
+        GList *bl = backup_state.stat.bitmap_list;
+        while (bl) {
+            g_free(((PBSBitmapInfo *)bl->data)->drive);
+            g_free(bl->data);
+            bl = g_list_next(bl);
+        }
+        g_list_free(backup_state.stat.bitmap_list);
+        backup_state.stat.bitmap_list = NULL;
+    }
+
     if (format == BACKUP_FORMAT_PBS) {
         if (!task->has_password) {
             error_set(task->errp, ERROR_CLASS_GENERIC_ERROR, "missing parameter 'password'");
-            goto err;
+            goto err_mutex;
         }
         if (!task->has_backup_id) {
             error_set(task->errp, ERROR_CLASS_GENERIC_ERROR, "missing parameter 'backup-id'");
-            goto err;
+            goto err_mutex;
         }
         if (!task->has_backup_time) {
             error_set(task->errp, ERROR_CLASS_GENERIC_ERROR, "missing parameter 'backup-time'");
-            goto err;
+            goto err_mutex;
         }
 
         int dump_cb_block_size = PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE; // Hardcoded (4M)
@@ -733,12 +748,12 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
             error_set(task->errp, ERROR_CLASS_GENERIC_ERROR,
                       "proxmox_backup_new failed: %s", pbs_err);
             proxmox_backup_free_error(pbs_err);
-            goto err;
+            goto err_mutex;
         }
 
         int connect_result = proxmox_backup_co_connect(pbs, task->errp);
         if (connect_result < 0)
-            goto err;
+            goto err_mutex;
 
         /* register all devices */
         l = di_list;
@@ -749,6 +764,8 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
             di->block_size = dump_cb_block_size;
 
             const char *devname = bdrv_get_device_name(di->bs);
+            PBSBitmapAction action = PBS_BITMAP_ACTION_NOT_USED;
+            size_t dirty = di->size;
 
             BdrvDirtyBitmap *bitmap = bdrv_find_dirty_bitmap(di->bs, PBS_BITMAP_NAME);
             bool expect_only_dirty = false;
@@ -757,49 +774,59 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
                 if (bitmap == NULL) {
                     bitmap = bdrv_create_dirty_bitmap(di->bs, dump_cb_block_size, PBS_BITMAP_NAME, task->errp);
                     if (!bitmap) {
-                        goto err;
+                        goto err_mutex;
                     }
+                    action = PBS_BITMAP_ACTION_NEW;
                 } else {
                     expect_only_dirty = proxmox_backup_check_incremental(pbs, devname, di->size) != 0;
                 }
 
                 if (expect_only_dirty) {
-                    dirty += bdrv_get_dirty_count(bitmap);
+                    /* track clean chunks as reused */
+                    dirty = MIN(bdrv_get_dirty_count(bitmap), di->size);
+                    backup_state.stat.reused += di->size - dirty;
+                    action = PBS_BITMAP_ACTION_USED;
                 } else {
                     /* mark entire bitmap as dirty to make full backup */
                     bdrv_set_dirty_bitmap(bitmap, 0, di->size);
-                    dirty += di->size;
+                    if (action != PBS_BITMAP_ACTION_NEW) {
+                        action = PBS_BITMAP_ACTION_INVALID;
+                    }
                 }
                 di->bitmap = bitmap;
             } else {
-                dirty += di->size;
-
                 /* after a full backup the old dirty bitmap is invalid anyway */
                 if (bitmap != NULL) {
                     bdrv_release_dirty_bitmap(bitmap);
+                    action = PBS_BITMAP_ACTION_NOT_USED_REMOVED;
                 }
             }
 
             int dev_id = proxmox_backup_co_register_image(pbs, devname, di->size, expect_only_dirty, task->errp);
             if (dev_id < 0) {
-                goto err;
+                goto err_mutex;
             }
 
             if (!(di->target = bdrv_backup_dump_create(dump_cb_block_size, di->size, pvebackup_co_dump_pbs_cb, di, task->errp))) {
-                goto err;
+                goto err_mutex;
             }
 
             di->dev_id = dev_id;
+
+            PBSBitmapInfo *info = g_malloc(sizeof(*info));
+            info->drive = g_strdup(devname);
+            info->action = action;
+            info->size = di->size;
+            info->dirty = dirty;
+            backup_state.stat.bitmap_list = g_list_append(backup_state.stat.bitmap_list, info);
         }
     } else if (format == BACKUP_FORMAT_VMA) {
-        dirty = total;
-
         vmaw = vma_writer_create(task->backup_file, uuid, &local_err);
         if (!vmaw) {
             if (local_err) {
                 error_propagate(task->errp, local_err);
             }
-            goto err;
+            goto err_mutex;
         }
 
         /* register all devices for vma writer */
@@ -809,7 +836,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
             l = g_list_next(l);
 
             if (!(di->target = bdrv_backup_dump_create(VMA_CLUSTER_SIZE, di->size, pvebackup_co_dump_vma_cb, di, task->errp))) {
-                goto err;
+                goto err_mutex;
             }
 
             const char *devname = bdrv_get_device_name(di->bs);
@@ -817,16 +844,14 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
             if (di->dev_id <= 0) {
                 error_set(task->errp, ERROR_CLASS_GENERIC_ERROR,
                           "register_stream failed");
-                goto err;
+                goto err_mutex;
             }
         }
     } else if (format == BACKUP_FORMAT_DIR) {
-        dirty = total;
-
         if (mkdir(task->backup_file, 0640) != 0) {
             error_setg_errno(task->errp, errno, "can't create directory '%s'\n",
                              task->backup_file);
-            goto err;
+            goto err_mutex;
         }
         backup_dir = task->backup_file;
 
@@ -843,18 +868,18 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
                             di->size, flags, false, &local_err);
             if (local_err) {
                 error_propagate(task->errp, local_err);
-                goto err;
+                goto err_mutex;
             }
 
             di->target = bdrv_open(di->targetfile, NULL, NULL, flags, &local_err);
             if (!di->target) {
                 error_propagate(task->errp, local_err);
-                goto err;
+                goto err_mutex;
             }
         }
     } else {
         error_set(task->errp, ERROR_CLASS_GENERIC_ERROR, "unknown backup format");
-        goto err;
+        goto err_mutex;
     }
 
 
@@ -862,7 +887,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
     if (task->has_config_file) {
         if (pvebackup_co_add_config(task->config_file, config_name, format, backup_dir,
                                     vmaw, pbs, task->errp) != 0) {
-            goto err;
+            goto err_mutex;
         }
     }
 
@@ -870,12 +895,11 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
     if (task->has_firewall_file) {
         if (pvebackup_co_add_config(task->firewall_file, firewall_name, format, backup_dir,
                                     vmaw, pbs, task->errp) != 0) {
-            goto err;
+            goto err_mutex;
         }
     }
     /* initialize global backup_state now */
-
-    qemu_mutex_lock(&backup_state.stat.lock);
+    /* note: 'reused' and 'bitmap_list' are initialized earlier */
 
     if (backup_state.stat.error) {
         error_free(backup_state.stat.error);
@@ -895,10 +919,9 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
     char *uuid_str = g_strdup(backup_state.stat.uuid_str);
 
     backup_state.stat.total = total;
-    backup_state.stat.dirty = dirty;
+    backup_state.stat.dirty = total - backup_state.stat.reused;
     backup_state.stat.transferred = 0;
     backup_state.stat.zero_bytes = 0;
-    backup_state.stat.reused = format == BACKUP_FORMAT_PBS && dirty >= total ? 0 : total - dirty;
 
     qemu_mutex_unlock(&backup_state.stat.lock);
 
@@ -915,6 +938,9 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
     task->result = uuid_info;
     return;
 
+err_mutex:
+    qemu_mutex_unlock(&backup_state.stat.lock);
+
 err:
 
     l = di_list;
@@ -1078,9 +1104,40 @@ BackupStatus *qmp_query_backup(Error **errp)
     return info;
 }
 
+PBSBitmapInfoList *qmp_query_pbs_bitmap_info(Error **errp)
+{
+    PBSBitmapInfoList *head = NULL, **p_next = &head;
+
+    qemu_mutex_lock(&backup_state.stat.lock);
+
+    GList *l = backup_state.stat.bitmap_list;
+    while (l) {
+        PBSBitmapInfo *info = (PBSBitmapInfo *)l->data;
+        l = g_list_next(l);
+
+        /* clone bitmap info to avoid auto free after QMP marshalling */
+        PBSBitmapInfo *info_ret = g_malloc0(sizeof(*info_ret));
+        info_ret->drive = g_strdup(info->drive);
+        info_ret->action = info->action;
+        info_ret->size = info->size;
+        info_ret->dirty = info->dirty;
+
+        PBSBitmapInfoList *info_list = g_malloc0(sizeof(*info_list));
+        info_list->value = info_ret;
+
+        *p_next = info_list;
+        p_next = &info_list->next;
+    }
+
+    qemu_mutex_unlock(&backup_state.stat.lock);
+
+    return head;
+}
+
 ProxmoxSupportStatus *qmp_query_proxmox_support(Error **errp)
 {
     ProxmoxSupportStatus *ret = g_malloc0(sizeof(*ret));
     ret->pbs_dirty_bitmap = true;
+    ret->query_bitmap_info = true;
     return ret;
 }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 355a184ea8..a3d54b85e2 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -875,9 +875,11 @@
 # @pbs-dirty-bitmap: True if dirty-bitmap-incremental backups to PBS are
 #                    supported.
 #
+# @query-bitmap-info: True if the 'query-pbs-bitmap-info' QMP call is supported.
+#
 ##
 { 'struct': 'ProxmoxSupportStatus',
-  'data': { 'pbs-dirty-bitmap': 'bool' } }
+  'data': { 'pbs-dirty-bitmap': 'bool', 'query-bitmap-info': 'bool' } }
 
 ##
 # @query-proxmox-support:
@@ -889,6 +891,59 @@
 ##
 { 'command': 'query-proxmox-support', 'returns': 'ProxmoxSupportStatus' }
 
+##
+# @PBSBitmapAction:
+#
+# An action taken on a dirty-bitmap when a backup job was started.
+#
+# @not-used: Bitmap mode was not enabled.
+#
+# @not-used-removed: Bitmap mode was not enabled, but a bitmap from a
+#                    previous backup still existed and was removed.
+#
+# @new: A new bitmap was attached to the drive for this backup.
+#
+# @used: An existing bitmap will be used to only backup changed data.
+#
+# @invalid: A bitmap existed, but had to be cleared since it's associated
+#           base snapshot did not match the base given for the current job or
+#           the crypt mode has changed.
+#
+##
+{ 'enum': 'PBSBitmapAction',
+  'data': ['not-used', 'not-used-removed', 'new', 'used', 'invalid'] }
+
+##
+# @PBSBitmapInfo:
+#
+# Contains information about dirty bitmaps used for each drive in a PBS backup.
+#
+# @drive: The underlying drive.
+#
+# @action: The action that was taken when the backup started.
+#
+# @size: The total size of the drive.
+#
+# @dirty: How much of the drive is considered dirty and will be backed up,
+#         or 'size' if everything will be.
+#
+##
+{ 'struct': 'PBSBitmapInfo',
+  'data': { 'drive': 'str', 'action': 'PBSBitmapAction', 'size': 'int',
+            'dirty': 'int' } }
+
+##
+# @query-pbs-bitmap-info:
+#
+# Returns information about dirty bitmaps used on the most recently started
+# backup. Returns nothing when the last backup was not using PBS or if no
+# backup occured in this session.
+#
+# Returns: @PBSBitmapInfo
+#
+##
+{ 'command': 'query-pbs-bitmap-info', 'returns': ['PBSBitmapInfo'] }
+
 ##
 # @BlockDeviceTimedStats:
 #
-- 
2.20.1

[pve-devel] [PATCH v2 qemu-server 2/5] vzdump: improve logging output with dirty bitmaps

[Stefan Reiter]

Uses the new 'query-pbs-bitmap-info' QMP call to retrieve additional
information about each drive's dirty bitmap. Returned info is also used
to calculate $target by simply adding all the dirty values (dirty is
equal to size in case the entire drive will be backed up).

"Backup is sparse" message is suppressed for PBS, since it makes little
sense (if zero chunks appear in the clean area of a bitmap, they won't
be counted, and a user is probably more interested in the 'reused' data
anyway).

Also removes the need for the hacky $first_round query-backup handling.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 PVE/VZDump/QemuServer.pm | 57 +++++++++++++++++++++++++++++++---------
 1 file changed, 45 insertions(+), 12 deletions(-)

diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 4614efc..6420bf4 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -275,14 +275,39 @@ sub bytes_to_human {
     return $num2str->($tb, $precission) . " TiB";
 };
 
+my $bitmap_action_to_human = sub {
+    my ($info) = @_;
+
+    my $action = $info->{action};
+
+    if ($action eq "not-used") {
+	return "disabled";
+    } elsif ($action eq "not-used-removed") {
+	return "disabled (old bitmap cleared)";
+    } elsif ($action eq "new") {
+	return "created new bitmap";
+    } elsif ($action eq "used") {
+	if ($info->{dirty} == 0) {
+	    return "OK, drive clean";
+	} else {
+	    my $size = bytes_to_human($info->{size});
+	    my $dirty = bytes_to_human($info->{dirty});
+	    return "OK, $dirty of $size dirty";
+	}
+    } elsif ($action eq "invalid") {
+	return "existing bitmap was invalid and has been cleared";
+    } else {
+	return "unknown";
+    }
+};
+
 my $query_backup_status_loop = sub {
-    my ($self, $vmid, $job_uuid) = @_;
+    my ($self, $vmid, $job_uuid, $pbs_features) = @_;
 
     my $starttime = time ();
     my $last_time = $starttime;
     my ($last_percent, $last_total, $last_target,  $last_zero, $last_transferred) = (-1, 0, 0, 0, 0);
     my ($transferred, $reused);
-    my $first_round = 1;
 
     my $get_mbps = sub {
 	my ($mb, $delta) = @_;
@@ -291,12 +316,26 @@ my $query_backup_status_loop = sub {
 	return bytes_to_human($bw) . "/s";
     };
 
+    my $target = 0;
+    my $has_query_bitmap = 0;
+    if (defined($pbs_features) && $pbs_features->{'query-bitmap-info'}) {
+	$has_query_bitmap = 1;
+	my $bitmap_info = mon_cmd($vmid, 'query-pbs-bitmap-info');
+	$self->loginfo("Fast incremental status:");
+	foreach my $info (@$bitmap_info) {
+	    my $text = $bitmap_action_to_human->($info);
+	    my $drive = $info->{drive};
+	    $drive =~ s/^drive-//; # for consistency
+	    $self->loginfo("$drive: $text");
+	    $target += $info->{dirty};
+	}
+    }
+
     while(1) {
 	my $status = mon_cmd($vmid, 'query-backup');
 
 	my $total = $status->{total} || 0;
-	my $dirty = $status->{dirty};
-	my $target = (defined($dirty) && $dirty < $total) ? $dirty : $total;
+	$target = $total if !$has_query_bitmap;
 	$transferred = $status->{transferred} || 0;
 	$reused = $status->{reused};
 	my $percent = $target ? int(($transferred * 100)/$target) : 0;
@@ -316,11 +355,6 @@ my $query_backup_status_loop = sub {
 	my $target_h = bytes_to_human($target);
 	my $transferred_h = bytes_to_human($transferred);
 
-	if ($first_round && $target != $total) {
-	    my $total_h = bytes_to_human($total);
-	    $self->loginfo("using fast incremental mode (dirty-bitmap), $target_h dirty of $total_h total");
-	}
-
 	my $statusline = "status: $percent% ($transferred_h of $target_h), duration $duration"
 	    .", read: $mbps_read, write: $mbps_write";
 
@@ -347,7 +381,6 @@ my $query_backup_status_loop = sub {
 	    $last_time = $ctime;
 	}
 	sleep(1);
-	$first_round = 0;
     }
 
     my $duration = time() - $starttime;
@@ -362,7 +395,7 @@ my $query_backup_status_loop = sub {
 	$self->loginfo("transferred $transferred_h in $duration seconds ($mbps)");
     }
 
-    if ($last_zero) {
+    if (!defined($pbs_features) && $last_zero) {
 	my $zero_per = $last_target ? int(($last_zero * 100)/$last_target) : 0;
 	my $zero_h = bytes_to_human($last_zero, 2);
 	$self->loginfo("Backup is sparse: ${zero_per}% ($zero_h) zero data");
@@ -490,7 +523,7 @@ sub archive_pbs {
 
 	$self->resume_vm_after_job_start($task, $vmid);
 
-	my $stat = $query_backup_status_loop->($self, $vmid, $backup_job_uuid);
+	my $stat = $query_backup_status_loop->($self, $vmid, $backup_job_uuid, $qemu_support);
 	$task->{size} = $stat->{total};
     };
     my $err = $@;
-- 
2.20.1

[pve-devel] [PATCH v2 qemu-server 3/5] vzdump: display actually uploaded chunks as 'write' speed

[Stefan Reiter]

Previously 'read' and 'write' would always show the same value, which is
of little use. Change it so 'write' excludes reused bytes, thus
displaying the actual upload speed.

$last_reused needs to be initialized to contain reused data from 'clean'
dirty bitmaps to ensure the first output line is correct.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 PVE/VZDump/QemuServer.pm | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 6420bf4..5edc62b 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -306,7 +306,7 @@ my $query_backup_status_loop = sub {
 
     my $starttime = time ();
     my $last_time = $starttime;
-    my ($last_percent, $last_total, $last_target,  $last_zero, $last_transferred) = (-1, 0, 0, 0, 0);
+    my ($last_percent, $last_total, $last_target, $last_zero, $last_transferred) = (-1, 0, 0, 0, 0);
     my ($transferred, $reused);
 
     my $get_mbps = sub {
@@ -317,6 +317,7 @@ my $query_backup_status_loop = sub {
     };
 
     my $target = 0;
+    my $last_reused = 0;
     my $has_query_bitmap = 0;
     if (defined($pbs_features) && $pbs_features->{'query-bitmap-info'}) {
 	$has_query_bitmap = 1;
@@ -328,6 +329,7 @@ my $query_backup_status_loop = sub {
 	    $drive =~ s/^drive-//; # for consistency
 	    $self->loginfo("$drive: $text");
 	    $target += $info->{dirty};
+	    $last_reused += $info->{size} - $info->{dirty};
 	}
     }
 
@@ -347,7 +349,13 @@ my $query_backup_status_loop = sub {
 	my $duration = $ctime - $starttime;
 
 	my $rbytes = $transferred - $last_transferred;
-	my $wbytes = $rbytes - ($zero - $last_zero);
+	my $wbytes;
+	if ($reused) {
+	    # reused includes zero bytes for PBS
+	    $wbytes = $rbytes - ($reused - $last_reused);
+	} else {
+	    $wbytes = $rbytes - ($zero - $last_zero);
+	}
 
 	my $timediff = ($ctime - $last_time) || 1; # fixme
 	my $mbps_read = $get_mbps->($rbytes, $timediff);
@@ -379,6 +387,7 @@ my $query_backup_status_loop = sub {
 	    $last_zero = $zero if $zero;
 	    $last_transferred = $transferred if $transferred;
 	    $last_time = $ctime;
+	    $last_reused = $reused;
 	}
 	sleep(1);
     }
-- 
2.20.1

[pve-devel] [PATCH v2 qemu-server 4/5] vzdump: log 100% percent in case $target is 0

[Stefan Reiter]

When $target is 0, that means we don't have to upload any data, in which
case we're immediately done.

Otherwise incremental backups with no changes display a really weird
  status: 0% (0.0 B of 0.0 B), duration 0, read: 0 B/s, write: 0 B/s
when they're actually done already.

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

diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 5edc62b..c919aa9 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -340,7 +340,7 @@ my $query_backup_status_loop = sub {
 	$target = $total if !$has_query_bitmap;
 	$transferred = $status->{transferred} || 0;
 	$reused = $status->{reused};
-	my $percent = $target ? int(($transferred * 100)/$target) : 0;
+	my $percent = $target ? int(($transferred * 100)/$target) : 100;
 	my $zero = $status->{'zero-bytes'} || 0;
 
 	die "got unexpected uuid\n" if !$status->{uuid} || ($status->{uuid} ne $job_uuid);
-- 
2.20.1

[pve-devel] [PATCH v2 qemu-server 5/5] vzdump: don't use dirty bitmap when VM was off

[Stefan Reiter]

There can't be a dirty bitmap when the VM was off, and if it was off we
will also shut it down after the backup, so no point in creating one.

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

diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index c919aa9..f390344 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -513,7 +513,8 @@ sub archive_pbs {
 	    $params->{encrypt} = JSON::false;
 	}
 
-	$params->{'use-dirty-bitmap'} = JSON::true if $qemu_support->{'pbs-dirty-bitmap'};
+	$params->{'use-dirty-bitmap'} = JSON::true
+	    if $qemu_support->{'pbs-dirty-bitmap'} && $self->{vm_was_running};
 
 	$params->{timeout} = 60; # give some time to connect to the backup server
 
-- 
2.20.1

[pve-devel] applied-series: [PATCH v2 0/5] improve logging for dirty bitmap PBS backups

[Thomas Lamprecht]

On 19.08.20 17:01, Stefan Reiter wrote:
> Makes the log output more useful for dirty bitmap backups and fixes some minor
> issues.
> 
> A new QMP call is introduced to avoid shoving even more data in the possibly
> often called 'query-backup' and not having to do some hacky special casing for
> the first query iteration.
> 
> The new QMP call is gated behind a new proxmox feature flag. The previous
> behaviour of having 'dirty' in 'query-backup' is retained for compatibility with
> older qemu-server versions.
> 
> 
> v2:
> * keep 'dirty' flag in query-backup
> * add patch 5
> 
> 
> qemu: Stefan Reiter (1):
>   PVE: add query-pbs-bitmap-info QMP call
> 
>  monitor/hmp-cmds.c   |  28 ++++++-----
>  pve-backup.c         | 117 ++++++++++++++++++++++++++++++++-----------
>  qapi/block-core.json |  57 ++++++++++++++++++++-
>  3 files changed, 159 insertions(+), 43 deletions(-)
> 
> qemu-server: Stefan Reiter (4):
>   vzdump: improve logging output with dirty bitmaps
>   vzdump: display actually uploaded chunks as 'write' speed
>   vzdump: log 100% percent in case $target is 0
>   vzdump: don't use dirty bitmap when VM was off
> 
>  PVE/VZDump/QemuServer.pm | 75 +++++++++++++++++++++++++++++++---------
>  1 file changed, 59 insertions(+), 16 deletions(-)
> 



applied series, thanks!