backup: add various cleanups

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht 2019-10-24 08:49:05 +02:00
parent 5a98917c4b
commit a5db0e22d7
4 changed files with 266 additions and 0 deletions

View File

@ -0,0 +1,123 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Dietmar Maurer <dietmar@proxmox.com>
Date: Thu, 24 Oct 2019 08:06:50 +0200
Subject: [PATCH] backup_job_create: pass cluster size for dump
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
block/backup.c | 8 +++++++-
block/replication.c | 2 +-
blockdev.c | 10 ++++++----
include/block/block_int.h | 4 ++++
4 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 5240f71bb5..2ccec79db6 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -579,6 +579,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
BlockdevOnError on_target_error,
int creation_flags,
BackupDumpFunc *dump_cb,
+ int dump_cb_block_size,
BlockCompletionFunc *cb, void *opaque,
int pause_count,
JobTxn *txn, Error **errp)
@@ -649,7 +650,12 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
goto error;
}
- cluster_size = backup_calculate_cluster_size(target ? target : bs, errp);
+ if (target) {
+ cluster_size = backup_calculate_cluster_size(target, errp);
+ } else {
+ cluster_size = dump_cb_block_size;
+ }
+
if (cluster_size < 0) {
goto error;
}
diff --git a/block/replication.c b/block/replication.c
index e85c62ba9c..a2ad512251 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -543,7 +543,7 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
0, MIRROR_SYNC_MODE_NONE, NULL, false,
BLOCKDEV_ON_ERROR_REPORT,
BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL,
- NULL,
+ NULL, 0,
backup_job_completed, bs, 0, NULL, &local_err);
if (local_err) {
error_propagate(errp, local_err);
diff --git a/blockdev.c b/blockdev.c
index 7e9241cf42..6d16043131 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3524,6 +3524,7 @@ static void coroutine_fn pvebackup_co_start(void *opaque)
GList *l;
UuidInfo *uuid_info;
BlockJob *job;
+ int dump_cb_block_size = -1;
if (!backup_state.backup_mutex_initialized) {
qemu_co_mutex_init(&backup_state.backup_mutex);
@@ -3611,6 +3612,7 @@ static void coroutine_fn pvebackup_co_start(void *opaque)
uuid_generate(uuid);
if (format == BACKUP_FORMAT_VMA) {
+ dump_cb_block_size = VMA_CLUSTER_SIZE;
vmaw = vma_writer_create(task->backup_file, uuid, &local_err);
if (!vmaw) {
if (local_err) {
@@ -3718,8 +3720,8 @@ static void coroutine_fn pvebackup_co_start(void *opaque)
l = g_list_next(l);
job = backup_job_create(NULL, di->bs, di->target, backup_state.speed, MIRROR_SYNC_MODE_FULL, NULL,
false, BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
- JOB_DEFAULT, pvebackup_co_dump_cb, pvebackup_complete_cb, di,
- 1, NULL, &local_err);
+ JOB_DEFAULT, pvebackup_co_dump_cb, dump_cb_block_size,
+ pvebackup_complete_cb, di, 1, NULL, &local_err);
if (!job || local_err != NULL) {
error_setg(&backup_state.error, "backup_job_create failed");
break;
@@ -4284,7 +4286,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
backup->sync, bmap, backup->compress,
backup->on_source_error, backup->on_target_error,
- job_flags, NULL, NULL, NULL, 0, txn, &local_err);
+ job_flags, NULL, 0, NULL, NULL, 0, txn, &local_err);
bdrv_unref(target_bs);
if (local_err != NULL) {
error_propagate(errp, local_err);
@@ -4394,7 +4396,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
backup->sync, bmap, backup->compress,
backup->on_source_error, backup->on_target_error,
- job_flags, NULL, NULL, NULL, 0, txn, &local_err);
+ job_flags, NULL, 0, NULL, NULL, 0, txn, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index fd1828cd70..0ac312b359 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1144,6 +1144,9 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
* @on_target_error: The action to take upon error writing to the target.
* @creation_flags: Flags that control the behavior of the Job lifetime.
* See @BlockJobCreateFlags
+ * @dump_cb: Callback for PVE backup code. Called for each data block when
+ * target is NULL.
+ * @dump_cb_block_size: The minimum block size expected by dump_cb.
* @cb: Completion function for the job.
* @opaque: Opaque pointer value passed to @cb.
* @txn: Transaction that this job is part of (may be NULL).
@@ -1160,6 +1163,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
BlockdevOnError on_target_error,
int creation_flags,
BackupDumpFunc *dump_cb,
+ int dump_cb_block_size,
BlockCompletionFunc *cb, void *opaque,
int pause_count,
JobTxn *txn, Error **errp);

View File

@ -0,0 +1,31 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Dietmar Maurer <dietmar@proxmox.com>
Date: Thu, 24 Oct 2019 08:06:51 +0200
Subject: [PATCH] avoid calling dump_cb with NULL data pointer for small/last
cluster
The last block of a backup may be smaller than cluster_size.
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
block/backup.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/block/backup.c b/block/backup.c
index 2ccec79db6..cc20d77b9f 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -133,7 +133,12 @@ static int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob *job,
if (qemu_iovec_is_zero(&qiov)) {
if (job->dump_cb) {
- ret = job->dump_cb(job->common.job.opaque, job->target, start, qiov.size, NULL);
+ if (qiov.size == job->cluster_size) {
+ // Note: pass NULL to indicate that we want to write [0u8; cluster_size]
+ ret = job->dump_cb(job->common.job.opaque, job->target, start, qiov.size, NULL);
+ } else {
+ ret = job->dump_cb(job->common.job.opaque, job->target, start, qiov.size, *bounce_buffer);
+ }
} else {
ret = blk_co_pwrite_zeroes(job->target, start,
qiov.size, write_flags | BDRV_REQ_MAY_UNMAP);

View File

@ -0,0 +1,109 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Dietmar Maurer <dietmar@proxmox.com>
Date: Thu, 24 Oct 2019 08:06:52 +0200
Subject: [PATCH] rename config_to_vma into pvebackup_co_add_config
- mark it with coroutine_fn
- add an additional parameter 'name'
- return -1 on error (instead of 1)
- code cleanup
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
blockdev.c | 40 ++++++++++++++++++++++++++--------------
1 file changed, 26 insertions(+), 14 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 6d16043131..786921da0a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3416,10 +3416,16 @@ void qmp_backup_cancel(Error **errp)
block_on_coroutine_fn(pvebackup_co_cancel, NULL);
}
-static int config_to_vma(const char *file, BackupFormat format,
- const char *backup_dir, VmaWriter *vmaw,
- Error **errp)
+static int coroutine_fn pvebackup_co_add_config(
+ const char *file,
+ const char *name,
+ BackupFormat format,
+ const char *backup_dir,
+ VmaWriter *vmaw,
+ Error **errp)
{
+ int res = 0;
+
char *cdata = NULL;
gsize clen = 0;
GError *err = NULL;
@@ -3429,28 +3435,30 @@ static int config_to_vma(const char *file, BackupFormat format,
}
char *basename = g_path_get_basename(file);
+ if (name == NULL) name = basename;
if (format == BACKUP_FORMAT_VMA) {
- if (vma_writer_add_config(vmaw, basename, cdata, clen) != 0) {
+ if (vma_writer_add_config(vmaw, name, cdata, clen) != 0) {
error_setg(errp, "unable to add %s config data to vma archive", file);
- g_free(cdata);
- g_free(basename);
- return 1;
+ goto err;
}
} else if (format == BACKUP_FORMAT_DIR) {
char config_path[PATH_MAX];
- snprintf(config_path, PATH_MAX, "%s/%s", backup_dir, basename);
+ snprintf(config_path, PATH_MAX, "%s/%s", backup_dir, name);
if (!g_file_set_contents(config_path, cdata, clen, &err)) {
error_setg(errp, "unable to write config file '%s'", config_path);
- g_free(cdata);
- g_free(basename);
- return 1;
+ goto err;
}
}
+ out:
g_free(basename);
g_free(cdata);
- return 0;
+ return res;
+
+ err:
+ res = -1;
+ goto out;
}
bool job_should_pause(Job *job);
@@ -3526,6 +3534,9 @@ static void coroutine_fn pvebackup_co_start(void *opaque)
BlockJob *job;
int dump_cb_block_size = -1;
+ const char *config_name = "qemu-server.conf";
+ const char *firewall_name = "qemu-server.fw";
+
if (!backup_state.backup_mutex_initialized) {
qemu_co_mutex_init(&backup_state.backup_mutex);
backup_state.backup_mutex_initialized = true;
@@ -3670,16 +3681,17 @@ static void coroutine_fn pvebackup_co_start(void *opaque)
goto err;
}
+
/* add configuration file to archive */
if (task->has_config_file) {
- if (config_to_vma(task->config_file, format, backup_dir, vmaw, task->errp) != 0) {
+ if (pvebackup_co_add_config(task->config_file, config_name, format, backup_dir, vmaw, task->errp) != 0) {
goto err;
}
}
/* add firewall file to archive */
if (task->has_firewall_file) {
- if (config_to_vma(task->firewall_file, format, backup_dir, vmaw, task->errp) != 0) {
+ if (pvebackup_co_add_config(task->firewall_file, firewall_name, format, backup_dir, vmaw, task->errp) != 0) {
goto err;
}
}

View File

@ -33,3 +33,6 @@ pve/0031-PVE-bug-fix-1071-vma-writer.c-use-correct-AioContext.patch
pve/0032-qmp_backup-run-backup-related-code-inside-coroutines.patch
pve/0033-qmp_backup-use-a-CoMutex-to-protect-access-to-backup.patch
pve/0034-vma_writer_close-avoid-call-to-aio_poll-acquire-flus.patch
pve/0035-backup_job_create-pass-cluster-size-for-dump.patch
pve/0036-avoid-calling-dump_cb-with-NULL-data-pointer-for-sma.patch
pve/0037-rename-config_to_vma-into-pvebackup_co_add_config.patch