place_r2() links a new refblock into R1, but moves neither file_size
nor file_preallocated_area_start. The refblock stays at or above the
watermark if truncate_prealloc_safe() in allocate_cluster() fails
after place_r2(), or if a parallel mark_cluster_unused() lowers
free_cluster_search_pos before the second pass of
find_unused_cluster(). Truncation of preallocations on suspend then
cuts the file under the refblock: R1 on disk points past EOF, and
the next session gets -EIO on allocations in the range it covers.

Make find_unused_cluster() report a position not covered by R2
instead of placing the refblock there itself. allocate_cluster()
prepares that cluster the same way as a data cluster, with the punch,
extend and watermark update moved into prepare_unused_cluster(), and
calls place_r2() before searching again.

https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver
Fixes: 222b6f1291c1c ("dm-qcow2: Introduce driver to create block devices over 
QCOW2 files")

Signed-off-by: Alexander Ivanov <[email protected]>
---
 drivers/md/dm-qcow2-map.c | 88 ++++++++++++++++++++++-----------------
 1 file changed, 50 insertions(+), 38 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 34d9d9810f60..e8a41954c4c6 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2083,21 +2083,12 @@ static int parse_metadata(struct qcow2 *qcow2, struct 
qio **qio,
  * and connects it to R1 table entry.
  */
 static int place_r2(struct qcow2 *qcow2, struct qcow2_map_item *r1,
-                   struct qcow2_map_item *r2, loff_t r2_pos, struct qio **qio)
+                   struct qcow2_map_item *r2, loff_t r2_pos)
 {
        struct dm_target *ti = qcow2->tgt->ti;
        u64 page_id = r2_pos >> PAGE_SHIFT;
        int ret;
 
-       if (delay_if_writeback(qcow2, r1->md, r1->index_in_page, qio, true))
-               return 0;
-
-       ret = qcow2_punch_hole(qcow2->file, r2_pos, qcow2->clu_size);
-       if (ret) {
-               QC_ERR(ti, "punch hole: %d", ret);
-               return ret;
-       }
-
        ret = qcow2_alloc_and_insert_md_page(qcow2, page_id, &r2->md);
        if (ret < 0) {
                QC_ERR(ti, "Can't alloc: ret=%d, page_id=%llu", ret, page_id);
@@ -2113,7 +2104,7 @@ static int place_r2(struct qcow2 *qcow2, struct 
qcow2_map_item *r1,
        spin_unlock_irq(&qcow2->md_pages_lock);
 
        mark_cluster_used(qcow2, r2->md, r2->index_in_page);
-       return 1;
+       return 0;
 }
 
 static s32 find_unused_block_entry(struct qcow2 *qcow2, struct md_page *md,
@@ -2135,11 +2126,13 @@ static s32 find_unused_block_entry(struct qcow2 *qcow2, 
struct md_page *md,
 
 static loff_t find_unused_cluster(struct qcow2 *qcow2, struct qio **qio,
                                  struct qcow2_map_item *r1,
-                                 struct qcow2_map_item *r2)
+                                 struct qcow2_map_item *r2, bool *needs_r2)
 {
        u32 clu_size = qcow2->clu_size;
        s32 index, ret;
        loff_t pos;
+
+       *needs_r2 = false;
 again:
        pos = READ_ONCE(qcow2->free_cluster_search_pos);
        if (pos >= qcow2->reftable_max_file_size)
@@ -2151,15 +2144,15 @@ static loff_t find_unused_cluster(struct qcow2 *qcow2, 
struct qio **qio,
                        return ret;
                /*
                 * Since pos is not covered by R2, the whole cluster
-                * must be unused. Use it to store R2 cluster.
+                * must be unused. The caller stores R2 cluster there.
                 * Both indexes must be 0 here, because of we allocate
                 * clusters from small to big.
                 */
                WARN_ON_ONCE(r2->index_in_page != 0 || r2->index != 0);
-               ret = place_r2(qcow2, r1, r2, pos, qio);
-               if (ret <= 0)
-                       return ret;
-               goto again;
+               if (delay_if_writeback(qcow2, r1->md, r1->index_in_page, qio, 
true))
+                       return 0;
+               *needs_r2 = true;
+               return pos;
        }
 
        spin_lock_irq(&qcow2->md_pages_lock);
@@ -2436,6 +2429,37 @@ static int relocate_refcount_table(struct qcow2 *qcow2, 
struct qio **qio)
        return -EIO;
 }
 
+/*
+ * Zero the unused cluster at @pos and make the file size and
+ * file_preallocated_area_start cover it.
+ */
+static int prepare_unused_cluster(struct qcow2 *qcow2, loff_t pos)
+{
+       loff_t off, end = pos + qcow2->clu_size;
+       loff_t old_size = qcow2->file_size;
+       int ret;
+
+       if (pos < qcow2->file_preallocated_area_start) {
+               /* Clu at @pos may contain dirty data */
+               off = min_t(loff_t, old_size, end);
+               ret = qcow2_punch_hole(qcow2->file, pos, off - pos);
+               if (ret) {
+                       QC_ERR(qcow2->tgt->ti, "punch hole: %d", ret);
+                       return ret;
+               }
+       }
+
+       if (end > old_size) {
+               ret = truncate_prealloc_safe(qcow2, end, __func__);
+               if (ret)
+                       return ret;
+       }
+
+       if (end > qcow2->file_preallocated_area_start)
+               qcow2->file_preallocated_area_start = end;
+       return 0;
+}
+
 /*
  * This function is aimed to be called only from main work.
  * In case of wish to use it from more places, it's needed
@@ -2445,13 +2469,12 @@ static int relocate_refcount_table(struct qcow2 *qcow2, 
struct qio **qio)
 static loff_t allocate_cluster(struct qcow2 *qcow2, struct qio *qio,
                      struct md_page **r2_md, u32 *r2_index_in_page)
 {
-       u32 clu_size = qcow2->clu_size;
-       struct file *file = qcow2->file;
-       loff_t pos, off, end, old_size;
        struct qcow2_map_item r1, r2;
+       bool needs_r2;
+       loff_t pos;
        int ret;
 again:
-       pos = find_unused_cluster(qcow2, &qio, &r1, &r2);
+       pos = find_unused_cluster(qcow2, &qio, &r1, &r2, &needs_r2);
        if (unlikely(pos == -ENOENT)) {
                ret = relocate_refcount_table(qcow2, &qio);
                if (ret <= 0)
@@ -2461,28 +2484,17 @@ static loff_t allocate_cluster(struct qcow2 *qcow2, 
struct qio *qio,
        if (pos <= 0)
                return pos;
 
-       end = pos + clu_size;
-       old_size = qcow2->file_size;
-
-       if (pos < qcow2->file_preallocated_area_start) {
-               /* Clu at @pos may contain dirty data */
-               off = min_t(loff_t, old_size, end);
-               ret = qcow2_punch_hole(file, pos, off - pos);
-               if (ret) {
-                       QC_ERR(qcow2->tgt->ti, "punch hole: %d", ret);
-                       return ret;
-               }
-       }
+       ret = prepare_unused_cluster(qcow2, pos);
+       if (ret)
+               return ret;
 
-       if (end > old_size) {
-               ret = truncate_prealloc_safe(qcow2, end, __func__);
+       if (needs_r2) {
+               ret = place_r2(qcow2, &r1, &r2, pos);
                if (ret)
                        return ret;
+               goto again;
        }
 
-       if (end > qcow2->file_preallocated_area_start)
-               qcow2->file_preallocated_area_start = end;
-
        mark_cluster_used(qcow2, r2.md, r2.index_in_page);
        if (r2_md)
                *r2_md = r2.md;
-- 
2.53.0

_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to