[PATCH 0/5] iscsi: fix hang after abrupt target disconnect

[PATCH 0/5] iscsi: fix hang after abrupt target disconnect

[Konstantin Nigmatullin]

When an iSCSI target disappears (for example tgtd is killed), QEMU can
leave guest I/O stuck and then fail to exit on SIGINT because
virtio_scsi_dataplane_stop blocks forever in blk_drain_all().

This is a stack of problems in block/iscsi.c, not only the documented
default command timeout of 0:

1. NOP dead-session detection never fired in practice because
   iscsi_get_nops_in_flight() stayed at 0 on a dead TCP session.
2. SCSI_STATUS_TIMEOUT was retried like BUSY (up to seven full
   timeouts), so drain stayed busy for minutes even with -iscsi
   timeout=N.
3. In-flight coroutines were not cancelled locally, so with timeout=0
   waiters never woke.
4. After one failure, queued I/O restarted another long wait, so drain
   never stayed idle (permanent hang).

This series addresses those without changing the default timeout value.

Verified with the GitLab #3067 recipe (default timeout=0). After killing
tgtd and unmounting, the guest correctly sees I/O errors, and Ctrl-C
makes QEMU exit cleanly instead of hanging in blk_drain_all:

  (initramfs) umount /mnt
  qemu-system-x86_64: terminating on signal 2
  qemu-system-x86_64: iSCSI: NOP timeout. Reconnecting...
  qemu-system-x86_64: iSCSI SYNCHRONIZECACHE10 failed: SENSE KEY:UNIT_ATTENTION(6) ASCQ:BUS_RESET(0x2900)

Residual host messages such as NOP timeout / SYNCHRONIZECACHE10
UNIT_ATTENTION(BUS_RESET) are expected while in-flight flush fails on a
dead session during reconnect; they are cosmetic and not a correctness
issue for this series.

Buglink: https://gitlab.com/qemu-project/qemu/-/issues/3067

Konstantin Nigmatullin (5):
  iscsi: detect dead iSCSI sessions via local NOP failure counting
  iscsi: stop retrying timed-out iSCSI commands
  iscsi: cancel in-flight iSCSI tasks when the session dies
  iscsi: fail new iSCSI I/O while the transport is known down
  iscsi: avoid null iSCSI error strings after cancel or timeout

 block/iscsi.c | 124 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 113 insertions(+), 11 deletions(-)

-- 
2.53.0

[PATCH 1/5] iscsi: detect dead iSCSI sessions via local NOP failure counting

[Konstantin Nigmatullin]

libiscsi nops_in_flight stayed at 0 on a dead TCP session, so QEMU never
noticed the target had vanished when command timeout was disabled.

See: https://gitlab.com/qemu-project/qemu/-/work_items/3067
Signed-off-by: Konstantin Nigmatullin <rangolit@gmail.com>
---
 block/iscsi.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 7d6bf185ea..bf74b4cb65 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -103,6 +103,8 @@ typedef struct IscsiLun {
     bool dpofua;
     bool has_write_same;
     bool request_timed_out;
+    /* Consecutive NOP-Outs without a successful NOP-In reply. */
+    int nop_failures;
 } IscsiLun;
 
 typedef struct IscsiTask {
@@ -1398,17 +1400,37 @@ static char *get_initiator_name(QemuOpts *opts)
     return iscsi_name;
 }
 
+static void iscsi_nop_cb(struct iscsi_context *iscsi, int status,
+                         void *command_data, void *opaque)
+{
+    IscsiLun *iscsilun = opaque;
+
+    if (status == SCSI_STATUS_GOOD) {
+        iscsilun->nop_failures = 0;
+    }
+}
+
 static void iscsi_nop_timed_event(void *opaque)
 {
     IscsiLun *iscsilun = opaque;
 
     QEMU_LOCK_GUARD(&iscsilun->mutex);
-    if (iscsi_get_nops_in_flight(iscsilun->iscsi) >= MAX_NOP_FAILURES) {
+    /*
+     * Prefer libiscsi's counter when it works; also track locally because
+     * some libiscsi builds leave nops_in_flight at 0 across a dead TCP
+     * session, which disabled disconnect detection entirely.
+     */
+    if (iscsilun->nop_failures >= MAX_NOP_FAILURES ||
+        iscsi_get_nops_in_flight(iscsilun->iscsi) >= MAX_NOP_FAILURES) {
         error_report("iSCSI: NOP timeout. Reconnecting...");
         iscsilun->request_timed_out = true;
-    } else if (iscsi_nop_out_async(iscsilun->iscsi, NULL, NULL, 0, NULL) != 0) {
+        iscsilun->nop_failures = 0;
+    } else if (iscsi_nop_out_async(iscsilun->iscsi, iscsi_nop_cb, NULL, 0,
+                                   iscsilun) != 0) {
         error_report("iSCSI: failed to sent NOP-Out. Disabling NOP messages.");
         return;
+    } else {
+        iscsilun->nop_failures++;
     }
 
     timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL);
-- 
2.53.0

[PATCH 2/5] iscsi: stop retrying timed-out iSCSI commands

[Konstantin Nigmatullin]

Retrying SCSI_STATUS_TIMEOUT up to seven times kept drain and shutdown
blocked for minutes after the target was already gone.

See: https://gitlab.com/qemu-project/qemu/-/work_items/3067
Signed-off-by: Konstantin Nigmatullin <rangolit@gmail.com>
---
 block/iscsi.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index bf74b4cb65..e7f2d54986 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -240,19 +240,21 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
 
     if (status != SCSI_STATUS_GOOD) {
         iTask->err_code = -EIO;
-        if (iTask->retries++ < ISCSI_CMD_RETRIES) {
+        if (status == SCSI_STATUS_TIMEOUT) {
+            /*
+             * Do not retry timed-out commands. Retries keep in-flight I/O
+             * alive across a dead session and can block blk_drain_all()
+             * (and thus QEMU exit) for minutes or indefinitely. Kick
+             * reconnect and fail this request.
+             */
+            error_report("iSCSI timed out: %s", iscsi_get_error(iscsi));
+            iscsilun->request_timed_out = true;
+        } else if (iTask->retries++ < ISCSI_CMD_RETRIES) {
             if (status == SCSI_STATUS_BUSY ||
-                status == SCSI_STATUS_TIMEOUT ||
                 status == SCSI_STATUS_TASK_SET_FULL) {
                 unsigned retry_time =
                     exp_random(iscsi_retry_times[iTask->retries - 1]);
-                if (status == SCSI_STATUS_TIMEOUT) {
-                    /* make sure the request is rescheduled AFTER the
-                     * reconnect is initiated */
-                    retry_time = EVENT_INTERVAL * 2;
-                    iTask->iscsilun->request_timed_out = true;
-                }
-                error_report("iSCSI Busy/TaskSetFull/TimeOut"
+                error_report("iSCSI Busy/TaskSetFull"
                              " (retry #%u in %u ms): %s",
                              iTask->retries, retry_time,
                              iscsi_get_error(iscsi));
-- 
2.53.0

[PATCH 3/5] iscsi: cancel in-flight iSCSI tasks when the session dies

[Konstantin Nigmatullin]

Coroutines waiting on a dead target never woke, so blk_drain_all could
not finish and QEMU would not exit.

See: https://gitlab.com/qemu-project/qemu/-/work_items/3067
Signed-off-by: Konstantin Nigmatullin <rangolit@gmail.com>
---
 block/iscsi.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/block/iscsi.c b/block/iscsi.c
index e7f2d54986..330f17e08b 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -381,6 +381,12 @@ static void iscsi_timed_check_events(void *opaque)
 
         if (iscsilun->request_timed_out) {
             iscsilun->request_timed_out = false;
+            /*
+             * Drop local in-flight tasks so coroutines waiting on a dead
+             * session wake with CANCELLED instead of blocking drain forever
+             * (especially when command timeout is disabled).
+             */
+            iscsi_scsi_cancel_all_tasks(iscsilun->iscsi);
             iscsi_reconnect(iscsilun->iscsi);
         }
 
@@ -1427,6 +1433,7 @@ static void iscsi_nop_timed_event(void *opaque)
         error_report("iSCSI: NOP timeout. Reconnecting...");
         iscsilun->request_timed_out = true;
         iscsilun->nop_failures = 0;
+        iscsi_scsi_cancel_all_tasks(iscsilun->iscsi);
     } else if (iscsi_nop_out_async(iscsilun->iscsi, iscsi_nop_cb, NULL, 0,
                                    iscsilun) != 0) {
         error_report("iSCSI: failed to sent NOP-Out. Disabling NOP messages.");
-- 
2.53.0

[PATCH 4/5] iscsi: fail new iSCSI I/O while the transport is known down

[Konstantin Nigmatullin]

After the first disconnect failure, queued requests kept restarting long
waits so blk_drain_all never stayed idle and QEMU could not exit.

See: https://gitlab.com/qemu-project/qemu/-/work_items/3067
Signed-off-by: Konstantin Nigmatullin <rangolit@gmail.com>
---
 block/iscsi.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 330f17e08b..b6207d03e5 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -103,6 +103,12 @@ typedef struct IscsiLun {
     bool dpofua;
     bool has_write_same;
     bool request_timed_out;
+    /*
+     * Set when the transport is known to be down (command or NOP timeout).
+     * New I/O fails immediately until libiscsi reports logged-in again after
+     * reconnect. Prevents drain/shutdown from waiting forever on a dead target.
+     */
+    bool fail_io;
     /* Consecutive NOP-Outs without a successful NOP-In reply. */
     int nop_failures;
 } IscsiLun;
@@ -245,10 +251,12 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
              * Do not retry timed-out commands. Retries keep in-flight I/O
              * alive across a dead session and can block blk_drain_all()
              * (and thus QEMU exit) for minutes or indefinitely. Kick
-             * reconnect and fail this request.
+             * reconnect and fail this request; further I/O is rejected via
+             * fail_io until the session is logged in again.
              */
             error_report("iSCSI timed out: %s", iscsi_get_error(iscsi));
             iscsilun->request_timed_out = true;
+            iscsilun->fail_io = true;
         } else if (iTask->retries++ < ISCSI_CMD_RETRIES) {
             if (status == SCSI_STATUS_BUSY ||
                 status == SCSI_STATUS_TASK_SET_FULL) {
@@ -277,6 +285,9 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
                 }
             }
         }
+    } else {
+        iscsilun->fail_io = false;
+        iscsilun->nop_failures = 0;
     }
 
     /*
@@ -379,6 +390,10 @@ static void iscsi_timed_check_events(void *opaque)
         /* check for timed out requests */
         iscsi_service(iscsilun->iscsi, 0);
 
+        if (iscsilun->fail_io && iscsi_is_logged_in(iscsilun->iscsi)) {
+            iscsilun->fail_io = false;
+        }
+
         if (iscsilun->request_timed_out) {
             iscsilun->request_timed_out = false;
             /*
@@ -604,6 +619,15 @@ static void coroutine_fn iscsi_co_wait_for_task(IscsiTask *iTask,
     qemu_mutex_lock(&iscsilun->mutex);
 }
 
+/* Called with iscsilun->mutex held. */
+static int iscsi_co_reject_if_failing(IscsiLun *iscsilun)
+{
+    if (iscsilun->fail_io) {
+        return -EIO;
+    }
+    return 0;
+}
+
 static int coroutine_fn
 iscsi_co_writev(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
                 QEMUIOVector *iov, int flags)
@@ -631,6 +655,10 @@ iscsi_co_writev(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
     iscsi_co_init_iscsitask(iscsilun, &iTask);
     qemu_mutex_lock(&iscsilun->mutex);
 retry:
+    if (iscsi_co_reject_if_failing(iscsilun)) {
+        r = -EIO;
+        goto out_unlock;
+    }
     if (iscsilun->use_16_for_rw) {
 #if LIBISCSI_API_VERSION >= (20160603)
         iTask.task = iscsi_write16_iov_task(iscsilun->iscsi, iscsilun->lun, lba,
@@ -730,6 +758,10 @@ static int coroutine_fn iscsi_co_block_status(BlockDriverState *bs,
 
     qemu_mutex_lock(&iscsilun->mutex);
 retry:
+    if (iscsi_co_reject_if_failing(iscsilun)) {
+        ret = -EIO;
+        goto out_unlock;
+    }
     if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,
                                   lba, 8 + 16, iscsi_co_generic_cb,
                                   &iTask) == NULL) {
@@ -861,6 +893,10 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
     iscsi_co_init_iscsitask(iscsilun, &iTask);
     qemu_mutex_lock(&iscsilun->mutex);
 retry:
+    if (iscsi_co_reject_if_failing(iscsilun)) {
+        qemu_mutex_unlock(&iscsilun->mutex);
+        return -EIO;
+    }
     if (iscsilun->use_16_for_rw) {
 #if LIBISCSI_API_VERSION >= (20160603)
         iTask.task = iscsi_read16_iov_task(iscsilun->iscsi, iscsilun->lun, lba,
@@ -927,6 +963,10 @@ static int coroutine_fn iscsi_co_flush(BlockDriverState *bs)
     iscsi_co_init_iscsitask(iscsilun, &iTask);
     qemu_mutex_lock(&iscsilun->mutex);
 retry:
+    if (iscsi_co_reject_if_failing(iscsilun)) {
+        qemu_mutex_unlock(&iscsilun->mutex);
+        return -EIO;
+    }
     if (iscsi_synchronizecache10_task(iscsilun->iscsi, iscsilun->lun, 0, 0, 0,
                                       0, iscsi_co_generic_cb, &iTask) == NULL) {
         qemu_mutex_unlock(&iscsilun->mutex);
@@ -1170,6 +1210,10 @@ coroutine_fn iscsi_co_pdiscard(BlockDriverState *bs, int64_t offset,
     iscsi_co_init_iscsitask(iscsilun, &iTask);
     qemu_mutex_lock(&iscsilun->mutex);
 retry:
+    if (iscsi_co_reject_if_failing(iscsilun)) {
+        r = -EIO;
+        goto out_unlock;
+    }
     if (iscsi_unmap_task(iscsilun->iscsi, iscsilun->lun, 0, 0, &list, 1,
                          iscsi_co_generic_cb, &iTask) == NULL) {
         r = -ENOMEM;
@@ -1255,6 +1299,10 @@ coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
     qemu_mutex_lock(&iscsilun->mutex);
     iscsi_co_init_iscsitask(iscsilun, &iTask);
 retry:
+    if (iscsi_co_reject_if_failing(iscsilun)) {
+        qemu_mutex_unlock(&iscsilun->mutex);
+        return -EIO;
+    }
     if (use_16_for_ws) {
         /*
          * iscsi_writesame16_task num_blocks argument is uint32_t. We rely here
@@ -1423,6 +1471,10 @@ static void iscsi_nop_timed_event(void *opaque)
     IscsiLun *iscsilun = opaque;
 
     QEMU_LOCK_GUARD(&iscsilun->mutex);
+    if (iscsilun->fail_io && iscsi_is_logged_in(iscsilun->iscsi)) {
+        iscsilun->fail_io = false;
+    }
+
     /*
      * Prefer libiscsi's counter when it works; also track locally because
      * some libiscsi builds leave nops_in_flight at 0 across a dead TCP
@@ -1432,6 +1484,7 @@ static void iscsi_nop_timed_event(void *opaque)
         iscsi_get_nops_in_flight(iscsilun->iscsi) >= MAX_NOP_FAILURES) {
         error_report("iSCSI: NOP timeout. Reconnecting...");
         iscsilun->request_timed_out = true;
+        iscsilun->fail_io = true;
         iscsilun->nop_failures = 0;
         iscsi_scsi_cancel_all_tasks(iscsilun->iscsi);
     } else if (iscsi_nop_out_async(iscsilun->iscsi, iscsi_nop_cb, NULL, 0,
@@ -2398,6 +2451,10 @@ iscsi_co_copy_range_to(BlockDriverState *bs,
     qemu_mutex_lock(&dst_lun->mutex);
     iscsi_task.task = iscsi_xcopy_task(data.size);
 retry:
+    if (iscsi_co_reject_if_failing(dst_lun)) {
+        r = -EIO;
+        goto out_unlock;
+    }
     if (iscsi_scsi_command_async(dst_lun->iscsi, dst_lun->lun,
                                  iscsi_task.task, iscsi_co_generic_cb,
                                  &data,
-- 
2.53.0

[PATCH 5/5] iscsi: avoid null iSCSI error strings after cancel or timeout

[Konstantin Nigmatullin]

Cancelled and timed-out tasks often left err_str unset, so logs printed
"(null)" instead of a useful reason.

See: https://gitlab.com/qemu-project/qemu/-/work_items/3067
Signed-off-by: Konstantin Nigmatullin <rangolit@gmail.com>
---
 block/iscsi.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/block/iscsi.c b/block/iscsi.c
index b6207d03e5..0ab12987b8 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -290,6 +290,20 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
         iscsilun->nop_failures = 0;
     }
 
+    if (status != SCSI_STATUS_GOOD && !iTask->do_retry && !iTask->err_str) {
+        const char *e = iscsi_get_error(iscsi);
+
+        if (e && e[0]) {
+            iTask->err_str = g_strdup(e);
+        } else if (status == SCSI_STATUS_CANCELLED) {
+            iTask->err_str = g_strdup("task cancelled");
+        } else if (status == SCSI_STATUS_TIMEOUT) {
+            iTask->err_str = g_strdup("command timed out");
+        } else {
+            iTask->err_str = g_strdup("I/O error");
+        }
+    }
+
     /*
      * aio_co_wake() is safe to call: iscsi_service(), which called us, is only
      * run from the event_timer and/or the FD handlers, never from the request
-- 
2.53.0