Markus Armbruster <arm...@redhat.com> wrote:
> Eric Blake <ebl...@redhat.com> writes:
>
>> On 05/15/2017 04:46 AM, Dr. David Alan Gilbert wrote:
>>> * Juan Quintela (quint...@redhat.com) wrote:
>>>> Eric Blake <ebl...@redhat.com> wrote:
>>>>> On 05/11/2017 11:32 AM, Juan Quintela wrote:
>>>>>> Those two capabilities were added through the command line. Notice that
>>>>>> we just created them. This is just the boilerplate.
>>>>>>
>>>>>> Signed-off-by: Juan Quintela <quint...@redhat.com>
>>>>>> Reviewed-by: Eric Blake <ebl...@redhat.com>
>>>>>>
>>>>>> --
>>>>>>
>>>>>> Make migrate_set_block_* take a boolean argument.
>>>>>
>>>>> Question - do we support the orthogonal selection of all 4 combinations
>>>>> under HMP 'migrate' (no argument, -b alone, -i alone, -b and -i
>>>>> together), or are there only 3 actual states? If the latter, should we
>>>>> represent this as a single enum-valued property, rather than as two
>>>>> independent boolean properties?
>>>>
>>>> { 'enum': 'MigrationCapability',
>>>> 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
>>>> 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram'] }
>>>>
>>>>
>>>> My understanding is that we can only have boolean capabilities here.
>>>> Or, how could we put a non-boolean capability?
>>
>> If we want a non-boolean, then we make it a migration parameter rather
>> than a migration capability. There may be other advantages to using
>> MigrationParameter instead of MigrationCapability (such as making it
>> easier to figure out whether the parameter settings are persistent or
>> apply per-migration).
>
> What makes a migration knob a MigrationCapability rather than a
> MigrationParameter? Type bool, or is there more to it?
I didn't started this, but *my* undersanding:
Migration capability: we have to set this up before migration starts.
It is like a property that we can't change later. We can't add a
compression thread in the middle of migration (notice that we "could"
but current code can't).
So, block enabled migration is a capability.
Block shared on the other hand is weird. *My* understading of the code
is that we have a qcow2 overlay on top of the base image, and:
- without shared: we migrate all the block device
- with shared: we migrate only the top overlay
So, thisk could be a parameter of block migration. If anyone understand
better than me, please stand up.
>>> Lets keep this simple and stick with the booleans.
>>>
>>> Dave
>>>
>>>> There are three states as far as I can see.
>
> Begs the question how the fourth state behaves. Documentation is of no
> help:
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 5728b7f..109852e 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -894,11 +894,16 @@
> # @release-ram: if enabled, qemu will free the migrated ram pages on the
> source
> # during postcopy-ram migration. (since 2.9)
> #
> +# @block-enabled: enable block migration (Since 2.10)
> +#
> +# @block-shared: enable block shared migration (Since 2.10)
> +#
> # Since: 1.2
> ##
>
> Please explain all four states clearly there.
Is the previous explanation enough?
>> I'll leave it up to you as maintainers which way you prefer, I'm just
>> offering the potential design tradeoffs for simplicity of booleans (but
>> complexity in an unused state) vs. simplicity of design (but complexity
>> in code).
>
> For what it's worth, I dislike entangled booleans.
Some here, but except if we put shared as a migration parameter, I don't
know what to do here.
{ 'enum': 'MigrationParameter',
'data': ['compress-level', 'compress-threads', 'decompress-threads',
'cpu-throttle-initial', 'cpu-throttle-increment',
'tls-creds', 'tls-hostname', 'max-bandwidth',
'downtime-limit', 'x-checkpoint-delay' ] }
I don't see either how to define that block_shared will be a boolean parameter.
Later, Juan.
