[pbs-devel] [PATCH v2 qemu] PVE: handle PBS write callback with big blocks correctly

[pbs-devel] [PATCH v2 qemu] PVE: handle PBS write callback with big blocks correctly

[Stefan Reiter]

Under certain conditions QEMU will push more than the given blocksize
into the callback at once. Handle it like VMA does, by iterating the
data until all is written.

The block size is stored per backup device to be used in the callback.
This avoids relying on PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE, in case it is
made configurable in the future.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---

v2:
* save chunk size explicitly to avoid depending on
  PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE

 pve-backup.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/pve-backup.c b/pve-backup.c
index 77eb475563..40d8136f1a 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -67,6 +67,7 @@ opts_init(pvebackup_init);
 typedef struct PVEBackupDevInfo {
     BlockDriverState *bs;
     size_t size;
+    uint64_t block_size;
     uint8_t dev_id;
     bool completed;
     char targetfile[PATH_MAX];
@@ -147,17 +148,28 @@ pvebackup_co_dump_pbs_cb(
         return -1;
     }
 
-    pbs_res = proxmox_backup_co_write_data(backup_state.pbs, di->dev_id, buf, start, size, &local_err);
-    qemu_co_mutex_unlock(&backup_state.dump_callback_mutex);
+    uint64_t transferred = 0;
+    uint64_t reused = 0;
+    while (transferred < size) {
+        uint64_t left = size - transferred;
+        uint64_t to_transfer = left < di->block_size ? left : di->block_size;
 
-    if (pbs_res < 0) {
-        pvebackup_propagate_error(local_err);
-        return pbs_res;
-    } else {
-        size_t reused = (pbs_res == 0) ? size : 0;
-        pvebackup_add_transfered_bytes(size, !buf ? size : 0, reused);
+        pbs_res = proxmox_backup_co_write_data(backup_state.pbs, di->dev_id,
+            buf ? buf + transferred : NULL, start + transferred, to_transfer, &local_err);
+        transferred += to_transfer;
+
+        if (pbs_res < 0) {
+            pvebackup_propagate_error(local_err);
+            qemu_co_mutex_unlock(&backup_state.dump_callback_mutex);
+            return pbs_res;
+        }
+
+        reused += pbs_res == 0 ? to_transfer : 0;
     }
 
+    qemu_co_mutex_unlock(&backup_state.dump_callback_mutex);
+    pvebackup_add_transfered_bytes(size, !buf ? size : 0, reused);
+
     return size;
 }
 
@@ -730,6 +742,8 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
             PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
             l = g_list_next(l);
 
+            di->block_size = dump_cb_block_size;
+
             const char *devname = bdrv_get_device_name(di->bs);
 
             BdrvDirtyBitmap *bitmap = bdrv_find_dirty_bitmap(di->bs, PBS_BITMAP_NAME);
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 qemu] PVE: handle PBS write callback with big blocks correctly

[Fabian Grünbichler]

On July 22, 2020 3:47 pm, Stefan Reiter wrote:
> Under certain conditions QEMU will push more than the given blocksize
> into the callback at once. Handle it like VMA does, by iterating the
> data until all is written.
> 
> The block size is stored per backup device to be used in the callback.
> This avoids relying on PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE, in case it is
> made configurable in the future.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> 
> v2:
> * save chunk size explicitly to avoid depending on
>   PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE
> 
>  pve-backup.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/pve-backup.c b/pve-backup.c
> index 77eb475563..40d8136f1a 100644
> --- a/pve-backup.c
> +++ b/pve-backup.c
> @@ -67,6 +67,7 @@ opts_init(pvebackup_init);
>  typedef struct PVEBackupDevInfo {
>      BlockDriverState *bs;
>      size_t size;
> +    uint64_t block_size;
>      uint8_t dev_id;
>      bool completed;
>      char targetfile[PATH_MAX];
> @@ -147,17 +148,28 @@ pvebackup_co_dump_pbs_cb(
>          return -1;
>      }
>  
> -    pbs_res = proxmox_backup_co_write_data(backup_state.pbs, di->dev_id, buf, start, size, &local_err);
> -    qemu_co_mutex_unlock(&backup_state.dump_callback_mutex);
> +    uint64_t transferred = 0;
> +    uint64_t reused = 0;
> +    while (transferred < size) {
> +        uint64_t left = size - transferred;
> +        uint64_t to_transfer = left < di->block_size ? left : di->block_size;
>  
> -    if (pbs_res < 0) {
> -        pvebackup_propagate_error(local_err);
> -        return pbs_res;
> -    } else {
> -        size_t reused = (pbs_res == 0) ? size : 0;
> -        pvebackup_add_transfered_bytes(size, !buf ? size : 0, reused);
> +        pbs_res = proxmox_backup_co_write_data(backup_state.pbs, di->dev_id,
> +            buf ? buf + transferred : NULL, start + transferred, to_transfer, &local_err);
> +        transferred += to_transfer;
> +
> +        if (pbs_res < 0) {
> +            pvebackup_propagate_error(local_err);
> +            qemu_co_mutex_unlock(&backup_state.dump_callback_mutex);
> +            return pbs_res;
> +        }
> +
> +        reused += pbs_res == 0 ? to_transfer : 0;
>      }
>  
> +    qemu_co_mutex_unlock(&backup_state.dump_callback_mutex);
> +    pvebackup_add_transfered_bytes(size, !buf ? size : 0, reused);
> +
>      return size;
>  }
>  
> @@ -730,6 +742,8 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
>              PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
>              l = g_list_next(l);
>  
> +            di->block_size = dump_cb_block_size;
> +
>              const char *devname = bdrv_get_device_name(di->bs);
>  
>              BdrvDirtyBitmap *bitmap = bdrv_find_dirty_bitmap(di->bs, PBS_BITMAP_NAME);
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
>