diff mbox series

[2/2] hw/block/nvme: add 'nvme_ana_inject_state' HMP command

Message ID 20210214112400.26956-3-minwoo.im.dev@gmail.com (mailing list archive)
State New, archived
Headers show
Series hw/block/nvme: support ANA | expand

Commit Message

Minwoo Im Feb. 14, 2021, 11:24 a.m. UTC
Human Monitor Interface(HMP) is there for easy human debugging.  This
patch added a HMP command 'nvme_ana_inject_state'.  This can be executed
from the QEMU monitor.  This command will have the following syntax:

	# nvme_ana_inject_state <nvme-id> <anagrpid> <state>
	(qemu) nvme_ana_inject_state nvme0 1 inaccessible

The example above will make ANA group #1 transitioned to
ANA_INACCESSIBLE state for `nvme0` controller device.  Additionally,
device will notify to the host that ANA has been changed via
Asynchronous Event Notifier(AEN).  Then the host will figure out another
path to I/O for the namespace by reading the log page for ANA
information again, and this is what we call the multipath I/O.

This feature is good to debug the host multipath I/O by controlling the
device ANA group state transition.  The path-related errors can be
tested and debugged by this feature.  Also, the HMP command interafce
will make us not to build QEMU itself again to control things in device.

This interface supports Persistent Loss state transition, but it's not
going to be persistent: volatile of qemu perspective.

Cc: Dr . David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 hmp-commands.hx       | 13 ++++++
 hw/block/nvme.c       | 93 +++++++++++++++++++++++++++++++++++++++++++
 hw/block/nvme.h       |  1 +
 include/block/nvme.h  |  1 +
 include/monitor/hmp.h |  1 +
 5 files changed, 109 insertions(+)

Comments

Minwoo Im Feb. 26, 2021, 1:16 p.m. UTC | #1
On 21-02-14 20:24:00, Minwoo Im wrote:
> Human Monitor Interface(HMP) is there for easy human debugging.  This
> patch added a HMP command 'nvme_ana_inject_state'.  This can be executed
> from the QEMU monitor.  This command will have the following syntax:
> 
> 	# nvme_ana_inject_state <nvme-id> <anagrpid> <state>
> 	(qemu) nvme_ana_inject_state nvme0 1 inaccessible
> 
> The example above will make ANA group #1 transitioned to
> ANA_INACCESSIBLE state for `nvme0` controller device.  Additionally,
> device will notify to the host that ANA has been changed via
> Asynchronous Event Notifier(AEN).  Then the host will figure out another
> path to I/O for the namespace by reading the log page for ANA
> information again, and this is what we call the multipath I/O.
> 
> This feature is good to debug the host multipath I/O by controlling the
> device ANA group state transition.  The path-related errors can be
> tested and debugged by this feature.  Also, the HMP command interafce
> will make us not to build QEMU itself again to control things in device.
> 
> This interface supports Persistent Loss state transition, but it's not
> going to be persistent: volatile of qemu perspective.
> 
> Cc: Dr . David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>

Hello Keith,

Do you have any comments about this injection method?  As discussed
ealier, I've tried to introduce a natural way to control some of device
status like ANA state which device can behave properly.

It would be great if I can have your feedback on this :)

Thanks!
diff mbox series

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index d4001f9c5dc6..5a099191429d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1307,6 +1307,19 @@  SRST
   Inject PCIe AER error
 ERST
 
+    {
+        .name       = "nvme_ana_inject_state",
+        .args_type  = "id:s,grpid:i,state:s",
+        .params     = "id grpid [optimized|non-optimized|inaccessible|change]",
+        .help       = "inject ANA state",
+        .cmd        = hmp_nvme_ana_inject_state,
+    },
+
+SRST
+``nvme_ana_inject_state``
+  Inject ANA state to NVMe subsystem
+ERST
+
     {
         .name       = "netdev_add",
         .args_type  = "netdev:O",
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 8f5c2c1ab7f7..b40fd3230b8d 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -131,6 +131,8 @@ 
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "qemu/cutils.h"
+#include "qapi/qmp/qdict.h"
+#include "monitor/monitor.h"
 #include "trace.h"
 #include "nvme.h"
 #include "nvme-ns.h"
@@ -229,6 +231,46 @@  static uint16_t nvme_sqid(NvmeRequest *req)
     return le16_to_cpu(req->sq->sqid);
 }
 
+static void nvme_notice_event(NvmeCtrl *n, uint8_t event_info);
+static bool nvme_ana_state_change(NvmeCtrl *n, uint32_t grpid, uint8_t state)
+{
+    uint8_t old;
+
+    old = n->ana[grpid].state;
+
+    if (state == old) {
+        return false;
+    }
+
+    n->ana[grpid].state = state;
+    n->ana_change_count++;
+    nvme_notice_event(n, NVME_AER_INFO_ANA_CHANGE);
+
+    return true;
+}
+
+static const char *nvme_ana_states[] = {
+    [NVME_ANA_STATE_OPTIMIZED]          = "optimized",
+    [NVME_ANA_STATE_NON_OPTIMIZED]      = "non-optimized",
+    [NVME_ANA_STATE_INACCESSIBLE]       = "inaccessible",
+    [NVME_ANA_STATE_PERSISTENT_LOSS]    = "persistent-loss",
+    [NVME_ANA_STATE_CHANGE]             = "change",
+};
+
+static inline bool nvme_ana_state_valid(uint8_t state)
+{
+    switch (state) {
+    case NVME_ANA_STATE_OPTIMIZED:
+    case NVME_ANA_STATE_NON_OPTIMIZED:
+    case NVME_ANA_STATE_INACCESSIBLE:
+    case NVME_ANA_STATE_PERSISTENT_LOSS:
+    case NVME_ANA_STATE_CHANGE:
+        return true;
+    default:
+        return false;
+    }
+}
+
 static inline uint16_t nvme_ana_check_state(uint8_t state)
 {
     switch (state) {
@@ -243,6 +285,42 @@  static inline uint16_t nvme_ana_check_state(uint8_t state)
     }
 }
 
+void hmp_nvme_ana_inject_state(Monitor *mon, const QDict *qdict)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    const uint32_t grpid = qdict_get_int(qdict, "grpid");
+    const char *state = qdict_get_str(qdict, "state");
+    NvmeCtrl *n;
+    DeviceState *dev;
+    int i;
+
+    dev = qdev_find_recursive(sysbus_get_default(), id);
+    if (!dev) {
+        monitor_printf(mon, "%s: invalid device id\n", id);
+        return;
+    }
+
+    if (!grpid) {
+        monitor_printf(mon, "%s: grpid should not be 0\n", id);
+        return;
+    }
+
+    n = NVME(dev);
+
+    for (i = 0; i < ARRAY_SIZE(nvme_ana_states); i++) {
+        if (nvme_ana_state_valid(i) &&
+                !strcmp(nvme_ana_states[i], state)) {
+            if (nvme_ana_state_change(n, grpid, i)) {
+                monitor_printf(mon, "%s: ANA state %s(%d) injected\n",
+                               id, state, i);
+            }
+            return;
+        }
+    }
+
+    monitor_printf(mon, "%s: invalid state %s\n", id, state);
+}
+
 static void nvme_assign_zone_state(NvmeNamespace *ns, NvmeZone *zone,
                                    NvmeZoneState state)
 {
@@ -1080,6 +1158,21 @@  static void nvme_smart_event(NvmeCtrl *n, uint8_t event)
     nvme_enqueue_event(n, NVME_AER_TYPE_SMART, aer_info, NVME_LOG_SMART_INFO);
 }
 
+static void nvme_notice_event(NvmeCtrl *n, uint8_t event_info)
+{
+    uint8_t log_page;
+
+    switch (event_info) {
+    case NVME_AER_INFO_ANA_CHANGE:
+        log_page = NVME_LOG_ANA;
+        break;
+    default:
+        return;
+    }
+
+    nvme_enqueue_event(n, NVME_AER_TYPE_NOTICE, event_info, log_page);
+}
+
 static void nvme_clear_events(NvmeCtrl *n, uint8_t event_type)
 {
     n->aer_mask &= ~(1 << event_type);
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 842245d0a845..1a1a022dad0a 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -234,5 +234,6 @@  static inline long nvme_ana_nr_ns(NvmeAna *ana)
 }
 
 int nvme_register_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp);
+void hmp_nvme_ana_inject_state(Monitor *mon, const QDict *qdict);
 
 #endif /* HW_NVME_H */
diff --git a/include/block/nvme.h b/include/block/nvme.h
index c51fbf845f5a..b27dd8eab7e9 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -771,6 +771,7 @@  enum NvmeAsyncEventRequest {
     NVME_AER_INFO_SMART_RELIABILITY         = 0,
     NVME_AER_INFO_SMART_TEMP_THRESH         = 1,
     NVME_AER_INFO_SMART_SPARE_THRESH        = 2,
+    NVME_AER_INFO_ANA_CHANGE                = 3,
 };
 
 typedef struct QEMU_PACKED NvmeAerResult {
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index ed2913fd18e8..cbeaf7935900 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -133,5 +133,6 @@  void hmp_info_replay(Monitor *mon, const QDict *qdict);
 void hmp_replay_break(Monitor *mon, const QDict *qdict);
 void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
 void hmp_replay_seek(Monitor *mon, const QDict *qdict);
+void hmp_nvme_ana_inject_state(Monitor *mon, const QDict *qdict);
 
 #endif