[Qemu-devel] [PATCH 07/10] ide: Drop redundant IDEState member version

[Markus Armbruster]

It's a copy of dev->version.  The copy was needed for non-qdevified
controllers, which lacked dev.

Note that pci_piix3_xen_ide_unplug() did not clear the copy (it only
cleared the copy of bs).  Begs the question whether stale data could
have been used after unplug.  As far as I can tell, the copy was used
only when the copy of bs was non-null, thus no bug there.

Signed-off-by: Markus Armbruster <address@hidden>
---
 hw/ide/atapi.c    |  2 +-
 hw/ide/core.c     | 14 ++++----------
 hw/ide/internal.h |  3 +--
 hw/ide/qdev.c     |  9 +++++----
 4 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index d077d18..240a62c 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -634,7 +634,7 @@ static void cmd_inquiry(IDEState *s, uint8_t *buf)
     buf[7] = 0; /* reserved */
     padstr8(buf + 8, 8, "QEMU");
     padstr8(buf + 16, 16, "QEMU DVD-ROM");
-    padstr8(buf + 32, 4, s->version);
+    padstr8(buf + 32, 4, s->dev->version);
     ide_atapi_cmd_reply(s, 36, max_len);
 }
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index d0b3ca6..05a2e61 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -98,7 +98,7 @@ static void ide_identify(IDEState *s)
     put_le16(p + 20, 3); /* XXX: retired, remove ? */
     put_le16(p + 21, 512); /* cache size in sectors */
     put_le16(p + 22, 4); /* ecc bytes */
-    padstr((char *)(p + 23), s->version, 8); /* firmware version */
+    padstr((char *)(p + 23), s->dev->version, 8); /* firmware version */
     padstr((char *)(p + 27), s->drive_model_str, 40); /* model */
 #if MAX_MULT_SECTORS > 1
     put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS);
@@ -202,7 +202,7 @@ static void ide_atapi_identify(IDEState *s)
     put_le16(p + 20, 3); /* buffer type */
     put_le16(p + 21, 512); /* cache size in sectors */
     put_le16(p + 22, 4); /* ecc bytes */
-    padstr((char *)(p + 23), s->version, 8); /* firmware version */
+    padstr((char *)(p + 23), s->dev->version, 8); /* firmware version */
     padstr((char *)(p + 27), s->drive_model_str, 40); /* model */
     put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */
 #ifdef USE_DMA_CDROM
@@ -259,7 +259,7 @@ static void ide_cfata_identify(IDEState *s)
     put_le16(p + 8, s->nb_sectors);            /* Sectors per card */
     padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */
     put_le16(p + 22, 0x0004);                  /* ECC bytes */
-    padstr((char *) (p + 23), s->version, 8);  /* Firmware Revision */
+    padstr((char *) (p + 23), s->dev->version, 8); /* Firmware Revision */
     padstr((char *) (p + 27), s->drive_model_str, 40);/* Model number */
 #if MAX_MULT_SECTORS > 1
     put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS);
@@ -1931,7 +1931,7 @@ static const BlockDevOps ide_cd_block_ops = {
 };
 
 int ide_init_drive(IDEState *s, IDEDriveKind kind,
-                   const char *version, const char *serial, const char *model,
+                   const char *serial, const char *model,
                    uint64_t wwn)
 {
     BlockDriverState *bs = s->dev->conf.bs;
@@ -1983,12 +1983,6 @@ int ide_init_drive(IDEState *s, IDEDriveKind kind,
         }
     }
 
-    if (version) {
-        pstrcpy(s->version, sizeof(s->version), version);
-    } else {
-        pstrcpy(s->version, sizeof(s->version), qemu_get_version());
-    }
-
     ide_reset(s);
     bdrv_iostatus_enable(bs);
     return 0;
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index aa226e4..544f3b1 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -373,7 +373,6 @@ struct IDEState {
 
     /* set for lba48 access */
     uint8_t lba48;
-    char version[9];
     /* ATAPI specific */
     struct unreported_events events;
     uint8_t sense_key;
@@ -544,7 +543,7 @@ void ide_data_writel(void *opaque, uint32_t addr, uint32_t 
val);
 uint32_t ide_data_readl(void *opaque, uint32_t addr);
 
 int ide_init_drive(IDEState *s, IDEDriveKind kind,
-                   const char *version, const char *serial, const char *model,
+                   const char *serial, const char *model,
                    uint64_t wwn);
 void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 82d9ca4..0139cd1 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -148,14 +148,15 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind 
kind)
         return -1;
     }
 
+    if (!dev->version) {
+        dev->version = g_strdup(qemu_get_version());
+    }
+
     if (ide_init_drive(s, kind,
-                       dev->version, dev->serial, dev->model, dev->wwn) < 0) {
+                       dev->serial, dev->model, dev->wwn) < 0) {
         return -1;
     }
 
-    if (!dev->version) {
-        dev->version = g_strdup(s->version);
-    }
     if (!dev->serial) {
         dev->serial = g_strdup(s->drive_serial_str);
     }
-- 
1.7.11.7