Blame SOURCES/kvm-atapi-migration-Throw-recoverable-error-to-avoid-rec.patch

05bba0
From e44bfb41173183a85bb6fa94a6f48486ac4ab0a2 Mon Sep 17 00:00:00 2001
05bba0
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
05bba0
Date: Tue, 10 Feb 2015 11:45:36 +0100
05bba0
Subject: [PATCH 10/16] atapi migration: Throw recoverable error to avoid
05bba0
 recovery
05bba0
05bba0
Message-id: <1423568736-19538-3-git-send-email-dgilbert@redhat.com>
05bba0
Patchwork-id: 63779
05bba0
O-Subject: [RHEL-7.2 qemu-kvm PATCH 2/2] atapi migration: Throw recoverable error to avoid recovery
05bba0
Bugzilla: 892258
05bba0
RH-Acked-by: Juan Quintela <quintela@redhat.com>
05bba0
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
05bba0
RH-Acked-by: John Snow <jsnow@redhat.com>
05bba0
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
05bba0
05bba0
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
05bba0
05bba0
(With the previous atapi_dma flag recovery)
05bba0
If migration happens between the ATAPI command being written and the
05bba0
bmdma being started, the DMA is dropped.  Eventually the guest times
05bba0
out and recovers, but that can take many seconds.
05bba0
(This is rare, on a pingpong reading the CD continuously I hit
05bba0
this about ~1/30-1/50 migrates)
05bba0
05bba0
I don't think we've got enough state to be able to recover safely
05bba0
at this point, so I throw a 'medium error, no seek complete'
05bba0
that I'm assuming guests will try and recover from an apparently
05bba0
dirty CD.
05bba0
05bba0
OK, it's a hack, the real solution is probably to push a lot of
05bba0
ATAPI state into the migration stream, but this is a fix that
05bba0
works with no stream changes. Tested only on Linux (both RHEL5
05bba0
(pre-libata) and RHEL7).
05bba0
05bba0
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
05bba0
Reviewed-by: John Snow <jsnow@redhat.com>
05bba0
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
05bba0
(cherry picked from commit a71754e5b03fd3b8b8c6d3bc2a39f75bead729de)
05bba0
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
05bba0
---
05bba0
 hw/ide/atapi.c    | 17 +++++++++++++++++
05bba0
 hw/ide/internal.h |  2 ++
05bba0
 hw/ide/pci.c      | 11 +++++++++++
05bba0
 3 files changed, 30 insertions(+)
05bba0
05bba0
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
05bba0
index 05e60b1..46a2c26 100644
05bba0
--- a/hw/ide/atapi.c
05bba0
+++ b/hw/ide/atapi.c
05bba0
@@ -393,6 +393,23 @@ static void ide_atapi_cmd_read(IDEState *s, int lba, int nb_sectors,
05bba0
     }
05bba0
 }
05bba0
 
05bba0
+
05bba0
+/* Called by *_restart_bh when the transfer function points
05bba0
+ * to ide_atapi_cmd
05bba0
+ */
05bba0
+void ide_atapi_dma_restart(IDEState *s)
05bba0
+{
05bba0
+    /*
05bba0
+     * I'm not sure we have enough stored to restart the command
05bba0
+     * safely, so give the guest an error it should recover from.
05bba0
+     * I'm assuming most guests will try to recover from something
05bba0
+     * listed as a medium error on a CD; it seems to work on Linux.
05bba0
+     * This would be more of a problem if we did any other type of
05bba0
+     * DMA operation.
05bba0
+     */
05bba0
+    ide_atapi_cmd_error(s, MEDIUM_ERROR, ASC_NO_SEEK_COMPLETE);
05bba0
+}
05bba0
+
05bba0
 static inline uint8_t ide_atapi_set_profile(uint8_t *buf, uint8_t *index,
05bba0
                                             uint16_t profile)
05bba0
 {
05bba0
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
05bba0
index 048a052..0a2d6bc 100644
05bba0
--- a/hw/ide/internal.h
05bba0
+++ b/hw/ide/internal.h
05bba0
@@ -289,6 +289,7 @@ typedef struct IDEDMAOps IDEDMAOps;
05bba0
 #define ATAPI_INT_REASON_TAG            0xf8
05bba0
 
05bba0
 /* same constants as bochs */
05bba0
+#define ASC_NO_SEEK_COMPLETE                 0x02
05bba0
 #define ASC_ILLEGAL_OPCODE                   0x20
05bba0
 #define ASC_LOGICAL_BLOCK_OOR                0x21
05bba0
 #define ASC_INV_FIELD_IN_CMD_PACKET          0x24
05bba0
@@ -536,6 +537,7 @@ void ide_dma_error(IDEState *s);
05bba0
 
05bba0
 void ide_atapi_cmd_ok(IDEState *s);
05bba0
 void ide_atapi_cmd_error(IDEState *s, int sense_key, int asc);
05bba0
+void ide_atapi_dma_restart(IDEState *s);
05bba0
 void ide_atapi_io_error(IDEState *s, int ret);
05bba0
 
05bba0
 void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val);
05bba0
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
05bba0
index 635a364..cf7acb0 100644
05bba0
--- a/hw/ide/pci.c
05bba0
+++ b/hw/ide/pci.c
05bba0
@@ -220,6 +220,17 @@ static void bmdma_restart_bh(void *opaque)
05bba0
         }
05bba0
     } else if (error_status & BM_STATUS_RETRY_FLUSH) {
05bba0
         ide_flush_cache(bmdma_active_if(bm));
05bba0
+    } else {
05bba0
+        IDEState *s = bmdma_active_if(bm);
05bba0
+
05bba0
+        /*
05bba0
+         * We've not got any bits to tell us about ATAPI - but
05bba0
+         * we do have the end_transfer_func that tells us what
05bba0
+         * we're trying to do.
05bba0
+         */
05bba0
+        if (s->end_transfer_func == ide_atapi_cmd) {
05bba0
+            ide_atapi_dma_restart(s);
05bba0
+        }
05bba0
     }
05bba0
 }
05bba0
 
05bba0
-- 
05bba0
1.8.3.1
05bba0