[pbs-devel] [PATCH qemu] PVE: handle PBS write callback with big blocks correctly
Fabian Grünbichler
f.gruenbichler at proxmox.com
Wed Jul 15 08:51:41 CEST 2020
On July 14, 2020 3:17 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 in PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE (or smaller, for last one)
> sized blocks.
>
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
>
> As briefly tested by Fabian, it seems to fix the original issue.
Tested and it fixes the issue (patched smaller fixed chunk size + I/O
pressure from within the guest), but it only works for the currently
hard-coded case of chunk_size == PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE.
Given that we likely only trigger this when/if we make chunk size
configurable by the caller, we either need to handle this inside
libproxmox-backup-qemu (where we have the configured chunk size
available), or expose the configured chunk size to Qemu throughout.
We can still apply it on the off-chance that it is also triggerable with
the default 4M chunk size (haven't been able to yet), but then I'd add a
comment here to make it clear that this constant is just because we
don't have the actual one available (yet).
>
> pve-backup.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/pve-backup.c b/pve-backup.c
> index 77eb475563..4d423611e1 100644
> --- a/pve-backup.c
> +++ b/pve-backup.c
> @@ -147,17 +147,29 @@ 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 < PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE ?
> + left : PROXMOX_BACKUP_DEFAULT_CHUNK_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;
> }
>
> --
> 2.20.1
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
More information about the pbs-devel
mailing list