Double-check for double-alloc bugs

mon/mon.js

@@ -709,10 +709,13 @@ class Mon
         for (const node_id in this.state.config.node_placement||{})
         {
             const node_cfg = this.state.config.node_placement[node_id];
-            if (!node_id || /^\d/.exec(node_id) ||
-                !node_cfg.level || !levels[node_cfg.level])
+            if (/^\d+$/.exec(node_id))
             {
-                // All nodes must have non-empty non-numeric IDs and valid levels
+                node_cfg.level = 'osd';
+            }
+            if (!node_id || !node_cfg.level || !levels[node_cfg.level])
+            {
+                // All nodes must have non-empty IDs and valid levels
                 continue;
             }
             tree[node_id] = { id: node_id, level: node_cfg.level, parent: node_cfg.parent, children: [] };
@@ -745,10 +748,10 @@ class Mon
                         .reduce((a, c) => { a[c] = true; return a; }, {});
                 }
                 delete tree[osd_num].children;
-                if (!tree[tree[osd_num].parent])
+                if (!tree[stat.host])
                 {
-                    tree[tree[osd_num].parent] = {
-                        id: tree[osd_num].parent,
+                    tree[stat.host] = {
+                        id: stat.host,
                         level: 'host',
                         parent: null,
                         children: [],

src/blockstore_flush.cpp

@@ -547,8 +547,9 @@ resume_1:
             clean_disk_entry *new_entry = (clean_disk_entry*)(meta_new.buf + meta_new.pos*bs->clean_entry_size);
             if (new_entry->oid.inode != 0 && new_entry->oid != cur.oid)
             {
-                printf("Fatal error (metadata corruption or bug): tried to delete metadata entry %lu (%lx:%lx) while deleting %lx:%lx\n",
-                    clean_loc >> bs->block_order, new_entry->oid.inode, new_entry->oid.stripe, cur.oid.inode, cur.oid.stripe);
+                printf("Fatal error (metadata corruption or bug): tried to delete metadata entry %lu (%lx:%lx v%lu) while deleting %lx:%lx\n",
+                    clean_loc >> bs->block_order, new_entry->oid.inode, new_entry->oid.stripe,
+                    new_entry->version, cur.oid.inode, cur.oid.stripe);
                 exit(1);
             }
             // zero out new metadata entry
@@ -559,8 +560,9 @@ resume_1:
             clean_disk_entry *new_entry = (clean_disk_entry*)(meta_new.buf + meta_new.pos*bs->clean_entry_size);
             if (new_entry->oid.inode != 0 && new_entry->oid != cur.oid)
             {
-                printf("Fatal error (metadata corruption or bug): tried to overwrite non-zero metadata entry %lu (%lx:%lx) with %lx:%lx\n",
-                    clean_loc >> bs->block_order, new_entry->oid.inode, new_entry->oid.stripe, cur.oid.inode, cur.oid.stripe);
+                printf("Fatal error (metadata corruption or bug): tried to overwrite non-zero metadata entry %lu (%lx:%lx v%lu) with %lx:%lx v%lu\n",
+                    clean_loc >> bs->block_order, new_entry->oid.inode, new_entry->oid.stripe, new_entry->version,
+                    cur.oid.inode, cur.oid.stripe, cur.version);
                 exit(1);
             }
             new_entry->oid = cur.oid;

src/blockstore_impl.h

@@ -251,6 +251,7 @@ class blockstore_impl_t
     int data_fd;
     uint64_t meta_size, meta_area, meta_len;
     uint64_t data_size, data_len;
+    uint64_t data_device_sect, meta_device_sect, journal_device_sect;
 
     void *metadata_buffer = NULL;
 

src/blockstore_open.cpp

@@ -295,9 +295,9 @@ void blockstore_impl_t::calc_lengths()
     }
 }
 
-void check_size(int fd, uint64_t *size, std::string name)
+static void check_size(int fd, uint64_t *size, uint64_t *sectsize, std::string name)
 {
-    int sectsize;
+    int sect;
     struct stat st;
     if (fstat(fd, &st) < 0)
     {
@@ -309,11 +309,14 @@ void check_size(int fd, uint64_t *size, std::string name)
     }
     else if (S_ISBLK(st.st_mode))
     {
-        if (ioctl(fd, BLKSSZGET, &sectsize) < 0 ||
-            ioctl(fd, BLKGETSIZE64, size) < 0 ||
-            sectsize != 512)
+        if (ioctl(fd, BLKGETSIZE64, size) < 0 ||
+            ioctl(fd, BLKSSZGET, &sect) < 0)
         {
-            throw std::runtime_error(name+" sector is not equal to 512 bytes");
+            throw std::runtime_error("failed to get "+name+" size or block size: "+strerror(errno));
+        }
+        if (sectsize)
+        {
+            *sectsize = sect;
         }
     }
     else
@@ -329,7 +332,14 @@ void blockstore_impl_t::open_data()
     {
         throw std::runtime_error("Failed to open data device");
     }
-    check_size(data_fd, &data_size, "data device");
+    check_size(data_fd, &data_size, &data_device_sect, "data device");
+    if (disk_alignment % data_device_sect)
+    {
+        throw std::runtime_error(
+            "disk_alignment ("+std::to_string(disk_alignment)+
+            ") is not a multiple of data device sector size ("+std::to_string(data_device_sect)+")"
+        );
+    }
     if (data_offset >= data_size)
     {
         throw std::runtime_error("data_offset exceeds device size = "+std::to_string(data_size));
@@ -350,7 +360,7 @@ void blockstore_impl_t::open_meta()
         {
             throw std::runtime_error("Failed to open metadata device");
         }
-        check_size(meta_fd, &meta_size, "metadata device");
+        check_size(meta_fd, &meta_size, &meta_device_sect, "metadata device");
         if (meta_offset >= meta_size)
         {
             throw std::runtime_error("meta_offset exceeds device size = "+std::to_string(meta_size));
@@ -363,12 +373,20 @@ void blockstore_impl_t::open_meta()
     else
     {
         meta_fd = data_fd;
+        meta_device_sect = data_device_sect;
         meta_size = 0;
         if (meta_offset >= data_size)
         {
             throw std::runtime_error("meta_offset exceeds device size = "+std::to_string(data_size));
         }
     }
+    if (meta_block_size % meta_device_sect)
+    {
+        throw std::runtime_error(
+            "meta_block_size ("+std::to_string(meta_block_size)+
+            ") is not a multiple of data device sector size ("+std::to_string(meta_device_sect)+")"
+        );
+    }
 }
 
 void blockstore_impl_t::open_journal()
@@ -380,7 +398,7 @@ void blockstore_impl_t::open_journal()
         {
             throw std::runtime_error("Failed to open journal device");
         }
-        check_size(journal.fd, &journal.device_size, "journal device");
+        check_size(journal.fd, &journal.device_size, &journal_device_sect, "journal device");
         if (!disable_flock && flock(journal.fd, LOCK_EX|LOCK_NB) != 0)
         {
             throw std::runtime_error(std::string("Failed to lock journal device: ") + strerror(errno));
@@ -389,6 +407,7 @@ void blockstore_impl_t::open_journal()
     else
     {
         journal.fd = meta_fd;
+        journal_device_sect = meta_device_sect;
         journal.device_size = 0;
         if (journal.offset >= data_size)
         {
@@ -406,4 +425,11 @@ void blockstore_impl_t::open_journal()
         if (!journal.sector_buf)
             throw std::bad_alloc();
     }
+    if (journal_block_size % journal_device_sect)
+    {
+        throw std::runtime_error(
+            "journal_block_size ("+std::to_string(journal_block_size)+
+            ") is not a multiple of journal device sector size ("+std::to_string(journal_device_sect)+")"
+        );
+    }
 }

src/blockstore_write.cpp

@@ -268,6 +268,16 @@ int blockstore_impl_t::dequeue_write(blockstore_op_t *op)
             cancel_all_writes(op, dirty_it, -ENOSPC);
             return 2;
         }
+#ifndef NDEBUG
+        // Double-check to not overwrite anything if easily possible
+        if (inmemory_meta)
+        {
+            uint64_t sector = (loc / (meta_block_size / clean_entry_size)) * meta_block_size;
+            uint64_t pos = (loc % (meta_block_size / clean_entry_size));
+            clean_disk_entry *meta_entry = (clean_disk_entry*)(metadata_buffer + sector + pos*clean_entry_size);
+            assert(!meta_entry->oid.inode);
+        }
+#endif
         write_iodepth++;
         BS_SUBMIT_GET_SQE(sqe, data);
         dirty_it->second.location = loc << block_order;

src/osd_peering.cpp

@@ -194,6 +194,24 @@ void osd_t::start_pg_peering(pg_t & pg)
             });
         }
     }
+    if (pg.pg_cursize < pg.pg_minsize)
+    {
+        pg.state = PG_INCOMPLETE;
+        report_pg_state(pg);
+        return;
+    }
+    std::set<osd_num_t> cur_peers;
+    for (auto pg_osd: pg.all_peers)
+    {
+        if (pg_osd == this->osd_num || msgr.osd_peer_fds.find(pg_osd) != msgr.osd_peer_fds.end())
+        {
+            cur_peers.insert(pg_osd);
+        }
+        else if (msgr.wanted_peers.find(pg_osd) == msgr.wanted_peers.end())
+        {
+            msgr.connect_peer(pg_osd, st_cli.peer_states[pg_osd]);
+        }
+    }
     if (pg.target_history.size())
     {
         // Refuse to start PG if no peers are available from any of the historical OSD sets
@@ -222,24 +240,6 @@ void osd_t::start_pg_peering(pg_t & pg)
             }
         }
     }
-    if (pg.pg_cursize < pg.pg_minsize)
-    {
-        pg.state = PG_INCOMPLETE;
-        report_pg_state(pg);
-        return;
-    }
-    std::set<osd_num_t> cur_peers;
-    for (auto pg_osd: pg.all_peers)
-    {
-        if (pg_osd == this->osd_num || msgr.osd_peer_fds.find(pg_osd) != msgr.osd_peer_fds.end())
-        {
-            cur_peers.insert(pg_osd);
-        }
-        else if (msgr.wanted_peers.find(pg_osd) == msgr.wanted_peers.end())
-        {
-            msgr.connect_peer(pg_osd, st_cli.peer_states[pg_osd]);
-        }
-    }
     pg.cur_peers.insert(pg.cur_peers.begin(), cur_peers.begin(), cur_peers.end());
     if (pg.peering_state)
     {

src/rw_blocking.cpp

@@ -60,7 +60,7 @@ int readv_blocking(int fd, iovec *iov, int iovcnt)
     int done = 0;
     while (v < iovcnt)
     {
-        ssize_t r = readv(fd, iov, iovcnt);
+        ssize_t r = readv(fd, iov+v, iovcnt-v);
         if (r < 0)
         {
             if (errno != EAGAIN && errno != EPIPE)
@@ -70,6 +70,7 @@ int readv_blocking(int fd, iovec *iov, int iovcnt)
             }
             continue;
         }
+        done += r;
         while (v < iovcnt)
         {
             if (iov[v].iov_len > r)
@@ -80,10 +81,10 @@ int readv_blocking(int fd, iovec *iov, int iovcnt)
             }
             else
             {
+                r -= iov[v].iov_len;
                 v++;
             }
         }
-        done += r;
     }
     return done;
 }
@@ -94,7 +95,7 @@ int writev_blocking(int fd, iovec *iov, int iovcnt)
     int done = 0;
     while (v < iovcnt)
     {
-        ssize_t r = writev(fd, iov, iovcnt);
+        ssize_t r = writev(fd, iov+v, iovcnt-v);
         if (r < 0)
         {
             if (errno != EAGAIN && errno != EPIPE)
@@ -104,6 +105,7 @@ int writev_blocking(int fd, iovec *iov, int iovcnt)
             }
             continue;
         }
+        done += r;
         while (v < iovcnt)
         {
             if (iov[v].iov_len > r)
@@ -114,10 +116,10 @@ int writev_blocking(int fd, iovec *iov, int iovcnt)
             }
             else
             {
+                r -= iov[v].iov_len;
                 v++;
             }
         }
-        done += r;
     }
     return done;
 }

src/stub_osd.cpp

@@ -116,7 +116,7 @@ int bind_stub(const char *bind_address, int bind_port)
 void run_stub(int peer_fd)
 {
     osd_any_op_t op;
-    osd_any_reply_t reply;
+    osd_any_reply_t reply = { 0 };
     void *buf = NULL;
     while (1)
     {
@@ -139,7 +139,7 @@ void run_stub(int peer_fd)
             buf = malloc(op.sec_rw.len);
             r = write_blocking(peer_fd, reply.buf, OSD_PACKET_SIZE);
             if (r == OSD_PACKET_SIZE)
-                r = write_blocking(peer_fd, &buf, op.sec_rw.len);
+                r = write_blocking(peer_fd, buf, op.sec_rw.len);
             free(buf);
             if (r < op.sec_rw.len)
                 break;

src/stub_uring_osd.cpp

@@ -39,6 +39,8 @@ int main(int narg, char *args[])
     msgr->ringloop = ringloop;
     msgr->repeer_pgs = [](osd_num_t) {};
     msgr->exec_op = [msgr](osd_op_t *op) { stub_exec_op(msgr, op); };
+    json11::Json config = json11::Json::object { { "log_level", 1 } };
+    msgr->parse_config(config);
     // Accept new connections
     int listen_fd = bind_stub("0.0.0.0", 11203);
     epmgr->set_fd_handler(listen_fd, false, [listen_fd, msgr](int fd, int events)