[Qemu-devel] [PATCH v3 7/9] ide: Drop redundant IDEState member drive_serial_str

[Markus Armbruster <armbru@redhat.com>]

It's a copy of dev->serial.  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 <armbru@redhat.com>
---
 hw/ide/core.c     | 14 ++++----------
 hw/ide/internal.h |  3 +--
 hw/ide/qdev.c     | 10 +++++-----
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 564ad71..c123a14 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -94,7 +94,7 @@ static void ide_identify(IDEState *s)
     put_le16(p + 4, 512 * s->dev->conf.secs); /* XXX: retired, remove ? */
     put_le16(p + 5, 512); /* XXX: retired, remove ? */
     put_le16(p + 6, s->dev->conf.secs);
-    padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */
+    padstr((char *)(p + 10), s->dev->serial, 20); /* serial number */
     put_le16(p + 20, 3); /* XXX: retired, remove ? */
     put_le16(p + 21, 512); /* cache size in sectors */
     put_le16(p + 22, 4); /* ecc bytes */
@@ -198,7 +198,7 @@ static void ide_atapi_identify(IDEState *s)
     p = (uint16_t *)s->io_buffer;
     /* Removable CDROM, 50us response, 12 byte packets */
     put_le16(p + 0, (2 << 14) | (5 << 8) | (1 << 7) | (2 << 5) | (0 << 0));
-    padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */
+    padstr((char *)(p + 10), s->dev->serial, 20); /* serial number */
     put_le16(p + 20, 3); /* buffer type */
     put_le16(p + 21, 512); /* cache size in sectors */
     put_le16(p + 22, 4); /* ecc bytes */
@@ -257,7 +257,7 @@ static void ide_cfata_identify(IDEState *s)
     put_le16(p + 6, s->dev->conf.secs);         /* Default sectors per track */
     put_le16(p + 7, s->nb_sectors >> 16);	/* Sectors per card */
     put_le16(p + 8, s->nb_sectors);		/* Sectors per card */
-    padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */
+    padstr((char *)(p + 10), s->dev->serial, 20); /* serial number */
     put_le16(p + 22, 0x0004);			/* ECC bytes */
     padstr((char *) (p + 23), s->dev->version, 8); /* Firmware Revision */
     padstr((char *) (p + 27), s->drive_model_str, 40);/* Model number */
@@ -2089,7 +2089,7 @@ static const BlockDevOps ide_cd_block_ops = {
 };
 
 int ide_init_drive(IDEState *s, IDEDriveKind kind,
-                   const char *serial, const char *model,
+                   const char *model,
                    uint64_t wwn)
 {
     BlockDriverState *bs = s->dev->conf.bs;
@@ -2119,12 +2119,6 @@ int ide_init_drive(IDEState *s, IDEDriveKind kind,
             return -1;
         }
     }
-    if (serial) {
-        pstrcpy(s->drive_serial_str, sizeof(s->drive_serial_str), serial);
-    } else {
-        snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
-                 "QM%05d", s->drive_serial);
-    }
     if (model) {
         pstrcpy(s->drive_model_str, sizeof(s->drive_model_str), model);
     } else {
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 4c0fb8e..f6b5b7a 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -350,7 +350,6 @@ struct IDEState {
     int identify_set;
     uint8_t identify_data[512];
     int drive_serial;
-    char drive_serial_str[21];
     char drive_model_str[41];
     uint64_t wwn;
     /* ide regs */
@@ -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 *serial, const char *model,
+                   const char *model,
                    uint64_t wwn);
 void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 0326360..c1a4cb6 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -160,19 +160,19 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
         return -1;
     }
 
+    if (!dev->serial) {
+        dev->serial = g_strdup_printf("QM%05d", s->drive_serial);
+    }
+
     if (!dev->version) {
         dev->version = g_strdup(qemu_get_version());
     }
 
     if (ide_init_drive(s, kind,
-                       dev->serial, dev->model, dev->wwn) < 0) {
+                       dev->model, dev->wwn) < 0) {
         return -1;
     }
 
-    if (!dev->serial) {
-        dev->serial = g_strdup(s->drive_serial_str);
-    }
-
     add_boot_device_path(dev->conf.bootindex, &dev->qdev,
                          dev->unit ? "/disk@1" : "/disk@0");
 
-- 
1.8.1.4