Re: [PATCH v2 3/3] nvme: fire discovery log page change events to userspace

From: Sagi Grimberg
Date: Fri Aug 30 2019 - 14:14:45 EST



You are correct that this information can be derived from sysfs, but the
main reason why we add these here, is because in udev rule we can't
just go ahead and start looking these up and parsing these..

We could send the discovery aen with NVME_CTRL_NAME and have
then have systemd run something like:

nvme connect-all -d nvme0 --sysfs

and have nvme-cli retrieve all this stuff from sysfs?

Actually that may be a problem.

There could be a hypothetical case where after the event was fired
and before it was handled, the discovery controller went away and
came back again with a different controller instance, and the old
instance is now a different discovery controller.

This is why we need this information in the event. And we verify this
information in sysfs in nvme-cli.

Well, that must be a usual issue with uevents, right? Don't we usually
have a increasing serial number for that or something?

Yes we do, userspace should use it to order events. Does udev not
handle that properly today?

The problem is not ordering of events, its really about the fact that
the chardev can be removed and reallocated for a different controller
(could be a completely different discovery controller) by the time
that userspace handles the event.

So? You will have gotten the remove and then new addition uevent in
order showing you this. So your userspace code knows that something
went away and then came back properly so you should be kept in sync.

Still don't understand how this is ok...

I have /dev/nvme0 represents a network endpoint that I would discover
from, it is raising me an event to do a discovery operation (namely to
issue an ioctl to it) so my udev code calls a systemd script.

By the time I actually get to do that, /dev/nvme0 represents now a new
network endpoint (where the event is no longer relevant to). I would
rather the discovery to explicitly fail than to give me something
different, so we pass some arguments that we verify in the operation.

Its a stretch case, but it was raised by people as a potential issue.