From 8dd76cc52d359de2020cdbbbb7e1d8cb910271f3 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Tue, 12 Mar 2024 13:54:59 +0100 Subject: [PATCH] backup: factor out & clean up gathering device info into helper Squash the two original patches [0][1] from Fiona, which got send separate to be easier to review, into the big patch that adds the Proxmox backup integration. [0]: https://lists.proxmox.com/pipermail/pve-devel/2024-January/061479.html [1]: https://lists.proxmox.com/pipermail/pve-devel/2024-January/061478.html Originally-by: Fiona Ebner Signed-off-by: Thomas Lamprecht --- ...nnel-implementation-for-savevm-async.patch | 1 + ...ckup-Proxmox-backup-patches-for-QEMU.patch | 119 ++++++++++-------- ...igrate-dirty-bitmap-state-via-savevm.patch | 4 +- ...-rbd-workaround-for-ceph-issue-53784.patch | 1 + ...-fix-handling-of-holes-in-.bdrv_co_b.patch | 1 + ...k-rbd-implement-bdrv_co_block_status.patch | 1 + ...alloc-track-fix-deadlock-during-drop.patch | 1 + ...apshots-hold-the-BQL-during-setup-ca.patch | 1 + ...vm-async-don-t-hold-BQL-during-setup.patch | 1 + 9 files changed, 77 insertions(+), 53 deletions(-) diff --git a/debian/patches/pve/0016-PVE-add-IOChannel-implementation-for-savevm-async.patch b/debian/patches/pve/0016-PVE-add-IOChannel-implementation-for-savevm-async.patch index 40c9b32..df086e3 100644 --- a/debian/patches/pve/0016-PVE-add-IOChannel-implementation-for-savevm-async.patch +++ b/debian/patches/pve/0016-PVE-add-IOChannel-implementation-for-savevm-async.patch @@ -14,6 +14,7 @@ Additionally, allows tracking the current position from the outside (intended to be used for progress tracking). Signed-off-by: Fiona Ebner +Signed-off-by: Thomas Lamprecht --- migration/channel-savevm-async.c | 183 +++++++++++++++++++++++++++++++ migration/channel-savevm-async.h | 51 +++++++++ diff --git a/debian/patches/pve/0030-PVE-Backup-Proxmox-backup-patches-for-QEMU.patch b/debian/patches/pve/0030-PVE-Backup-Proxmox-backup-patches-for-QEMU.patch index 054e31c..8bc528e 100644 --- a/debian/patches/pve/0030-PVE-Backup-Proxmox-backup-patches-for-QEMU.patch +++ b/debian/patches/pve/0030-PVE-Backup-Proxmox-backup-patches-for-QEMU.patch @@ -94,11 +94,11 @@ Signed-off-by: Fiona Ebner monitor/hmp-cmds.c | 72 +++ proxmox-backup-client.c | 146 +++++ proxmox-backup-client.h | 60 ++ - pve-backup.c | 1072 ++++++++++++++++++++++++++++++++ + pve-backup.c | 1089 ++++++++++++++++++++++++++++++++ qapi/block-core.json | 229 +++++++ qapi/common.json | 14 + qapi/machine.json | 16 +- - 14 files changed, 1687 insertions(+), 14 deletions(-) + 14 files changed, 1704 insertions(+), 14 deletions(-) create mode 100644 proxmox-backup-client.c create mode 100644 proxmox-backup-client.h create mode 100644 pve-backup.c @@ -586,10 +586,10 @@ index 0000000000..8cbf645b2c +#endif /* PROXMOX_BACKUP_CLIENT_H */ diff --git a/pve-backup.c b/pve-backup.c new file mode 100644 -index 0000000000..5ed3c6a310 +index 0000000000..ae3d137e12 --- /dev/null +++ b/pve-backup.c -@@ -0,0 +1,1072 @@ +@@ -0,0 +1,1089 @@ +#include "proxmox-backup-client.h" +#include "vma.h" + @@ -1173,6 +1173,66 @@ index 0000000000..5ed3c6a310 + aio_co_enter(data->ctx, data->co); +} + ++/* ++ * Returns a list of device infos, which needs to be freed by the caller. In ++ * case of an error, errp will be set, but the returned value might still be a ++ * list. ++ */ ++static GList coroutine_fn *get_device_info( ++ const char *devlist, ++ Error **errp) ++{ ++ gchar **devs = NULL; ++ GList *di_list = NULL; ++ ++ if (devlist) { ++ devs = g_strsplit_set(devlist, ",;:", -1); ++ ++ gchar **d = devs; ++ while (d && *d) { ++ BlockBackend *blk = blk_by_name(*d); ++ if (!blk) { ++ error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, ++ "Device '%s' not found", *d); ++ goto err; ++ } ++ BlockDriverState *bs = blk_bs(blk); ++ if (!bdrv_co_is_inserted(bs)) { ++ error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, *d); ++ goto err; ++ } ++ PVEBackupDevInfo *di = g_new0(PVEBackupDevInfo, 1); ++ di->bs = bs; ++ di_list = g_list_append(di_list, di); ++ d++; ++ } ++ } else { ++ BdrvNextIterator it; ++ ++ for (BlockDriverState *bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { ++ if (!bdrv_co_is_inserted(bs) || bdrv_is_read_only(bs)) { ++ continue; ++ } ++ ++ PVEBackupDevInfo *di = g_new0(PVEBackupDevInfo, 1); ++ di->bs = bs; ++ di_list = g_list_append(di_list, di); ++ } ++ } ++ ++ if (!di_list) { ++ error_set(errp, ERROR_CLASS_GENERIC_ERROR, "empty device list"); ++ goto err; ++ } ++ ++err: ++ if (devs) { ++ g_strfreev(devs); ++ } ++ ++ return di_list; ++} ++ +UuidInfo coroutine_fn *qmp_backup( + const char *backup_file, + const char *password, @@ -1198,13 +1258,10 @@ index 0000000000..5ed3c6a310 + + qemu_co_mutex_lock(&backup_state.backup_mutex); + -+ BlockBackend *blk; -+ BlockDriverState *bs = NULL; + Error *local_err = NULL; + uuid_t uuid; + VmaWriter *vmaw = NULL; + ProxmoxBackupHandle *pbs = NULL; -+ gchar **devs = NULL; + GList *di_list = NULL; + GList *l; + UuidInfo *uuid_info; @@ -1222,48 +1279,12 @@ index 0000000000..5ed3c6a310 + /* Todo: try to auto-detect format based on file name */ + format = has_format ? format : BACKUP_FORMAT_VMA; + -+ if (devlist) { -+ devs = g_strsplit_set(devlist, ",;:", -1); -+ -+ gchar **d = devs; -+ while (d && *d) { -+ blk = blk_by_name(*d); -+ if (blk) { -+ bs = blk_bs(blk); -+ if (!bdrv_co_is_inserted(bs)) { -+ error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, *d); -+ goto err; -+ } -+ PVEBackupDevInfo *di = g_new0(PVEBackupDevInfo, 1); -+ di->bs = bs; -+ di_list = g_list_append(di_list, di); -+ } else { -+ error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, -+ "Device '%s' not found", *d); -+ goto err; -+ } -+ d++; -+ } -+ -+ } else { -+ BdrvNextIterator it; -+ -+ bs = NULL; -+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { -+ if (!bdrv_co_is_inserted(bs) || bdrv_is_read_only(bs)) { -+ continue; -+ } -+ -+ PVEBackupDevInfo *di = g_new0(PVEBackupDevInfo, 1); -+ di->bs = bs; -+ di_list = g_list_append(di_list, di); -+ } -+ } -+ -+ if (!di_list) { -+ error_set(errp, ERROR_CLASS_GENERIC_ERROR, "empty device list"); ++ di_list = get_device_info(devlist, &local_err); ++ if (local_err) { ++ error_propagate(errp, local_err); + goto err; + } ++ assert(di_list); + + size_t total = 0; + @@ -1551,10 +1572,6 @@ index 0000000000..5ed3c6a310 + g_list_free(di_list); + backup_state.di_list = NULL; + -+ if (devs) { -+ g_strfreev(devs); -+ } -+ + if (vmaw) { + Error *err = NULL; + vma_writer_close(vmaw, &err); diff --git a/debian/patches/pve/0034-PVE-Migrate-dirty-bitmap-state-via-savevm.patch b/debian/patches/pve/0034-PVE-Migrate-dirty-bitmap-state-via-savevm.patch index dffe12c..1015db3 100644 --- a/debian/patches/pve/0034-PVE-Migrate-dirty-bitmap-state-via-savevm.patch +++ b/debian/patches/pve/0034-PVE-Migrate-dirty-bitmap-state-via-savevm.patch @@ -174,10 +174,10 @@ index 0000000000..887e998b9e + NULL); +} diff --git a/pve-backup.c b/pve-backup.c -index 5ed3c6a310..6720e985bc 100644 +index ae3d137e12..e6b17b797e 100644 --- a/pve-backup.c +++ b/pve-backup.c -@@ -1065,6 +1065,7 @@ ProxmoxSupportStatus *qmp_query_proxmox_support(Error **errp) +@@ -1082,6 +1082,7 @@ ProxmoxSupportStatus *qmp_query_proxmox_support(Error **errp) ret->pbs_library_version = g_strdup(proxmox_backup_qemu_version()); ret->pbs_dirty_bitmap = true; ret->pbs_dirty_bitmap_savevm = true; diff --git a/debian/patches/pve/0040-Revert-block-rbd-workaround-for-ceph-issue-53784.patch b/debian/patches/pve/0040-Revert-block-rbd-workaround-for-ceph-issue-53784.patch index e60e74f..094f353 100644 --- a/debian/patches/pve/0040-Revert-block-rbd-workaround-for-ceph-issue-53784.patch +++ b/debian/patches/pve/0040-Revert-block-rbd-workaround-for-ceph-issue-53784.patch @@ -7,6 +7,7 @@ This reverts commit fc176116cdea816ceb8dd969080b2b95f58edbc0 in preparation to revert 0347a8fd4c3faaedf119be04c197804be40a384b. Signed-off-by: Fabian Ebner +Signed-off-by: Thomas Lamprecht --- block/rbd.c | 42 ++---------------------------------------- 1 file changed, 2 insertions(+), 40 deletions(-) diff --git a/debian/patches/pve/0041-Revert-block-rbd-fix-handling-of-holes-in-.bdrv_co_b.patch b/debian/patches/pve/0041-Revert-block-rbd-fix-handling-of-holes-in-.bdrv_co_b.patch index e5fad08..e4aca73 100644 --- a/debian/patches/pve/0041-Revert-block-rbd-fix-handling-of-holes-in-.bdrv_co_b.patch +++ b/debian/patches/pve/0041-Revert-block-rbd-fix-handling-of-holes-in-.bdrv_co_b.patch @@ -8,6 +8,7 @@ This reverts commit 9e302f64bb407a9bb097b626da97228c2654cfee in preparation to revert 0347a8fd4c3faaedf119be04c197804be40a384b. Signed-off-by: Fabian Ebner +Signed-off-by: Thomas Lamprecht --- block/rbd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/debian/patches/pve/0042-Revert-block-rbd-implement-bdrv_co_block_status.patch b/debian/patches/pve/0042-Revert-block-rbd-implement-bdrv_co_block_status.patch index 7e1ee52..9e9a385 100644 --- a/debian/patches/pve/0042-Revert-block-rbd-implement-bdrv_co_block_status.patch +++ b/debian/patches/pve/0042-Revert-block-rbd-implement-bdrv_co_block_status.patch @@ -18,6 +18,7 @@ Upstream bug report: https://gitlab.com/qemu-project/qemu/-/issues/1026 Signed-off-by: Fabian Ebner +Signed-off-by: Thomas Lamprecht --- block/rbd.c | 112 ---------------------------------------------------- 1 file changed, 112 deletions(-) diff --git a/debian/patches/pve/0043-alloc-track-fix-deadlock-during-drop.patch b/debian/patches/pve/0043-alloc-track-fix-deadlock-during-drop.patch index 8169445..153b8ef 100644 --- a/debian/patches/pve/0043-alloc-track-fix-deadlock-during-drop.patch +++ b/debian/patches/pve/0043-alloc-track-fix-deadlock-during-drop.patch @@ -67,6 +67,7 @@ general to not leave unnecessary block nodes lying around. Suggested-by: Wolfgang Bumiller Signed-off-by: Fiona Ebner +Signed-off-by: Thomas Lamprecht --- block/alloc-track.c | 54 ++++++++++++++------------------------------- 1 file changed, 16 insertions(+), 38 deletions(-) diff --git a/debian/patches/pve/0044-migration-for-snapshots-hold-the-BQL-during-setup-ca.patch b/debian/patches/pve/0044-migration-for-snapshots-hold-the-BQL-during-setup-ca.patch index 3fa7ef7..635f64a 100644 --- a/debian/patches/pve/0044-migration-for-snapshots-hold-the-BQL-during-setup-ca.patch +++ b/debian/patches/pve/0044-migration-for-snapshots-hold-the-BQL-during-setup-ca.patch @@ -43,6 +43,7 @@ callbacks that need the BQL and only take the lock if it's not already held. Signed-off-by: Fiona Ebner +Signed-off-by: Thomas Lamprecht --- include/migration/register.h | 2 +- migration/block-dirty-bitmap.c | 15 ++++++++++++--- diff --git a/debian/patches/pve/0045-savevm-async-don-t-hold-BQL-during-setup.patch b/debian/patches/pve/0045-savevm-async-don-t-hold-BQL-during-setup.patch index 3ff0bf7..ac00f1a 100644 --- a/debian/patches/pve/0045-savevm-async-don-t-hold-BQL-during-setup.patch +++ b/debian/patches/pve/0045-savevm-async-don-t-hold-BQL-during-setup.patch @@ -8,6 +8,7 @@ callbacks" for why. This is separate, because a version of that one will hopefully land upstream. Signed-off-by: Fiona Ebner +Signed-off-by: Thomas Lamprecht --- migration/savevm-async.c | 2 -- 1 file changed, 2 deletions(-)