qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Peter Xu <peterx@redhat.com>
Cc: Xiao Guangrong <guangrong.xiao@gmail.com>,
	Eric Blake <eblake@redhat.com>,
	pbonzini@redhat.com, mst@redhat.com, mtosatti@redhat.com,
	kvm@vger.kernel.org, Xiao Guangrong <xiaoguangrong@tencent.com>,
	qemu-devel@nongnu.org, wei.w.wang@intel.com,
	jiang.biao2@zte.com.cn
Subject: Re: [Qemu-devel] [PATCH] migration: introduce decompress-error-check
Date: Wed, 2 May 2018 15:57:13 +0100	[thread overview]
Message-ID: <20180502145713.GE2679@work-vm> (raw)
In-Reply-To: <20180502030309.GC25938@xz-mi>

* Peter Xu (peterx@redhat.com) wrote:
> On Fri, Apr 27, 2018 at 06:40:09PM +0800, Xiao Guangrong wrote:
> > 
> > 
> > On 04/27/2018 05:31 PM, Peter Xu wrote:
> > > On Fri, Apr 27, 2018 at 11:15:37AM +0800, Xiao Guangrong wrote:
> > > > 
> > > > 
> > > > On 04/26/2018 10:01 PM, Eric Blake wrote:
> > > > > On 04/26/2018 04:15 AM, guangrong.xiao@gmail.com wrote:
> > > > > > From: Xiao Guangrong <xiaoguangrong@tencent.com>
> > > > > > 
> > > > > > QEMU 2.13 enables strict check for compression & decompression to
> > > > > > make the migration more robuster, that depends on the source to fix
> > > > > 
> > > > > s/robuster/robust/
> > > > > 
> > > > 
> > > > Will fix, thank you for pointing it out.
> > > > 
> > > > > > the internal design which triggers the unexpected error conditions
> > > > > 
> > > > > 2.13 hasn't been released yet.  Why do we need a knob to explicitly turn
> > > > > off strict checking?  Can we not instead make 2.13 automatically smart
> > > > > enough to tell if the incoming stream is coming from an older qemu
> > > > > (which might fail if the strict checks are enabled) vs. a newer qemu
> > > > > (the sender gave us what we need to ensure the strict checks are
> > > > > worthwhile)?
> > > > > 
> > > > 
> > > > Really smart.
> > > > 
> > > > How about introduce a new command, MIG_CMD_DECOMPRESS_ERR_CHECK,
> > > > the destination will do strict check if got this command (i.e, new
> > > > QEMU is running on the source), otherwise, turn the check off.
> > > 
> > > Why not we just introduce a compat bit for that?  I mean something
> > > like: 15c3850325 ("migration: move skip_section_footers",
> > > 2017-06-28).  Then we turn that check bit off for <=2.12.
> > > 
> > > Would that work?
> > 
> > I am afraid it can not. :(
> > 
> > The compat bit only impacts local behavior, however, in this case, we
> > need the source QEMU to tell the destination if it supports strict
> > error check.
> 
> My understanding is that the new compat bit will only take effect when
> at destination.
> 
> I'm not sure I'm thinking that correctly. I'll give some examples.
> 
> When we migrate from <2.12 to 2.13, on 2.13 QEMU we'll possibly with
> (using q35 as example, always) "-M pc-q35-2.12" to make the migration
> work, so this will let the destination QEMU stop checking
> decompressing errors.  IMHO that's what we want so it's fine (forward
> migration).
> 
> When we migrate from 2.13 to <2.12, on 2.12 it'll always skip checking
> decompression errors, so it's fine too even if we don't send some
> compress-errored pages.
> 
> Then, would this mean that the compat bit could work too just like
> this patch?  AFAIU the compat bit idea is very similar to current
> patch, however we don't really need a new parameter to make things
> complicated, we just let old QEMUs behave differently and
> automatically, then user won't need to worry about manually specify
> that parameter.

I think you're saying just to wire it to the machine type for receive;
that would work and would be fairly simple, although wouldn't provide
the protection when going from new->new using an old machine type.

Dave

> Thanks,
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

  reply	other threads:[~2018-05-02 14:57 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-26  9:15 [Qemu-devel] [PATCH] migration: introduce decompress-error-check guangrong.xiao
2018-04-26  9:19 ` Xiao Guangrong
2018-04-26  9:34 ` Dr. David Alan Gilbert
2018-04-26 13:18   ` Xiao Guangrong
2018-04-26 14:01 ` Eric Blake
2018-04-27  3:15   ` Xiao Guangrong
2018-04-27  9:31     ` Peter Xu
2018-04-27 10:40       ` Xiao Guangrong
2018-05-02  3:03         ` Peter Xu
2018-05-02 14:57           ` Dr. David Alan Gilbert [this message]
2018-05-03  2:10             ` Peter Xu
2018-04-27 11:29     ` Dr. David Alan Gilbert
2018-04-28  6:13       ` Xiao Guangrong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180502145713.GE2679@work-vm \
    --to=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=guangrong.xiao@gmail.com \
    --cc=jiang.biao2@zte.com.cn \
    --cc=kvm@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=wei.w.wang@intel.com \
    --cc=xiaoguangrong@tencent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).