[{"id":1765108,"web_url":"http://patchwork.ozlabs.org/comment/1765108/","msgid":"<20170908063106.GF4511@lemon>","list_archive_url":null,"date":"2017-09-08T06:31:06","subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","submitter":{"id":24872,"url":"http://patchwork.ozlabs.org/api/people/24872/","name":"Fam Zheng","email":"famz@redhat.com"},"content":"On Wed, 09/06 20:49, Fam Zheng wrote:\n> This patch groups the crypto objects into a few .mo objects based on\n> functional submodules, and moves inclusion conditions to *-objs\n> variables, then moves the global cflags/libs to the *-cflags and *-libs\n> variables.\n> \n> For init.o and cipher.o, which may or may not need the library flags\n> depending on config, adding flags and libs unconditionally doesn't hurt,\n> because if the library is not available, the variables are empty.  This\n> makes less code.\n> \n> Signed-off-by: Fam Zheng <famz@redhat.com>\n\nCc Dan. No idea why it was empty.\n\nFam","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=famz@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xpSCG55tlz9sBZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  8 Sep 2017 16:31:38 +1000 (AEST)","from localhost ([::1]:43528 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dqCpE-0001MG-RT\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 02:31:36 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:41502)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dqCot-0001Ls-11\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 02:31:19 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dqCoo-00035h-3w\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 02:31:15 -0400","from mx1.redhat.com ([209.132.183.28]:58866)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <famz@redhat.com>) id 1dqCon-00035a-Tz\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 02:31:10 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id CFF12883AB\n\tfor <qemu-devel@nongnu.org>; Fri,  8 Sep 2017 06:31:08 +0000 (UTC)","from localhost (ovpn-12-98.pek2.redhat.com [10.72.12.98])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 2016B5C548;\n\tFri,  8 Sep 2017 06:31:07 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com CFF12883AB","Date":"Fri, 8 Sep 2017 14:31:06 +0800","From":"Fam Zheng <famz@redhat.com>","To":"qemu-devel@nongnu.org","Message-ID":"<20170908063106.GF4511@lemon>","References":"<20170906124900.17354-1-famz@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170906124900.17354-1-famz@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.26]);\n\tFri, 08 Sep 2017 06:31:08 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1765114,"web_url":"http://patchwork.ozlabs.org/comment/1765114/","msgid":"<20170908065919.GG4511@lemon>","list_archive_url":null,"date":"2017-09-08T06:59:19","subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","submitter":{"id":24872,"url":"http://patchwork.ozlabs.org/api/people/24872/","name":"Fam Zheng","email":"famz@redhat.com"},"content":"On Fri, 09/08 14:31, Fam Zheng wrote:\n> On Wed, 09/06 20:49, Fam Zheng wrote:\n> > This patch groups the crypto objects into a few .mo objects based on\n> > functional submodules, and moves inclusion conditions to *-objs\n> > variables, then moves the global cflags/libs to the *-cflags and *-libs\n> > variables.\n> > \n> > For init.o and cipher.o, which may or may not need the library flags\n> > depending on config, adding flags and libs unconditionally doesn't hurt,\n> > because if the library is not available, the variables are empty.  This\n> > makes less code.\n> > \n> > Signed-off-by: Fam Zheng <famz@redhat.com>\n> \n> Cc Dan. No idea why it was empty.\n\nHmm, what is going on? Trying again with email address only.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=famz@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xpSqx6Y2Mz9s83\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  8 Sep 2017 16:59:55 +1000 (AEST)","from localhost ([::1]:43597 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dqDGb-0000Pu-Ds\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 02:59:53 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:52921)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dqDGB-0000PW-Ch\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 02:59:32 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dqDG6-0000QE-NB\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 02:59:27 -0400","from mx1.redhat.com ([209.132.183.28]:39906)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <famz@redhat.com>) id 1dqDG6-0000P7-Ho\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 02:59:22 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 57412C04B303\n\tfor <qemu-devel@nongnu.org>; Fri,  8 Sep 2017 06:59:21 +0000 (UTC)","from localhost (ovpn-12-98.pek2.redhat.com [10.72.12.98])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id B375E600C0;\n\tFri,  8 Sep 2017 06:59:20 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 57412C04B303","Date":"Fri, 8 Sep 2017 14:59:19 +0800","From":"Fam Zheng <famz@redhat.com>","To":"qemu-devel@nongnu.org","Message-ID":"<20170908065919.GG4511@lemon>","References":"<20170906124900.17354-1-famz@redhat.com>\n\t<20170908063106.GF4511@lemon>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170908063106.GF4511@lemon>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tFri, 08 Sep 2017 06:59:21 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1765219,"web_url":"http://patchwork.ozlabs.org/comment/1765219/","msgid":"<20170908100537.GI3609@redhat.com>","list_archive_url":null,"date":"2017-09-08T10:05:37","subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"content":"On Wed, Sep 06, 2017 at 08:49:00PM +0800, Fam Zheng wrote:\n> This patch groups the crypto objects into a few .mo objects based on\n> functional submodules, and moves inclusion conditions to *-objs\n> variables, then moves the global cflags/libs to the *-cflags and *-libs\n> variables.\n> \n> For init.o and cipher.o, which may or may not need the library flags\n> depending on config, adding flags and libs unconditionally doesn't hurt,\n> because if the library is not available, the variables are empty.  This\n> makes less code.\n> \n> Signed-off-by: Fam Zheng <famz@redhat.com>\n> \n> ---\n> \n> v4: Merge into one patch which is supposedly easier to manage and\n> review, and use .mo appraoch to avoid $(foreach) and $(eval) magics.\n\nI don't think using  .mo is suitable here. You've used it as a generic\nmechanism for grouping .o files, but that is not what it does. There\nare special semantics around .mo rules that affect how the final\nbinaries are linked.\n\neg looking back at the description of .mo files \n\n[quote]\ncommit c261d774fb9093d00e0938a19f502fb220f62718\nAuthor: Fam Zheng <famz@redhat.com>\nDate:   Mon Sep 1 18:35:10 2014 +0800\n\n[...snip...]\n\n    3) When linking an executable, those .mo files in its \"-y\" variables are\n       filtered out, and replaced by one or more -Wl,-u,$symbol flags. This\n       is done in the added macro \"process-archive-undefs\".\n    \n       These \"-Wl,-u,$symbol\" flags will force ld to pull in the function\n       definition from the archives when linking.\n    \n       Note that the .mo objects, that are actually meant to be linked in\n       the executables, are already expanded in unnest-vars, before the\n       linking command. So we are safe to simply filter out .mo for the\n       purpose of pulling undefined symbols.\n    \n       process-archive-undefs works as this: For each \".mo\", find all the\n       undefined symbols in it, filter ones that are defined in the\n       archives. For each of these symbols, generate a \"-Wl,-u,$symbol\" in\n       the link command, and put them before archive names in the command\n       line.\n[/quote]\n\nBased on this, I don't think I can ack this patch, because it can\nhave unexpected consequences.\n\n> ---\n>  configure              | 15 +++++-------\n>  crypto/Makefile.objs   | 66 ++++++++++++++++++++++++++++++++++++--------------\n>  tests/Makefile.include | 10 ++++----\n>  3 files changed, 59 insertions(+), 32 deletions(-)\n> \n> diff --git a/configure b/configure\n> index fb7e34a901..b37cd54bda 100755\n> --- a/configure\n> +++ b/configure\n> @@ -2472,9 +2472,6 @@ if test \"$gnutls\" != \"no\"; then\n>      if gnutls_works; then\n>          gnutls_cflags=$($pkg_config --cflags gnutls)\n>          gnutls_libs=$($pkg_config --libs gnutls)\n> -        libs_softmmu=\"$gnutls_libs $libs_softmmu\"\n> -        libs_tools=\"$gnutls_libs $libs_tools\"\n> -\tQEMU_CFLAGS=\"$QEMU_CFLAGS $gnutls_cflags\"\n>          gnutls=\"yes\"\n>  \n>  \t# gnutls_rnd requires >= 2.11.0\n> @@ -2568,9 +2565,6 @@ if test \"$gcrypt\" != \"no\"; then\n>          then\n>              gcrypt_libs=\"$gcrypt_libs -lgpg-error\"\n>          fi\n> -        libs_softmmu=\"$gcrypt_libs $libs_softmmu\"\n> -        libs_tools=\"$gcrypt_libs $libs_tools\"\n> -        QEMU_CFLAGS=\"$QEMU_CFLAGS $gcrypt_cflags\"\n>          gcrypt=\"yes\"\n>          if test -z \"$nettle\"; then\n>             nettle=\"no\"\n> @@ -2616,9 +2610,6 @@ if test \"$nettle\" != \"no\"; then\n>          nettle_cflags=$($pkg_config --cflags nettle)\n>          nettle_libs=$($pkg_config --libs nettle)\n>          nettle_version=$($pkg_config --modversion nettle)\n> -        libs_softmmu=\"$nettle_libs $libs_softmmu\"\n> -        libs_tools=\"$nettle_libs $libs_tools\"\n> -        QEMU_CFLAGS=\"$QEMU_CFLAGS $nettle_cflags\"\n>          nettle=\"yes\"\n>  \n>          cat > $TMPC << EOF\n> @@ -5713,12 +5704,16 @@ fi\n>  echo \"CONFIG_TLS_PRIORITY=\\\"$tls_priority\\\"\" >> $config_host_mak\n>  if test \"$gnutls\" = \"yes\" ; then\n>    echo \"CONFIG_GNUTLS=y\" >> $config_host_mak\n> +  echo \"GNUTLS_CFLAGS=$gnutls_cflags\" >> $config_host_mak\n> +  echo \"GNUTLS_LIBS=$gnutls_libs\" >> $config_host_mak\n>  fi\n>  if test \"$gnutls_rnd\" = \"yes\" ; then\n>    echo \"CONFIG_GNUTLS_RND=y\" >> $config_host_mak\n>  fi\n>  if test \"$gcrypt\" = \"yes\" ; then\n>    echo \"CONFIG_GCRYPT=y\" >> $config_host_mak\n> +  echo \"GCRYPT_CFLAGS=$gcrypt_cflags\" >> $config_host_mak\n> +  echo \"GCRYPT_LIBS=$gcrypt_libs\" >> $config_host_mak\n>    if test \"$gcrypt_hmac\" = \"yes\" ; then\n>      echo \"CONFIG_GCRYPT_HMAC=y\" >> $config_host_mak\n>    fi\n> @@ -5732,6 +5727,8 @@ if test \"$nettle\" = \"yes\" ; then\n>    if test \"$nettle_kdf\" = \"yes\" ; then\n>      echo \"CONFIG_NETTLE_KDF=y\" >> $config_host_mak\n>    fi\n> +  echo \"NETTLE_CFLAGS=$nettle_cflags\" >> $config_host_mak\n> +  echo \"NETTLE_LIBS=$nettle_libs\" >> $config_host_mak\n>  fi\n>  if test \"$tasn1\" = \"yes\" ; then\n>    echo \"CONFIG_TASN1=y\" >> $config_host_mak\n> diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs\n> index 2b99e08062..a3ff1417c7 100644\n> --- a/crypto/Makefile.objs\n> +++ b/crypto/Makefile.objs\n> @@ -1,29 +1,53 @@\n>  crypto-obj-y = init.o\n> -crypto-obj-y += hash.o\n> -crypto-obj-$(CONFIG_NETTLE) += hash-nettle.o\n> -crypto-obj-$(if $(CONFIG_NETTLE),n,$(CONFIG_GCRYPT)) += hash-gcrypt.o\n> -crypto-obj-$(if $(CONFIG_NETTLE),n,$(if $(CONFIG_GCRYPT),n,y)) += hash-glib.o\n> -crypto-obj-y += hmac.o\n> -crypto-obj-$(CONFIG_NETTLE) += hmac-nettle.o\n> -crypto-obj-$(CONFIG_GCRYPT_HMAC) += hmac-gcrypt.o\n> -crypto-obj-$(if $(CONFIG_NETTLE),n,$(if $(CONFIG_GCRYPT_HMAC),n,y)) += hmac-glib.o\n> +crypto-obj-y += hash.mo\n> +\n> +hash.mo-objs := hash.o \\\n> +\t$(if $(CONFIG_NETTLE), \\\n> +\t\thash-nettle.o, \\\n> +\t\t$(if $(CONFIG_GCRYPT), hash-gcrypt.o, hash-glib.o))\n> +hash.mo-cflags := $(NETTLE_CFLAGS) $(GCRYPT_CFLAGS)\n> +hash.mo-libs := $(NETTLE_LIBS) $(GCRYPT_LIBS)\n> +\n> +crypto-obj-y += hmac.mo\n> +hmac.mo-objs := hmac.o \\\n> +\t$(if $(CONFIG_NETTLE), \\\n> +\t\thmac-nettle.o, \\\n> +\t\t$(if $(CONFIG_GCRYPT_HMAC), hmac-gcrypt.o, hmac-glib.o))\n> +hmac.mo-cflags := $(NETTLE_CFLAGS) $(GCRYPT_CFLAGS)\n> +hmac.mo-libs := $(NETTLE_LIBS) $(GCRYPT_LIBS)\n> +\n>  crypto-obj-y += aes.o\n>  crypto-obj-y += desrfb.o\n>  crypto-obj-y += cipher.o\n>  crypto-obj-$(CONFIG_AF_ALG) += afalg.o\n>  crypto-obj-$(CONFIG_AF_ALG) += cipher-afalg.o\n>  crypto-obj-$(CONFIG_AF_ALG) += hash-afalg.o\n> -crypto-obj-y += tlscreds.o\n> -crypto-obj-y += tlscredsanon.o\n> -crypto-obj-y += tlscredsx509.o\n> -crypto-obj-y += tlssession.o\n> +\n> +crypto-obj-y += tls.mo\n> +tls.mo-objs := \\\n> +\ttlscreds.o \\\n> +\ttlscredsanon.o \\\n> +\ttlscredsx509.o \\\n> +\ttlssession.o\n> +tls.mo-cflags := $(GNUTLS_CFLAGS)\n> +tls.mo-libs := $(GNUTLS_LIBS)\n> +\n>  crypto-obj-y += secret.o\n> -crypto-obj-$(CONFIG_GCRYPT) += random-gcrypt.o\n> -crypto-obj-$(if $(CONFIG_GCRYPT),n,$(CONFIG_GNUTLS_RND)) += random-gnutls.o\n> -crypto-obj-$(if $(CONFIG_GCRYPT),n,$(if $(CONFIG_GNUTLS_RND),n,y)) += random-platform.o\n> -crypto-obj-y += pbkdf.o\n> -crypto-obj-$(CONFIG_NETTLE_KDF) += pbkdf-nettle.o\n> -crypto-obj-$(if $(CONFIG_NETTLE_KDF),n,$(CONFIG_GCRYPT_KDF)) += pbkdf-gcrypt.o\n> +\n> +crypto-obj-y += random.mo\n> +random.mo-objs := \\\n> +\t$(if $(CONFIG_GCRYPT), random-gcrypt.o, \\\n> +\t\t$(if $(CONFIG_GNUTLS_RND), random-gnutls.o, random-platform.o))\n> +random.mo-cflags := $(GNUTLS_CFLAGS) $(GCRYPT_CFLAGS)\n> +random.mo-libs := $(GNUTLS_LIBS) $(GCRYPT_LIBS)\n> +\n> +crypto-obj-y += pbkdf.mo\n> +pbkdf.mo-objs := pbkdf.o \\\n> +\t$(if $(CONFIG_NETTLE_KDF), pbkdf-nettle.o, \\\n> +\t\t$(if $(CONFIG_GCRYPT_KDF), pbkdf-gcrypt.o))\n> +pbkdf.mo-cflags := $(NETTLE_CFLAGS) $(GCRYPT_CFLAGS)\n> +pbkdf.mo-libs := $(NETTLE_LIBS) $(GCRYPT_LIBS)\n> +\n>  crypto-obj-y += ivgen.o\n>  crypto-obj-y += ivgen-essiv.o\n>  crypto-obj-y += ivgen-plain.o\n> @@ -34,6 +58,12 @@ crypto-obj-y += block.o\n>  crypto-obj-y += block-qcow.o\n>  crypto-obj-y += block-luks.o\n>  \n> +init.o-cflags := $(GNUTLS_CFLAGS) $(GCRYPT_CFLAGS)\n> +init.o-libs := $(GNUTLS_LIBS) $(GCRYPT_LIBS)\n> +\n> +cipher.o-cflags := $(NETTLE_CFLAGS) $(GCRYPT_CFLAGS)\n> +cipher.o-libs := $(NETTLE_LIBS) $(GCRYPT_LIBS)\n> +\n>  # Let the userspace emulators avoid linking gnutls/etc\n>  crypto-aes-obj-y = aes.o\n>  \n> diff --git a/tests/Makefile.include b/tests/Makefile.include\n> index fae5715e9c..d46c22d1ec 100644\n> --- a/tests/Makefile.include\n> +++ b/tests/Makefile.include\n> @@ -677,15 +677,15 @@ tests/benchmark-crypto-cipher$(EXESUF): tests/benchmark-crypto-cipher.o $(test-c\n>  tests/test-crypto-secret$(EXESUF): tests/test-crypto-secret.o $(test-crypto-obj-y)\n>  tests/test-crypto-xts$(EXESUF): tests/test-crypto-xts.o $(test-crypto-obj-y)\n>  \n> -tests/crypto-tls-x509-helpers.o-cflags := $(TASN1_CFLAGS)\n> -tests/crypto-tls-x509-helpers.o-libs := $(TASN1_LIBS)\n> -tests/pkix_asn1_tab.o-cflags := $(TASN1_CFLAGS)\n> +tests/crypto-tls-x509-helpers.o-cflags := $(TASN1_CFLAGS) $(GNUTLS_CFLAGS)\n> +tests/crypto-tls-x509-helpers.o-libs := $(TASN1_LIBS) $(GNUTLS_LIBS)\n> +tests/pkix_asn1_tab.o-cflags := $(TASN1_CFLAGS) $(GNUTLS_LIBS)\n>  \n> -tests/test-crypto-tlscredsx509.o-cflags := $(TASN1_CFLAGS)\n> +tests/test-crypto-tlscredsx509.o-cflags := $(TASN1_CFLAGS) $(GNUTLS_CFLAGS)\n>  tests/test-crypto-tlscredsx509$(EXESUF): tests/test-crypto-tlscredsx509.o \\\n>  \ttests/crypto-tls-x509-helpers.o tests/pkix_asn1_tab.o $(test-crypto-obj-y)\n>  \n> -tests/test-crypto-tlssession.o-cflags := $(TASN1_CFLAGS)\n> +tests/test-crypto-tlssession.o-cflags := $(TASN1_CFLAGS) $(GNUTLS_CFLAGS)\n>  tests/test-crypto-tlssession$(EXESUF): tests/test-crypto-tlssession.o \\\n>  \ttests/crypto-tls-x509-helpers.o tests/pkix_asn1_tab.o $(test-crypto-obj-y)\n>  tests/test-io-task$(EXESUF): tests/test-io-task.o $(test-io-obj-y)\n> -- \n> 2.13.5\n> \n\nRegards,\nDaniel","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=berrange@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xpY4301WRz9s3T\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  8 Sep 2017 20:10:42 +1000 (AEST)","from localhost ([::1]:44302 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dqGFE-0004Bi-MA\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 06:10:40 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:46311)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1dqGAX-0000Gp-Ij\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 06:05:55 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1dqGAR-00044d-VN\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 06:05:49 -0400","from mx1.redhat.com ([209.132.183.28]:51650)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <berrange@redhat.com>) id 1dqGAR-00044B-DC\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 06:05:43 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 69C5B80F90\n\tfor <qemu-devel@nongnu.org>; Fri,  8 Sep 2017 10:05:42 +0000 (UTC)","from redhat.com (unknown [10.33.36.66])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 99A685C8B7;\n\tFri,  8 Sep 2017 10:05:39 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 69C5B80F90","Date":"Fri, 8 Sep 2017 11:05:37 +0100","From":"\"Daniel P. Berrange\" <berrange@redhat.com>","To":"Fam Zheng <famz@redhat.com>","Message-ID":"<20170908100537.GI3609@redhat.com>","References":"<20170906124900.17354-1-famz@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20170906124900.17354-1-famz@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.27]);\n\tFri, 08 Sep 2017 10:05:42 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Reply-To":"\"Daniel P. Berrange\" <berrange@redhat.com>","Cc":"qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1765229,"web_url":"http://patchwork.ozlabs.org/comment/1765229/","msgid":"<20170908102701.GL4511@lemon>","list_archive_url":null,"date":"2017-09-08T10:27:01","subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","submitter":{"id":24872,"url":"http://patchwork.ozlabs.org/api/people/24872/","name":"Fam Zheng","email":"famz@redhat.com"},"content":"On Fri, 09/08 11:05, Daniel P. Berrange wrote:\n> On Wed, Sep 06, 2017 at 08:49:00PM +0800, Fam Zheng wrote:\n> > This patch groups the crypto objects into a few .mo objects based on\n> > functional submodules, and moves inclusion conditions to *-objs\n> > variables, then moves the global cflags/libs to the *-cflags and *-libs\n> > variables.\n> > \n> > For init.o and cipher.o, which may or may not need the library flags\n> > depending on config, adding flags and libs unconditionally doesn't hurt,\n> > because if the library is not available, the variables are empty.  This\n> > makes less code.\n> > \n> > Signed-off-by: Fam Zheng <famz@redhat.com>\n> > \n> > ---\n> > \n> > v4: Merge into one patch which is supposedly easier to manage and\n> > review, and use .mo appraoch to avoid $(foreach) and $(eval) magics.\n> \n> I don't think using  .mo is suitable here. You've used it as a generic\n> mechanism for grouping .o files, but that is not what it does. There\n> are special semantics around .mo rules that affect how the final\n> binaries are linked.\n\nUsing .mo is okay here, but after a hindsight I think grouping by library\n(nettle.mo, gcrypt.mo, etc.) is better than grouping by functionality, for\nmodularization in the future. But that also means assigning the cflags/libs\nvariable cannot be simplified like this.\n\n> \n> eg looking back at the description of .mo files \n> \n> [quote]\n> commit c261d774fb9093d00e0938a19f502fb220f62718\n> Author: Fam Zheng <famz@redhat.com>\n> Date:   Mon Sep 1 18:35:10 2014 +0800\n> \n> [...snip...]\n> \n>     3) When linking an executable, those .mo files in its \"-y\" variables are\n>        filtered out, and replaced by one or more -Wl,-u,$symbol flags. This\n>        is done in the added macro \"process-archive-undefs\".\n>     \n>        These \"-Wl,-u,$symbol\" flags will force ld to pull in the function\n>        definition from the archives when linking.\n>     \n>        Note that the .mo objects, that are actually meant to be linked in\n>        the executables, are already expanded in unnest-vars, before the\n>        linking command. So we are safe to simply filter out .mo for the\n>        purpose of pulling undefined symbols.\n>     \n>        process-archive-undefs works as this: For each \".mo\", find all the\n>        undefined symbols in it, filter ones that are defined in the\n>        archives. For each of these symbols, generate a \"-Wl,-u,$symbol\" in\n>        the link command, and put them before archive names in the command\n>        line.\n> [/quote]\n> \n> Based on this, I don't think I can ack this patch, because it can\n> have unexpected consequences.\n\nThis described the process-archive-undefs semantics of .mo, but not the essence\nof it.  Basically .mo is just partial linking with the additional services of\n-cflags, -libs and the above -Wl,-u thing. I cannot think of any unexpected\nconsequences with this change. We've had sdl.mo in ui/Makefile.objs for long,\njust for the same purpose of this patch, with no problem.\n\nFam","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=famz@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xpYWf0640z9s7f\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  8 Sep 2017 20:31:10 +1000 (AEST)","from localhost ([::1]:44414 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dqGZ1-0005ai-Vy\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 06:31:08 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:55089)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dqGVC-0002ib-1b\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 06:27:15 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dqGV6-0004AO-Sl\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 06:27:10 -0400","from mx1.redhat.com ([209.132.183.28]:40508)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <famz@redhat.com>) id 1dqGV6-00049m-Ja\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 06:27:04 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 83A25883AE\n\tfor <qemu-devel@nongnu.org>; Fri,  8 Sep 2017 10:27:03 +0000 (UTC)","from localhost (ovpn-12-98.pek2.redhat.com [10.72.12.98])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id DA976610B0;\n\tFri,  8 Sep 2017 10:27:02 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 83A25883AE","Date":"Fri, 8 Sep 2017 18:27:01 +0800","From":"Fam Zheng <famz@redhat.com>","To":"\"Daniel P. Berrange\" <berrange@redhat.com>","Message-ID":"<20170908102701.GL4511@lemon>","References":"<20170906124900.17354-1-famz@redhat.com>\n\t<20170908100537.GI3609@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170908100537.GI3609@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.26]);\n\tFri, 08 Sep 2017 10:27:03 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1765234,"web_url":"http://patchwork.ozlabs.org/comment/1765234/","msgid":"<20170908103602.GJ3609@redhat.com>","list_archive_url":null,"date":"2017-09-08T10:36:02","subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"content":"On Fri, Sep 08, 2017 at 06:27:01PM +0800, Fam Zheng wrote:\n> On Fri, 09/08 11:05, Daniel P. Berrange wrote:\n> > On Wed, Sep 06, 2017 at 08:49:00PM +0800, Fam Zheng wrote:\n> > > This patch groups the crypto objects into a few .mo objects based on\n> > > functional submodules, and moves inclusion conditions to *-objs\n> > > variables, then moves the global cflags/libs to the *-cflags and *-libs\n> > > variables.\n> > > \n> > > For init.o and cipher.o, which may or may not need the library flags\n> > > depending on config, adding flags and libs unconditionally doesn't hurt,\n> > > because if the library is not available, the variables are empty.  This\n> > > makes less code.\n> > > \n> > > Signed-off-by: Fam Zheng <famz@redhat.com>\n> > > \n> > > ---\n> > > \n> > > v4: Merge into one patch which is supposedly easier to manage and\n> > > review, and use .mo appraoch to avoid $(foreach) and $(eval) magics.\n> > \n> > I don't think using  .mo is suitable here. You've used it as a generic\n> > mechanism for grouping .o files, but that is not what it does. There\n> > are special semantics around .mo rules that affect how the final\n> > binaries are linked.\n> \n> Using .mo is okay here, but after a hindsight I think grouping by library\n> (nettle.mo, gcrypt.mo, etc.) is better than grouping by functionality, for\n> modularization in the future. But that also means assigning the cflags/libs\n> variable cannot be simplified like this.\n> \n> > \n> > eg looking back at the description of .mo files \n> > \n> > [quote]\n> > commit c261d774fb9093d00e0938a19f502fb220f62718\n> > Author: Fam Zheng <famz@redhat.com>\n> > Date:   Mon Sep 1 18:35:10 2014 +0800\n> > \n> > [...snip...]\n> > \n> >     3) When linking an executable, those .mo files in its \"-y\" variables are\n> >        filtered out, and replaced by one or more -Wl,-u,$symbol flags. This\n> >        is done in the added macro \"process-archive-undefs\".\n> >     \n> >        These \"-Wl,-u,$symbol\" flags will force ld to pull in the function\n> >        definition from the archives when linking.\n> >     \n> >        Note that the .mo objects, that are actually meant to be linked in\n> >        the executables, are already expanded in unnest-vars, before the\n> >        linking command. So we are safe to simply filter out .mo for the\n> >        purpose of pulling undefined symbols.\n> >     \n> >        process-archive-undefs works as this: For each \".mo\", find all the\n> >        undefined symbols in it, filter ones that are defined in the\n> >        archives. For each of these symbols, generate a \"-Wl,-u,$symbol\" in\n> >        the link command, and put them before archive names in the command\n> >        line.\n> > [/quote]\n> > \n> > Based on this, I don't think I can ack this patch, because it can\n> > have unexpected consequences.\n> \n> This described the process-archive-undefs semantics of .mo, but not the essence\n> of it.  Basically .mo is just partial linking with the additional services of\n> -cflags, -libs and the above -Wl,-u thing. I cannot think of any unexpected\n> consequences with this change. We've had sdl.mo in ui/Makefile.objs for long,\n> just for the same purpose of this patch, with no problem.\n\nWhile I'm in favour of moving the linker/compiler flags out of the global\nvars, I'm not convinced this impl is a step forward.\n\nWe already have a mechanism for grouping object files - the 'NNNN-obj-y'\nvariables we use throughout our Makefiles.\n\nThis patch is adding a second level of grouping purely to work around the\nfact that we can't set linker/compiler flags on the NNN-obj-y variables\nwe use. I think this second level of grouping makes the makefiles more\ncomplex than they ought to be.\n\nIOW, I'd rather see the rules fixed so that we can set variables against\nthe existing grouping we have. eg\n\n   crypto-obj-y-cflags := ...\n   crypto-obj-y-libs := ...\n\nso we avoid having to introduce second level groups every time we want\nto set these cflags/libs.\n\nRegards,\nDaniel","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=berrange@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xpYlC51PCz9s82\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  8 Sep 2017 20:41:11 +1000 (AEST)","from localhost ([::1]:44470 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dqGij-0006FJ-Kb\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 06:41:09 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:58494)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1dqGdx-000221-81\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 06:36:22 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1dqGds-00035a-90\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 06:36:13 -0400","from mx1.redhat.com ([209.132.183.28]:50036)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <berrange@redhat.com>) id 1dqGds-00034n-07\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 06:36:08 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id E986B81DFA\n\tfor <qemu-devel@nongnu.org>; Fri,  8 Sep 2017 10:36:06 +0000 (UTC)","from redhat.com (unknown [10.33.36.66])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 4F52F610B0;\n\tFri,  8 Sep 2017 10:36:04 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com E986B81DFA","Date":"Fri, 8 Sep 2017 11:36:02 +0100","From":"\"Daniel P. Berrange\" <berrange@redhat.com>","To":"Fam Zheng <famz@redhat.com>","Message-ID":"<20170908103602.GJ3609@redhat.com>","References":"<20170906124900.17354-1-famz@redhat.com>\n\t<20170908100537.GI3609@redhat.com> <20170908102701.GL4511@lemon>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20170908102701.GL4511@lemon>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tFri, 08 Sep 2017 10:36:07 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Reply-To":"\"Daniel P. Berrange\" <berrange@redhat.com>","Cc":"qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1765242,"web_url":"http://patchwork.ozlabs.org/comment/1765242/","msgid":"<20170908105853.GM4511@lemon>","list_archive_url":null,"date":"2017-09-08T10:58:53","subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","submitter":{"id":24872,"url":"http://patchwork.ozlabs.org/api/people/24872/","name":"Fam Zheng","email":"famz@redhat.com"},"content":"On Fri, 09/08 11:36, Daniel P. Berrange wrote:\n> On Fri, Sep 08, 2017 at 06:27:01PM +0800, Fam Zheng wrote:\n> > On Fri, 09/08 11:05, Daniel P. Berrange wrote:\n> > > On Wed, Sep 06, 2017 at 08:49:00PM +0800, Fam Zheng wrote:\n> > > > This patch groups the crypto objects into a few .mo objects based on\n> > > > functional submodules, and moves inclusion conditions to *-objs\n> > > > variables, then moves the global cflags/libs to the *-cflags and *-libs\n> > > > variables.\n> > > > \n> > > > For init.o and cipher.o, which may or may not need the library flags\n> > > > depending on config, adding flags and libs unconditionally doesn't hurt,\n> > > > because if the library is not available, the variables are empty.  This\n> > > > makes less code.\n> > > > \n> > > > Signed-off-by: Fam Zheng <famz@redhat.com>\n> > > > \n> > > > ---\n> > > > \n> > > > v4: Merge into one patch which is supposedly easier to manage and\n> > > > review, and use .mo appraoch to avoid $(foreach) and $(eval) magics.\n> > > \n> > > I don't think using  .mo is suitable here. You've used it as a generic\n> > > mechanism for grouping .o files, but that is not what it does. There\n> > > are special semantics around .mo rules that affect how the final\n> > > binaries are linked.\n> > \n> > Using .mo is okay here, but after a hindsight I think grouping by library\n> > (nettle.mo, gcrypt.mo, etc.) is better than grouping by functionality, for\n> > modularization in the future. But that also means assigning the cflags/libs\n> > variable cannot be simplified like this.\n> > \n> > > \n> > > eg looking back at the description of .mo files \n> > > \n> > > [quote]\n> > > commit c261d774fb9093d00e0938a19f502fb220f62718\n> > > Author: Fam Zheng <famz@redhat.com>\n> > > Date:   Mon Sep 1 18:35:10 2014 +0800\n> > > \n> > > [...snip...]\n> > > \n> > >     3) When linking an executable, those .mo files in its \"-y\" variables are\n> > >        filtered out, and replaced by one or more -Wl,-u,$symbol flags. This\n> > >        is done in the added macro \"process-archive-undefs\".\n> > >     \n> > >        These \"-Wl,-u,$symbol\" flags will force ld to pull in the function\n> > >        definition from the archives when linking.\n> > >     \n> > >        Note that the .mo objects, that are actually meant to be linked in\n> > >        the executables, are already expanded in unnest-vars, before the\n> > >        linking command. So we are safe to simply filter out .mo for the\n> > >        purpose of pulling undefined symbols.\n> > >     \n> > >        process-archive-undefs works as this: For each \".mo\", find all the\n> > >        undefined symbols in it, filter ones that are defined in the\n> > >        archives. For each of these symbols, generate a \"-Wl,-u,$symbol\" in\n> > >        the link command, and put them before archive names in the command\n> > >        line.\n> > > [/quote]\n> > > \n> > > Based on this, I don't think I can ack this patch, because it can\n> > > have unexpected consequences.\n> > \n> > This described the process-archive-undefs semantics of .mo, but not the essence\n> > of it.  Basically .mo is just partial linking with the additional services of\n> > -cflags, -libs and the above -Wl,-u thing. I cannot think of any unexpected\n> > consequences with this change. We've had sdl.mo in ui/Makefile.objs for long,\n> > just for the same purpose of this patch, with no problem.\n> \n> While I'm in favour of moving the linker/compiler flags out of the global\n> vars, I'm not convinced this impl is a step forward.\n> \n> We already have a mechanism for grouping object files - the 'NNNN-obj-y'\n> variables we use throughout our Makefiles.\n> \n> This patch is adding a second level of grouping purely to work around the\n> fact that we can't set linker/compiler flags on the NNN-obj-y variables\n> we use. I think this second level of grouping makes the makefiles more\n> complex than they ought to be.\n\nNot quite, it is actually a required step to modularization, which I'm inclined\nto get my hands on next. That is also why .mo was introduced.\n\n> \n> IOW, I'd rather see the rules fixed so that we can set variables against\n> the existing grouping we have. eg\n> \n>    crypto-obj-y-cflags := ...\n>    crypto-obj-y-libs := ...\n> \n> so we avoid having to introduce second level groups every time we want\n> to set these cflags/libs.\n\nThis is certainly true, but taking the modularization work into account, .mo\nbased -cflags and -libs are more natural and consistent. IMO we already have the\nlatter, so other mechanisms are not really necessary. Remember how complex the\ngeneral unnest-vars code is?  I believe adding support to crypto-obj-y-cflags is\nmore complex than (ab)using .mo objects, even if just for flags/libs\nlocalization.\n\nIf you don't like introducing {nettle,gcrypt,gnutls}.mo for now, we can probably\ndefer it to the time when crypto subsystem is modularized.\n\nEither way let's drop this patch for now.\n\nFam","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=famz@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xpZQM55Kjz9s83\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  8 Sep 2017 21:11:39 +1000 (AEST)","from localhost ([::1]:44654 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dqHCD-0007tP-M2\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 07:11:37 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:40119)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dqH0A-0005tK-D3\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 06:59:15 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dqH02-0005OM-Gu\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 06:59:10 -0400","from mx1.redhat.com ([209.132.183.28]:53986)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <famz@redhat.com>) id 1dqH02-0005O1-86\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 06:59:02 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id F0AA081DF1\n\tfor <qemu-devel@nongnu.org>; Fri,  8 Sep 2017 10:58:55 +0000 (UTC)","from localhost (ovpn-12-98.pek2.redhat.com [10.72.12.98])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 5081461982;\n\tFri,  8 Sep 2017 10:58:55 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com F0AA081DF1","Date":"Fri, 8 Sep 2017 18:58:53 +0800","From":"Fam Zheng <famz@redhat.com>","To":"\"Daniel P. Berrange\" <berrange@redhat.com>","Message-ID":"<20170908105853.GM4511@lemon>","References":"<20170906124900.17354-1-famz@redhat.com>\n\t<20170908100537.GI3609@redhat.com> <20170908102701.GL4511@lemon>\n\t<20170908103602.GJ3609@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170908103602.GJ3609@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tFri, 08 Sep 2017 10:58:56 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1765249,"web_url":"http://patchwork.ozlabs.org/comment/1765249/","msgid":"<20170908110033.GK3609@redhat.com>","list_archive_url":null,"date":"2017-09-08T11:00:33","subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"content":"On Fri, Sep 08, 2017 at 06:58:53PM +0800, Fam Zheng wrote:\n> On Fri, 09/08 11:36, Daniel P. Berrange wrote:\n> > On Fri, Sep 08, 2017 at 06:27:01PM +0800, Fam Zheng wrote:\n> > > On Fri, 09/08 11:05, Daniel P. Berrange wrote:\n> > > > On Wed, Sep 06, 2017 at 08:49:00PM +0800, Fam Zheng wrote:\n> > > > > This patch groups the crypto objects into a few .mo objects based on\n> > > > > functional submodules, and moves inclusion conditions to *-objs\n> > > > > variables, then moves the global cflags/libs to the *-cflags and *-libs\n> > > > > variables.\n> > > > > \n> > > > > For init.o and cipher.o, which may or may not need the library flags\n> > > > > depending on config, adding flags and libs unconditionally doesn't hurt,\n> > > > > because if the library is not available, the variables are empty.  This\n> > > > > makes less code.\n> > > > > \n> > > > > Signed-off-by: Fam Zheng <famz@redhat.com>\n> > > > > \n> > > > > ---\n> > > > > \n> > > > > v4: Merge into one patch which is supposedly easier to manage and\n> > > > > review, and use .mo appraoch to avoid $(foreach) and $(eval) magics.\n> > > > \n> > > > I don't think using  .mo is suitable here. You've used it as a generic\n> > > > mechanism for grouping .o files, but that is not what it does. There\n> > > > are special semantics around .mo rules that affect how the final\n> > > > binaries are linked.\n> > > \n> > > Using .mo is okay here, but after a hindsight I think grouping by library\n> > > (nettle.mo, gcrypt.mo, etc.) is better than grouping by functionality, for\n> > > modularization in the future. But that also means assigning the cflags/libs\n> > > variable cannot be simplified like this.\n> > > \n> > > > \n> > > > eg looking back at the description of .mo files \n> > > > \n> > > > [quote]\n> > > > commit c261d774fb9093d00e0938a19f502fb220f62718\n> > > > Author: Fam Zheng <famz@redhat.com>\n> > > > Date:   Mon Sep 1 18:35:10 2014 +0800\n> > > > \n> > > > [...snip...]\n> > > > \n> > > >     3) When linking an executable, those .mo files in its \"-y\" variables are\n> > > >        filtered out, and replaced by one or more -Wl,-u,$symbol flags. This\n> > > >        is done in the added macro \"process-archive-undefs\".\n> > > >     \n> > > >        These \"-Wl,-u,$symbol\" flags will force ld to pull in the function\n> > > >        definition from the archives when linking.\n> > > >     \n> > > >        Note that the .mo objects, that are actually meant to be linked in\n> > > >        the executables, are already expanded in unnest-vars, before the\n> > > >        linking command. So we are safe to simply filter out .mo for the\n> > > >        purpose of pulling undefined symbols.\n> > > >     \n> > > >        process-archive-undefs works as this: For each \".mo\", find all the\n> > > >        undefined symbols in it, filter ones that are defined in the\n> > > >        archives. For each of these symbols, generate a \"-Wl,-u,$symbol\" in\n> > > >        the link command, and put them before archive names in the command\n> > > >        line.\n> > > > [/quote]\n> > > > \n> > > > Based on this, I don't think I can ack this patch, because it can\n> > > > have unexpected consequences.\n> > > \n> > > This described the process-archive-undefs semantics of .mo, but not the essence\n> > > of it.  Basically .mo is just partial linking with the additional services of\n> > > -cflags, -libs and the above -Wl,-u thing. I cannot think of any unexpected\n> > > consequences with this change. We've had sdl.mo in ui/Makefile.objs for long,\n> > > just for the same purpose of this patch, with no problem.\n> > \n> > While I'm in favour of moving the linker/compiler flags out of the global\n> > vars, I'm not convinced this impl is a step forward.\n> > \n> > We already have a mechanism for grouping object files - the 'NNNN-obj-y'\n> > variables we use throughout our Makefiles.\n> > \n> > This patch is adding a second level of grouping purely to work around the\n> > fact that we can't set linker/compiler flags on the NNN-obj-y variables\n> > we use. I think this second level of grouping makes the makefiles more\n> > complex than they ought to be.\n> \n> Not quite, it is actually a required step to modularization, which I'm inclined\n> to get my hands on next. That is also why .mo was introduced.\n> \n> > \n> > IOW, I'd rather see the rules fixed so that we can set variables against\n> > the existing grouping we have. eg\n> > \n> >    crypto-obj-y-cflags := ...\n> >    crypto-obj-y-libs := ...\n> > \n> > so we avoid having to introduce second level groups every time we want\n> > to set these cflags/libs.\n> \n> This is certainly true, but taking the modularization work into account, .mo\n> based -cflags and -libs are more natural and consistent. IMO we already have the\n> latter, so other mechanisms are not really necessary. Remember how complex the\n> general unnest-vars code is?  I believe adding support to crypto-obj-y-cflags is\n> more complex than (ab)using .mo objects, even if just for flags/libs\n> localization.\n> \n> If you don't like introducing {nettle,gcrypt,gnutls}.mo for now, we can probably\n> defer it to the time when crypto subsystem is modularized.\n\nI don't anticipate the crypot subsystem ever being modularized - it is\nreally core functionality used across all other subsystems (block layer,\nchardev, ui, migration, and more)\n\n\nRegards,\nDaniel","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=berrange@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xpZWs1syGz9sCZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  8 Sep 2017 21:16:25 +1000 (AEST)","from localhost ([::1]:44697 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dqHGp-00040g-At\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 07:16:23 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:40807)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1dqH1e-0007LC-T1\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 07:00:51 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1dqH1Z-0006Dy-Qf\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 07:00:42 -0400","from mx1.redhat.com ([209.132.183.28]:53478)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <berrange@redhat.com>) id 1dqH1Z-0006DT-Gr\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 07:00:37 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 7E1D3356D9\n\tfor <qemu-devel@nongnu.org>; Fri,  8 Sep 2017 11:00:36 +0000 (UTC)","from redhat.com (unknown [10.33.36.66])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id D2DA15C8B7;\n\tFri,  8 Sep 2017 11:00:35 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 7E1D3356D9","Date":"Fri, 8 Sep 2017 12:00:33 +0100","From":"\"Daniel P. Berrange\" <berrange@redhat.com>","To":"Fam Zheng <famz@redhat.com>","Message-ID":"<20170908110033.GK3609@redhat.com>","References":"<20170906124900.17354-1-famz@redhat.com>\n\t<20170908100537.GI3609@redhat.com> <20170908102701.GL4511@lemon>\n\t<20170908103602.GJ3609@redhat.com> <20170908105853.GM4511@lemon>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20170908105853.GM4511@lemon>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tFri, 08 Sep 2017 11:00:36 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Reply-To":"\"Daniel P. Berrange\" <berrange@redhat.com>","Cc":"qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1765261,"web_url":"http://patchwork.ozlabs.org/comment/1765261/","msgid":"<20170908112352.GN4511@lemon>","list_archive_url":null,"date":"2017-09-08T11:23:52","subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","submitter":{"id":24872,"url":"http://patchwork.ozlabs.org/api/people/24872/","name":"Fam Zheng","email":"famz@redhat.com"},"content":"On Fri, 09/08 12:00, Daniel P. Berrange wrote:\n> On Fri, Sep 08, 2017 at 06:58:53PM +0800, Fam Zheng wrote:\n> > On Fri, 09/08 11:36, Daniel P. Berrange wrote:\n> > > On Fri, Sep 08, 2017 at 06:27:01PM +0800, Fam Zheng wrote:\n> > > > On Fri, 09/08 11:05, Daniel P. Berrange wrote:\n> > > > > On Wed, Sep 06, 2017 at 08:49:00PM +0800, Fam Zheng wrote:\n> > > > > > This patch groups the crypto objects into a few .mo objects based on\n> > > > > > functional submodules, and moves inclusion conditions to *-objs\n> > > > > > variables, then moves the global cflags/libs to the *-cflags and *-libs\n> > > > > > variables.\n> > > > > > \n> > > > > > For init.o and cipher.o, which may or may not need the library flags\n> > > > > > depending on config, adding flags and libs unconditionally doesn't hurt,\n> > > > > > because if the library is not available, the variables are empty.  This\n> > > > > > makes less code.\n> > > > > > \n> > > > > > Signed-off-by: Fam Zheng <famz@redhat.com>\n> > > > > > \n> > > > > > ---\n> > > > > > \n> > > > > > v4: Merge into one patch which is supposedly easier to manage and\n> > > > > > review, and use .mo appraoch to avoid $(foreach) and $(eval) magics.\n> > > > > \n> > > > > I don't think using  .mo is suitable here. You've used it as a generic\n> > > > > mechanism for grouping .o files, but that is not what it does. There\n> > > > > are special semantics around .mo rules that affect how the final\n> > > > > binaries are linked.\n> > > > \n> > > > Using .mo is okay here, but after a hindsight I think grouping by library\n> > > > (nettle.mo, gcrypt.mo, etc.) is better than grouping by functionality, for\n> > > > modularization in the future. But that also means assigning the cflags/libs\n> > > > variable cannot be simplified like this.\n> > > > \n> > > > > \n> > > > > eg looking back at the description of .mo files \n> > > > > \n> > > > > [quote]\n> > > > > commit c261d774fb9093d00e0938a19f502fb220f62718\n> > > > > Author: Fam Zheng <famz@redhat.com>\n> > > > > Date:   Mon Sep 1 18:35:10 2014 +0800\n> > > > > \n> > > > > [...snip...]\n> > > > > \n> > > > >     3) When linking an executable, those .mo files in its \"-y\" variables are\n> > > > >        filtered out, and replaced by one or more -Wl,-u,$symbol flags. This\n> > > > >        is done in the added macro \"process-archive-undefs\".\n> > > > >     \n> > > > >        These \"-Wl,-u,$symbol\" flags will force ld to pull in the function\n> > > > >        definition from the archives when linking.\n> > > > >     \n> > > > >        Note that the .mo objects, that are actually meant to be linked in\n> > > > >        the executables, are already expanded in unnest-vars, before the\n> > > > >        linking command. So we are safe to simply filter out .mo for the\n> > > > >        purpose of pulling undefined symbols.\n> > > > >     \n> > > > >        process-archive-undefs works as this: For each \".mo\", find all the\n> > > > >        undefined symbols in it, filter ones that are defined in the\n> > > > >        archives. For each of these symbols, generate a \"-Wl,-u,$symbol\" in\n> > > > >        the link command, and put them before archive names in the command\n> > > > >        line.\n> > > > > [/quote]\n> > > > > \n> > > > > Based on this, I don't think I can ack this patch, because it can\n> > > > > have unexpected consequences.\n> > > > \n> > > > This described the process-archive-undefs semantics of .mo, but not the essence\n> > > > of it.  Basically .mo is just partial linking with the additional services of\n> > > > -cflags, -libs and the above -Wl,-u thing. I cannot think of any unexpected\n> > > > consequences with this change. We've had sdl.mo in ui/Makefile.objs for long,\n> > > > just for the same purpose of this patch, with no problem.\n> > > \n> > > While I'm in favour of moving the linker/compiler flags out of the global\n> > > vars, I'm not convinced this impl is a step forward.\n> > > \n> > > We already have a mechanism for grouping object files - the 'NNNN-obj-y'\n> > > variables we use throughout our Makefiles.\n> > > \n> > > This patch is adding a second level of grouping purely to work around the\n> > > fact that we can't set linker/compiler flags on the NNN-obj-y variables\n> > > we use. I think this second level of grouping makes the makefiles more\n> > > complex than they ought to be.\n> > \n> > Not quite, it is actually a required step to modularization, which I'm inclined\n> > to get my hands on next. That is also why .mo was introduced.\n> > \n> > > \n> > > IOW, I'd rather see the rules fixed so that we can set variables against\n> > > the existing grouping we have. eg\n> > > \n> > >    crypto-obj-y-cflags := ...\n> > >    crypto-obj-y-libs := ...\n> > > \n> > > so we avoid having to introduce second level groups every time we want\n> > > to set these cflags/libs.\n> > \n> > This is certainly true, but taking the modularization work into account, .mo\n> > based -cflags and -libs are more natural and consistent. IMO we already have the\n> > latter, so other mechanisms are not really necessary. Remember how complex the\n> > general unnest-vars code is?  I believe adding support to crypto-obj-y-cflags is\n> > more complex than (ab)using .mo objects, even if just for flags/libs\n> > localization.\n> > \n> > If you don't like introducing {nettle,gcrypt,gnutls}.mo for now, we can probably\n> > defer it to the time when crypto subsystem is modularized.\n> \n> I don't anticipate the crypot subsystem ever being modularized - it is\n> really core functionality used across all other subsystems (block layer,\n> chardev, ui, migration, and more)\n\nI get your point that crypto is a fundamental thing, \"optionally secure\" is not\nwhat I meant.  But moduarization still has the advantage of offering more\nflexibility to end users, potentially. Crypto backends could be shipped as\nqemu-crypto-{nettle,gcrypt,gnutls} packages, and depending on which are\ninstalled and which are not, the core crypto code in QEMU can pick the suitable\nimplementation at runtime, based on a hardcoded priority or even an option.\n\nTo be \"secure by default\", qemu-crypto-nettle could be a hard requirement of\nqemu core package.\n\nIs it worth the effort?\n\nFam","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=famz@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xpZlJ0YM8z9s83\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  8 Sep 2017 21:26:20 +1000 (AEST)","from localhost ([::1]:44754 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dqHQQ-0006Hd-2C\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 07:26:18 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:50394)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dqHOC-0003n0-Fb\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 07:24:05 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dqHO7-00081D-Hk\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 07:24:00 -0400","from mx1.redhat.com ([209.132.183.28]:48900)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <famz@redhat.com>) id 1dqHO7-00080V-8Z\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 07:23:55 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 3157F81DE7\n\tfor <qemu-devel@nongnu.org>; Fri,  8 Sep 2017 11:23:54 +0000 (UTC)","from localhost (ovpn-12-98.pek2.redhat.com [10.72.12.98])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 874545D989;\n\tFri,  8 Sep 2017 11:23:53 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 3157F81DE7","Date":"Fri, 8 Sep 2017 19:23:52 +0800","From":"Fam Zheng <famz@redhat.com>","To":"\"Daniel P. Berrange\" <berrange@redhat.com>","Message-ID":"<20170908112352.GN4511@lemon>","References":"<20170906124900.17354-1-famz@redhat.com>\n\t<20170908100537.GI3609@redhat.com> <20170908102701.GL4511@lemon>\n\t<20170908103602.GJ3609@redhat.com> <20170908105853.GM4511@lemon>\n\t<20170908110033.GK3609@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170908110033.GK3609@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tFri, 08 Sep 2017 11:23:54 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1765294,"web_url":"http://patchwork.ozlabs.org/comment/1765294/","msgid":"<20170908123641.GA32645@redhat.com>","list_archive_url":null,"date":"2017-09-08T12:36:41","subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"content":"On Fri, Sep 08, 2017 at 07:23:52PM +0800, Fam Zheng wrote:\n> On Fri, 09/08 12:00, Daniel P. Berrange wrote:\n> > On Fri, Sep 08, 2017 at 06:58:53PM +0800, Fam Zheng wrote:\n> > > If you don't like introducing {nettle,gcrypt,gnutls}.mo for now, we can probably\n> > > defer it to the time when crypto subsystem is modularized.\n> > \n> > I don't anticipate the crypot subsystem ever being modularized - it is\n> > really core functionality used across all other subsystems (block layer,\n> > chardev, ui, migration, and more)\n> \n> I get your point that crypto is a fundamental thing, \"optionally secure\" is not\n> what I meant.  But moduarization still has the advantage of offering more\n> flexibility to end users, potentially. Crypto backends could be shipped as\n> qemu-crypto-{nettle,gcrypt,gnutls} packages, and depending on which are\n> installed and which are not, the core crypto code in QEMU can pick the suitable\n> implementation at runtime, based on a hardcoded priority or even an option.\n\nThere's no choice between gnutls vs the other two - it is always a case of\nbuilding gnutls *and* either nettle or gcrypt. We pick nettle or gcrypt based\non which is used by gnutls, to minimise number of different crypto libraries\nwe load. So dynamically picking them at runtime makese no sense.\n\n> To be \"secure by default\", qemu-crypto-nettle could be a hard requirement of\n> qemu core package.\n> \n> Is it worth the effort?\n\nI don't really think that is useful. You should think of the crypto stuff\nas being part of the 'libqemuutil.la' library - the only reason it is not\nlinked into that is because we wanted to avoid adds deps on the userspace\nemulators - everything else we build wants it present. \n\nRegards,\nDaniel","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=berrange@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xpcKM5qTfz9tYc\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  8 Sep 2017 22:37:27 +1000 (AEST)","from localhost ([::1]:45265 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dqIXF-0006Hu-B7\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 08:37:25 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:47354)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1dqIWl-0006H3-0S\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 08:36:56 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1dqIWh-0007jz-Tz\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 08:36:55 -0400","from mx1.redhat.com ([209.132.183.28]:45906)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <berrange@redhat.com>) id 1dqIWh-0007j4-Ne\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 08:36:51 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 1BB9F128E\n\tfor <qemu-devel@nongnu.org>; Fri,  8 Sep 2017 12:36:50 +0000 (UTC)","from redhat.com (unknown [10.33.36.66])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 775C11710E;\n\tFri,  8 Sep 2017 12:36:44 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 1BB9F128E","Date":"Fri, 8 Sep 2017 13:36:41 +0100","From":"\"Daniel P. Berrange\" <berrange@redhat.com>","To":"Fam Zheng <famz@redhat.com>","Message-ID":"<20170908123641.GA32645@redhat.com>","References":"<20170906124900.17354-1-famz@redhat.com>\n\t<20170908100537.GI3609@redhat.com> <20170908102701.GL4511@lemon>\n\t<20170908103602.GJ3609@redhat.com> <20170908105853.GM4511@lemon>\n\t<20170908110033.GK3609@redhat.com> <20170908112352.GN4511@lemon>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20170908112352.GN4511@lemon>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.29]);\n\tFri, 08 Sep 2017 12:36:50 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v4] buildsys: Move crypto cflags/libs to\n\tper object variables","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Reply-To":"\"Daniel P. Berrange\" <berrange@redhat.com>","Cc":"qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}}]