debian/patches/pve/0036-PVE-Backup-Use-a-transaction-to-synchronize-job-stat.patch

   1 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
   2 From: Stefan Reiter <s.reiter@proxmox.com>
   3 Date: Thu, 20 Aug 2020 14:25:00 +0200
   4 Subject: [PATCH] PVE-Backup: Use a transaction to synchronize job states
   5
   6 By using a JobTxn, we can sync dirty bitmaps only when *all* jobs were
   7 successful - meaning we don't need to remove them when the backup fails,
   8 since QEMU's BITMAP_SYNC_MODE_ON_SUCCESS will now handle that for us.
   9
  10 To keep the rate-limiting and IO impact from before, we use a sequential
  11 transaction, so drives will still be backed up one after the other.
  12
  13 Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
  14 Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
  15 [add new force parameter to job_cancel_sync calls]
  16 Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
  17 ---
  18  pve-backup.c | 169 +++++++++++++++------------------------------------
  19  1 file changed, 50 insertions(+), 119 deletions(-)
  20
  21 diff --git a/pve-backup.c b/pve-backup.c
  22 index f90abaa50a..63c686463f 100644
  23 --- a/pve-backup.c
  24 +++ b/pve-backup.c
  25 @@ -52,6 +52,7 @@ static struct PVEBackupState {
  26      VmaWriter *vmaw;
  27      ProxmoxBackupHandle *pbs;
  28      GList *di_list;
  29 +    JobTxn *txn;
  30      QemuMutex backup_mutex;
  31      CoMutex dump_callback_mutex;
  32  } backup_state;
  33 @@ -71,32 +72,12 @@ typedef struct PVEBackupDevInfo {
  34      size_t size;
  35      uint64_t block_size;
  36      uint8_t dev_id;
  37 -    bool completed;
  38      char targetfile[PATH_MAX];
  39      BdrvDirtyBitmap *bitmap;
  40      BlockDriverState *target;
  41 +    BlockJob *job;
  42  } PVEBackupDevInfo;
  43
  44 -static void pvebackup_run_next_job(void);
  45 -
  46 -static BlockJob *
  47 -lookup_active_block_job(PVEBackupDevInfo *di)
  48 -{
  49 -    if (!di->completed && di->bs) {
  50 -        for (BlockJob *job = block_job_next(NULL); job; job = block_job_next(job)) {
  51 -            if (job->job.driver->job_type != JOB_TYPE_BACKUP) {
  52 -                continue;
  53 -            }
  54 -
  55 -            BackupBlockJob *bjob = container_of(job, BackupBlockJob, common);
  56 -            if (bjob && bjob->source_bs == di->bs) {
  57 -                return job;
  58 -            }
  59 -        }
  60 -    }
  61 -    return NULL;
  62 -}
  63 -
  64  static void pvebackup_propagate_error(Error *err)
  65  {
  66      qemu_mutex_lock(&backup_state.stat.lock);
  67 @@ -272,18 +253,6 @@ static void coroutine_fn pvebackup_co_cleanup(void *unused)
  68              if (local_err != NULL) {
  69                  pvebackup_propagate_error(local_err);
  70              }
  71 -        } else {
  72 -            // on error or cancel we cannot ensure synchronization of dirty
  73 -            // bitmaps with backup server, so remove all and do full backup next
  74 -            GList *l = backup_state.di_list;
  75 -            while (l) {
  76 -                PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
  77 -                l = g_list_next(l);
  78 -
  79 -                if (di->bitmap) {
  80 -                    bdrv_release_dirty_bitmap(di->bitmap);
  81 -                }
  82 -            }
  83          }
  84
  85          proxmox_backup_disconnect(backup_state.pbs);
  86 @@ -322,8 +291,6 @@ static void pvebackup_complete_cb(void *opaque, int ret)
  87
  88      qemu_mutex_lock(&backup_state.backup_mutex);
  89
  90 -    di->completed = true;
  91 -
  92      if (ret < 0) {
  93          Error *local_err = NULL;
  94          error_setg(&local_err, "job failed with err %d - %s", ret, strerror(-ret));
  95 @@ -336,20 +303,17 @@ static void pvebackup_complete_cb(void *opaque, int ret)
  96
  97      block_on_coroutine_fn(pvebackup_complete_stream, di);
  98
  99 -    // remove self from job queue
 100 +    // remove self from job list
 101      backup_state.di_list = g_list_remove(backup_state.di_list, di);
 102
 103 -    if (di->bitmap && ret < 0) {
 104 -        // on error or cancel we cannot ensure synchronization of dirty
 105 -        // bitmaps with backup server, so remove all and do full backup next
 106 -        bdrv_release_dirty_bitmap(di->bitmap);
 107 -    }
 108 -
 109      g_free(di);
 110
 111 -    qemu_mutex_unlock(&backup_state.backup_mutex);
 112 +    /* call cleanup if we're the last job */
 113 +    if (!g_list_first(backup_state.di_list)) {
 114 +        block_on_coroutine_fn(pvebackup_co_cleanup, NULL);
 115 +    }
 116
 117 -    pvebackup_run_next_job();
 118 +    qemu_mutex_unlock(&backup_state.backup_mutex);
 119  }
 120
 121  static void pvebackup_cancel(void)
 122 @@ -371,36 +335,28 @@ static void pvebackup_cancel(void)
 123          proxmox_backup_abort(backup_state.pbs, "backup canceled");
 124      }
 125
 126 +    /* it's enough to cancel one job in the transaction, the rest will follow
 127 +     * automatically */
 128 +    GList *bdi = g_list_first(backup_state.di_list);
 129 +    BlockJob *cancel_job = bdi && bdi->data ?
 130 +        ((PVEBackupDevInfo *)bdi->data)->job :
 131 +        NULL;
 132 +
 133 +    /* ref the job before releasing the mutex, just to be safe */
 134 +    if (cancel_job) {
 135 +        job_ref(&cancel_job->job);
 136 +    }
 137 +
 138 +    /* job_cancel_sync may enter the job, so we need to release the
 139 +     * backup_mutex to avoid deadlock */
 140      qemu_mutex_unlock(&backup_state.backup_mutex);
 141
 142 -    for(;;) {
 143 -
 144 -        BlockJob *next_job = NULL;
 145 -
 146 -        qemu_mutex_lock(&backup_state.backup_mutex);
 147 -
 148 -        GList *l = backup_state.di_list;
 149 -        while (l) {
 150 -            PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
 151 -            l = g_list_next(l);
 152 -
 153 -            BlockJob *job = lookup_active_block_job(di);
 154 -            if (job != NULL) {
 155 -                next_job = job;
 156 -                break;
 157 -            }
 158 -        }
 159 -
 160 -        qemu_mutex_unlock(&backup_state.backup_mutex);
 161 -
 162 -        if (next_job) {
 163 -            AioContext *aio_context = next_job->job.aio_context;
 164 -            aio_context_acquire(aio_context);
 165 -            job_cancel_sync(&next_job->job, true);
 166 -            aio_context_release(aio_context);
 167 -        } else {
 168 -            break;
 169 -        }
 170 +    if (cancel_job) {
 171 +        AioContext *aio_context = cancel_job->job.aio_context;
 172 +        aio_context_acquire(aio_context);
 173 +        job_cancel_sync(&cancel_job->job, true);
 174 +        job_unref(&cancel_job->job);
 175 +        aio_context_release(aio_context);
 176      }
 177  }
 178
 179 @@ -459,51 +415,19 @@ static int coroutine_fn pvebackup_co_add_config(
 180      goto out;
 181  }
 182
 183 -bool job_should_pause(Job *job);
 184 -
 185 -static void pvebackup_run_next_job(void)
 186 -{
 187 -    assert(!qemu_in_coroutine());
 188 -
 189 -    qemu_mutex_lock(&backup_state.backup_mutex);
 190 -
 191 -    GList *l = backup_state.di_list;
 192 -    while (l) {
 193 -        PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
 194 -        l = g_list_next(l);
 195 -
 196 -        BlockJob *job = lookup_active_block_job(di);
 197 -
 198 -        if (job) {
 199 -            qemu_mutex_unlock(&backup_state.backup_mutex);
 200 -
 201 -            AioContext *aio_context = job->job.aio_context;
 202 -            aio_context_acquire(aio_context);
 203 -
 204 -            if (job_should_pause(&job->job)) {
 205 -                bool error_or_canceled = pvebackup_error_or_canceled();
 206 -                if (error_or_canceled) {
 207 -                    job_cancel_sync(&job->job, true);
 208 -                } else {
 209 -                    job_resume(&job->job);
 210 -                }
 211 -            }
 212 -            aio_context_release(aio_context);
 213 -            return;
 214 -        }
 215 -    }
 216 -
 217 -    block_on_coroutine_fn(pvebackup_co_cleanup, NULL); // no more jobs, run cleanup
 218 -
 219 -    qemu_mutex_unlock(&backup_state.backup_mutex);
 220 -}
 221 -
 222  static bool create_backup_jobs(void) {
 223
 224      assert(!qemu_in_coroutine());
 225
 226      Error *local_err = NULL;
 227
 228 +    /* create job transaction to synchronize bitmap commit and cancel all
 229 +     * jobs in case one errors */
 230 +    if (backup_state.txn) {
 231 +        job_txn_unref(backup_state.txn);
 232 +    }
 233 +    backup_state.txn = job_txn_new_seq();
 234 +
 235      BackupPerf perf = { .max_workers = 16 };
 236
 237      /* create and start all jobs (paused state) */
 238 @@ -526,7 +450,7 @@ static bool create_backup_jobs(void) {
 239          BlockJob *job = backup_job_create(
 240              NULL, di->bs, di->target, backup_state.speed, sync_mode, di->bitmap,
 241              bitmap_mode, false, NULL, &perf, BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
 242 -            JOB_DEFAULT, pvebackup_complete_cb, di, NULL, &local_err);
 243 +            JOB_DEFAULT, pvebackup_complete_cb, di, backup_state.txn, &local_err);
 244
 245          aio_context_release(aio_context);
 246
 247 @@ -538,7 +462,8 @@ static bool create_backup_jobs(void) {
 248              pvebackup_propagate_error(create_job_err);
 249              break;
 250          }
 251 -        job_start(&job->job);
 252 +
 253 +        di->job = job;
 254
 255          bdrv_unref(di->target);
 256          di->target = NULL;
 257 @@ -556,6 +481,10 @@ static bool create_backup_jobs(void) {
 258                  bdrv_unref(di->target);
 259                  di->target = NULL;
 260              }
 261 +
 262 +            if (di->job) {
 263 +                job_unref(&di->job->job);
 264 +            }
 265          }
 266      }
 267
 268 @@ -946,10 +875,6 @@ err:
 269          PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
 270          l = g_list_next(l);
 271
 272 -        if (di->bitmap) {
 273 -            bdrv_release_dirty_bitmap(di->bitmap);
 274 -        }
 275 -
 276          if (di->target) {
 277              bdrv_unref(di->target);
 278          }
 279 @@ -1038,9 +963,15 @@ UuidInfo *qmp_backup(
 280      block_on_coroutine_fn(pvebackup_co_prepare, &task);
 281
 282      if (*errp == NULL) {
 283 -        create_backup_jobs();
 284 +        bool errors = create_backup_jobs();
 285          qemu_mutex_unlock(&backup_state.backup_mutex);
 286 -        pvebackup_run_next_job();
 287 +
 288 +        if (!errors) {
 289 +            /* start the first job in the transaction
 290 +             * note: this might directly enter the job, so we need to do this
 291 +             * after unlocking the backup_mutex */
 292 +            job_txn_start_seq(backup_state.txn);
 293 +        }
 294      } else {
 295          qemu_mutex_unlock(&backup_state.backup_mutex);
 296      }