Het Gala <het.g...@nutanix.com> writes:
> On 19/07/22 12:36 pm, Markus Armbruster wrote:
>> Het Gala <het.g...@nutanix.com> writes:
>>
>>> On 18/07/22 8:03 pm, Markus Armbruster wrote:
>>>> Het Gala <het.g...@nutanix.com> writes:
>>>>
>>>>> On 18/07/22 2:05 pm, Markus Armbruster wrote:
>>>>>> Het Gala <het.g...@nutanix.com> writes:
>>>>>>
>>>>>>> i) Modified the format of the qemu monitor command : 'migrate' by
>>>>>>> adding a list,
>>>>>>> each element in the list consists of multi-FD connection
>>>>>>> parameters: source
>>>>>>> and destination uris and of the number of multi-fd channels
>>>>>>> between each pair.
>>>>>>>
>>>>>>> ii) Information of all multi-FD connection parameters’ list, length of
>>>>>>> the list
>>>>>>> and total number of multi-fd channels for all the connections
>>>>>>> together is
>>>>>>> stored in ‘OutgoingArgs’ struct.
>>>>>>>
>>>>>>> Suggested-by: Manish Mishra <manish.mis...@nutanix.com>
>>>>>>> Signed-off-by: Het Gala <het.g...@nutanix.com>
>>>>>>> ---
>> [...]
>>
>>>>>>> diff --git a/migration/socket.c b/migration/socket.c
>>>>>>> index 4fd5e85f50..7ca6af8cca 100644
>>>>>>> --- a/migration/socket.c
>>>>>>> +++ b/migration/socket.c
>>>>>>> @@ -32,6 +32,17 @@ struct SocketOutgoingArgs {
>>>>>>> SocketAddress *saddr;
>>>>>>> } outgoing_args;
>>>>>>>
>>>>>>> +struct SocketArgs {
>>>>>>> + struct SrcDestAddr data;
>>>>>>> + uint8_t multifd_channels;
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct OutgoingMigrateParams {
>>>>>>> + struct SocketArgs *socket_args;
>>>>>>> + size_t length;
>>>>>> Length of what?
>>>>> length of the socket_args[] array. Thanks for pointing it out. I will
>>>>> be more specific for this variable in the v2 patchset series.
>>>>>
>>>>>>> + uint64_t total_multifd_channel;
>>>>>> @total_multifd_channels appears to be the sum of the
>>>>>> socket_args[*].multifd_channels. Correct?
>>>>> Yes Markus, you are correct.
>>>> Sure you need to keep the sum separately?
>>> So earlier, the idea behind this was that, we had this intention to
>>> depreciate the migrate_multifd_channels() API from the live migration
>>> process. We made total_multifd_channels() function, where it used to
>>> calculate total number of multifd channels every time, for whichever
>>> function called was computation internsive so we replaced it by returning
>>> sum of socket_args[*].multifd_channels i.e.
>>> total_multifd_channel in the later patches.
>>>
>>> But now in the v2 patchset onwards, Thanks to inputs from Dr. David and
>>> Daniel, we are not depricating migrate_multifd_channels() API but
>>> the value from the API will be cross-referenced with sum of
>>> socket_args[*].multifd_channels i.e. total_multifd_channel, and error if
>>> they are not equal.
>> I'm afraid I don't understand. I'm not sure I have to. Let me loop
>> back to my question.
>>
>> If @total_multifd_channel is always the sum of the
>> socket_args[*].multifd_channels, then you can always compute it on the
>> fly.
>>
>> I.e. you can replace @total_multifd_channel by a function that returns
>> the sum.
>>
>> Precomputing it instead is more complex, because then you need to
>> document that the two are the same. Also, bug oppertunity: letting them
>> deviate somehow. I figure that's worthwhile only if computing on the
>> fly is too expensive.
>> Okay, I understand your concern. I am okay with your approach too, but these
>> things are not expected to change out of qmp command context.
>
> So is keeping @total_multifd_channel variable should be fine? or making a
> function is better?
I recommend making it a function unless we need a variable for
performance.
[...]
