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

[Vladimir Sementsov-Ogievskiy]

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.


--
Best regards,
Vladimir