From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59329) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwStz-0003B6-HF for qemu-devel@nongnu.org; Tue, 18 Oct 2016 07:49:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bwSty-0001xL-Mn for qemu-devel@nongnu.org; Tue, 18 Oct 2016 07:49:51 -0400 Date: Tue, 18 Oct 2016 13:49:42 +0200 From: Kevin Wolf Message-ID: <20161018114942.GE4706@noname.str.redhat.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2] Add 'offset' and 'size' options List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?utf-8?B?VG9tw6HFoSBHb2xlbWJpb3Zza8O9?= Cc: qemu-devel@nongnu.org, Max Reitz , Markus Armbruster , Eric Blake , qemu-block@nongnu.org Am 18.10.2016 um 00:25 hat Tom=C3=A1=C5=A1 Golembiovsk=C3=BD geschrieben: > This is a follow-up to the patch: > [PATCH] raw-posix: add 'offset' and 'size' options >=20 > The main changes are: > - options were moved from 'file' driver into 'raw' driver as suggeste= d > - added support for writing, reopen and truncate when possible >=20 > If I forgot to address somebody's comments feel free to raise them agai= n, > please. >=20 > Some general notes to the code: >=20 > 1) The size is rounded *down* to the 512 byte boundary. It's not that > the raw driver really cares about this, but if we don't do it then=20 > bdrv_getlength() will do that instead of us. The problem is that > bdrv_getlength() does round *up* and this can lead to reads/writes > outside the specified 'size'. I think it might be better to just check whether offset/size are correctly aligned and error out if they aren't. Then once we made the necessary changes to allow byte alignment (I think what prevents it is mostly bs->total_sectors, right?) we can allow that in the raw format driver, which will only make previously failing options work rather than changing the behaviour of already working configurations (we can't do the latter without a very good justification because of compatibility). > 2) We don't provide '.bdrv_get_allocated_file_size' function. As a > result the information about allocated disk size reports size of th= e > whole file. This is, rather confusingly, larger than the provided > 'size'. But I don't think this matters much. Note that we don't hav= e > any easy way how to get the correct information here apart from > checking all the block with bdrv_co_get_block_status() (as suggeste= d > by Kevin Wolf). >=20 > 3) No options for raw_create(). The 'size' and 'offset' options were > added only to open/reopen. In my opinion there is no real reason fo= r > them there. AFAIK you cannot create embeded QCOW2/VMDK/etc. image > that way anyway. These two things are fine with me. Kevin