From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51457) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T7Vr2-0001WL-Q9 for qemu-devel@nongnu.org; Fri, 31 Aug 2012 14:22:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T7Vr1-0003Yu-4Y for qemu-devel@nongnu.org; Fri, 31 Aug 2012 14:22:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:27484) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T7Vr0-0003Ym-SI for qemu-devel@nongnu.org; Fri, 31 Aug 2012 14:22:03 -0400 Date: Fri, 31 Aug 2012 13:15:45 -0400 From: Jason Baron Message-ID: <20120831171545.GF12212@redhat.com> References: <201208301800.q7UI04R9009678@int-mx12.intmail.prod.int.phx2.redhat.com> <5040DE81.7090305@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <5040DE81.7090305@suse.de> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] ahci: add migration support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: kwolf@redhat.com, aliguori@us.ibm.com, Juan Quintela , jan.kiszka@siemens.com, agraf@suse.de, qemu-devel@nongnu.org, yamahata@valinux.co.jp, alex.williamson@redhat.com On Fri, Aug 31, 2012 at 05:55:45PM +0200, Andreas F=E4rber wrote: > Am 30.08.2012 20:00, schrieb Jason Baron: > > Add support for ahci migration. This patch builds upon the patches po= sted > > previously by Andreas Faerber: > >=20 > > http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01538.html > >=20 > > (I hope I am giving Andreas proper credit for his work.) >=20 > Not quite. :) You should adopt the Signed-off-by line(s) from the patch > you pick up, add a v3 marker and include a change log to the previous > version(s) below "---" or in the cover letter. A link to previous > discussion threads is then not necessary. The change log would be even > more interesting since this does not seem to be my patch plus your diff > from the link. >=20 > `git commit --amend -s -a` would've even got you my name in UTF-8 the > easy way, assuming previous `git-am my.patch` for testing. >=20 > > I've tested these patches by migrating Windows 7 and Fedora 16 guests= on > > both piix with ahci attached and on q35 (which has a built-in ahci co= ntroller). >=20 > This is good for us to know, but in general sentences with "I" don't > need to go into the commit message; once more people handle it up- and > downstream (submaintainers, committers, stable branches, SLE/RHEL) it > becomes less clear who "I" is. >=20 Ok, I'll fix these things up for the next version. > >=20 > > Signed-off-by: Jason Baron > > --- > > hw/ide/ahci.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++= +++++++++- > > hw/ide/ahci.h | 10 +++++++++ > > hw/ide/ich.c | 11 +++++++-- > > 3 files changed, 81 insertions(+), 4 deletions(-) > >=20 > > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > > index b53c757..e94509b 100644 > > --- a/hw/ide/ahci.c > > +++ b/hw/ide/ahci.c > > @@ -1204,6 +1204,65 @@ void ahci_reset(AHCIState *s) > > } > > } > > =20 > > +static const VMStateDescription vmstate_ahci_device =3D { > > + .name =3D "ahci port", > > + .version_id =3D 1, > > + .fields =3D (VMStateField []) { > > + VMSTATE_IDE_BUS(port, AHCIDevice), > > + VMSTATE_UINT32(port_state, AHCIDevice), > > + VMSTATE_UINT32(finished, AHCIDevice), > > + VMSTATE_UINT32(port_regs.lst_addr, AHCIDevice), > > + VMSTATE_UINT32(port_regs.lst_addr_hi, AHCIDevice), > > + VMSTATE_UINT32(port_regs.fis_addr, AHCIDevice), > > + VMSTATE_UINT32(port_regs.fis_addr_hi, AHCIDevice), > > + VMSTATE_UINT32(port_regs.irq_stat, AHCIDevice), > > + VMSTATE_UINT32(port_regs.irq_mask, AHCIDevice), > > + VMSTATE_UINT32(port_regs.cmd, AHCIDevice), > > + VMSTATE_UINT32(port_regs.tfdata, AHCIDevice), > > + VMSTATE_UINT32(port_regs.sig, AHCIDevice), > > + VMSTATE_UINT32(port_regs.scr_stat, AHCIDevice), > > + VMSTATE_UINT32(port_regs.scr_ctl, AHCIDevice), > > + VMSTATE_UINT32(port_regs.scr_err, AHCIDevice), > > + VMSTATE_UINT32(port_regs.scr_act, AHCIDevice), > > + VMSTATE_UINT32(port_regs.cmd_issue, AHCIDevice), >=20 > Didn't your diff add port_no to this VMSD? Did that turn out > unnecessary? (Did not get around to look into this yet and probably > won't before the release since Kevin considered this 1.3 material.) >=20 Yes, I dropped port_no, since its setup by ahci_init(). > > + VMSTATE_END_OF_LIST() > > + }, > > +}; > > + > > +static int ahci_state_post_load(void *opaque, int version_id) > > +{ > > + int i; > > + AHCIState *s =3D opaque; > > + > > + for (i =3D 0; i < s->ports; i++) { > > + AHCIPortRegs *pr =3D &s->dev[i].port_regs; > > + > > + map_page(&s->dev[i].lst, > > + ((uint64_t)pr->lst_addr_hi << 32) | pr->lst_addr, 1= 024); > > + map_page(&s->dev[i].res_fis, > > + ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr, 2= 56); > > + } > > + > > + return 0; > > +} > > + > > +const VMStateDescription vmstate_ahci =3D { > > + .name =3D "ahci", > > + .version_id =3D 1, > > + .post_load =3D ahci_state_post_load, > > + .fields =3D (VMStateField []) { > > + VMSTATE_STRUCT_VARRAY_POINTER_INT32(dev, AHCIState, ports, > > + vmstate_ahci_device, AHCIDevice= ), >=20 > Where did the declaration of this new macro go? I would expect this to > be a series of two patches, first introducing that (so that Juan can ac= k > that part) and then using it here for ahci. >=20 Right, so my previous patch, had 'VMSTATE_STRUCT_VARRAY_POINTER_UINT32', which if we convert 'ports' back to an int can be, 'VMSTATE_STRUCT_VARRAY_POINTER_INT32', which is already defined. Thanks, -Jason