Re: [Qemu-devel] [PATCHv2 11/11] iscsi: assert that sectors are aligned to LUN blocksize

[Stefan Hajnoczi]

On Mon, Jul 01, 2013 at 05:59:02PM +0200, Peter Lieven wrote:
> 
> Am 01.07.2013 um 16:35 schrieb Stefan Hajnoczi <address@hidden>:
> 
> > On Thu, Jun 27, 2013 at 03:11:35PM +0200, Peter Lieven wrote:
> >> if the blocksize of an iSCSI LUN is bigger than the BDRV_SECTOR_SIZE
> >> it is possible that sector_num or nb_sectors are not correctly
> >> alligned.
> >> 
> >> to avoid corruption we fail requests which are misaligned.
> >> 
> >> Signed-off-by: Peter Lieven <address@hidden>
> >> ---
> >> block/iscsi.c |   27 +++++++++++++++++++++++++++
> >> 1 file changed, 27 insertions(+)
> >> 
> >> diff --git a/block/iscsi.c b/block/iscsi.c
> >> index 0567b46..bff2e1f 100644
> >> --- a/block/iscsi.c
> >> +++ b/block/iscsi.c
> >> @@ -298,6 +298,13 @@ static int64_t sector_lun2qemu(int64_t sector, 
> >> IscsiLun *iscsilun)
> >>     return sector * iscsilun->block_size / BDRV_SECTOR_SIZE;
> >> }
> >> 
> >> +static int64_t is_request_lun_aligned(int64_t sector_num, int nb_sectors,
> >> +                                      IscsiLun *iscsilun)
> >> +{
> >> +    return ((sector_num * BDRV_SECTOR_SIZE) % iscsilun->block_size ||
> >> +            (nb_sectors * BDRV_SECTOR_SIZE) % iscsilun->block_size) ? 0 : 
> >> 1;
> >> +}
> > 
> > When QEMU does geometry probing it reads 2KB.  So if the LUN has 4KB
> > block size then QEMU will fail to open it?  This would also affect image
> > formats on top of iSCSI LUNs.
> 
> opening a 4K LUN does not fail with my target. So writing unaligned sectors 
> will
> result in corruption. we should at least fail all those unaligned operations 
> until
> we have a fix or workaround in place in place.

When QEMU tries to probe the master boot record or when an image format
performs an unaligned read (say 512 bytes instead of 4 KB), won't we
fail the read request?  Therefore opening will fail.

I agree that it's better to emit an error than to corrupt the disk.  I'm
just trying to understand what needs to be done on top of this to make 4
KB LUNs work.

> > AFAICT we have no way to passing I/O topology information up from the
> > block driver.
> 
> not at the moment. Paolo had a patch series back in Dec 2011, but it never 
> went
> upstream. I asked him off list and he told me that 4K drives where not 
> important
> enough and the Redhat bug for this was closed. Now with 4K iSCSI targets these
> old work could become important again.
> 
> It would be nice to find out if there was anything wrong with them or what 
> has to
> be done to get them integrated.

Okay