[{"id":1762938,"web_url":"http://patchwork.ozlabs.org/comment/1762938/","msgid":"<20170905031853.GN22515@lemon.lan>","list_archive_url":null,"date":"2017-09-05T03:18:53","subject":"Re: [Qemu-devel] [PATCH v8 06/13] qemu.py: make sure we only remove\n\tfiles we create","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> To launch a VM, we need to create basically two files: the monitor\n> socket (if it's a UNIX socket) and the qemu log file.\n> \n> For the qemu log file, we currently just open the path, which will\n> create the file if it does not exist or overwrite the file if it does\n> exist.\n> \n> For the monitor socket, if it already exists, we are currently removing\n> it, even if it's not created by us.\n> \n> This patch moves to pre_launch() the responsibility to make sure we only\n> create files that are not pre-existent and to populate a list of\n> controlled files. This list will then be used as the reference of\n> files to remove during the cleanup (post_shutdown()).\n> \n> Signed-off-by: Amador Pahim \n> ---\n> scripts/qemu.py | 30 +++++++++++++++++++++++-------\n> 1 file changed, 23 insertions(+), 7 deletions(-)\n> \n> diff --git a/scripts/qemu.py b/scripts/qemu.py\n> index 3ebe5ee0a4..c26e1412f9 100644\n> --- a/scripts/qemu.py\n> +++ b/scripts/qemu.py\n> @@ -41,6 +41,7 @@ class QEMUMachine(object):\n> monitor_address = os.path.join(test_dir, name + \"-monitor.sock\")\n> self._monitor_address = monitor_address\n> self._qemu_log_path = os.path.join(test_dir, name + \".log\")\n> + self._qemu_log_fd = None\n> self._popen = None\n> self._binary = binary\n> self._args = list(args) # Force copy args in case we modify them\n> @@ -50,6 +51,7 @@ class QEMUMachine(object):\n> self._socket_scm_helper = socket_scm_helper\n> self._debug = debug\n> self._qemu_full_args = None\n> + self._created_files = []\n> \n> # This can be used to add an unused monitor instance.\n> def add_monitor_telnet(self, ip, port):\n> @@ -128,30 +130,44 @@ class QEMUMachine(object):\n> '-display', 'none', '-vga', 'none']\n> \n> def _pre_launch(self):\n> - self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address, server=True,\n> - debug=self._debug)\n> + try:\n> + self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,\n> + server=True,\n> + debug=self._debug)\n> + except:\n> + raise\n\nWhat's the point of \"except: raise\"? It seems useless.\n\n> + else:\n> + if not isinstance(self._monitor_address, tuple):\n> + self._created_files.append(self._monitor_address)\n> +\n> + try:\n> + flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY\n> + os.open(self._qemu_log_path, flags)\n\nWhy change to os.open() instead of open()?\n\n> + except:\n> + raise\n> + else:\n> + self._created_files.append(self._qemu_log_path)\n> + self._qemu_log_fd = open(self._qemu_log_path, 'wb')\n> \n> def _post_launch(self):\n> self._qmp.accept()\n> \n> def _post_shutdown(self):\n> - if not isinstance(self._monitor_address, tuple):\n> - self._remove_if_exists(self._monitor_address)\n> - self._remove_if_exists(self._qemu_log_path)\n> + while self._created_files:\n> + self._remove_if_exists(self._created_files.pop())\n> \n> def launch(self):\n> '''Launch the VM and establish a QMP connection'''\n> self._iolog = None\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> self._qemu_full_args = (self._wrapper + [self._binary] +\n> self._base_args() + self._args)\n> self._popen = subprocess.Popen(self._qemu_full_args,\n> stdin=devnull,\n> - stdout=qemulog,\n> + stdout=self._qemu_log_fd,\n> stderr=subprocess.STDOUT,\n> shell=False)\n> self._post_launch()\n> -- \n> 2.13.5\n> \n\nFam","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-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.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 3xmX525KGfz9sNr\n\tfor ;\n\tTue, 5 Sep 2017 13:19:33 +1000 (AEST)","from localhost ([::1]:56616 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dp4Og-0008R8-6o\n\tfor incoming@patchwork.ozlabs.org; Mon, 04 Sep 2017 23:19:30 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:59831)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dp4OG-0008Ql-Pm\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 23:19:09 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dp4OC-0001cz-0w\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 23:19:04 -0400","from mx1.redhat.com ([209.132.183.28]:60390)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dp4OB-0001c0-Ns\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 23:18:59 -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 71009272CD;\n\tTue, 5 Sep 2017 03:18:58 +0000 (UTC)","from localhost (ovpn-12-97.pek2.redhat.com [10.72.12.97])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 8123A6F7F4;\n\tTue, 5 Sep 2017 03:18:55 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 71009272CD","Date":"Tue, 5 Sep 2017 11:18:53 +0800","From":"Fam Zheng ","To":"Amador Pahim ","Message-ID":"<20170905031853.GN22515@lemon.lan>","References":"<20170901112829.2571-1-apahim@redhat.com>\n\t<20170901112829.2571-7-apahim@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170901112829.2571-7-apahim@redhat.com>","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.39]);\n\tTue, 05 Sep 2017 03:18:58 +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 06/13] qemu.py: make sure we only remove\n\tfiles we create","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":1768798,"web_url":"http://patchwork.ozlabs.org/comment/1768798/","msgid":"","list_archive_url":null,"date":"2017-09-14T19:38:13","subject":"Re: [Qemu-devel] [PATCH v8 06/13] qemu.py: make sure we only remove\n\tfiles we create","submitter":{"id":71999,"url":"http://patchwork.ozlabs.org/api/people/71999/","name":"Amador Pahim","email":"apahim@redhat.com"},"content":"On Tue, Sep 5, 2017 at 5:18 AM, Fam Zheng wrote:\n> On Fri, 09/01 13:28, Amador Pahim wrote:\n>> To launch a VM, we need to create basically two files: the monitor\n>> socket (if it's a UNIX socket) and the qemu log file.\n>>\n>> For the qemu log file, we currently just open the path, which will\n>> create the file if it does not exist or overwrite the file if it does\n>> exist.\n>>\n>> For the monitor socket, if it already exists, we are currently removing\n>> it, even if it's not created by us.\n>>\n>> This patch moves to pre_launch() the responsibility to make sure we only\n>> create files that are not pre-existent and to populate a list of\n>> controlled files. This list will then be used as the reference of\n>> files to remove during the cleanup (post_shutdown()).\n>>\n>> Signed-off-by: Amador Pahim \n>> ---\n>> scripts/qemu.py | 30 +++++++++++++++++++++++-------\n>> 1 file changed, 23 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/scripts/qemu.py b/scripts/qemu.py\n>> index 3ebe5ee0a4..c26e1412f9 100644\n>> --- a/scripts/qemu.py\n>> +++ b/scripts/qemu.py\n>> @@ -41,6 +41,7 @@ class QEMUMachine(object):\n>> monitor_address = os.path.join(test_dir, name + \"-monitor.sock\")\n>> self._monitor_address = monitor_address\n>> self._qemu_log_path = os.path.join(test_dir, name + \".log\")\n>> + self._qemu_log_fd = None\n>> self._popen = None\n>> self._binary = binary\n>> self._args = list(args) # Force copy args in case we modify them\n>> @@ -50,6 +51,7 @@ class QEMUMachine(object):\n>> self._socket_scm_helper = socket_scm_helper\n>> self._debug = debug\n>> self._qemu_full_args = None\n>> + self._created_files = []\n>>\n>> # This can be used to add an unused monitor instance.\n>> def add_monitor_telnet(self, ip, port):\n>> @@ -128,30 +130,44 @@ class QEMUMachine(object):\n>> '-display', 'none', '-vga', 'none']\n>>\n>> def _pre_launch(self):\n>> - self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address, server=True,\n>> - debug=self._debug)\n>> + try:\n>> + self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,\n>> + server=True,\n>> + debug=self._debug)\n>> + except:\n>> + raise\n>\n> What's the point of \"except: raise\"? It seems useless.\n\nThe point is to execute the block in the else only when no exception\nhappens. When some exception happens, I want to raise it without\nexecuting the else block.\n\n>\n>> + else:\n>> + if not isinstance(self._monitor_address, tuple):\n>> + self._created_files.append(self._monitor_address)\n>> +\n>> + try:\n>> + flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY\n>> + os.open(self._qemu_log_path, flags)\n>\n> Why change to os.open() instead of open()?\n\nI want to create the file only if it does not exist. The open() flag\n'x' is available only in python 3.3. For python <3.3, we need the\nos.open() to have that feature.\n\n>\n>> + except:\n>> + raise\n>> + else:\n>> + self._created_files.append(self._qemu_log_path)\n>> + self._qemu_log_fd = open(self._qemu_log_path, 'wb')\n>>\n>> def _post_launch(self):\n>> self._qmp.accept()\n>>\n>> def _post_shutdown(self):\n>> - if not isinstance(self._monitor_address, tuple):\n>> - self._remove_if_exists(self._monitor_address)\n>> - self._remove_if_exists(self._qemu_log_path)\n>> + while self._created_files:\n>> + self._remove_if_exists(self._created_files.pop())\n>>\n>> def launch(self):\n>> '''Launch the VM and establish a QMP connection'''\n>> self._iolog = None\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>> self._qemu_full_args = (self._wrapper + [self._binary] +\n>> self._base_args() + self._args)\n>> self._popen = subprocess.Popen(self._qemu_full_args,\n>> stdin=devnull,\n>> - stdout=qemulog,\n>> + stdout=self._qemu_log_fd,\n>> stderr=subprocess.STDOUT,\n>> shell=False)\n>> self._post_launch()\n>> --\n>> 2.13.5\n>>\n>\n> Fam","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=)","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 3xtTPF2YPnz9s7c\n\tfor ;\n\tFri, 15 Sep 2017 05:39:13 +1000 (AEST)","from localhost ([::1]:49721 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dsZyh-0000Z3-Et\n\tfor incoming@patchwork.ozlabs.org; Thu, 14 Sep 2017 15:39:11 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:33314)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dsZxr-00006n-F7\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 15:38:22 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dsZxo-0007C4-7P\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 15:38:19 -0400","from mail-io0-f178.google.com ([209.85.223.178]:55418)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from ) id 1dsZxo-0007Ay-2b\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 15:38:16 -0400","by mail-io0-f178.google.com with SMTP id z187so3191367ioz.12\n\tfor ; Thu, 14 Sep 2017 12:38:15 -0700 (PDT)","by 10.107.43.147 with HTTP; Thu, 14 Sep 2017 12:38:13 -0700 (PDT)"],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=V0hAvHW1+HJhC6d+H0EpOMQqoecthUftpfU/nG3s9J8=;\n\tb=jBIxMls9Clf5DfhQIIDJNsZbIwp6LNGYBMVSontIx25U4+FiaVXvrqaztcyHff0UcR\n\tUeMpjwwfiE+db5k2bsodZg7uP22+5ik1AU6w0xoVuXolT+pha/w0s26dOpHZQfVJFnuG\n\tCRYxPK/jri3azMn6nyKZZjRmR2VcMUdGGeFUcBQgcRhlTvSIm2dbmRwrAWYdVUyez8bF\n\te56Dm4mQxpMeZ9njGCZipbAI+vsStspVyfP5wsbOL1DIxY/iH1o79wALuSebutSsWQfK\n\tvzueNTvp+n5D5HexJ2bH/CnLD+FsXGDQdbVobztgdfy7Crjfqe2a3d+kznC1hMAmIPF+\n\t8EZg==","X-Gm-Message-State":"AHPjjUjgA7cy/n1v38Ffh3P3fGWWjnegznEBrnH+8frkIk+Agqq94adg\n\tSDTQjXuUKIECGtqQjl40xNckZcfiSa3lpvBC3vuYLg==","X-Google-Smtp-Source":"AOwi7QC+jMy0Y6g9unlsKbDQYdfujSWGcDuvUebrSbAoEOi/ky/ElrW/hXVK50+muboX35v7Va3nRRJmUjuuRCRR7GQ=","X-Received":"by 10.107.57.70 with SMTP id g67mr5122167ioa.224.1505417894378; \n\tThu, 14 Sep 2017 12:38:14 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170905031853.GN22515@lemon.lan>","References":"<20170901112829.2571-1-apahim@redhat.com>\n\t<20170901112829.2571-7-apahim@redhat.com>\n\t<20170905031853.GN22515@lemon.lan>","From":"Amador Pahim ","Date":"Thu, 14 Sep 2017 21:38:13 +0200","Message-ID":"","To":"Fam Zheng ","Content-Type":"text/plain; charset=\"UTF-8\"","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.85.223.178","Subject":"Re: [Qemu-devel] [PATCH v8 06/13] qemu.py: make sure we only remove\n\tfiles we create","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":"Kevin Wolf , =?utf-8?b?THVrw6HFoSBEb2t0b3I=?=\n\t, \tEduardo Habkost ,\n\tStefan Hajnoczi , qemu-devel@nongnu.org, Max Reitz\n\t, Cleber Rosa , Markus Armbruster\n\t","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1768803,"web_url":"http://patchwork.ozlabs.org/comment/1768803/","msgid":"<20170914194610.GB23492@localhost.localdomain>","list_archive_url":null,"date":"2017-09-14T19:46:10","subject":"Re: [Qemu-devel] [PATCH v8 06/13] qemu.py: make sure we only remove\n\tfiles we create","submitter":{"id":195,"url":"http://patchwork.ozlabs.org/api/people/195/","name":"Eduardo Habkost","email":"ehabkost@redhat.com"},"content":"On Thu, Sep 14, 2017 at 09:38:13PM +0200, Amador Pahim wrote:\n> On Tue, Sep 5, 2017 at 5:18 AM, Fam Zheng wrote:\n> > On Fri, 09/01 13:28, Amador Pahim wrote:\n> >> To launch a VM, we need to create basically two files: the monitor\n> >> socket (if it's a UNIX socket) and the qemu log file.\n> >>\n> >> For the qemu log file, we currently just open the path, which will\n> >> create the file if it does not exist or overwrite the file if it does\n> >> exist.\n> >>\n> >> For the monitor socket, if it already exists, we are currently removing\n> >> it, even if it's not created by us.\n> >>\n> >> This patch moves to pre_launch() the responsibility to make sure we only\n> >> create files that are not pre-existent and to populate a list of\n> >> controlled files. This list will then be used as the reference of\n> >> files to remove during the cleanup (post_shutdown()).\n> >>\n> >> Signed-off-by: Amador Pahim \n> >> ---\n> >> scripts/qemu.py | 30 +++++++++++++++++++++++-------\n> >> 1 file changed, 23 insertions(+), 7 deletions(-)\n> >>\n> >> diff --git a/scripts/qemu.py b/scripts/qemu.py\n> >> index 3ebe5ee0a4..c26e1412f9 100644\n> >> --- a/scripts/qemu.py\n> >> +++ b/scripts/qemu.py\n> >> @@ -41,6 +41,7 @@ class QEMUMachine(object):\n> >> monitor_address = os.path.join(test_dir, name + \"-monitor.sock\")\n> >> self._monitor_address = monitor_address\n> >> self._qemu_log_path = os.path.join(test_dir, name + \".log\")\n> >> + self._qemu_log_fd = None\n> >> self._popen = None\n> >> self._binary = binary\n> >> self._args = list(args) # Force copy args in case we modify them\n> >> @@ -50,6 +51,7 @@ class QEMUMachine(object):\n> >> self._socket_scm_helper = socket_scm_helper\n> >> self._debug = debug\n> >> self._qemu_full_args = None\n> >> + self._created_files = []\n> >>\n> >> # This can be used to add an unused monitor instance.\n> >> def add_monitor_telnet(self, ip, port):\n> >> @@ -128,30 +130,44 @@ class QEMUMachine(object):\n> >> '-display', 'none', '-vga', 'none']\n> >>\n> >> def _pre_launch(self):\n> >> - self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address, server=True,\n> >> - debug=self._debug)\n> >> + try:\n> >> + self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,\n> >> + server=True,\n> >> + debug=self._debug)\n> >> + except:\n> >> + raise\n> >\n> > What's the point of \"except: raise\"? It seems useless.\n> \n> The point is to execute the block in the else only when no exception\n> happens. When some exception happens, I want to raise it without\n> executing the else block.\n\nIsn't this exactly what Python does when an exception is raised\nwith no \"try\" block?\n\n\n> \n> >\n> >> + else:\n> >> + if not isinstance(self._monitor_address, tuple):\n> >> + self._created_files.append(self._monitor_address)\n> >> +\n> >> + try:\n> >> + flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY\n> >> + os.open(self._qemu_log_path, flags)\n> >\n> > Why change to os.open() instead of open()?\n> \n> I want to create the file only if it does not exist. The open() flag\n> 'x' is available only in python 3.3. For python <3.3, we need the\n> os.open() to have that feature.\n\nI'm not sure this extra complexity is really necessary. We could\nfix all that by using mkdtemp() and deleting the temporary\ndirectory on shutdown.\n\n> \n> >\n> >> + except:\n> >> + raise\n> >> + else:\n> >> + self._created_files.append(self._qemu_log_path)\n> >> + self._qemu_log_fd = open(self._qemu_log_path, 'wb')\n> >>\n> >> def _post_launch(self):\n> >> self._qmp.accept()\n> >>\n> >> def _post_shutdown(self):\n> >> - if not isinstance(self._monitor_address, tuple):\n> >> - self._remove_if_exists(self._monitor_address)\n> >> - self._remove_if_exists(self._qemu_log_path)\n> >> + while self._created_files:\n> >> + self._remove_if_exists(self._created_files.pop())\n> >>\n> >> def launch(self):\n> >> '''Launch the VM and establish a QMP connection'''\n> >> self._iolog = None\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> >> self._qemu_full_args = (self._wrapper + [self._binary] +\n> >> self._base_args() + self._args)\n> >> self._popen = subprocess.Popen(self._qemu_full_args,\n> >> stdin=devnull,\n> >> - stdout=qemulog,\n> >> + stdout=self._qemu_log_fd,\n> >> stderr=subprocess.STDOUT,\n> >> shell=False)\n> >> self._post_launch()\n> >> --\n> >> 2.13.5\n> >>\n> >\n> > Fam","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=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 3xtTYx2496z9s72\n\tfor ;\n\tFri, 15 Sep 2017 05:46:44 +1000 (AEST)","from localhost ([::1]:49753 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dsa5x-00046I-OR\n\tfor incoming@patchwork.ozlabs.org; Thu, 14 Sep 2017 15:46:41 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:38422)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dsa5e-000468-Is\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 15:46:23 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dsa5Z-0001MG-GU\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 15:46:22 -0400","from mx1.redhat.com ([209.132.183.28]:34392)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dsa5Z-0001KA-7N\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 15:46:17 -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 DFA3DC04D28D;\n\tThu, 14 Sep 2017 19:46:15 +0000 (UTC)","from localhost (ovpn-116-51.gru2.redhat.com [10.97.116.51])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 5ABAA65E97;\n\tThu, 14 Sep 2017 19:46:11 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com DFA3DC04D28D","Date":"Thu, 14 Sep 2017 16:46:10 -0300","From":"Eduardo Habkost ","To":"Amador Pahim ","Message-ID":"<20170914194610.GB23492@localhost.localdomain>","References":"<20170901112829.2571-1-apahim@redhat.com>\n\t<20170901112829.2571-7-apahim@redhat.com>\n\t<20170905031853.GN22515@lemon.lan>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","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.31]);\n\tThu, 14 Sep 2017 19:46: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 06/13] qemu.py: make sure we only remove\n\tfiles we create","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":"Kevin Wolf , =?utf-8?b?THVrw6HFoQ==?=\n\tDoktor , Stefan Hajnoczi ,\n\tqemu-devel@nongnu.org, Max Reitz ,\n\tCleber Rosa , Fam Zheng ,\n\tMarkus Armbruster ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1768814,"web_url":"http://patchwork.ozlabs.org/comment/1768814/","msgid":"","list_archive_url":null,"date":"2017-09-14T20:05:50","subject":"Re: [Qemu-devel] [PATCH v8 06/13] qemu.py: make sure we only remove\n\tfiles we create","submitter":{"id":71999,"url":"http://patchwork.ozlabs.org/api/people/71999/","name":"Amador Pahim","email":"apahim@redhat.com"},"content":"On Thu, Sep 14, 2017 at 9:46 PM, Eduardo Habkost wrote:\n> On Thu, Sep 14, 2017 at 09:38:13PM +0200, Amador Pahim wrote:\n>> On Tue, Sep 5, 2017 at 5:18 AM, Fam Zheng wrote:\n>> > On Fri, 09/01 13:28, Amador Pahim wrote:\n>> >> To launch a VM, we need to create basically two files: the monitor\n>> >> socket (if it's a UNIX socket) and the qemu log file.\n>> >>\n>> >> For the qemu log file, we currently just open the path, which will\n>> >> create the file if it does not exist or overwrite the file if it does\n>> >> exist.\n>> >>\n>> >> For the monitor socket, if it already exists, we are currently removing\n>> >> it, even if it's not created by us.\n>> >>\n>> >> This patch moves to pre_launch() the responsibility to make sure we only\n>> >> create files that are not pre-existent and to populate a list of\n>> >> controlled files. This list will then be used as the reference of\n>> >> files to remove during the cleanup (post_shutdown()).\n>> >>\n>> >> Signed-off-by: Amador Pahim \n>> >> ---\n>> >> scripts/qemu.py | 30 +++++++++++++++++++++++-------\n>> >> 1 file changed, 23 insertions(+), 7 deletions(-)\n>> >>\n>> >> diff --git a/scripts/qemu.py b/scripts/qemu.py\n>> >> index 3ebe5ee0a4..c26e1412f9 100644\n>> >> --- a/scripts/qemu.py\n>> >> +++ b/scripts/qemu.py\n>> >> @@ -41,6 +41,7 @@ class QEMUMachine(object):\n>> >> monitor_address = os.path.join(test_dir, name + \"-monitor.sock\")\n>> >> self._monitor_address = monitor_address\n>> >> self._qemu_log_path = os.path.join(test_dir, name + \".log\")\n>> >> + self._qemu_log_fd = None\n>> >> self._popen = None\n>> >> self._binary = binary\n>> >> self._args = list(args) # Force copy args in case we modify them\n>> >> @@ -50,6 +51,7 @@ class QEMUMachine(object):\n>> >> self._socket_scm_helper = socket_scm_helper\n>> >> self._debug = debug\n>> >> self._qemu_full_args = None\n>> >> + self._created_files = []\n>> >>\n>> >> # This can be used to add an unused monitor instance.\n>> >> def add_monitor_telnet(self, ip, port):\n>> >> @@ -128,30 +130,44 @@ class QEMUMachine(object):\n>> >> '-display', 'none', '-vga', 'none']\n>> >>\n>> >> def _pre_launch(self):\n>> >> - self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address, server=True,\n>> >> - debug=self._debug)\n>> >> + try:\n>> >> + self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,\n>> >> + server=True,\n>> >> + debug=self._debug)\n>> >> + except:\n>> >> + raise\n>> >\n>> > What's the point of \"except: raise\"? It seems useless.\n>>\n>> The point is to execute the block in the else only when no exception\n>> happens. When some exception happens, I want to raise it without\n>> executing the else block.\n>\n> Isn't this exactly what Python does when an exception is raised\n> with no \"try\" block?\n\nSure, cleaning this up.\n\n>\n>\n>>\n>> >\n>> >> + else:\n>> >> + if not isinstance(self._monitor_address, tuple):\n>> >> + self._created_files.append(self._monitor_address)\n>> >> +\n>> >> + try:\n>> >> + flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY\n>> >> + os.open(self._qemu_log_path, flags)\n>> >\n>> > Why change to os.open() instead of open()?\n>>\n>> I want to create the file only if it does not exist. The open() flag\n>> 'x' is available only in python 3.3. For python <3.3, we need the\n>> os.open() to have that feature.\n>\n> I'm not sure this extra complexity is really necessary. We could\n> fix all that by using mkdtemp() and deleting the temporary\n> directory on shutdown.\n\nI thought about that, but I foresee the question: hat happens if\nbetween the mkdtemp and the file creation (i.e. self._qemu_log_path)\nsomeone goes in that directory and creates a file with the same name\nof the self._qemu_log_path? Are we going to overwrite it? Ok, very\nunlikely, but possible. This extra step takes care of that.\n\n>\n>>\n>> >\n>> >> + except:\n>> >> + raise\n>> >> + else:\n>> >> + self._created_files.append(self._qemu_log_path)\n>> >> + self._qemu_log_fd = open(self._qemu_log_path, 'wb')\n>> >>\n>> >> def _post_launch(self):\n>> >> self._qmp.accept()\n>> >>\n>> >> def _post_shutdown(self):\n>> >> - if not isinstance(self._monitor_address, tuple):\n>> >> - self._remove_if_exists(self._monitor_address)\n>> >> - self._remove_if_exists(self._qemu_log_path)\n>> >> + while self._created_files:\n>> >> + self._remove_if_exists(self._created_files.pop())\n>> >>\n>> >> def launch(self):\n>> >> '''Launch the VM and establish a QMP connection'''\n>> >> self._iolog = None\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>> >> self._qemu_full_args = (self._wrapper + [self._binary] +\n>> >> self._base_args() + self._args)\n>> >> self._popen = subprocess.Popen(self._qemu_full_args,\n>> >> stdin=devnull,\n>> >> - stdout=qemulog,\n>> >> + stdout=self._qemu_log_fd,\n>> >> stderr=subprocess.STDOUT,\n>> >> shell=False)\n>> >> self._post_launch()\n>> >> --\n>> >> 2.13.5\n>> >>\n>> >\n>> > Fam\n>\n> --\n> Eduardo","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=)","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 3xtV0b0Zgkz9s78\n\tfor ;\n\tFri, 15 Sep 2017 06:06:22 +1000 (AEST)","from localhost ([::1]:49830 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dsaOx-0006le-Pc\n\tfor incoming@patchwork.ozlabs.org; Thu, 14 Sep 2017 16:06:19 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:48634)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dsaOb-0006lT-4t\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 16:05:58 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dsaOX-0001KG-2o\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 16:05:57 -0400","from mail-io0-f175.google.com ([209.85.223.175]:52227)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from ) id 1dsaOW-0001Jn-S0\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 16:05:52 -0400","by mail-io0-f175.google.com with SMTP id i197so3390814ioe.9\n\tfor ; Thu, 14 Sep 2017 13:05:52 -0700 (PDT)","by 10.107.43.147 with HTTP; Thu, 14 Sep 2017 13:05:50 -0700 (PDT)"],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=w7fPBU/76YlV8YT29di67Xd/Us3ruxeAeHJPFVDH07M=;\n\tb=K19GPc2QYNztA6TrOq7yb2IrKfEpAUsPMuxsGoo867qtuJ5AtACVKHEsEGfLMb9909\n\tErscTUKLjwaME36zsNBueyLFIVbvCNV1kG/sCSk3gY4wApOx7nl0V9QdEtSTLGAkDvz8\n\tD2+R9ydvY79RYA/dD3esbJnq3clTaakAkuOCiH3mlCTU//ZH/YwS4+4e8M+FVR6Lg6D5\n\tTUIT6VX5vGdyyZoyylgPQZHWT/QYhdr6W8bFC2Eu1QBbwYm1zeVSg0g2GoA+pMqbXph8\n\tWUEXyx7FcEmmWGaIJfLozoL0pix+m2KVWXwiEVp/tasJ4S54DRkHWN9Xv2NhshX9Hsli\n\tNJew==","X-Gm-Message-State":"AHPjjUiCgjPr5vM85r8XNY/JINlwzPMA5Zc9eTzCKUFNfp2SWFz6Vf9u\n\thTcnk2PrlpbG6/3dVt8r1dE74jk1Ty8k15UzosHwJw==","X-Google-Smtp-Source":"AOwi7QBs5wW2VPRt5DuyH+USe5axNdWdAQIM46ejg6yfwdwJmF1e/TpQDltwad29gWSGmiA2aQYseNm//2yB2F0s4Qg=","X-Received":"by 10.107.59.17 with SMTP id i17mr4335692ioa.136.1505419551644; \n\tThu, 14 Sep 2017 13:05:51 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170914194610.GB23492@localhost.localdomain>","References":"<20170901112829.2571-1-apahim@redhat.com>\n\t<20170901112829.2571-7-apahim@redhat.com>\n\t<20170905031853.GN22515@lemon.lan>\n\t\n\t<20170914194610.GB23492@localhost.localdomain>","From":"Amador Pahim ","Date":"Thu, 14 Sep 2017 22:05:50 +0200","Message-ID":"","To":"Eduardo Habkost ","Content-Type":"text/plain; charset=\"UTF-8\"","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.85.223.175","Subject":"Re: [Qemu-devel] [PATCH v8 06/13] qemu.py: make sure we only remove\n\tfiles we create","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":"Kevin Wolf , =?utf-8?b?THVrw6HFoSBEb2t0b3I=?=\n\t, Stefan Hajnoczi ,\n\tqemu-devel@nongnu.org, Max Reitz ,\n\tCleber Rosa , Fam Zheng ,\n\tMarkus Armbruster ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1768818,"web_url":"http://patchwork.ozlabs.org/comment/1768818/","msgid":"<20170914201853.GC23492@localhost.localdomain>","list_archive_url":null,"date":"2017-09-14T20:18:53","subject":"Re: [Qemu-devel] [PATCH v8 06/13] qemu.py: make sure we only remove\n\tfiles we create","submitter":{"id":195,"url":"http://patchwork.ozlabs.org/api/people/195/","name":"Eduardo Habkost","email":"ehabkost@redhat.com"},"content":"On Thu, Sep 14, 2017 at 10:05:50PM +0200, Amador Pahim wrote:\n> On Thu, Sep 14, 2017 at 9:46 PM, Eduardo Habkost wrote:\n> > On Thu, Sep 14, 2017 at 09:38:13PM +0200, Amador Pahim wrote:\n> >> On Tue, Sep 5, 2017 at 5:18 AM, Fam Zheng wrote:\n> >> > On Fri, 09/01 13:28, Amador Pahim wrote:\n[...]\n> >> >> + else:\n> >> >> + if not isinstance(self._monitor_address, tuple):\n> >> >> + self._created_files.append(self._monitor_address)\n> >> >> +\n> >> >> + try:\n> >> >> + flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY\n> >> >> + os.open(self._qemu_log_path, flags)\n> >> >\n> >> > Why change to os.open() instead of open()?\n> >>\n> >> I want to create the file only if it does not exist. The open() flag\n> >> 'x' is available only in python 3.3. For python <3.3, we need the\n> >> os.open() to have that feature.\n> >\n> > I'm not sure this extra complexity is really necessary. We could\n> > fix all that by using mkdtemp() and deleting the temporary\n> > directory on shutdown.\n> \n> I thought about that, but I foresee the question: hat happens if\n> between the mkdtemp and the file creation (i.e. self._qemu_log_path)\n> someone goes in that directory and creates a file with the same name\n> of the self._qemu_log_path? Are we going to overwrite it? Ok, very\n> unlikely, but possible. This extra step takes care of that.\n\nIf someone creates a file inside a directory we created using\nmkdtemp(), we will just delete it. Why would that be a problem?","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=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 3xtVJC2Bctz9s7f\n\tfor ;\n\tFri, 15 Sep 2017 06:19:55 +1000 (AEST)","from localhost ([::1]:49865 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dsac5-0003r3-CJ\n\tfor incoming@patchwork.ozlabs.org; Thu, 14 Sep 2017 16:19:53 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:55338)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dsabG-0003Zi-DT\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 16:19:04 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dsabF-0005Eq-GX\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 16:19:02 -0400","from mx1.redhat.com ([209.132.183.28]:58352)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dsabF-0005EZ-A3\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 16:19:01 -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 715F6C058EAE;\n\tThu, 14 Sep 2017 20:19:00 +0000 (UTC)","from localhost (ovpn-116-51.gru2.redhat.com [10.97.116.51])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 9FB0A5D75C;\n\tThu, 14 Sep 2017 20:18:54 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 715F6C058EAE","Date":"Thu, 14 Sep 2017 17:18:53 -0300","From":"Eduardo Habkost ","To":"Amador Pahim ","Message-ID":"<20170914201853.GC23492@localhost.localdomain>","References":"<20170901112829.2571-1-apahim@redhat.com>\n\t<20170901112829.2571-7-apahim@redhat.com>\n\t<20170905031853.GN22515@lemon.lan>\n\t\n\t<20170914194610.GB23492@localhost.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","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.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.32]);\n\tThu, 14 Sep 2017 20:19:00 +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 06/13] qemu.py: make sure we only remove\n\tfiles we create","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":"Kevin Wolf , =?utf-8?b?THVrw6HFoQ==?=\n\tDoktor , Stefan Hajnoczi ,\n\tqemu-devel@nongnu.org, Max Reitz ,\n\tCleber Rosa , Fam Zheng ,\n\tMarkus Armbruster ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1768820,"web_url":"http://patchwork.ozlabs.org/comment/1768820/","msgid":"","list_archive_url":null,"date":"2017-09-14T20:21:11","subject":"Re: [Qemu-devel] [PATCH v8 06/13] qemu.py: make sure we only remove\n\tfiles we create","submitter":{"id":71999,"url":"http://patchwork.ozlabs.org/api/people/71999/","name":"Amador Pahim","email":"apahim@redhat.com"},"content":"On Thu, Sep 14, 2017 at 10:18 PM, Eduardo Habkost wrote:\n> On Thu, Sep 14, 2017 at 10:05:50PM +0200, Amador Pahim wrote:\n>> On Thu, Sep 14, 2017 at 9:46 PM, Eduardo Habkost wrote:\n>> > On Thu, Sep 14, 2017 at 09:38:13PM +0200, Amador Pahim wrote:\n>> >> On Tue, Sep 5, 2017 at 5:18 AM, Fam Zheng wrote:\n>> >> > On Fri, 09/01 13:28, Amador Pahim wrote:\n> [...]\n>> >> >> + else:\n>> >> >> + if not isinstance(self._monitor_address, tuple):\n>> >> >> + self._created_files.append(self._monitor_address)\n>> >> >> +\n>> >> >> + try:\n>> >> >> + flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY\n>> >> >> + os.open(self._qemu_log_path, flags)\n>> >> >\n>> >> > Why change to os.open() instead of open()?\n>> >>\n>> >> I want to create the file only if it does not exist. The open() flag\n>> >> 'x' is available only in python 3.3. For python <3.3, we need the\n>> >> os.open() to have that feature.\n>> >\n>> > I'm not sure this extra complexity is really necessary. We could\n>> > fix all that by using mkdtemp() and deleting the temporary\n>> > directory on shutdown.\n>>\n>> I thought about that, but I foresee the question: hat happens if\n>> between the mkdtemp and the file creation (i.e. self._qemu_log_path)\n>> someone goes in that directory and creates a file with the same name\n>> of the self._qemu_log_path? Are we going to overwrite it? Ok, very\n>> unlikely, but possible. This extra step takes care of that.\n>\n> If someone creates a file inside a directory we created using\n> mkdtemp(), we will just delete it. Why would that be a problem?\n\nOk then. That simplifies the control a lot.\nThanks.\n\n>\n> --\n> Eduardo","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=)","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 3xtVNR2rjBz9s9Y\n\tfor ;\n\tFri, 15 Sep 2017 06:23:35 +1000 (AEST)","from localhost ([::1]:49892 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dsafd-0007MO-Fs\n\tfor incoming@patchwork.ozlabs.org; Thu, 14 Sep 2017 16:23:33 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56523)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dsadQ-0005ZJ-7a\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 16:21:17 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dsadM-0007Pq-VN\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 16:21:16 -0400","from mail-it0-f54.google.com ([209.85.214.54]:54010)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from ) id 1dsadM-0007Pd-Qh\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 16:21:12 -0400","by mail-it0-f54.google.com with SMTP id 85so1511416ith.2\n\tfor ; Thu, 14 Sep 2017 13:21:12 -0700 (PDT)","by 10.107.43.147 with HTTP; Thu, 14 Sep 2017 13:21:11 -0700 (PDT)"],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=cMO0uouf+aSxZvS4w0dp1U2eylEpkugofm0wB5nvycU=;\n\tb=kgDkXbdBjkzfJsqtUURQtV9NauFqoNkFRj1AFr+uy8eRz1+Tqn6fteZvlbd/FT9HJF\n\t/PWOLSiGRb/XCdarHMrbMUTq6r/+bRQYRUHjS/nutEdw3VT+F1iO3tGAq5nPzm4rAAEe\n\tbbdaK+DH7XtkPpqx2w4jj3pTs0eR/rnhM+82gbTLSNG3uXbhl+CB6ohqlQf0dkQAO0KX\n\trd5zeXPq2v4/zMTliZiCSDyxAyRsr+3MboaZokR5g12+3hKqryYf81tiwkYNpaTHt/Px\n\tH2CPLLCCpd0CS46AHXbooh+tAt3MoX4bwsEkZuNl91Vh5UsxoWBKZlXoOty7NpJuaQ5C\n\tKITw==","X-Gm-Message-State":"AHPjjUj4fEiQIs7eu4VFjJclJMjaijt9uuJGzrKoVue0dHg9xXjUzFZ3\n\tiu1n+/XqpNYi7XllG4UpHFmEwk2griVBYvzzNKh1AQ==","X-Google-Smtp-Source":"AOwi7QAPhkUxl+ppP2OmqRHApFPl/IMp3q7hIjHCKo+zBw75IZ9WrpASP4BNyIWwA9ak8Vn282wzBzFF9zETQayA5ZI=","X-Received":"by 10.36.219.132 with SMTP id c126mr2266239itg.16.1505420471971; \n\tThu, 14 Sep 2017 13:21:11 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170914201853.GC23492@localhost.localdomain>","References":"<20170901112829.2571-1-apahim@redhat.com>\n\t<20170901112829.2571-7-apahim@redhat.com>\n\t<20170905031853.GN22515@lemon.lan>\n\t\n\t<20170914194610.GB23492@localhost.localdomain>\n\t\n\t<20170914201853.GC23492@localhost.localdomain>","From":"Amador Pahim ","Date":"Thu, 14 Sep 2017 22:21:11 +0200","Message-ID":"","To":"Eduardo Habkost ","Content-Type":"text/plain; charset=\"UTF-8\"","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.85.214.54","Subject":"Re: [Qemu-devel] [PATCH v8 06/13] qemu.py: make sure we only remove\n\tfiles we create","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":"Kevin Wolf , =?utf-8?b?THVrw6HFoSBEb2t0b3I=?=\n\t, Stefan Hajnoczi ,\n\tqemu-devel@nongnu.org, Max Reitz ,\n\tCleber Rosa , Fam Zheng ,\n\tMarkus Armbruster ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]