Re: [Qemu-devel] [PATCH 09/16] migration: Start of multiple fd work

[Juan Quintela]

"Daniel P. Berrange" <address@hidden> wrote:
> On Mon, Mar 13, 2017 at 05:58:06PM +0100, Juan Quintela wrote:
>> "Daniel P. Berrange" <address@hidden> 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.