[PATCH] xen-blkback: convert hole punching to discard request on loop devices

[PATCH] xen-blkback: convert hole punching to discard request on loop devices

[Li Dongyang]

As of dfaa2ef68e80c378e610e3c8c536f1c239e8d3ef, loop devices support
discard request now. We could just issue a discard request, and
the loop driver will punch the hole for us, so we don't need to touch
the internals of loop device and punch the hole ourselves, Thanks.

Signed-off-by: Li Dongyang <lidongyang@novell.com>
---
 drivers/block/xen-blkback/blkback.c |   21 +++------------------
 1 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index 15ec4db..ad365a7 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -39,9 +39,6 @@
 #include <linux/list.h>
 #include <linux/delay.h>
 #include <linux/freezer.h>
-#include <linux/loop.h>
-#include <linux/falloc.h>
-#include <linux/fs.h>
 
 #include <xen/events.h>
 #include <xen/page.h>
@@ -422,25 +419,13 @@ static void xen_blk_discard(struct xen_blkif *blkif, struct blkif_request *req)
 	int status = BLKIF_RSP_OKAY;
 	struct block_device *bdev = blkif->vbd.bdev;
 
-	if (blkif->blk_backend_type == BLKIF_BACKEND_PHY)
-		/* just forward the discard request */
+	if (blkif->blk_backend_type == BLKIF_BACKEND_PHY ||
+	    blkif->blk_backend_type == BLKIF_BACKEND_FILE)
 		err = blkdev_issue_discard(bdev,
 				req->u.discard.sector_number,
 				req->u.discard.nr_sectors,
 				GFP_KERNEL, 0);
-	else if (blkif->blk_backend_type == BLKIF_BACKEND_FILE) {
-		/* punch a hole in the backing file */
-		struct loop_device *lo = bdev->bd_disk->private_data;
-		struct file *file = lo->lo_backing_file;
-
-		if (file->f_op->fallocate)
-			err = file->f_op->fallocate(file,
-				FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
-				req->u.discard.sector_number << 9,
-				req->u.discard.nr_sectors << 9);
-		else
-			err = -EOPNOTSUPP;
-	} else
+	else
 		err = -EOPNOTSUPP;
 
 	if (err == -EOPNOTSUPP) {
-- 
1.7.7

Re: [PATCH] xen-blkback: convert hole punching to discard request on loop devices

[Lukas Czerner]

On Mon, 7 Nov 2011, Li Dongyang wrote:

> As of dfaa2ef68e80c378e610e3c8c536f1c239e8d3ef, loop devices support
> discard request now. We could just issue a discard request, and
> the loop driver will punch the hole for us, so we don't need to touch
> the internals of loop device and punch the hole ourselves, Thanks.

Nice, the patch looks good to me.

Thanks!
-Lukas

> 
> Signed-off-by: Li Dongyang <lidongyang@novell.com>
> ---
>  drivers/block/xen-blkback/blkback.c |   21 +++------------------
>  1 files changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
> index 15ec4db..ad365a7 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -39,9 +39,6 @@
>  #include <linux/list.h>
>  #include <linux/delay.h>
>  #include <linux/freezer.h>
> -#include <linux/loop.h>
> -#include <linux/falloc.h>
> -#include <linux/fs.h>
>  
>  #include <xen/events.h>
>  #include <xen/page.h>
> @@ -422,25 +419,13 @@ static void xen_blk_discard(struct xen_blkif *blkif, struct blkif_request *req)
>  	int status = BLKIF_RSP_OKAY;
>  	struct block_device *bdev = blkif->vbd.bdev;
>  
> -	if (blkif->blk_backend_type == BLKIF_BACKEND_PHY)
> -		/* just forward the discard request */
> +	if (blkif->blk_backend_type == BLKIF_BACKEND_PHY ||
> +	    blkif->blk_backend_type == BLKIF_BACKEND_FILE)
>  		err = blkdev_issue_discard(bdev,
>  				req->u.discard.sector_number,
>  				req->u.discard.nr_sectors,
>  				GFP_KERNEL, 0);
> -	else if (blkif->blk_backend_type == BLKIF_BACKEND_FILE) {
> -		/* punch a hole in the backing file */
> -		struct loop_device *lo = bdev->bd_disk->private_data;
> -		struct file *file = lo->lo_backing_file;
> -
> -		if (file->f_op->fallocate)
> -			err = file->f_op->fallocate(file,
> -				FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
> -				req->u.discard.sector_number << 9,
> -				req->u.discard.nr_sectors << 9);
> -		else
> -			err = -EOPNOTSUPP;
> -	} else
> +	else
>  		err = -EOPNOTSUPP;
>  
>  	if (err == -EOPNOTSUPP) {
> 

-- 

Re: [PATCH] xen-blkback: convert hole punching to discard request on loop devices

[Jan Beulich]

>>> On 07.11.11 at 09:34, Li Dongyang <lidongyang@novell.com> wrote:
> As of dfaa2ef68e80c378e610e3c8c536f1c239e8d3ef, loop devices support
> discard request now. We could just issue a discard request, and
> the loop driver will punch the hole for us, so we don't need to touch
> the internals of loop device and punch the hole ourselves, Thanks.

Looking at what the loop driver now does - doesn't the original code
here lack support for lo_offset being non-zero then? If so, should we
fix this in pre-3.2 kernels?

Jan

> Signed-off-by: Li Dongyang <lidongyang@novell.com>
> ---
>  drivers/block/xen-blkback/blkback.c |   21 +++------------------
>  1 files changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/block/xen-blkback/blkback.c 
> b/drivers/block/xen-blkback/blkback.c
> index 15ec4db..ad365a7 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -39,9 +39,6 @@
>  #include <linux/list.h>
>  #include <linux/delay.h>
>  #include <linux/freezer.h>
> -#include <linux/loop.h>
> -#include <linux/falloc.h>
> -#include <linux/fs.h>
>  
>  #include <xen/events.h>
>  #include <xen/page.h>
> @@ -422,25 +419,13 @@ static void xen_blk_discard(struct xen_blkif *blkif, 
> struct blkif_request *req)
>  	int status = BLKIF_RSP_OKAY;
>  	struct block_device *bdev = blkif->vbd.bdev;
>  
> -	if (blkif->blk_backend_type == BLKIF_BACKEND_PHY)
> -		/* just forward the discard request */
> +	if (blkif->blk_backend_type == BLKIF_BACKEND_PHY ||
> +	    blkif->blk_backend_type == BLKIF_BACKEND_FILE)
>  		err = blkdev_issue_discard(bdev,
>  				req->u.discard.sector_number,
>  				req->u.discard.nr_sectors,
>  				GFP_KERNEL, 0);
> -	else if (blkif->blk_backend_type == BLKIF_BACKEND_FILE) {
> -		/* punch a hole in the backing file */
> -		struct loop_device *lo = bdev->bd_disk->private_data;
> -		struct file *file = lo->lo_backing_file;
> -
> -		if (file->f_op->fallocate)
> -			err = file->f_op->fallocate(file,
> -				FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
> -				req->u.discard.sector_number << 9,
> -				req->u.discard.nr_sectors << 9);
> -		else
> -			err = -EOPNOTSUPP;
> -	} else
> +	else
>  		err = -EOPNOTSUPP;
>  
>  	if (err == -EOPNOTSUPP) {

Re: [PATCH] xen-blkback: convert hole punching to discard request on loop devices

[Konrad Rzeszutek Wilk]

On Mon, Nov 07, 2011 at 04:34:26PM +0800, Li Dongyang wrote:
> As of dfaa2ef68e80c378e610e3c8c536f1c239e8d3ef, loop devices support
> discard request now. We could just issue a discard request, and
> the loop driver will punch the hole for us, so we don't need to touch
> the internals of loop device and punch the hole ourselves, Thanks.

Can I ask you to do two things:
 1). Look in whether we can just eliminate the BLKIF_BACKEND_FILE altogether?
 2). Rebase this on top #devel/for-jens-3.3 (git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git)

Thanks!
> 
> Signed-off-by: Li Dongyang <lidongyang@novell.com>
> ---
>  drivers/block/xen-blkback/blkback.c |   21 +++------------------
>  1 files changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
> index 15ec4db..ad365a7 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -39,9 +39,6 @@
>  #include <linux/list.h>
>  #include <linux/delay.h>
>  #include <linux/freezer.h>
> -#include <linux/loop.h>
> -#include <linux/falloc.h>
> -#include <linux/fs.h>
>  
>  #include <xen/events.h>
>  #include <xen/page.h>
> @@ -422,25 +419,13 @@ static void xen_blk_discard(struct xen_blkif *blkif, struct blkif_request *req)
>  	int status = BLKIF_RSP_OKAY;
>  	struct block_device *bdev = blkif->vbd.bdev;
>  
> -	if (blkif->blk_backend_type == BLKIF_BACKEND_PHY)
> -		/* just forward the discard request */
> +	if (blkif->blk_backend_type == BLKIF_BACKEND_PHY ||
> +	    blkif->blk_backend_type == BLKIF_BACKEND_FILE)
>  		err = blkdev_issue_discard(bdev,
>  				req->u.discard.sector_number,
>  				req->u.discard.nr_sectors,
>  				GFP_KERNEL, 0);
> -	else if (blkif->blk_backend_type == BLKIF_BACKEND_FILE) {
> -		/* punch a hole in the backing file */
> -		struct loop_device *lo = bdev->bd_disk->private_data;
> -		struct file *file = lo->lo_backing_file;
> -
> -		if (file->f_op->fallocate)
> -			err = file->f_op->fallocate(file,
> -				FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
> -				req->u.discard.sector_number << 9,
> -				req->u.discard.nr_sectors << 9);
> -		else
> -			err = -EOPNOTSUPP;
> -	} else
> +	else
>  		err = -EOPNOTSUPP;
>  
>  	if (err == -EOPNOTSUPP) {
> -- 
> 1.7.7
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] xen-blkback: convert hole punching to discard request on loop devices

[Li Dongyang]

On Wed, Nov 9, 2011 at 12:26 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Mon, Nov 07, 2011 at 04:34:26PM +0800, Li Dongyang wrote:
>> As of dfaa2ef68e80c378e610e3c8c536f1c239e8d3ef, loop devices support
>> discard request now. We could just issue a discard request, and
>> the loop driver will punch the hole for us, so we don't need to touch
>> the internals of loop device and punch the hole ourselves, Thanks.
>
> Can I ask you to do two things:
>  1). Look in whether we can just eliminate the BLKIF_BACKEND_FILE altogether?
I think we still need that flag, as for other type of backing dev, we
will figure it out and send back a EOPNOTSUPP.
>  2). Rebase this on top #devel/for-jens-3.3 (git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git)
no problem
>
> Thanks!
>>
>> Signed-off-by: Li Dongyang <lidongyang@novell.com>
>> ---
>>  drivers/block/xen-blkback/blkback.c |   21 +++------------------
>>  1 files changed, 3 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
>> index 15ec4db..ad365a7 100644
>> --- a/drivers/block/xen-blkback/blkback.c
>> +++ b/drivers/block/xen-blkback/blkback.c
>> @@ -39,9 +39,6 @@
>>  #include <linux/list.h>
>>  #include <linux/delay.h>
>>  #include <linux/freezer.h>
>> -#include <linux/loop.h>
>> -#include <linux/falloc.h>
>> -#include <linux/fs.h>
>>
>>  #include <xen/events.h>
>>  #include <xen/page.h>
>> @@ -422,25 +419,13 @@ static void xen_blk_discard(struct xen_blkif *blkif, struct blkif_request *req)
>>       int status = BLKIF_RSP_OKAY;
>>       struct block_device *bdev = blkif->vbd.bdev;
>>
>> -     if (blkif->blk_backend_type == BLKIF_BACKEND_PHY)
>> -             /* just forward the discard request */
>> +     if (blkif->blk_backend_type == BLKIF_BACKEND_PHY ||
>> +         blkif->blk_backend_type == BLKIF_BACKEND_FILE)
>>               err = blkdev_issue_discard(bdev,
>>                               req->u.discard.sector_number,
>>                               req->u.discard.nr_sectors,
>>                               GFP_KERNEL, 0);
>> -     else if (blkif->blk_backend_type == BLKIF_BACKEND_FILE) {
>> -             /* punch a hole in the backing file */
>> -             struct loop_device *lo = bdev->bd_disk->private_data;
>> -             struct file *file = lo->lo_backing_file;
>> -
>> -             if (file->f_op->fallocate)
>> -                     err = file->f_op->fallocate(file,
>> -                             FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
>> -                             req->u.discard.sector_number << 9,
>> -                             req->u.discard.nr_sectors << 9);
>> -             else
>> -                     err = -EOPNOTSUPP;
>> -     } else
>> +     else
>>               err = -EOPNOTSUPP;
>>
>>       if (err == -EOPNOTSUPP) {
>> --
>> 1.7.7
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>

Re: [PATCH] xen-blkback: convert hole punching to discard request on loop devices

[Li Dongyang]

What am bit not sure is the secure_discard, since you've added the
sec_discard to your tree,
I'm wondering if we should treat hole punching as a secure discard to guest.
Lukas, why u didn't add QUEUE_FLAG_SECDISCARD in your commit? is there
something need to
do with loop driver to have that flag? Thanks

On Wed, Nov 9, 2011 at 5:17 PM, Li Dongyang <lidongyang@novell.com> wrote:
> On Wed, Nov 9, 2011 at 12:26 AM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
>> On Mon, Nov 07, 2011 at 04:34:26PM +0800, Li Dongyang wrote:
>>> As of dfaa2ef68e80c378e610e3c8c536f1c239e8d3ef, loop devices support
>>> discard request now. We could just issue a discard request, and
>>> the loop driver will punch the hole for us, so we don't need to touch
>>> the internals of loop device and punch the hole ourselves, Thanks.
>>
>> Can I ask you to do two things:
>>  1). Look in whether we can just eliminate the BLKIF_BACKEND_FILE altogether?
> I think we still need that flag, as for other type of backing dev, we
> will figure it out and send back a EOPNOTSUPP.
>>  2). Rebase this on top #devel/for-jens-3.3 (git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git)
> no problem
>>
>> Thanks!
>>>
>>> Signed-off-by: Li Dongyang <lidongyang@novell.com>
>>> ---
>>>  drivers/block/xen-blkback/blkback.c |   21 +++------------------
>>>  1 files changed, 3 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
>>> index 15ec4db..ad365a7 100644
>>> --- a/drivers/block/xen-blkback/blkback.c
>>> +++ b/drivers/block/xen-blkback/blkback.c
>>> @@ -39,9 +39,6 @@
>>>  #include <linux/list.h>
>>>  #include <linux/delay.h>
>>>  #include <linux/freezer.h>
>>> -#include <linux/loop.h>
>>> -#include <linux/falloc.h>
>>> -#include <linux/fs.h>
>>>
>>>  #include <xen/events.h>
>>>  #include <xen/page.h>
>>> @@ -422,25 +419,13 @@ static void xen_blk_discard(struct xen_blkif *blkif, struct blkif_request *req)
>>>       int status = BLKIF_RSP_OKAY;
>>>       struct block_device *bdev = blkif->vbd.bdev;
>>>
>>> -     if (blkif->blk_backend_type == BLKIF_BACKEND_PHY)
>>> -             /* just forward the discard request */
>>> +     if (blkif->blk_backend_type == BLKIF_BACKEND_PHY ||
>>> +         blkif->blk_backend_type == BLKIF_BACKEND_FILE)
>>>               err = blkdev_issue_discard(bdev,
>>>                               req->u.discard.sector_number,
>>>                               req->u.discard.nr_sectors,
>>>                               GFP_KERNEL, 0);
>>> -     else if (blkif->blk_backend_type == BLKIF_BACKEND_FILE) {
>>> -             /* punch a hole in the backing file */
>>> -             struct loop_device *lo = bdev->bd_disk->private_data;
>>> -             struct file *file = lo->lo_backing_file;
>>> -
>>> -             if (file->f_op->fallocate)
>>> -                     err = file->f_op->fallocate(file,
>>> -                             FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
>>> -                             req->u.discard.sector_number << 9,
>>> -                             req->u.discard.nr_sectors << 9);
>>> -             else
>>> -                     err = -EOPNOTSUPP;
>>> -     } else
>>> +     else
>>>               err = -EOPNOTSUPP;
>>>
>>>       if (err == -EOPNOTSUPP) {
>>> --
>>> 1.7.7
>>>
>>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xensource.com
>>> http://lists.xensource.com/xen-devel
>>
>

Re: [PATCH] xen-blkback: convert hole punching to discard request on loop devices

[Lukas Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4243 bytes --]

On Thu, 10 Nov 2011, Li Dongyang wrote:

> What am bit not sure is the secure_discard, since you've added the
> sec_discard to your tree,
> I'm wondering if we should treat hole punching as a secure discard to guest.
> Lukas, why u didn't add QUEUE_FLAG_SECDISCARD in your commit? is there
> something need to
> do with loop driver to have that flag? Thanks

Hi,

IIRC secure discard is almost the same as the "regular" discard, except
all sectors previously used for this data (moved by garbage collector)
are discarded as well. That is something only hardware can do, so I do
not think we should support this in loop driver. We would not be doing
anything different from "regular" discard anyway - that means just send
punch hole to the file system.

So I think that there is no reason secure discard should be supported by
loop driver, since there is nothing more "secure" about it.

Thanks!
-Lukas

> 
> On Wed, Nov 9, 2011 at 5:17 PM, Li Dongyang <lidongyang@novell.com> wrote:
> > On Wed, Nov 9, 2011 at 12:26 AM, Konrad Rzeszutek Wilk
> > <konrad.wilk@oracle.com> wrote:
> >> On Mon, Nov 07, 2011 at 04:34:26PM +0800, Li Dongyang wrote:
> >>> As of dfaa2ef68e80c378e610e3c8c536f1c239e8d3ef, loop devices support
> >>> discard request now. We could just issue a discard request, and
> >>> the loop driver will punch the hole for us, so we don't need to touch
> >>> the internals of loop device and punch the hole ourselves, Thanks.
> >>
> >> Can I ask you to do two things:
> >>  1). Look in whether we can just eliminate the BLKIF_BACKEND_FILE altogether?
> > I think we still need that flag, as for other type of backing dev, we
> > will figure it out and send back a EOPNOTSUPP.
> >>  2). Rebase this on top #devel/for-jens-3.3 (git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git)
> > no problem
> >>
> >> Thanks!
> >>>
> >>> Signed-off-by: Li Dongyang <lidongyang@novell.com>
> >>> ---
> >>>  drivers/block/xen-blkback/blkback.c |   21 +++------------------
> >>>  1 files changed, 3 insertions(+), 18 deletions(-)
> >>>
> >>> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
> >>> index 15ec4db..ad365a7 100644
> >>> --- a/drivers/block/xen-blkback/blkback.c
> >>> +++ b/drivers/block/xen-blkback/blkback.c
> >>> @@ -39,9 +39,6 @@
> >>>  #include <linux/list.h>
> >>>  #include <linux/delay.h>
> >>>  #include <linux/freezer.h>
> >>> -#include <linux/loop.h>
> >>> -#include <linux/falloc.h>
> >>> -#include <linux/fs.h>
> >>>
> >>>  #include <xen/events.h>
> >>>  #include <xen/page.h>
> >>> @@ -422,25 +419,13 @@ static void xen_blk_discard(struct xen_blkif *blkif, struct blkif_request *req)
> >>>       int status = BLKIF_RSP_OKAY;
> >>>       struct block_device *bdev = blkif->vbd.bdev;
> >>>
> >>> -     if (blkif->blk_backend_type == BLKIF_BACKEND_PHY)
> >>> -             /* just forward the discard request */
> >>> +     if (blkif->blk_backend_type == BLKIF_BACKEND_PHY ||
> >>> +         blkif->blk_backend_type == BLKIF_BACKEND_FILE)
> >>>               err = blkdev_issue_discard(bdev,
> >>>                               req->u.discard.sector_number,
> >>>                               req->u.discard.nr_sectors,
> >>>                               GFP_KERNEL, 0);
> >>> -     else if (blkif->blk_backend_type == BLKIF_BACKEND_FILE) {
> >>> -             /* punch a hole in the backing file */
> >>> -             struct loop_device *lo = bdev->bd_disk->private_data;
> >>> -             struct file *file = lo->lo_backing_file;
> >>> -
> >>> -             if (file->f_op->fallocate)
> >>> -                     err = file->f_op->fallocate(file,
> >>> -                             FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
> >>> -                             req->u.discard.sector_number << 9,
> >>> -                             req->u.discard.nr_sectors << 9);
> >>> -             else
> >>> -                     err = -EOPNOTSUPP;
> >>> -     } else
> >>> +     else
> >>>               err = -EOPNOTSUPP;
> >>>
> >>>       if (err == -EOPNOTSUPP) {
> >>> --
> >>> 1.7.7
> >>>
> >>>
> >>> _______________________________________________
> >>> Xen-devel mailing list
> >>> Xen-devel@lists.xensource.com
> >>> http://lists.xensource.com/xen-devel
> >>
> >
> 

-- 

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] xen-blkback: convert hole punching to discard request on loop devices

[Li Dongyang]

Thanks for this, so we are good then, we just set the SEC_DISCARD flag
only if the backing dev
really have that flag on the queue and, if it's a file backend, the
flag is always cleared, I'll send out
the rebased patch soon, Thanks.

On Thu, Nov 10, 2011 at 3:31 PM, Lukas Czerner <lczerner@redhat.com> wrote:
> On Thu, 10 Nov 2011, Li Dongyang wrote:
>
>> What am bit not sure is the secure_discard, since you've added the
>> sec_discard to your tree,
>> I'm wondering if we should treat hole punching as a secure discard to guest.
>> Lukas, why u didn't add QUEUE_FLAG_SECDISCARD in your commit? is there
>> something need to
>> do with loop driver to have that flag? Thanks
>
> Hi,
>
> IIRC secure discard is almost the same as the "regular" discard, except
> all sectors previously used for this data (moved by garbage collector)
> are discarded as well. That is something only hardware can do, so I do
> not think we should support this in loop driver. We would not be doing
> anything different from "regular" discard anyway - that means just send
> punch hole to the file system.
>
> So I think that there is no reason secure discard should be supported by
> loop driver, since there is nothing more "secure" about it.
>
> Thanks!
> -Lukas
>
>>
>> On Wed, Nov 9, 2011 at 5:17 PM, Li Dongyang <lidongyang@novell.com> wrote:
>> > On Wed, Nov 9, 2011 at 12:26 AM, Konrad Rzeszutek Wilk
>> > <konrad.wilk@oracle.com> wrote:
>> >> On Mon, Nov 07, 2011 at 04:34:26PM +0800, Li Dongyang wrote:
>> >>> As of dfaa2ef68e80c378e610e3c8c536f1c239e8d3ef, loop devices support
>> >>> discard request now. We could just issue a discard request, and
>> >>> the loop driver will punch the hole for us, so we don't need to touch
>> >>> the internals of loop device and punch the hole ourselves, Thanks.
>> >>
>> >> Can I ask you to do two things:
>> >>  1). Look in whether we can just eliminate the BLKIF_BACKEND_FILE altogether?
>> > I think we still need that flag, as for other type of backing dev, we
>> > will figure it out and send back a EOPNOTSUPP.
>> >>  2). Rebase this on top #devel/for-jens-3.3 (git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git)
>> > no problem
>> >>
>> >> Thanks!
>> >>>
>> >>> Signed-off-by: Li Dongyang <lidongyang@novell.com>
>> >>> ---
>> >>>  drivers/block/xen-blkback/blkback.c |   21 +++------------------
>> >>>  1 files changed, 3 insertions(+), 18 deletions(-)
>> >>>
>> >>> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
>> >>> index 15ec4db..ad365a7 100644
>> >>> --- a/drivers/block/xen-blkback/blkback.c
>> >>> +++ b/drivers/block/xen-blkback/blkback.c
>> >>> @@ -39,9 +39,6 @@
>> >>>  #include <linux/list.h>
>> >>>  #include <linux/delay.h>
>> >>>  #include <linux/freezer.h>
>> >>> -#include <linux/loop.h>
>> >>> -#include <linux/falloc.h>
>> >>> -#include <linux/fs.h>
>> >>>
>> >>>  #include <xen/events.h>
>> >>>  #include <xen/page.h>
>> >>> @@ -422,25 +419,13 @@ static void xen_blk_discard(struct xen_blkif *blkif, struct blkif_request *req)
>> >>>       int status = BLKIF_RSP_OKAY;
>> >>>       struct block_device *bdev = blkif->vbd.bdev;
>> >>>
>> >>> -     if (blkif->blk_backend_type == BLKIF_BACKEND_PHY)
>> >>> -             /* just forward the discard request */
>> >>> +     if (blkif->blk_backend_type == BLKIF_BACKEND_PHY ||
>> >>> +         blkif->blk_backend_type == BLKIF_BACKEND_FILE)
>> >>>               err = blkdev_issue_discard(bdev,
>> >>>                               req->u.discard.sector_number,
>> >>>                               req->u.discard.nr_sectors,
>> >>>                               GFP_KERNEL, 0);
>> >>> -     else if (blkif->blk_backend_type == BLKIF_BACKEND_FILE) {
>> >>> -             /* punch a hole in the backing file */
>> >>> -             struct loop_device *lo = bdev->bd_disk->private_data;
>> >>> -             struct file *file = lo->lo_backing_file;
>> >>> -
>> >>> -             if (file->f_op->fallocate)
>> >>> -                     err = file->f_op->fallocate(file,
>> >>> -                             FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
>> >>> -                             req->u.discard.sector_number << 9,
>> >>> -                             req->u.discard.nr_sectors << 9);
>> >>> -             else
>> >>> -                     err = -EOPNOTSUPP;
>> >>> -     } else
>> >>> +     else
>> >>>               err = -EOPNOTSUPP;
>> >>>
>> >>>       if (err == -EOPNOTSUPP) {
>> >>> --
>> >>> 1.7.7
>> >>>
>> >>>
>> >>> _______________________________________________
>> >>> Xen-devel mailing list
>> >>> Xen-devel@lists.xensource.com
>> >>> http://lists.xensource.com/xen-devel
>> >>
>> >
>>
>
> --

Re: [PATCH] xen-blkback: convert hole punching to discard request on loop devices

[Li Dongyang]

On Wed, Nov 9, 2011 at 12:01 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 07.11.11 at 09:34, Li Dongyang <lidongyang@novell.com> wrote:
>> As of dfaa2ef68e80c378e610e3c8c536f1c239e8d3ef, loop devices support
>> discard request now. We could just issue a discard request, and
>> the loop driver will punch the hole for us, so we don't need to touch
>> the internals of loop device and punch the hole ourselves, Thanks.
>
> Looking at what the loop driver now does - doesn't the original code
> here lack support for lo_offset being non-zero then? If so, should we
> fix this in pre-3.2 kernels?
yes, you are right, but will we ever setup the loop device with
lo_offset non-zero?
I don't think so but I'm not sure.
>
> Jan
>
>> Signed-off-by: Li Dongyang <lidongyang@novell.com>
>> ---
>>  drivers/block/xen-blkback/blkback.c |   21 +++------------------
>>  1 files changed, 3 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/block/xen-blkback/blkback.c
>> b/drivers/block/xen-blkback/blkback.c
>> index 15ec4db..ad365a7 100644
>> --- a/drivers/block/xen-blkback/blkback.c
>> +++ b/drivers/block/xen-blkback/blkback.c
>> @@ -39,9 +39,6 @@
>>  #include <linux/list.h>
>>  #include <linux/delay.h>
>>  #include <linux/freezer.h>
>> -#include <linux/loop.h>
>> -#include <linux/falloc.h>
>> -#include <linux/fs.h>
>>
>>  #include <xen/events.h>
>>  #include <xen/page.h>
>> @@ -422,25 +419,13 @@ static void xen_blk_discard(struct xen_blkif *blkif,
>> struct blkif_request *req)
>>       int status = BLKIF_RSP_OKAY;
>>       struct block_device *bdev = blkif->vbd.bdev;
>>
>> -     if (blkif->blk_backend_type == BLKIF_BACKEND_PHY)
>> -             /* just forward the discard request */
>> +     if (blkif->blk_backend_type == BLKIF_BACKEND_PHY ||
>> +         blkif->blk_backend_type == BLKIF_BACKEND_FILE)
>>               err = blkdev_issue_discard(bdev,
>>                               req->u.discard.sector_number,
>>                               req->u.discard.nr_sectors,
>>                               GFP_KERNEL, 0);
>> -     else if (blkif->blk_backend_type == BLKIF_BACKEND_FILE) {
>> -             /* punch a hole in the backing file */
>> -             struct loop_device *lo = bdev->bd_disk->private_data;
>> -             struct file *file = lo->lo_backing_file;
>> -
>> -             if (file->f_op->fallocate)
>> -                     err = file->f_op->fallocate(file,
>> -                             FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
>> -                             req->u.discard.sector_number << 9,
>> -                             req->u.discard.nr_sectors << 9);
>> -             else
>> -                     err = -EOPNOTSUPP;
>> -     } else
>> +     else
>>               err = -EOPNOTSUPP;
>>
>>       if (err == -EOPNOTSUPP) {
>
>
>
>