Only update bitmaps when writing data in primary subops

Fixes fully degraded EC writes and recovering EC writes corrupting bitmaps

src/blockstore/blockstore_heap.cpp

@@ -1571,10 +1571,22 @@ int blockstore_heap_t::add_small_write(object_id oid, heap_entry_t **obj_ptr, ui
         wr->small().location = location;
         if (bitmap)
             memcpy(wr->get_ext_bitmap(this), bitmap, dsk->clean_entry_bitmap_size);
-        else if (obj)
-            memcpy(wr->get_ext_bitmap(this), obj->get_ext_bitmap(this), dsk->clean_entry_bitmap_size);
         else
-            memset(wr->get_ext_bitmap(this), 0, dsk->clean_entry_bitmap_size);
+        {
+            bool found = false;
+            iterate_with_stable(obj, UINT64_MAX, [&](heap_entry_t *old_wr, bool stable)
+            {
+                if (old_wr->get_ext_bitmap(this))
+                {
+                    found = true;
+                    memcpy(wr->get_ext_bitmap(this), old_wr->get_ext_bitmap(this), dsk->clean_entry_bitmap_size);
+                    return false;
+                }
+                return true;
+            });
+            if (!found)
+                memset(wr->get_ext_bitmap(this), 0, dsk->clean_entry_bitmap_size);
+        }
         calc_checksums(wr, (uint8_t*)data, true);
         *obj_ptr = wr;
     });

src/osd/osd_primary_subops.cpp

@@ -192,8 +192,16 @@ void osd_t::submit_primary_subop(osd_op_t *cur_op, osd_op_t *subop,
         subop_len = 0;
     }
     si->read_error = false;
-    subop->bitmap = si->bmp_buf;
-    subop->bitmap_len = clean_entry_bitmap_size;
+    if (!wr || subop_len)
+    {
+        subop->bitmap = si->bmp_buf;
+        subop->bitmap_len = clean_entry_bitmap_size;
+    }
+    else
+    {
+        subop->bitmap = 0;
+        subop->bitmap_len = 0;
+    }
     // Using rmw_buf to pass pointer to stripes. Dirty but works
     subop->rmw_buf = si;
     if (si->osd_num == this->osd_num)
@@ -217,14 +225,14 @@ void osd_t::submit_primary_subop(osd_op_t *cur_op, osd_op_t *subop,
                 .len = subop_len,
             } },
             .buf = (uint8_t*)(wr ? si->write_buf : si->read_buf),
-            .bitmap = (uint8_t*)si->bmp_buf,
+            .bitmap = (uint8_t*)subop->bitmap,
         });
 #ifdef OSD_DEBUG
-         printf(
-             "Submit %s to local: %jx:%jx v%ju %u-%u\n", wr ? "write" : "read",
-             inode, op_data->oid.stripe | si->role, op_version,
-             subop->bs_op->offset, subop->bs_op->len
-         );
+        printf(
+            "Submit %s to local: %jx:%jx v%ju %u-%u bmp %08x\n", wr ? "write" : "read",
+            inode, cur_op->op_data->oid.stripe | si->role, op_version,
+            subop->bs_op->offset, subop->bs_op->len, *(uint32_t*)si->bmp_buf
+        );
 #endif
         bs->enqueue_op(subop->bs_op);
     }
@@ -243,14 +251,14 @@ void osd_t::submit_primary_subop(osd_op_t *cur_op, osd_op_t *subop,
             .version = op_version,
             .offset = wr ? si->write_start : si->read_start,
             .len = subop_len,
-            .attr_len = wr ? clean_entry_bitmap_size : 0,
+            .attr_len = !wr || subop_len ? clean_entry_bitmap_size : 0,
             .flags = cur_op->client_id == SELF_CLIENT && cur_op->req.hdr.opcode != OSD_OP_SCRUB ? OSD_OP_RECOVERY_RELATED : 0,
         };
 #ifdef OSD_DEBUG
         printf(
-            "Submit %s to osd %ju: %jx:%jx v%ju %u-%u\n", wr ? "write" : "read", si->osd_num,
-            inode, op_data->oid.stripe | si->role, op_version,
-            subop->req.sec_rw.offset, subop->req.sec_rw.len
+            "Submit %s to osd %ju: %jx:%jx v%ju %u-%u bmp %08x\n", wr ? "write" : "read", si->osd_num,
+            inode, cur_op->op_data->oid.stripe | si->role, op_version,
+            subop->req.sec_rw.offset, subop->req.sec_rw.len, *(uint32_t*)si->bmp_buf
         );
 #endif
         if (wr)

src/osd/osd_secondary.cpp

@@ -162,7 +162,10 @@ void osd_t::exec_secondary_real(osd_op_t *cur_op)
         cur_op->bs_op->offset = cur_op->req.sec_rw.offset;
         cur_op->bs_op->len = cur_op->req.sec_rw.len;
         cur_op->bs_op->buf = (uint8_t*)cur_op->buf;
-        cur_op->bs_op->bitmap = (uint8_t*)cur_op->bitmap;
+        if (cur_op->req.sec_rw.attr_len)
+            cur_op->bs_op->bitmap = (uint8_t*)cur_op->bitmap;
+        else
+            cur_op->bs_op->bitmap = NULL;
 #ifdef OSD_STUB
         cur_op->bs_op->retval = cur_op->bs_op->len;
 #endif

src/test/test_heap.cpp

@@ -230,7 +230,26 @@ void test_update(bool csum)
         _test_small_write(heap, dsk, 1, 0, 2, 8192, 4096, 16384, true, buffer_area.data()+16384, false);
 
         object_id oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0 };
-        assert(count_writes(heap, heap.read_entry(oid)) == 2);
+        heap_entry_t *obj = heap.read_entry(oid);
+        uint32_t mblock = 999;
+        int res = heap.add_small_write(oid, &obj, BS_HEAP_SMALL_WRITE, 3, 0, 0, 0, NULL, NULL, &mblock);
+        assert(res == 0);
+        assert(mblock == 0);
+
+        // Check inherited bitmap
+        uint8_t ref_ext_bitmap[dsk.clean_entry_bitmap_size];
+        memset(ref_ext_bitmap, 0xff, dsk.clean_entry_bitmap_size);
+        assert(!memcmp(obj->get_ext_bitmap(&heap), ref_ext_bitmap, dsk.clean_entry_bitmap_size));
+
+        mblock = 999;
+        res = heap.add_small_write(oid, &obj, BS_HEAP_SMALL_WRITE, 4, 0, 0, 0, NULL, NULL, &mblock);
+        assert(res == 0);
+        assert(mblock == 0);
+
+        // Check again - inherited from an unstable write
+        assert(!memcmp(obj->get_ext_bitmap(&heap), ref_ext_bitmap, dsk.clean_entry_bitmap_size));
+
+        assert(count_writes(heap, heap.read_entry(oid)) == 4);
     }
 
     printf("OK test_update %s\n", csum ? "csum" : "no_csum");