unecessary leftover printf in iet.c

[ahvlima]

Unless I'm missing something, in iet.c line 134 there is an unnecessary 'printf("error\n")", probably a debugging leftover.

Since the code uses readlink, any non-link file on "dev/disk/by-path" will cause "error" to print, giving the impression something is actually wrong. It still works, but commands like "multipath -ll" prints "error" for each target that is uses to the iet prioritizer.

The fix is simple: just remove line 134.

[mwilck]

Sponteneously I would say that any non-symlink file under /dev/disk/by-path is an error. Do you have an example of such a file on your system?

[mwilck]

Hm, no you're right ...

[mwilck]

Oh my. systemd commit 3af66c0 ("udev: generate system-unique storage symlinks using device path"). For cases "When the same disk image is written to multiple storage units". Break healthy systems by a feature that no sane person would ever need.

[mwilck]

I'm tempted to leave the printf statement where it is. And tell everyone compalaining that the problem is in systemd, not multipath-tools.

[mwilck]

systemd/systemd#42844

[ahvlima]

While it's fair to say that the problem is with systemd, the printf is not useful. In fact it does not convey enough information so the user can actually understand what's going on... and that there is not way to prevent the logs from being flooded...

I would say either remove it or make the message clear (and still useless as the user can't do s*** about it).

Doesn't hurt to remove it, as for multipath-tools only links (and starting with "IP-") have meaning. Testing for -1 is actually a nice way to filter non-links!!!

[mwilck]

It turns out that this has been discussed previously, with me not being able to prevent it from being merged, and that I'd forgotten about it 🤦

[ahvlima]

I know it's off-topic, but since we are dealing with iet.c, have you ever thought about adding another type of prio_args to iet? Like:

ippriorities="[IP|CIDR]:priority [IP|CIDR]:priority ...." and if not match found, return 10

That would be great for scenarios with more than 2 IP addresses, for more complex failover scenarios. CIDR would even allow for rules covering server on a given subnet, like:

ippriorities="10.10.30.0/24:30 10.10.20.0/24:20 10.10.10.0/24:10"

[mwilck]

No. Actually I haven't seen anyone using iet for a loong time, if ever.

[ahvlima]

Well, I think I have a use case where iet is nice and clean solution.

Proxmox server with 3 network adapters, each on a different VLAN/subnet/switch. One VLAN is for VM access (1Gbps), the second is iSCSI 10G+MTU900 and the third is iSCSI 1G+MTU9000 (failover). IET is perfect for selecting the 10G as the preferred path being the iSCSI servers TrueNAS Community (so no ALUA), and also connected to the 3 VLANs. With more than 2 IET priorities, it would be possible to use the VM's 1G connection as a last resort.

[mwilck]

I wasn't saying that it's useless. I just haven't seen any reports against it until you wrote this one. And the code has hardly seen any changes since it was added in 2011. So TrueNAS supports IET only, no LIO? I heard it uses SCST...

Anyway, if you want to work on this prioritizer, feel welcome!

[ahvlima]

the code is relatively simple and straightforward, so only a major change on a "decade old convention" was like to cause problem. And it still works, just printing "error" all the time and flooding the logs.

Also, Indeed TrueNAS Scale uses SCST, but icomes in two flavors: Enterprise and Community, having the later some features like ALUA disabled.

I will, given some free time, try to create a VM to give it a try.

[mwilck]

Please don't get me wrong, I am not saying that I won't work on this. But it's a matter of priorities, I wouldn't have time for it soon.

[ahvlima]

Sorry if you misunderstood, I'll be glad to give it a try.