Re: [Xen-devel] [PATCH] xen-block: only advertize discard to the frontend when it is enabled...

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Mar 20, 2019 at 02:28:25PM +0000, Paul Durrant wrote:
> ...and properly enable it when synthesizing a drive.
> 
> The Xen toolstack sets 'discard-enable' to '1' in xenstore when it wants
> to enable discard on a specified image. The code in
> xen_block_driver_create() correctly parses this and uses it to set

typo: It's xen_block_drive_create (s/driver/drive/), otherwise my IDE
can't find it :-(.

> 'discard' to 'unamp' for the file_layer, but fails to do the same for the

Looks like s/unamp/unmap/

> driver_layer (which effectively disables it). Meanwhile the code in
> xen_block_realize() advertizes discard support to the frontend in the
> default case (because conf->discard_granularity defaults to -1), even when

That doesn't match the code I'm reading, before the patch apply.
    if (discard_granularity > 0) feature-discard=1
Nothing seems to set discard_granularity, so it keeps it's default to
-1, so .... wait, discard_granularity is unsigned :-(

The description is fine then.

> the underlying image may not handle it.
> 
> This patch adds the missing option to the driver_layer in
> xen_block_driver_create() and checks whether BDRV_O_UNMAP is actually
> set on the block device before advertizing discard to the frontend.
> In the case that discard is supported it also makes sure that the
> granularity is set to the physical block size.
> 
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

With the two typos fixed (which I can try to remember to do on commit):
Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel