improve error handling of debug commands

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>

c_src/cmd_debug.c

@@ -38,16 +38,21 @@ void write_field(void *base, u64 size, u64 offset, u64 value)
 
 int cmd_dump_bkey(struct bch_fs *c, enum btree_id id, struct bpos pos)
 {
-	struct printbuf buf = PRINTBUF;
 	struct btree_trans *trans = bch2_trans_get(c);
 	struct btree_iter iter = { NULL };
+	struct printbuf buf = PRINTBUF;
 	int ret = 0;
 
 	bch2_trans_iter_init(trans, &iter, id, pos, BTREE_ITER_ALL_SNAPSHOTS);
 
 	struct bkey_s_c k = bch2_btree_iter_peek(&iter);
-	if (!bpos_eq(pos, k.k->p)) {
+	if ((ret = bkey_err(k))) {
-		printf("no such key\n");
+		fprintf(stderr, "bch2_btree_iter_peek() failed: %s\n", bch2_err_str(ret));
+		goto out;
+	}
+	if (!k.k || !bpos_eq(pos, k.k->p)) {
+		bch2_bpos_to_text(&buf, pos);
+		printf("no key at pos %s\n", buf.buf);
 		ret = 1;
 		goto out;
 	}
@@ -66,44 +71,67 @@ int cmd_update_bkey(struct bch_fs *c, struct bkey_update u, struct bpos pos)
 {
 	struct btree_trans *trans = bch2_trans_get(c);
 	struct btree_iter iter = { NULL };
+	struct printbuf buf = PRINTBUF;
 	int ret = 0;
 
 	set_bit(BCH_FS_no_invalid_checks, &c->flags);
 
-	if (!strcmp(u.bkey, "bch_inode_unpacked")) {
+	bch2_trans_iter_init(trans, &iter, u.id, pos, BTREE_ITER_ALL_SNAPSHOTS);
-		bch2_trans_iter_init(trans, &iter, BTREE_ID_inodes, pos,
-				     BTREE_ITER_ALL_SNAPSHOTS);
 
-		struct bkey_s_c k = bch2_btree_iter_peek(&iter);
+	struct bkey_s_c k = bch2_btree_iter_peek(&iter);
-		if (bkey_err(k)) {
+	if ((ret = bkey_err(k))) {
-			// TODO: is this proper error handling?
+		fprintf(stderr, "bch2_btree_iter_peek() failed: %s\n", bch2_err_str(ret));
-			printf("error getting key: %s\n", bch2_err_str(PTR_ERR(k.k)));
+		goto out;
-			ret = 1;
+	}
+	if (!k.k || !bpos_eq(pos, k.k->p)) {
+		bch2_bpos_to_text(&buf, pos);
+		printf("no key at pos %s\n", buf.buf);
+		ret = 1;
+		goto out;
+	}
+
+	if (u.inode_bkey) {
+		if (k.k->type != KEY_TYPE_inode_v2 && k.k->type != KEY_TYPE_inode_v3) {
+			fprintf(stderr, "Wanted bch_inode_unpacked, got 'bch_%s'\n",
+				bch2_bkey_types[k.k->type]);
 			goto out;
 		}
+	} else if (u.bkey != k.k->type) {
+		fprintf(stderr, "Wanted type 'bch_%s', got type 'bch_%s'\n",
+			bch2_bkey_types[u.bkey], bch2_bkey_types[k.k->type]);
+		goto out;
+	}
+
+	if (u.inode_bkey) {
 		struct bch_inode_unpacked inode;
-		// TODO: check error
+		ret = bch2_inode_unpack(k, &inode);
-		bch2_inode_unpack(k, &inode);
+		if (ret != 0) {
+			fprintf(stderr, "bch2_inode_unpack() failed: %s\n", bch2_err_str(ret));
+			goto out;
+		}
 
 		write_field(&inode, u.size, u.offset, u.value);
 
 		ret = bch2_inode_write(trans, &iter, &inode) ?:
-			  bch2_trans_commit(trans, NULL, NULL, BCH_TRANS_COMMIT_no_invalid_checks);
+		      bch2_trans_commit(trans, NULL, NULL, 0);
 		if (ret != 0) {
-			printf("ret = %s (%d)\n", bch2_err_str(ret), ret);
+			fprintf(stderr, "inode update failed: %s\n", bch2_err_str(ret));
 		}
 	} else {
-		// TODO can this return an error?
-		struct bkey_i *k = bch2_bkey_get_mut(trans, &iter, u.id, pos,
-						     BTREE_ITER_ALL_SNAPSHOTS);
-		// TODO: check bkey type to confirm it matches specified type?
 		bch2_trans_unlock(trans);
 
-		write_field(&k->v, u.size, u.offset, u.value);
+		struct bkey_i *n = bch2_bkey_make_mut_noupdate(trans, k);
+		if ((ret = PTR_ERR_OR_ZERO(n))) {
+			fprintf(stderr, "bch2_bkey_make_mut_noupdate() failed: %s\n",
+				bch2_err_str(ret));
+			goto out;
+		}
 
-		ret = bch2_btree_insert(c, u.id, k, NULL, BCH_TRANS_COMMIT_no_invalid_checks);
+		write_field(&n->v, u.size, u.offset, u.value);
+
+		ret = bch2_btree_insert(c, u.id, n, NULL, 0);
 		if (ret != 0) {
-			printf("ret = %s (%d)\n", bch2_err_str(ret), ret);
+			fprintf(stderr, "bch2_btree_insert() failed: %s\n", bch2_err_str(ret));
 		}
 	}
 

c_src/cmds.h

@@ -11,7 +11,8 @@
 
 struct bkey_update {
 	enum btree_id id;
-	const char *bkey;
+	enum bch_bkey_type bkey;
+	bool inode_bkey;
 	u64 offset;
 	u64 size;
 	u64 value;

src/commands/debug/bkey_types.rs

@@ -331,5 +331,9 @@ pub fn get_bkey_type_info() -> BkeyTypes {
     let mut struct_list = BkeyTypes::new();
     process_file(&object, &mut struct_list).unwrap();
 
+    if struct_list.0.len() == 0 {
+        eprintln!("Warning: could not find bkey debug info.");
+        eprintln!("Was the bcachefs binary compiled with debug info?");
+    }
     struct_list
 }

src/commands/debug/mod.rs

@@ -1,5 +1,4 @@
 use clap::Parser;
-use std::ffi::{c_char, CString};
 use std::io::{BufRead, Write};
 
 use bch_bindgen::bcachefs;
@@ -43,15 +42,19 @@ struct UpdateCommand {
 }
 
 fn update(fs: &Fs, type_list: &bkey_types::BkeyTypes, cmd: UpdateCommand) {
-    let bkey = CString::new(cmd.bkey.clone()).unwrap();
-    let bkey = bkey.as_ptr() as *const c_char;
-
     let id: bch_bindgen::c::btree_id = cmd.btree.parse().expect("no such btree");
 
+    let (bkey, inode_bkey) = if cmd.bkey == "bch_inode_unpacked" {
+        (c::bch_bkey_type::KEY_TYPE_MAX, true)
+    } else {
+        (cmd.bkey["bch_".len()..].parse().expect("no such bkey"), false)
+    };
+
     if let Some((size, offset)) = type_list.get_member_layout(&cmd.bkey, &cmd.field) {
         let update = c::bkey_update {
             id,
             bkey,
+            inode_bkey,
             offset,
             size,
             value: cmd.value,