debian/patches/pve/0037-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 | 167 +++++++++++++++------------------------------------
  19  1 file changed, 49 insertions(+), 118 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 -    qemu_mutex_unlock(&backup_state.backup_mutex);
 127 -
 128 -    for(;;) {
 129 -
 130 -        BlockJob *next_job = NULL;
 131 -
 132 -        qemu_mutex_lock(&backup_state.backup_mutex);
 133 -
 134 -        GList *l = backup_state.di_list;
 135 -        while (l) {
 136 -            PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
 137 -            l = g_list_next(l);
 138 +    /* it's enough to cancel one job in the transaction, the rest will follow
 139 +     * automatically */
 140 +    GList *bdi = g_list_first(backup_state.di_list);
 141 +    BlockJob *cancel_job = bdi && bdi->data ?
 142 +        ((PVEBackupDevInfo *)bdi->data)->job :
 143 +        NULL;
 144
 145 -            BlockJob *job = lookup_active_block_job(di);
 146 -            if (job != NULL) {
 147 -                next_job = job;
 148 -                break;
 149 -            }
 150 -        }
 151 +    /* ref the job before releasing the mutex, just to be safe */
 152 +    if (cancel_job) {
 153 +        job_ref(&cancel_job->job);
 154 +    }
 155
 156 -        qemu_mutex_unlock(&backup_state.backup_mutex);
 157 +    /* job_cancel_sync may enter the job, so we need to release the
 158 +     * backup_mutex to avoid deadlock */
 159 +    qemu_mutex_unlock(&backup_state.backup_mutex);
 160
 161 -        if (next_job) {
 162 -            AioContext *aio_context = next_job->job.aio_context;
 163 -            aio_context_acquire(aio_context);
 164 -            job_cancel_sync(&next_job->job, true);
 165 -            aio_context_release(aio_context);
 166 -        } else {
 167 -            break;
 168 -        }
 169 +    if (cancel_job) {
 170 +        AioContext *aio_context = cancel_job->job.aio_context;
 171 +        aio_context_acquire(aio_context);
 172 +        job_cancel_sync(&cancel_job->job, true);
 173 +        job_unref(&cancel_job->job);
 174 +        aio_context_release(aio_context);
 175      }
 176  }
 177
 178 @@ -459,51 +415,19 @@ static int coroutine_fn pvebackup_co_add_config(
 179      goto out;
 180  }
 181
 182 -bool job_should_pause(Job *job);
 183 -
 184 -static void pvebackup_run_next_job(void)
 185 -{
 186 -    assert(!qemu_in_coroutine());
 187 -
 188 -    qemu_mutex_lock(&backup_state.backup_mutex);
 189 -
 190 -    GList *l = backup_state.di_list;
 191 -    while (l) {
 192 -        PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
 193 -        l = g_list_next(l);
 194 -
 195 -        BlockJob *job = lookup_active_block_job(di);
 196 -
 197 -        if (job) {
 198 -            qemu_mutex_unlock(&backup_state.backup_mutex);
 199 -
 200 -            AioContext *aio_context = job->job.aio_context;
 201 -            aio_context_acquire(aio_context);
 202 -
 203 -            if (job_should_pause(&job->job)) {
 204 -                bool error_or_canceled = pvebackup_error_or_canceled();
 205 -                if (error_or_canceled) {
 206 -                    job_cancel_sync(&job->job, true);
 207 -                } else {
 208 -                    job_resume(&job->job);
 209 -                }
 210 -            }
 211 -            aio_context_release(aio_context);
 212 -            return;
 213 -        }
 214 -    }
 215 -
 216 -    block_on_coroutine_fn(pvebackup_co_cleanup, NULL); // no more jobs, run cleanup
 217 -
 218 -    qemu_mutex_unlock(&backup_state.backup_mutex);
 219 -}
 220 -
 221  static bool create_backup_jobs(void) {
 222
 223      assert(!qemu_in_coroutine());
 224
 225      Error *local_err = NULL;
 226
 227 +    /* create job transaction to synchronize bitmap commit and cancel all
 228 +     * jobs in case one errors */
 229 +    if (backup_state.txn) {
 230 +        job_txn_unref(backup_state.txn);
 231 +    }
 232 +    backup_state.txn = job_txn_new_seq();
 233 +
 234      BackupPerf perf = { .max_workers = 16 };
 235
 236      /* create and start all jobs (paused state) */
 237 @@ -526,7 +450,7 @@ static bool create_backup_jobs(void) {
 238          BlockJob *job = backup_job_create(
 239              NULL, di->bs, di->target, backup_state.speed, sync_mode, di->bitmap,
 240              bitmap_mode, false, NULL, &perf, BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
 241 -            JOB_DEFAULT, pvebackup_complete_cb, di, NULL, &local_err);
 242 +            JOB_DEFAULT, pvebackup_complete_cb, di, backup_state.txn, &local_err);
 243
 244          aio_context_release(aio_context);
 245
 246 @@ -538,7 +462,8 @@ static bool create_backup_jobs(void) {
 247              pvebackup_propagate_error(create_job_err);
 248              break;
 249          }
 250 -        job_start(&job->job);
 251 +
 252 +        di->job = job;
 253
 254          bdrv_unref(di->target);
 255          di->target = NULL;
 256 @@ -556,6 +481,10 @@ static bool create_backup_jobs(void) {
 257                  bdrv_unref(di->target);
 258                  di->target = NULL;
 259              }
 260 +
 261 +            if (di->job) {
 262 +                job_unref(&di->job->job);
 263 +            }
 264          }
 265      }
 266
 267 @@ -946,10 +875,6 @@ err:
 268          PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
 269          l = g_list_next(l);
 270
 271 -        if (di->bitmap) {
 272 -            bdrv_release_dirty_bitmap(di->bitmap);
 273 -        }
 274 -
 275          if (di->target) {
 276              bdrv_unref(di->target);
 277          }
 278 @@ -1038,9 +963,15 @@ UuidInfo *qmp_backup(
 279      block_on_coroutine_fn(pvebackup_co_prepare, &task);
 280
 281      if (*errp == NULL) {
 282 -        create_backup_jobs();
 283 +        bool errors = create_backup_jobs();
 284          qemu_mutex_unlock(&backup_state.backup_mutex);
 285 -        pvebackup_run_next_job();
 286 +
 287 +        if (!errors) {
 288 +            /* start the first job in the transaction
 289 +             * note: this might directly enter the job, so we need to do this
 290 +             * after unlocking the backup_mutex */
 291 +            job_txn_start_seq(backup_state.txn);
 292 +        }
 293      } else {
 294          qemu_mutex_unlock(&backup_state.backup_mutex);
 295      }