[pve-devel] [PATCH] update proxmox patches to qemu 2.4

Wolfgang Bumiller w.bumiller at proxmox.com
Wed Jul 1 15:26:45 CEST 2015


My patch doesn't actually apply to the current master branch any longer.
And a big migration related pull request appeared on the mailing list
today, so I'm sure if we fixup the patches today they'll be broken again
tomorrow.
But if you want to test them early I can still help going over the C
code changes if you like.
My suggestion though would be to wait for the hard-feature-freeze next
week. It might save us the trouble of updating and re-updating the
patches over and over.

What do you think?

On Wed, Jul 01, 2015 at 08:49:30AM +0200, Alexandre DERUMIER wrote:
> Thanks Wolfgang,I'll try them (I think you are better with C language than me ;)
> 
> 
> qemu 2.4 have some great improvements 
> - memory hot-unplug support
> - drive-mirror fixes with discard(should be merge in master in coming days)
> - sata migratable
> - incremental backup (don't known if it'll be easy to adapt proxmox patch for it)
> 
> ----- Mail original -----
> De: "Wolfgang Bumiller" <w.bumiller at proxmox.com>
> À: "aderumier" <aderumier at odiso.com>
> Cc: "pve-devel" <pve-devel at pve.proxmox.com>
> Envoyé: Mercredi 1 Juillet 2015 08:14:17
> Objet: Re: [pve-devel] [PATCH] update proxmox patches to qemu 2.4
> 
> I'll merge this into my 2.4 branch. 
> I've already started the branch for 2.4 after finishing the 2.3 patches, 
> but then decided to wait for qemu's hard-freeze on 2015-07-07 (see 
> <http://wiki.qemu.org/Planning/2.4>) before finishing it off. 
> (I probably should have just posted the patch for reviewing anyway.) 
> My branch doesn't include your new patches yet. 
> I'll send my current diff in a minute. It seems to overlap a bit 
> 
> On Tue, Jun 30, 2015 at 06:17:35PM +0200, Alexandre Derumier wrote: 
> > fixme : 
> > -internal-snapshot-async.patch 
> > 
> > - backups : seem that they are lot of changes with bitmap support add (backup_start have new arguments for example) 
> > 
> > blockdev.c:2486:7: error: conflicting types for ‘qmp_backup’ 
> > char *qmp_backup(const char *backup_file, bool has_format, 
> > ^ 
> > In file included from blockdev.c:49:0: 
> > qmp-commands.h:111:11: note: previous declaration of ‘qmp_backup’ was here 
> > UuidInfo *qmp_backup(const char *backup_file, bool has_format, BackupFormat format, bool has_config_file, const char *config_file, bool has_devlist, const char *devlist, bool has_speed, int64_t speed, Error **errp); 
> > ^ 
> > blockdev.c: In function ‘qmp_backup’: 
> > blockdev.c:2520:37: error: ‘QERR_DEVICE_IS_READ_ONLY’ undeclared (first use in this function) 
> > error_set(errp, QERR_DEVICE_IS_READ_ONLY, *d); 
> > ^ 
> > blockdev.c:2520:37: note: each undeclared identifier is reported only once for each function it appears in 
> > blockdev.c:2524:21: error: incompatible type for argument 2 of ‘error_set’ 
> > error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, *d); 
> > ^ 
> > In file included from /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/include/sysemu/block-backend.h:17:0, 
> > from blockdev.c:33: 
> > /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/include/qapi/error.h:30:6: note: expected ‘ErrorClass’ but argument is of type ‘const char *’ 
> > void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) 
> > ^ 
> > blockdev.c:2531:33: error: ‘QERR_DEVICE_NOT_FOUND’ undeclared (first use in this function) 
> > error_set(errp, QERR_DEVICE_NOT_FOUND, *d); 
> > ^ 
> > blockdev.c:2707:9: error: incompatible type for argument 7 of ‘backup_start’ 
> > backup_start(di->bs, di->target, speed, MIRROR_SYNC_MODE_FULL, 
> > ^ 
> > In file included from /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/include/block/throttle-groups.h:29:0, 
> > from blockdev.c:37: 
> > /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/include/block/block_int.h:650:6: note: expected ‘BlockdevOnError’ but argument is of type ‘int (*)(void *, struct BlockDriverState *, int64_t, int, unsigned char *)’ 
> > void backup_start(BlockDriverState *bs, BlockDriverState *target, 
> > ^ 
> > blockdev.c:2709:41: warning: passing argument 8 of ‘backup_start’ from incompatible pointer type 
> > pvebackup_dump_cb, pvebackup_complete_cb, di, 
> > ^ 
> > In file included from /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/include/block/throttle-groups.h:29:0, 
> > from blockdev.c:37: 
> > /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/include/block/block_int.h:650:6: note: expected ‘int (*)(void *, struct BlockDriverState *, int64_t, int, unsigned char *)’ but argument is of type ‘void (*)(void *, int)’ 
> > void backup_start(BlockDriverState *bs, BlockDriverState *target, 
> > ^ 
> > blockdev.c:2709:64: warning: passing argument 9 of ‘backup_start’ from incompatible pointer type 
> > pvebackup_dump_cb, pvebackup_complete_cb, di, 
> > ^ 
> > In file included from /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/include/block/throttle-groups.h:29:0, 
> > from blockdev.c:37: 
> > /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/include/block/block_int.h:650:6: note: expected ‘void (*)(void *, int)’ but argument is of type ‘struct PVEBackupDevInfo *’ 
> > void backup_start(BlockDriverState *bs, BlockDriverState *target, 
> > ^ 
> > blockdev.c:2710:22: warning: passing argument 10 of ‘backup_start’ makes pointer from integer without a cast 
> > true, &local_err); 
> > ^ 
> > In file included from /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/include/block/throttle-groups.h:29:0, 
> > from blockdev.c:37: 
> > /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/include/block/block_int.h:650:6: note: expected ‘void *’ but argument is of type ‘int’ 
> > void backup_start(BlockDriverState *bs, BlockDriverState *target, 
> > ^ 
> > blockdev.c:2710:22: warning: the address of ‘local_err’ will always evaluate as ‘true’ [-Waddress] 
> > true, &local_err); 
> > ^ 
> > blockdev.c:2707:9: error: too few arguments to function ‘backup_start’ 
> > backup_start(di->bs, di->target, speed, MIRROR_SYNC_MODE_FULL, 
> > ^ 
> > In file included from /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/include/block/throttle-groups.h:29:0, 
> > from blockdev.c:37: 
> > /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/include/block/block_int.h:650:6: note: declared here 
> > void backup_start(BlockDriverState *bs, BlockDriverState *target, 
> > ^ 
> > /var/lib/vz/proxmox2/pve-qemu-kvm2.4/qemu-kvm/rules.mak:57: recipe for target 'blockdev.o' failed 
> > 
> > Signed-off-by: Alexandre Derumier <aderumier at odiso.com> 
> > --- 
> > debian/patches/add-qmp-get-link-status.patch | 15 ++- 
> > .../patches/backup-add-pve-monitor-commands.patch | 4 +- 
> > debian/patches/backup-modify-job-api.patch | 24 +++-- 
> > debian/patches/internal-snapshot-async.patch | 2 +- 
> > debian/patches/jemalloc.patch | 112 ++++++++++++++++++--- 
> > debian/patches/modify-query-machines.patch | 2 +- 
> > debian/patches/modify-query-spice.patch | 2 +- 
> > debian/patches/series | 2 - 
> > debian/patches/virtio-balloon-fix-query.patch | 4 +- 
> > 9 files changed, 129 insertions(+), 38 deletions(-) 
> > 
> > diff --git a/debian/patches/add-qmp-get-link-status.patch b/debian/patches/add-qmp-get-link-status.patch 
> > index 105d415..850188a 100644 
> > --- a/debian/patches/add-qmp-get-link-status.patch 
> > +++ b/debian/patches/add-qmp-get-link-status.patch 
> > @@ -2,11 +2,20 @@ Index: new/qapi-schema.json 
> > =================================================================== 
> > --- new.orig/qapi-schema.json 2014-12-10 09:15:50.890262765 +0100 
> > +++ new/qapi-schema.json 2014-12-11 09:20:31.072561486 +0100 
> > -@@ -1366,6 +1366,22 @@ 
> > +@@ -1366,6 +1366,31 @@ 
> > ## 
> > { 'command': 'set_link', 'data': {'name': 'str', 'up': 'bool'} } 
> > 
> > -+ 
> > ++## 
> > ++# @LinkStatus: 
> > ++# 
> > ++# Nic LinkStatus information. 
> > ++# 
> > ++# @status: the status of the nic 
> > ++# 
> > ++## 
> > ++{ 'struct': 'LinkStatus', 'data': {'status': 'int'} } 
> > ++ 
> > +## 
> > +# @get_link_status 
> > +# 
> > @@ -20,7 +29,7 @@ Index: new/qapi-schema.json 
> > +# 
> > +# Notes: this is an Proxmox VE extension and not offical part of Qemu. 
> > +## 
> > -+{ 'command': 'get_link_status', 'data': {'name': 'str'}, 'returns': 'int'} 
> > ++{ 'command': 'get_link_status', 'data': {'name': 'str'}, 'returns': 'LinkStatus' } 
> > + 
> > ## 
> > # @balloon: 
> > diff --git a/debian/patches/backup-add-pve-monitor-commands.patch b/debian/patches/backup-add-pve-monitor-commands.patch 
> > index e58033e..26b2548 100644 
> > --- a/debian/patches/backup-add-pve-monitor-commands.patch 
> > +++ b/debian/patches/backup-add-pve-monitor-commands.patch 
> > @@ -650,7 +650,7 @@ Index: new/qapi-schema.json 
> > +# @uuid: #optional uuid for this backup job 
> > +# 
> > +## 
> > -+{ 'type': 'BackupStatus', 
> > ++{ 'struct': 'BackupStatus', 
> > + 'data': {'*status': 'str', '*errmsg': 'str', '*total': 'int', 
> > + '*transferred': 'int', '*zero-bytes': 'int', 
> > + '*start-time': 'int', '*end-time': 'int', 
> > @@ -690,7 +690,7 @@ Index: new/qapi-schema.json 
> > + '*format': 'BackupFormat', 
> > + '*config-file': 'str', 
> > + '*devlist': 'str', '*speed': 'int' }, 
> > -+ 'returns': 'str' } 
> > ++ 'returns': 'UuidInfo' } 
> > + 
> > +## 
> > +# @query-backup 
> > diff --git a/debian/patches/backup-modify-job-api.patch b/debian/patches/backup-modify-job-api.patch 
> > index f5e81a7..4f1649f 100644 
> > --- a/debian/patches/backup-modify-job-api.patch 
> > +++ b/debian/patches/backup-modify-job-api.patch 
> > @@ -125,8 +125,8 @@ Index: new/block/backup.c 
> > return; 
> > } 
> > 
> > -@@ -397,12 +415,15 @@ in backup_start 
> > - return; 
> > +@@ -529,12 +529,15 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, 
> > + goto error; 
> > } 
> > 
> > - bdrv_op_block_all(target, job->common.blocker); 
> > @@ -139,28 +139,29 @@ Index: new/block/backup.c 
> > job->target = target; 
> > job->sync_mode = sync_mode; 
> > + job->common.paused = paused; 
> > + job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP ? 
> > + sync_bitmap : NULL; 
> > job->common.len = len; 
> > - job->common.co = qemu_coroutine_create(backup_run); 
> > - qemu_coroutine_enter(job->common.co, job); 
> > + 
> > Index: new/blockdev.c 
> > =================================================================== 
> > --- new.orig/blockdev.c 2014-11-20 07:55:31.000000000 +0100 
> > +++ new/blockdev.c 2014-11-20 08:48:02.000000000 +0100 
> > -@@ -2223,7 +2223,7 @@ qmp_drive_backup 
> > - bdrv_set_aio_context(target_bs, aio_context); 
> > +@@ -2575,7 +2575,7 @@ void qmp_drive_backup(const char *device, const char *target, 
> > 
> > - backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error, 
> > + backup_start(bs, target_bs, speed, sync, bmap, 
> > + on_source_error, on_target_error, 
> > - block_job_cb, bs, &local_err); 
> > + NULL, block_job_cb, bs, false, &local_err); 
> > if (local_err != NULL) { 
> > bdrv_unref(target_bs); 
> > error_propagate(errp, local_err); 
> > -@@ -2284,7 +2284,7 @@ qmp_blockdev_backup 
> > +@@ -2636,7 +2636,7 @@ void qmp_blockdev_backup(const char *device, const char *target, 
> > bdrv_ref(target_bs); 
> > bdrv_set_aio_context(target_bs, aio_context); 
> > - backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error, 
> > -- block_job_cb, bs, &local_err); 
> > -+ NULL, block_job_cb, bs, false, &local_err); 
> > + backup_start(bs, target_bs, speed, sync, NULL, on_source_error, 
> > +- on_target_error, block_job_cb, bs, &local_err); 
> > ++ on_target_error, NULL, block_job_cb, bs, false, &local_err); 
> > if (local_err != NULL) { 
> > bdrv_unref(target_bs); 
> > error_propagate(errp, local_err); 
> > @@ -180,6 +181,7 @@ Index: new/include/block/block_int.h 
> > int64_t offset; 
> > @@ -583,7 +586,9 @@ 
> > int64_t speed, MirrorSyncMode sync_mode, 
> > + BdrvDirtyBitmap *sync_bitmap, 
> > BlockdevOnError on_source_error, 
> > BlockdevOnError on_target_error, 
> > + BackupDumpFunc *dump_cb, 
> > diff --git a/debian/patches/internal-snapshot-async.patch b/debian/patches/internal-snapshot-async.patch 
> > index e6d8a13..280830d 100644 
> > --- a/debian/patches/internal-snapshot-async.patch 
> > +++ b/debian/patches/internal-snapshot-async.patch 
> > @@ -267,7 +267,7 @@ Index: new/qapi-schema.json 
> > +# 
> > +# Since: 1.3 
> > +## 
> > -+{ 'type': 'SaveVMInfo', 
> > ++{ 'struct': 'SaveVMInfo', 
> > + 'data': {'*status': 'str', '*error': 'str', 
> > + '*total-time': 'int', '*bytes': 'int'} } 
> > + 
> > diff --git a/debian/patches/jemalloc.patch b/debian/patches/jemalloc.patch 
> > index b5dba0d..449cb61 100644 
> > --- a/debian/patches/jemalloc.patch 
> > +++ b/debian/patches/jemalloc.patch 
> > @@ -1,16 +1,95 @@ 
> > +From patchwork Fri Jun 19 10:56:58 2015 
> > +Content-Type: text/plain; charset="utf-8" 
> > +MIME-Version: 1.0 
> > +Content-Transfer-Encoding: 7bit 
> > +Subject: configure: Add support for jemalloc 
> > +From: Alexandre DERUMIER <aderumier at odiso.com> 
> > +X-Patchwork-Id: 486671 
> > +Message-Id: <1434711418-20429-1-git-send-email-aderumier at odiso.com> 
> > +To: qemu-devel at nongnu.org 
> > +Cc: Alexandre Derumier <aderumier at odiso.com> 
> > +Date: Fri, 19 Jun 2015 12:56:58 +0200 
> > + 
> > +This adds "--enable-jemalloc" and "--disable-jemalloc" to allow linking 
> > +to jemalloc memory allocator. 
> > + 
> > +We have already tcmalloc support, 
> > +but it seem to not working well with a lot of iothreads/disks. 
> > + 
> > +The main problem is that tcmalloc use a shared thread cache of 16MB 
> > +by default. 
> > +With more threads, this cache is shared, and some bad garbage collections 
> > +can occur if the cache is too low. 
> > + 
> > +It's possible to tcmalloc cache increase it with a env var: 
> > +TCMALLOC_MAX_TOTAL_THREAD_CACHE_BYTES=256MB 
> > + 
> > +With default 16MB, performances are really bad with more than 2 disks. 
> > +Increasing to 256MB, it's helping but still have problem with 16 disks/iothreads. 
> > + 
> > +Jemalloc don't have performance problem with default configuration. 
> > + 
> > +Here the benchmark results in iops of 1 qemu vm randread 4K iodepth=32, 
> > +with rbd block backend (librbd is doing a lot of memory allocation), 
> > +1 iothread by disk 
> > + 
> > +glibc malloc 
> > +------------ 
> > + 
> > +1 disk 29052 
> > +2 disks 55878 
> > +4 disks 127899 
> > +8 disks 240566 
> > +15 disks 269976 
> > + 
> > +jemalloc 
> > +-------- 
> > + 
> > +1 disk 41278 
> > +2 disks 75781 
> > +4 disks 195351 
> > +8 disks 294241 
> > +15 disks 298199 
> > + 
> > +tcmalloc 2.2.1 default 16M cache 
> > +-------------------------------- 
> > + 
> > +1 disk 37911 
> > +2 disks 67698 
> > +4 disks 41076 
> > +8 disks 43312 
> > +15 disks 37569 
> > + 
> > +tcmalloc : 256M cache 
> > +--------------------------- 
> > + 
> > +1 disk 33914 
> > +2 disks 58839 
> > +4 disks 148205 
> > +8 disks 213298 
> > +15 disks 218383 
> > + 
> > +Signed-off-by: Alexandre Derumier <aderumier at odiso.com> 
> > +Reviewed-by: Fam Zheng <famz at redhat.com> 
> > +--- 
> > + configure | 29 +++++++++++++++++++++++++++++ 
> > + 1 file changed, 29 insertions(+) 
> > + 
> > +diff --git a/configure b/configure 
> > +index 222694f..2fe1e05 100755 
> > --- a/configure 
> > +++ b/configure 
> > -@@ -336,6 +336,7 @@ libssh2="" 
> > - vhdx="" 
> > +@@ -336,6 +336,7 @@ vhdx="" 
> > quorum="" 
> > numa="" 
> > + tcmalloc="no" 
> > +jemalloc="no" 
> > 
> > # parse CC options first 
> > for opt do 
> > -@@ -1134,6 +1135,10 @@ for opt do 
> > +@@ -1147,6 +1148,10 @@ for opt do 
> > ;; 
> > - --enable-numa) numa="yes" 
> > + --enable-tcmalloc) tcmalloc="yes" 
> > ;; 
> > + --disable-jemalloc) jemalloc="no" 
> > + ;; 
> > @@ -19,16 +98,19 @@ 
> > *) 
> > echo "ERROR: unknown option $opt" 
> > echo "Try '$0 --help' for more information" 
> > -@@ -1407,6 +1412,8 @@ Advanced options (experts only): 
> > - --enable-quorum enable quorum block filter support 
> > - --disable-numa disable libnuma support 
> > - --enable-numa enable libnuma support 
> > -+ --disable-jemalloc disable jemalloc support 
> > -+ --enable-numa enable jemalloc support 
> > +@@ -3344,6 +3351,11 @@ EOF 
> > + fi 
> > + fi 
> > + 
> > ++if test "$tcmalloc" = "yes" && test "$jemalloc" = "yes" ; then 
> > ++ echo "ERROR: tcmalloc && jemalloc can't be used at the same time" 
> > ++ exit 1 
> > ++fi 
> > ++ 
> > + ########################################## 
> > + # tcmalloc probe 
> > 
> > - NOTE: The object files are built at the place where configure is launched 
> > - EOF 
> > -@@ -3325,6 +3332,22 @@ EOF 
> > +@@ -3361,6 +3373,22 @@ EOF 
> > fi 
> > 
> > ########################################## 
> > @@ -51,10 +133,10 @@ 
> > # signalfd probe 
> > signalfd="no" 
> > cat > $TMPC << EOF 
> > -@@ -4435,6 +4458,7 @@ echo "lzo support $lzo" 
> > - echo "snappy support $snappy" 
> > +@@ -4499,6 +4527,7 @@ echo "snappy support $snappy" 
> > echo "bzip2 support $bzip2" 
> > echo "NUMA host support $numa" 
> > + echo "tcmalloc support $tcmalloc" 
> > +echo "jemalloc support $jemalloc" 
> > 
> > if test "$sdl_too_old" = "yes"; then 
> > diff --git a/debian/patches/modify-query-machines.patch b/debian/patches/modify-query-machines.patch 
> > index d028e3e..5edcdd2 100644 
> > --- a/debian/patches/modify-query-machines.patch 
> > +++ b/debian/patches/modify-query-machines.patch 
> > @@ -13,7 +13,7 @@ Index: new/qapi-schema.json 
> > # 
> > @@ -2400,7 +2402,7 @@ 
> > ## 
> > - { 'type': 'MachineInfo', 
> > + { 'struct': 'MachineInfo', 
> > 'data': { 'name': 'str', '*alias': 'str', 
> > - '*is-default': 'bool', 'cpu-max': 'int' } } 
> > + '*is-default': 'bool', '*is-current': 'bool', 'cpu-max': 'int' } } 
> > diff --git a/debian/patches/modify-query-spice.patch b/debian/patches/modify-query-spice.patch 
> > index c485b03..4e5222d 100644 
> > --- a/debian/patches/modify-query-spice.patch 
> > +++ b/debian/patches/modify-query-spice.patch 
> > @@ -35,7 +35,7 @@ Index: new/qapi-schema.json 
> > +# 
> > # Since: 0.14.0 
> > ## 
> > - { 'type': 'SpiceInfo', 
> > + { 'struct': 'SpiceInfo', 
> > 'data': {'enabled': 'bool', 'migrated': 'bool', '*host': 'str', '*port': 'int', 
> > '*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str', 
> > + '*ticket': 'str', 
> > diff --git a/debian/patches/series b/debian/patches/series 
> > index 1105537..664e3f1 100644 
> > --- a/debian/patches/series 
> > +++ b/debian/patches/series 
> > @@ -30,6 +30,4 @@ backup-vma-extract-add-block-driver-type.patch 
> > glusterfs-daemonize.patch 
> > gluster-backupserver.patch 
> > add-qmp-get-link-status.patch 
> > -0001-friendlier-ai_flag-hints-for-ipv6-hosts.patch 
> > -0001-vvfat-add-a-label-option.patch 
> > jemalloc.patch 
> > diff --git a/debian/patches/virtio-balloon-fix-query.patch b/debian/patches/virtio-balloon-fix-query.patch 
> > index 236a624..741e44f 100644 
> > --- a/debian/patches/virtio-balloon-fix-query.patch 
> > +++ b/debian/patches/virtio-balloon-fix-query.patch 
> > @@ -116,8 +116,8 @@ Index: new/qapi-schema.json 
> > +# 
> > +# Since: 0.14.0 
> > ## 
> > --{ 'type': 'BalloonInfo', 'data': {'actual': 'int' } } 
> > -+{ 'type': 'BalloonInfo', 
> > +-{ 'struct': 'BalloonInfo', 'data': {'actual': 'int' } } 
> > ++{ 'struct': 'BalloonInfo', 
> > + 'data': {'actual': 'int', '*last_update': 'int', '*mem_swapped_in': 'int', 
> > + '*mem_swapped_out': 'int', '*major_page_faults': 'int', 
> > + '*minor_page_faults': 'int', '*free_mem': 'int', 
> > -- 
> > 2.1.4 
> > 
> > _______________________________________________ 
> > pve-devel mailing list 
> > pve-devel at pve.proxmox.com 
> > http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel 
> 
> 




More information about the pve-devel mailing list