On 03/28/2016 07:48 AM, Denis V. Lunev wrote: > From: Pavel Butsykin <pbutsy...@virtuozzo.com> > > If the migration occurs after the IDE DMA has been set up but before it > has been initiated, the state gets lost upon save/restore. Specifically, > ->dma_cb callback gets cleared, so, when the guest eventually starts bus > mastering, the DMA never completes, causing the guest to time out the > operation. > > OTOH all the infrastructure is already in place to restart the DMA if > the migration happens while the DMA is in progress. > > So reuse that infrastructure, by setting bus->error_status based on > ->dma_cmd in pre_save if ->dma_cb callback is already set but DMAING is > clear. This will indicate the need for restart and make sure ->dma_cb is > restored in ide_restart_bh(); however since DMAING is clear the state > upon restore will be exactly "ready for DMA" as before the save. > > Signed-off-by: Pavel Butsykin <pbutsy...@virtuozzo.com> > Reviewed-by: Roman Kagan <rka...@virtuozzo.com> > Signed-off-by: Denis V. Lunev <d...@openvz.org> > CC: John Snow <js...@redhat.com> > --- > hw/ide/core.c | 9 +-------- > hw/ide/internal.h | 15 +++++++++++++++ > hw/ide/pci.c | 4 ++++ > 3 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index 241e840..8f86036 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -803,14 +803,7 @@ static void ide_dma_cb(void *opaque, int ret) > return; > } > if (ret < 0) { > - int op = IDE_RETRY_DMA; > - > - if (s->dma_cmd == IDE_DMA_READ) > - op |= IDE_RETRY_READ; > - else if (s->dma_cmd == IDE_DMA_TRIM) > - op |= IDE_RETRY_TRIM; > - > - if (ide_handle_rw_error(s, -ret, op)) { > + if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) { > return; > } > } > diff --git a/hw/ide/internal.h b/hw/ide/internal.h > index 86bde26..68c7d0d 100644 > --- a/hw/ide/internal.h > +++ b/hw/ide/internal.h > @@ -513,6 +513,21 @@ struct IDEDevice { > #define IDE_RETRY_TRIM 0x80 > #define IDE_RETRY_HBA 0x100 > > +static inline uint8_t ide_dma_cmd_to_retry(uint8_t dma_cmd) > +{ > + switch (dma_cmd) { > + case IDE_DMA_READ: > + return IDE_RETRY_DMA | IDE_RETRY_READ; > + case IDE_DMA_WRITE: > + return IDE_RETRY_DMA; > + case IDE_DMA_TRIM: > + return IDE_RETRY_DMA | IDE_RETRY_TRIM; > + default: > + break; > + } > + return 0; > +} > + > static inline IDEState *idebus_active_if(IDEBus *bus) > { > return bus->ifs + bus->unit; > diff --git a/hw/ide/pci.c b/hw/ide/pci.c > index 92ffee7..8d56a00 100644 > --- a/hw/ide/pci.c > +++ b/hw/ide/pci.c > @@ -308,6 +308,10 @@ static void ide_bmdma_pre_save(void *opaque) > BMDMAState *bm = opaque; > uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS; > > + if (!(bm->status & BM_STATUS_DMAING) && bm->dma_cb) { > + bm->bus->error_status = > + ide_dma_cmd_to_retry(bmdma_active_if(bm)->dma_cmd); > + } > bm->migration_retry_unit = bm->bus->retry_unit; > bm->migration_retry_sector_num = bm->bus->retry_sector_num; > bm->migration_retry_nsector = bm->bus->retry_nsector; >
_this_ is the one I meant to R-B, thanks Denis. (Testing the others now, but having some laptop issues. Please stand by.) Reviewed-by: John Snow <js...@redhat.com>