* [pve-devel] [PATCH qemu 1/4] PVE: add query-pbs-bitmap-info QMP call
2020-08-13 12:03 [pve-devel] [PATCH 0/4] improve logging for dirty bitmap PBS backups Stefan Reiter
@ 2020-08-13 12:03 ` Stefan Reiter
2020-08-19 9:41 ` Thomas Lamprecht
2020-08-13 12:03 ` [pve-devel] [PATCH qemu-server 2/4] vzdump: improve logging output with dirty bitmaps Stefan Reiter
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Stefan Reiter @ 2020-08-13 12:03 UTC (permalink / raw)
To: pve-devel
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 | 119 +++++++++++++++++++++++++++++++------------
qapi/block-core.json | 62 ++++++++++++++++++++--
3 files changed, 159 insertions(+), 50 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..f4eaea3d52 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -42,10 +42,10 @@ static struct PVEBackupState {
uuid_t uuid;
char uuid_str[37];
size_t total;
- size_t dirty;
size_t transferred;
size_t reused;
size_t zero_bytes;
+ GList *bitmap_list;
} stat;
int64_t speed;
VmaWriter *vmaw;
@@ -674,7 +674,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 +694,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 +747,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 +763,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 +773,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 +835,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 +843,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 +867,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 +886,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 +894,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 +918,8 @@ 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.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 +936,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;
@@ -1064,8 +1088,6 @@ BackupStatus *qmp_query_backup(Error **errp)
info->has_total = true;
info->total = backup_state.stat.total;
- info->has_dirty = true;
- info->dirty = backup_state.stat.dirty;
info->has_zero_bytes = true;
info->zero_bytes = backup_state.stat.zero_bytes;
info->has_transferred = true;
@@ -1078,9 +1100,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..fb3d38e9c8 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -757,9 +757,6 @@
#
# @total: total amount of bytes involved in the backup process
#
-# @dirty: with incremental mode (PBS) this is the amount of bytes involved
-# in the backup process which are marked dirty.
-#
# @transferred: amount of bytes already backed up.
#
# @reused: amount of bytes reused due to deduplication.
@@ -776,7 +773,7 @@
#
##
{ 'struct': 'BackupStatus',
- 'data': {'*status': 'str', '*errmsg': 'str', '*total': 'int', '*dirty': 'int',
+ 'data': {'*status': 'str', '*errmsg': 'str', '*total': 'int',
'*transferred': 'int', '*zero-bytes': 'int', '*reused': 'int',
'*start-time': 'int', '*end-time': 'int',
'*backup-file': 'str', '*uuid': 'str' } }
@@ -875,9 +872,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 +888,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
^ permalink raw reply [flat|nested] 6+ messages in thread* [pve-devel] [PATCH qemu-server 2/4] vzdump: improve logging output with dirty bitmaps
2020-08-13 12:03 [pve-devel] [PATCH 0/4] improve logging for dirty bitmap PBS backups Stefan Reiter
2020-08-13 12:03 ` [pve-devel] [PATCH qemu 1/4] PVE: add query-pbs-bitmap-info QMP call Stefan Reiter
@ 2020-08-13 12:03 ` Stefan Reiter
2020-08-13 12:03 ` [pve-devel] [PATCH qemu-server 3/4] vzdump: display actually uploaded chunks as 'write' speed Stefan Reiter
2020-08-13 12:03 ` [pve-devel] [PATCH qemu-server 4/4] vzdump: log 100% percent in case $target is 0 Stefan Reiter
3 siblings, 0 replies; 6+ messages in thread
From: Stefan Reiter @ 2020-08-13 12:03 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread