[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH] xen-block: only advertize discard to the fronte

From: Anthony PERARD
Subject: Re: [Qemu-block] [PATCH] xen-block: only advertize discard to the frontend when it is enabled...
Date: Thu, 21 Mar 2019 11:41:57 +0000
User-agent: Mutt/1.11.3 (2019-02-01)

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 <address@hidden>

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


Anthony PERARD

reply via email to

[Prev in Thread] Current Thread [Next in Thread]