From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57346) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYRvg-0001j0-UT for qemu-devel@nongnu.org; Thu, 19 Mar 2015 00:19:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YYRvf-0008Su-DF for qemu-devel@nongnu.org; Thu, 19 Mar 2015 00:19:32 -0400 Received: from ozlabs.org ([2401:3900:2:1::2]:43625) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYRve-0008Sm-Pe for qemu-devel@nongnu.org; Thu, 19 Mar 2015 00:19:31 -0400 Date: Thu, 19 Mar 2015 15:18:30 +1100 From: David Gibson Message-ID: <20150319041830.GU5741@voom.redhat.com> References: <1424883128-9841-1-git-send-email-dgilbert@redhat.com> <1424883128-9841-24-git-send-email-dgilbert@redhat.com> <20150313012652.GB11973@voom.redhat.com> <20150313111905.GC2486@work-vm> <20150316062355.GG5741@voom.redhat.com> <20150318175951.GL2355@work-vm> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="OtP8TOIcu2/MSuWQ" Content-Disposition: inline In-Reply-To: <20150318175951.GL2355@work-vm> Subject: Re: [Qemu-devel] [PATCH v5 23/45] migrate_start_postcopy: Command to trigger transition to postcopy List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, quintela@redhat.com, qemu-devel@nongnu.org, amit.shah@redhat.com, pbonzini@redhat.com, yanghy@cn.fujitsu.com --OtP8TOIcu2/MSuWQ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 18, 2015 at 05:59:51PM +0000, Dr. David Alan Gilbert wrote: > * David Gibson (david@gibson.dropbear.id.au) wrote: > > On Fri, Mar 13, 2015 at 11:19:06AM +0000, Dr. David Alan Gilbert wrote: > > > * David Gibson (david@gibson.dropbear.id.au) wrote: > > > > On Wed, Feb 25, 2015 at 04:51:46PM +0000, Dr. David Alan Gilbert (g= it) wrote: > > > > > From: "Dr. David Alan Gilbert" > > > > >=20 > > > > > Once postcopy is enabled (with migrate_set_capability), the migra= tion > > > > > will still start on precopy mode. To cause a transition into pos= tcopy > > > > > the: > > > > >=20 > > > > > migrate_start_postcopy > > > > >=20 > > > > > command must be issued. Postcopy will start sometime after this > > > > > (when it's next checked in the migration loop). > > > > >=20 > > > > > Issuing the command before migration has started will error, > > > > > and issuing after it has finished is ignored. > > > > >=20 > > > > > Signed-off-by: Dr. David Alan Gilbert > > > > > Reviewed-by: Eric Blake > > > > > --- > > > > > hmp-commands.hx | 15 +++++++++++++++ > > > > > hmp.c | 7 +++++++ > > > > > hmp.h | 1 + > > > > > include/migration/migration.h | 3 +++ > > > > > migration/migration.c | 22 ++++++++++++++++++++++ > > > > > qapi-schema.json | 8 ++++++++ > > > > > qmp-commands.hx | 19 +++++++++++++++++++ > > > > > 7 files changed, 75 insertions(+) > > > > >=20 > > > > > diff --git a/hmp-commands.hx b/hmp-commands.hx > > > > > index e37bc8b..03b8b78 100644 > > > > > --- a/hmp-commands.hx > > > > > +++ b/hmp-commands.hx > > > > > @@ -985,6 +985,21 @@ Enable/Disable the usage of a capability @va= r{capability} for migration. > > > > > ETEXI > > > > > =20 > > > > > { > > > > > + .name =3D "migrate_start_postcopy", > > > > > + .args_type =3D "", > > > > > + .params =3D "", > > > > > + .help =3D "Switch migration to postcopy mode", > > > > > + .mhandler.cmd =3D hmp_migrate_start_postcopy, > > > > > + }, > > > > > + > > > > > +STEXI > > > > > +@item migrate_start_postcopy > > > > > +@findex migrate_start_postcopy > > > > > +Switch in-progress migration to postcopy mode. Ignored after the= end of > > > > > +migration (or once already in postcopy). > > > > > +ETEXI > > > > > + > > > > > + { > > > > > .name =3D "client_migrate_info", > > > > > .args_type =3D "protocol:s,hostname:s,port:i?,tls-port:= i?,cert-subject:s?", > > > > > .params =3D "protocol hostname port tls-port cert-su= bject", > > > > > diff --git a/hmp.c b/hmp.c > > > > > index b47f331..df9736c 100644 > > > > > --- a/hmp.c > > > > > +++ b/hmp.c > > > > > @@ -1140,6 +1140,13 @@ void hmp_migrate_set_capability(Monitor *m= on, const QDict *qdict) > > > > > } > > > > > } > > > > > =20 > > > > > +void hmp_migrate_start_postcopy(Monitor *mon, const QDict *qdict) > > > > > +{ > > > > > + Error *err =3D NULL; > > > > > + qmp_migrate_start_postcopy(&err); > > > > > + hmp_handle_error(mon, &err); > > > > > +} > > > > > + > > > > > void hmp_set_password(Monitor *mon, const QDict *qdict) > > > > > { > > > > > const char *protocol =3D qdict_get_str(qdict, "protocol"); > > > > > diff --git a/hmp.h b/hmp.h > > > > > index 4bb5dca..da1334f 100644 > > > > > --- a/hmp.h > > > > > +++ b/hmp.h > > > > > @@ -64,6 +64,7 @@ void hmp_migrate_set_downtime(Monitor *mon, con= st QDict *qdict); > > > > > void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict); > > > > > void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict= ); > > > > > void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict= ); > > > > > +void hmp_migrate_start_postcopy(Monitor *mon, const QDict *qdict= ); > > > > > void hmp_set_password(Monitor *mon, const QDict *qdict); > > > > > void hmp_expire_password(Monitor *mon, const QDict *qdict); > > > > > void hmp_eject(Monitor *mon, const QDict *qdict); > > > > > diff --git a/include/migration/migration.h b/include/migration/mi= gration.h > > > > > index e6a814a..293c83e 100644 > > > > > --- a/include/migration/migration.h > > > > > +++ b/include/migration/migration.h > > > > > @@ -104,6 +104,9 @@ struct MigrationState > > > > > int64_t xbzrle_cache_size; > > > > > int64_t setup_time; > > > > > int64_t dirty_sync_count; > > > > > + > > > > > + /* Flag set once the migration has been asked to enter postc= opy */ > > > > > + bool start_postcopy; > > > > > }; > > > > > =20 > > > > > void process_incoming_migration(QEMUFile *f); > > > > > diff --git a/migration/migration.c b/migration/migration.c > > > > > index a4fc7d7..43ca656 100644 > > > > > --- a/migration/migration.c > > > > > +++ b/migration/migration.c > > > > > @@ -372,6 +372,28 @@ void qmp_migrate_set_capabilities(MigrationC= apabilityStatusList *params, > > > > > } > > > > > } > > > > > =20 > > > > > +void qmp_migrate_start_postcopy(Error **errp) > > > > > +{ > > > > > + MigrationState *s =3D migrate_get_current(); > > > > > + > > > > > + if (!migrate_postcopy_ram()) { > > > > > + error_setg(errp, "Enable postcopy with migration_set_cap= ability before" > > > > > + " the start of migration"); > > > > > + return; > > > > > + } > > > > > + > > > > > + if (s->state =3D=3D MIG_STATE_NONE) { > > > > > + error_setg(errp, "Postcopy must be started after migrati= on has been" > > > > > + " started"); > > > > > + return; > > > > > + } > > > > > + /* > > > > > + * we don't error if migration has finished since that would= be racy > > > > > + * with issuing this command. > > > > > + */ > > > > > + atomic_set(&s->start_postcopy, true); > > > >=20 > > > > Why atomic_set? > > >=20 > > > It's being read by the migration thread, this is happening in the mai= n thread. > > >=20 > > > There's no strict ordering requirement or anything. > >=20 > > I don't think you need an atomic then. AFAIK an atomic_set() in > > isolation without some sort of atomic on the other side is pretty much > > meaningless. >=20 > The other side has an atomic_read: >=20 > if (migrate_postcopy_ram() && > s->state !=3D MIGRATION_STATUS_POSTCOPY_ACTIVE && > pend_nonpost <=3D max_size && > atomic_read(&s->start_postcopy)) { >=20 > if (!postcopy_start(s, &old_vm_running)) { > current_active_type =3D MIGRATION_STATUS_POSTCOPY= _ACTIVE; > entered_postcopy =3D true; > } >=20 > so it is at least symmetric. But still pointless. Atomicity isn't magic pixie dust; it only makes sense if you're making atomic specific operations that need to be. Simple integer loads and stores are already atomic. Unless at least some of the atomic operations are something more complex, there's really no point to atomically marked operations. --=20 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 --OtP8TOIcu2/MSuWQ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVCk4WAAoJEGw4ysog2bOSzakQALyjtb8v13AjNNsW/hqtuUVN fYV4yc7tU5e+nMIfUq1NPglBR9cs3P/l4/uYTTymfJkAXiz6Q6NSma4Gu6o8Gc77 Sv4t4mij4kOWn+4tCRxYX6sz7zsXatu0nHRbZfbZZLIuC8ExzVoru9cAuOnoGjIv gEtoNXwvLli7aAA/Gc7eiEGPO01rrwE9rRdOSw7eoqk/02O7P5cEevH1bPyEzUX5 kZveq6BJ6DspqKQlYm4pXLuNhDh38jEVSJfoim05Mbk5Q3TCmrOMVhmDe56HXRDR ZsE0qVds90E66Qy72mOScrP3RlZjSsQrJE81y0RTzbt0vRswuQLLO4u/dQEzaiQv 90bmPr3cgSBWmJYUIZrXr5wgaluPXqr3A7KCeoiwnOiEx9sHobOBR+7KFyS4JGag Gl4MjYTIV1gPNHpP7ywtGQffwa+kToOCmp+VfZGGZZxoJovGgUXDuz+IfW7nE2fI J7YYHGjsyA7TSOFp+iP1R2HK11T14ZdhUMoC39p1CAPRg3+zGYETJ6gnogsFA4ev V/rjX8uzwb5heacmfg8X1mszMgXDnCOO3hoURYmYLwSdvIQ+e4ahfoPJfDTC1dJE bOGy8t/83lMa8bTS1XcyWLWXKNH/4uFcdEdmbUBZN6sUweSIFloeG24v5uip/L0F JZFFdEBi3V7Nw/QTk1u2 =9bXY -----END PGP SIGNATURE----- --OtP8TOIcu2/MSuWQ--