[{"id":1762928,"web_url":"http://patchwork.ozlabs.org/comment/1762928/","msgid":"<20170905025901.GF22515@lemon.lan>","list_archive_url":null,"date":"2017-09-05T02:59:01","subject":"Re: [Qemu-devel] [PATCH v8 02/13] qemu.py: avoid writing to\n\tstdout/stderr","submitter":{"id":24872,"url":"http://patchwork.ozlabs.org/api/people/24872/","name":"Fam Zheng","email":"famz@redhat.com"},"content":"On Fri, 09/01 13:28, Amador Pahim wrote:\n> This module should not write directly to stdout/stderr. Instead, it\n> should either raise exceptions or just log the messages and let the\n> callers handle them and decide what to do. For example, scripts could\n> choose to send the log messages stderr or/and write them to a file if\n> verbose or debugging mode is enabled.\n> \n> This patch replaces the writes to stderr by an exception in the\n> send_fd_scm() when _socket_scm_helper is not set or not present. In the\n> same method, the subprocess Popen will now redirect the stdout/stderr to\n> logging.debug instead of writing to system stderr. As consequence, since\n> the Popen.communicate() is now used (in order to get the stdout), the\n> further call to wait() became redundant and was replaced by\n> Popen.returncode.\n> \n> The shutdown() message on negative exit code will now be logged\n> to logging.warn instead of written to system stderr.\n> \n> Signed-off-by: Amador Pahim \n\nReviewed-by: Fam Zheng ","headers":{"Return-Path":"","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=)","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 3xmWhT5Pmdz9sNc\n\tfor ;\n\tTue, 5 Sep 2017 13:01:45 +1000 (AEST)","from localhost ([::1]:56567 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dp47T-0000Ph-TZ\n\tfor incoming@patchwork.ozlabs.org; Mon, 04 Sep 2017 23:01:43 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:51424)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dp451-00074A-9i\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 22:59:15 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dp44w-0004SK-OF\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 22:59:11 -0400","from mx1.redhat.com ([209.132.183.28]:35490)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dp44w-0004Rg-Ht\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 22:59:06 -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 76070C047B73;\n\tTue, 5 Sep 2017 02:59:05 +0000 (UTC)","from localhost (ovpn-12-97.pek2.redhat.com [10.72.12.97])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id C57647EA8D;\n\tTue, 5 Sep 2017 02:59:02 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 76070C047B73","Date":"Tue, 5 Sep 2017 10:59:01 +0800","From":"Fam Zheng ","To":"Amador Pahim ","Message-ID":"<20170905025901.GF22515@lemon.lan>","References":"<20170901112829.2571-1-apahim@redhat.com>\n\t<20170901112829.2571-3-apahim@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170901112829.2571-3-apahim@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.31]);\n\tTue, 05 Sep 2017 02:59:05 +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 v8 02/13] qemu.py: avoid writing to\n\tstdout/stderr","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, ldoktor@redhat.com, ehabkost@redhat.com,\n\tstefanha@gmail.com, qemu-devel@nongnu.org, mreitz@redhat.com,\n\tcrosa@redhat.com, armbru@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1772662,"web_url":"http://patchwork.ozlabs.org/comment/1772662/","msgid":"<20170921103508.GB4677@localhost.localdomain>","list_archive_url":null,"date":"2017-09-21T10:35:08","subject":"Re: [Qemu-devel] [PATCH v8 02/13] qemu.py: avoid writing to\n\tstdout/stderr","submitter":{"id":2714,"url":"http://patchwork.ozlabs.org/api/people/2714/","name":"Kevin Wolf","email":"kwolf@redhat.com"},"content":"Am 01.09.2017 um 13:28 hat Amador Pahim geschrieben:\n> This module should not write directly to stdout/stderr. Instead, it\n> should either raise exceptions or just log the messages and let the\n> callers handle them and decide what to do. For example, scripts could\n> choose to send the log messages stderr or/and write them to a file if\n> verbose or debugging mode is enabled.\n> \n> This patch replaces the writes to stderr by an exception in the\n> send_fd_scm() when _socket_scm_helper is not set or not present. In the\n> same method, the subprocess Popen will now redirect the stdout/stderr to\n> logging.debug instead of writing to system stderr. As consequence, since\n> the Popen.communicate() is now used (in order to get the stdout), the\n> further call to wait() became redundant and was replaced by\n> Popen.returncode.\n> \n> The shutdown() message on negative exit code will now be logged\n> to logging.warn instead of written to system stderr.\n> \n> Signed-off-by: Amador Pahim \n> ---\n> scripts/qemu.py | 30 ++++++++++++++++++++++--------\n> 1 file changed, 22 insertions(+), 8 deletions(-)\n> \n> diff --git a/scripts/qemu.py b/scripts/qemu.py\n> index 0ae5d39414..aca6fa4d82 100644\n> --- a/scripts/qemu.py\n> +++ b/scripts/qemu.py\n> @@ -13,6 +13,7 @@\n> #\n> \n> import errno\n> +import logging\n> import string\n> import os\n> import sys\n> @@ -20,6 +21,15 @@ import subprocess\n> import qmp.qmp\n> \n> \n> +LOG = logging.getLogger(__name__)\n\nThis logger isn't properly initialised, so whenever it would actually be\nused, the only thing that ends up in the qemu-iotests output is:\n\n+No handlers could be found for logger \"qemu\"\n\nThe actual error message (that was previously logged to stderr) doesn't\nmake it to the output any more.\n\nI find it sad how the recent test infrastructure \"improvements\" tend to\nbreak the tests instead of making them easier to work with. Please be a\nbit more careful when touching this code, and *test* your code before\nyou submit it. I expect from anyone that they test their code, but if we\ndon't even manage to do that for the tests themselves, I'm losing hope.\n\nKevin","headers":{"Return-Path":"","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=)","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=kwolf@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 3xyY2B4qnCz9s0Z\n\tfor ;\n\tThu, 21 Sep 2017 20:36:50 +1000 (AEST)","from localhost ([::1]:52827 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1duyqe-0002OP-Nn\n\tfor incoming@patchwork.ozlabs.org; Thu, 21 Sep 2017 06:36:48 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:46804)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1duypF-0001p3-P0\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 06:35:22 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1duypB-0001pB-PY\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 06:35:21 -0400","from mx1.redhat.com ([209.132.183.28]:39356)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1duypB-0001o6-Ii\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 06:35:17 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\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 9F18B7EA90;\n\tThu, 21 Sep 2017 10:35:16 +0000 (UTC)","from localhost.localdomain (ovpn-116-78.ams2.redhat.com\n\t[10.36.116.78])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id D406E5EDE1;\n\tThu, 21 Sep 2017 10:35:09 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 9F18B7EA90","Date":"Thu, 21 Sep 2017 12:35:08 +0200","From":"Kevin Wolf ","To":"Amador Pahim ","Message-ID":"<20170921103508.GB4677@localhost.localdomain>","References":"<20170901112829.2571-1-apahim@redhat.com>\n\t<20170901112829.2571-3-apahim@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170901112829.2571-3-apahim@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tThu, 21 Sep 2017 10:35:16 +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 v8 02/13] qemu.py: avoid writing to\n\tstdout/stderr","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"ldoktor@redhat.com, ehabkost@redhat.com, stefanha@gmail.com,\n\tqemu-devel@nongnu.org, mreitz@redhat.com, crosa@redhat.com,\n\tfamz@redhat.com, armbru@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1772968,"web_url":"http://patchwork.ozlabs.org/comment/1772968/","msgid":"<20170921121711.GT3030@localhost.localdomain>","list_archive_url":null,"date":"2017-09-21T12:17:11","subject":"Re: [Qemu-devel] [PATCH v8 02/13] qemu.py: avoid writing to\n\tstdout/stderr","submitter":{"id":195,"url":"http://patchwork.ozlabs.org/api/people/195/","name":"Eduardo Habkost","email":"ehabkost@redhat.com"},"content":"On Thu, Sep 21, 2017 at 12:35:08PM +0200, Kevin Wolf wrote:\n> Am 01.09.2017 um 13:28 hat Amador Pahim geschrieben:\n> > This module should not write directly to stdout/stderr. Instead, it\n> > should either raise exceptions or just log the messages and let the\n> > callers handle them and decide what to do. For example, scripts could\n> > choose to send the log messages stderr or/and write them to a file if\n> > verbose or debugging mode is enabled.\n> > \n> > This patch replaces the writes to stderr by an exception in the\n> > send_fd_scm() when _socket_scm_helper is not set or not present. In the\n> > same method, the subprocess Popen will now redirect the stdout/stderr to\n> > logging.debug instead of writing to system stderr. As consequence, since\n> > the Popen.communicate() is now used (in order to get the stdout), the\n> > further call to wait() became redundant and was replaced by\n> > Popen.returncode.\n> > \n> > The shutdown() message on negative exit code will now be logged\n> > to logging.warn instead of written to system stderr.\n> > \n> > Signed-off-by: Amador Pahim \n> > ---\n> > scripts/qemu.py | 30 ++++++++++++++++++++++--------\n> > 1 file changed, 22 insertions(+), 8 deletions(-)\n> > \n> > diff --git a/scripts/qemu.py b/scripts/qemu.py\n> > index 0ae5d39414..aca6fa4d82 100644\n> > --- a/scripts/qemu.py\n> > +++ b/scripts/qemu.py\n> > @@ -13,6 +13,7 @@\n> > #\n> > \n> > import errno\n> > +import logging\n> > import string\n> > import os\n> > import sys\n> > @@ -20,6 +21,15 @@ import subprocess\n> > import qmp.qmp\n> > \n> > \n> > +LOG = logging.getLogger(__name__)\n> \n> This logger isn't properly initialised, so whenever it would actually be\n> used, the only thing that ends up in the qemu-iotests output is:\n> \n> +No handlers could be found for logger \"qemu\"\n> \n> The actual error message (that was previously logged to stderr) doesn't\n> make it to the output any more.\n> \n> I find it sad how the recent test infrastructure \"improvements\" tend to\n> break the tests instead of making them easier to work with. Please be a\n> bit more careful when touching this code, and *test* your code before\n> you submit it. I expect from anyone that they test their code, but if we\n> don't even manage to do that for the tests themselves, I'm losing hope.\n\nIs the test that was broken triggered as part of \"make check\"?\nWhat can we do to ensure it is?","headers":{"Return-Path":"","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=)","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=ehabkost@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 3xyhk454Byz9ryv\n\tfor ;\n\tFri, 22 Sep 2017 02:23:24 +1000 (AEST)","from localhost ([::1]:54565 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dv4G2-00045d-Oy\n\tfor incoming@patchwork.ozlabs.org; Thu, 21 Sep 2017 12:23:22 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:55064)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dv4FT-000458-49\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 12:22:49 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dv4FS-0003rw-5N\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 12:22:47 -0400","from mx1.redhat.com ([209.132.183.28]:38762)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dv4FR-0003pr-TF\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 12:22:46 -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 EC6CE81236;\n\tThu, 21 Sep 2017 16:22:44 +0000 (UTC)","from localhost (ovpn-116-204.phx2.redhat.com [10.3.116.204])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 5A5595C544;\n\tThu, 21 Sep 2017 16:22:42 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com EC6CE81236","Date":"Thu, 21 Sep 2017 09:17:11 -0300","From":"Eduardo Habkost ","To":"Kevin Wolf ","Message-ID":"<20170921121711.GT3030@localhost.localdomain>","References":"<20170901112829.2571-1-apahim@redhat.com>\n\t<20170901112829.2571-3-apahim@redhat.com>\n\t<20170921103508.GB4677@localhost.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170921103508.GB4677@localhost.localdomain>","X-Fnord":"you can see the fnord","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.25]);\n\tThu, 21 Sep 2017 16:22:45 +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 v8 02/13] qemu.py: avoid writing to\n\tstdout/stderr","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"ldoktor@redhat.com, famz@redhat.com, stefanha@gmail.com,\n\tAmador Pahim , qemu-devel@nongnu.org,\n\tmreitz@redhat.com, crosa@redhat.com, armbru@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]