Re: [Qemu-devel] [PATCH 4/7 V8] configure: add libpmem support

From: Igor Mammedov <imammedo@redhat.com>
To: junyan.he@gmx.com
Cc: qemu-devel@nongnu.org, Haozhong Zhang <haozhong.zhang@intel.com>,
	xiaoguangrong.eric@gmail.com, crosthwaite.peter@gmail.com,
	mst@redhat.com, dgilbert@redhat.com, ehabkost@redhat.com,
	quintela@redhat.com, Junyan He <junyan.he@intel.com>,
	stefanha@redhat.com, pbonzini@redhat.com, rth@twiddle.net
Subject: Re: [Qemu-devel] [PATCH 4/7 V8] configure: add libpmem support
Date: Tue, 10 Jul 2018 10:49:14 +0200	[thread overview]
Message-ID: <20180710104914.1ca245db@redhat.com> (raw)
In-Reply-To: <1531202789-31006-5-git-send-email-junyan.he@gmx.com>

On Tue, 10 Jul 2018 14:06:26 +0800
junyan.he@gmx.com wrote:

> From: Junyan He <junyan.he@intel.com>
> 
> Add a pair of configure options --{enable,disable}-libpmem to control
> whether QEMU is compiled with PMDK libpmem [1].
> 
> QEMU may write to the host persistent memory (e.g. in vNVDIMM label
> emulation and live migration), so it must take the proper operations
> to ensure the persistence of its own writes. Depending on the CPU
> models and available instructions, the optimal operation can vary [2].
> PMDK libpmem have already implemented those operations on multiple CPU
> models (x86 and ARM) and the logic to select the optimal ones, so QEMU
> can just use libpmem rather than re-implement them.
> 
> [1] PMDK (formerly known as NMVL), https://github.com/pmem/pmdk/
pls use document names in addition to links that could be found using search engine

> [2] https://github.com/pmem/pmdk/blob/38bfa652721a37fd94c0130ce0e3f5d8baa3ed40/src/libpmem/pmem.c#L33
it probably would be better to point out a specific specs/chapters
that describe difference. since the link can disappear and line change
so reader of commit message won't have any clue what [2] meant.

> 
> Signed-off-by: Junyan He <junyan.he@intel.com>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>

patch probably should be reordered and placed before 
[PATCH 3/7 V8] hostmem-file: add the 'pmem' option

> ---
>  configure | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/configure b/configure
> index 2a7796e..1c9288b 100755
> --- a/configure
> +++ b/configure
> @@ -475,6 +475,7 @@ vxhs=""
>  libxml2=""
>  docker="no"
>  debug_mutex="no"
> +libpmem=""
>  
>  # cross compilers defaults, can be overridden with --cross-cc-ARCH
>  cross_cc_aarch64="aarch64-linux-gnu-gcc"
> @@ -1435,6 +1436,10 @@ for opt do
>    ;;
>    --disable-debug-mutex) debug_mutex=no
>    ;;
> +  --enable-libpmem) libpmem=yes
> +  ;;
> +  --disable-libpmem) libpmem=no
> +  ;;
>    *)
>        echo "ERROR: unknown option $opt"
>        echo "Try '$0 --help' for more information"
> @@ -1710,6 +1715,7 @@ disabled with --disable-FEATURE, default is enabled if available:
>    vhost-user      vhost-user support
>    capstone        capstone disassembler support
>    debug-mutex     mutex debugging support
> +  libpmem         libpmem support
>  
>  NOTE: The object files are built at the place where configure is launched
>  EOF
> @@ -5546,6 +5552,24 @@ if has "docker"; then
>  fi
>  
>  ##########################################
> +# check for libpmem
> +
> +if test "$libpmem" != "no"; then
> +	if $pkg_config --exists "libpmem"; then
> +		libpmem="yes"
> +		libpmem_libs=$($pkg_config --libs libpmem)
> +		libpmem_cflags=$($pkg_config --cflags libpmem)
> +		libs_softmmu="$libs_softmmu $libpmem_libs"
> +		QEMU_CFLAGS="$QEMU_CFLAGS $libpmem_cflags"
> +	else
> +		if test "$libpmem" = "yes" ; then
> +			feature_not_found "libpmem" "Install nvml or pmdk"
> +		fi
> +		libpmem="no"
> +	fi
> +fi
> +
> +##########################################
>  # End of CC checks
>  # After here, no more $cc or $ld runs
>  
> @@ -6010,6 +6034,7 @@ echo "replication support $replication"
>  echo "VxHS block device $vxhs"
>  echo "capstone          $capstone"
>  echo "docker            $docker"
> +echo "libpmem support   $libpmem"
>  
>  if test "$sdl_too_old" = "yes"; then
>  echo "-> Your SDL version is too old - please upgrade to have SDL support"
> @@ -6763,6 +6788,10 @@ if test "$vxhs" = "yes" ; then
>    echo "VXHS_LIBS=$vxhs_libs" >> $config_host_mak
>  fi
>  
> +if test "$libpmem" = "yes" ; then
> +  echo "CONFIG_LIBPMEM=y" >> $config_host_mak
> +fi
> +
>  if test "$tcg_interpreter" = "yes"; then
>    QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
>  elif test "$ARCH" = "sparc64" ; then