> -----Original Message-----
> From: Lukas Straub <lukasstra...@web.de>
> Sent: Friday, May 5, 2023 6:46 AM
> To: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
> Cc: qemu-devel@nongnu.org; quint...@redhat.com; Zhang, Chen
> <chen.zh...@intel.com>; Peter Xu <pet...@redhat.com>; Leonardo Bras
> <leob...@redhat.com>
> Subject: Re: [PATCH v4 10/10] migration: block incoming colo when capability
> is disabled
> 
> On Fri, 5 May 2023 01:30:34 +0300
> Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> wrote:
> 
> > On 05.05.23 01:10, Lukas Straub wrote:
> > > On Fri, 28 Apr 2023 22:49:28 +0300
> > > Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> wrote:
> > >
> > >> We generally require same set of capabilities on source and target.
> > >> Let's require x-colo capability to use COLO on target.
> > >>
> > >> Signed-off-by: Vladimir Sementsov-Ogievskiy
> > >> <vsement...@yandex-team.ru>
> > >
> > > Good patch, this is needed anyway for COLO with multifd.
> > >
> > > Also, it allows to remove a some code, like
> > > migration_incoming_enable_colo(), qemu_savevm_send_colo_enable()
> etc.
> > > I will send patches for that.
> >
> > Great! But with such changes we should be careful to not break
> compatibility between versions.. On the other hand, x-colo - is still
> experimental with that x-, so there is no guarantee how it works.
> 
> Given COLO's usecase, I think it should only be run with the same qemu
> version on both sides anyway. Maybe we should even enforce that somehow,
> while we're at it doing breaking changes.
> 
> For upgrading qemu without downtime, normal migration can still be used.
> 
> Zhang Cheng, what do you think?

It's OK for me. We can add the same version requirement comments to the 
docs/COLO-FT.txt.

Thanks
Chen

> 
> > > Or you can do it if you like.
> >
> > To be honest, I don't :)
> >
> > >
> > > Please update the docs like below, then:
> > > Reviewed-by: Lukas Straub <lukasstra...@web.de>
> > >
> > > diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index
> > > 8ec653f81c..2e760a4aee 100644
> > > --- a/docs/COLO-FT.txt
> > > +++ b/docs/COLO-FT.txt
> > > @@ -210,6 +210,7 @@ children.0=childs0 \
> > >
> > >   3. On Secondary VM's QEMU monitor, issue command
> > >   {"execute":"qmp_capabilities"}
> > > +{"execute": "migrate-set-capabilities", "arguments":
> > > +{"capabilities": [ {"capability": "x-colo", "state": true } ] } }
> > >   {"execute": "nbd-server-start", "arguments": {"addr": {"type": "inet",
> "data": {"host": "0.0.0.0", "port": "9999"} } } }
> > >   {"execute": "nbd-server-add", "arguments": {"device": "parent0",
> > > "writable": true } }
> > >
> >
> > I'll resend with this addition, thanks for reviewing!
> >
> > >
> > >> ---
> > >>   migration/migration.c | 6 ++++++
> > >>   1 file changed, 6 insertions(+)
> > >>
> > >> diff --git a/migration/migration.c b/migration/migration.c index
> > >> 8c5bbf3e94..5e162c0622 100644
> > >> --- a/migration/migration.c
> > >> +++ b/migration/migration.c
> > >> @@ -395,6 +395,12 @@ int migration_incoming_enable_colo(void)
> > >>       return -ENOTSUP;
> > >>   #endif
> > >>
> > >> +    if (!migrate_colo()) {
> > >> +        error_report("ENABLE_COLO command come in migration stream,
> but c-colo "
> > >> +                     "capability is not set");
> > >> +        return -EINVAL;
> > >> +    }
> > >> +
> > >>       if (ram_block_discard_disable(true)) {
> > >>           error_report("COLO: cannot disable RAM discard");
> > >>           return -EBUSY;
> > >
> > >
> > >
> >
> 
> 
> 
> --


Reply via email to