[PATCH v2] xen-blkfront: allow discard-* nodes to be optional

[PATCH v2] xen-blkfront: allow discard-* nodes to be optional

[Roger Pau Monne]

This is inline with the specification described in blkif.h:

 * discard-granularity: should be set to the physical block size if
   node is not present.
 * discard-alignment, discard-secure: should be set to 0 if node not
   present.

This was detected as QEMU would only create the discard-granularity
node but not discard-alignment, and thus the setup done in
blkfront_setup_discard would fail.

Fix blkfront_setup_discard to not fail on missing nodes, and also fix
blkif_set_queue_limits to set the discard granularity to the physical
block size if none is specified in xenbus.

Fixes: ed30bf317c5ce ('xen-blkfront: Handle discard requests.')
Reported-by: Arthur Borsboom <arthurborsboom@gmail.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: xen-devel@lists.xenproject.org
Cc: linux-block@vger.kernel.org
Cc: Arthur Borsboom <arthurborsboom@gmail.com>
---
Changes since v2:
 - Allow all discard-* nodes to be optional.
---
 drivers/block/xen-blkfront.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 5265975b3fba..e1c6798889f4 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -945,7 +945,8 @@ static void blkif_set_queue_limits(struct blkfront_info *info)
 	if (info->feature_discard) {
 		blk_queue_flag_set(QUEUE_FLAG_DISCARD, rq);
 		blk_queue_max_discard_sectors(rq, get_capacity(gd));
-		rq->limits.discard_granularity = info->discard_granularity;
+		rq->limits.discard_granularity = info->discard_granularity ?:
+						 info->physical_sector_size;
 		rq->limits.discard_alignment = info->discard_alignment;
 		if (info->feature_secdiscard)
 			blk_queue_flag_set(QUEUE_FLAG_SECERASE, rq);
@@ -2179,19 +2180,12 @@ static void blkfront_closing(struct blkfront_info *info)
 
 static void blkfront_setup_discard(struct blkfront_info *info)
 {
-	int err;
-	unsigned int discard_granularity;
-	unsigned int discard_alignment;
-
 	info->feature_discard = 1;
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-		"discard-granularity", "%u", &discard_granularity,
-		"discard-alignment", "%u", &discard_alignment,
-		NULL);
-	if (!err) {
-		info->discard_granularity = discard_granularity;
-		info->discard_alignment = discard_alignment;
-	}
+	info->discard_granularity = xenbus_read_unsigned(info->xbdev->otherend,
+							 "discard-granularity",
+							 0);
+	info->discard_alignment = xenbus_read_unsigned(info->xbdev->otherend,
+						       "discard-alignment", 0);
 	info->feature_secdiscard =
 		!!xenbus_read_unsigned(info->xbdev->otherend, "discard-secure",
 				       0);
-- 
2.29.2

Re: [PATCH v2] xen-blkfront: allow discard-* nodes to be optional

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 919 bytes --]

On 19.01.21 11:57, Roger Pau Monne wrote:
> This is inline with the specification described in blkif.h:
> 
>   * discard-granularity: should be set to the physical block size if
>     node is not present.
>   * discard-alignment, discard-secure: should be set to 0 if node not
>     present.
> 
> This was detected as QEMU would only create the discard-granularity
> node but not discard-alignment, and thus the setup done in
> blkfront_setup_discard would fail.
> 
> Fix blkfront_setup_discard to not fail on missing nodes, and also fix
> blkif_set_queue_limits to set the discard granularity to the physical
> block size if none is specified in xenbus.
> 
> Fixes: ed30bf317c5ce ('xen-blkfront: Handle discard requests.')
> Reported-by: Arthur Borsboom <arthurborsboom@gmail.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2] xen-blkfront: allow discard-* nodes to be optional

[Roger Pau Monné]

Forgot to Cc stable for the Fixes tag. Doing it now.

On Tue, Jan 19, 2021 at 11:57:27AM +0100, Roger Pau Monne wrote:
> This is inline with the specification described in blkif.h:
> 
>  * discard-granularity: should be set to the physical block size if
>    node is not present.
>  * discard-alignment, discard-secure: should be set to 0 if node not
>    present.
> 
> This was detected as QEMU would only create the discard-granularity
> node but not discard-alignment, and thus the setup done in
> blkfront_setup_discard would fail.
> 
> Fix blkfront_setup_discard to not fail on missing nodes, and also fix
> blkif_set_queue_limits to set the discard granularity to the physical
> block size if none is specified in xenbus.
> 
> Fixes: ed30bf317c5ce ('xen-blkfront: Handle discard requests.')
> Reported-by: Arthur Borsboom <arthurborsboom@gmail.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: xen-devel@lists.xenproject.org
> Cc: linux-block@vger.kernel.org
> Cc: Arthur Borsboom <arthurborsboom@gmail.com>
> ---
> Changes since v2:
>  - Allow all discard-* nodes to be optional.
> ---
>  drivers/block/xen-blkfront.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 5265975b3fba..e1c6798889f4 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -945,7 +945,8 @@ static void blkif_set_queue_limits(struct blkfront_info *info)
>  	if (info->feature_discard) {
>  		blk_queue_flag_set(QUEUE_FLAG_DISCARD, rq);
>  		blk_queue_max_discard_sectors(rq, get_capacity(gd));
> -		rq->limits.discard_granularity = info->discard_granularity;
> +		rq->limits.discard_granularity = info->discard_granularity ?:
> +						 info->physical_sector_size;
>  		rq->limits.discard_alignment = info->discard_alignment;
>  		if (info->feature_secdiscard)
>  			blk_queue_flag_set(QUEUE_FLAG_SECERASE, rq);
> @@ -2179,19 +2180,12 @@ static void blkfront_closing(struct blkfront_info *info)
>  
>  static void blkfront_setup_discard(struct blkfront_info *info)
>  {
> -	int err;
> -	unsigned int discard_granularity;
> -	unsigned int discard_alignment;
> -
>  	info->feature_discard = 1;
> -	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
> -		"discard-granularity", "%u", &discard_granularity,
> -		"discard-alignment", "%u", &discard_alignment,
> -		NULL);
> -	if (!err) {
> -		info->discard_granularity = discard_granularity;
> -		info->discard_alignment = discard_alignment;
> -	}
> +	info->discard_granularity = xenbus_read_unsigned(info->xbdev->otherend,
> +							 "discard-granularity",
> +							 0);
> +	info->discard_alignment = xenbus_read_unsigned(info->xbdev->otherend,
> +						       "discard-alignment", 0);
>  	info->feature_secdiscard =
>  		!!xenbus_read_unsigned(info->xbdev->otherend, "discard-secure",
>  				       0);
> -- 
> 2.29.2
> 

Re: [PATCH v2] xen-blkfront: allow discard-* nodes to be optional

[Greg KH]

On Tue, Jan 19, 2021 at 01:36:22PM +0100, Roger Pau Monné wrote:
> Forgot to Cc stable for the Fixes tag. Doing it now.

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH v2] xen-blkfront: allow discard-* nodes to be optional

[Arthur Borsboom]

Hi Roger,

I have set up a test environment based on Linux 5.11.0-rc4.
The patch did not apply clean, so I copied/pasted the patch manually.

Without the patch the call trace (as reported) is visible in dmesg.
With the patch the call trace in dmesg is gone, but ... (there is
always a but) ...

Now the discard action returns the following.

[arthur@test-arch ~]$ sudo fstrim -v /
fstrim: /: the discard operation is not supported

It might be correct, but of course I was hoping the Xen VM guest would
pass on the discard request to the block device in the Xen VM host,
which is a disk partition.
Any suggestions?

(Resend due to the previous email being HTML instead of plain text).


On Tue, 19 Jan 2021 at 11:57, Roger Pau Monne <roger.pau@citrix.com> wrote:
>
> This is inline with the specification described in blkif.h:
>
>  * discard-granularity: should be set to the physical block size if
>    node is not present.
>  * discard-alignment, discard-secure: should be set to 0 if node not
>    present.
>
> This was detected as QEMU would only create the discard-granularity
> node but not discard-alignment, and thus the setup done in
> blkfront_setup_discard would fail.
>
> Fix blkfront_setup_discard to not fail on missing nodes, and also fix
> blkif_set_queue_limits to set the discard granularity to the physical
> block size if none is specified in xenbus.
>
> Fixes: ed30bf317c5ce ('xen-blkfront: Handle discard requests.')
> Reported-by: Arthur Borsboom <arthurborsboom@gmail.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: xen-devel@lists.xenproject.org
> Cc: linux-block@vger.kernel.org
> Cc: Arthur Borsboom <arthurborsboom@gmail.com>
> ---
> Changes since v2:
>  - Allow all discard-* nodes to be optional.
> ---
>  drivers/block/xen-blkfront.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 5265975b3fba..e1c6798889f4 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -945,7 +945,8 @@ static void blkif_set_queue_limits(struct blkfront_info *info)
>         if (info->feature_discard) {
>                 blk_queue_flag_set(QUEUE_FLAG_DISCARD, rq);
>                 blk_queue_max_discard_sectors(rq, get_capacity(gd));
> -               rq->limits.discard_granularity = info->discard_granularity;
> +               rq->limits.discard_granularity = info->discard_granularity ?:
> +                                                info->physical_sector_size;
>                 rq->limits.discard_alignment = info->discard_alignment;
>                 if (info->feature_secdiscard)
>                         blk_queue_flag_set(QUEUE_FLAG_SECERASE, rq);
> @@ -2179,19 +2180,12 @@ static void blkfront_closing(struct blkfront_info *info)
>
>  static void blkfront_setup_discard(struct blkfront_info *info)
>  {
> -       int err;
> -       unsigned int discard_granularity;
> -       unsigned int discard_alignment;
> -
>         info->feature_discard = 1;
> -       err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
> -               "discard-granularity", "%u", &discard_granularity,
> -               "discard-alignment", "%u", &discard_alignment,
> -               NULL);
> -       if (!err) {
> -               info->discard_granularity = discard_granularity;
> -               info->discard_alignment = discard_alignment;
> -       }
> +       info->discard_granularity = xenbus_read_unsigned(info->xbdev->otherend,
> +                                                        "discard-granularity",
> +                                                        0);
> +       info->discard_alignment = xenbus_read_unsigned(info->xbdev->otherend,
> +                                                      "discard-alignment", 0);
>         info->feature_secdiscard =
>                 !!xenbus_read_unsigned(info->xbdev->otherend, "discard-secure",
>                                        0);
> --
> 2.29.2
>


-- 
Arthur Borsboom

Re: [PATCH v2] xen-blkfront: allow discard-* nodes to be optional

[Roger Pau Monné]

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

On Wed, Jan 20, 2021 at 03:23:30PM +0100, Arthur Borsboom wrote:
> Hi Roger,
> 
> I have set up a test environment based on Linux 5.11.0-rc4.
> The patch did not apply clean, so I copied/pasted the patch manually.
> 
> Without the patch the call trace (as reported) is visible in dmesg.
> With the patch the call trace in dmesg is gone, but ... (there is always a
> but) ...
> 
> Now the discard action returns the following.
> 
> [arthur@test-arch ~]$ sudo fstrim -v /
> fstrim: /: the discard operation is not supported
> 
> It might be correct, but of course I was hoping the Xen VM guest would pass
> on the discard request to the block device in the Xen VM host, which is a
> disk partition.
> Any suggestions?

Hm, that's not what I did see on my testing, the operation worked OK,
and that's what I would expect to happen in your case also, since I
know the xenstore keys.

I think it's possible your email client has mangled the patch, I'm
attaching the same patch to this email, could you try to apply it
again and report back? (this time it should apply cleanly)

Thanks, Roger.

[-- Attachment #2: v2-0001-xen-blkfront-allow-discard-nodes-to-be-optional.patch --]
[-- Type: text/plain, Size: 3239 bytes --]

From f09acedb2e40fa84f31cfe4c960dea2037d06e3f Mon Sep 17 00:00:00 2001
From: Roger Pau Monne <roger.pau@citrix.com>
Date: Mon, 18 Jan 2021 11:47:05 +0100
Subject: [PATCH v2] xen-blkfront: allow discard-* nodes to be optional
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is inline with the specification described in blkif.h:

 * discard-granularity: should be set to the physical block size if
   node is not present.
 * discard-alignment, discard-secure: should be set to 0 if node not
   present.

This was detected as QEMU would only create the discard-granularity
node but not discard-alignment, and thus the setup done in
blkfront_setup_discard would fail.

Fix blkfront_setup_discard to not fail on missing nodes, and also fix
blkif_set_queue_limits to set the discard granularity to the physical
block size if none is specified in xenbus.

Fixes: ed30bf317c5ce ('xen-blkfront: Handle discard requests.')
Reported-by: Arthur Borsboom <arthurborsboom@gmail.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: xen-devel@lists.xenproject.org
Cc: linux-block@vger.kernel.org
Cc: Arthur Borsboom <arthurborsboom@gmail.com>
---
Changes since v2:
 - Allow all discard-* nodes to be optional.
---
 drivers/block/xen-blkfront.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 5265975b3fba..e1c6798889f4 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -945,7 +945,8 @@ static void blkif_set_queue_limits(struct blkfront_info *info)
 	if (info->feature_discard) {
 		blk_queue_flag_set(QUEUE_FLAG_DISCARD, rq);
 		blk_queue_max_discard_sectors(rq, get_capacity(gd));
-		rq->limits.discard_granularity = info->discard_granularity;
+		rq->limits.discard_granularity = info->discard_granularity ?:
+						 info->physical_sector_size;
 		rq->limits.discard_alignment = info->discard_alignment;
 		if (info->feature_secdiscard)
 			blk_queue_flag_set(QUEUE_FLAG_SECERASE, rq);
@@ -2179,19 +2180,12 @@ static void blkfront_closing(struct blkfront_info *info)
 
 static void blkfront_setup_discard(struct blkfront_info *info)
 {
-	int err;
-	unsigned int discard_granularity;
-	unsigned int discard_alignment;
-
 	info->feature_discard = 1;
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-		"discard-granularity", "%u", &discard_granularity,
-		"discard-alignment", "%u", &discard_alignment,
-		NULL);
-	if (!err) {
-		info->discard_granularity = discard_granularity;
-		info->discard_alignment = discard_alignment;
-	}
+	info->discard_granularity = xenbus_read_unsigned(info->xbdev->otherend,
+							 "discard-granularity",
+							 0);
+	info->discard_alignment = xenbus_read_unsigned(info->xbdev->otherend,
+						       "discard-alignment", 0);
 	info->feature_secdiscard =
 		!!xenbus_read_unsigned(info->xbdev->otherend, "discard-secure",
 				       0);
-- 
2.29.2

Re: [PATCH v2] xen-blkfront: allow discard-* nodes to be optional

[Jan Beulich]

On 20.01.2021 15:35, Roger Pau Monné wrote:
> On Wed, Jan 20, 2021 at 03:23:30PM +0100, Arthur Borsboom wrote:
>> Hi Roger,
>>
>> I have set up a test environment based on Linux 5.11.0-rc4.
>> The patch did not apply clean, so I copied/pasted the patch manually.
>>
>> Without the patch the call trace (as reported) is visible in dmesg.
>> With the patch the call trace in dmesg is gone, but ... (there is always a
>> but) ...
>>
>> Now the discard action returns the following.
>>
>> [arthur@test-arch ~]$ sudo fstrim -v /
>> fstrim: /: the discard operation is not supported
>>
>> It might be correct, but of course I was hoping the Xen VM guest would pass
>> on the discard request to the block device in the Xen VM host, which is a
>> disk partition.
>> Any suggestions?
> 
> Hm, that's not what I did see on my testing, the operation worked OK,
> and that's what I would expect to happen in your case also, since I
> know the xenstore keys.

Does discard work on partitions (and not just on entire disks)?
It's been a while since I last had anything to do with this code,
so I may well misremember.

Jan

Re: [PATCH v2] xen-blkfront: allow discard-* nodes to be optional

[Roger Pau Monné]

On Wed, Jan 20, 2021 at 03:37:57PM +0100, Jan Beulich wrote:
> On 20.01.2021 15:35, Roger Pau Monné wrote:
> > On Wed, Jan 20, 2021 at 03:23:30PM +0100, Arthur Borsboom wrote:
> >> Hi Roger,
> >>
> >> I have set up a test environment based on Linux 5.11.0-rc4.
> >> The patch did not apply clean, so I copied/pasted the patch manually.
> >>
> >> Without the patch the call trace (as reported) is visible in dmesg.
> >> With the patch the call trace in dmesg is gone, but ... (there is always a
> >> but) ...
> >>
> >> Now the discard action returns the following.
> >>
> >> [arthur@test-arch ~]$ sudo fstrim -v /
> >> fstrim: /: the discard operation is not supported
> >>
> >> It might be correct, but of course I was hoping the Xen VM guest would pass
> >> on the discard request to the block device in the Xen VM host, which is a
> >> disk partition.
> >> Any suggestions?
> > 
> > Hm, that's not what I did see on my testing, the operation worked OK,
> > and that's what I would expect to happen in your case also, since I
> > know the xenstore keys.
> 
> Does discard work on partitions (and not just on entire disks)?
> It's been a while since I last had anything to do with this code,
> so I may well misremember.

The command provided did work for me, ie:

# fstrim -v /
/: 19.8 GiB (21190717440 bytes) trimmed

AFAICT the blkif discard interface we provide operates at physical
block granularity, so it's possible to discard a single block, and
hence should work against partitions.

Roger.

Re: [PATCH v2] xen-blkfront: allow discard-* nodes to be optional

[Arthur Borsboom]

This time the patch applied cleanly.

The trim command seems to work as well, meaning no error messages and
a certain amount of blocks (5GB) is trimmed.
The trimming did consume a bit of time (10-20 seconds), assuming it is
actually discarding the blocks at the host.

First run:

[arthur@test-arch ~]$ sudo fstrim -v /
/: 5.7 GiB (6074368000 bytes) trimmed

Second run:

[arthur@test-arch ~]$ sudo fstrim -v /
/: 0 B (0 bytes) trimmed

No errors were reported in the dmesg of the VM; no errors in Dom0 and
no errors in dmesg of Xen (xl dmesg).

Based on this single test, it seems to work.
You can add me as Tested-By.

On Wed, 20 Jan 2021 at 15:35, Roger Pau Monné <roger.pau@citrix.com> wrote:
>
> On Wed, Jan 20, 2021 at 03:23:30PM +0100, Arthur Borsboom wrote:
> > Hi Roger,
> >
> > I have set up a test environment based on Linux 5.11.0-rc4.
> > The patch did not apply clean, so I copied/pasted the patch manually.
> >
> > Without the patch the call trace (as reported) is visible in dmesg.
> > With the patch the call trace in dmesg is gone, but ... (there is always a
> > but) ...
> >
> > Now the discard action returns the following.
> >
> > [arthur@test-arch ~]$ sudo fstrim -v /
> > fstrim: /: the discard operation is not supported
> >
> > It might be correct, but of course I was hoping the Xen VM guest would pass
> > on the discard request to the block device in the Xen VM host, which is a
> > disk partition.
> > Any suggestions?
>
> Hm, that's not what I did see on my testing, the operation worked OK,
> and that's what I would expect to happen in your case also, since I
> know the xenstore keys.
>
> I think it's possible your email client has mangled the patch, I'm
> attaching the same patch to this email, could you try to apply it
> again and report back? (this time it should apply cleanly)
>
> Thanks, Roger.



-- 
Arthur Borsboom

Re: [PATCH v2] xen-blkfront: allow discard-* nodes to be optional

[Arthur Borsboom]

Any chance of getting this backported to stable?
I believe the previous attempt by CC-ing stable was not following the
correct procedure and probably rejected.

https://lkml.org/lkml/2021/1/19/543

On Wed, 20 Jan 2021 at 16:17, Arthur Borsboom <arthurborsboom@gmail.com> wrote:
>
> This time the patch applied cleanly.
>
> The trim command seems to work as well, meaning no error messages and
> a certain amount of blocks (5GB) is trimmed.
> The trimming did consume a bit of time (10-20 seconds), assuming it is
> actually discarding the blocks at the host.
>
> First run:
>
> [arthur@test-arch ~]$ sudo fstrim -v /
> /: 5.7 GiB (6074368000 bytes) trimmed
>
> Second run:
>
> [arthur@test-arch ~]$ sudo fstrim -v /
> /: 0 B (0 bytes) trimmed
>
> No errors were reported in the dmesg of the VM; no errors in Dom0 and
> no errors in dmesg of Xen (xl dmesg).
>
> Based on this single test, it seems to work.
> You can add me as Tested-By.
>
> On Wed, 20 Jan 2021 at 15:35, Roger Pau Monné <roger.pau@citrix.com> wrote:
> >
> > On Wed, Jan 20, 2021 at 03:23:30PM +0100, Arthur Borsboom wrote:
> > > Hi Roger,
> > >
> > > I have set up a test environment based on Linux 5.11.0-rc4.
> > > The patch did not apply clean, so I copied/pasted the patch manually.
> > >
> > > Without the patch the call trace (as reported) is visible in dmesg.
> > > With the patch the call trace in dmesg is gone, but ... (there is always a
> > > but) ...
> > >
> > > Now the discard action returns the following.
> > >
> > > [arthur@test-arch ~]$ sudo fstrim -v /
> > > fstrim: /: the discard operation is not supported
> > >
> > > It might be correct, but of course I was hoping the Xen VM guest would pass
> > > on the discard request to the block device in the Xen VM host, which is a
> > > disk partition.
> > > Any suggestions?
> >
> > Hm, that's not what I did see on my testing, the operation worked OK,
> > and that's what I would expect to happen in your case also, since I
> > know the xenstore keys.
> >
> > I think it's possible your email client has mangled the patch, I'm
> > attaching the same patch to this email, could you try to apply it
> > again and report back? (this time it should apply cleanly)
> >
> > Thanks, Roger.
>
>
>
> --
> Arthur Borsboom



-- 
Arthur Borsboom

Re: [PATCH v2] xen-blkfront: allow discard-* nodes to be optional

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 914 bytes --]

On 19.01.21 11:57, Roger Pau Monne wrote:
> This is inline with the specification described in blkif.h:
> 
>   * discard-granularity: should be set to the physical block size if
>     node is not present.
>   * discard-alignment, discard-secure: should be set to 0 if node not
>     present.
> 
> This was detected as QEMU would only create the discard-granularity
> node but not discard-alignment, and thus the setup done in
> blkfront_setup_discard would fail.
> 
> Fix blkfront_setup_discard to not fail on missing nodes, and also fix
> blkif_set_queue_limits to set the discard granularity to the physical
> block size if none is specified in xenbus.
> 
> Fixes: ed30bf317c5ce ('xen-blkfront: Handle discard requests.')
> Reported-by: Arthur Borsboom <arthurborsboom@gmail.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Committed to xen/tip.git for-linus-5.11


Juergen

[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]