From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59622) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YsYB3-0007lS-V4 for qemu-devel@nongnu.org; Wed, 13 May 2015 11:02:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YsYAx-0006HB-Uo for qemu-devel@nongnu.org; Wed, 13 May 2015 11:02:29 -0400 Date: Wed, 13 May 2015 23:02:19 +0800 From: Fam Zheng Message-ID: <20150513150219.GF30644@ad.nay.redhat.com> References: <1431538099-3286-1-git-send-email-famz@redhat.com> <1431538099-3286-2-git-send-email-famz@redhat.com> <55533DB3.20402@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55533DB3.20402@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 01/11] block: Add op blocker type "device IO" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Kevin Wolf , qemu-block@nongnu.org, armbru@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, Stefan Hajnoczi On Wed, 05/13 14:04, Paolo Bonzini wrote: > > > On 13/05/2015 19:28, Fam Zheng wrote: > > @@ -478,6 +478,10 @@ static int blk_check_request(BlockBackend *blk, int64_t sector_num, > > return -EIO; > > } > > > > + if (bdrv_op_is_blocked(blk->bs, BLOCK_OP_TYPE_DEVICE_IO, NULL)) { > > + return -EBUSY; > > + } > > I think this is incorrect. It's fine for backends to generate more I/O > after a blocker is submitted, as long as it's bounded. > > For example, SCSI requests can result in many consecutive I/Os: > > (1) FUA requests are split in write+flush > > (2) adapters that do not use QEMUSGList-based I/O only read 128K at a time > > (3) WRITE SAME operations are also split in chunks > > (4) UNMAP operations process one descriptor at a time I don't understand the point of these examples. If we don't return -EBUSY here, the request will sneak into block/io.c and perhaps break qmp transaction semantics, if it lands between two backups. Fam > > Paolo > > > return blk_check_byte_request(blk, sector_num * BDRV_SECTOR_SIZE, > > nb_sectors * BDRV_SECTOR_SIZE); > > }