ipmicli: repeated -p/-k message, terminal detection on Java 22-24, --no-escape on a terminal, 'power on' hint, owner in sensors and fru output (#218)

src/main/java/org/metricshub/ipmi/cli/CliArguments.java

@@ -89,6 +89,8 @@ enum Subcommand {
 	/** The last request network function: the odd codes are those of the responses (IPMI 2.0 section 5.1). */
 	private static final int LAST_NETWORK_FUNCTION = 0x3e;
 	private static final char DELETE = 0x7f;
+	private static final String PASSWORD_OPTIONS = "--password and --password-file";
+	private static final String KEY_OPTIONS = "--bmc-key and --bmc-key-file";
 
 	private Subcommand subcommand;
 	private String hostname;
@@ -117,7 +119,9 @@ enum Subcommand {
 	private int escape = DEFAULT_ESCAPE;
 	private boolean escapeGiven;
 	private boolean noEscape;
+	/** The long name of the password option given so far, null for none. */
 	private String passwordOption;
+	/** The long name of the BMC key option given so far, null for none. */
 	private String keyOption;
 
 	private CliArguments() {}
@@ -186,29 +190,29 @@ private int parseOption(String[] args, int index) throws CliUsageException {
 				// Rejected: this copy is wiped too
 				Arrays.fill(value, '\0');
 			}
-			exclusive(passwordOption, "--password and --password-file");
-			passwordOption = option;
+			once(passwordOption, "--password", PASSWORD_OPTIONS);
+			passwordOption = "--password";
 			replacePassword(value);
 			return nextIndex(argument, index);
 		}
 		case "-pf":
 		case "--password-file":
-			exclusive(passwordOption, "--password and --password-file");
-			passwordOption = option;
+			once(passwordOption, "--password-file", PASSWORD_OPTIONS);
+			passwordOption = "--password-file";
 			replacePassword(decodeUtf8(readSecretFile(optionValue(args, index, option), option), option));
 			return nextIndex(argument, index);
 		case "-k":
 		case "--bmc-key": {
 			String value = consumeSecret(args, index, option);
-			exclusive(keyOption, "--bmc-key and --bmc-key-file");
-			keyOption = option;
+			once(keyOption, "--bmc-key", KEY_OPTIONS);
+			keyOption = "--bmc-key";
 			bmcKey = parseHex(value.getBytes(StandardCharsets.US_ASCII), option);
 			return nextIndex(argument, index);
 		}
 		case "-kf":
 		case "--bmc-key-file":
-			exclusive(keyOption, "--bmc-key and --bmc-key-file");
-			keyOption = option;
+			once(keyOption, "--bmc-key-file", KEY_OPTIONS);
+			keyOption = "--bmc-key-file";
 			bmcKey = parseHex(readSecretFile(optionValue(args, index, option), option), option);
 			return nextIndex(argument, index);
 		case "-P":
@@ -478,7 +482,14 @@ private void validate() throws CliUsageException {
 		}
 	}
 
-	private static void exclusive(String given, String pair) throws CliUsageException {
+	/**
+	 * Refuses a secret option given twice, in either of its forms, or together with the option it excludes: a secret
+	 * given twice is likely a mistake (the other options take their last value).
+	 */
+	private static void once(String given, String option, String pair) throws CliUsageException {
+		if (option.equals(given)) {
+			throw new CliUsageException(option + " is given twice");
+		}
 		if (given != null) {
 			throw new CliUsageException(pair + " are mutually exclusive");
 		}

src/main/java/org/metricshub/ipmi/cli/IpmiCli.java

@@ -74,7 +74,9 @@
 import org.metricshub.ipmi.client.SkippedRecord;
 import org.metricshub.ipmi.client.SolConsole;
 import org.metricshub.ipmi.core.coding.commands.PrivilegeLevel;
+import org.metricshub.ipmi.core.coding.commands.chassis.PowerCommand;
 import org.metricshub.ipmi.core.coding.commands.sdr.record.AbstractSensorRecord;
+import org.metricshub.ipmi.core.coding.commands.sdr.record.AddressType;
 import org.metricshub.ipmi.core.coding.commands.sdr.record.SensorType;
 import org.metricshub.ipmi.core.coding.commands.sdr.record.SensorUnit;
 import org.metricshub.ipmi.core.coding.commands.fru.record.BoardInfo;
@@ -116,6 +118,11 @@ public final class IpmiCli {
 	private static final int RAW_BYTES_PER_LINE = 16;
 	/** Event/reading type code of the threshold sensors (IPMI 2.0 table 42-1). */
 	private static final int THRESHOLD_EVENT_READING_TYPE = 1;
+	/** The 8-bit slave address of the BMC. */
+	private static final int BMC_ADDRESS = 0x20;
+	private static final int OWNER_ID_MASK = 0x7f;
+	private static final int LUN_MASK = 0x03;
+	private static final int CHANNEL_MASK = 0x0f;
 
 	private IpmiCli() {}
 
@@ -125,7 +132,25 @@ private IpmiCli() {}
 	 * @param args the command line
 	 */
 	public static void main(String[] args) {
-		System.exit(run(args, System.out, System.err, IpmiCli::readConsolePassword, System.in, System.console() != null));
+		System
+				.exit(run(args, System.out, System.err, IpmiCli::readConsolePassword, System.in, isTerminal(System.console())));
+	}
+
+	/**
+	 * Tells whether a console is an interactive terminal. Up to Java 21, {@code System.console()} is null when stdin or
+	 * stdout is redirected; Java 22 to 24 return a console even then (JDK-8308591) and added {@code isTerminal()} to
+	 * tell, which is called by reflection as the library compiles for Java 8.
+	 */
+	static boolean isTerminal(Console console) {
+		if (console == null) {
+			return false;
+		}
+		try {
+			return (Boolean) Console.class.getMethod("isTerminal").invoke(console);
+		} catch (ReflectiveOperationException e) {
+			// Java 8 to 21: a console exists only on a terminal
+			return true;
+		}
 	}
 
 	/**
@@ -155,6 +180,13 @@ static int run(
 				out.println("ipmi-java " + version());
 				return out.checkError() ? outputFailed(err) : 0;
 			}
+			if (terminal && arguments.subcommand() == CliArguments.Subcommand.SOL && arguments.escape() < 0) {
+				// In raw mode every keystroke is for the BMC, Ctrl+C and Ctrl+D included: without an escape sequence, only
+				// a kill from another terminal would close the console
+				throw new CliUsageException(
+						"--no-escape needs a redirected input: on a terminal, " + CliArguments.DEFAULT_ESCAPE
+								+ ". is the only way out (use --escape to pick another character)");
+			}
 			if (arguments.subcommand() != CliArguments.Subcommand.PROBE && arguments.password() == null) {
 				char[] password;
 				try {
@@ -288,10 +320,20 @@ private static int execute(
 				out.println(timed(arguments, err, "chassis status", client::chassisStatus).powerOn() ? "on" : "off");
 			} else {
 				// Nothing to print: the exit code says whether the BMC accepted the command
-				timed(arguments, err, "chassis control", () -> {
-					client.chassisControl(arguments.powerCommand());
-					return null;
-				});
+				try {
+					timed(arguments, err, "chassis control", () -> {
+						client.chassisControl(arguments.powerCommand());
+						return null;
+					});
+				} catch (IllegalStateException e) {
+					if (arguments.powerCommand() != PowerCommand.PowerCycle) {
+						throw e;
+					}
+					// The message of the client names the Java constant: say the CLI action instead
+					throw new IllegalStateException(
+							"the system is off: a power cycle would power it up on some BMCs, use 'power on'",
+							e);
+				}
 			}
 			return 0;
 		case IDENTIFY:
@@ -623,6 +665,18 @@ private static Map<String, Object> sensorFields(Sensor sensor) {
 		fields.put("typeCode", sensor.typeCode());
 		fields.put("entity", sensor.entity().name());
 		fields.put("entityInstance", sensor.entityInstance());
+		if (sensor.record() instanceof AbstractSensorRecord) {
+			// The owner as the low byte of a SEL generator ID: the 7-bit address shifted left, bit 0 set for a software
+			// ID (IPMI 2.0 table 32-1), so that a script joins sel --json on it as SelEntry.isFrom() does
+			AbstractSensorRecord owner = (AbstractSensorRecord) sensor.record();
+			fields
+					.put(
+							"ownerId",
+							(owner.getSensorOwnerId() & OWNER_ID_MASK) << 1
+									| (owner.getAddressType() == AddressType.SystemSoftwareId ? 1 : 0));
+			fields.put("ownerLun", owner.getSensorOwnerLun() & LUN_MASK);
+			fields.put("ownerChannel", owner.getChannelNumber() & CHANNEL_MASK);
+		}
 		if (sensor.unit() != SensorUnit.Unspecified) {
 			fields.put("unit", sensor.unit().name());
 		}
@@ -668,7 +722,7 @@ private static int frus(CliArguments arguments, IpmiClient client, PrintStream o
 			}
 			found.add(fru.id());
 			for (String problem : fru.problems()) {
-				diagnostic(err, "FRU " + fru.id() + ": " + problem);
+				diagnostic(err, fruLabel(fru) + ": " + problem);
 				code = EXIT_PARTIAL;
 			}
 		}
@@ -681,10 +735,30 @@ private static int frus(CliArguments arguments, IpmiClient client, PrintStream o
 		return code;
 	}
 
+	/** The 8-bit slave address of the controller that holds a FRU: 20h for the BMC. */
+	private static int fruAddress(Fru fru) {
+		return fru.locator().getDeviceAccessAddress() << 1;
+	}
+
+	/**
+	 * Names a FRU by its ID, with the address, LUN and channel of its controller when it is not held by the BMC at
+	 * LUN 0 ({@code FRU 5 at 2Ch}, {@code FRU 5 at 20h LUN 1}): since two FRUs on different controllers or LUNs may
+	 * have the same ID, the ID alone does not tell them apart. The channel of a FRU held by the BMC itself is ignored,
+	 * as the client ignores it (a Supermicro BMC locates its own FRU at 20h on channel 2).
+	 */
+	private static String fruLabel(Fru fru) {
+		int address = fruAddress(fru);
+		int lun = fru.locator().getAccessLun();
+		int channel = address == BMC_ADDRESS ? 0 : fru.locator().getManagementChannelNumber();
+		if (address == BMC_ADDRESS && lun == 0) {
+			return "FRU " + fru.id();
+		}
+		return String.format(Locale.ROOT, "FRU %d at %02Xh", fru.id(), address) + (lun == 0 ? "" : " LUN " + lun)
+				+ (channel == 0 ? "" : " channel " + channel);
+	}
+
 	private static void fru(Fru fru, PrintStream out) {
-		out
-				.println(
-						"FRU " + fru.id() + ": " + printable(fru.name()) + " (" + fru.entity() + " " + fru.entityInstance() + ")");
+		out.println(fruLabel(fru) + ": " + printable(fru.name()) + " (" + fru.entity() + " " + fru.entityInstance() + ")");
 		Optional<ChassisInfo> chassis = fru.chassis();
 		if (chassis.isPresent()) {
 			ChassisInfo area = chassis.get();
@@ -775,6 +849,9 @@ private static String mfgDate(SpdInfo module) {
 	private static Map<String, Object> fruFields(Fru fru) {
 		Map<String, Object> fields = new LinkedHashMap<String, Object>();
 		fields.put("id", fru.id());
+		fields.put("address", fruAddress(fru));
+		fields.put("lun", fru.locator().getAccessLun());
+		fields.put("channel", fru.locator().getManagementChannelNumber());
 		fields.put("name", fru.name());
 		fields.put("entity", fru.entity().name());
 		fields.put("entityInstance", fru.entityInstance());
@@ -962,10 +1039,8 @@ private static int sol(
 								+ (raw == null ?
 										", line-oriented: the input is sent when Enter is pressed and echoed by the remote console only" :
 										"")
-								+ (arguments.escape() < 0 ?
-										"; the end of the input closes it" :
-										"; " + (char) arguments.escape() + ". at the start of a line closes it, "
-												+ (char) arguments.escape() + "? lists the escape sequences"));
+								+ "; " + (char) arguments.escape() + ". at the start of a line closes it, "
+								+ (char) arguments.escape() + "? lists the escape sequences");
 			}
 			boolean lineMode = terminal && raw == null;
 			// In line mode the console would echo the typed line locally, and the remote console echoes it again (or
@@ -1379,7 +1454,7 @@ private static String confidentialityName(byte code) {
 
 	private static char[] readConsolePassword() throws CliUsageException {
 		Console console = System.console();
-		if (console == null) {
+		if (!isTerminal(console)) {
 			throw new CliUsageException("no interactive console to ask the password: use --password-file");
 		}
 		return console.readPassword("Password: ");

src/site/markdown/cli.md

@@ -114,7 +114,10 @@ A sensor is selected when its type is one of the `--type` values and its name ma
 `bmcKey()`, `timeout()`, `messageTimeout()` and `retries()` of the client builder: see
 [Configuration](configuration.html#options) for what each one does.
 
-The two password options are mutually exclusive, and so are the two BMC key options. Every option
+The two password options are mutually exclusive, and so are the two BMC key options; a password
+or key option given twice, in either of its forms (`--password=a -p b`), is refused as
+*`--password` is given twice*, since a secret given twice is likely a mistake. Every other option
+given twice takes its last value. Every option
 that takes a value also takes the `option=value` form (`--port=6230`, `-P=6230`); a flag (`-v`,
 `--json`, `--long`, `--no-escape`, `--help`, `--version`) takes no value: `--json=true` is
 refused. `-p` is the password and `-P` the port, as in the other MetricsHub command-line clients;
@@ -375,11 +378,16 @@ record. `unit` is present only when the record names one (not `Unspecified`, whi
 `Unit` column of the text output empty), `percentage` (`true`) only for a percentage sensor,
 `reading` only when the sensor has a valid converted reading, `rawReading` whenever its
 reading is valid, discrete sensors included, the thresholds only when the record sets them, and
-`problem` only when there is one:
+`problem` only when there is one. `ownerId`, `ownerLun` and `ownerChannel` identify the owner of
+the sensor ([Sensor owners](sensors.html#sensor-owners)): `ownerId` is the 8-bit slave address of
+the controller (`32`, that is `20h`, for the BMC) or, with bit 0 set, the system software ID
+(`177` for the BIOS sensors of a Dell iDRAC, `B1h`), encoded like the low byte of the
+`generatorId` of a [`sel --json`](#sel) entry; a sensor number is unique only within its owner
+and LUN:
 
 ```shell-session
 $ ipmicli -h bmc.example.com -u monitor -pf pw.txt sensors --type Fan --name 'Fan 1' --json
-{"name":"Fan 1","number":122,"type":"Fan","typeCode":4,"entity":"Fan","entityInstance":1,"unit":"Rpm","reading":6600.0,"rawReading":66,"status":"OK","states":[],"lowerCritical":1600.0}
+{"name":"Fan 1","number":122,"type":"Fan","typeCode":4,"entity":"Fan","entityInstance":1,"ownerId":32,"ownerLun":0,"ownerChannel":0,"unit":"Rpm","reading":6600.0,"rawReading":66,"status":"OK","states":[],"lowerCritical":1600.0}
 ```
 
 An SDR record the library cannot decode is reported on stderr (`ipmicli: SDR record 0006h (type
@@ -453,9 +461,16 @@ stderr (`ipmicli: FRU <id>: ...`), and `fru` exits with 1 once every FRU is prin
 is declared but absent (FRU 3 above, or an empty power supply bay) is the usual case
 ([Troubleshooting](troubleshooting.html#sensors-or-frus-are-missing)).
 
+A FRU held by another controller than the BMC, or at another LUN than 0, is headed by the 8-bit
+slave address, the LUN and the channel of its locator record (`FRU 5 at 2Ch: ...`,
+`FRU 5 at 20h LUN 1: ...`, `FRU 5 at 2Ch channel 6: ...`), in the problem lines too: a FRU ID
+is unique only per controller and LUN, and two FRUs with the same ID are both printed
+([FRUs of other controllers](fru-inventory.html#frus-of-other-controllers)).
+
 `fru 0 17` reads only these FRUs; an ID that no locator record declares exits with 66
 (`ipmicli: no such FRU 9`), also when another FRU is incomplete. `--json` prints one object per
-FRU, with the summary fields of [`Fru`](fru-inventory.html) (`manufacturer`, `model`,
+FRU, with its `id`, the `address` (8-bit, `32` for the BMC), `lun` and `channel` of its
+controller, the summary fields of [`Fru`](fru-inventory.html) (`manufacturer`, `model`,
 `serialNumber`, `partNumber`) and one nested object per area, its empty fields omitted:
 
 | Object | Keys |
@@ -469,7 +484,7 @@ FRU, with the summary fields of [`Fru`](fru-inventory.html) (`manufacturer`, `mo
 FRU 17 of the example above:
 
 ```json
-{"id":17,"name":"PSU1 FRU","entity":"SystemInternalExpansionBoard","entityInstance":2,"manufacturer":"LITEON","model":"PS-2451-6L-LF","serialNumber":"0000","product":{"manufacturerName":"LITEON","productName":"PS-2451-6L-LF","serialNumber":"0000"},"complete":true}
+{"id":17,"address":32,"lun":0,"channel":0,"name":"PSU1 FRU","entity":"SystemInternalExpansionBoard","entityInstance":2,"manufacturer":"LITEON","model":"PS-2451-6L-LF","serialNumber":"0000","product":{"manufacturerName":"LITEON","productName":"PS-2451-6L-LF","serialNumber":"0000"},"complete":true}
 ```
 
 An incomplete FRU has `"complete":false` and a `problems` array.
@@ -522,9 +537,12 @@ $ ipmicli -h bmc.example.com -u monitor -pf pw.txt sel --since 5 --json
 
 (An excerpt: the first three entries.) The sensor names are not resolved: that takes a walk of
 the SDR repository, as long as `sensors` on a slow BMC. A sensor number is unique only within its
-owner (the BMC, a satellite controller, the BIOS), which `generatorId` identifies: in code,
-[`SelEntry.isFrom(Sensor)`](system-event-log.html#the-selentry-object) matches the owner and LUN
-as well as the number, as `ipmitool sel elist` does.
+owner (the BMC, a satellite controller, the BIOS), which `generatorId` identifies: a script joins
+the entries with the output of [`sensors --json`](#sensors) on `sensorNumber` and `number`, the
+low byte of `generatorId` and `ownerId`, and bits 1:0 of its high byte and `ownerLun`
+(`generatorId` 32 and 1 above are the BMC and the BIOS, `ownerId` 32 and 1), as
+[`SelEntry.isFrom(Sensor)`](system-event-log.html#the-selentry-object) does in code, like
+`ipmitool sel elist`.
 
 ### `raw`
 
@@ -570,7 +588,9 @@ sequences of the remote console; the typed bytes go to the BMC.
   `ipmitool sol activate`: `~.` closes the console, `~B` sends a serial break, `~?` lists the
   sequences, `~~` sends a `~`. `--escape <char>` changes the escape character (one printable
   ASCII character other than space); `--no-escape` disables the sequences, for binary transfers
-  through a pipe, whose end closes the console.
+  through a pipe, whose end closes the console. It is refused on a terminal (exit code 64):
+  there, every keystroke goes to the BMC, Ctrl+C and Ctrl+D included, and the escape sequence
+  is the only way out.
 * **Every keystroke goes to the BMC**, Ctrl+C included, without local echo (the remote console
   echoes, or hides a password), so arrow keys, function keys and BIOS setup screens work; `~.` at
   the start of a line is the way out.
@@ -617,7 +637,7 @@ CLI:
 | ---: | --- |
 | `0` | Success (including `sol` closed with `~.` or by the end of the input). |
 | `1` | Partial result: `fru` could not read some FRUs completely (reported on stderr); the rest was printed. |
-| `64` | Invalid usage, including an unreadable password or key file, no password option when there is no console to ask the password on (input or output redirected), no password entered at the prompt, and a setting the client refuses (a cipher suite without integrity, a user name longer than 16 bytes, a password or BMC key longer than 20 bytes). |
+| `64` | Invalid usage, including a password or key option given twice, an unreadable password or key file, no password option when there is no console to ask the password on (input or output redirected), no password entered at the prompt, `sol --no-escape` on a terminal, and a setting the client refuses (a cipher suite without integrity, a user name longer than 16 bytes, a password or BMC key longer than 20 bytes). |
 | `66` | `fru <id>`: no such FRU. |
 | `69` | BMC unreachable: unknown host, socket error, no reply to the session setup (wrong host, UDP port 623 filtered, IPMI over LAN disabled, a BMC without IPMI 2.0 with a one-ID `-C`). |
 | `70` | IPMI failure: an error completion code (also in the reply to `raw`), no common cipher suite, a BMC without IPMI 2.0 (RMCP+), a malformed reply, a `power cycle` of a system that is off, the SOL payload in use by another session, SOL input the BMC does not accept, the SOL console deactivated by the BMC. |