All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Krempa <pkrempa@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, "Denis V. Lunev" <den@openvz.org>,
	Amit Shah <amit.shah@redhat.com>,
	qemu-devel@nongnu.org, Juan Quintela <quintela@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 2/8] qmp: create qmp_savevm command
Date: Tue, 26 Jan 2016 13:56:25 +0100	[thread overview]
Message-ID: <20160126125625.GE191786@andariel.pipo.sk> (raw)
In-Reply-To: <878u3hd41z.fsf@blackfin.pond.sub.org>

[-- Attachment #1: Type: text/plain, Size: 7077 bytes --]

On Fri, Jan 22, 2016 at 16:31:04 +0100, Markus Armbruster wrote:
> "Denis V. Lunev" <den@openvz.org> writes:
> > On 01/19/2016 09:11 PM, Markus Armbruster wrote:
> >> "Denis V. Lunev" <den@openvz.org> writes:

[...]

> >> This would be like HMP savevm with the following differences:
> >>
> >> * Separate parameters for tag and ID.  I'll have none of this
> >>    overloading nonsense in QMP.
> > why do we need both 'name' and 'id' values in the future at all?
> > Why single descriptive parameter is not enough? From my point
> > of view all this overloading is non-sense and in future one
> > name parameter should be kept.
> 
> Ever since we support multiple snapshots (commit faea38e, Aug 2006), a
> snapshot has both an ID and a tag.  We can debate whether that's a good
> idea, but we can't go back in time and change the image formats.
> 
> Monitor commands and qemu-img let you identify a snapshot by ID or by
> tag.  They generally use a single, overloaded parameter, and match it
> against both.  Exception: QMP blockdev-snapshot-delete-internal-sync has
> two unoverloaded parameters, to avoid ambiguity.  Quoting commit
> 44e3e05:
> 
>     This interface use id and name as optional parameters, to handle the
>     case that one image contain multiple snapshots with same name which
>     may be '', but with different id.
>     
>     Adding parameter id is for historical compatiability reason, and
>     that case is not possible in qemu's new interface for internal
>     snapshot at block device level, but still possible in qemu-img.
> 
> Are you proposing to ditch the ability to identify snapshots by ID?  I
> doubt we can.

Speaking for libvirt: We don't care, we use the snapshot name as the
identifier, so ID can be killed from our PoV. Backwards compatibility
can be hard though ...

> 
> Since HMP commands should be build on top of QMP commands, HMP savevm's
> underlying QMP commands cannot be less general than HMP savevm.  HMP
> savevm can do both ID and tag.  Therefore, the underlying QMP commands
> must be able to do that, too.
> 
> Once we accept that, the only question is whether to continue to do that
> with a single overloaded parameter, or two unambigous ones.  For QMP, my
> answer is unambigous.
> 
> >> * Specify the destination block device.  I'll have none of this "pick a
> >>    device in some magic, undocumented way" in QMP.
> > agreed
> >
> >> * Leave alone the other block devices.  Adding transaction actions to
> >>    snapshot all the writable block devices to get a full system snapshot
> >>    is the user's responsibility.
> > this requires clarification:
> > - do you mean that the list of the devices should come from the
> > management layer?
> 
> See below.
> 
> > If so this is error prone at the restoration stage. From my point of
> > view restoration
> > must check that all RW devices are restored properly and there are no missed
> > ones.
> 
> Agreed.
> 
> >       Do you propose to put this into libvirt? From my point of view
> > this flexibility
> > is not that necessary.
> 
> VM snapshot / restore has much in common with migration: both save and
> restore machine state.  Another similarity is that you have to ensure
> identical block device contents.  For migration, this is the management
> layer's job.  My point is: the management layer is already prepared to
> track block device contents.
> 
> For the special case all-internal snapshot / restore, QEMU provides
> convenience features:
> 
> * You can take a consistent, complete, all-internal snapshot with
>   savevm.
> 
> * You can restore all parts of an all-internal snapshot with loadvm.
> 
> * You can delete all parts of an all-internal snapshot with delvm.
> 
> Except "all parts" is a lie: if you manage to lose a block device since
> savevm, QEMU won't notice.  Ensuring you still have them all is left to
> the management layer even there.

If you work only on internal snapshots with libvirt we always make sure
that all parts of the 

> 
> Once external snapshots come into play, QEMU doesn't even try.  It has
> no idea where to find external snapshots.  It's completely left to the
> management layer.

This is still mostly buggy/unimplemented in libvirt although the plan is
to do it properly.

> 
> I'm not fundamentally opposed to convenience features in HMP or even
> QMP.  But I don't want to start with them.  I want us to get the core
> functionality right before we add convenience.

This depends on whether anybody would want to use that interface
manually. As we are speaking about QMP it's not really likely that
anyone would need convenience wrappers, since you need a lot of data and
decisions to do such operation properly anyways.

> 
> I wrote:
> 
>     A point-in-time snapshot of a system consists of a snapshot of its
>     machine state and snapshots of its storage.  All the snapshots need
>     to be made at the same point in time for the result to be
>     consistent.
> 
> and
> 
>     I figure a fully general QMP interface would let you perform a
>     system snapshot by combining storage snapshots of either kind with
>     either kind of machine state snapshot.
> 
> where "either kind" is external or internal.
> 
> If we decide that we'll never need that much flexibility, we can discuss
> restrictions, and how to design a restricted interface.
> 
> For instance, if we decide that we'll never need to mix external and
> internal in the same snapshot, we can discuss how an the all-internal
> and all-external interfaces could look like.
> 
> I'm not the person to decide how much flexibility we'll need.  I'm happy
> to listen to advice.  I'm hoping the management layer guys I cc'ed can
> provide some.

This is really hard to decide. Libvirt currently only allows
full-internal or full-external snapshots. Doing a mixed one is not
really the focus. We need to cover mixed trees of snapshots and their
children first. If we do it properly doing combined snapshot should be
trivial then.

I can't imagine how that would be useful though.

> 
> > There is one important thing missed in your description.
> > Machine snapshot MUST handle configuration change. This means
> > that at the moment of snapshot VM can have different amount of
> > disks and other peripherals and this should be properly handled.
> 
> I'm not sure I understand.
> 
> Configuration changes between which two points of time?  savevm and
> loadvm?
> 
> How should configuraton change be handled?

This is handled in libvirt. If you 'savevm' then do some stuff (e.g.
hot-unplug a disk) and then attempt to loadvm, libvirt detects that the
guest ABI would change and kills qemu and restarts it with the config at
the time of 'savevm' along with loading of the snapshot. When we added
external snapshots the approach may be buggy (we don't detect that the
backing file of a disk has changed yet) but the idea of the
infrastructure already exists.

Peter

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  parent reply	other threads:[~2016-01-26 12:56 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-14 11:28 [Qemu-devel] [PATCH v5 0/8] QMP wrappers for VM snapshot operations Denis V. Lunev
2016-01-14 11:28 ` [Qemu-devel] [PATCH 1/8] migration: split hmp_savevm to migrate_savevm and hmp_savevm wrapper Denis V. Lunev
2016-01-18 15:47   ` Markus Armbruster
2016-01-18 16:00     ` Denis V. Lunev
2016-01-14 11:28 ` [Qemu-devel] [PATCH 2/8] qmp: create qmp_savevm command Denis V. Lunev
2016-01-18 15:58   ` Markus Armbruster
2016-01-18 16:10     ` Denis V. Lunev
2016-01-19 18:11       ` Markus Armbruster
2016-01-22  8:01         ` Denis V. Lunev
2016-01-22 15:31           ` Markus Armbruster
2016-01-22 16:20             ` Denis V. Lunev
2016-01-26 12:56             ` Peter Krempa [this message]
2016-01-26 12:39         ` Peter Krempa
2016-01-14 11:28 ` [Qemu-devel] [PATCH 3/8] qmp: create qmp_delvm command Denis V. Lunev
2016-01-14 11:28 ` [Qemu-devel] [PATCH 4/8] migration: improve error reporting for load_vmstate Denis V. Lunev
2016-01-14 11:28 ` [Qemu-devel] [PATCH 5/8] qmp: create QMP implementation of loadvm command Denis V. Lunev
2016-01-14 11:28 ` [Qemu-devel] [PATCH 6/8] migration, block: better select BDS for VM state loading Denis V. Lunev
2016-05-07 10:45   ` Denis V. Lunev
2016-01-14 11:29 ` [Qemu-devel] [PATCH 7/8] migration, qmp: add optional argument to specify BDS to save VM state to Denis V. Lunev
2016-01-14 11:29 ` [Qemu-devel] [PATCH 8/8] block: allow to skip block driver in selection of one for VM state saving Denis V. Lunev
2016-01-18  9:19   ` [Qemu-devel] [PATCH v6 " Denis V. Lunev

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=20160126125625.GE191786@andariel.pipo.sk \
    --to=pkrempa@redhat.com \
    --cc=amit.shah@redhat.com \
    --cc=armbru@redhat.com \
    --cc=den@openvz.org \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.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 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.