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