"Daniel P. Berrange" <berra...@redhat.com> wrote: > On Mon, Mar 13, 2017 at 05:58:06PM +0100, Juan Quintela wrote: >> "Daniel P. Berrange" <berra...@redhat.com> wrote: >> > On Mon, Mar 13, 2017 at 01:44:27PM +0100, Juan Quintela wrote: >> > >> > I still don't really like any of the changes in this file. We've now got >> > two sets of methods which connect to a remote host and two sets of methods >> > which accept incoming clients. I've got to think there's a better way to >> > refactor the existing code, such that we don't need two sets of methods >> > for the same actions >> >> I am open to suggestions, basically we want to be able to: >> - open one + n channels >> - be sure that we got the same id on both sides of the connection. >> >> You suggested on the previous iteration that I changed the FALSE in >> *HERE* for TRUE, but I was not able to: >> - make sure that we have opened n sockets before we continue with >> migration >> - making sure that we got same id numbers in both sides, that is doable, >> just add a new id field >> - right now I open a channel, and wait for the other side to open it >> before open the following one. I can do things in parallel, but >> locking is going to be "interesting". >> >> So, as said, I don't really care how we open the channels, I am totally >> open to suggestions. Looking at the current code, this is the best way >> that I have been able to think of. > > I think the key problem in the current design is that you delay the opening > of the extra socket channels. To be able to remove most of this duplication, > I think you need to open all the channels at once right at the start. > > > IOW, in qmp_migrate() instead of calling tcp_start_outgoing_migration() > just once, use a loop to call it N times (where N == number of threads). > > Now this method is asynchronous, and eventually triggers a call to > migration_channel_connect() when the connection actually succeeds. > You will need to change migration_channel_connect() so that it can be > called multiple times. migration_channel_connect() should count how > many channels have been opened, and only start the migration once all > of them are open. > > The incoming side is a little different - in qemu_start_incoming_migration() > you only need call tcp_start_incoming_migration() once. In the > socket_accept_incoming_migration() method though, you need to change the > 'return FALSE' to 'return TRUE', so that it continues to accept multiple > incoming clients. The socket_start_outgoing_migration()method needs to again > count the number of channels that have been opened so far, and only start > the actual migration once the right number are open. > > > By doing all this opening of channels upfront, you'll also make it much > easier to support the other migration protocols - in particular 'fd' > protocol needs to be extended so that libvirt can pass in multiple FDs > in the monitor command at once. The 'exec' protocol should also be > able to trivially support this by simply launching the command multiple > times.
Ok. Thanks. Will look into this. Later, Juan.
