qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	Kevin Wolf <kwolf@redhat.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org, armbru@redhat.com,
	eblake@redhat.com, famz@redhat.com, stefanha@redhat.com,
	quintela@redhat.com, mreitz@redhat.com, peter.maydell@linaro.org,
	den@openvz.org, jsnow@redhat.com, lirans@il.ibm.com
Subject: Re: [Qemu-devel] [PATCH 3/4] savevm: fix savevm after migration
Date: Wed, 29 Mar 2017 16:29:42 +0100	[thread overview]
Message-ID: <20170329152941.GC2501@work-vm> (raw)
In-Reply-To: <09db3476-b1e4-fa80-6d83-d5da0a06bdfc@redhat.com>

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> 
> 
> On 28/03/2017 15:16, Vladimir Sementsov-Ogievskiy wrote:
> > 28.03.2017 15:09, Kevin Wolf wrote:
> >> Am 28.03.2017 um 13:13 hat Dr. David Alan Gilbert geschrieben:
> >>> * Kevin Wolf (kwolf@redhat.com) wrote:
> >>>> Am 28.03.2017 um 12:55 hat Dr. David Alan Gilbert geschrieben:
> >>>>> * Kevin Wolf (kwolf@redhat.com) wrote:
> >>>>>> Am 25.02.2017 um 20:31 hat Vladimir Sementsov-Ogievskiy geschrieben:
> >>>>>>> After migration all drives are inactive and savevm will fail with
> >>>>>>>
> >>>>>>> qemu-kvm: block/io.c:1406: bdrv_co_do_pwritev:
> >>>>>>>     Assertion `!(bs->open_flags & 0x0800)' failed.
> >>>>>>>
> >>>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy
> >>>>>>> <vsementsov@virtuozzo.com>
> >>>>>> What's the exact state you're in? I tried to reproduce this, but just
> >>>>>> doing a live migration and then savevm on the destination works
> >>>>>> fine for
> >>>>>> me.
> >>>>>>
> >>>>>> Hm... Or do you mean on the source? In that case, I think the
> >>>>>> operation
> >>>>>> must fail, but of course more gracefully than now.
> >>>>>>
> >>>>>> Actually, the question that you're asking implicitly here is how the
> >>>>>> source qemu process should be "reactivated" after a failed migration.
> >>>>>> Currently, as far as I know, this is only with issuing a "cont"
> >>>>>> command.
> >>>>>> It might make sense to provide a way to get control without
> >>>>>> resuming the
> >>>>>> VM, but I doubt that adding automatic resume to every QMP command
> >>>>>> is the
> >>>>>> right way to achieve it.
> >>>>>>
> >>>>>> Dave, Juan, what do you think?
> >>>>> I'd only ever really thought of 'cont' or retrying the migration.
> >>>>> However, it does make sense to me that you might want to do a savevm
> >>>>> instead; if you can't migrate then perhaps a savevm is the best you
> >>>>> can do before your machine dies.  Are there any other things that
> >>>>> should be allowed?
> >>>> I think we need to ask the other way round: Any reason _not_ to allow
> >>>> certain operations that you can normally perform on a stopped VM?
> >>>>
> >>>>> We would want to be careful not to accidentally reactivate the disks
> >>>>> on the source after what was actually a succesful migration.
> >>>> Yes, that's exactly my concern, even with savevm. That's why I
> >>>> suggested
> >>>> we could have a 'cont'-like thing that just gets back control of the
> >>>> images and moves into the normal paused state, but doesn't immediately
> >>>> resume the actual VM.
> >>> OK, lets say we had that block-reactivate (for want of a better name),
> >>> how would we stop everything asserting if the user tried to do it
> >>> before they'd run block-reactivate?
> >> We would have to add checks to the monitor commands that assume that the
> >> image is activated and error out if it isn't.
> >>
> >> Maybe just adding the check to blk_is_available() would be enough, but
> >> we'd have to check carefully whether it covers all cases and causes no
> >> false positives.
> >>
> >> By the way, I wouldn't call this 'block-reactivate' because I don't
> >> think this should be a block-specific command. It's a VM lifecycle
> >> command that switches from a postmigrate state (that assumes we have no
> >> control over the VM's resources any more) to a paused state (where we do
> >> have this control). Maybe something like 'migration-abort'.
> > 
> > 'abort' is not very good too I think. migration is completed, nothing to
> > abort.. (may be successful migration to file for suspend, some kind of
> > vm cloning, etc)
> 
> There is already migrate_cancel.  Does it make sense to make it
> reactivate fds if migration is completed?

It's potentially racy to do that.
Imagine if your migration is almost finished and you issue a migrate_cancel,
what happens?
Maybe it cancelled it.
Maybe it just completed in time - and you really better not be accessing
the disks on the source unless you're sure the destination isn't running.

Dave

> Paolo
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

  reply	other threads:[~2017-03-29 15:30 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-25 19:31 [Qemu-devel] [PATCH 0/4] some migration bugs Vladimir Sementsov-Ogievskiy
2017-02-25 19:31 ` [Qemu-devel] [PATCH 1/4] iotests: add migration corner cases test Vladimir Sementsov-Ogievskiy
2017-03-07  9:14   ` Fam Zheng
2017-03-07 11:23   ` Dr. David Alan Gilbert
2017-02-25 19:31 ` [Qemu-devel] [PATCH 2/4] qmp-cont: invalidate on RUN_STATE_PRELAUNCH Vladimir Sementsov-Ogievskiy
2017-03-07  9:19   ` Fam Zheng
2017-03-07 10:02   ` Kevin Wolf
2017-03-07 10:11     ` Vladimir Sementsov-Ogievskiy
2017-03-07 10:22       ` Kevin Wolf
2017-04-26 12:22       ` [Qemu-devel] [Qemu-block] " Kashyap Chamarthy
2017-04-26 13:43         ` Vladimir Sementsov-Ogievskiy
2017-02-25 19:31 ` [Qemu-devel] [PATCH 3/4] savevm: fix savevm after migration Vladimir Sementsov-Ogievskiy
2017-02-27  9:42   ` Denis V. Lunev
2017-03-07  9:53   ` Kevin Wolf
2017-03-07  9:59     ` Vladimir Sementsov-Ogievskiy
2017-03-07 11:01       ` Kevin Wolf
2017-03-07 11:20         ` Dr. David Alan Gilbert
2017-03-28 10:55     ` Dr. David Alan Gilbert
2017-03-28 11:09       ` Kevin Wolf
2017-03-28 11:13         ` Dr. David Alan Gilbert
2017-03-28 12:09           ` Kevin Wolf
2017-03-28 13:16             ` Vladimir Sementsov-Ogievskiy
2017-03-28 14:15               ` Paolo Bonzini
2017-03-29 15:29                 ` Dr. David Alan Gilbert [this message]
2017-03-29 15:53                   ` Paolo Bonzini
2017-04-25 14:22                     ` [Qemu-devel] ping " Vladimir Sementsov-Ogievskiy
2017-03-28 11:18       ` [Qemu-devel] " Denis V. Lunev
2017-02-25 19:31 ` [Qemu-devel] [PATCH 4/4] migration: fix use-after-free of to_dst_file Vladimir Sementsov-Ogievskiy
2017-02-27 10:44   ` Dr. David Alan Gilbert
2017-02-28  9:59   ` Dr. David Alan Gilbert
2017-03-06 14:23 ` [Qemu-devel] [PATCH 0/4] some migration bugs Denis V. Lunev
2017-03-20  9:44   ` [Qemu-devel] ping " Vladimir Sementsov-Ogievskiy

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=20170329152941.GC2501@work-vm \
    --to=dgilbert@redhat.com \
    --cc=armbru@redhat.com \
    --cc=den@openvz.org \
    --cc=eblake@redhat.com \
    --cc=famz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lirans@il.ibm.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.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).