Re: [Qemu-devel] [PATCH] ahci: add migration support
works fine here with debian squeeze + debian wheezy guests.
- Mail original -
De: Jason Baron jba...@redhat.com
À: kw...@redhat.com, afaer...@suse.de, ag...@suse.de
Cc: qemu-devel@nongnu.org, yamah...@valinux.co.jp, alex williamson
alex.william...@redhat.com, aligu...@us.ibm.com, jan kiszka
jan.kis...@siemens.com
Envoyé: Jeudi 30 Août 2012 20:00:04
Objet: [Qemu-devel] [PATCH] ahci: add migration support
Add support for ahci migration. This patch builds upon the patches posted
previously by Andreas Faerber:
http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01538.html
(I hope I am giving Andreas proper credit for his work.)
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 controller).
Signed-off-by: Jason Baron jba...@redhat.com
---
hw/ide/ahci.c | 64 -
hw/ide/ahci.h | 10 +
hw/ide/ich.c | 11 +++--
3 files changed, 81 insertions(+), 4 deletions(-)
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)
}
}
+static const VMStateDescription vmstate_ahci_device = {
+ .name = ahci port,
+ .version_id = 1,
+ .fields = (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),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
+static int ahci_state_post_load(void *opaque, int version_id)
+{
+ int i;
+ AHCIState *s = opaque;
+
+ for (i = 0; i s-ports; i++) {
+ AHCIPortRegs *pr = s-dev[i].port_regs;
+
+ map_page(s-dev[i].lst,
+ ((uint64_t)pr-lst_addr_hi 32) | pr-lst_addr, 1024);
+ map_page(s-dev[i].res_fis,
+ ((uint64_t)pr-fis_addr_hi 32) | pr-fis_addr, 256);
+ }
+
+ return 0;
+}
+
+const VMStateDescription vmstate_ahci = {
+ .name = ahci,
+ .version_id = 1,
+ .post_load = ahci_state_post_load,
+ .fields = (VMStateField []) {
+ VMSTATE_STRUCT_VARRAY_POINTER_INT32(dev, AHCIState, ports,
+ vmstate_ahci_device, AHCIDevice),
+ VMSTATE_UINT32(control_regs.cap, AHCIState),
+ VMSTATE_UINT32(control_regs.ghc, AHCIState),
+ VMSTATE_UINT32(control_regs.irqstatus, AHCIState),
+ VMSTATE_UINT32(control_regs.impl, AHCIState),
+ VMSTATE_UINT32(control_regs.version, AHCIState),
+ VMSTATE_UINT32(idp_index, AHCIState),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
typedef struct SysbusAHCIState {
SysBusDevice busdev;
AHCIState ahci;
@@ -1212,7 +1271,10 @@ typedef struct SysbusAHCIState {
static const VMStateDescription vmstate_sysbus_ahci = {
.name = sysbus-ahci,
- .unmigratable = 1,
+ .fields = (VMStateField []) {
+ VMSTATE_AHCI(ahci, AHCIPCIState),
+ VMSTATE_END_OF_LIST()
+ },
};
static void sysbus_ahci_reset(DeviceState *dev)
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index 1200a56..7719dbf 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -307,6 +307,16 @@ typedef struct AHCIPCIState {
AHCIState ahci;
} AHCIPCIState;
+extern const VMStateDescription vmstate_ahci;
+
+#define VMSTATE_AHCI(_field, _state) { \
+ .name = (stringify(_field)), \
+ .size = sizeof(AHCIState), \
+ .vmsd = vmstate_ahci, \
+ .flags = VMS_STRUCT, \
+ .offset = vmstate_offset_value(_state, _field, AHCIState), \
+}
+
typedef struct NCQFrame {
uint8_t fis_type;
uint8_t c;
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 272b773..ae6f56f 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -79,9 +79,14 @@
#define ICH9_IDP_INDEX 0x10
#define ICH9_IDP_INDEX_LOG2 0x04
-static const VMStateDescription vmstate_ahci = {
+static const VMStateDescription vmstate_ich9_ahci = {
.name = ahci,
- .unmigratable = 1,
+ .version_id = 1,
+ .fields = (VMStateField []) {
+ VMSTATE_PCI_DEVICE(card, AHCIPCIState),
+ VMSTATE_AHCI(ahci, AHCIPCIState),
+ VMSTATE_END_OF_LIST()
+ },
};
static void pci_ich9_reset(DeviceState *dev)
@@ -152,7 +157,7 @@ static void ich_ahci_class_init(ObjectClass *klass, void
*data)
k-device_id = PCI_DEVICE_ID_INTEL_82801IR;
k-revision = 0x02;
k-class_id = PCI_CLASS_STORAGE_SATA;
- dc-vmsd = vmstate_ahci;
+ dc-vmsd = vmstate_ich9_ahci;
dc-reset = pci_ich9_reset;
}
--
1.7.1
--
--
Alexandre D e rumier
Ingénieur Systèmes et Réseaux
Fixe : 03 20 68 88 85
Fax : 03 20 68 90 88
45 Bvd du Général Leclerc 59100 Roubaix
12 rue
Re: [Qemu-devel] [PATCH] ahci: add migration support
Am 30.08.2012 20:00, schrieb Jason Baron:
Add support for ahci migration. This patch builds upon the patches posted
previously by Andreas Faerber:
http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01538.html
(I hope I am giving Andreas proper credit for his work.)
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
controller).
Signed-off-by: Jason Baron jba...@redhat.com
---
hw/ide/ahci.c | 64
-
hw/ide/ahci.h | 10 +
hw/ide/ich.c | 11 +++--
3 files changed, 81 insertions(+), 4 deletions(-)
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)
}
}
+static const VMStateDescription vmstate_ahci_device = {
+.name = ahci port,
+.version_id = 1,
+.fields = (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),
+VMSTATE_END_OF_LIST()
+},
+};
This looks incomplete to me. Everything below port_regs in AHCIDevice is
missing from the saved fields:
struct AHCIDevice {
IDEDMA dma;
IDEBus port;
int port_no;
uint32_t port_state;
uint32_t finished;
AHCIPortRegs port_regs;
struct AHCIState *hba;
QEMUBH *check_bh;
uint8_t *lst;
uint8_t *res_fis;
int dma_status;
int done_atapi_packet;
int busy_slot;
int init_d2h_sent;
BlockDriverCompletionFunc *dma_cb;
AHCICmdHdr *cur_cmd;
NCQTransferState ncq_tfs[AHCI_MAX_CMDS];
};
I haven't checked each single one, but things like done_atapi_packet or
init_d2h_sent certainly look as if they should be migrated.
Hm... Could we possibly test migration with qtest? I can imagine some
nice test cases there. It would require some new infrastructure, I
guess, but it could be doable.
Kevin
Re: [Qemu-devel] [PATCH] ahci: add migration support
On Fri, Aug 31, 2012 at 04:47:37PM +0200, Kevin Wolf wrote:
Am 30.08.2012 20:00, schrieb Jason Baron:
Add support for ahci migration. This patch builds upon the patches posted
previously by Andreas Faerber:
http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01538.html
(I hope I am giving Andreas proper credit for his work.)
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
controller).
Signed-off-by: Jason Baron jba...@redhat.com
---
hw/ide/ahci.c | 64
-
hw/ide/ahci.h | 10 +
hw/ide/ich.c | 11 +++--
3 files changed, 81 insertions(+), 4 deletions(-)
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)
}
}
+static const VMStateDescription vmstate_ahci_device = {
+.name = ahci port,
+.version_id = 1,
+.fields = (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),
+VMSTATE_END_OF_LIST()
+},
+};
This looks incomplete to me. Everything below port_regs in AHCIDevice is
missing from the saved fields:
struct AHCIDevice {
IDEDMA dma;
IDEBus port;
int port_no;
uint32_t port_state;
uint32_t finished;
AHCIPortRegs port_regs;
struct AHCIState *hba;
set up in achi_init(), so I don't think we need this.
QEMUBH *check_bh;
indicates if there is outstanding bh. Could cause a crash if there is an
outstanding bh, and we don't have this field set. We could add a field
to indicate if a bh is outstanding, cancel it in a pre_save, and then
re-enable it during restore. Is i/o quiesced in any any way for
migration? It also doesn't seem like other migration code paths are
preserving the bh, for example, vmstate_bmdma, doesn't appear to save
BMDMAState-bh?
uint8_t *lst;
uint8_t *res_fis;
These map h/w addresses into qemu memory space, and are handled by
'ahci_state_post_load()' that I included. So not needed.
int dma_status;
int done_atapi_packet;
int busy_slot;
int init_d2h_sent;
These could easily be saved.
BlockDriverCompletionFunc *dma_cb;
Not sure we even need this field in ahci?
AHCICmdHdr *cur_cmd;
Could be restored in a post save if we store the outstanding current
command slot number, maybe via 'busy_slot', then this field is set by
offsetting the solot number on the 'lst' field.
NCQTransferState ncq_tfs[AHCI_MAX_CMDS];
This one would require a new VMStateDescription. Again I'd like to
better understand how to think about i/o quiesce across a migration. Can
we make any assumptions about i/o being flushed out?
};
I haven't checked each single one, but things like done_atapi_packet or
init_d2h_sent certainly look as if they should be migrated.
Hm... Could we possibly test migration with qtest? I can imagine some
nice test cases there. It would require some new infrastructure, I
guess, but it could be doable.
That would be nice to shake out any bugs here.
Thanks,
-Jason
Re: [Qemu-devel] [PATCH] ahci: add migration support
Am 30.08.2012 20:00, schrieb Jason Baron:
Add support for ahci migration. This patch builds upon the patches posted
previously by Andreas Faerber:
http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01538.html
(I hope I am giving Andreas proper credit for his work.)
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.
`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.
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
controller).
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.
Signed-off-by: Jason Baron jba...@redhat.com
---
hw/ide/ahci.c | 64
-
hw/ide/ahci.h | 10 +
hw/ide/ich.c | 11 +++--
3 files changed, 81 insertions(+), 4 deletions(-)
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)
}
}
+static const VMStateDescription vmstate_ahci_device = {
+.name = ahci port,
+.version_id = 1,
+.fields = (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),
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.)
+VMSTATE_END_OF_LIST()
+},
+};
+
+static int ahci_state_post_load(void *opaque, int version_id)
+{
+int i;
+AHCIState *s = opaque;
+
+for (i = 0; i s-ports; i++) {
+AHCIPortRegs *pr = s-dev[i].port_regs;
+
+map_page(s-dev[i].lst,
+ ((uint64_t)pr-lst_addr_hi 32) | pr-lst_addr, 1024);
+map_page(s-dev[i].res_fis,
+ ((uint64_t)pr-fis_addr_hi 32) | pr-fis_addr, 256);
+}
+
+return 0;
+}
+
+const VMStateDescription vmstate_ahci = {
+.name = ahci,
+.version_id = 1,
+.post_load = ahci_state_post_load,
+.fields = (VMStateField []) {
+VMSTATE_STRUCT_VARRAY_POINTER_INT32(dev, AHCIState, ports,
+ vmstate_ahci_device, AHCIDevice),
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 ack
that part) and then using it here for ahci.
Regards,
Andreas
+VMSTATE_UINT32(control_regs.cap, AHCIState),
+VMSTATE_UINT32(control_regs.ghc, AHCIState),
+VMSTATE_UINT32(control_regs.irqstatus, AHCIState),
+VMSTATE_UINT32(control_regs.impl, AHCIState),
+VMSTATE_UINT32(control_regs.version, AHCIState),
+VMSTATE_UINT32(idp_index, AHCIState),
+VMSTATE_END_OF_LIST()
+},
+};
+
typedef struct SysbusAHCIState {
SysBusDevice busdev;
AHCIState ahci;
@@ -1212,7 +1271,10 @@ typedef struct SysbusAHCIState {
static const VMStateDescription vmstate_sysbus_ahci = {
.name = sysbus-ahci,
-.unmigratable = 1,
+.fields = (VMStateField []) {
+VMSTATE_AHCI(ahci, AHCIPCIState),
+VMSTATE_END_OF_LIST()
+},
};
static void sysbus_ahci_reset(DeviceState *dev)
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index 1200a56..7719dbf 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -307,6 +307,16 @@ typedef struct AHCIPCIState {
AHCIState ahci;
} AHCIPCIState;
+extern const VMStateDescription vmstate_ahci;
+
Re: [Qemu-devel] [PATCH] ahci: add migration support
Am 31.08.2012 17:41, schrieb Jason Baron:
On Fri, Aug 31, 2012 at 04:47:37PM +0200, Kevin Wolf wrote:
Am 30.08.2012 20:00, schrieb Jason Baron:
Add support for ahci migration. This patch builds upon the patches posted
previously by Andreas Faerber:
http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01538.html
(I hope I am giving Andreas proper credit for his work.)
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
controller).
Signed-off-by: Jason Baron jba...@redhat.com
---
hw/ide/ahci.c | 64
-
hw/ide/ahci.h | 10 +
hw/ide/ich.c | 11 +++--
3 files changed, 81 insertions(+), 4 deletions(-)
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)
}
}
+static const VMStateDescription vmstate_ahci_device = {
+.name = ahci port,
+.version_id = 1,
+.fields = (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),
+VMSTATE_END_OF_LIST()
+},
+};
This looks incomplete to me. Everything below port_regs in AHCIDevice is
missing from the saved fields:
struct AHCIDevice {
IDEDMA dma;
IDEBus port;
int port_no;
uint32_t port_state;
uint32_t finished;
AHCIPortRegs port_regs;
struct AHCIState *hba;
set up in achi_init(), so I don't think we need this.
QEMUBH *check_bh;
indicates if there is outstanding bh. Could cause a crash if there is an
outstanding bh, and we don't have this field set. We could add a field
to indicate if a bh is outstanding, cancel it in a pre_save, and then
re-enable it during restore. Is i/o quiesced in any any way for
migration? It also doesn't seem like other migration code paths are
preserving the bh, for example, vmstate_bmdma, doesn't appear to save
BMDMAState-bh?
Before sending the VM state, the migration code calls bdrv_drain_all(),
so we can be sure that no requests are in flight any more in the block
layer. There could still be requests pending in the AHCI or IDE code if
they aren't submitted instantly or just for resubmission after an I/O error.
int dma_status;
int done_atapi_packet;
int busy_slot;
int init_d2h_sent;
These could easily be saved.
dma_status looks completely unused, it is never read. Can probably
remove it from the struct.
BlockDriverCompletionFunc *dma_cb;
Not sure we even need this field in ahci?
Looks unused indeed.
Hm... Could we possibly test migration with qtest? I can imagine some
nice test cases there. It would require some new infrastructure, I
guess, but it could be doable.
That would be nice to shake out any bugs here.
Can you give it a try?
Kevin
Re: [Qemu-devel] [PATCH] ahci: add migration support
On Fri, Aug 31, 2012 at 05:55:45PM +0200, Andreas Färber wrote:
Am 30.08.2012 20:00, schrieb Jason Baron:
Add support for ahci migration. This patch builds upon the patches posted
previously by Andreas Faerber:
http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01538.html
(I hope I am giving Andreas proper credit for his work.)
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.
`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.
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
controller).
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.
Ok, I'll fix these things up for the next version.
Signed-off-by: Jason Baron jba...@redhat.com
---
hw/ide/ahci.c | 64
-
hw/ide/ahci.h | 10 +
hw/ide/ich.c | 11 +++--
3 files changed, 81 insertions(+), 4 deletions(-)
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)
}
}
+static const VMStateDescription vmstate_ahci_device = {
+.name = ahci port,
+.version_id = 1,
+.fields = (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),
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.)
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 = opaque;
+
+for (i = 0; i s-ports; i++) {
+AHCIPortRegs *pr = s-dev[i].port_regs;
+
+map_page(s-dev[i].lst,
+ ((uint64_t)pr-lst_addr_hi 32) | pr-lst_addr, 1024);
+map_page(s-dev[i].res_fis,
+ ((uint64_t)pr-fis_addr_hi 32) | pr-fis_addr, 256);
+}
+
+return 0;
+}
+
+const VMStateDescription vmstate_ahci = {
+.name = ahci,
+.version_id = 1,
+.post_load = ahci_state_post_load,
+.fields = (VMStateField []) {
+VMSTATE_STRUCT_VARRAY_POINTER_INT32(dev, AHCIState, ports,
+ vmstate_ahci_device, AHCIDevice),
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 ack
that part) and then using it here for ahci.
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
