Re: [win-pv-devel] [PATCH] Fixed improper SCSI UNMAP request implementation

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Dave Buches [mailto:davebuch@xxxxxxxxxx]
> Sent: 22 January 2016 04:30
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Paul Durrant; David Buches
> Subject: [PATCH] Fixed improper SCSI UNMAP request implementation
> 
> From: David Buches <davebuch@xxxxxxxxxx>
> 
> The XenDisk disk class filter driver was generating requests
> that did not adhere to the t10 SBC-3 SCSI specification for UNMAP
> operations.  Specifically, the data length and block descriptor data
> length fields were not populated properly per the spec.
> 
> Although the XenVBD miniport driver handled these malformed
> requests correctly, it failed to handle *properly* formed requests,
> which would result in unpredictable behavior.
> 
> Additionally, the XenDisk filter wasn't properly responding to
> StorageDeviceTrimProperty queries per the MSDN spec. Specifically,
> the DEVICE_TRIM_DESCRIPTOR::Version structure member needs to be set
> to sizeof(DEVICE_TRIM_DESCRIPTOR).
> 
> Signed-off-by: David Buches <davebuch@xxxxxxxxxx>

Thanks.

Acked-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

...and applied.

> ---
>  src/xendisk/pdo.c | 6 +++---
>  src/xenvbd/pdo.c  | 3 +--
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/src/xendisk/pdo.c b/src/xendisk/pdo.c
> index 07f4cfd..eaa3e2d 100644
> --- a/src/xendisk/pdo.c
> +++ b/src/xendisk/pdo.c
> @@ -401,7 +401,7 @@ PdoQueryProperty(
> 
>              Trim = Irp->AssociatedIrp.SystemBuffer;
> 
> -            Trim->Version = 0;
> +            Trim->Version = sizeof(DEVICE_TRIM_DESCRIPTOR);
>              Trim->Size = sizeof(DEVICE_TRIM_DESCRIPTOR);
>              Trim->TrimEnabled = TRUE;
> 
> @@ -557,8 +557,8 @@ PdoSendTrimSynchronous(
>      Cdb->UNMAP.OperationCode = SCSIOP_UNMAP;
>      *(PUSHORT)Cdb->UNMAP.AllocationLength =
> _byteswap_ushort((USHORT)Length);
> 
> -    *(PUSHORT)Unmap->DataLength = _byteswap_ushort((USHORT)Length);
> -    *(PUSHORT)Unmap->BlockDescrDataLength =
> _byteswap_ushort((USHORT)sizeof(UNMAP_BLOCK_DESCRIPTOR));
> +     *(PUSHORT)Unmap->DataLength =
> _byteswap_ushort((USHORT)(Length -
> FIELD_OFFSET(UNMAP_LIST_HEADER, BlockDescrDataLength)));
> +     *(PUSHORT)Unmap->BlockDescrDataLength =
> _byteswap_ushort((USHORT)(Length -
> FIELD_OFFSET(UNMAP_LIST_HEADER, Descriptors[0])));
> 
>      for (Index = 0; Index < Count; ++Index) {
>          PUNMAP_BLOCK_DESCRIPTOR Block = &Unmap->Descriptors[Index];
> diff --git a/src/xenvbd/pdo.c b/src/xenvbd/pdo.c
> index 488a74b..67800e4 100644
> --- a/src/xenvbd/pdo.c
> +++ b/src/xenvbd/pdo.c
> @@ -1264,8 +1264,7 @@ PrepareUnmap(
>  {
>      PXENVBD_SRBEXT      SrbExt = GetSrbExt(Srb);
>      PUNMAP_LIST_HEADER  Unmap = Srb->DataBuffer;
> -    ULONG               Count = _byteswap_ushort(*(PUSHORT)Unmap-
> >DataLength) /
> -                                _byteswap_ushort(*(PUSHORT)Unmap-
> >BlockDescrDataLength);
> +     ULONG               Count = _byteswap_ushort(*(PUSHORT)Unmap-
> >BlockDescrDataLength) / sizeof(UNMAP_BLOCK_DESCRIPTOR);
>      ULONG               Index;
>      LIST_ENTRY          List;
> 
> --
> 2.5.3.windows.1


_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel