redfish: Be more robust by retrying IPMI transactions

This fixes creating users when the BMC is otherwise busy.

plugins/redfish/fu-ipmi-device.c

@@ -22,6 +22,9 @@
 
 #define FU_IPMI_DEVICE_TIMEOUT 1500 /* ms */
 
+#define FU_IPMI_TRANSACTION_RETRY_COUNT 5
+#define FU_IPMI_TRANSACTION_RETRY_DELAY 200 /* ms */
+
 /* not defined in linux/ipmi_msgdefs.h */
 #define IPMI_SET_USER_ACCESS   0x43
 #define IPMI_SET_USER_NAME     0x45
@@ -234,19 +237,51 @@ fu_ipmi_device_errcode_to_string(guint8 errcode)
 }
 
 static gboolean
-fu_ipmi_device_transaction(FuIpmiDevice *self,
+fu_ipmi_device_errcode_to_error(guint8 errcode, GError **error)
-			   guint8 netfn,
-			   guint8 cmd,
-			   const guint8 *req_buf,
-			   gsize req_bufsz,
-			   guint8 *resp_buf, /* optional */
-			   gsize resp_bufsz,
-			   gsize *resp_len, /* optional, out */
-			   gint timeout_ms,
-			   GError **error)
 {
+	/* success */
+	if (errcode == IPMI_CC_NO_ERROR)
+		return TRUE;
+
+	/* data not found, seemingly Lenovo specific */
+	if (errcode == IPMI_INVALID_DATA_FIELD_ERR || errcode == IPMI_NOT_FOUND_ERR) {
+		g_set_error(error,
+			    G_IO_ERROR,
+			    G_IO_ERROR_NOT_FOUND,
+			    "CC error: %s [0x%02X]",
+			    fu_ipmi_device_errcode_to_string(errcode),
+			    errcode);
+		return FALSE;
+	}
+
+	/* fallback */
+	g_set_error(error,
+		    G_IO_ERROR,
+		    G_IO_ERROR_FAILED,
+		    "CC error: %s [0x%02X]",
+		    fu_ipmi_device_errcode_to_string(errcode),
+		    errcode);
+	return FALSE;
+}
+
+typedef struct {
+	guint8 netfn;
+	guint8 cmd;
+	const guint8 *req_buf;
+	gsize req_bufsz;
+	guint8 *resp_buf;
+	gsize resp_bufsz;
+	gsize *resp_len;
+	gint timeout_ms;
+} FuIpmiDeviceTransactionHelper;
+
+static gboolean
+fu_ipmi_device_transaction_cb(FuDevice *device, gpointer user_data, GError **error)
+{
+	FuIpmiDevice *self = FU_IPMI_DEVICE(device);
+	FuIpmiDeviceTransactionHelper *helper = (FuIpmiDeviceTransactionHelper *)user_data;
 	GPollFD pollfds[1];
-	gsize resp_buf2sz = resp_bufsz + 1;
+	gsize resp_buf2sz = helper->resp_bufsz + 1;
 	gsize resp_len2 = 0;
 	g_autoptr(GTimer) timer = g_timer_new();
 	g_autoptr(FuDeviceLocker) lock = NULL;
@@ -256,7 +291,12 @@ fu_ipmi_device_transaction(FuIpmiDevice *self,
 	if (lock == NULL)
 		return FALSE;
 
-	if (!fu_ipmi_device_send(self, netfn, cmd, req_buf, req_bufsz, error))
+	if (!fu_ipmi_device_send(self,
+				 helper->netfn,
+				 helper->cmd,
+				 helper->req_buf,
+				 helper->req_bufsz,
+				 error))
 		return FALSE;
 
 	pollfds[0].fd = fu_udev_device_get_fd(FU_UDEV_DEVICE(self));
@@ -268,7 +308,9 @@ fu_ipmi_device_transaction(FuIpmiDevice *self,
 		glong seq = 0;
 		gint rc;
 
-		rc = g_poll(pollfds, 1, timeout_ms - (g_timer_elapsed(timer, NULL) * 1000.f));
+		rc = g_poll(pollfds,
+			    1,
+			    helper->timeout_ms - (g_timer_elapsed(timer, NULL) * 1000.f));
 		if (rc < 0) {
 			g_set_error(error, G_IO_ERROR, G_IO_ERROR_FAILED, "poll() error %m");
 			return FALSE;
@@ -279,8 +321,8 @@ fu_ipmi_device_transaction(FuIpmiDevice *self,
 				    G_IO_ERROR_FAILED,
 				    "timeout waiting for response "
 				    "(netfn %d, cmd %d)",
-				    netfn,
+				    helper->netfn,
-				    cmd);
+				    helper->cmd);
 			return FALSE;
 		}
 
@@ -307,7 +349,7 @@ fu_ipmi_device_transaction(FuIpmiDevice *self,
 				"expected %ld, got %ld",
 				self->seq,
 				seq);
-			if (g_timer_elapsed(timer, NULL) * 1000.f >= timeout_ms) {
+			if (g_timer_elapsed(timer, NULL) * 1000.f >= helper->timeout_ms) {
 				g_set_error_literal(error,
 						    G_IO_ERROR,
 						    G_IO_ERROR_FAILED,
@@ -315,33 +357,26 @@ fu_ipmi_device_transaction(FuIpmiDevice *self,
 				return FALSE;
 			}
 		} else {
-			if (resp_buf2[0] != IPMI_CC_NO_ERROR) {
+			if (!fu_ipmi_device_errcode_to_error(resp_buf2[0], error))
-				g_set_error(error,
-					    G_IO_ERROR,
-					    G_IO_ERROR_FAILED,
-					    "CC error: %s [%02x]",
-					    fu_ipmi_device_errcode_to_string(resp_buf2[0]),
-					    resp_buf2[0]);
 				return FALSE;
-			}
+			if (helper->resp_buf != NULL) {
-			if (resp_buf != NULL) {
+				if (!fu_memcpy_safe(helper->resp_buf,
-				if (!fu_memcpy_safe(resp_buf,
+						    helper->resp_bufsz,
-						    resp_bufsz,
 						    0x0, /* dst */
 						    resp_buf2,
 						    resp_buf2sz,
 						    0x01, /* src */
-						    resp_bufsz,
+						    helper->resp_bufsz,
 						    error))
 					return FALSE;
 			}
-			if (resp_len != NULL)
+			if (helper->resp_len != NULL)
-				*resp_len = resp_len2 - 1;
+				*helper->resp_len = resp_len2 - 1;
 			if (g_getenv("FWUPD_REDFISH_VERBOSE") != NULL) {
 				g_debug("IPMI netfn: %02x->%02x, cmd: %02x->%02x",
-					netfn,
+					helper->netfn,
 					resp_netfn,
-					cmd,
+					helper->cmd,
 					resp_cmd);
 			}
 			break;
@@ -350,6 +385,37 @@ fu_ipmi_device_transaction(FuIpmiDevice *self,
 	return TRUE;
 }
 
+static gboolean
+fu_ipmi_device_transaction(FuIpmiDevice *self,
+			   guint8 netfn,
+			   guint8 cmd,
+			   const guint8 *req_buf,
+			   gsize req_bufsz,
+			   guint8 *resp_buf, /* optional */
+			   gsize resp_bufsz,
+			   gsize *resp_len, /* optional, out */
+			   gint timeout_ms,
+			   GError **error)
+{
+	FuIpmiDeviceTransactionHelper helper = {
+	    .netfn = netfn,
+	    .cmd = cmd,
+	    .req_buf = req_buf,
+	    .req_bufsz = req_bufsz,
+	    .resp_buf = resp_buf,
+	    .resp_bufsz = resp_bufsz,
+	    .resp_len = resp_len,
+	    .timeout_ms = timeout_ms,
+	};
+	fu_device_retry_add_recovery(FU_DEVICE(self), G_IO_ERROR, G_IO_ERROR_NOT_FOUND, NULL);
+	return fu_device_retry_full(FU_DEVICE(self),
+				    fu_ipmi_device_transaction_cb,
+				    FU_IPMI_TRANSACTION_RETRY_COUNT,
+				    FU_IPMI_TRANSACTION_RETRY_DELAY,
+				    &helper,
+				    error);
+}
+
 static gboolean
 fu_ipmi_device_probe(FuDevice *device, GError **error)
 {