]> git.proxmox.com Git - pve-qemu.git/commitdiff
backup: improve QAPI info and remove all dirty-bitmaps on failed drive-job
authorThomas Lamprecht <t.lamprecht@proxmox.com>
Thu, 2 Jul 2020 11:03:44 +0000 (13:03 +0200)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Thu, 2 Jul 2020 11:03:49 +0000 (13:03 +0200)
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>
debian/patches/pve/0051-PVE-Backup-Add-dirty-bitmap-tracking-for-incremental.patch
qemu

index 11849219e2321711148f08f8ce4d237342e78375..94c13f826a3b4e335cbbd8c27e58b9f525e0a313 100644 (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
-
diff --git a/qemu b/qemu
index fdd76fecdde1ad444ff4deb7f1c4f7e4a1ef97d6..652ef2ebdfe202981c91739089c4bf074c919b66 160000 (submodule)
--- a/qemu
+++ b/qemu
@@ -1 +1 @@
-Subproject commit fdd76fecdde1ad444ff4deb7f1c4f7e4a1ef97d6
+Subproject commit 652ef2ebdfe202981c91739089c4bf074c919b66