All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Jianjun Duan <duanj@linux.vnet.ibm.com>,
	qemu-devel@nongnu.org, qemu-ppc@nongnu.org, dmitry@daynix.com,
	peter.maydell@linaro.org, kraxel@redhat.com, mst@redhat.com,
	pbonzini@redhat.com, veroniabahaa@gmail.com, quintela@redhat.com,
	amit.shah@redhat.com, mreitz@redhat.com, kwolf@redhat.com,
	rth@twiddle.net, aurelien@aurel32.net, leon.alrae@imgtec.com,
	blauwirbel@gmail.com, mark.cave-ayland@ilande.co.uk,
	mdroth@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [QEMU PATCH v5 1/6] migration: alternative way to set instance_id in SaveStateEntry
Date: Mon, 10 Oct 2016 16:31:20 +1100	[thread overview]
Message-ID: <20161010053120.GG22498@umbus.fritz.box> (raw)
In-Reply-To: <20161007080749.GA2040@work-vm>

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

On Fri, Oct 07, 2016 at 09:07:49AM +0100, Dr. David Alan Gilbert wrote:
> * David Gibson (david@gibson.dropbear.id.au) wrote:
> > On Wed, Oct 05, 2016 at 09:44:57AM -0700, Jianjun Duan wrote:
> > > Please see comments below:
> > > 
> > > On 10/05/2016 03:12 AM, Dr. David Alan Gilbert wrote:
> > > > * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
> > > >> In QOM(QEMU Object Model) migrated objects are identified with instance_id
> > > >> which is calculated automatically using their path in the QOM composition
> > > >> tree. For some objects, this path could change from source to target in
> > > >> migration. To migrate such objects, we need to make sure the instance_id does
> > > >> not change from source to target. We add a hook in DeviceClass to do customized
> > > >> instance_id calculation in such cases.
> > > > 
> > > > Can you explain a bit about why the path changes from source to destination;
> > > > the path here should be a feature of the guest state not the host, and so I
> > > > don't understand why it changes.
> > > Please see the discussion with David in the previous versions:
> > > http://lists.nongnu.org/archive/html/qemu-ppc/2016-06/msg00062.html
> > 
> > Um.. your description above really isn't an accurate summary of that
> > discussion.
> > 
> > The point is not that the qom path will vary from source to
> > destination for some arbitrary reason, but rather that we anticipate
> > future changes in the QOM structure.  Specifically we're considering
> > eliminating the DRC objects, and folding their (limited) state into an
> > array in the parent object (either the machine or a PCI host bridge).
> > 
> > That would change the qom paths, and hence the auto-generated instance
> > ids, which would break migration between qemu versions before and
> > after the restructure.
> > 
> > I'm not sure that changing the instance ids is enough though, anyway,
> > since we're talking about eliminating the object entirely, the
> > class/type information in the migration stream also wouldn't match.
> > 
> > Dave, if you have ideas on how to deal with that, I'd love to hear
> > them
> 
> Eliminating the object entirely would be tricky to deal with;
> allowing the structure to change would work if instead of a custom instance_id
> you had a custom idstr.

Sorry, two misunderstandings here.

  * When I said "structure" I meant in the high-level sense of what
    objects exist and are responsible for what things, not in the
    'struct WhateverState' sense.

  * In fact right now eliminating the objects would be ok, since they
    have no migration state (which causes the problems this series is
    trying to address).  But applying this series which adds migration
    state would make it difficult to eliminate the objects in future.
    That's an awkward constraint given that we've already got some
    hints that these objects are not a good idea.

> However, the question then becomes why is the structure changing so much;
> ideally things in the migration stream should represent some tangible object
> that corresponds to something real, but (again ideally) the contents
> of the stream should reflect the state of those objects not the current
> implementation - so if you want to change the implementation the stream
> doesn't change.  Is it your implementation, or the understanding of what
> the objects actually represent that's in flux?

So, the objects in question are DRCs - "Dynamic Re-configuration
Connector"; silly IBM talk for "a port into which something can be
hotplugged", bascially.  These aren't "real" devices, but rather a
firmware/hypervisor abstraction which are used to describe a hotplug
point.  Each DRC allows either one CPU core, one PCI device, or one
LMB (256MiB chunk of RAM) to be hotplugged (or removed).  The PCI DRCs
are "owned" by the PCI host bridge to which the device would be
connected, the CPU and memory DRCs are owned by the machine.

The state variables which Jianjun Duan is adding to migration are
values defined in the PAPR (hypervisor interface) spec, and so are
tangible enough to be sensible to migrate.  At the moment, each LMB is
represented by a discrete QOM object, but I've been thinking for a
while that this may be a mistake.  In particular it's a problem for
the LMB DRCs - because each LMB is only 256MiB of memory, we end up
with thousands, maybe tens of thousands of DRC objects on a guest with
with large maxmem (even if initial memory is small).  The QOM
infrastructure doesn't deal terribly well that many object child
properties.

The construction of those DRCs used to be an N^3 algorithm, which as
you'd imagine caused horrible startup times with large maxmem (minutes
to hours above ~512GiB).  We fixed it to be only N^2, so now the
startup delays are merely bad (at least once you get to ~2TiB+
maxmem).

We could fix that by changing QOM to use a hash or tree for object
children, instead of a plain list, but I'm not sure if it makes sense
to do that with only this use case.

So, for some time, I've been considering rewriting the DRC stuff to be
implemented via QOM interface on the "owner" object, rather than
separate objects for every single connector.

So the difficulty is that if we add the state migration to the DRC
objects as they stand, we prevent ourselves from doing a refactor
that's probably a good idea.  But no-one's yet had time to actually
thrash out such a refactor. :/

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

  reply	other threads:[~2016-10-10  5:59 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-03 18:24 [Qemu-devel] [QEMU PATCH v5 0/6] migration: ensure hotplug and migration work together Jianjun Duan
2016-10-03 18:24 ` [Qemu-devel] [QEMU PATCH v5 1/6] migration: alternative way to set instance_id in SaveStateEntry Jianjun Duan
2016-10-05 10:12   ` Dr. David Alan Gilbert
2016-10-05 16:44     ` Jianjun Duan
2016-10-07  2:54       ` David Gibson
2016-10-07  8:07         ` Dr. David Alan Gilbert
2016-10-10  5:31           ` David Gibson [this message]
2016-10-11 16:17             ` Michael Roth
2016-10-11 23:37               ` David Gibson
2016-11-15 23:45         ` Michael Roth
2016-10-05 16:46     ` [Qemu-devel] [Qemu-ppc] " Jianjun Duan
2016-10-03 18:24 ` [Qemu-devel] [QEMU PATCH v5 2/6] migration: spapr_drc: defined VMStateDescription struct Jianjun Duan
2016-10-05 11:38   ` Dr. David Alan Gilbert
2016-10-07  3:17     ` David Gibson
2016-10-07  3:12   ` David Gibson
2016-10-07 17:17     ` Jianjun Duan
2016-10-10  5:09       ` David Gibson
2016-10-10 16:48         ` [Qemu-devel] [Qemu-ppc] " Jianjun Duan
2016-10-03 18:24 ` [Qemu-devel] [QEMU PATCH v5 3/6] migration: extend VMStateInfo Jianjun Duan
2016-10-07 12:08   ` Dr. David Alan Gilbert
2016-10-07 16:35     ` [Qemu-devel] [Qemu-ppc] " Jianjun Duan
2016-10-07 18:42       ` Dr. David Alan Gilbert
2016-10-10  5:02         ` David Gibson
2016-10-12 11:59   ` [Qemu-devel] " Halil Pasic
2016-10-12 12:07     ` Paolo Bonzini
2016-10-12 12:30       ` Halil Pasic
2016-10-12 14:59         ` Dr. David Alan Gilbert
2016-10-13 10:33           ` Halil Pasic
2016-10-13 11:12             ` Dr. David Alan Gilbert
2016-10-12 17:27       ` [Qemu-devel] [Qemu-ppc] " Jianjun Duan
2016-10-13  8:22         ` Paolo Bonzini
2016-10-13 10:48           ` Halil Pasic
2016-10-13 11:20             ` Paolo Bonzini
2016-10-13 16:23             ` Jianjun Duan
2016-10-13 16:32               ` Halil Pasic
2016-10-13 16:35                 ` Jianjun Duan
2016-10-03 18:24 ` [Qemu-devel] [QEMU PATCH v5 4/6] migration: migrate QTAILQ Jianjun Duan
2016-10-05 16:56   ` Dr. David Alan Gilbert
2016-10-05 17:19     ` Jianjun Duan
2016-10-06 19:01       ` Dr. David Alan Gilbert
2016-10-06 19:49         ` Jianjun Duan
2016-10-07  3:25         ` David Gibson
2016-10-07 14:31         ` Paolo Bonzini
2016-10-07 14:34           ` Dr. David Alan Gilbert
2016-10-07 16:31             ` [Qemu-devel] [Qemu-ppc] " Jianjun Duan
2016-10-07 16:32               ` Paolo Bonzini
2016-10-07 17:25                 ` Jianjun Duan
2016-10-07 17:34                   ` Dr. David Alan Gilbert
2016-10-07 17:43                     ` Jianjun Duan
2016-10-08 11:37                       ` Paolo Bonzini
2016-10-08 19:28                         ` Halil Pasic
2016-10-10 21:29                           ` Jianjun Duan
2016-10-11  7:33                             ` Paolo Bonzini
2016-10-10 21:40                           ` Jianjun Duan
2016-10-06 11:05     ` [Qemu-devel] " Paolo Bonzini
2016-10-06 11:56       ` Dr. David Alan Gilbert
2016-10-06 12:23         ` Paolo Bonzini
2016-10-06 15:21           ` Dr. David Alan Gilbert
2016-10-03 18:24 ` [Qemu-devel] [QEMU PATCH v5 5/6] migration: spapr: migrate ccs_list in spapr state Jianjun Duan
2016-10-07  3:36   ` David Gibson
2016-10-07 14:52     ` Michael Roth
2016-10-10  5:05       ` David Gibson
2016-10-03 18:24 ` [Qemu-devel] [QEMU PATCH v5 6/6] migration: spapr: migrate pending_events of " Jianjun Duan
2016-10-03 18:35 ` [Qemu-devel] [QEMU PATCH v5 0/6] migration: ensure hotplug and migration work together no-reply
2016-10-03 19:00 ` no-reply
2016-10-03 19:11 ` Jianjun Duan

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=20161010053120.GG22498@umbus.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=amit.shah@redhat.com \
    --cc=aurelien@aurel32.net \
    --cc=blauwirbel@gmail.com \
    --cc=dgilbert@redhat.com \
    --cc=dmitry@daynix.com \
    --cc=duanj@linux.vnet.ibm.com \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=leon.alrae@imgtec.com \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mreitz@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=rth@twiddle.net \
    --cc=veroniabahaa@gmail.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.