[Qemu-devel] [RFC V1 00/12] QCOW2 asynchronous deduplication

[Qemu-devel] [RFC V1 00/12] QCOW2 asynchronous deduplication

[Benoît Canet]

This patchset implements asynchronous deduplication in QCOW2 on top of the
"QCOW2 deduplication metrics" patchset.

It's a work in progress posted so reviewers can see where the feature is
heading.

Benoît Canet (12):
  block: Add BlockDriver function prototype to pause and resume
    deduplication.
  qcow2: Add code to deduplicate cluster flagged with
    QCOW_OFLAG_TO_DEDUP.
  block: Add bdrv_has_dedup.
  block: Add bdrv_is_dedup_running.
  block: Add bdrv_resume_dedup.
  block: Add bdrv_pause_dedup.
  qcow2: Add qcow2_pause_dedup.
  qcow2: Add qcow2_resume_dedup.
  qcow2: Make dedup status persists.
  qerror: Add QERR_DEVICE_NOT_DEDUPLICATED.
  qmp: Add block-pause-dedup.
  qmp: Add block_resume_dedup.

 block.c                   |   72 ++++++++++++++++++++
 block/qcow2-dedup.c       |  166 +++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c             |    8 ++-
 block/qcow2.h             |    3 +
 blockdev.c                |   36 ++++++++++
 include/block/block.h     |    4 ++
 include/block/block_int.h |    4 ++
 include/qapi/qmp/qerror.h |    3 +
 qapi-schema.json          |   36 ++++++++++
 qmp-commands.hx           |   46 +++++++++++++
 10 files changed, 377 insertions(+), 1 deletion(-)

-- 
1.7.10.4

[Qemu-devel] [RFC V1 01/12] block: Add BlockDriver function prototype to pause and resume deduplication.

[Benoît Canet]

---
 include/block/block_int.h |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index b7ed3e6..bb35df9 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -203,6 +203,10 @@ struct BlockDriver {
      */
     int (*bdrv_has_zero_init)(BlockDriverState *bs);
 
+    /* to pause and resume deduplication (mainly qcow2) */
+    void (*bdrv_resume_dedup)(BlockDriverState *bs);
+    void (*bdrv_pause_dedup)(BlockDriverState *bs);
+
     QLIST_ENTRY(BlockDriver) list;
 };
 
-- 
1.7.10.4

[Qemu-devel] [RFC V1 02/12] qcow2: Add code to deduplicate cluster flagged with QCOW_OFLAG_TO_DEDUP.

[Benoît Canet]

---
 block/qcow2-dedup.c |  126 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/block/qcow2-dedup.c b/block/qcow2-dedup.c
index 4305746..dd320bf 100644
--- a/block/qcow2-dedup.c
+++ b/block/qcow2-dedup.c
@@ -1166,6 +1166,130 @@ static int qcow2_drop_to_dedup_hashes(BlockDriverState *bs)
     return 0;
 }
 
+static bool qcow2_try_dedup_on_disk_cluster(BlockDriverState *bs,
+                                            QcowPersistantHash *phash,
+                                            uint64_t index,
+                                            uint64_t physical_sect)
+{
+    BDRVQcowState *s = bs->opaque;
+    uint8_t *data;
+    int ret = 0;
+    bool result = false;
+
+    data = qemu_blockalign(bs, s->cluster_size);
+
+    /* read the cluster data from disk */
+    ret = bdrv_pread(bs->file, physical_sect << 9, data, s->cluster_size);
+
+    if (ret < 0) {
+        goto exit;
+    }
+
+    /* force computation of the hash */
+    phash->reuse = false;
+
+    ret = qcow2_try_dedup_cluster(bs,
+                                  phash,
+                                  index * s->cluster_sectors,
+                                  data,
+                                  0);
+
+    if (ret < 0) {
+        goto exit;
+    }
+
+    /* cluster was deduplicated -> result is true */
+    if (ret) {
+        result = true;
+    }
+
+exit:
+   qemu_vfree(data);
+   return result;
+}
+
+static bool qcow2_process_undeduplicated_cluster(BlockDriverState *bs,
+                                                 QcowPersistantHash *phash,
+                                                 uint64_t index,
+                                                 uint64_t physical_sect)
+{
+    BDRVQcowState *s = bs->opaque;
+    int ret = 0;
+
+    /* not duplicated */
+    ret = qcow2_store_hash(bs, &phash->hash,
+                           index * s->cluster_sectors,
+                           physical_sect);
+
+    if (ret < 0) {
+        error_printf("Error while storing hash");
+        return false;
+    }
+
+    /* remove the QCOW_OFLAG_TO_DEDUP flag from l2 entry
+     * note: we should take care of setting QCOW_OFLAG_COPIED if needed
+     */
+    ret = qcow2_dedup_link_l2(bs, index * s->cluster_sectors,
+                              physical_sect, true);
+
+    return ret == 0 ? true : false;
+}
+
+static bool qcow2_process_to_dedup_cluster(BlockDriverState *bs,
+                                           uint64_t index)
+{
+    QcowPersistantHash phash;
+    bool to_dedup, deduplicated;
+    uint64_t physical_sect;
+    int ret = 0;
+
+    to_dedup = qcow2_is_cluster_to_dedup(bs, index, &physical_sect, &ret);
+
+    if (ret < 0) {
+        error_printf("Error checking if cluster must be deduplicated");
+        return false;
+    }
+
+    if (!to_dedup) {
+        return false;
+    }
+
+    deduplicated = qcow2_try_dedup_on_disk_cluster(bs,
+                                                   &phash,
+                                                   index,
+                                                   physical_sect);
+
+    if (deduplicated) {
+        return true;
+    }
+
+    qcow2_process_undeduplicated_cluster(bs,
+                                         &phash,
+                                         index,
+                                         physical_sect);
+
+    return true;
+}
+
+/* This function try to deduplicate clusters written when dedup was not running.
+ */
+static void qcow2_deduplicate_after_resuming(BlockDriverState *bs)
+{
+    BDRVQcowState *s = bs->opaque;
+    uint64_t i;
+    bool processed;
+
+    /* for each l2 entry */
+    for (i = 0; i < s->l2_size * s->l1_size; i++) {
+        qemu_co_mutex_lock(&s->lock);
+        processed = qcow2_process_to_dedup_cluster(bs, i);
+        qemu_co_mutex_unlock(&s->lock);
+        if (processed || !(i % s->l2_size)) {
+            co_sleep_ns(rt_clock, s->dedup_co_delay);
+        }
+    }
+}
+
 /*
  * This coroutine resume deduplication
  *
@@ -1194,6 +1318,8 @@ static void coroutine_fn qcow2_co_dedup_resume(void *opaque)
     s->dedup_status = QCOW_DEDUP_STARTED;
     qemu_co_mutex_unlock(&s->lock);
 
+    qcow2_deduplicate_after_resuming(bs);
+
     return;
 
 fail:
-- 
1.7.10.4

[Qemu-devel] [RFC V1 03/12] block: Add bdrv_has_dedup.

[Benoît Canet]

---
 block.c               |   15 +++++++++++++++
 include/block/block.h |    1 +
 2 files changed, 16 insertions(+)

diff --git a/block.c b/block.c
index a245653..aee33e0 100644
--- a/block.c
+++ b/block.c
@@ -4327,6 +4327,21 @@ void bdrv_lock_medium(BlockDriverState *bs, bool locked)
     }
 }
 
+/* Return true if the device has deduplication */
+bool bdrv_has_dedup(BlockDriverState *bs)
+{
+    BlockDriverInfo bdi;
+    int ret = 0;
+
+    ret = bdrv_get_info((BlockDriverState *) bs, &bdi);
+
+    if (ret < 0) {
+        return false;
+    }
+
+    return bdi.has_dedup;
+}
+
 /* needed for generic scsi interface */
 
 int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
diff --git a/include/block/block.h b/include/block/block.h
index 2043560..e6f86ac 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -306,6 +306,7 @@ void bdrv_set_enable_write_cache(BlockDriverState *bs, bool wce);
 int bdrv_is_inserted(BlockDriverState *bs);
 int bdrv_media_changed(BlockDriverState *bs);
 void bdrv_lock_medium(BlockDriverState *bs, bool locked);
+bool bdrv_has_dedup(BlockDriverState *bs);
 void bdrv_eject(BlockDriverState *bs, bool eject_flag);
 const char *bdrv_get_format_name(BlockDriverState *bs);
 BlockDriverState *bdrv_find(const char *name);
-- 
1.7.10.4

[Qemu-devel] [RFC V1 04/12] block: Add bdrv_is_dedup_running.

[Benoît Canet]

---
 block.c               |   19 +++++++++++++++++++
 include/block/block.h |    1 +
 2 files changed, 20 insertions(+)

diff --git a/block.c b/block.c
index aee33e0..e83666a 100644
--- a/block.c
+++ b/block.c
@@ -4342,6 +4342,25 @@ bool bdrv_has_dedup(BlockDriverState *bs)
     return bdi.has_dedup;
 }
 
+/* Return true if the device has deduplication and it's running */
+bool bdrv_is_dedup_running(BlockDriverState *bs)
+{
+    BlockDriverInfo bdi;
+    int ret = 0;
+
+    ret = bdrv_get_info((BlockDriverState *) bs, &bdi);
+
+    if (ret < 0) {
+        return false;
+    }
+
+    if (!bdi.has_dedup) {
+        return false;
+    }
+
+    return bdi.dedup_running;
+}
+
 /* needed for generic scsi interface */
 
 int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
diff --git a/include/block/block.h b/include/block/block.h
index e6f86ac..f4e1d2a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -307,6 +307,7 @@ int bdrv_is_inserted(BlockDriverState *bs);
 int bdrv_media_changed(BlockDriverState *bs);
 void bdrv_lock_medium(BlockDriverState *bs, bool locked);
 bool bdrv_has_dedup(BlockDriverState *bs);
+bool bdrv_is_dedup_running(BlockDriverState *bs);
 void bdrv_eject(BlockDriverState *bs, bool eject_flag);
 const char *bdrv_get_format_name(BlockDriverState *bs);
 BlockDriverState *bdrv_find(const char *name);
-- 
1.7.10.4

[Qemu-devel] [RFC V1 05/12] block: Add bdrv_resume_dedup.

[Benoît Canet]

---
 block.c               |   19 +++++++++++++++++++
 include/block/block.h |    1 +
 2 files changed, 20 insertions(+)

diff --git a/block.c b/block.c
index e83666a..4e80da8 100644
--- a/block.c
+++ b/block.c
@@ -4361,6 +4361,25 @@ bool bdrv_is_dedup_running(BlockDriverState *bs)
     return bdi.dedup_running;
 }
 
+int bdrv_resume_dedup(BlockDriverState *bs)
+{
+    BlockDriver *drv = bs->drv;
+
+    if (!bdrv_has_dedup(bs)) {
+        return -EINVAL;
+    }
+
+    if (bdrv_is_dedup_running(bs)) {
+        return 0;
+    }
+
+    if (drv && drv->bdrv_resume_dedup) {
+        drv->bdrv_resume_dedup(bs);
+    }
+
+    return 0;
+}
+
 /* needed for generic scsi interface */
 
 int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
diff --git a/include/block/block.h b/include/block/block.h
index f4e1d2a..94ac50a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -308,6 +308,7 @@ int bdrv_media_changed(BlockDriverState *bs);
 void bdrv_lock_medium(BlockDriverState *bs, bool locked);
 bool bdrv_has_dedup(BlockDriverState *bs);
 bool bdrv_is_dedup_running(BlockDriverState *bs);
+int bdrv_resume_dedup(BlockDriverState *bs);
 void bdrv_eject(BlockDriverState *bs, bool eject_flag);
 const char *bdrv_get_format_name(BlockDriverState *bs);
 BlockDriverState *bdrv_find(const char *name);
-- 
1.7.10.4

[Qemu-devel] [RFC V1 06/12] block: Add bdrv_pause_dedup.

[Benoît Canet]

---
 block.c               |   19 +++++++++++++++++++
 include/block/block.h |    1 +
 2 files changed, 20 insertions(+)

diff --git a/block.c b/block.c
index 4e80da8..8c527b6 100644
--- a/block.c
+++ b/block.c
@@ -4380,6 +4380,25 @@ int bdrv_resume_dedup(BlockDriverState *bs)
     return 0;
 }
 
+int bdrv_pause_dedup(BlockDriverState *bs)
+{
+    BlockDriver *drv = bs->drv;
+
+    if (!bdrv_has_dedup(bs)) {
+        return -EINVAL;
+    }
+
+    if (!bdrv_is_dedup_running(bs)) {
+        return 0;
+    }
+
+    if (drv && drv->bdrv_pause_dedup) {
+        drv->bdrv_pause_dedup(bs);
+    }
+
+    return 0;
+}
+
 /* needed for generic scsi interface */
 
 int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
diff --git a/include/block/block.h b/include/block/block.h
index 94ac50a..1328a27 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -309,6 +309,7 @@ void bdrv_lock_medium(BlockDriverState *bs, bool locked);
 bool bdrv_has_dedup(BlockDriverState *bs);
 bool bdrv_is_dedup_running(BlockDriverState *bs);
 int bdrv_resume_dedup(BlockDriverState *bs);
+int bdrv_pause_dedup(BlockDriverState *bs);
 void bdrv_eject(BlockDriverState *bs, bool eject_flag);
 const char *bdrv_get_format_name(BlockDriverState *bs);
 BlockDriverState *bdrv_find(const char *name);
-- 
1.7.10.4

[Qemu-devel] [RFC V1 07/12] qcow2: Add qcow2_pause_dedup.

[Benoît Canet]

---
 block/qcow2-dedup.c |   19 +++++++++++++++++++
 block/qcow2.c       |    2 ++
 block/qcow2.h       |    1 +
 3 files changed, 22 insertions(+)

diff --git a/block/qcow2-dedup.c b/block/qcow2-dedup.c
index dd320bf..e007387 100644
--- a/block/qcow2-dedup.c
+++ b/block/qcow2-dedup.c
@@ -1278,15 +1278,20 @@ static void qcow2_deduplicate_after_resuming(BlockDriverState *bs)
     BDRVQcowState *s = bs->opaque;
     uint64_t i;
     bool processed;
+    bool exit;
 
     /* for each l2 entry */
     for (i = 0; i < s->l2_size * s->l1_size; i++) {
         qemu_co_mutex_lock(&s->lock);
         processed = qcow2_process_to_dedup_cluster(bs, i);
+        exit = s->dedup_status == QCOW_DEDUP_STOPPING;
         qemu_co_mutex_unlock(&s->lock);
         if (processed || !(i % s->l2_size)) {
             co_sleep_ns(rt_clock, s->dedup_co_delay);
         }
+        if (exit) {
+            return;
+        }
     }
 }
 
@@ -1450,3 +1455,17 @@ void qcow2_dedup_update_metrics(BlockDriverState *bs)
     s->dedup_metrics.ram_usage = nb_hashs * GTREE_NODE_SIZE * 2;
     s->dedup_metrics.ram_usage += nb_hashs * sizeof(QCowHashNode);
 }
+
+void qcow2_pause_dedup(BlockDriverState *bs)
+{
+    BDRVQcowState *s = bs->opaque;
+
+    if (s->dedup_status != QCOW_DEDUP_STARTED) {
+        return;
+    }
+
+    s->dedup_status = QCOW_DEDUP_STOPPING;
+    /* must handle half processed write requests */
+    qcow2_dedup_reset(bs);
+    s->dedup_status = QCOW_DEDUP_STOPPED;
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index e442268..c17ab63 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2011,6 +2011,8 @@ static BlockDriver bdrv_qcow2 = {
 
     .bdrv_invalidate_cache      = qcow2_invalidate_cache,
 
+    .bdrv_pause_dedup           = qcow2_pause_dedup,
+
     .create_options = qcow2_create_options,
     .bdrv_check = qcow2_check,
 };
diff --git a/block/qcow2.h b/block/qcow2.h
index d8e8539..5940c89 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -511,5 +511,6 @@ bool qcow2_dedup_is_running(BlockDriverState *bs);
 int qcow2_dedup_init(BlockDriverState *bs);
 void qcow2_dedup_close(BlockDriverState *bs);
 void qcow2_dedup_update_metrics(BlockDriverState *bs);
+void qcow2_pause_dedup(BlockDriverState *bs);
 
 #endif
-- 
1.7.10.4

[Qemu-devel] [RFC V1 08/12] qcow2: Add qcow2_resume_dedup.

[Benoît Canet]

---
 block/qcow2-dedup.c |   14 ++++++++++++++
 block/qcow2.c       |    1 +
 block/qcow2.h       |    1 +
 3 files changed, 16 insertions(+)

diff --git a/block/qcow2-dedup.c b/block/qcow2-dedup.c
index e007387..93545af 100644
--- a/block/qcow2-dedup.c
+++ b/block/qcow2-dedup.c
@@ -1469,3 +1469,17 @@ void qcow2_pause_dedup(BlockDriverState *bs)
     qcow2_dedup_reset(bs);
     s->dedup_status = QCOW_DEDUP_STOPPED;
 }
+
+void qcow2_resume_dedup(BlockDriverState *bs)
+{
+    BDRVQcowState *s = bs->opaque;
+
+    if (s->dedup_status != QCOW_DEDUP_STOPPED) {
+        return;
+    }
+
+    s->dedup_status = QCOW_DEDUP_STARTING;
+
+    s->dedup_resume_co = qemu_coroutine_create(qcow2_co_dedup_resume);
+    qemu_coroutine_enter(s->dedup_resume_co, bs);
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index c17ab63..d5681ad 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2012,6 +2012,7 @@ static BlockDriver bdrv_qcow2 = {
     .bdrv_invalidate_cache      = qcow2_invalidate_cache,
 
     .bdrv_pause_dedup           = qcow2_pause_dedup,
+    .bdrv_resume_dedup           = qcow2_resume_dedup,
 
     .create_options = qcow2_create_options,
     .bdrv_check = qcow2_check,
diff --git a/block/qcow2.h b/block/qcow2.h
index 5940c89..2b5a7d4 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -512,5 +512,6 @@ int qcow2_dedup_init(BlockDriverState *bs);
 void qcow2_dedup_close(BlockDriverState *bs);
 void qcow2_dedup_update_metrics(BlockDriverState *bs);
 void qcow2_pause_dedup(BlockDriverState *bs);
+void qcow2_resume_dedup(BlockDriverState *bs);
 
 #endif
-- 
1.7.10.4

[Qemu-devel] [RFC V1 09/12] qcow2: Make dedup status persists.

[Benoît Canet]

---
 block/qcow2-dedup.c |    7 +++++++
 block/qcow2.c       |    5 ++++-
 block/qcow2.h       |    1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-dedup.c b/block/qcow2-dedup.c
index 93545af..85ef66f 100644
--- a/block/qcow2-dedup.c
+++ b/block/qcow2-dedup.c
@@ -1323,6 +1323,8 @@ static void coroutine_fn qcow2_co_dedup_resume(void *opaque)
     s->dedup_status = QCOW_DEDUP_STARTED;
     qemu_co_mutex_unlock(&s->lock);
 
+    qcow2_update_header(bs);
+
     qcow2_deduplicate_after_resuming(bs);
 
     return;
@@ -1429,6 +1431,10 @@ int qcow2_dedup_init(BlockDriverState *bs)
         return 0;
     }
 
+    if (!s->start_dedup) {
+        return 0;
+    }
+
     s->dedup_status = QCOW_DEDUP_STARTING;
 
     /* resume deduplication */
@@ -1465,6 +1471,7 @@ void qcow2_pause_dedup(BlockDriverState *bs)
     }
 
     s->dedup_status = QCOW_DEDUP_STOPPING;
+    qcow2_update_header(bs);
     /* must handle half processed write requests */
     qcow2_dedup_reset(bs);
     s->dedup_status = QCOW_DEDUP_STOPPED;
diff --git a/block/qcow2.c b/block/qcow2.c
index d5681ad..1e61050 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -168,6 +168,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
                 s->dedup_table_size =
                     be32_to_cpu(dedup_table_extension.size);
                 s->dedup_hash_algo = dedup_table_extension.hash_algo;
+                s->start_dedup = dedup_table_extension.strategies & (1 << 2);
             break;
 
         default:
@@ -1221,7 +1222,9 @@ int qcow2_update_header(BlockDriverState *bs)
         dedup_table_extension.size = cpu_to_be32(s->dedup_table_size);
         dedup_table_extension.hash_algo = s->dedup_hash_algo;
         dedup_table_extension.strategies |= 1; /* RAM based lookup */
-        dedup_table_extension.strategies |= 1 << 2; /* deduplication running */
+        if (s->has_dedup && s->dedup_status == QCOW_DEDUP_STARTED) {
+            dedup_table_extension.strategies |= 1 << 2;
+        }
         ret = header_ext_add(buf,
                              QCOW2_EXT_MAGIC_DEDUP_TABLE,
                              &dedup_table_extension,
diff --git a/block/qcow2.h b/block/qcow2.h
index 2b5a7d4..3fdfe14 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -228,6 +228,7 @@ typedef struct BDRVQcowState {
     int64_t free_byte_offset;
 
     bool has_dedup;
+    bool start_dedup;
     QCowDedupStatus dedup_status;
     QCowHashAlgo dedup_hash_algo;
     Coroutine *dedup_resume_co;
-- 
1.7.10.4

[Qemu-devel] [RFC V1 10/12] qerror: Add QERR_DEVICE_NOT_DEDUPLICATED.

[Benoît Canet]

---
 include/qapi/qmp/qerror.h |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..3f99c8c 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -108,6 +108,9 @@ void assert_no_error(Error *err);
 #define QERR_DEVICE_NOT_ACTIVE \
     ERROR_CLASS_DEVICE_NOT_ACTIVE, "Device '%s' has not been activated"
 
+#define QERR_DEVICE_NOT_DEDUPLICATED \
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' doesn't support deduplication"
+
 #define QERR_DEVICE_NOT_ENCRYPTED \
     ERROR_CLASS_GENERIC_ERROR, "Device '%s' is not encrypted"
 
-- 
1.7.10.4

Re: [Qemu-devel] [RFC V1 10/12] qerror: Add QERR_DEVICE_NOT_DEDUPLICATED.

[Eric Blake]

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

On 01/16/2013 09:25 AM, Benoît Canet wrote:
> ---
>  include/qapi/qmp/qerror.h |    3 +++
>  1 file changed, 3 insertions(+)

You don't want this patch; instead, code in later patches that currently
uses
 error_set(errp, QERR_DEVICE_NOT_DEDUPLICATED,
    bdrv_get_device_name(bs));
should instead call
 error_setg(errp, "Device '%s' doesn't support deduplication",
    bdrv_get_device_name(bs));

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

[Qemu-devel] [RFC V1 11/12] qmp: Add block-pause-dedup.

[Benoît Canet]

---
 blockdev.c       |   18 ++++++++++++++++++
 qapi-schema.json |   18 ++++++++++++++++++
 qmp-commands.hx  |   23 +++++++++++++++++++++++
 3 files changed, 59 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index d724e2d..4c5f954 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -896,6 +896,24 @@ void qmp_block_passwd(const char *device, const char *password, Error **errp)
     }
 }
 
+void qmp_block_pause_dedup(const char *device, Error **errp)
+{
+    BlockDriverState *bs;
+    int err;
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    err = bdrv_pause_dedup(bs);
+    if (err == -EINVAL) {
+        error_set(errp, QERR_DEVICE_NOT_DEDUPLICATED, bdrv_get_device_name(bs));
+        return;
+    }
+}
+
 static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
                                     int bdrv_flags, BlockDriver *drv,
                                     const char *password, Error **errp)
diff --git a/qapi-schema.json b/qapi-schema.json
index 1a5014c..d8c8348 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -720,6 +720,24 @@
 { 'command': 'query-block', 'returns': ['BlockInfo'] }
 
 ##
+# @block-pause-dedup:
+#
+# This command pause the deduplication on a device that support it.
+#
+# @device:   the name of the device to pause the deduplication on
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#          If @device is not deduplicated, DeviceNotDeduplicated
+#
+# Notes:  Not all block formats support deduplication one must use
+#         query-blockstats before and look at the optional deduplication field.
+#
+# Since: 1.5
+##
+{ 'command': 'block-pause-dedup', 'data': {'device': 'str' } }
+
+##
 # @BlockDeviceDedupInfo
 #
 # Statistics of the deduplication on a virtual block device implementing it
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 5c692d0..acc9fd0 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1236,6 +1236,29 @@ Example:
 EQMP
 
     {
+        .name       = "block-pause-dedup",
+        .args_type  = "device:B",
+        .mhandler.cmd_new = qmp_marshal_input_block_pause_dedup,
+    },
+
+SQMP
+block-pause-dedup
+------------
+
+Pause the deduplication on a device that support it.
+
+Arguments:
+
+- "device": device name (json-string)
+
+Example:
+
+-> { "execute": "block-pause-dedup", "arguments": { "device": "ide0-hd0" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "block_set_io_throttle",
         .args_type  = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
         .mhandler.cmd_new = qmp_marshal_input_block_set_io_throttle,
-- 
1.7.10.4

Re: [Qemu-devel] [RFC V1 11/12] qmp: Add block-pause-dedup.

[Eric Blake]

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

On 01/16/2013 09:25 AM, Benoît Canet wrote:
> ---
>  blockdev.c       |   18 ++++++++++++++++++
>  qapi-schema.json |   18 ++++++++++++++++++
>  qmp-commands.hx  |   23 +++++++++++++++++++++++
>  3 files changed, 59 insertions(+)
> 
>  
>  ##
> +# @block-pause-dedup:
> +#
> +# This command pause the deduplication on a device that support it.

s/support/supports/

> +#
> +# @device:   the name of the device to pause the deduplication on
> +#
> +# Returns: nothing on success
> +#          If @device is not a valid block device, DeviceNotFound
> +#          If @device is not deduplicated, DeviceNotDeduplicated

I don't think you need this second error.  A generic error is good
enough unless we can prove that having a dedicated error class makes
algorithmic sense for a given client, and I can't come up with such a
scenario off the top of my head for libvirt.

> +SQMP
> +block-pause-dedup
> +------------
> +
> +Pause the deduplication on a device that support it.

s/support/supports/

I notice that between this and patch 12, you are adding two very similar
commands (block-pause-dedup, block-resume-dedup); would it be any
simpler to add a single command instead:

{ 'command': 'block-dedup-control',
  'data': { 'device': 'str', 'enable': 'bool' } }

where the user calls:

{ "execute": "block-dedup-control",
  "arguments": { "device": "ide0-hd0", "enable": false } }

to pause, and "enable":true to resume?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [RFC V1 11/12] qmp: Add block-pause-dedup.

[Benoît Canet]

> > +#
> > +# @device:   the name of the device to pause the deduplication on
> > +#
> > +# Returns: nothing on success
> > +#          If @device is not a valid block device, DeviceNotFound
> > +#          If @device is not deduplicated, DeviceNotDeduplicated
> 
> I don't think you need this second error.  A generic error is good
> enough unless we can prove that having a dedicated error class makes
> algorithmic sense for a given client, and I can't come up with such a
> scenario off the top of my head for libvirt.

Ok i'll remove it.

> 
> > +SQMP
> > +block-pause-dedup
> > +------------
> > +
> > +Pause the deduplication on a device that support it.
> 
> s/support/supports/
> 
> I notice that between this and patch 12, you are adding two very similar
> commands (block-pause-dedup, block-resume-dedup); would it be any
> simpler to add a single command instead:
> 
> { 'command': 'block-dedup-control',
>   'data': { 'device': 'str', 'enable': 'bool' } }
> 
> where the user calls:
> 
> { "execute": "block-dedup-control",
>   "arguments": { "device": "ide0-hd0", "enable": false } }
> 
> to pause, and "enable":true to resume?

Ok I'll merge these.

Regards

Benoît

[Qemu-devel] [RFC V1 12/12] qmp: Add block_resume_dedup.

[Benoît Canet]

---
 blockdev.c       |   18 ++++++++++++++++++
 qapi-schema.json |   18 ++++++++++++++++++
 qmp-commands.hx  |   23 +++++++++++++++++++++++
 3 files changed, 59 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 4c5f954..02b6535 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -914,6 +914,24 @@ void qmp_block_pause_dedup(const char *device, Error **errp)
     }
 }
 
+void qmp_block_resume_dedup(const char *device, Error **errp)
+{
+    BlockDriverState *bs;
+    int err;
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    err = bdrv_resume_dedup(bs);
+    if (err == -EINVAL) {
+        error_set(errp, QERR_DEVICE_NOT_DEDUPLICATED, bdrv_get_device_name(bs));
+        return;
+    }
+}
+
 static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
                                     int bdrv_flags, BlockDriver *drv,
                                     const char *password, Error **errp)
diff --git a/qapi-schema.json b/qapi-schema.json
index d8c8348..607f24b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -738,6 +738,24 @@
 { 'command': 'block-pause-dedup', 'data': {'device': 'str' } }
 
 ##
+# @block-resume-dedup:
+#
+# This command resume the deduplication on a device that support it.
+#
+# @device:   the name of the device to resume the deduplication on
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#          If @device is not deduplicated, DeviceNotDeduplicated
+#
+# Notes:  Not all block formats support deduplication one must use
+#         query-blockstats before and look at the optional deduplication field.
+#
+# Since: 1.5
+##
+{ 'command': 'block-resume-dedup', 'data': {'device': 'str' } }
+
+##
 # @BlockDeviceDedupInfo
 #
 # Statistics of the deduplication on a virtual block device implementing it
diff --git a/qmp-commands.hx b/qmp-commands.hx
index acc9fd0..d953847 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1259,6 +1259,29 @@ Example:
 EQMP
 
     {
+        .name       = "block-resume-dedup",
+        .args_type  = "device:B",
+        .mhandler.cmd_new = qmp_marshal_input_block_resume_dedup,
+    },
+
+SQMP
+block-resume-dedup
+------------
+
+Resume the deduplication on a device that support it.
+
+Arguments:
+
+- "device": device name (json-string)
+
+Example:
+
+-> { "execute": "block-resume-dedup", "arguments": { "device": "ide0-hd0" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "block_set_io_throttle",
         .args_type  = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
         .mhandler.cmd_new = qmp_marshal_input_block_set_io_throttle,
-- 
1.7.10.4