Release 3.0.13

- New store fixes:
  - Fix repeated rollback logic
  - Fix crash on rolled back object compaction
  - Fix postpone_load possibly merging different object chains
  - Fix block_csums import in vitastor-disk write-meta
  - Fix header checksum after vitastor-disk write-meta
- Old store fixes:
  - Fix batched fsync possibly skipped by some flush coroutines
- Improve ENOSPC test, fix possible crash on ENOSPC
- Add fsyncs to vitastor-disk prepare
- Fix possible crash on pg_lock check failure in sec_read_bmp
- Fix VitastorFS initialization when local_reads are enabled

CMakeLists.txt

@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 2.8...3.30)
 
 project(vitastor)
 
-set(VITASTOR_VERSION "3.0.12")
+set(VITASTOR_VERSION "3.0.13")
 
 include(CTest)
 

csi/Makefile

@@ -1,4 +1,4 @@
-VITASTOR_VERSION ?= v3.0.12
+VITASTOR_VERSION ?= v3.0.13
 
 all: build push
 

csi/deploy/004-csi-nodeplugin.yaml

@@ -49,7 +49,7 @@ spec:
             capabilities:
               add: ["SYS_ADMIN"]
             allowPrivilegeEscalation: true
-          image: vitalif/vitastor-csi:v3.0.12
+          image: vitalif/vitastor-csi:v3.0.13
           args:
             - "--node=$(NODE_ID)"
             - "--endpoint=$(CSI_ENDPOINT)"

csi/deploy/007-csi-provisioner.yaml

@@ -121,7 +121,7 @@ spec:
             privileged: true
             capabilities:
               add: ["SYS_ADMIN"]
-          image: vitalif/vitastor-csi:v3.0.12
+          image: vitalif/vitastor-csi:v3.0.13
           args:
             - "--node=$(NODE_ID)"
             - "--endpoint=$(CSI_ENDPOINT)"

csi/src/config.go

@@ -5,7 +5,7 @@ package vitastor
 
 const (
     vitastorCSIDriverName    = "csi.vitastor.io"
-    vitastorCSIDriverVersion = "3.0.12"
+    vitastorCSIDriverVersion = "3.0.13"
 )
 
 // Config struct fills the parameters of request or user input

debian/changelog

@@ -1,4 +1,4 @@
-vitastor (3.0.12-1) unstable; urgency=medium
+vitastor (3.0.13-1) unstable; urgency=medium
 
   * Bugfixes
 

docker/Makefile

@@ -1,4 +1,4 @@
-VITASTOR_VERSION ?= v3.0.12
+VITASTOR_VERSION ?= v3.0.13
 
 all: build push
 

docker/etc/vitastor/docker.conf

@@ -4,7 +4,7 @@
 #
 
 # Desired Vitastor version
-VITASTOR_VERSION=v3.0.12
+VITASTOR_VERSION=v3.0.13
 
 # Additional arguments for all containers
 # For example, you may want to specify a custom logging driver here

docs/installation/docker.en.md

@@ -26,9 +26,9 @@ at Vitastor Kubernetes operator: https://github.com/Antilles7227/vitastor-operat
 The instruction is very simple.
 
 1. Download a Docker image of the desired version: \
-   `docker pull vitalif/vitastor:v3.0.12`
+   `docker pull vitalif/vitastor:v3.0.13`
 2. Install scripts to the host system: \
-   `docker run --rm -it -v /etc:/host-etc -v /usr/bin:/host-bin vitalif/vitastor:v3.0.12 install.sh`
+   `docker run --rm -it -v /etc:/host-etc -v /usr/bin:/host-bin vitalif/vitastor:v3.0.13 install.sh`
 3. Reload udev rules: \
    `udevadm control --reload-rules`
 4. Enable the vitastor-host service: \

docs/installation/docker.ru.md

@@ -25,9 +25,9 @@ Vitastor можно установить в Docker/Podman. При этом etcd,
 Инструкция по установке максимально простая.
 
 1. Скачайте Docker-образ желаемой версии: \
-   `docker pull vitalif/vitastor:v3.0.12`
+   `docker pull vitalif/vitastor:v3.0.13`
 2. Установите скрипты в хост-систему командой: \
-   `docker run --rm -it -v /etc:/host-etc -v /usr/bin:/host-bin vitalif/vitastor:v3.0.12 install.sh`
+   `docker run --rm -it -v /etc:/host-etc -v /usr/bin:/host-bin vitalif/vitastor:v3.0.13 install.sh`
 3. Перезагрузите правила udev: \
    `udevadm control --reload-rules`
 4. Включите сервис vitastor-host: \

mon/lp_optimizer/lp_optimizer.js

@@ -112,9 +112,10 @@ function make_cyclic(pgs, parity_space)
 {
     if (parity_space > 1)
     {
-        for (const pg in pgs)
+        for (const id in pgs)
         {
-            for (let i = 1; i < pg.size; i++)
+            const pg = pgs[id];
+            for (let i = 1; i < pg.length; i++)
             {
                 const cyclic = [ ...pg.slice(i), ...pg.slice(0, i) ];
                 pgs['pg_'+cyclic.join('_')] = cyclic;

mon/package.json

@@ -1,6 +1,6 @@
 {
   "name": "vitastor-mon",
-  "version": "3.0.12",
+  "version": "3.0.13",
   "description": "Vitastor SDS monitor service",
   "main": "mon-main.js",
   "scripts": {

node-binding/package.json

@@ -1,6 +1,6 @@
 {
   "name": "vitastor",
-  "version": "3.0.12",
+  "version": "3.0.13",
   "description": "Low-level native bindings to Vitastor client library",
   "main": "index.js",
   "keywords": [

patches/cinder-vitastor.py

@@ -50,7 +50,7 @@ from cinder.volume import configuration
 from cinder.volume import driver
 from cinder.volume import volume_utils
 
-VITASTOR_VERSION = '3.0.12'
+VITASTOR_VERSION = '3.0.13'
 
 LOG = logging.getLogger(__name__)
 

rpm/vitastor-el10.spec

@@ -1,11 +1,11 @@
 Name:           vitastor
-Version:        3.0.12
+Version:        3.0.13
 Release:        1%{?dist}
 Summary:        Vitastor, a fast software-defined clustered block storage
 
 License:        Vitastor Network Public License 1.1
 URL:            https://vitastor.io/
-Source0:        vitastor-3.0.12.el10.tar.gz
+Source0:        vitastor-3.0.13.el10.tar.gz
 
 BuildRequires:  gperftools-devel
 BuildRequires:  gcc-c++

rpm/vitastor-el7.spec

@@ -1,11 +1,11 @@
 Name:           vitastor
-Version:        3.0.12
+Version:        3.0.13
 Release:        1%{?dist}
 Summary:        Vitastor, a fast software-defined clustered block storage
 
 License:        Vitastor Network Public License 1.1
 URL:            https://vitastor.io/
-Source0:        vitastor-3.0.12.el7.tar.gz
+Source0:        vitastor-3.0.13.el7.tar.gz
 
 BuildRequires:  gperftools-devel
 BuildRequires:  devtoolset-9-gcc-c++

rpm/vitastor-el8.spec

@@ -1,11 +1,11 @@
 Name:           vitastor
-Version:        3.0.12
+Version:        3.0.13
 Release:        1%{?dist}
 Summary:        Vitastor, a fast software-defined clustered block storage
 
 License:        Vitastor Network Public License 1.1
 URL:            https://vitastor.io/
-Source0:        vitastor-3.0.12.el8.tar.gz
+Source0:        vitastor-3.0.13.el8.tar.gz
 
 BuildRequires:  gperftools-devel
 BuildRequires:  gcc-toolset-9-gcc-c++

rpm/vitastor-el9.spec

@@ -1,11 +1,11 @@
 Name:           vitastor
-Version:        3.0.12
+Version:        3.0.13
 Release:        1%{?dist}
 Summary:        Vitastor, a fast software-defined clustered block storage
 
 License:        Vitastor Network Public License 1.1
 URL:            https://vitastor.io/
-Source0:        vitastor-3.0.12.el9.tar.gz
+Source0:        vitastor-3.0.13.el9.tar.gz
 
 BuildRequires:  gperftools-devel
 BuildRequires:  gcc-c++

src/CMakeLists.txt

@@ -20,7 +20,7 @@ if("${CMAKE_INSTALL_PREFIX}" MATCHES "^/usr/local/?$")
 endif()
 set(ENABLE_COVERAGE false CACHE BOOL "Enable code coverage")
 
-add_definitions(-DVITASTOR_VERSION="3.0.12")
+add_definitions(-DVITASTOR_VERSION="3.0.13")
 add_definitions(-D_GNU_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -Wno-sign-compare -Wno-comment -Wno-parentheses -Wno-pointer-arith -fdiagnostics-color=always -fno-omit-frame-pointer -fvisibility=hidden -I ${CMAKE_SOURCE_DIR}/src)
 add_link_options(-fno-omit-frame-pointer)
 if (${WITH_ASAN})

src/blockstore/blockstore_disk.cpp

@@ -517,7 +517,7 @@ void blockstore_disk_t::close_all()
 
 // Sadly DISCARD only works through ioctl(), but it seems to always block the device queue,
 // so it's not a big deal that we can only run it synchronously.
-int blockstore_disk_t::trim_data(std::function<bool(uint64_t)> is_free)
+int blockstore_disk_t::trim_data(std::function<bool(uint64_t)> is_used)
 {
     if (mock_mode)
     {
@@ -528,7 +528,7 @@ int blockstore_disk_t::trim_data(std::function<bool(uint64_t)> is_free)
     uint64_t discarded = 0;
     for (; i <= block_count; i++)
     {
-        if (i >= block_count || is_free(i))
+        if (i >= block_count || is_used(i))
         {
             if (i > j && (i-j)*data_block_size >= min_discard_size)
             {

src/blockstore/blockstore_disk.h

@@ -83,7 +83,7 @@ struct blockstore_disk_t
     void calc_lengths(bool skip_meta_check = false);
     void check_lengths();
     void close_all();
-    int trim_data(std::function<bool(uint64_t)> is_free);
+    int trim_data(std::function<bool(uint64_t)> is_used);
 
     inline uint64_t dirty_dyn_size(uint64_t offset, uint64_t len)
     {

src/blockstore/blockstore_flush.cpp

@@ -289,30 +289,41 @@ resume_1:
     {
         init_fsync_data();
     }
-    if (bs->log_level > 10)
+    if (compact_info.do_delete)
     {
-        printf("Compacting %jx:%jx v%ju..v%ju / l%ju..l%ju (%d writes)\n", cur_oid.inode, cur_oid.stripe,
+        if (bs->log_level > 10)
-            compact_info.clean_wr->version, compact_info.compact_version,
-            compact_info.clean_wr->lsn, compact_info.compact_lsn, copy_count);
-    }
-    mem_or(new_bmp, compact_info.clean_wr->get_int_bitmap(bs->heap), bs->dsk.clean_entry_bitmap_size);
-    if (!bitmap_copied)
-    {
-        memcpy(new_ext_bmp, compact_info.clean_wr->get_ext_bitmap(bs->heap), bs->dsk.clean_entry_bitmap_size);
-        bitmap_copied = true;
-    }
-    if (bs->dsk.csum_block_size && bs->dsk.csum_block_size <= bs->dsk.bitmap_granularity)
-    {
-        memcpy(new_csums, compact_info.clean_wr->get_checksums(bs->heap), bs->dsk.data_block_size/bs->dsk.csum_block_size * (bs->dsk.data_csum_type & 0xFF));
-        for (size_t i = csum_copy.size(); i > 0; i--)
         {
-            auto wr = csum_copy[i-1];
+            printf("Compacting %jx:%jx up to l%ju (delete)\n", cur_oid.inode, cur_oid.stripe, compact_info.compact_lsn);
-            memcpy(new_csums + wr->small().offset/bs->dsk.csum_block_size*(bs->dsk.data_csum_type & 0xFF),
-                wr->get_checksums(bs->heap), wr->small().len/bs->dsk.csum_block_size*(bs->dsk.data_csum_type & 0xFF));
         }
-        csum_copy.clear();
+        clean_loc = UINT64_MAX;
+    }
+    else
+    {
+        if (bs->log_level > 10)
+        {
+            printf("Compacting %jx:%jx v%ju..v%ju / l%ju..l%ju (%d writes)\n", cur_oid.inode, cur_oid.stripe,
+                compact_info.clean_wr->version, compact_info.compact_version,
+                compact_info.clean_wr->lsn, compact_info.compact_lsn, copy_count);
+        }
+        mem_or(new_bmp, compact_info.clean_wr->get_int_bitmap(bs->heap), bs->dsk.clean_entry_bitmap_size);
+        if (!bitmap_copied)
+        {
+            memcpy(new_ext_bmp, compact_info.clean_wr->get_ext_bitmap(bs->heap), bs->dsk.clean_entry_bitmap_size);
+            bitmap_copied = true;
+        }
+        if (bs->dsk.csum_block_size && bs->dsk.csum_block_size <= bs->dsk.bitmap_granularity)
+        {
+            memcpy(new_csums, compact_info.clean_wr->get_checksums(bs->heap), bs->dsk.data_block_size/bs->dsk.csum_block_size * (bs->dsk.data_csum_type & 0xFF));
+            for (size_t i = csum_copy.size(); i > 0; i--)
+            {
+                auto wr = csum_copy[i-1];
+                memcpy(new_csums + wr->small().offset/bs->dsk.csum_block_size*(bs->dsk.data_csum_type & 0xFF),
+                    wr->get_checksums(bs->heap), wr->small().len/bs->dsk.csum_block_size*(bs->dsk.data_csum_type & 0xFF));
+            }
+            csum_copy.clear();
+        }
+        clean_loc = compact_info.clean_wr->big_location(bs->heap);
     }
-    clean_loc = compact_info.clean_wr->big_location(bs->heap);
     overwrite_start = overwrite_end = 0;
     if (read_vec.size() > 0)
     {
@@ -625,7 +636,7 @@ int journal_flusher_co::check_and_punch_checksums()
 
 bool journal_flusher_co::calc_block_checksums()
 {
-    if (bs->dsk.csum_block_size <= bs->dsk.bitmap_granularity)
+    if (bs->dsk.csum_block_size <= bs->dsk.bitmap_granularity || compact_info.do_delete)
     {
         return true;
     }

src/blockstore/blockstore_heap.cpp

@@ -560,7 +560,7 @@ void blockstore_heap_t::finish_load()
         size_t s = 0, e, n = postponed_items.size();
         for (e = 1; e <= n; e++)
         {
-            if (e >= n || postponed_items[e]->entry.inode != postponed_items[s]->entry.inode &&
+            if (e >= n || postponed_items[e]->entry.inode != postponed_items[s]->entry.inode ||
                 postponed_items[e]->entry.stripe != postponed_items[s]->entry.stripe)
             {
                 insert_list_items(postponed_items.data()+s, e-s, false);
@@ -2038,7 +2038,10 @@ void blockstore_heap_t::iterate_with_stable(heap_entry_t *obj, uint64_t max_lsn,
     {
         if (old_wr->type() == BS_HEAP_ROLLBACK)
         {
-            rollback_version = old_wr->version;
+            if (rollback_version > old_wr->version)
+            {
+                rollback_version = old_wr->version;
+            }
         }
         else if (old_wr->type() == BS_HEAP_COMMIT)
         {
@@ -2090,7 +2093,10 @@ heap_compact_t blockstore_heap_t::iterate_compaction(heap_entry_t *obj, uint64_t
                 res.compact_lsn = wr->lsn;
                 res.compact_version = wr->version;
             }
-            rollback_version = wr->version;
+            if (rollback_version > wr->version)
+            {
+                rollback_version = wr->version;
+            }
             continue;
         }
         if (wr->type() == BS_HEAP_COMMIT && wr->lsn <= fsynced_lsn)

src/blockstore/multilist.cpp

@@ -12,7 +12,7 @@ multilist_alloc_t::multilist_alloc_t(uint32_t count, uint32_t maxn):
     count(count), maxn(maxn)
 {
     // not-so-memory-efficient: 16 MB memory per 1 GB buffer space, but buffer spaces are small, so OK
-    assert(count > 1 && count < 0x80000000);
+    assert(count > 1 && count < 0x80000000 && count >= maxn);
     sizes.resize(count);
     nexts.resize(count); // nexts[i] = 0 -> area is used; nexts[i] = 1 -> no next; nexts[i] >= 2 -> next item
     prevs.resize(count);

src/blockstore/v1/flush.cpp

@@ -1349,7 +1349,7 @@ bool journal_flusher_co::fsync_batch(bool fsync_meta, int wait_base)
         cur_sync->ready_count++;
         flusher->syncing_flushers++;
     resume_1:
-        if (!cur_sync->state)
+        if (cur_sync->state == 0)
         {
             if (flusher->syncing_flushers >= flusher->active_flushers || !flusher->flush_queue.size())
             {
@@ -1377,6 +1377,12 @@ bool journal_flusher_co::fsync_batch(bool fsync_meta, int wait_base)
                 return false;
             }
         }
+        else if (cur_sync->state == 1)
+        {
+            // Wait for fsync completion
+            wait_state = wait_base+1;
+            return false;
+        }
         flusher->syncing_flushers--;
         cur_sync->ready_count--;
         if (cur_sync->ready_count == 0)

src/client/vitastor.pc.in

@@ -6,7 +6,7 @@ includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@
 
 Name: Vitastor
 Description: Vitastor client library
-Version: 3.0.12
+Version: 3.0.13
 Libs: -L${libdir} -lvitastor_client
 Cflags: -I${includedir}
 

src/disk_tool/disk_tool_meta.cpp

@@ -154,7 +154,7 @@ csum_unknown:
         dsk.bitmap_granularity = hdr->bitmap_granularity;
         dsk.clean_entry_bitmap_size = (hdr->data_block_size / hdr->bitmap_granularity + 7) / 8;
         dsk.clean_entry_size = sizeof(clean_disk_entry) + 2*dsk.clean_entry_bitmap_size
-            + (hdr->data_csum_type
+            + (hdr->csum_block_size
                 ? ((hdr->data_block_size+hdr->csum_block_size-1)/hdr->csum_block_size
                     *(hdr->data_csum_type & 0xff))
                 : 0)
@@ -544,7 +544,7 @@ int disk_tool_t::write_json_meta(json11::Json meta)
     uint32_t new_clean_entry_header_size = (new_hdr->version == BLOCKSTORE_META_FORMAT_V1
         ? sizeof(clean_disk_entry) : sizeof(clean_disk_entry) + 4 /*entry_csum*/);
     new_clean_entry_bitmap_size = (new_hdr->data_block_size / new_hdr->bitmap_granularity + 7) / 8;
-    new_data_csum_size = (new_hdr->data_csum_type
+    new_data_csum_size = (new_hdr->csum_block_size
         ? ((new_hdr->data_block_size+new_hdr->csum_block_size-1)/new_hdr->csum_block_size*(new_hdr->data_csum_type & 0xFF))
         : 0);
     new_clean_entry_size = new_clean_entry_header_size + 2*new_clean_entry_bitmap_size + new_data_csum_size;
@@ -573,7 +573,7 @@ int disk_tool_t::write_json_meta(json11::Json meta)
             ((uint8_t*)new_entry) + sizeof(clean_disk_entry) + new_clean_entry_bitmap_size);
         if (new_hdr->version == BLOCKSTORE_META_FORMAT_V2)
         {
-            if (new_hdr->data_csum_type != 0)
+            if (new_data_csum_size)
             {
                 fromhexstr(e["data_csum"].string_value(), new_data_csum_size,
                     ((uint8_t*)new_entry) + sizeof(clean_disk_entry) + 2*new_clean_entry_bitmap_size);
@@ -613,7 +613,7 @@ int disk_tool_t::write_json_heap(json11::Json meta, json11::Json journal)
     new_clean_entry_bitmap_size = (new_meta_hdr->data_block_size / new_meta_hdr->bitmap_granularity + 7) / 8;
     new_clean_entry_size = 0;
     new_entries_per_block = 0;
-    new_data_csum_size = (new_meta_hdr->data_csum_type
+    new_data_csum_size = (new_meta_hdr->csum_block_size
         ? ((new_meta_hdr->data_block_size+new_meta_hdr->csum_block_size-1)/new_meta_hdr->csum_block_size*(new_meta_hdr->data_csum_type & 0xFF))
         : 0);
     new_journal_buf = NULL;
@@ -746,7 +746,7 @@ close_err0:
                 }
                 if (write_entry["block_csums"].is_string() && wr->get_checksums(&heap))
                 {
-                    fromhexstr(write_entry["block_csums"].string_value(), heap.get_csum_size(wr), wr->get_ext_bitmap(&heap));
+                    fromhexstr(write_entry["block_csums"].string_value(), heap.get_csum_size(wr), wr->get_checksums(&heap));
                 }
                 if (write_entry["data_crc32c"].is_string() && wr->get_checksum(&heap))
                 {
@@ -793,7 +793,7 @@ close_err:
             wr->size = wr->get_size(&heap);
             fromhexstr(meta_entry["bitmap"].string_value(), new_clean_entry_bitmap_size, wr->get_int_bitmap(&heap));
             fromhexstr(meta_entry["ext_bitmap"].string_value(), new_clean_entry_bitmap_size, wr->get_ext_bitmap(&heap));
-            if (new_meta_hdr->data_csum_type != 0)
+            if (new_data_csum_size)
                 fromhexstr(meta_entry["data_csum"].string_value(), new_data_csum_size, wr->get_checksums(&heap));
             wr->crc32c = wr->calc_crc32c();
             assert((uint8_t*)wr + wr->size == new_meta_buf + meta_offset + used_space);
@@ -828,7 +828,7 @@ close_err:
                         fromhexstr(rec["data"].string_value(), wr_len, new_journal_buf+buffer_pos);
                         if (wr_len > 0)
                         {
-                            if (!new_meta_hdr->data_csum_type)
+                            if (!new_meta_hdr->csum_block_size)
                                 *wr->get_checksum(&heap) = crc32c(0, new_journal_buf+buffer_pos, wr_len);
                             else
                                 heap.calc_block_checksums((uint32_t*)wr->get_checksums(&heap), new_journal_buf+buffer_pos, NULL, wr_offset, wr_offset+wr_len, true, NULL);
@@ -841,7 +841,7 @@ close_err:
                         wr->set_big_location(&heap, sscanf_json(NULL, rec["loc"]));
                         bitmap_set(wr->get_int_bitmap(&heap), wr_offset, wr_len, new_meta_hdr->bitmap_granularity);
                         fromhexstr(rec["bitmap"].string_value(), new_clean_entry_bitmap_size, wr->get_ext_bitmap(&heap));
-                        if (new_meta_hdr->data_csum_type != 0)
+                        if (new_meta_hdr->csum_block_size)
                         {
                             if ((wr_offset % new_meta_hdr->csum_block_size) || (wr_len % new_meta_hdr->csum_block_size))
                             {

src/disk_tool/disk_tool_udev.cpp

@@ -138,6 +138,7 @@ uint32_t disk_tool_t::write_osd_superblock(std::string device, json11::Json para
         free(buf);
         return 0;
     }
+    fsync(fd);
     close(fd);
     free(buf);
     if (!test_mode)
@@ -419,9 +420,9 @@ int disk_tool_t::clear_osd_superblock(const std::string & dev)
                     r = 0;
             }
         }
-    }
+        fsync(fd);
-    if (fd >= 0)
         close(fd);
+    }
     free(buf);
     buf = NULL;
     return r;

src/kv/kv_db.cpp

@@ -1613,6 +1613,7 @@ void kv_op_t::create_root()
         return;
     }
     auto new_offset = db->alloc_block();
+    auto new_next = db->next_free;
     assert(new_offset == 0);
     auto blk = &db->block_cache[0];
     blk->usage = db->usage_counter;
@@ -1628,6 +1629,11 @@ void kv_op_t::create_root()
         if (res == -EINTR)
         {
             db->clear_allocation_block(blk->offset);
+            if (db->next_free == new_next)
+            {
+                // When retrying create_root, reset the position
+                db->next_free = 0;
+            }
             auto blk_offset = blk->offset;
             del_block_level(db, blk);
             db->block_cache.erase(blk_offset);

src/nfs/nfs_kv_write.cpp

@@ -338,6 +338,7 @@ static void nfs_do_fsync(nfs_kv_write_state *st, int state)
     op->opcode = OSD_OP_SYNC;
     op->callback = [st, state](cluster_op_t *op)
     {
+        st->res = op->retval;
         delete op;
         nfs_kv_continue_write(st, state);
     };
@@ -904,6 +905,12 @@ resume_7:
                 return;
             }
 resume_8:
+            if (st->res < 0)
+            {
+                auto cb = std::move(st->cb);
+                cb(st->res);
+                return;
+            }
             // We always have to change inode entry on shared writes
             st->proxy->kvfs->write_inode(st->ino, new_shared_ientry(st), true, [st](int res)
             {

src/osd/osd_primary_subops.cpp

@@ -200,6 +200,7 @@ void osd_t::submit_primary_subop(osd_op_t *cur_op, osd_op_t *subop,
     {
         clock_gettime(CLOCK_REALTIME, &subop->tv_begin);
         subop->op_type = (uint64_t)cur_op; // also dirty
+        subop->osd_num = this->osd_num;
         subop->bs_op = new blockstore_op_t((blockstore_op_t){
             .opcode = (uint64_t)(wr ? (cur_op->op_data->pg->scheme == POOL_SCHEME_REPLICATED ? BS_OP_WRITE_STABLE : BS_OP_WRITE) : BS_OP_READ),
             .callback = [subop, this](blockstore_op_t *bs_subop)

src/osd/osd_secondary.cpp

@@ -258,8 +258,7 @@ void osd_t::exec_sec_read_bmp(osd_op_t *cur_op)
             if (!sec_check_pg_lock(cl->in_osd_num, ov[i].oid, cur_op->req.sec_read_bmp.flags))
             {
                 free(reply_buf);
-                cur_op->bs_op->retval = -EPIPE;
+                finish_op(cur_op, -EPIPE);
-                secondary_op_callback(cur_op);
                 return;
             }
             bs->read_bitmap(ov[i].oid, ov[i].version, (uint8_t*)cur_buf + sizeof(uint64_t), (uint64_t*)cur_buf);

src/test/test_blockstore.cpp

@@ -642,6 +642,63 @@ static void test_padded_csum_parallel_read(bool perfect, uint32_t offset)
     free(op2.buf);
 }
 
+static void test_compact_rollback()
+{
+    printf("\n-- test_compact_rollback\n");
+
+    bs_test_t test;
+    test.default_cfg();
+    test.config["csum_block_size"] = "16384";
+    test.config["atomic_write_size"] = "0";
+    test.init();
+
+    // Write
+    printf("write\n");
+    blockstore_op_t op;
+    op.opcode = BS_OP_WRITE;
+    op.oid = { .inode = 1, .stripe = 0 };
+    op.version = 1;
+    op.offset = 8192;
+    op.len = 16384;
+    op.buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, 16384);
+    memset(op.buf, 0xaa, 16384);
+    test.exec_op(&op);
+    assert(op.retval == op.len);
+
+    // Rollback
+    printf("rollback\n");
+    op.opcode = BS_OP_ROLLBACK;
+    op.len = 1;
+    ((obj_ver_id*)op.buf)[0] = { .oid = { .inode = 1, .stripe = 0 }, .version = 0 };
+    test.exec_op(&op);
+    assert(op.retval == 0);
+
+    // Trigger & wait compaction
+    test.bs->flusher->request_trim();
+    while (test.bs->heap->get_compact_queue_size())
+        test.ringloop->loop();
+    while (test.bs->flusher->is_active())
+        test.ringloop->loop();
+    test.bs->flusher->release_trim();
+    // Check that compaction succeeded
+    assert(!test.bs->heap->get_to_compact_count());
+
+    // Check that the object does not exist
+    printf("checking that the object does not exist\n");
+    blockstore_op_t op2;
+    op2.opcode = BS_OP_READ;
+    op2.oid = { .inode = 1, .stripe = 0 };
+    op2.version = 1;
+    op2.offset = 0;
+    op2.len = 128*1024;
+    op2.buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, 128*1024);
+    test.exec_op(&op2);
+    assert(op2.retval == -ENOENT);
+
+    free(op.buf);
+    free(op2.buf);
+}
+
 // FIXME Add a simple intent_write / big_intent test
 
 int main(int narg, char *args[])
@@ -657,5 +714,6 @@ int main(int narg, char *args[])
     test_padded_csum_parallel_read(true, 8192);
     test_padded_csum_parallel_read(false, 16384);
     test_padded_csum_parallel_read(true, 16384);
+    test_compact_rollback();
     return 0;
 }

src/test/test_heap.cpp

@@ -965,6 +965,78 @@ void test_iterate_compaction()
         assert(small_writes == 1);
     }
 
+    {
+        blockstore_heap_t heap(&dsk, buffer_area.data());
+        heap.finish_recheck();
+
+        // Case: BIG_STABLE(v1 l1) SMALL(v2 l2) SMALL(v3 l3) ROLLBACK(v2 l4) ROLLBACK(v1 l5)
+        // -> compact by adding BIG_STABLE(v1 l6) and skip l2 and l3
+        uint32_t mblock = 0;
+        _test_big_write(heap, dsk, 1, 0, 1, 0, true, 0, 4096, buffer_area.data());
+        _test_small_write(heap, dsk, 1, 0, 2, 0, 4096, 0, false, buffer_area.data(), false);
+        _test_small_write(heap, dsk, 1, 0, 3, 4096, 4096, 4096, false, buffer_area.data(), false);
+
+        object_id oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0 };
+        auto obj = heap.read_entry(oid);
+        assert(obj->lsn == 3);
+
+        res = heap.add_rollback(obj, 2, &mblock);
+        assert(res == 0);
+        heap.start_block_write(mblock);
+        heap.complete_block_write(mblock);
+
+        res = heap.add_rollback(obj, 1, &mblock);
+        assert(res == 0);
+        heap.start_block_write(mblock);
+        heap.complete_block_write(mblock);
+
+        obj = heap.read_entry(oid);
+        assert(count_writes(heap, obj) == 5);
+
+        assert(heap.get_fsynced_lsn() == 5);
+        int small_writes = 0;
+        obj = heap.read_entry(oid);
+        auto compact_info = heap.iterate_compaction(obj, heap.get_fsynced_lsn(), false, [&](heap_entry_t *wr)
+        {
+            small_writes++;
+        });
+        assert(small_writes == 0);
+        assert(compact_info.compact_lsn == 5);
+        assert(compact_info.compact_version == 1);
+        assert(compact_info.clean_wr->lsn == 1);
+        assert(!compact_info.do_delete);
+
+        // persist
+        assert(heap.get_meta_block_used_space(0) > 0);
+        tmp.resize(dsk.meta_block_size);
+        heap.get_meta_block(0, tmp.data());
+    }
+    {
+        // reload heap and check that object state isn't changed and validation passes
+        blockstore_heap_t heap(&dsk, buffer_area.data());
+        uint64_t entries_loaded;
+        heap.load_blocks(0, dsk.meta_block_size, tmp.data(), false, entries_loaded);
+        heap.finish_load();
+        bool done = heap.recheck_small_writes([&](bool, uint64_t, uint64_t, uint8_t*, std::function<void()> cb) {}, 1);
+        assert(done);
+        heap.finish_recheck();
+        auto mod = heap.get_recheck_modified_blocks();
+        assert(mod.size() == 0);
+
+        object_id oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0 };
+        auto obj = heap.read_entry(oid);
+        int small_writes = 0;
+        auto compact_info = heap.iterate_compaction(obj, heap.get_fsynced_lsn(), true, [&](heap_entry_t *wr)
+        {
+            small_writes++;
+        });
+        assert(compact_info.compact_lsn == 5);
+        assert(compact_info.compact_version == 1);
+        assert(compact_info.clean_wr->lsn == 1);
+        assert(!compact_info.do_delete);
+        assert(small_writes == 0);
+    }
+
     printf("OK test_iterate_compaction\n");
 }
 

src/util/allocator.cpp

@@ -152,8 +152,8 @@ void bitmap_set(void *bitmap, uint64_t start, uint64_t len, uint64_t bitmap_gran
         *((uint64_t*)bitmap) = UINT64_MAX;
     else
     {
-        unsigned bit_start = start / bitmap_granularity;
+        uint64_t bit_start = start / bitmap_granularity;
-        unsigned bit_end = ((start + len) + bitmap_granularity - 1) / bitmap_granularity;
+        uint64_t bit_end = ((start + len) + bitmap_granularity - 1) / bitmap_granularity;
         while (bit_start < bit_end)
         {
             if (!(bit_start & 7) && bit_end >= bit_start+8)

src/util/crc32c.c

@@ -417,5 +417,5 @@ uint32_t crc32c_pad(uint32_t prev_crc, const void *buf, size_t len, size_t left_
 
 uint32_t crc32c_nopad(uint32_t prev_crc, const void *buf, size_t len, size_t left_pad, size_t right_pad)
 {
-    return crc32c(0, buf, len);
+    return crc32c(prev_crc, buf, len);
 }