|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/3] Refactor Features and DiskInfo
On 19/02/2026 15:40, Owen Smith wrote:
> From: Owen Smith <owen.smith@xxxxxxxxx>
>
> Make the XENVBD_FEATURES structure contain ring protocol related
> feature flags and values, including which ring operations the backend
> exposes to the frontend. The backend may fail any operation with
> BLKIF_OP_ENOTSUPP, which instructs the frontend to stop issuing these
> operations, and in response the frontend will disable the associated
> feature flags.
>
> Make the XENVBD_DISKINFO structure contain the backend storage
> parameters, such as disk size, physical layout and disk specific flags.
>
> Zeroing the XENVBD_FEATURES structure and re-reading it on connection
> will allow feature flags to be modified when the backend changes.
>
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxx>
Reviewed-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
Could you remove
FrontendGetDiscard/FrontendGetFlushCache/FrontendGetBarrier now that
they have been replaced by FrontendGetFeatures? I think that'll also
help us replace the manually-implemented FrontendGet* functions with
FRONTEND_GET_PROPERTY as it once was.
> ---
> src/xenvbd/frontend.c | 203 ++++++++++++++++++------------------------
> src/xenvbd/frontend.h | 12 +--
> src/xenvbd/ring.c | 2 +-
> src/xenvbd/target.c | 8 +-
> 4 files changed, 99 insertions(+), 126 deletions(-)
>
> diff --git a/src/xenvbd/frontend.c b/src/xenvbd/frontend.c
> index 9f157fe..912fc7e 100644
> --- a/src/xenvbd/frontend.c
> +++ b/src/xenvbd/frontend.c
> @@ -184,7 +184,7 @@ FrontendGetDiscard(
> IN PXENVBD_FRONTEND Frontend
> )
> {
> - return Frontend->DiskInfo.Discard;
> + return Frontend->Features.Discard;
> }
> //FRONTEND_GET_PROPERTY(FlushCache, BOOLEAN)
> BOOLEAN
> @@ -192,7 +192,7 @@ FrontendGetFlushCache(
> IN PXENVBD_FRONTEND Frontend
> )
> {
> - return Frontend->DiskInfo.FlushCache;
> + return Frontend->Features.FlushCache;
> }
> //FRONTEND_GET_PROPERTY(Barrier, BOOLEAN)
> BOOLEAN
> @@ -200,7 +200,7 @@ FrontendGetBarrier(
> IN PXENVBD_FRONTEND Frontend
> )
> {
> - return Frontend->DiskInfo.Barrier;
> + return Frontend->Features.Barrier;
> }
> FRONTEND_GET_PROPERTY(MaxQueues, ULONG)
> FRONTEND_GET_PROPERTY(NumQueues, ULONG)
> @@ -240,15 +240,15 @@ FrontendRemoveFeature(
> switch (BlkifOperation) {
> case BLKIF_OP_FLUSH_DISKCACHE:
> Verbose("FLUSH_DISKCACHE\n");
> - Frontend->DiskInfo.FlushCache = FALSE;
> + Frontend->Features.FlushCache = FALSE;
> break;
> case BLKIF_OP_WRITE_BARRIER:
> Verbose("WRITE_BARRIER\n");
> - Frontend->DiskInfo.Barrier = FALSE;
> + Frontend->Features.Barrier = FALSE;
> break;
> case BLKIF_OP_DISCARD:
> Verbose("DISCARD\n");
> - Frontend->DiskInfo.Discard = FALSE;
> + Frontend->Features.Discard = FALSE;
> break;
> case BLKIF_OP_INDIRECT:
> Verbose("INDIRECT\n");
> @@ -824,10 +824,78 @@ __Units(
> return "GB";
> }
>
> -__drv_requiresIRQL(DISPATCH_LEVEL)
> -static VOID
> -__ReadDiskInfo(
> - __in PXENVBD_FRONTEND Frontend
> +static FORCEINLINE VOID
> +FrontendReadFeatures(
> + IN PXENVBD_FRONTEND Frontend
> + )
> +{
> + BOOLEAN DiscardFeature = FALSE;
> + BOOLEAN DiscardEnable = TRUE;
> +
> + FrontendReadFeature(Frontend,
> + FeatureRemovable,
> + &Frontend->Features.Removable);
> + FrontendReadValue32(Frontend,
> + FeatureMaxIndirectSegments,
> + TRUE,
> + &Frontend->Features.Indirect);
> + FrontendReadFeature(Frontend,
> + FeaturePersistent,
> + &Frontend->Features.Persistent);
> + FrontendReadFeature(Frontend,
> + FeatureBarrier,
> + &Frontend->Features.Barrier);
> + FrontendReadFeature(Frontend,
> + FeatureFlushCache,
> + &Frontend->Features.FlushCache);
> +
> + FrontendReadFeature(Frontend,
> + FeatureDiscard,
> + &DiscardFeature);
> + FrontendReadFeature(Frontend,
> + FeatureDiscardEnable,
> + &DiscardEnable);
> + Frontend->Features.Discard = DiscardFeature && DiscardEnable;
> +
> + FrontendReadFeature(Frontend,
> + FeatureDiscardSecure,
> + &Frontend->Features.DiscardSecure);
> + FrontendReadValue32(Frontend,
> + FeatureDiscardAlignment,
> + TRUE,
> + &Frontend->Features.DiscardAlignment);
> + FrontendReadValue32(Frontend,
> + FeatureDiscardGranularity,
> + TRUE,
> + &Frontend->Features.DiscardGranularity);
> +
> + Verbose("Target[%d] : Features: %s%s%s%s%s%s\n",
> + Frontend->TargetId,
> + Frontend->Features.Persistent ? "PERSISTENT " : "",
> + Frontend->Features.Indirect > 0 ? "INDIRECT " : "",
> + Frontend->Features.Removable ? "REMOVABLE " : "",
> + Frontend->Features.Barrier ? "BARRIER " : "",
> + Frontend->Features.FlushCache ? "FLUSH " : "",
> + Frontend->Features.Discard ? "DISCARD " : "");
> +
> + if (Frontend->Features.Indirect) {
> + Verbose("Target[%d] : INDIRECT %x\n",
> + Frontend->TargetId,
> + Frontend->Features.Indirect);
> + }
> +
> + if (Frontend->Features.Discard) {
> + Verbose("Target[%d] : DISCARD %s%x/%x\n",
> + Frontend->TargetId,
> + Frontend->Features.DiscardSecure ? "SECURE " : "",
> + Frontend->Features.DiscardAlignment,
> + Frontend->Features.DiscardGranularity);
> + }
> +}
> +
> +static FORCEINLINE VOID
> +FrontendReadDiskInfo(
> + IN PXENVBD_FRONTEND Frontend
> )
> {
> BOOLEAN Changed;
> @@ -873,92 +941,6 @@ __ReadDiskInfo(
> Frontend->DiskInfo.DiskInfo);
> }
>
> -static FORCEINLINE VOID
> -FrontendReadFeatures(
> - IN PXENVBD_FRONTEND Frontend
> - )
> -{
> - BOOLEAN Changed;
> -
> - Changed = FrontendReadFeature(Frontend,
> - FeatureRemovable,
> - &Frontend->Features.Removable);
> - Changed |= FrontendReadValue32(Frontend,
> - FeatureMaxIndirectSegments,
> - TRUE,
> - &Frontend->Features.Indirect);
> - Changed |= FrontendReadFeature(Frontend,
> - FeaturePersistent,
> - &Frontend->Features.Persistent);
> -
> - if (!Changed)
> - return;
> -
> - Verbose("Target[%d] : Features: %s%s%s\n",
> - Frontend->TargetId,
> - Frontend->Features.Persistent ? "PERSISTENT " : "",
> - Frontend->Features.Indirect ? "INDIRECT " : "",
> - Frontend->Features.Removable ? "REMOVABLE" : "");
> -
> - if (Frontend->Features.Indirect) {
> - Verbose("Target[%d] : INDIRECT %x\n",
> - Frontend->TargetId,
> - Frontend->Features.Indirect);
> - }
> -}
> -
> -static FORCEINLINE VOID
> -FrontendReadDiskInfo(
> - IN PXENVBD_FRONTEND Frontend
> - )
> -{
> - BOOLEAN DiscardFeature = FALSE;
> - BOOLEAN DiscardEnable = TRUE;
> -
> - FrontendReadFeature(Frontend,
> - FeatureBarrier,
> - &Frontend->DiskInfo.Barrier);
> - FrontendReadFeature(Frontend,
> - FeatureFlushCache,
> - &Frontend->DiskInfo.FlushCache);
> -
> - // discard related
> - FrontendReadFeature(Frontend,
> - FeatureDiscard,
> - &DiscardFeature);
> - FrontendReadFeature(Frontend,
> - FeatureDiscardEnable,
> - &DiscardEnable);
> -
> - Frontend->DiskInfo.Discard = DiscardFeature && DiscardEnable;
> -
> - FrontendReadFeature(Frontend,
> - FeatureDiscardSecure,
> - &Frontend->DiskInfo.DiscardSecure);
> - FrontendReadValue32(Frontend,
> - FeatureDiscardAlignment,
> - TRUE,
> - &Frontend->DiskInfo.DiscardAlignment);
> - FrontendReadValue32(Frontend,
> - FeatureDiscardGranularity,
> - TRUE,
> - &Frontend->DiskInfo.DiscardGranularity);
> -
> - Verbose("Target[%d] : Features: %s%s%s\n",
> - Frontend->TargetId,
> - Frontend->DiskInfo.Barrier ? "BARRIER " : "",
> - Frontend->DiskInfo.FlushCache ? "FLUSH " : "",
> - Frontend->DiskInfo.Discard ? "DISCARD " : "");
> -
> - if (Frontend->DiskInfo.Discard) {
> - Verbose("Target[%d] : DISCARD %s%x/%x\n",
> - Frontend->TargetId,
> - Frontend->DiskInfo.DiscardSecure ? "SECURE " : "",
> - Frontend->DiskInfo.DiscardAlignment,
> - Frontend->DiskInfo.DiscardGranularity);
> - }
> -}
> -
> static FORCEINLINE VOID
> FrontendReadInquiryOverrides(
> IN PXENVBD_FRONTEND Frontend
> @@ -1142,8 +1124,6 @@ FrontendPrepare(
> Frontend->BackendDomain,
> Frontend->BackendPath);
>
> - FrontendReadFeatures(Frontend);
> -
> return STATUS_SUCCESS;
>
> fail7:
> @@ -1314,7 +1294,6 @@ abort:
> goto fail6;
>
> // read disk info
> - __ReadDiskInfo(Frontend);
> FrontendReadDiskInfo(Frontend);
>
> // read inquiry data
> @@ -1332,6 +1311,7 @@ abort:
>
> fail7:
> Error("Fail7\n");
> + RtlZeroMemory(&Frontend->Features, sizeof(XENVBD_FEATURES));
> fail6:
> Error("Fail6\n");
> fail5:
> @@ -1370,14 +1350,7 @@ FrontendDisconnect(
> Frontend->Page83.Data = NULL;
> Frontend->Page83.Size = 0;
>
> - // clear some disk info values, so they can be re-read on connect
> - // allows migration to a backend with different supported features
> - Frontend->DiskInfo.Barrier = FALSE;
> - Frontend->DiskInfo.FlushCache = FALSE;
> - Frontend->DiskInfo.Discard = FALSE;
> - Frontend->DiskInfo.DiscardSecure = FALSE;
> - Frontend->DiskInfo.DiscardAlignment = 0;
> - Frontend->DiskInfo.DiscardGranularity = 0;
> + RtlZeroMemory(&Frontend->Features, sizeof(XENVBD_FEATURES));
> }
> __drv_requiresIRQL(DISPATCH_LEVEL)
> static FORCEINLINE VOID
> @@ -1649,9 +1622,9 @@ FrontendDebugCallback(
> Frontend->Features.Persistent ? "PERSISTENT " : "",
> Frontend->Features.Indirect > 0 ? "INDIRECT " : "",
> Frontend->Features.Removable ? "REMOVABLE " : "",
> - Frontend->DiskInfo.Barrier ? "BARRIER " : "",
> - Frontend->DiskInfo.FlushCache ? "FLUSH " : "",
> - Frontend->DiskInfo.Discard ? "DISCARD " : "");
> + Frontend->Features.Barrier ? "BARRIER " : "",
> + Frontend->Features.FlushCache ? "FLUSH " : "",
> + Frontend->Features.Discard ? "DISCARD " : "");
>
> if (Frontend->Features.Indirect > 0) {
> XENBUS_DEBUG(Printf,
> @@ -1659,13 +1632,13 @@ FrontendDebugCallback(
> "INDIRECT %x\n",
> Frontend->Features.Indirect);
> }
> - if (Frontend->DiskInfo.Discard) {
> + if (Frontend->Features.Discard) {
> XENBUS_DEBUG(Printf,
> &Frontend->DebugInterface,
> "DISCARD %s%x/%x\n",
> - Frontend->DiskInfo.DiscardSecure ? "SECURE " : "",
> - Frontend->DiskInfo.DiscardAlignment,
> - Frontend->DiskInfo.DiscardGranularity);
> + Frontend->Features.DiscardSecure ? "SECURE " : "",
> + Frontend->Features.DiscardAlignment,
> + Frontend->Features.DiscardGranularity);
> }
>
> XENBUS_DEBUG(Printf,
> @@ -1834,7 +1807,7 @@ FrontendBackend(
> KeAcquireSpinLock(&Frontend->StateLock, &Irql);
> // Only attempt this if Active, Active is set/cleared on
> D3->D0/D0->D3
> if (Frontend->Active) {
> - __ReadDiskInfo(Frontend);
> + FrontendReadDiskInfo(Frontend);
> __CheckBackendForEject(Frontend);
> }
> KeReleaseSpinLock(&Frontend->StateLock, Irql);
> diff --git a/src/xenvbd/frontend.h b/src/xenvbd/frontend.h
> index a83c1f0..34f91b4 100644
> --- a/src/xenvbd/frontend.h
> +++ b/src/xenvbd/frontend.h
> @@ -56,6 +56,12 @@ typedef struct _XENVBD_FEATURES {
> ULONG Indirect;
> BOOLEAN Persistent;
> BOOLEAN Removable;
> + BOOLEAN Barrier;
> + BOOLEAN FlushCache;
> + BOOLEAN Discard;
> + BOOLEAN DiscardSecure;
> + ULONG DiscardAlignment;
> + ULONG DiscardGranularity;
> } XENVBD_FEATURES, *PXENVBD_FEATURES;
>
> typedef struct _XENVBD_DISKINFO {
> @@ -63,12 +69,6 @@ typedef struct _XENVBD_DISKINFO {
> ULONG SectorSize;
> ULONG PhysSectorSize;
> ULONG DiskInfo;
> - BOOLEAN Barrier;
> - BOOLEAN FlushCache;
> - BOOLEAN Discard;
> - BOOLEAN DiscardSecure;
> - ULONG DiscardAlignment;
> - ULONG DiscardGranularity;
> } XENVBD_DISKINFO, *PXENVBD_DISKINFO;
>
> typedef struct _XENVBD_FRONTEND XENVBD_FRONTEND, *PXENVBD_FRONTEND;
> diff --git a/src/xenvbd/ring.c b/src/xenvbd/ring.c
> index 86a71ac..50f8d58 100644
> --- a/src/xenvbd/ring.c
> +++ b/src/xenvbd/ring.c
> @@ -914,7 +914,7 @@ BlkifRingPrepareSyncCache(
> InitializeListHead(&List);
> Srb->SrbStatus = SRB_STATUS_PENDING;
>
> - if (FrontendGetDiskInfo(Frontend)->FlushCache)
> + if (FrontendGetFeatures(Frontend)->FlushCache)
> Operation = BLKIF_OP_FLUSH_DISKCACHE;
> else
> Operation = BLKIF_OP_WRITE_BARRIER;
> diff --git a/src/xenvbd/target.c b/src/xenvbd/target.c
> index f07ee8b..a07fc45 100644
> --- a/src/xenvbd/target.c
> +++ b/src/xenvbd/target.c
> @@ -806,7 +806,7 @@ TargetInquiryB0(
> IN PSCSI_REQUEST_BLOCK Srb
> )
> {
> - PXENVBD_DISKINFO DiskInfo = FrontendGetDiskInfo(Target->Frontend);
> + PXENVBD_FEATURES Features = FrontendGetFeatures(Target->Frontend);
> PVPD_BLOCK_LIMITS_PAGE Data = Srb->DataBuffer;
> ULONG Length = Srb->DataTransferLength;
>
> @@ -822,10 +822,10 @@ TargetInquiryB0(
> Data->PageCode = 0xB0;
> Data->PageLength[1] = 0x3C; // as per spec
>
> - *(PULONG)Data->OptimalUnmapGranularity =
> _byteswap_ulong(DiskInfo->DiscardGranularity);
> - *(PULONG)Data->UnmapGranularityAlignment =
> _byteswap_ulong(DiskInfo->DiscardAlignment);
> + *(PULONG)Data->OptimalUnmapGranularity =
> _byteswap_ulong(Features->DiscardGranularity);
> + *(PULONG)Data->UnmapGranularityAlignment =
> _byteswap_ulong(Features->DiscardAlignment);
> // alignment is only valid if a granularity has been set
> - Data->UGAValid = (DiskInfo->DiscardGranularity != 0) ? 1 : 0;
> + Data->UGAValid = (Features->DiscardGranularity != 0) ? 1 : 0;
>
> Srb->DataTransferLength = sizeof(VPD_BLOCK_LIMITS_PAGE);
> Srb->SrbStatus = SRB_STATUS_SUCCESS;
--
Ngoc Tu Dinh | Vates XCP-ng Developer
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |