All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.