From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49120) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WuIhV-0003s9-C8 for qemu-devel@nongnu.org; Tue, 10 Jun 2014 05:50:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WuIhO-0001jC-NY for qemu-devel@nongnu.org; Tue, 10 Jun 2014 05:50:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46868) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WuIhM-0001hX-46 for qemu-devel@nongnu.org; Tue, 10 Jun 2014 05:50:34 -0400 Date: Tue, 10 Jun 2014 12:50:51 +0300 From: "Michael S. Tsirkin" Message-ID: <20140610095050.GE7423@redhat.com> References: <558890f22dfa66b5818f9795b27ea712b866409b.1402299637.git.hutao@cn.fujitsu.com> <20140609105904.GA4130@redhat.com> <20140610065149.GA13030@G08FNSTD100614.fnst.cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140610065149.GA13030@G08FNSTD100614.fnst.cn.fujitsu.com> Subject: Re: [Qemu-devel] [PATCH v4 24/29] Introduce signed range. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hu Tao Cc: Peter Maydell , Eduardo Habkost , Yasunori Goto , QEMU Developers , Paolo Bonzini , Igor Mammedov On Tue, Jun 10, 2014 at 02:51:49PM +0800, Hu Tao wrote: > On Mon, Jun 09, 2014 at 01:59:04PM +0300, Michael S. Tsirkin wrote: > > On Mon, Jun 09, 2014 at 11:42:14AM +0100, Peter Maydell wrote: > > > On 9 June 2014 11:25, Hu Tao wrote: > > > > Signed-off-by: Hu Tao > > > > --- > > > > include/qemu/range.h | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 124 insertions(+) > > > > > > > > diff --git a/include/qemu/range.h b/include/qemu/range.h > > > > index aae9720..8879f8a 100644 > > > > --- a/include/qemu/range.h > > > > +++ b/include/qemu/range.h > > > > @@ -3,6 +3,7 @@ > > > > > > > > #include > > > > #include > > > > +#include "qemu/queue.h" > > > > > > > > /* > > > > * Operations on 64 bit address ranges. > > > > @@ -60,4 +61,127 @@ static inline int ranges_overlap(uint64_t first1, uint64_t len1, > > > > return !(last2 < first1 || last1 < first2); > > > > } > > > > > > > > +typedef struct SignedRangeList SignedRangeList; > > > > + > > > > +typedef struct SignedRange { > > > > + int64_t start; > > > > + int64_t length; > > > > + > > > > + QTAILQ_ENTRY(SignedRange) entry; > > > > +} SignedRange; > > > > + > > > > +QTAILQ_HEAD(SignedRangeList, SignedRange); > > > > > > This seems to be missing documentation about what the > > > semantics are and why we need it as well as the standard > > > Range. For instance, what does a SignedRange with a > > > negative length mean? > > > > > > thanks > > > -- PMM > > > > > > Yes I also don't care for list macros being mixed in with structure. > > > > Also, numa surely uses positive numbers? why do you want > > signed values? > > It's not purely for numa but for parsing int list like in string > input/output visitor. > > Hu I doubt we need negative ranges anywhere. Let's stick to the minimum functionality necessary for numa, this patchset is big as is. I would be happier if we could drop this patch completely use existing unsigned range APIs, if you need a list struct add it within the file that uses it. Having said that we can clean this up afterwards. -- MST