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

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

On 19/02/2026 15:40, 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>

I might have missed something, but isn't this already done in xenvbd's 
TargetUnmap, and so xendisk shouldn't need to probe this by itself?

Also, at this point I wonder which other feature could we remove from 
xendisk and fold into xenvbd instead.

> ---
>   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