Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> wrote:
> On 20.04.23 13:03, Juan Quintela wrote:
>> Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> wrote:
>>> We don't allow to use x-colo capability when replication is not
>>> configured. So, no reason to build COLO when replication is disabled,
>>> it's unusable in this case.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
>>
>>> +bool migrate_colo_enabled(void)
>>> +{
>>> + MigrationState *s = migrate_get_current();
>>> + return s->enabled_capabilities[MIGRATION_CAPABILITY_X_COLO];
>>> +}
>> Aha, It is for this that you changed the black magic on the previous
>> patch. Looks ok from my ignorance. As said before, I would not remove
>> the capability, left it the way it was.
>> You have less code (less #ifdefs that you just had to add), and
>> enabling/disabling checking capabilities don't need anything from
>> replication.
>
> Yes, I had a sense of doubt while adding these #ifdefs.
>
> Still, on the other hand I feel that it's strange to have public interface
> which only can say "I'm not built in" :)
>
> Actually with my way, we have just two #ifdefs instead of one. Seems,
> not too many. And instead of "I'm not supported" error we just not
> include the public interface for unsupported feature. It seems to be
> better user experience. What do you think?
I preffer the regularity that all capabilities are the same, and the
only place to look if something is disabled is a single place.
But on the other hand, the main user is libvirt, so
Daniel, what does libvirt preffers?
/me guess that they have to do both anyways to detect old versions, but
who knows.
Later, Juan.
