[Qemu-devel] [PATCH 18/22] block: implement BDRV_O_UNMAP

[Kevin Wolf]

From: Paolo Bonzini <address@hidden>

It is better to present homogeneous hardware independent of the storage
technology that is chosen on the host, hence we make discard a host
parameter; the user can choose whether to pass it down to the image
format and protocol, or to ignore it.

Using DISCARD with filesystems can cause very severe fragmentation, so it
is left default-off for now.  This can change later when we implement the
"anchor" operation for efficient management of preallocated files.

There is still one choice to make: whether DISCARD has an effect on the
dirty bitmap or not.  I chose yes, though there is a disadvantage: if
the guest is buggy and issues discards for data that is in use, there
will be no way to migrate storage for that guest without downgrading
the machine type to an older one.

Signed-off-by: Paolo Bonzini <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
---
 block.c               | 25 +++++++++++++++++++++++++
 include/block/block.h |  2 ++
 qemu-io.c             | 11 +++++++++--
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index f184b37..4582961 100644
--- a/block.c
+++ b/block.c
@@ -581,6 +581,26 @@ static int refresh_total_sectors(BlockDriverState *bs, 
int64_t hint)
 }
 
 /**
+ * Set open flags for a given discard mode
+ *
+ * Return 0 on success, -1 if the discard mode was invalid.
+ */
+int bdrv_parse_discard_flags(const char *mode, int *flags)
+{
+    *flags &= ~BDRV_O_UNMAP;
+
+    if (!strcmp(mode, "off") || !strcmp(mode, "ignore")) {
+        /* do nothing */
+    } else if (!strcmp(mode, "on") || !strcmp(mode, "unmap")) {
+        *flags |= BDRV_O_UNMAP;
+    } else {
+        return -1;
+    }
+
+    return 0;
+}
+
+/**
  * Set open flags for a given cache mode
  *
  * Return 0 on success, -1 if the cache mode was invalid.
@@ -4191,6 +4211,11 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, 
int64_t sector_num,
         bdrv_reset_dirty(bs, sector_num, nb_sectors);
     }
 
+    /* Do nothing if disabled.  */
+    if (!(bs->open_flags & BDRV_O_UNMAP)) {
+        return 0;
+    }
+
     if (bs->drv->bdrv_co_discard) {
         return bs->drv->bdrv_co_discard(bs, sector_num, nb_sectors);
     } else if (bs->drv->bdrv_aio_discard) {
diff --git a/include/block/block.h b/include/block/block.h
index b4a24da..0f750d7 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -84,6 +84,7 @@ typedef struct BlockDevOps {
 #define BDRV_O_INCOMING    0x0800  /* consistency hint for incoming migration 
*/
 #define BDRV_O_CHECK       0x1000  /* open solely for consistency check */
 #define BDRV_O_ALLOW_RDWR  0x2000  /* allow reopen to change from r/o to r/w */
+#define BDRV_O_UNMAP       0x4000  /* execute guest UNMAP/TRIM operations */
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
 
@@ -133,6 +134,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState 
*bs_old);
 void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
 void bdrv_delete(BlockDriverState *bs);
 int bdrv_parse_cache_flags(const char *mode, int *flags);
+int bdrv_parse_discard_flags(const char *mode, int *flags);
 int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
 int bdrv_open_backing_file(BlockDriverState *bs);
 int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
diff --git a/qemu-io.c b/qemu-io.c
index 6188093..7b3de42 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -1899,7 +1899,7 @@ int main(int argc, char **argv)
 {
     int readonly = 0;
     int growable = 0;
-    const char *sopt = "hVc:rsnmgkt:T:";
+    const char *sopt = "hVc:d:rsnmgkt:T:";
     const struct option lopt[] = {
         { "help", 0, NULL, 'h' },
         { "version", 0, NULL, 'V' },
@@ -1911,13 +1911,14 @@ int main(int argc, char **argv)
         { "misalign", 0, NULL, 'm' },
         { "growable", 0, NULL, 'g' },
         { "native-aio", 0, NULL, 'k' },
+        { "discard", 1, NULL, 'd' },
         { "cache", 1, NULL, 't' },
         { "trace", 1, NULL, 'T' },
         { NULL, 0, NULL, 0 }
     };
     int c;
     int opt_index = 0;
-    int flags = 0;
+    int flags = BDRV_O_UNMAP;
 
     progname = basename(argv[0]);
 
@@ -1929,6 +1930,12 @@ int main(int argc, char **argv)
         case 'n':
             flags |= BDRV_O_NOCACHE | BDRV_O_CACHE_WB;
             break;
+        case 'd':
+            if (bdrv_parse_discard_flags(optarg, &flags) < 0) {
+                error_report("Invalid discard option: %s", optarg);
+                exit(1);
+            }
+            break;
         case 'c':
             add_user_command(optarg);
             break;
-- 
1.8.1.2