No, it's not a good idea to destroy client in the child nbd process
Should probably have been an obvious side effect :-) Child process gets open file descriptors to parent's epoll/timerfd, and it's totally OK to just close() all of them, but it's absolutely NOT OK to run destructors - they modify the kernel state of epoll/timerfd before destroying. So, basically, when we destroy the client in the child process, we break it in the parent too. This also means that cluster_client_t doesn't support fork(). :-)
This commit is contained in:
@@ -709,21 +709,13 @@ help:
|
||||
ringloop->loop();
|
||||
ringloop->wait();
|
||||
}
|
||||
destroy_client();
|
||||
}
|
||||
|
||||
void destroy_client()
|
||||
{
|
||||
if (cli)
|
||||
{
|
||||
cli->flush();
|
||||
delete cli;
|
||||
delete epmgr;
|
||||
delete ringloop;
|
||||
cli = NULL;
|
||||
epmgr = NULL;
|
||||
ringloop = NULL;
|
||||
}
|
||||
cli->flush();
|
||||
delete cli;
|
||||
delete epmgr;
|
||||
delete ringloop;
|
||||
cli = NULL;
|
||||
epmgr = NULL;
|
||||
ringloop = NULL;
|
||||
}
|
||||
|
||||
void load_module()
|
||||
@@ -911,7 +903,6 @@ protected:
|
||||
// works correctly, and we don't accidentally break previous NBD devices while setting
|
||||
// up a new device. Forking to check every device is of course rather slow, so we also
|
||||
// do an additional check by calling list_mapped() before searching for a free NBD device.
|
||||
destroy_client();
|
||||
if (bg)
|
||||
{
|
||||
daemonize_fork();
|
||||
|
||||
Reference in New Issue
Block a user