{"id":814429,"url":"http://patchwork.ozlabs.org/api/patches/814429/?format=json","web_url":"http://patchwork.ozlabs.org/project/qemu-devel/patch/20170915233739.26860-13-ehabkost@redhat.com/","project":{"id":14,"url":"http://patchwork.ozlabs.org/api/projects/14/?format=json","name":"QEMU Development","link_name":"qemu-devel","list_id":"qemu-devel.nongnu.org","list_email":"qemu-devel@nongnu.org","web_url":"","scm_url":"","webscm_url":"","list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<20170915233739.26860-13-ehabkost@redhat.com>","list_archive_url":null,"date":"2017-09-15T23:37:36","name":"[PULL,12/15] qemu.py: avoid writing to stdout/stderr","commit_ref":null,"pull_url":null,"state":"new","archived":false,"hash":"7d34eed58271aa238427b5a3f1fe035a060bb6be","submitter":{"id":195,"url":"http://patchwork.ozlabs.org/api/people/195/?format=json","name":"Eduardo Habkost","email":"ehabkost@redhat.com"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/qemu-devel/patch/20170915233739.26860-13-ehabkost@redhat.com/mbox/","series":[{"id":3400,"url":"http://patchwork.ozlabs.org/api/series/3400/?format=json","web_url":"http://patchwork.ozlabs.org/project/qemu-devel/list/?series=3400","date":"2017-09-15T23:37:24","name":"[PULL,01/15] qemu.py: Pylint/style fixes","version":1,"mbox":"http://patchwork.ozlabs.org/series/3400/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/814429/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/814429/checks/","tags":{},"related":[],"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=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 3xvBkK6M9zz9sPk\n\tfor <incoming@patchwork.ozlabs.org>;\n\tSat, 16 Sep 2017 09:41:29 +1000 (AEST)","from localhost ([::1]:55365 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 1dt0Eh-0007MC-Rx\n\tfor incoming@patchwork.ozlabs.org; Fri, 15 Sep 2017 19:41:27 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56632)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <ehabkost@redhat.com>) id 1dt0C6-0005f0-UF\n\tfor qemu-devel@nongnu.org; Fri, 15 Sep 2017 19:38:50 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <ehabkost@redhat.com>) id 1dt0C5-0002aN-Q9\n\tfor qemu-devel@nongnu.org; Fri, 15 Sep 2017 19:38:46 -0400","from mx1.redhat.com ([209.132.183.28]:51322)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <ehabkost@redhat.com>) id 1dt0C5-0002Vw-IQ\n\tfor qemu-devel@nongnu.org; Fri, 15 Sep 2017 19:38:45 -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 A3E2CC04B927;\n\tFri, 15 Sep 2017 23:38:44 +0000 (UTC)","from localhost (ovpn-116-24.gru2.redhat.com [10.97.116.24])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 79B5760468;\n\tFri, 15 Sep 2017 23:38:39 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com A3E2CC04B927","From":"Eduardo Habkost <ehabkost@redhat.com>","To":"Peter Maydell <peter.maydell@linaro.org>,\n\tqemu-devel@nongnu.org","Date":"Fri, 15 Sep 2017 20:37:36 -0300","Message-Id":"<20170915233739.26860-13-ehabkost@redhat.com>","In-Reply-To":"<20170915233739.26860-1-ehabkost@redhat.com>","References":"<20170915233739.26860-1-ehabkost@redhat.com>","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, 15 Sep 2017 23:38:44 +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":"[Qemu-devel] [PULL 12/15] qemu.py: avoid writing to stdout/stderr","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":"=?utf-8?b?THVrw6HFoSBEb2t0b3I=?= <ldoktor@redhat.com>,\n\tAmador Pahim <apahim@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>","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>"},"content":"From: Amador Pahim <apahim@redhat.com>\n\nThis module should not write directly to stdout/stderr. Instead, it\nshould either raise exceptions or just log the messages and let the\ncallers handle them and decide what to do. For example, scripts could\nchoose to send the log messages stderr or/and write them to a file if\nverbose or debugging mode is enabled.\n\nThis patch replaces the writes to stderr by an exception in the\nsend_fd_scm() when _socket_scm_helper is not set or not present. In the\nsame method, the subprocess Popen will now redirect the stdout/stderr to\nlogging.debug instead of writing to system stderr. As consequence, since\nthe Popen.communicate() is now used (in order to get the stdout), the\nfurther call to wait() became redundant and was replaced by\nPopen.returncode.\n\nThe shutdown() message on negative exit code will now be logged\nto logging.warn instead of written to system stderr.\n\nSigned-off-by: Amador Pahim <apahim@redhat.com>\nMessage-Id: <20170901112829.2571-3-apahim@redhat.com>\nReviewed-by: Fam Zheng <famz@redhat.com>\nSigned-off-by: Eduardo Habkost <ehabkost@redhat.com>\n---\n scripts/qemu.py | 31 ++++++++++++++++++++++---------\n 1 file changed, 22 insertions(+), 9 deletions(-)","diff":"diff --git a/scripts/qemu.py b/scripts/qemu.py\nindex f80b008f7f..8d3d24dd2b 100644\n--- a/scripts/qemu.py\n+++ b/scripts/qemu.py\n@@ -13,12 +13,22 @@\n #\n \n import errno\n+import logging\n import os\n import sys\n import subprocess\n import qmp.qmp\n \n \n+LOG = logging.getLogger(__name__)\n+\n+\n+class QEMUMachineError(Exception):\n+    \"\"\"\n+    Exception called when an error in QEMUMachine happens.\n+    \"\"\"\n+\n+\n class MonitorResponseError(qmp.qmp.QMPError):\n     '''\n     Represents erroneous QMP monitor reply\n@@ -107,18 +117,21 @@ class QEMUMachine(object):\n         # In iotest.py, the qmp should always use unix socket.\n         assert self._qmp.is_scm_available()\n         if self._socket_scm_helper is None:\n-            print >>sys.stderr, \"No path to socket_scm_helper set\"\n-            return -1\n+            raise QEMUMachineError(\"No path to socket_scm_helper set\")\n         if not os.path.exists(self._socket_scm_helper):\n-            print >>sys.stderr, \"%s does not exist\" % self._socket_scm_helper\n-            return -1\n+            raise QEMUMachineError(\"%s does not exist\" %\n+                                   self._socket_scm_helper)\n         fd_param = [\"%s\" % self._socket_scm_helper,\n                     \"%d\" % self._qmp.get_sock_fd(),\n                     \"%s\" % fd_file_path]\n         devnull = open('/dev/null', 'rb')\n-        proc = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,\n-                                stderr=sys.stderr)\n-        return proc.wait()\n+        proc = subprocess.Popen(fd_param, stdin=devnull, stdout=subprocess.PIPE,\n+                                stderr=subprocess.STDOUT)\n+        output = proc.communicate()[0]\n+        if output:\n+            LOG.debug(output)\n+\n+        return proc.returncode\n \n     @staticmethod\n     def _remove_if_exists(path):\n@@ -202,8 +215,8 @@ class QEMUMachine(object):\n \n             exitcode = self._popen.wait()\n             if exitcode < 0:\n-                sys.stderr.write('qemu received signal %i: %s\\n'\n-                                 % (-exitcode, ' '.join(self._args)))\n+                LOG.warn('qemu received signal %i: %s', -exitcode,\n+                          ' '.join(self._args))\n             self._load_io_log()\n             self._post_shutdown()\n \n","prefixes":["PULL","12/15"]}