Re: [PATCH] Clear operations exposed by backend on disconnect

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

Hello,

On 03/09/2025 17:42, Owen Smith wrote:
> By clearing the backend operations, it will be possible to live migrate
> to a backend that exposes different operations. This will allow live migration
> to/from backends that expose different enablement for Barrier, Flush,
> and Discard operations without relying on the backend to return
> BLKIF_RSP_EOPNOTSUPP to disable an unsupported operation.
> 
> Also removes the check for change while reading disk features and always
> log the values read, as this is only called once during connection.
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxx>

The changes look OK by themselves, but I have a question:

Does this mean the availability of discard ops may now change under the 
guest's feet? Looks like this has always been the case for barrier/flush 
but I'm not sure if storage drivers are supposed to change their 
behavior during a boot session, especially if other components may have 
depended on some operations being present (e.g. for data durability 
purposes).

> ---
>   src/xenvbd/frontend.c | 54 +++++++++++++++++++++----------------------
>   1 file changed, 27 insertions(+), 27 deletions(-)
> 
> diff --git a/src/xenvbd/frontend.c b/src/xenvbd/frontend.c
> index ece7527..40611cd 100644
> --- a/src/xenvbd/frontend.c
> +++ b/src/xenvbd/frontend.c
> @@ -912,17 +912,15 @@ FrontendReadDiskInfo(
>       IN  PXENVBD_FRONTEND    Frontend
>       )
>   {
> -    BOOLEAN                 Changed;
> -    BOOLEAN                 Discard;
>       BOOLEAN                 DiscardFeature = FALSE;
>       BOOLEAN                 DiscardEnable = TRUE;
>   
> -    Changed = FrontendReadFeature(Frontend,
> -                                  FeatureBarrier,
> -                                  &Frontend->DiskInfo.Barrier);
> -    Changed |= FrontendReadFeature(Frontend,
> -                                   FeatureFlushCache,
> -                                   &Frontend->DiskInfo.FlushCache);
> +    FrontendReadFeature(Frontend,
> +                        FeatureBarrier,
> +                        &Frontend->DiskInfo.Barrier);
> +    FrontendReadFeature(Frontend,
> +                        FeatureFlushCache,
> +                        &Frontend->DiskInfo.FlushCache);
>   
>       // discard related
>       FrontendReadFeature(Frontend,
> @@ -932,26 +930,19 @@ FrontendReadDiskInfo(
>                           FeatureDiscardEnable,
>                           &DiscardEnable);
>   
> -    Discard = DiscardFeature && DiscardEnable;
> +    Frontend->DiskInfo.Discard = DiscardFeature && DiscardEnable;
>   
> -    Changed |= (Discard != Frontend->DiskInfo.Discard);
> -
> -    Frontend->DiskInfo.Discard = Discard;
> -
> -    Changed |= FrontendReadFeature(Frontend,
> -                                   FeatureDiscardSecure,
> -                                   &Frontend->DiskInfo.DiscardSecure);
> -    Changed |= FrontendReadValue32(Frontend,
> -                                   FeatureDiscardAlignment,
> -                                   TRUE,
> -                                   &Frontend->DiskInfo.DiscardAlignment);
> -    Changed |= FrontendReadValue32(Frontend,
> -                                   FeatureDiscardGranularity,
> -                                   TRUE,
> -                                   &Frontend->DiskInfo.DiscardGranularity);
> -
> -    if (!Changed)
> -        return;
> +    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,
> @@ -1378,6 +1369,15 @@ FrontendDisconnect(
>           Base64Free(Frontend->Page83.Data);
>       Frontend->Page83.Data = NULL;
>       Frontend->Page83.Size = 0;
> +
> +    // Clear some DiskInfo values, so they can be re-read on connect.
> +    // Allows migration to a backend with different supported operations.
> +    Frontend->DiskInfo.Barrier = FALSE;
> +    Frontend->DiskInfo.FlushCache = FALSE;
> +    Frontend->DiskInfo.Discard = FALSE;
> +    Frontend->DiskInfo.DiscardSecure = FALSE;
> +    Frontend->DiskInfo.DiscardAlignment = 0;
> +    Frontend->DiskInfo.DiscardGranularity = 0;
>   }
>   __drv_requiresIRQL(DISPATCH_LEVEL)
>   static FORCEINLINE VOID



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech