There is a fun race that can happen if we begin migrating while the
allocation map is still building. We call bitset_enable_stream()
when the migration begins, which causes the builder to start putting
events into the stream. This is bad all by itself, as it slows the
migration down for no reason, but the stream is a limited-size queue
and there are situations (migration fails and is restarted) where we
can end up with the queue full and nobody able to empty it, freezing
the whole thing.
This prevents a fairly nasty situation occurring where the rate of change on the disc is high enough that
just servicing it generates enough traffic to keep us over the bwlimit threshold indefinitely. That would
cause us to sleep during the only windows we'd ordinarily have to advance the offset.
This removes the concept of 'passes' completely from mirror.c,
although it leaves the relevant bits in mirror.h to keep status from
failing - although its current code is now Wrong. FIXME.
We also now get the previous test passing, meaning mirroring works
again.
Given our bitset_stream events, we no longer need to worry about
keeping track of the dirty map. This also lets us rip out the
server I/O lock from mirroring.
It's possible that we can remove the lock from client.c as well at
this point, but I need to have a bit more of a think about possible
races
Currently, we prevent clients from processing requests by taking
the server I/O lock. This leads to requests hanging for a long
time before being terminated when the migration completes, which
is not ideal. With this change, at the start of the final pass,
existing clients are closed and any new connections will be closed
immediately (so no NBD server handshake will be seen).
This is part of the work required to remove remove the server I/O
lock completely.
If we're above max_bytes_per_second once we've finished a transfer
(8MB chunks, worst-case) then we delay the next transfer until
all_dirty_bytes / duration < max_bytes_per_second - checking once
per second.
If this isn't good enough, we can improve it - leaky bucket is one
option. To begin with, though, we'll mostly be using this to set
max_bps to either 0 or 100MB/sec or so. So it should be fine.
This makes it easier for the tests (and supervisor) to guarantee to be
able to connect to the server socket.
Also this patch moves freeing the mirror supervisor into the server
thread.
This prevents the supervisor from thinking that the migration completed
successfully.
In order to do this, I've introduced a new lock around the start (and
finish) of the migration so that we avoid a race between the signal
handler in the server_accept loop and the control thread mirror startup.
Without that, we'd risk successfully starting a migration after the
SIGTERM handler fired, which would be Bad.
The three-way hand-off has a problem: there's no way to arrange for the
state of the migration to be unambiguous in case of failure. If the
final "disconnect" message is lost (as in, the destination never
receives it whether it is sent by the sender or not), the destination
has no option but to quit with an error status and let a human sort it
out. However, at that point we can either arrange to have a .INCOMPLETE
file still on disc or not - and it doesn't matter which we choose, we
can still end up with dataloss by picking a specific calamity to have
befallen the sender.
Given this, it makes sense to fall back to a simpler protocol: just send
all the data, then send a "disconnect" message. This has the same
downside that we need a human to sort out specific failure cases, but
combined with --unlink before sending "disconnect" (see next patch) it
will always be possible for a human to disambiguate, whether the
destination quit with an error status or not.
The mirror_super signals the commit state to the control thread via an
mbox, and this mbox is moved to control. It was owned by mirror_super,
but the problem with that is that mirror_super can free the mbox before
the control client has been scheduled to receive the message. If it's
owned by the control object, that can't happen.
If the mirror attempt failed and we were able to report an error to the
user, it makes no sense to attempt a retry. We don't have a way to
abort a mirror attempt yet, so if the user got a setting wrong and it's
failing for that reason, the only recourse they'd have would be to
restart the server.
At the moment, a first-pass failed migration will retry. This is wrong,
it should abort. However, to make that happen the mirror supervisor
needs to know the commit state of the mirror thread. With a self_pipe
mirror commit signal that information wasn't there.
If the mirror attempt connects ok, but is rejected (say, for reporting
the wrong size), the client socket needs to be closed. The destination
end can't close its socket and accept another connection attempt unless
it does.
O_DIRECT causes problems on (at least) a wheezy VM, and there are mixed
reports about its performance impact. This patch makes it a
compile-time choice which should remain until it's been benchmarked.
