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

[Li Dongyang <lidongyang@xxxxxxxxxx>]

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@xxxxxxxxxx> 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@xxxxxxxxxx> wrote:
>> > On Wed, Nov 9, 2011 at 12:26 AM, Konrad Rzeszutek Wilk
>> > <konrad.wilk@xxxxxxxxxx> 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@xxxxxxxxxx>
>> >>> ---
>> >>>  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@xxxxxxxxxxxxxxxxxxx
>> >>> http://lists.xensource.com/xen-devel
>> >>
>> >
>>
>
> --

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