[pve-devel] [PATCH qemu 1/2] refresh patch context

[pve-devel] [PATCH qemu 1/2] refresh patch context

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 ...-PVE-add-savevm-async-for-background-state-snapshots.patch | 2 +-
 .../pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch   | 2 +-
 .../pve/0027-PVE-Backup-add-vma-backup-format-code.patch      | 4 ++--
 .../pve/0045-savevm-async-don-t-hold-BQL-during-setup.patch   | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/debian/patches/pve/0017-PVE-add-savevm-async-for-background-state-snapshots.patch b/debian/patches/pve/0017-PVE-add-savevm-async-for-background-state-snapshots.patch
index 762ba9b..0c98142 100644
--- a/debian/patches/pve/0017-PVE-add-savevm-async-for-background-state-snapshots.patch
+++ b/debian/patches/pve/0017-PVE-add-savevm-async-for-background-state-snapshots.patch
@@ -139,7 +139,7 @@ index 8a142fc7a9..a7824b5266 100644
    'threadinfo.c',
 diff --git a/migration/savevm-async.c b/migration/savevm-async.c
 new file mode 100644
-index 0000000000..ac1fac6378
+index 0000000000..aa2017d496
 --- /dev/null
 +++ b/migration/savevm-async.c
 @@ -0,0 +1,533 @@
diff --git a/debian/patches/pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch b/debian/patches/pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch
index 98cd3ce..f5f6f7b 100644
--- a/debian/patches/pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch
+++ b/debian/patches/pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch
@@ -192,7 +192,7 @@ index 9d0155a2a1..cc06240e8d 100644
  int qemu_fclose(QEMUFile *f);
  
 diff --git a/migration/savevm-async.c b/migration/savevm-async.c
-index ac1fac6378..ea3b2f36a6 100644
+index aa2017d496..b97f2c4f14 100644
 --- a/migration/savevm-async.c
 +++ b/migration/savevm-async.c
 @@ -380,7 +380,7 @@ void qmp_savevm_start(const char *statefile, Error **errp)
diff --git a/debian/patches/pve/0027-PVE-Backup-add-vma-backup-format-code.patch b/debian/patches/pve/0027-PVE-Backup-add-vma-backup-format-code.patch
index 3c3ac54..5ee84e1 100644
--- a/debian/patches/pve/0027-PVE-Backup-add-vma-backup-format-code.patch
+++ b/debian/patches/pve/0027-PVE-Backup-add-vma-backup-format-code.patch
@@ -1735,7 +1735,7 @@ index 0000000000..ac7da237d0
 +}
 diff --git a/vma.c b/vma.c
 new file mode 100644
-index 0000000000..304f02bc84
+index 0000000000..c76ecefa0f
 --- /dev/null
 +++ b/vma.c
 @@ -0,0 +1,878 @@
@@ -2619,7 +2619,7 @@ index 0000000000..304f02bc84
 +}
 diff --git a/vma.h b/vma.h
 new file mode 100644
-index 0000000000..1b62859165
+index 0000000000..86d2873aa5
 --- /dev/null
 +++ b/vma.h
 @@ -0,0 +1,150 @@
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 4a1b1b8..f5fa200 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
@@ -13,7 +13,7 @@ Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
  1 file changed, 2 deletions(-)
 
 diff --git a/migration/savevm-async.c b/migration/savevm-async.c
-index ea3b2f36a6..dd7744ab66 100644
+index b97f2c4f14..87ea0573d3 100644
 --- a/migration/savevm-async.c
 +++ b/migration/savevm-async.c
 @@ -403,10 +403,8 @@ void qmp_savevm_start(const char *statefile, Error **errp)
-- 
2.39.2

[pve-devel] [PATCH qemu 2/2] backup: trim heap after finishing

[Fiona Ebner]

Reported in the community forum [0]. By default, there can be large
amounts of memory left assigned to the QEMU process after backup.
Likely because of fragmentation, it's necessary to explicitly call
malloc_trim() to tell glibc that it shouldn't keep all that memory
resident for the process.

QEMU itself already does a malloc_trim() in the RCU thread, but that
code path might not be reached (or not for a long time) under usual
operation. The value of 4 MiB for the argument was also copied from
there.

Example with the following configuration:
> agent: 1
> boot: order=scsi0
> cores: 4
> cpu: x86-64-v2-AES
> ide2: none,media=cdrom
> memory: 1024
> name: backup-mem
> net0: virtio=DA:58:18:26:59:9F,bridge=vmbr0,firewall=1
> numa: 0
> ostype: l26
> scsi0: rbd:base-107-disk-0/vm-106-disk-1,size=4302M
> scsihw: virtio-scsi-pci
> smbios1: uuid=b2d4511e-8d01-44f1-afd6-9581b30c24a6
> sockets: 2
> startup: order=2
> virtio0: lvmthin:vm-106-disk-1,iothread=1,size=1G
> virtio1: lvmthin:vm-106-disk-2,iothread=1,size=1G
> virtio2: lvmthin:vm-106-disk-3,iothread=1,size=1G
> vmgenid: 0a1d8751-5e02-449d-977e-c0160e900231

Before the change:

> root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status
> VmRSS:	  370948 kB
> root@pve8a1 ~ # vzdump 106 --storage pbs
> (...)
> INFO: Backup job finished successfully
> root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status
> VmRSS:	 2114964 kB

After the change:

> root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status
> VmRSS:	  398788 kB
> root@pve8a1 ~ # vzdump 106 --storage pbs
> (...)
> INFO: Backup job finished successfully
> root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status
> VmRSS:	  424356 kB

[0]: https://forum.proxmox.com/threads/131339/

Co-diagnosed-by: Friedrich Weber <f.weber@proxmox.com>
Co-diagnosed-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 ...ckup-Proxmox-backup-patches-for-QEMU.patch | 23 +++++++++++++++----
 ...igrate-dirty-bitmap-state-via-savevm.patch |  4 ++--
 2 files changed, 20 insertions(+), 7 deletions(-)

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 3753eff..d873601 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
@@ -79,7 +79,8 @@ Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
      adapt for new job lock mechanism replacing AioContext locks
      adapt to QAPI changes
      improve canceling
-     allow passing max-workers setting]
+     allow passing max-workers setting
+     use malloc_trim after backup]
 Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
 ---
  block/meson.build              |    5 +
@@ -92,11 +93,11 @@ Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
  monitor/hmp-cmds.c             |   72 +++
  proxmox-backup-client.c        |  146 +++++
  proxmox-backup-client.h        |   60 ++
- pve-backup.c                   | 1097 ++++++++++++++++++++++++++++++++
+ pve-backup.c                   | 1109 ++++++++++++++++++++++++++++++++
  qapi/block-core.json           |  226 +++++++
  qapi/common.json               |   13 +
  qapi/machine.json              |   15 +-
- 14 files changed, 1711 insertions(+), 13 deletions(-)
+ 14 files changed, 1723 insertions(+), 13 deletions(-)
  create mode 100644 proxmox-backup-client.c
  create mode 100644 proxmox-backup-client.h
  create mode 100644 pve-backup.c
@@ -587,10 +588,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..dd72ee0ed6
+index 0000000000..10ca8a0b1d
 --- /dev/null
 +++ b/pve-backup.c
-@@ -0,0 +1,1097 @@
+@@ -0,0 +1,1109 @@
 +#include "proxmox-backup-client.h"
 +#include "vma.h"
 +
@@ -605,6 +606,10 @@ index 0000000000..dd72ee0ed6
 +#include "qapi/qmp/qerror.h"
 +#include "qemu/cutils.h"
 +
++#if defined(CONFIG_MALLOC_TRIM)
++#include <malloc.h>
++#endif
++
 +#include <proxmox-backup-qemu.h>
 +
 +/* PVE backup state and related function */
@@ -869,6 +874,14 @@ index 0000000000..dd72ee0ed6
 +    backup_state.stat.end_time = time(NULL);
 +    backup_state.stat.finishing = false;
 +    qemu_mutex_unlock(&backup_state.stat.lock);
++
++#if defined(CONFIG_MALLOC_TRIM)
++    /*
++     * Try to reclaim memory for buffers (and, in case of PBS, Rust futures), etc.
++     * Won't happen by default if there is fragmentation.
++     */
++    malloc_trim(4 * 1024 * 1024);
++#endif
 +}
 +
 +static void coroutine_fn pvebackup_co_complete_stream(void *opaque)
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 9437869..7a906e9 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
@@ -175,10 +175,10 @@ index 0000000000..887e998b9e
 +                         NULL);
 +}
 diff --git a/pve-backup.c b/pve-backup.c
-index dd72ee0ed6..cb5312fff3 100644
+index 10ca8a0b1d..0a5ce2cab8 100644
 --- a/pve-backup.c
 +++ b/pve-backup.c
-@@ -1090,6 +1090,7 @@ ProxmoxSupportStatus *qmp_query_proxmox_support(Error **errp)
+@@ -1102,6 +1102,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;
-- 
2.39.2

Re: [pve-devel] [PATCH qemu 2/2] backup: trim heap after finishing

[Wolfgang Bumiller]

On Mon, Aug 14, 2023 at 11:21:33AM +0200, Fiona Ebner wrote:
> Reported in the community forum [0]. By default, there can be large
> amounts of memory left assigned to the QEMU process after backup.
> Likely because of fragmentation, it's necessary to explicitly call
> malloc_trim() to tell glibc that it shouldn't keep all that memory
> resident for the process.
> 
> QEMU itself already does a malloc_trim() in the RCU thread, but that
> code path might not be reached (or not for a long time) under usual
> operation. The value of 4 MiB for the argument was also copied from
> there.
> 
> Example with the following configuration:
> > agent: 1
> > boot: order=scsi0
> > cores: 4
> > cpu: x86-64-v2-AES
> > ide2: none,media=cdrom
> > memory: 1024
> > name: backup-mem
> > net0: virtio=DA:58:18:26:59:9F,bridge=vmbr0,firewall=1
> > numa: 0
> > ostype: l26
> > scsi0: rbd:base-107-disk-0/vm-106-disk-1,size=4302M
> > scsihw: virtio-scsi-pci
> > smbios1: uuid=b2d4511e-8d01-44f1-afd6-9581b30c24a6
> > sockets: 2
> > startup: order=2
> > virtio0: lvmthin:vm-106-disk-1,iothread=1,size=1G
> > virtio1: lvmthin:vm-106-disk-2,iothread=1,size=1G
> > virtio2: lvmthin:vm-106-disk-3,iothread=1,size=1G
> > vmgenid: 0a1d8751-5e02-449d-977e-c0160e900231
> 
> Before the change:
> 
> > root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status
> > VmRSS:	  370948 kB
> > root@pve8a1 ~ # vzdump 106 --storage pbs
> > (...)
> > INFO: Backup job finished successfully
> > root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status
> > VmRSS:	 2114964 kB
> 
> After the change:
> 
> > root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status
> > VmRSS:	  398788 kB
> > root@pve8a1 ~ # vzdump 106 --storage pbs
> > (...)
> > INFO: Backup job finished successfully
> > root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status
> > VmRSS:	  424356 kB
> 
> [0]: https://forum.proxmox.com/threads/131339/
> 
> Co-diagnosed-by: Friedrich Weber <f.weber@proxmox.com>
> Co-diagnosed-by: Dominik Csapak <d.csapak@proxmox.com>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>

Both patches

Acked-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

> ---
>  ...ckup-Proxmox-backup-patches-for-QEMU.patch | 23 +++++++++++++++----
>  ...igrate-dirty-bitmap-state-via-savevm.patch |  4 ++--
>  2 files changed, 20 insertions(+), 7 deletions(-)
> 
> 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 3753eff..d873601 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
> @@ -79,7 +79,8 @@ Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
>       adapt for new job lock mechanism replacing AioContext locks
>       adapt to QAPI changes
>       improve canceling
> -     allow passing max-workers setting]
> +     allow passing max-workers setting
> +     use malloc_trim after backup]
>  Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>  ---
>   block/meson.build              |    5 +
> @@ -92,11 +93,11 @@ Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>   monitor/hmp-cmds.c             |   72 +++
>   proxmox-backup-client.c        |  146 +++++
>   proxmox-backup-client.h        |   60 ++
> - pve-backup.c                   | 1097 ++++++++++++++++++++++++++++++++
> + pve-backup.c                   | 1109 ++++++++++++++++++++++++++++++++
>   qapi/block-core.json           |  226 +++++++
>   qapi/common.json               |   13 +
>   qapi/machine.json              |   15 +-
> - 14 files changed, 1711 insertions(+), 13 deletions(-)
> + 14 files changed, 1723 insertions(+), 13 deletions(-)
>   create mode 100644 proxmox-backup-client.c
>   create mode 100644 proxmox-backup-client.h
>   create mode 100644 pve-backup.c
> @@ -587,10 +588,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..dd72ee0ed6
> +index 0000000000..10ca8a0b1d
>  --- /dev/null
>  +++ b/pve-backup.c
> -@@ -0,0 +1,1097 @@
> +@@ -0,0 +1,1109 @@
>  +#include "proxmox-backup-client.h"
>  +#include "vma.h"
>  +
> @@ -605,6 +606,10 @@ index 0000000000..dd72ee0ed6
>  +#include "qapi/qmp/qerror.h"
>  +#include "qemu/cutils.h"
>  +
> ++#if defined(CONFIG_MALLOC_TRIM)
> ++#include <malloc.h>
> ++#endif
> ++
>  +#include <proxmox-backup-qemu.h>
>  +
>  +/* PVE backup state and related function */
> @@ -869,6 +874,14 @@ index 0000000000..dd72ee0ed6
>  +    backup_state.stat.end_time = time(NULL);
>  +    backup_state.stat.finishing = false;
>  +    qemu_mutex_unlock(&backup_state.stat.lock);
> ++
> ++#if defined(CONFIG_MALLOC_TRIM)
> ++    /*
> ++     * Try to reclaim memory for buffers (and, in case of PBS, Rust futures), etc.
> ++     * Won't happen by default if there is fragmentation.
> ++     */
> ++    malloc_trim(4 * 1024 * 1024);
> ++#endif
>  +}
>  +
>  +static void coroutine_fn pvebackup_co_complete_stream(void *opaque)
> 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 9437869..7a906e9 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
> @@ -175,10 +175,10 @@ index 0000000000..887e998b9e
>  +                         NULL);
>  +}
>  diff --git a/pve-backup.c b/pve-backup.c
> -index dd72ee0ed6..cb5312fff3 100644
> +index 10ca8a0b1d..0a5ce2cab8 100644
>  --- a/pve-backup.c
>  +++ b/pve-backup.c
> -@@ -1090,6 +1090,7 @@ ProxmoxSupportStatus *qmp_query_proxmox_support(Error **errp)
> +@@ -1102,6 +1102,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;
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

[pve-devel] applied-series: [PATCH qemu 1/2] refresh patch context

[Fiona Ebner]

applied both patches with Wolfgang's Acked-by