From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Raspl Subject: Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches Date: Fri, 10 Mar 2017 10:42:13 +0100 Message-ID: <7e65c785-f23d-12d1-222c-087d0572f30c@linux.vnet.ibm.com> References: <20170220154211.11882-1-raspl@linux.vnet.ibm.com> <20170220154211.11882-5-raspl@linux.vnet.ibm.com> <961ec6b4-7dfe-7b86-2dc0-16e3e0d99205@redhat.com> <4929cf77-378b-7183-9b69-7f6cfeecb077@linux.vnet.ibm.com> <766744941.327830.1489133657474.JavaMail.zimbra@redhat.com> Reply-To: raspl@linux.vnet.ibm.com Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, rkrcmar@redhat.com, frankja@linux.vnet.ibm.com To: Paolo Bonzini Return-path: Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:51603 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S965380AbdCJJmU (ORCPT ); Fri, 10 Mar 2017 04:42:20 -0500 Received: from pps.filterd (m0098421.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v2A9cZs4061115 for ; Fri, 10 Mar 2017 04:42:18 -0500 Received: from e06smtp12.uk.ibm.com (e06smtp12.uk.ibm.com [195.75.94.108]) by mx0a-001b2d01.pphosted.com with ESMTP id 293a83mrw4-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Fri, 10 Mar 2017 04:42:18 -0500 Received: from localhost by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 10 Mar 2017 09:42:16 -0000 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 10.03.2017 09:38, Paolo Bonzini wrote: > > > On 10/03/2017 09:33, Stefan Raspl wrote: >> On 10.03.2017 09:14, Paolo Bonzini wrote: >>> >>> >>> ----- Original Message ----- >>>> From: "Stefan Raspl" >>>> To: "Paolo Bonzini" , kvm@vger.kernel.org >>>> Cc: rkrcmar@redhat.com, frankja@linux.vnet.ibm.com >>>> Sent: Friday, March 10, 2017 7:04:46 AM >>>> Subject: Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches >>>> >>>> On 09.03.2017 17:51, Paolo Bonzini wrote: >>>>> >>>>> >>>>> On 20/02/2017 16:41, Stefan Raspl wrote: >>>>>> @@ -339,8 +338,7 @@ def get_filters(): >>>>>> filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons) >>>>>> return filters >>>>>> >>>>>> -libc = ctypes.CDLL('libc.so.6', use_errno=True) >>>>>> -syscall = libc.syscall >>>>>> +syscall = ctypes.CDLL('libc.so.6', use_errno=True).syscall >>>>>> >>>>>> class perf_event_attr(ctypes.Structure): >>>>>> """Struct that holds the necessary data to set up a trace event. >>>>>> @@ -950,11 +948,10 @@ class Tui(object): >>>>>> while True: >>>>>> self.refresh(sleeptime) >>>>>> curses.halfdelay(int(sleeptime * 10)) >>>>>> - sleeptime = 3 >>>>>> + sleeptime = 3. >>>>>> try: >>>>>> char = self.screen.getkey() >>>>>> if char == 'x': >>>>>> - self.drilldown = not self.drilldown >>>>>> self.update_drilldown() >>>>>> if char == 'q': >>>>>> break >>>>> >>>>> I'm not sure I understand the point of these; the rest is fine. >>>> >>>> 'sleeptime' starts out as a float (sleeptime = 0.25), but is here >>>> re-defined to an int - so we make it float all the way. >>>> The variable 'drilldown' is never used, so we remove its >>>> initialization in __init__() and the sole place where it is ever >>>> used, which is the line above. >>> >>> Yes, I was referring to libc and sleeptime. I don't like the >>> "3.", especially since Python 3 has "3/2" return a float. Is >>> this a PEP8 complaint? Does it still complain if you do >>> "from __future__ import division"? >> >> >> Ah, sorry, missed the first chunk: That was to eliminate unused >> variable 'libc'. > > It's not unused, it's used on the following line. :) Sorry, don't want to get annoying: Yeah, right, so I was eliminating that variable - do you want me to remove the chunk from the patch? >> As for the "3.": I noticed through pylint, PEP8 is fine. Seemed >> valid to me, but in the grand scheme of things it certainly doesn't >> matter (and wasn't aware it becomes moot in Python 3), so I'd take >> it out if you want. > > I think it's a valid complaint with Python 2 division. If pylint > complaints even with "from __future__ import division", it would be a > pylint bug in my opinion. > > Maybe it's just me... I wouldn't have complained about "3.0" for example. :) Tried the import, pylint still complains :O So: Just switch to 3.0 here and in 07/17, and repost? Ciao, Stefan