From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34659) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPuT0-0007uj-V0 for qemu-devel@nongnu.org; Thu, 13 Aug 2015 11:30:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZPuSw-0007ah-Ty for qemu-devel@nongnu.org; Thu, 13 Aug 2015 11:30:54 -0400 Date: Thu, 13 Aug 2015 17:30:34 +0200 From: Kevin Wolf Message-ID: <20150813153034.GF6922@noname.redhat.com> References: <1439394502-23619-1-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1439394502-23619-1-git-send-email-mreitz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 0/5] block: Drop BDS.filename List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org Am 12.08.2015 um 17:48 hat Max Reitz geschrieben: > The BDS filename field is generally only used when opening disk images > or emitting error or warning messages, the only exception to this rule > is the map command of qemu-img. However, using exact_filename there > instead should not be a problem. Therefore, we can drop the filename > field from the BlockDriverState and use a function instead which builds > the filename from scratch when called. > > This is slower than reading a static char array but the problem of that > static array is that it may become obsolete due to changes in any > BlockDriverState or in the BDS graph. Using a function which rebuilds > the filename every time it is called resolves this problem. > > The disadvantage of worse performance is negligible, on the other hand. > After patch 2 of this series, which replaces some queries of > BDS.filename by reads from somewhere else (mostly BDS.exact_filename), > the filename field is only used when a disk image is opened or some > message should be emitted, both of which cases do not suffer from the > performance hit. Apart from patch 2: Reviewed-by: Kevin Wolf