qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>,
	qemu-devel@nongnu.org, jjherne@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [PATCH RFC 1/1] virtio: migrate config_vector
Date: Thu, 14 May 2015 11:36:57 +0200	[thread overview]
Message-ID: <20150514112845-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <55546945.4050400@de.ibm.com>

On Thu, May 14, 2015 at 11:22:13AM +0200, Christian Borntraeger wrote:
> Am 13.05.2015 um 23:47 schrieb Michael S. Tsirkin:
> > On Wed, May 13, 2015 at 08:57:00PM +0200, Christian Borntraeger wrote:
> >> Am 13.05.2015 um 18:14 schrieb Michael S. Tsirkin:
> >>>> - AFAICS, there's no easy way to add transport-specific subsections -
> >>>>   and simply adding config_vector in ccw would break compatibility
> >>>
> >>> subsections break compatibility too.  The only way around that is to set
> >>> a flag to skip migrating config_vector for old machine types.
> >>
> >> My main concern is about undetected compatibility issues. A subsection will 
> >> tell the user that something went wrong. What happens if we just add a new
> >> qemu_put_byte in the stream. Will the savevm core always detect that we have
> >> too many or not enough bytes? If yes, adding new stuff in the stream will
> >> always be detected in some way as error we can go with just adding
> >> qemu_put_be16/qemu_get_be16 in virtio_ccw_save_config/virtio_ccw_load_config.
> >> Old/new QEMUs will then not be compatible - but thats probably ok as long as it
> >> errors out.
> >>
> >> My understanding was that we do not have a guarentee that this will be
> >> detected all the time and having random junk in some variables is a debugging
> >> nightmare. Is that correct?
> >>
> >>
> >> Christian
> > 
> > It's not too bad - normally there's a bunch of strings that
> > helps you find out what's going on.
> > But if you really care about debuggability of migration streams, help move
> > forward dgilbert's RFC that switched to a self-delimiting format.
> > Just piling up random hacks in virtio seems like a wrong approach.
> > 
> 
> Thats not my question. PLEASE try to understand my question.
> I want a hard stop if migration changes in incompatible ways.
> If adding a qemu_put_byte in virtio_ccw gets detected we can just fix
> virtio_ccw AS YOU SUGGESTED. I just want to know if I can rely on that 
> or not.
> 
> Christian 

I answered exactly this question but let me try to spell the answer
out a bit more.

There are three answers:
1.  Yes, it's sure to get detected because everything gets shifted
    and then you get an unexpected string instead of next device name.
2.  If you want a more generic way to detect this, then please work
    on changing format for devices generally so each device
    section has a byte length attached to it. Then we know that
    when we make changes, they are detected as device will end
    earlier/later than expected.
3.  You can have a different workaround: add property "skip config vec
    on migration" and set it for old spapr machine types.
    old types continue losing config vec; new ones work better.

-- 
MST

  reply	other threads:[~2015-05-14  9:37 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-13 14:42 [Qemu-devel] [PATCH RFC 0/1] virtio: fix config_vector migration issues Cornelia Huck
2015-05-13 14:42 ` [Qemu-devel] [PATCH RFC 1/1] virtio: migrate config_vector Cornelia Huck
2015-05-13 14:58   ` Michael S. Tsirkin
2015-05-13 15:03     ` Cornelia Huck
2015-05-13 16:14       ` Michael S. Tsirkin
2015-05-13 18:57         ` Christian Borntraeger
2015-05-13 21:47           ` Michael S. Tsirkin
2015-05-14  9:22             ` Christian Borntraeger
2015-05-14  9:36               ` Michael S. Tsirkin [this message]
2015-05-14 10:02                 ` Paolo Bonzini
2015-05-14 10:30                 ` Christian Borntraeger
2015-05-14 17:00                   ` Dr. David Alan Gilbert
2015-05-15  7:08                     ` Christian Borntraeger
2015-05-15  7:13                       ` Michael S. Tsirkin
2015-05-18 11:26                         ` Cornelia Huck
2015-05-18 15:29                           ` Cornelia Huck
2015-06-03 11:59                             ` Christian Borntraeger
2015-06-03 12:23                               ` Michael S. Tsirkin
2015-05-14  8:24         ` Paolo Bonzini
2015-05-14  9:56           ` Michael S. Tsirkin
2015-05-14 10:04             ` Paolo Bonzini
2015-05-14 10:07               ` Michael S. Tsirkin
2015-05-14 10:09                 ` Paolo Bonzini
2015-05-14 10:38                   ` Christian Borntraeger
2015-05-14  8:22   ` Paolo Bonzini

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=20150514112845-mutt-send-email-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=cornelia.huck@de.ibm.com \
    --cc=jjherne@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /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).