ublksrv: add one API for checking if the control device is for recove…

…ring We know if the created control device instance is for recovering or other purpose, so add one API for checking it instead of using device state. This way is more reliable, since DEAD device may appear in recovering code path. Fix one segfault issue reported by Changhui: https://lore.kernel.org/linux-block/CAGVVp+XD5MbYOWL4pbLMxXL0yNKO5NJ84--=KVnW6w5-GF7Drw@mail.gmail.com/ Signed-off-by: Ming Lei <[email protected]>
ublk-org · Jun 5, 2024 · 30f52dc · 30f52dc
1 parent c54760f
commit 30f52dc
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 5 deletions.
diff --git a/include/ublksrv.h b/include/ublksrv.h
@@ -539,6 +539,13 @@ extern void ublksrv_ctrl_prep_recovery(struct ublksrv_ctrl_dev *dev,
  */
 extern const char *ublksrv_ctrl_get_recovery_jbuf(const struct ublksrv_ctrl_dev *dev);
 
+/**
+ * Return true if this control device is for recovering
+ *
+ * @param dev the ublksrv control device instance
+ */
+extern bool ublksrv_is_recovering(const struct ublksrv_ctrl_dev *ctrl_dev);
+
 /** @} */ // end of ctrl_dev group
 
 

diff --git a/include/ublksrv_priv.h b/include/ublksrv_priv.h
@@ -55,8 +55,11 @@ struct ublksrv_ctrl_dev {
 			int tgt_argc;
 			char **tgt_argv;
 		};
-		/* used by ->recovery_tgt() */
-		const char *recovery_jbuf;
+		/* used by ->recovery_tgt(), tgt_argc == -1 */
+		struct {
+			int padding;
+			const char *recovery_jbuf;
+		};
 	};
 
 	cpu_set_t *queues_cpuset;

diff --git a/lib/ublksrv.c b/lib/ublksrv.c
@@ -8,6 +8,11 @@
 #include "ublksrv_priv.h"
 #include "ublksrv_aio.h"
 
+bool ublksrv_is_recovering(const struct ublksrv_ctrl_dev *ctrl_dev)
+{
+	return ctrl_dev->tgt_argc == -1;
+}
+
 static inline struct ublksrv_io_desc *ublksrv_get_iod(
 		const struct _ublksrv_queue *q, int tag)
 {
@@ -74,7 +79,7 @@ static int __ublksrv_tgt_init(struct _ublksrv_dev *dev, const char *type_name,
 	optind = 0;     /* so that we can parse our arguments */
 	tgt->ops = ops;
 
-	if (dev->ctrl_dev->dev_info.state != UBLK_S_DEV_QUIESCED)
+	if (!ublksrv_is_recovering(dev->ctrl_dev))
 		ret = ops->init_tgt(local_to_tdev(dev), type, argc, argv);
 	else {
 		if (ops->recovery_tgt)

diff --git a/lib/ublksrv_cmd.c b/lib/ublksrv_cmd.c
@@ -589,6 +589,7 @@ void ublksrv_ctrl_prep_recovery(struct ublksrv_ctrl_dev *dev,
 {
 	dev->tgt_type = tgt_type;
 	dev->tgt_ops = tgt_ops;
+	dev->tgt_argc = -1;
 	dev->recovery_jbuf = recovery_jbuf;
 }
 

diff --git a/ublksrv_tgt.cpp b/ublksrv_tgt.cpp
@@ -343,11 +343,10 @@ static void *ublksrv_io_handler_fn(void *data)
 	int buf_size;
 	char *buf;
 	const char *jbuf;
-	bool recoverying = dinfo->state == UBLK_S_DEV_QUIESCED;
 
 	pthread_mutex_lock(&jbuf_lock);
 
-	if (!recoverying) {
+	if (!ublksrv_is_recovering(cdev)) {
 		do {
 			buf = __ublksrv_tgt_realloc_json_buf(dev, &buf_size);
 			ret = ublksrv_json_write_queue_info(cdev, buf, buf_size,