[{"id":1770027,"web_url":"http://patchwork.ozlabs.org/comment/1770027/","msgid":"<20170918094400.GC2951@redhat.com>","list_archive_url":null,"date":"2017-09-18T09:44:00","subject":"Re: [Qemu-devel] [PULL 14/15] qemu.py: improve message on negative\n\texit code","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"content":"On Fri, Sep 15, 2017 at 08:37:38PM -0300, Eduardo Habkost wrote:\n> From: Amador Pahim \n> \n> The current message shows 'self._args', which contains only part of the\n> options used in the Qemu command line.\n> \n> This patch makes the qemu full args list an instance variable and then\n> uses it in the negative exit code message.\n> \n> Message was moved outside the 'if is_running' block to make sure it will\n> be logged if the VM finishes before the call to shutdown().\n> \n> Signed-off-by: Amador Pahim \n> Message-Id: <20170901112829.2571-5-apahim@redhat.com>\n> [ehabkost: removed superfluous parenthesis]\n> Reviewed-by: Fam Zheng \n> Signed-off-by: Eduardo Habkost \n> ---\n> scripts/qemu.py | 24 +++++++++++++++++-------\n> 1 file changed, 17 insertions(+), 7 deletions(-)\n> \n> diff --git a/scripts/qemu.py b/scripts/qemu.py\n> index c9bcaafe41..9440261ac3 100644\n> --- a/scripts/qemu.py\n> +++ b/scripts/qemu.py\n> @@ -87,6 +87,7 @@ class QEMUMachine(object):\n> self._socket_scm_helper = socket_scm_helper\n> self._debug = debug\n> self._qmp = None\n> + self._qemu_full_args = None\n> \n> def __enter__(self):\n> return self\n> @@ -186,13 +187,16 @@ class QEMUMachine(object):\n> \n> def launch(self):\n> '''Launch the VM and establish a QMP connection'''\n> + self._qemu_full_args = None\n> devnull = open(os.path.devnull, 'rb')\n> qemulog = open(self._qemu_log_path, 'wb')\n> try:\n> self._pre_launch()\n> - args = (self._wrapper + [self._binary] + self._base_args() +\n> - self._args)\n> - self._popen = subprocess.Popen(args, stdin=devnull, stdout=qemulog,\n> + self._qemu_full_args = self._wrapper + [self._binary] +\n> + self._base_args() + self._args\n\nFYI, this change causes a syntax error because you need either the ()\nbrackets, or an explicit \\ continuation. Unfortunately this wasn't\ncaught by Peter's merge tests, since this code is only execised by\nthe qemu iotests which aren't run from a 'make check'.\n\nIf someone has cycles, it would be great to write some unit tests\ndirectly targetting the qemu.py and qmp.py code, so that we get test\ncoverage of it independantly of the iotest execution.\n\nRegards,\nDaniel","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-mx08.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx08.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 3xwh1y56vMz9s7g\n\tfor ;\n\tMon, 18 Sep 2017 19:45:10 +1000 (AEST)","from localhost ([::1]:35466 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dtsc0-0000OM-Sq\n\tfor incoming@patchwork.ozlabs.org; Mon, 18 Sep 2017 05:45:08 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:60163)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dtsb7-0008Rx-SP\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 05:44:17 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dtsb6-0005hz-PE\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 05:44:13 -0400","from mx1.redhat.com ([209.132.183.28]:37668)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dtsb6-0005hj-GR\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 05:44:12 -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 82C30C0587DB;\n\tMon, 18 Sep 2017 09:44:11 +0000 (UTC)","from redhat.com (unknown [10.42.22.189])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 0A6035D6A8;\n\tMon, 18 Sep 2017 09:44:02 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 82C30C0587DB","Date":"Mon, 18 Sep 2017 10:44:00 +0100","From":"\"Daniel P. Berrange\" ","To":"Eduardo Habkost ","Message-ID":"<20170918094400.GC2951@redhat.com>","References":"<20170915233739.26860-1-ehabkost@redhat.com>\n\t<20170915233739.26860-15-ehabkost@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20170915233739.26860-15-ehabkost@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.32]);\n\tMon, 18 Sep 2017 09:44:11 +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] [PULL 14/15] qemu.py: improve message on negative\n\texit code","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","Reply-To":"\"Daniel P. Berrange\" ","Cc":"=?utf-8?b?THVrw6HFoQ==?= Doktor ,\n\tPeter Maydell , Amador Pahim\n\t, qemu-devel@nongnu.org, Stefan Hajnoczi\n\t","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1770097,"web_url":"http://patchwork.ozlabs.org/comment/1770097/","msgid":"<20170918114623.GG10621@localhost.localdomain>","list_archive_url":null,"date":"2017-09-18T11:46:23","subject":"Re: [Qemu-devel] [PULL 14/15] qemu.py: improve message on negative\n\texit code","submitter":{"id":195,"url":"http://patchwork.ozlabs.org/api/people/195/","name":"Eduardo Habkost","email":"ehabkost@redhat.com"},"content":"On Mon, Sep 18, 2017 at 10:44:00AM +0100, Daniel P. Berrange wrote:\n> On Fri, Sep 15, 2017 at 08:37:38PM -0300, Eduardo Habkost wrote:\n> > From: Amador Pahim \n> > \n> > The current message shows 'self._args', which contains only part of the\n> > options used in the Qemu command line.\n> > \n> > This patch makes the qemu full args list an instance variable and then\n> > uses it in the negative exit code message.\n> > \n> > Message was moved outside the 'if is_running' block to make sure it will\n> > be logged if the VM finishes before the call to shutdown().\n> > \n> > Signed-off-by: Amador Pahim \n> > Message-Id: <20170901112829.2571-5-apahim@redhat.com>\n> > [ehabkost: removed superfluous parenthesis]\n> > Reviewed-by: Fam Zheng \n> > Signed-off-by: Eduardo Habkost \n> > ---\n> > scripts/qemu.py | 24 +++++++++++++++++-------\n> > 1 file changed, 17 insertions(+), 7 deletions(-)\n> > \n> > diff --git a/scripts/qemu.py b/scripts/qemu.py\n> > index c9bcaafe41..9440261ac3 100644\n> > --- a/scripts/qemu.py\n> > +++ b/scripts/qemu.py\n> > @@ -87,6 +87,7 @@ class QEMUMachine(object):\n> > self._socket_scm_helper = socket_scm_helper\n> > self._debug = debug\n> > self._qmp = None\n> > + self._qemu_full_args = None\n> > \n> > def __enter__(self):\n> > return self\n> > @@ -186,13 +187,16 @@ class QEMUMachine(object):\n> > \n> > def launch(self):\n> > '''Launch the VM and establish a QMP connection'''\n> > + self._qemu_full_args = None\n> > devnull = open(os.path.devnull, 'rb')\n> > qemulog = open(self._qemu_log_path, 'wb')\n> > try:\n> > self._pre_launch()\n> > - args = (self._wrapper + [self._binary] + self._base_args() +\n> > - self._args)\n> > - self._popen = subprocess.Popen(args, stdin=devnull, stdout=qemulog,\n> > + self._qemu_full_args = self._wrapper + [self._binary] +\n> > + self._base_args() + self._args\n> \n> FYI, this change causes a syntax error because you need either the ()\n> brackets, or an explicit \\ continuation. Unfortunately this wasn't\n> caught by Peter's merge tests, since this code is only execised by\n> the qemu iotests which aren't run from a 'make check'.\n> \n> If someone has cycles, it would be great to write some unit tests\n> directly targetting the qemu.py and qmp.py code, so that we get test\n> coverage of it independantly of the iotest execution.\n\nOops, sorry! I thought \"make check\" was exercising that code\npath.","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-mx09.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx09.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 3xwkkj4pVhz9rxj\n\tfor ;\n\tMon, 18 Sep 2017 21:47:07 +1000 (AEST)","from localhost ([::1]:35874 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dtuW1-0003qC-TT\n\tfor incoming@patchwork.ozlabs.org; Mon, 18 Sep 2017 07:47:05 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:43354)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dtuVW-0003pe-Is\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 07:46:35 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dtuVT-0000tU-GV\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 07:46:34 -0400","from mx1.redhat.com ([209.132.183.28]:47726)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dtuVT-0000sn-8B\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 07:46:31 -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 53B9C4E049;\n\tMon, 18 Sep 2017 11:46:29 +0000 (UTC)","from localhost (ovpn-116-24.gru2.redhat.com [10.97.116.24])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 6300D61B60;\n\tMon, 18 Sep 2017 11:46:25 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 53B9C4E049","Date":"Mon, 18 Sep 2017 08:46:23 -0300","From":"Eduardo Habkost ","To":"\"Daniel P. Berrange\" ","Message-ID":"<20170918114623.GG10621@localhost.localdomain>","References":"<20170915233739.26860-1-ehabkost@redhat.com>\n\t<20170915233739.26860-15-ehabkost@redhat.com>\n\t<20170918094400.GC2951@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170918094400.GC2951@redhat.com>","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.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.38]);\n\tMon, 18 Sep 2017 11:46:29 +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] [PULL 14/15] qemu.py: improve message on negative\n\texit code","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":"=?utf-8?b?THVrw6HFoQ==?= Doktor ,\n\tPeter Maydell , Amador Pahim\n\t, qemu-devel@nongnu.org, Stefan Hajnoczi\n\t","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]