Patchwork [5/5] block: Always enable discard on the protocol level

login
register
mail settings
Submitter Kevin Wolf
Date June 13, 2013, 11:47 a.m.
Message ID <1371124063-12971-6-git-send-email-kwolf@redhat.com>
Download mbox | patch
Permalink /patch/251061/
State New
Headers show

Comments

Kevin Wolf - June 13, 2013, 11:47 a.m.
Turning on discard options in qcow2 doesn't help a lot when the discard
requests that it issues are thrown away by the raw-posix layer. This
patch always enables discard functionality on the protocol level so that
it's the image format's responsibility to send (or not) discard
requests. Requests sent by the guest will be allowed or ignored by the
top level BlockDriverState, which depends on the discard=... option like
before.

In particular, this means that even without specifying options, the
qcow2 default of discarding deleted snapshots actually takes effect now,
both for qemu and qemu-img.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Paolo Bonzini - June 13, 2013, 10:06 p.m.
Il 13/06/2013 07:47, Kevin Wolf ha scritto:
> Turning on discard options in qcow2 doesn't help a lot when the discard
> requests that it issues are thrown away by the raw-posix layer. This
> patch always enables discard functionality on the protocol level so that
> it's the image format's responsibility to send (or not) discard
> requests. Requests sent by the guest will be allowed or ignored by the
> top level BlockDriverState, which depends on the discard=... option like
> before.
> 
> In particular, this means that even without specifying options, the
> qcow2 default of discarding deleted snapshots actually takes effect now,
> both for qemu and qemu-img.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 79ad33d..0a7cf2f 100644
> --- a/block.c
> +++ b/block.c
> @@ -1045,7 +1045,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
>      extract_subqdict(options, &file_options, "file.");
>  
>      ret = bdrv_file_open(&file, filename, file_options,
> -                         bdrv_open_flags(bs, flags));
> +                         bdrv_open_flags(bs, flags | BDRV_O_UNMAP));
>      if (ret < 0) {
>          goto fail;
>      }
> 

Can you still disable it with -drive file.discard=ignore?

Paolo
Kevin Wolf - June 14, 2013, 8:14 a.m.
Am 14.06.2013 um 00:06 hat Paolo Bonzini geschrieben:
> Il 13/06/2013 07:47, Kevin Wolf ha scritto:
> > Turning on discard options in qcow2 doesn't help a lot when the discard
> > requests that it issues are thrown away by the raw-posix layer. This
> > patch always enables discard functionality on the protocol level so that
> > it's the image format's responsibility to send (or not) discard
> > requests. Requests sent by the guest will be allowed or ignored by the
> > top level BlockDriverState, which depends on the discard=... option like
> > before.
> > 
> > In particular, this means that even without specifying options, the
> > qcow2 default of discarding deleted snapshots actually takes effect now,
> > both for qemu and qemu-img.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  block.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/block.c b/block.c
> > index 79ad33d..0a7cf2f 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -1045,7 +1045,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
> >      extract_subqdict(options, &file_options, "file.");
> >  
> >      ret = bdrv_file_open(&file, filename, file_options,
> > -                         bdrv_open_flags(bs, flags));
> > +                         bdrv_open_flags(bs, flags | BDRV_O_UNMAP));
> >      if (ret < 0) {
> >          goto fail;
> >      }
> > 
> 
> Can you still disable it with -drive file.discard=ignore?

This requires a few more changes (basically moving BDRV_O_UNMAP from
flags into a boolean in the QDict), but I think eventually we'll want to
allow this.

Kevin

Patch

diff --git a/block.c b/block.c
index 79ad33d..0a7cf2f 100644
--- a/block.c
+++ b/block.c
@@ -1045,7 +1045,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     extract_subqdict(options, &file_options, "file.");
 
     ret = bdrv_file_open(&file, filename, file_options,
-                         bdrv_open_flags(bs, flags));
+                         bdrv_open_flags(bs, flags | BDRV_O_UNMAP));
     if (ret < 0) {
         goto fail;
     }