[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [Nbd] [w@...112...: Re: nbd-client started at boot (for root device) is not persistent]



Hi Victor,

On Mon, Nov 28, 2016 at 08:25:35AM +0200, Victor wrote:
> Here is an updated patch version, better matching your suggestions with a few
> comments:
> 
> a) I create the device with mknod, but i create it in /dev, not /tmp, because
> run-init deleted /tmp anyway. So if I have to re-create a directory, I have to
> choose between recreating /dev and recreating /tmp. So why not recreate /tmp
> and leave the nbddev string as it is, unchanged and less confusing? And if /dev
> /nbd0 was deleted for some reason but /dev is still udev-mounted, i can create
> it there manually anyway and it will be gone at reboot, which is ok. The other
> option (which avoided any directory creation) was to create the device directly
> in / but for some reason I find this too ugly.

That works too, I suppose.

> b) your current solution (checking for the real pid of the kernel process)
> seems to me more elegant than just waiting a second.

I agree, but unfortunately it isn't that easy.

When using systemd as pid1, the initramfs processes are in their own
private mount namespace, so mounting something there can't cause (much)
harm.

However, there are other pid1 implementations, not all of whom use that
feature. As such, mounting something opens the possibility of a race
condition where a different process might open the mount point and wreak
havoc with things.

nbd-client's responsibility is to set up an nbd connection, nothing
more. Mucking about with creating a device node is still within its
prerogative, presumably. Mounting /sys is decidedly out of what it
should be doing, and I am therefore hesitant to introduce it.

What I'm suggesting is that the parent process negotiates everything,
but that it does the fork() call only just before doing the NBD_DO_IT
ioctl(). At that point, we've already negotiated, and there is very
little that can go wrong anymore. The child process will need to wait a
short while to avoid the obvious race (although a second might indeed be
overdoing it), but beyond that it should be good.

> So i have made the
> check_conn function to test for /sys existence, and if it is missing, it will
> mount it, read what it needs and umount it back. This way, /sys will not remain
> mounted

NAK, as per above. I'm *very* uncomfortable doing that.

(side note: the packages in Ubuntu are a direct recompile of the
packages in Debian, maintained by me as well, so not sure you need to
talk to launchpad much... ;-)

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12



Reply to: