Re: [PATCH 1/4 v2] XenDisk: Report Discard support by issuing an Inquiry

["Tu Dinh" <ngoc-tu.dinh@xxxxxxxxxx>]

On 03/03/2026 10:36, Owen Smith wrote:
> From: Owen Smith <owen.smith@xxxxxxxxx>
> 
> When XenDisk starts, issue an inquiry to determine the status of discard
> support in the backend. If the backend exposes discard support, allow
> XenDisk to respond to the appropriate property requests, and forward the
> trim requests to XenVbd.
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

Shouldn't XenDisk report unmaps as being unconditionally supported so 
that unmap requests could be passed to XenVbd for processing (as pointed 
out in "Report VPD 0xB2 Logical Block Provisioning" in the XenDisk 
removal series)?

Otherwise, when booting on a backend that doesn't support discards, 
XenDisk will initially report unmaps as unsupported. This will cause 
Windows to not attempt unmaps even if the VM is later migrated to a 
backend that does support discards.

> ---
>   src/xendisk/pdo.c | 88 ++++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 84 insertions(+), 4 deletions(-)
> 
> diff --git a/src/xendisk/pdo.c b/src/xendisk/pdo.c
> index f714efb..5d40a13 100644
> --- a/src/xendisk/pdo.c
> +++ b/src/xendisk/pdo.c
> @@ -63,6 +63,7 @@ struct _XENDISK_PDO {
>       PXENDISK_FDO                Fdo;
>   
>       BOOLEAN                     InterceptTrim;
> +    BOOLEAN                     DiscardSupported;
>       ULONG                       SectorSize;
>       ULONG                       PhysSectorSize;
>   };
> @@ -519,6 +520,72 @@ fail1:
>       return status;
>   }
>   
> +static NTSTATUS
> +PdoSendInquiryB0Synchronous(
> +    IN  PXENDISK_PDO        Pdo,
> +    OUT PBOOLEAN            Supported
> +    )
> +{
> +    SCSI_REQUEST_BLOCK      Srb;
> +    PCDB                    Cdb;
> +    PVPD_BLOCK_LIMITS_PAGE  BlockLimits;
> +    ULONG                   Length;
> +    NTSTATUS                status;
> +
> +    Trace("====>\n");
> +
> +    Length = sizeof(VPD_BLOCK_LIMITS_PAGE);
> +    *Supported = FALSE;
> +
> +    status = STATUS_NO_MEMORY;
> +    BlockLimits = __PdoAllocate(Length);
> +    if (BlockLimits == NULL)
> +        goto fail1;
> +
> +    RtlZeroMemory(&Srb, sizeof(SCSI_REQUEST_BLOCK));
> +    Srb.Length = sizeof(SCSI_REQUEST_BLOCK);
> +    Srb.SrbFlags = 0;
> +    Srb.Function = SRB_FUNCTION_EXECUTE_SCSI;
> +    Srb.DataBuffer = BlockLimits;
> +    Srb.DataTransferLength = Length;
> +    Srb.TimeOutValue = (ULONG)-1;
> +    Srb.CdbLength = 6;
> +
> +    Cdb = (PCDB)&Srb.Cdb[0];
> +    Cdb->CDB6INQUIRY3.OperationCode = SCSIOP_INQUIRY;
> +    Cdb->CDB6INQUIRY3.PageCode = 0xB0;
> +    Cdb->CDB6INQUIRY3.EnableVitalProductData = 1;
> +    Cdb->CDB6INQUIRY3.AllocationLength = (UCHAR)Length;
> +
> +    status = PdoSendAwaitSrb(Pdo, &Srb);
> +    if (!NT_SUCCESS(status))
> +        goto fail2;
> +
> +    status = STATUS_UNSUCCESSFUL;
> +    if (Srb.DataTransferLength < Length)
> +        goto fail3;
> +
> +    *Supported = BlockLimits->UGAValid;
> +
> +    __PdoFree(BlockLimits);
> +
> +    Trace("<====\n");
> +    return STATUS_SUCCESS;
> +
> +fail3:
> +    Error("fail3\n");
> +
> +fail2:
> +    Error("fail2\n");
> +
> +    __PdoFree(BlockLimits);
> +
> +fail1:
> +    Error("fail1 (%08x)\n", status);
> +
> +    return status;
> +}
> +
>   static NTSTATUS
>   PdoSendTrimSynchronous(
>       IN  PXENDISK_PDO            Pdo,
> @@ -674,7 +741,7 @@ PdoQueryProperty(
>   
>       switch (Query->PropertyId) {
>       case StorageDeviceTrimProperty:
> -        if (!Pdo->InterceptTrim) {
> +        if (!Pdo->InterceptTrim || !Pdo->DiscardSupported) {
>               status = PdoForwardIrpAndForget(Pdo, Irp);
>               break;
>           }
> @@ -753,7 +820,7 @@ PdoManageDataSetAttributes(
>   
>       switch (Attributes->Action) {
>       case DeviceDsmAction_Trim:
> -        if (!Pdo->InterceptTrim) {
> +        if (!Pdo->InterceptTrim || !Pdo->DiscardSupported) {
>               status = PdoForwardIrpAndForget(Pdo, Irp);
>               break;
>           }
> @@ -830,6 +897,7 @@ PdoStartDevice(
>       ULONG               PhysSectorSize;
>       ULONG64             SectorCount;
>       ULONG64             Size;
> +    BOOLEAN             DiscardSupported;
>       POWER_STATE         PowerState;
>       NTSTATUS            status;
>   
> @@ -848,14 +916,22 @@ PdoStartDevice(
>       if (!NT_SUCCESS(status))
>           goto fail3;
>   
> +    status = PdoSendInquiryB0Synchronous(Pdo,
> +                                         &DiscardSupported);
> +    if (!NT_SUCCESS(status))
> +        goto fail4;
> +
>       Pdo->SectorSize = SectorSize;
>       Pdo->PhysSectorSize = PhysSectorSize;
> +    Pdo->DiscardSupported = DiscardSupported;
>   
>       Size = SectorSize * SectorCount;
>       Size >>= 20; // Scale to megabytes
>   
> -    Verbose("%s: %luMB (%uB sectors)\n",
> -            __PdoGetName(Pdo), Size, SectorSize);
> +    Verbose("%s: %luMB (%uB sectors)%s%s\n",
> +            __PdoGetName(Pdo), Size, SectorSize,
> +            Pdo->DiscardSupported ? " DISCARD" : "",
> +            Pdo->InterceptTrim ? "" : " (VETOED)");
>   
>       __PdoSetSystemPowerState(Pdo, PowerSystemWorking);
>       __PdoSetDevicePowerState(Pdo, PowerDeviceD0);
> @@ -874,6 +950,9 @@ PdoStartDevice(
>   
>       return STATUS_SUCCESS;
>   
> +fail4:
> +    Error("fail4\n");
> +
>   fail3:
>       Error("fail3\n");
>   
> @@ -1904,6 +1983,7 @@ PdoDestroy(
>       Dx->Pdo = NULL;
>   
>       Pdo->InterceptTrim = FALSE;
> +    Pdo->DiscardSupported = FALSE;
>   
>       RtlZeroMemory(Pdo->Name, sizeof (Pdo->Name));
>   



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech