Re: [PATCH v2 3/4] build: move COLO under CONFIG_REPLICATION

[Juan Quintela]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
> On 23.04.23 04:54, Zhang, Chen wrote:
>> 
>>> -----Original Message-----
>>> From: Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru>
>>> Sent: Friday, April 21, 2023 4:36 PM
>>> To: Zhang, Chen<chen.zhang@intel.com>;qemu-devel@nongnu.org
>>> Cc:qemu-block@nongnu.org;michael.roth@amd.com;armbru@redhat.com;
>>> eblake@redhat.com;jasowang@redhat.com;quintela@redhat.com; Zhang,
>>> Hailiang<zhanghailiang@xfusion.com>;philmd@linaro.org;
>>> thuth@redhat.com;berrange@redhat.com;marcandre.lureau@redhat.com;
>>> pbonzini@redhat.com;dave@treblig.org;hreitz@redhat.com;
>>> kwolf@redhat.com;lizhijian@fujitsu.com
>>> Subject: Re: [PATCH v2 3/4] build: move COLO under CONFIG_REPLICATION
>>>
>>> On 21.04.23 06:02, Zhang, Chen wrote:
>>>>
>>>>> -----Original Message-----
>>>>> From: Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru>
>>>>> Sent: Thursday, April 20, 2023 6:53 AM
>>>>> To:qemu-devel@nongnu.org
>>>>> Cc:qemu-block@nongnu.org;michael.roth@amd.com;
>>> armbru@redhat.com;
>>>>> eblake@redhat.com;jasowang@redhat.com;quintela@redhat.com;
>>> Zhang,
>>>>> Hailiang<zhanghailiang@xfusion.com>;philmd@linaro.org;
>>>>> thuth@redhat.com;berrange@redhat.com;
>>> marcandre.lureau@redhat.com;
>>>>> pbonzini@redhat.com;dave@treblig.org;hreitz@redhat.com;
>>>>> kwolf@redhat.com; Zhang, Chen<chen.zhang@intel.com>;
>>>>> lizhijian@fujitsu.com; Vladimir Sementsov-Ogievskiy
>>>>> <vsementsov@yandex- team.ru>
>>>>> Subject: [PATCH v2 3/4] build: move COLO under CONFIG_REPLICATION
>>>>>
>>>>> 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.
>>>> Yes, you are right for current status. Because COLO best practices is
>>> replication + colo live migration + colo proxy.
>>>> But doesn't mean it has to be done in all scenarios as I explanation in V1.
>>>> The better way is allow to use x-colo capability firstly, and separate
>>>> this patch with two config options: --disable-replication  and --disable-x-
>>> colo.
>>> But what for? We for sure don't have such scenarios now (COLO without
>>> replication), as it's not allowed by far 7e934f5b27eee1b0d7 (by you and
>>> David).
>>>
>>> If you think we need such scenario, I think it should be a separate series
>>> which reverts 7e934f5b27eee1b0d7 and adds corresponding test and
>>> probably documentation.
>> In the patch 7e934f5b27eee1b0d7 said it's for current independent disk mode,
>> And what we talked about before is the shared disk mode.
>> Rethink about the COLO shared disk mode, this feature still needs some 
>> enabling works.
>> It looks OK for now and separate the build options when enabling COLO shared 
>> disk mode.
>
> I've started working on this, and now I see, that check in the 
> migrate_caps_check() is not the only place.
>
> migration/colo.c has also several abort() points. For example,
> colo_process_checkpoint will simply abort if CONFIG_REPLICATION not
> defined.
>
> So for sure, current code is not prepared to use COLO with REPLICATION 
> disabled.
>
> If this possibility is needed it requires more work. Personally, I
> don't think that possibility to enable COLO with disabled REPLICATION
> is really needed and I know nobody who need it, so that seems to be
> extra work.

Whoever does the work to make COLO without REPLICATION work, it can also
do the aditional work of splitting it.  Changing a configure file is
going to be the smaller of its problems.

Later, Juan.