From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37381) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJhqu-0000wN-HH for qemu-devel@nongnu.org; Fri, 06 Feb 2015 07:17:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YJhqr-0007qA-Bh for qemu-devel@nongnu.org; Fri, 06 Feb 2015 07:17:40 -0500 Received: from mx2.parallels.com ([199.115.105.18]:42355) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJhqr-0007ps-5z for qemu-devel@nongnu.org; Fri, 06 Feb 2015 07:17:37 -0500 Message-ID: <54D4B0D6.4020402@openvz.org> Date: Fri, 6 Feb 2015 15:17:26 +0300 From: "Denis V. Lunev" MIME-Version: 1.0 References: <1423220051-16058-1-git-send-email-pl@kamp.de> <1423221883-16804-1-git-send-email-den@openvz.org> <20150206115347.GB13081@noname.redhat.com> <54D4AC9C.9000005@openvz.org> <20150206120745.GC13081@noname.redhat.com> In-Reply-To: <20150206120745.GC13081@noname.redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/1] nbd: fix max_discard/max_transfer_length List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Peter Lieven , qemu-devel@nongnu.org On 06/02/15 15:07, Kevin Wolf wrote: > Am 06.02.2015 um 12:59 hat Denis V. Lunev geschrieben: >> On 06/02/15 14:53, Kevin Wolf wrote: >>> Am 06.02.2015 um 12:24 hat Denis V. Lunev geschrieben: >>>> nbd_co_discard calls nbd_client_session_co_discard which uses uint32_t >>>> as the length in bytes of the data to discard due to the following >>>> definition: >>>> >>>> struct nbd_request { >>>> uint32_t magic; >>>> uint32_t type; >>>> uint64_t handle; >>>> uint64_t from; >>>> uint32_t len; <-- the length of data to be discarded, in bytes >>>> } QEMU_PACKED; >>>> >>>> Thus we should limit bl_max_discard to UINT32_MAX >> BDRV_SECTOR_BITS to >>>> avoid overflow. >>>> >>>> NBD read/write code uses the same structure for transfers. Fix >>>> max_transfer_length accordingly. >>>> >>>> Signed-off-by: Denis V. Lunev >>>> CC: Peter Lieven >>>> CC: Kevin Wolf >>> Thanks, I have applied both Peter's and your patch. Can you guys please >>> check whether the current state of my block branch is correct or whether >>> I forgot to include or remove some patch? >> can you give me tree URL? > Sure: > > git: git://repo.or.cz/qemu/kevin.git block > Web: http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block > >>> By the way, I don't think this NBD patch is strictly necessary as you'll >>> have a hard time finding a platform where INT_MAX > UINT32_MAX, but I >>> think it's good documentation at least and a safeguard if we ever decide >>> to lift the general block layer restrictions. >>> >>> Kevin >> nope, it is absolutely mandatory >> >> stdint.h: >> >> /* Limit of `size_t' type. */ >> # if __WORDSIZE == 64 >> # define SIZE_MAX (18446744073709551615UL) >> # else >> # define SIZE_MAX (4294967295U) >> # endif > But Peter defined it like this: > > #define BDRV_REQUEST_MAX_SECTORS MIN(SIZE_MAX >> BDRV_SECTOR_BITS, \ > INT_MAX >> BDRV_SECTOR_BITS) > > And having integers with more the 32 bits is at least unusual. I don't > know of any platform that has them. > > Anyway, as I said, your patch is good documentation, so I'm happy to > apply it nevertheless. > > Kevin I have misinterpreted this. Actually I think then the limit should be MAX() rather then MIN() as the stack is ready to size_t transfers. In the other case there is no need at all to use this construction. INT_MAX will be always less than SIZE_MAX. I do not know any platform where this is violated. Den