backup: improve QAPI info and remove all dirty-bitmaps on failed drive-job

effectively two commits merged as one:
https://pve.proxmox.com/pipermail/pve-devel/2020-July/044185.html
https://pve.proxmox.com/pipermail/pve-devel/2020-July/044194.html

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht 2020-07-02 13:03:44 +02:00
parent 0943af81a6
commit 20be7fa0a0
2 changed files with 223 additions and 22 deletions

View File

@ -11,20 +11,25 @@ previous index for everything it doesn't receive if reuse_index is true.
On error or cancellation, remove all dirty bitmaps to ensure
consistency.
Add PBS/incremental specific information to query backup info QMP and
HMP commands.
Only supported for PBS backups.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
block/monitor/block-hmp-cmds.c | 1 +
monitor/hmp-cmds.c | 45 ++++++++++++----
proxmox-backup-client.c | 3 +-
proxmox-backup-client.h | 1 +
pve-backup.c | 63 +++++++++++++++++++++++++++++++---
qapi/block-core.json | 3 ++
5 files changed, 65 insertions(+), 6 deletions(-)
pve-backup.c | 95 ++++++++++++++++++++++++++++++----
qapi/block-core.json | 12 ++++-
6 files changed, 134 insertions(+), 23 deletions(-)
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 4f03881286..0bc855132a 100644
index d485c3ac79..fdc85a5c0e 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -1038,6 +1038,7 @@ void hmp_backup(Monitor *mon, const QDict *qdict)
@ -35,6 +40,64 @@ index 4f03881286..0bc855132a 100644
true, dir ? BACKUP_FORMAT_DIR : BACKUP_FORMAT_VMA,
false, NULL, false, NULL, !!devlist,
devlist, qdict_haskey(qdict, "speed"), speed, &error);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 7fd59b1c22..4f692c15a2 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -218,19 +218,42 @@ void hmp_info_backup(Monitor *mon, const QDict *qdict)
monitor_printf(mon, "End time: %s", ctime(&info->end_time));
}
- int per = (info->has_total && info->total &&
- info->has_transferred && info->transferred) ?
- (info->transferred * 100)/info->total : 0;
- int zero_per = (info->has_total && info->total &&
- info->has_zero_bytes && info->zero_bytes) ?
- (info->zero_bytes * 100)/info->total : 0;
monitor_printf(mon, "Backup file: %s\n", info->backup_file);
monitor_printf(mon, "Backup uuid: %s\n", info->uuid);
- monitor_printf(mon, "Total size: %zd\n", info->total);
- monitor_printf(mon, "Transferred bytes: %zd (%d%%)\n",
- info->transferred, per);
- monitor_printf(mon, "Zero bytes: %zd (%d%%)\n",
- info->zero_bytes, zero_per);
+
+ if (!(info->has_total && info->total)) {
+ // 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;
+ }
+ }
+ }
+
+ int per = (info->transferred * 100)/total_or_dirty;
+
+ monitor_printf(mon, "Backup mode: %s\n", incremental ? "incremental" : "full");
+
+ 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);
+ monitor_printf(mon, "Transferred bytes: %zd (%d%%)\n",
+ info->transferred, per);
+ monitor_printf(mon, "Zero bytes: %zd (%d%%)\n",
+ info->zero_bytes, zero_per);
+
+ if (info->has_reused) {
+ int reused_per = (info->reused * 100)/total_or_dirty;
+ monitor_printf(mon, "Reused bytes: %zd (%d%%)\n",
+ info->reused, reused_per);
+ }
+ }
}
qapi_free_BackupStatus(info);
diff --git a/proxmox-backup-client.c b/proxmox-backup-client.c
index b7bc7f2574..0e9c584701 100644
--- a/proxmox-backup-client.c
@ -69,7 +132,7 @@ index b311bf8de8..20fd6b1719 100644
diff --git a/pve-backup.c b/pve-backup.c
index bb917ee972..61a8b4d2a4 100644
index bb917ee972..3a71270213 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -28,6 +28,8 @@
@ -81,7 +144,17 @@ index bb917ee972..61a8b4d2a4 100644
static struct PVEBackupState {
struct {
// Everithing accessed from qmp_backup_query command is protected using lock
@@ -66,6 +68,7 @@ typedef struct PVEBackupDevInfo {
@@ -39,7 +41,9 @@ 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;
} stat;
int64_t speed;
@@ -66,6 +70,7 @@ typedef struct PVEBackupDevInfo {
uint8_t dev_id;
bool completed;
char targetfile[PATH_MAX];
@ -89,7 +162,45 @@ index bb917ee972..61a8b4d2a4 100644
BlockDriverState *target;
} PVEBackupDevInfo;
@@ -248,6 +251,18 @@ static void coroutine_fn pvebackup_co_cleanup(void *unused)
@@ -105,11 +110,12 @@ static bool pvebackup_error_or_canceled(void)
return error_or_canceled;
}
-static void pvebackup_add_transfered_bytes(size_t transferred, size_t zero_bytes)
+static void pvebackup_add_transfered_bytes(size_t transferred, size_t zero_bytes, size_t reused)
{
qemu_mutex_lock(&backup_state.stat.lock);
backup_state.stat.zero_bytes += zero_bytes;
backup_state.stat.transferred += transferred;
+ backup_state.stat.reused += reused;
qemu_mutex_unlock(&backup_state.stat.lock);
}
@@ -148,7 +154,8 @@ pvebackup_co_dump_pbs_cb(
pvebackup_propagate_error(local_err);
return pbs_res;
} else {
- pvebackup_add_transfered_bytes(size, !buf ? size : 0);
+ size_t reused = (pbs_res == 0) ? size : 0;
+ pvebackup_add_transfered_bytes(size, !buf ? size : 0, reused);
}
return size;
@@ -208,11 +215,11 @@ pvebackup_co_dump_vma_cb(
} else {
if (remaining >= VMA_CLUSTER_SIZE) {
assert(ret == VMA_CLUSTER_SIZE);
- pvebackup_add_transfered_bytes(VMA_CLUSTER_SIZE, zero_bytes);
+ pvebackup_add_transfered_bytes(VMA_CLUSTER_SIZE, zero_bytes, 0);
remaining -= VMA_CLUSTER_SIZE;
} else {
assert(ret == remaining);
- pvebackup_add_transfered_bytes(remaining, zero_bytes);
+ pvebackup_add_transfered_bytes(remaining, zero_bytes, 0);
remaining = 0;
}
}
@@ -248,6 +255,18 @@ static void coroutine_fn pvebackup_co_cleanup(void *unused)
if (local_err != NULL) {
pvebackup_propagate_error(local_err);
}
@ -108,7 +219,20 @@ index bb917ee972..61a8b4d2a4 100644
}
proxmox_backup_disconnect(backup_state.pbs);
@@ -470,12 +485,18 @@ static bool create_backup_jobs(void) {
@@ -303,6 +322,12 @@ static void pvebackup_complete_cb(void *opaque, int ret)
// remove self from job queue
backup_state.di_list = g_list_remove(backup_state.di_list, di);
+ if (di->bitmap && ret < 0) {
+ // on error or cancel we cannot ensure synchronization of dirty
+ // bitmaps with backup server, so remove all and do full backup next
+ bdrv_release_dirty_bitmap(di->bitmap);
+ }
+
g_free(di);
qemu_mutex_unlock(&backup_state.backup_mutex);
@@ -470,12 +495,18 @@ static bool create_backup_jobs(void) {
assert(di->target != NULL);
@ -129,7 +253,7 @@ index bb917ee972..61a8b4d2a4 100644
JOB_DEFAULT, pvebackup_complete_cb, di, 1, NULL, &local_err);
aio_context_release(aio_context);
@@ -526,6 +547,8 @@ typedef struct QmpBackupTask {
@@ -526,6 +557,8 @@ typedef struct QmpBackupTask {
const char *fingerprint;
bool has_fingerprint;
int64_t backup_time;
@ -138,7 +262,15 @@ index bb917ee972..61a8b4d2a4 100644
bool has_format;
BackupFormat format;
bool has_config_file;
@@ -658,6 +681,8 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
@@ -621,6 +654,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
}
size_t total = 0;
+ size_t dirty = 0;
l = di_list;
while (l) {
@@ -658,6 +692,8 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
int dump_cb_block_size = PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE; // Hardcoded (4M)
firewall_name = "fw.conf";
@ -147,7 +279,7 @@ index bb917ee972..61a8b4d2a4 100644
char *pbs_err = NULL;
pbs = proxmox_backup_new(
task->backup_file,
@@ -677,7 +702,8 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
@@ -677,7 +713,8 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
goto err;
}
@ -157,7 +289,7 @@ index bb917ee972..61a8b4d2a4 100644
goto err;
/* register all devices */
@@ -688,9 +714,29 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
@@ -688,9 +725,32 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
const char *devname = bdrv_get_device_name(di->bs);
@ -174,11 +306,14 @@ index bb917ee972..61a8b4d2a4 100644
+ }
+ /* mark entire bitmap as dirty to make full backup first */
+ bdrv_set_dirty_bitmap(bitmap, 0, di->size);
+ dirty += di->size;
+ } else {
+ use_incremental = true;
+ dirty += bdrv_get_dirty_count(bitmap);
+ }
+ di->bitmap = bitmap;
+ } else if (bitmap != NULL) {
+ dirty += di->size;
+ bdrv_release_dirty_bitmap(bitmap);
+ }
+
@ -189,7 +324,36 @@ index bb917ee972..61a8b4d2a4 100644
if (!(di->target = bdrv_backup_dump_create(dump_cb_block_size, di->size, pvebackup_co_dump_pbs_cb, di, task->errp))) {
goto err;
@@ -823,6 +869,10 @@ err:
@@ -699,6 +759,8 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
di->dev_id = dev_id;
}
} else if (format == BACKUP_FORMAT_VMA) {
+ dirty = total;
+
vmaw = vma_writer_create(task->backup_file, uuid, &local_err);
if (!vmaw) {
if (local_err) {
@@ -726,6 +788,8 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
}
}
} 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);
@@ -798,8 +862,10 @@ 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 = 0;
qemu_mutex_unlock(&backup_state.stat.lock);
@@ -823,6 +889,10 @@ err:
PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
l = g_list_next(l);
@ -200,7 +364,7 @@ index bb917ee972..61a8b4d2a4 100644
if (di->target) {
bdrv_unref(di->target);
}
@@ -864,6 +914,7 @@ UuidInfo *qmp_backup(
@@ -864,6 +934,7 @@ UuidInfo *qmp_backup(
bool has_fingerprint, const char *fingerprint,
bool has_backup_id, const char *backup_id,
bool has_backup_time, int64_t backup_time,
@ -208,7 +372,7 @@ index bb917ee972..61a8b4d2a4 100644
bool has_format, BackupFormat format,
bool has_config_file, const char *config_file,
bool has_firewall_file, const char *firewall_file,
@@ -882,6 +933,8 @@ UuidInfo *qmp_backup(
@@ -882,6 +953,8 @@ UuidInfo *qmp_backup(
.backup_id = backup_id,
.has_backup_time = has_backup_time,
.backup_time = backup_time,
@ -217,11 +381,51 @@ index bb917ee972..61a8b4d2a4 100644
.has_format = has_format,
.format = format,
.has_config_file = has_config_file,
@@ -950,10 +1023,14 @@ 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;
info->transferred = backup_state.stat.transferred;
+ info->has_reused = true;
+ info->reused = backup_state.stat.reused;
qemu_mutex_unlock(&backup_state.stat.lock);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 8bdbccb397..f693bebdb4 100644
index 8bdbccb397..8ffff7aaab 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -815,6 +815,8 @@
@@ -757,8 +757,13 @@
#
# @total: total amount of bytes involved in the backup process
#
+# @dirty: with incremental mode, 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.
+#
# @zero-bytes: amount of 'zero' bytes detected.
#
# @start-time: time (epoch) when backup job started.
@@ -771,8 +776,8 @@
#
##
{ 'struct': 'BackupStatus',
- 'data': {'*status': 'str', '*errmsg': 'str', '*total': 'int',
- '*transferred': 'int', '*zero-bytes': 'int',
+ 'data': {'*status': 'str', '*errmsg': 'str', '*total': 'int', '*dirty': 'int',
+ '*transferred': 'int', '*zero-bytes': 'int', '*reused': 'int',
'*start-time': 'int', '*end-time': 'int',
'*backup-file': 'str', '*uuid': 'str' } }
@@ -815,6 +820,8 @@
#
# @backup-time: backup timestamp (Unix epoch, required for format 'pbs')
#
@ -230,7 +434,7 @@ index 8bdbccb397..f693bebdb4 100644
# Returns: the uuid of the backup job
#
##
@@ -825,6 +827,7 @@
@@ -825,6 +832,7 @@
'*fingerprint': 'str',
'*backup-id': 'str',
'*backup-time': 'int',
@ -238,6 +442,3 @@ index 8bdbccb397..f693bebdb4 100644
'*format': 'BackupFormat',
'*config-file': 'str',
'*firewall-file': 'str',
--
2.20.1

2
qemu

@ -1 +1 @@
Subproject commit fdd76fecdde1ad444ff4deb7f1c4f7e4a1ef97d6
Subproject commit 652ef2ebdfe202981c91739089c4bf074c919b66