* [Qemu-devel] [PATCH v2 1/9] ide: Move IDEDevice pointer from IDEBus to IDEState
2013-11-20 9:55 [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification armbru
@ 2013-11-20 9:55 ` armbru
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 2/9] ide: Use IDEState member dev for "device connected" test armbru
` (8 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: armbru @ 2013-11-20 9:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, afaerber
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/ide/core.c | 2 +-
hw/ide/internal.h | 3 +--
hw/ide/qdev.c | 12 +++---------
3 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index e1f4c33..2d382b2 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -79,7 +79,7 @@ static void ide_identify(IDEState *s)
{
uint16_t *p;
unsigned int oldsize;
- IDEDevice *dev = s->unit ? s->bus->slave : s->bus->master;
+ IDEDevice *dev = s->dev;
if (s->identify_set) {
memcpy(s->io_buffer, s->identify_data, sizeof(s->identify_data));
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 0567a52..908d91d 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -341,6 +341,7 @@ enum ide_dma_cmd {
/* NOTE: IDEState represents in fact one drive */
struct IDEState {
IDEBus *bus;
+ IDEDevice *dev;
uint8_t unit;
/* ide config */
IDEDriveKind drive_kind;
@@ -447,8 +448,6 @@ struct IDEDMA {
struct IDEBus {
BusState qbus;
- IDEDevice *master;
- IDEDevice *slave;
IDEState ifs[2];
int bus_id;
int max_units;
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 18c4b7e..6ea1698 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -76,7 +76,7 @@ static int ide_qdev_init(DeviceState *qdev)
goto err;
}
if (dev->unit == -1) {
- dev->unit = bus->master ? 1 : 0;
+ dev->unit = bus->ifs[0].dev ? 1 : 0;
}
if (dev->unit >= bus->max_units) {
@@ -87,18 +87,12 @@ static int ide_qdev_init(DeviceState *qdev)
switch (dev->unit) {
case 0:
- if (bus->master) {
- error_report("IDE unit %d is in use", dev->unit);
- goto err;
- }
- bus->master = dev;
- break;
case 1:
- if (bus->slave) {
+ if (bus->ifs[dev->unit].dev) {
error_report("IDE unit %d is in use", dev->unit);
goto err;
}
- bus->slave = dev;
+ bus->ifs[dev->unit].dev = dev;
break;
default:
error_report("Invalid IDE unit %d", dev->unit);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 2/9] ide: Use IDEState member dev for "device connected" test
2013-11-20 9:55 [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification armbru
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 1/9] ide: Move IDEDevice pointer from IDEBus to IDEState armbru
@ 2013-11-20 9:55 ` armbru
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 3/9] ide: Don't block-align IDEState member smart_selftest_data armbru
` (7 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: armbru @ 2013-11-20 9:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, afaerber
From: Markus Armbruster <armbru@redhat.com>
Testing dev is more obvious than testing bs, in my opinion.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/ide/ahci.c | 8 ++++----
hw/ide/core.c | 56 ++++++++++++++++++++++++++++-------------------------
hw/ide/microdrive.c | 2 +-
hw/ide/qdev.c | 2 +-
4 files changed, 36 insertions(+), 32 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index fbea9e8..eb6a6fe 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -85,7 +85,7 @@ static uint32_t ahci_port_read(AHCIState *s, int port, int offset)
val = pr->sig;
break;
case PORT_SCR_STAT:
- if (s->dev[port].port.ifs[0].bs) {
+ if (s->dev[port].port.ifs[0].dev) {
val = SATA_SCR_SSTATUS_DET_DEV_PRESENT_PHY_UP |
SATA_SCR_SSTATUS_SPD_GEN1 | SATA_SCR_SSTATUS_IPM_ACTIVE;
} else {
@@ -497,7 +497,7 @@ static void ahci_reset_port(AHCIState *s, int port)
d->init_d2h_sent = false;
ide_state = &s->dev[port].port.ifs[0];
- if (!ide_state->bs) {
+ if (!ide_state->dev) {
return;
}
@@ -523,7 +523,7 @@ static void ahci_reset_port(AHCIState *s, int port)
}
s->dev[port].port_state = STATE_RUN;
- if (!ide_state->bs) {
+ if (!ide_state->dev) {
s->dev[port].port_regs.sig = 0;
ide_state->status = SEEK_STAT | WRERR_STAT;
} else if (ide_state->drive_kind == IDE_CD) {
@@ -851,7 +851,7 @@ static int handle_cmd(AHCIState *s, int port, int slot)
/* The device we are working for */
ide_state = &s->dev[port].port.ifs[0];
- if (!ide_state->bs) {
+ if (!ide_state->dev) {
DPRINTF(port, "error: guest accessed unused port");
goto out;
}
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 2d382b2..0022cc5 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -312,7 +312,7 @@ static void ide_set_signature(IDEState *s)
if (s->drive_kind == IDE_CD) {
s->lcyl = 0x14;
s->hcyl = 0xeb;
- } else if (s->bs) {
+ } else if (s->dev) {
s->lcyl = 0;
s->hcyl = 0;
} else {
@@ -818,7 +818,7 @@ static void ide_flush_cb(void *opaque, int ret)
void ide_flush_cache(IDEState *s)
{
- if (s->bs == NULL) {
+ if (!s->dev) {
ide_flush_cb(s, 0);
return;
}
@@ -1022,7 +1022,7 @@ static bool cmd_data_set_management(IDEState *s, uint8_t cmd)
{
switch (s->feature) {
case DSM_TRIM:
- if (s->bs) {
+ if (s->dev) {
ide_sector_start_dma(s, IDE_DMA_TRIM);
return false;
}
@@ -1035,7 +1035,7 @@ static bool cmd_data_set_management(IDEState *s, uint8_t cmd)
static bool cmd_identify(IDEState *s, uint8_t cmd)
{
- if (s->bs && s->drive_kind != IDE_CD) {
+ if (s->dev && s->drive_kind != IDE_CD) {
if (s->drive_kind != IDE_CFATA) {
ide_identify(s);
} else {
@@ -1085,7 +1085,7 @@ static bool cmd_read_multiple(IDEState *s, uint8_t cmd)
{
bool lba48 = (cmd == WIN_MULTREAD_EXT);
- if (!s->bs || !s->mult_sectors) {
+ if (!s->dev || !s->mult_sectors) {
ide_abort_command(s);
return true;
}
@@ -1101,7 +1101,7 @@ static bool cmd_write_multiple(IDEState *s, uint8_t cmd)
bool lba48 = (cmd == WIN_MULTWRITE_EXT);
int n;
- if (!s->bs || !s->mult_sectors) {
+ if (!s->dev || !s->mult_sectors) {
ide_abort_command(s);
return true;
}
@@ -1129,7 +1129,7 @@ static bool cmd_read_pio(IDEState *s, uint8_t cmd)
return true;
}
- if (!s->bs) {
+ if (!s->dev) {
ide_abort_command(s);
return true;
}
@@ -1145,7 +1145,7 @@ static bool cmd_write_pio(IDEState *s, uint8_t cmd)
{
bool lba48 = (cmd == WIN_WRITE_EXT);
- if (!s->bs) {
+ if (!s->dev) {
ide_abort_command(s);
return true;
}
@@ -1165,7 +1165,7 @@ static bool cmd_read_dma(IDEState *s, uint8_t cmd)
{
bool lba48 = (cmd == WIN_READDMA_EXT);
- if (!s->bs) {
+ if (!s->dev) {
ide_abort_command(s);
return true;
}
@@ -1180,7 +1180,7 @@ static bool cmd_write_dma(IDEState *s, uint8_t cmd)
{
bool lba48 = (cmd == WIN_WRITEDMA_EXT);
- if (!s->bs) {
+ if (!s->dev) {
ide_abort_command(s);
return true;
}
@@ -1231,7 +1231,7 @@ static bool cmd_set_features(IDEState *s, uint8_t cmd)
{
uint16_t *identify_data;
- if (!s->bs) {
+ if (!s->dev) {
ide_abort_command(s);
return true;
}
@@ -1710,8 +1710,9 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
#endif
s = idebus_active_if(bus);
/* ignore commands to non existent slave */
- if (s != bus->ifs && !s->bs)
+ if (s != bus->ifs && !s->dev) {
return;
+ }
/* Only DEVICE RESET is allowed while BSY or/and DRQ are set */
if ((s->status & (BUSY_STAT|DRQ_STAT)) && val != WIN_DEVICE_RESET)
@@ -1755,8 +1756,8 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr1)
ret = 0xff;
break;
case 1:
- if ((!bus->ifs[0].bs && !bus->ifs[1].bs) ||
- (s != bus->ifs && !s->bs))
+ if ((!bus->ifs[0].dev && !bus->ifs[1].dev) ||
+ (s != bus->ifs && !s->dev))
ret = 0;
else if (!hob)
ret = s->error;
@@ -1764,7 +1765,7 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr1)
ret = s->hob_feature;
break;
case 2:
- if (!bus->ifs[0].bs && !bus->ifs[1].bs)
+ if (!bus->ifs[0].dev && !bus->ifs[1].dev)
ret = 0;
else if (!hob)
ret = s->nsector & 0xff;
@@ -1772,7 +1773,7 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr1)
ret = s->hob_nsector;
break;
case 3:
- if (!bus->ifs[0].bs && !bus->ifs[1].bs)
+ if (!bus->ifs[0].dev && !bus->ifs[1].dev)
ret = 0;
else if (!hob)
ret = s->sector;
@@ -1780,7 +1781,7 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr1)
ret = s->hob_sector;
break;
case 4:
- if (!bus->ifs[0].bs && !bus->ifs[1].bs)
+ if (!bus->ifs[0].dev && !bus->ifs[1].dev)
ret = 0;
else if (!hob)
ret = s->lcyl;
@@ -1788,7 +1789,7 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr1)
ret = s->hob_lcyl;
break;
case 5:
- if (!bus->ifs[0].bs && !bus->ifs[1].bs)
+ if (!bus->ifs[0].dev && !bus->ifs[1].dev)
ret = 0;
else if (!hob)
ret = s->hcyl;
@@ -1796,18 +1797,20 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr1)
ret = s->hob_hcyl;
break;
case 6:
- if (!bus->ifs[0].bs && !bus->ifs[1].bs)
+ if (!bus->ifs[0].dev && !bus->ifs[1].dev) {
ret = 0;
- else
+ } else {
ret = s->select;
+ }
break;
default:
case 7:
- if ((!bus->ifs[0].bs && !bus->ifs[1].bs) ||
- (s != bus->ifs && !s->bs))
+ if ((!bus->ifs[0].dev && !bus->ifs[1].dev) ||
+ (s != bus->ifs && !s->dev)) {
ret = 0;
- else
+ } else {
ret = s->status;
+ }
qemu_irq_lower(bus->irq);
break;
}
@@ -1823,11 +1826,12 @@ uint32_t ide_status_read(void *opaque, uint32_t addr)
IDEState *s = idebus_active_if(bus);
int ret;
- if ((!bus->ifs[0].bs && !bus->ifs[1].bs) ||
- (s != bus->ifs && !s->bs))
+ if ((!bus->ifs[0].dev && !bus->ifs[1].dev) ||
+ (s != bus->ifs && !s->dev)) {
ret = 0;
- else
+ } else {
ret = s->status;
+ }
#ifdef DEBUG_IDE
printf("ide: read status addr=0x%x val=%02x\n", addr, ret);
#endif
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index 21d6495..29f0d64 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -247,7 +247,7 @@ static uint16_t md_common_read(PCMCIACardState *card, uint32_t at)
return ide_ioport_read(&s->bus, 0x1);
case 0xe: /* Alternate Status */
ifs = idebus_active_if(&s->bus);
- if (ifs->bs) {
+ if (ifs->dev) {
return ifs->status;
} else {
return 0;
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6ea1698..7e8ddc2 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -120,7 +120,7 @@ int ide_get_geometry(BusState *bus, int unit,
{
IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit];
- if (s->drive_kind != IDE_HD || !s->bs) {
+ if (s->drive_kind != IDE_HD || !s->dev) {
return -1;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 3/9] ide: Don't block-align IDEState member smart_selftest_data
2013-11-20 9:55 [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification armbru
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 1/9] ide: Move IDEDevice pointer from IDEBus to IDEState armbru
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 2/9] ide: Use IDEState member dev for "device connected" test armbru
@ 2013-11-20 9:55 ` armbru
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 4/9] ide: Drop redundant IDEState member bs armbru
` (6 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: armbru @ 2013-11-20 9:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, afaerber
From: Markus Armbruster <armbru@redhat.com>
All uses are simple array subscripts.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/ide/core.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 0022cc5..2ed7cb8 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2164,8 +2164,7 @@ static void ide_init1(IDEBus *bus, int unit)
s->io_buffer = qemu_memalign(2048, s->io_buffer_total_len);
memset(s->io_buffer, 0, s->io_buffer_total_len);
- s->smart_selftest_data = qemu_blockalign(s->bs, 512);
- memset(s->smart_selftest_data, 0, 512);
+ s->smart_selftest_data = g_malloc0(512);
s->sector_write_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
ide_sector_write_timer_cb, s);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 4/9] ide: Drop redundant IDEState member bs
2013-11-20 9:55 [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification armbru
` (2 preceding siblings ...)
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 3/9] ide: Don't block-align IDEState member smart_selftest_data armbru
@ 2013-11-20 9:55 ` armbru
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 5/9] ide: Drop redundant IDEState geometry members armbru
` (5 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: armbru @ 2013-11-20 9:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, afaerber
From: Markus Armbruster <armbru@redhat.com>
It's a copy of dev->conf.bs. The copy was needed for non-qdevified
controllers, which lacked dev.
Note how pci_piix3_xen_ide_unplug() cleared the copy. We'll get back
to that in the next few commits.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/ide/ahci.c | 11 +++++-----
hw/ide/atapi.c | 37 ++++++++++++++++++---------------
hw/ide/core.c | 62 ++++++++++++++++++++++++++++++-------------------------
hw/ide/internal.h | 3 +--
hw/ide/macio.c | 26 ++++++++++++-----------
hw/ide/piix.c | 4 ----
hw/ide/qdev.c | 2 +-
7 files changed, 76 insertions(+), 69 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index eb6a6fe..1afc37e 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -735,7 +735,7 @@ static void ncq_cb(void *opaque, int ret)
DPRINTF(ncq_tfs->drive->port_no, "NCQ transfer tag %d finished\n",
ncq_tfs->tag);
- bdrv_acct_done(ncq_tfs->drive->port.ifs[0].bs, &ncq_tfs->acct);
+ bdrv_acct_done(ncq_tfs->drive->port.ifs[0].dev->conf.bs, &ncq_tfs->acct);
qemu_sglist_destroy(&ncq_tfs->sglist);
ncq_tfs->used = 0;
}
@@ -746,6 +746,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis,
NCQFrame *ncq_fis = (NCQFrame*)cmd_fis;
uint8_t tag = ncq_fis->tag >> 3;
NCQTransferState *ncq_tfs = &s->dev[port].ncq_tfs[tag];
+ BlockDriverState *bs = ncq_tfs->drive->port.ifs[0].dev->conf.bs;
if (ncq_tfs->used) {
/* error - already in use */
@@ -785,9 +786,9 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis,
DPRINTF(port, "tag %d aio read %"PRId64"\n",
ncq_tfs->tag, ncq_tfs->lba);
- dma_acct_start(ncq_tfs->drive->port.ifs[0].bs, &ncq_tfs->acct,
+ dma_acct_start(bs, &ncq_tfs->acct,
&ncq_tfs->sglist, BDRV_ACCT_READ);
- ncq_tfs->aiocb = dma_bdrv_read(ncq_tfs->drive->port.ifs[0].bs,
+ ncq_tfs->aiocb = dma_bdrv_read(bs,
&ncq_tfs->sglist, ncq_tfs->lba,
ncq_cb, ncq_tfs);
break;
@@ -798,9 +799,9 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis,
DPRINTF(port, "tag %d aio write %"PRId64"\n",
ncq_tfs->tag, ncq_tfs->lba);
- dma_acct_start(ncq_tfs->drive->port.ifs[0].bs, &ncq_tfs->acct,
+ dma_acct_start(bs, &ncq_tfs->acct,
&ncq_tfs->sglist, BDRV_ACCT_WRITE);
- ncq_tfs->aiocb = dma_bdrv_write(ncq_tfs->drive->port.ifs[0].bs,
+ ncq_tfs->aiocb = dma_bdrv_write(bs,
&ncq_tfs->sglist, ncq_tfs->lba,
ncq_cb, ncq_tfs);
break;
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index f7d2009..3dc2de0 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -106,18 +106,19 @@ static void cd_data_to_raw(uint8_t *buf, int lba)
static int cd_read_sector(IDEState *s, int lba, uint8_t *buf, int sector_size)
{
+ BlockDriverState *bs = s->dev->conf.bs;
int ret;
switch(sector_size) {
case 2048:
- bdrv_acct_start(s->bs, &s->acct, 4 * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
- ret = bdrv_read(s->bs, (int64_t)lba << 2, buf, 4);
- bdrv_acct_done(s->bs, &s->acct);
+ bdrv_acct_start(bs, &s->acct, 4 * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
+ ret = bdrv_read(bs, (int64_t)lba << 2, buf, 4);
+ bdrv_acct_done(bs, &s->acct);
break;
case 2352:
- bdrv_acct_start(s->bs, &s->acct, 4 * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
- ret = bdrv_read(s->bs, (int64_t)lba << 2, buf + 16, 4);
- bdrv_acct_done(s->bs, &s->acct);
+ bdrv_acct_start(bs, &s->acct, 4 * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
+ ret = bdrv_read(bs, (int64_t)lba << 2, buf + 16, 4);
+ bdrv_acct_done(bs, &s->acct);
if (ret < 0)
return ret;
cd_data_to_raw(buf, lba);
@@ -253,7 +254,7 @@ static void ide_atapi_cmd_reply(IDEState *s, int size, int max_size)
s->io_buffer_index = 0;
if (s->atapi_dma) {
- bdrv_acct_start(s->bs, &s->acct, size, BDRV_ACCT_READ);
+ bdrv_acct_start(s->dev->conf.bs, &s->acct, size, BDRV_ACCT_READ);
s->status = READY_STAT | SEEK_STAT | DRQ_STAT;
s->bus->dma->ops->start_dma(s->bus->dma, s,
ide_atapi_cmd_read_dma_cb);
@@ -349,13 +350,13 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
s->bus->dma->iov.iov_len = n * 4 * 512;
qemu_iovec_init_external(&s->bus->dma->qiov, &s->bus->dma->iov, 1);
- s->bus->dma->aiocb = bdrv_aio_readv(s->bs, (int64_t)s->lba << 2,
+ s->bus->dma->aiocb = bdrv_aio_readv(s->dev->conf.bs, (int64_t)s->lba << 2,
&s->bus->dma->qiov, n * 4,
ide_atapi_cmd_read_dma_cb, s);
return;
eot:
- bdrv_acct_done(s->bs, &s->acct);
+ bdrv_acct_done(s->dev->conf.bs, &s->acct);
s->bus->dma->ops->add_status(s->bus->dma, BM_STATUS_INT);
ide_set_inactive(s);
}
@@ -371,7 +372,8 @@ static void ide_atapi_cmd_read_dma(IDEState *s, int lba, int nb_sectors,
s->io_buffer_size = 0;
s->cd_sector_size = sector_size;
- bdrv_acct_start(s->bs, &s->acct, s->packet_transfer_size, BDRV_ACCT_READ);
+ bdrv_acct_start(s->dev->conf.bs, &s->acct, s->packet_transfer_size,
+ BDRV_ACCT_READ);
/* XXX: check if BUSY_STAT should be set */
s->status = READY_STAT | SEEK_STAT | DRQ_STAT | BUSY_STAT;
@@ -504,7 +506,7 @@ static unsigned int event_status_media(IDEState *s,
media_status = 0;
if (s->tray_open) {
media_status = MS_TRAY_OPEN;
- } else if (bdrv_is_inserted(s->bs)) {
+ } else if (bdrv_is_inserted(s->dev->conf.bs)) {
media_status = MS_MEDIA_PRESENT;
}
@@ -800,7 +802,7 @@ static void cmd_test_unit_ready(IDEState *s, uint8_t *buf)
static void cmd_prevent_allow_medium_removal(IDEState *s, uint8_t* buf)
{
s->tray_locked = buf[4] & 1;
- bdrv_lock_medium(s->bs, buf[4] & 1);
+ bdrv_lock_medium(s->dev->conf.bs, buf[4] & 1);
ide_atapi_cmd_ok(s);
}
@@ -884,14 +886,14 @@ static void cmd_start_stop_unit(IDEState *s, uint8_t* buf)
if (loej) {
if (!start && !s->tray_open && s->tray_locked) {
- sense = bdrv_is_inserted(s->bs)
+ sense = bdrv_is_inserted(s->dev->conf.bs)
? NOT_READY : ILLEGAL_REQUEST;
ide_atapi_cmd_error(s, sense, ASC_MEDIA_REMOVAL_PREVENTED);
return;
}
if (s->tray_open != !start) {
- bdrv_eject(s->bs, !start);
+ bdrv_eject(s->dev->conf.bs, !start);
s->tray_open = !start;
}
}
@@ -1094,6 +1096,7 @@ static const struct {
void ide_atapi_cmd(IDEState *s)
{
uint8_t *buf;
+ bool is_inserted;
buf = s->io_buffer;
#ifdef DEBUG_IDE_ATAPI
@@ -1124,8 +1127,9 @@ void ide_atapi_cmd(IDEState *s)
* GET_EVENT_STATUS_NOTIFICATION to detect such tray open/close
* states rely on this behavior.
*/
+ is_inserted = bdrv_is_inserted(s->dev->conf.bs);
if (!(atapi_cmd_table[s->io_buffer[0]].flags & ALLOW_UA) &&
- !s->tray_open && bdrv_is_inserted(s->bs) && s->cdrom_changed) {
+ !s->tray_open && is_inserted && s->cdrom_changed) {
if (s->cdrom_changed == 1) {
ide_atapi_cmd_error(s, NOT_READY, ASC_MEDIUM_NOT_PRESENT);
@@ -1134,13 +1138,12 @@ void ide_atapi_cmd(IDEState *s)
ide_atapi_cmd_error(s, UNIT_ATTENTION, ASC_MEDIUM_MAY_HAVE_CHANGED);
s->cdrom_changed = 0;
}
-
return;
}
/* Report a Not Ready condition if appropriate for the command */
if ((atapi_cmd_table[s->io_buffer[0]].flags & CHECK_READY) &&
- (!media_present(s) || !bdrv_is_inserted(s->bs)))
+ (!media_present(s) || !is_inserted))
{
ide_atapi_cmd_error(s, NOT_READY, ASC_MEDIUM_NOT_PRESENT);
return;
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 2ed7cb8..448dcbd 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -148,10 +148,11 @@ static void ide_identify(IDEState *s)
put_le16(p + 84, (1 << 14) | 0);
}
/* 14 = NOP supported, 5=WCACHE enabled, 0=SMART feature set enabled */
- if (bdrv_enable_write_cache(s->bs))
+ if (bdrv_enable_write_cache(s->dev->conf.bs)) {
put_le16(p + 85, (1 << 14) | (1 << 5) | 1);
- else
+ } else {
put_le16(p + 85, (1 << 14) | 1);
+ }
/* 13=flush_cache_ext,12=flush_cache,10=lba48 */
put_le16(p + 86, (1 << 13) | (1 <<12) | (1 << 10));
/* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */
@@ -507,7 +508,7 @@ static void ide_sector_read_cb(void *opaque, int ret)
s->pio_aiocb = NULL;
s->status &= ~BUSY_STAT;
- bdrv_acct_done(s->bs, &s->acct);
+ bdrv_acct_done(s->dev->conf.bs, &s->acct);
if (ret != 0) {
if (ide_handle_rw_error(s, -ret, BM_STATUS_PIO_RETRY |
BM_STATUS_RETRY_READ)) {
@@ -531,6 +532,7 @@ static void ide_sector_read_cb(void *opaque, int ret)
void ide_sector_read(IDEState *s)
{
+ BlockDriverState *bs = s->dev->conf.bs;
int64_t sector_num;
int n;
@@ -558,8 +560,8 @@ void ide_sector_read(IDEState *s)
s->iov.iov_len = n * BDRV_SECTOR_SIZE;
qemu_iovec_init_external(&s->qiov, &s->iov, 1);
- bdrv_acct_start(s->bs, &s->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
- s->pio_aiocb = bdrv_aio_readv(s->bs, sector_num, &s->qiov, n,
+ bdrv_acct_start(bs, &s->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
+ s->pio_aiocb = bdrv_aio_readv(bs, sector_num, &s->qiov, n,
ide_sector_read_cb, s);
}
@@ -594,7 +596,8 @@ void ide_dma_error(IDEState *s)
static int ide_handle_rw_error(IDEState *s, int error, int op)
{
bool is_read = (op & BM_STATUS_RETRY_READ) != 0;
- BlockErrorAction action = bdrv_get_error_action(s->bs, is_read, error);
+ BlockErrorAction action = bdrv_get_error_action(s->dev->conf.bs,
+ is_read, error);
if (action == BDRV_ACTION_STOP) {
s->bus->dma->ops->set_unit(s->bus->dma, s->unit);
@@ -607,13 +610,14 @@ static int ide_handle_rw_error(IDEState *s, int error, int op)
ide_rw_error(s);
}
}
- bdrv_error_action(s->bs, action, is_read, error);
+ bdrv_error_action(s->dev->conf.bs, action, is_read, error);
return action != BDRV_ACTION_IGNORE;
}
void ide_dma_cb(void *opaque, int ret)
{
IDEState *s = opaque;
+ BlockDriverState *bs = s->dev->conf.bs;
int n;
int64_t sector_num;
bool stay_active = false;
@@ -673,15 +677,15 @@ void ide_dma_cb(void *opaque, int ret)
switch (s->dma_cmd) {
case IDE_DMA_READ:
- s->bus->dma->aiocb = dma_bdrv_read(s->bs, &s->sg, sector_num,
+ s->bus->dma->aiocb = dma_bdrv_read(bs, &s->sg, sector_num,
ide_dma_cb, s);
break;
case IDE_DMA_WRITE:
- s->bus->dma->aiocb = dma_bdrv_write(s->bs, &s->sg, sector_num,
+ s->bus->dma->aiocb = dma_bdrv_write(bs, &s->sg, sector_num,
ide_dma_cb, s);
break;
case IDE_DMA_TRIM:
- s->bus->dma->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
+ s->bus->dma->aiocb = dma_bdrv_io(bs, &s->sg, sector_num,
ide_issue_trim, ide_dma_cb, s,
DMA_DIRECTION_TO_DEVICE);
break;
@@ -690,7 +694,7 @@ void ide_dma_cb(void *opaque, int ret)
eot:
if (s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) {
- bdrv_acct_done(s->bs, &s->acct);
+ bdrv_acct_done(bs, &s->acct);
}
ide_set_inactive(s);
if (stay_active) {
@@ -707,12 +711,12 @@ static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
switch (dma_cmd) {
case IDE_DMA_READ:
- bdrv_acct_start(s->bs, &s->acct, s->nsector * BDRV_SECTOR_SIZE,
- BDRV_ACCT_READ);
+ bdrv_acct_start(s->dev->conf.bs, &s->acct,
+ s->nsector * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
break;
case IDE_DMA_WRITE:
- bdrv_acct_start(s->bs, &s->acct, s->nsector * BDRV_SECTOR_SIZE,
- BDRV_ACCT_WRITE);
+ bdrv_acct_start(s->dev->conf.bs, &s->acct,
+ s->nsector * BDRV_SECTOR_SIZE, BDRV_ACCT_WRITE);
break;
default:
break;
@@ -732,7 +736,7 @@ static void ide_sector_write_cb(void *opaque, int ret)
IDEState *s = opaque;
int n;
- bdrv_acct_done(s->bs, &s->acct);
+ bdrv_acct_done(s->dev->conf.bs, &s->acct);
s->pio_aiocb = NULL;
s->status &= ~BUSY_STAT;
@@ -777,6 +781,7 @@ static void ide_sector_write_cb(void *opaque, int ret)
void ide_sector_write(IDEState *s)
{
+ BlockDriverState *bs = s->dev->conf.bs;
int64_t sector_num;
int n;
@@ -794,8 +799,8 @@ void ide_sector_write(IDEState *s)
s->iov.iov_len = n * BDRV_SECTOR_SIZE;
qemu_iovec_init_external(&s->qiov, &s->iov, 1);
- bdrv_acct_start(s->bs, &s->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
- s->pio_aiocb = bdrv_aio_writev(s->bs, sector_num, &s->qiov, n,
+ bdrv_acct_start(bs, &s->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
+ s->pio_aiocb = bdrv_aio_writev(bs, sector_num, &s->qiov, n,
ide_sector_write_cb, s);
}
@@ -810,7 +815,7 @@ static void ide_flush_cb(void *opaque, int ret)
}
}
- bdrv_acct_done(s->bs, &s->acct);
+ bdrv_acct_done(s->dev->conf.bs, &s->acct);
s->status = READY_STAT | SEEK_STAT;
ide_async_cmd_done(s);
ide_set_irq(s->bus);
@@ -824,8 +829,8 @@ void ide_flush_cache(IDEState *s)
}
s->status |= BUSY_STAT;
- bdrv_acct_start(s->bs, &s->acct, 0, BDRV_ACCT_FLUSH);
- bdrv_aio_flush(s->bs, ide_flush_cb, s);
+ bdrv_acct_start(s->dev->conf.bs, &s->acct, 0, BDRV_ACCT_FLUSH);
+ bdrv_aio_flush(s->dev->conf.bs, ide_flush_cb, s);
}
static void ide_cfata_metadata_inquiry(IDEState *s)
@@ -888,7 +893,7 @@ static void ide_cd_change_cb(void *opaque, bool load)
uint64_t nb_sectors;
s->tray_open = !load;
- bdrv_get_geometry(s->bs, &nb_sectors);
+ bdrv_get_geometry(s->dev->conf.bs, &nb_sectors);
s->nb_sectors = nb_sectors;
/*
@@ -1239,12 +1244,12 @@ static bool cmd_set_features(IDEState *s, uint8_t cmd)
/* XXX: valid for CDROM ? */
switch (s->feature) {
case 0x02: /* write cache enable */
- bdrv_set_enable_write_cache(s->bs, true);
+ bdrv_set_enable_write_cache(s->dev->conf.bs, true);
identify_data = (uint16_t *)s->identify_data;
put_le16(identify_data + 85, (1 << 14) | (1 << 5) | 1);
return true;
case 0x82: /* write cache disable */
- bdrv_set_enable_write_cache(s->bs, false);
+ bdrv_set_enable_write_cache(s->dev->conf.bs, false);
identify_data = (uint16_t *)s->identify_data;
put_le16(identify_data + 85, (1 << 14) | 1);
ide_flush_cache(s);
@@ -2081,15 +2086,15 @@ static const BlockDevOps ide_cd_block_ops = {
.is_medium_locked = ide_cd_is_medium_locked,
};
-int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
+int ide_init_drive(IDEState *s, IDEDriveKind kind,
const char *version, const char *serial, const char *model,
uint64_t wwn,
uint32_t cylinders, uint32_t heads, uint32_t secs,
int chs_trans)
{
+ BlockDriverState *bs = s->dev->conf.bs;
uint64_t nb_sectors;
- s->bs = bs;
s->drive_kind = kind;
bdrv_get_geometry(bs, &nb_sectors);
@@ -2109,7 +2114,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
bdrv_set_dev_ops(bs, &ide_cd_block_ops, s);
bdrv_set_buffer_alignment(bs, 2048);
} else {
- if (!bdrv_is_inserted(s->bs)) {
+ if (!bdrv_is_inserted(bs)) {
error_report("Device needs media, but drive is empty");
return -1;
}
@@ -2273,7 +2278,8 @@ static int ide_drive_post_load(void *opaque, int version_id)
IDEState *s = opaque;
if (s->identify_set) {
- bdrv_set_enable_write_cache(s->bs, !!(s->identify_data[85] & (1 << 5)));
+ bdrv_set_enable_write_cache(s->dev->conf.bs,
+ !!(s->identify_data[85] & (1 << 5)));
}
return 0;
}
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 908d91d..8f673f8 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -373,7 +373,6 @@ struct IDEState {
/* set for lba48 access */
uint8_t lba48;
- BlockDriverState *bs;
char version[9];
/* ATAPI specific */
struct unreported_events events;
@@ -546,7 +545,7 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr);
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, BlockDriverState *bs, IDEDriveKind kind,
+int ide_init_drive(IDEState *s, IDEDriveKind kind,
const char *version, const char *serial, const char *model,
uint64_t wwn,
uint32_t cylinders, uint32_t heads, uint32_t secs,
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index da94580..dfba4a5 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -134,7 +134,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
MACIO_DPRINTF("precopying unaligned %d bytes to %#" HWADDR_PRIx "\n",
unaligned, io->addr + io->len - unaligned);
- bdrv_read(s->bs, sector_num + nsector, io->remainder, 1);
+ bdrv_read(s->dev->conf.bs, sector_num + nsector, io->remainder, 1);
cpu_physical_memory_write(io->addr + io->len - unaligned,
io->remainder, unaligned);
@@ -164,14 +164,14 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
(s->lba << 2) + (s->io_buffer_index >> 9),
s->packet_transfer_size, s->dma_cmd);
- m->aiocb = dma_bdrv_read(s->bs, &s->sg,
+ m->aiocb = dma_bdrv_read(s->dev->conf.bs, &s->sg,
(int64_t)(s->lba << 2) + (s->io_buffer_index >> 9),
pmac_ide_atapi_transfer_cb, io);
return;
done:
MACIO_DPRINTF("done DMA\n");
- bdrv_acct_done(s->bs, &s->acct);
+ bdrv_acct_done(s->dev->conf.bs, &s->acct);
io->dma_end(opaque);
}
@@ -180,6 +180,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
DBDMA_io *io = opaque;
MACIOIDEState *m = io->opaque;
IDEState *s = idebus_active_if(&m->bus);
+ BlockDriverState *bs = s->dev->conf.bs;
int n = 0;
int64_t sector_num;
int unaligned;
@@ -229,7 +230,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
break;
case IDE_DMA_WRITE:
cpu_physical_memory_read(io->addr, p, remainder_len);
- bdrv_write(s->bs, sector_num - 1, io->remainder, 1);
+ bdrv_write(bs, sector_num - 1, io->remainder, 1);
break;
case IDE_DMA_TRIM:
break;
@@ -267,7 +268,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
switch (s->dma_cmd) {
case IDE_DMA_READ:
- bdrv_read(s->bs, sector_num + nsector, io->remainder, 1);
+ bdrv_read(bs, sector_num + nsector, io->remainder, 1);
cpu_physical_memory_write(io->addr + io->len - unaligned,
io->remainder, unaligned);
break;
@@ -306,15 +307,15 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
switch (s->dma_cmd) {
case IDE_DMA_READ:
- m->aiocb = dma_bdrv_read(s->bs, &s->sg, sector_num,
+ m->aiocb = dma_bdrv_read(bs, &s->sg, sector_num,
pmac_ide_transfer_cb, io);
break;
case IDE_DMA_WRITE:
- m->aiocb = dma_bdrv_write(s->bs, &s->sg, sector_num,
+ m->aiocb = dma_bdrv_write(bs, &s->sg, sector_num,
pmac_ide_transfer_cb, io);
break;
case IDE_DMA_TRIM:
- m->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
+ m->aiocb = dma_bdrv_io(bs, &s->sg, sector_num,
ide_issue_trim, pmac_ide_transfer_cb, io,
DMA_DIRECTION_TO_DEVICE);
break;
@@ -323,7 +324,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
done:
if (s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) {
- bdrv_acct_done(s->bs, &s->acct);
+ bdrv_acct_done(bs, &s->acct);
}
io->dma_end(io);
}
@@ -332,22 +333,23 @@ static void pmac_ide_transfer(DBDMA_io *io)
{
MACIOIDEState *m = io->opaque;
IDEState *s = idebus_active_if(&m->bus);
+ BlockDriverState *bs = s->dev->conf.bs;
MACIO_DPRINTF("\n");
s->io_buffer_size = 0;
if (s->drive_kind == IDE_CD) {
- bdrv_acct_start(s->bs, &s->acct, io->len, BDRV_ACCT_READ);
+ bdrv_acct_start(bs, &s->acct, io->len, BDRV_ACCT_READ);
pmac_ide_atapi_transfer_cb(io, 0);
return;
}
switch (s->dma_cmd) {
case IDE_DMA_READ:
- bdrv_acct_start(s->bs, &s->acct, io->len, BDRV_ACCT_READ);
+ bdrv_acct_start(bs, &s->acct, io->len, BDRV_ACCT_READ);
break;
case IDE_DMA_WRITE:
- bdrv_acct_start(s->bs, &s->acct, io->len, BDRV_ACCT_WRITE);
+ bdrv_acct_start(bs, &s->acct, io->len, BDRV_ACCT_WRITE);
break;
default:
break;
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index ab36749..34ce95f 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -169,12 +169,9 @@ static int pci_piix_ide_initfn(PCIDevice *dev)
static int pci_piix3_xen_ide_unplug(DeviceState *dev)
{
- PCIIDEState *pci_ide;
DriveInfo *di;
int i = 0;
- pci_ide = PCI_IDE(dev);
-
for (; i < 3; i++) {
di = drive_get_by_index(IF_IDE, i);
if (di != NULL && !di->media_cd) {
@@ -183,7 +180,6 @@ static int pci_piix3_xen_ide_unplug(DeviceState *dev)
bdrv_detach_dev(di->bdrv, ds);
}
bdrv_close(di->bdrv);
- pci_ide->bus[di->bus].ifs[di->unit].bs = NULL;
drive_put_ref(di);
}
}
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 7e8ddc2..f9a4d31 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -160,7 +160,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
return -1;
}
- if (ide_init_drive(s, dev->conf.bs, kind,
+ if (ide_init_drive(s, kind,
dev->version, dev->serial, dev->model, dev->wwn,
dev->conf.cyls, dev->conf.heads, dev->conf.secs,
dev->chs_trans) < 0) {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 5/9] ide: Drop redundant IDEState geometry members
2013-11-20 9:55 [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification armbru
` (3 preceding siblings ...)
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 4/9] ide: Drop redundant IDEState member bs armbru
@ 2013-11-20 9:55 ` armbru
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 6/9] ide: Drop redundant IDEState member version armbru
` (4 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: armbru @ 2013-11-20 9:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, afaerber
From: Markus Armbruster <armbru@redhat.com>
Members cylinders, heads, sectors, chs_trans are copies of
dev->conf.cyls, dev->conf.heads, dev->conf.secs, dev->chs_trans.
Copies were needed for non-qdevified controllers, which lacked dev.
Note that pci_piix3_xen_ide_unplug() did not clear the copies (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 copies were
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 | 52 ++++++++++++++++++++++++----------------------------
hw/ide/internal.h | 5 +----
hw/ide/qdev.c | 12 +++++-------
3 files changed, 30 insertions(+), 39 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 448dcbd..275df4b 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -89,11 +89,11 @@ static void ide_identify(IDEState *s)
memset(s->io_buffer, 0, 512);
p = (uint16_t *)s->io_buffer;
put_le16(p + 0, 0x0040);
- put_le16(p + 1, s->cylinders);
- put_le16(p + 3, s->heads);
- put_le16(p + 4, 512 * s->sectors); /* XXX: retired, remove ? */
+ put_le16(p + 1, s->dev->conf.cyls);
+ put_le16(p + 3, s->dev->conf.heads);
+ 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->sectors);
+ put_le16(p + 6, s->dev->conf.secs);
padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */
put_le16(p + 20, 3); /* XXX: retired, remove ? */
put_le16(p + 21, 512); /* cache size in sectors */
@@ -108,10 +108,10 @@ static void ide_identify(IDEState *s)
put_le16(p + 51, 0x200); /* PIO transfer cycle */
put_le16(p + 52, 0x200); /* DMA transfer cycle */
put_le16(p + 53, 1 | (1 << 1) | (1 << 2)); /* words 54-58,64-70,88 are valid */
- put_le16(p + 54, s->cylinders);
- put_le16(p + 55, s->heads);
- put_le16(p + 56, s->sectors);
- oldsize = s->cylinders * s->heads * s->sectors;
+ put_le16(p + 54, s->dev->conf.cyls);
+ put_le16(p + 55, s->dev->conf.heads);
+ put_le16(p + 56, s->dev->conf.secs);
+ oldsize = s->dev->conf.cyls * s->dev->conf.heads * s->dev->conf.secs;
put_le16(p + 57, oldsize);
put_le16(p + 58, oldsize >> 16);
if (s->mult_sectors)
@@ -249,12 +249,12 @@ static void ide_cfata_identify(IDEState *s)
memset(p, 0, sizeof(s->identify_data));
- cur_sec = s->cylinders * s->heads * s->sectors;
+ cur_sec = s->dev->conf.cyls * s->dev->conf.heads * s->dev->conf.secs;
put_le16(p + 0, 0x848a); /* CF Storage Card signature */
- put_le16(p + 1, s->cylinders); /* Default cylinders */
- put_le16(p + 3, s->heads); /* Default heads */
- put_le16(p + 6, s->sectors); /* Default sectors per track */
+ put_le16(p + 1, s->dev->conf.cyls); /* Default cylinders */
+ put_le16(p + 3, s->dev->conf.heads); /* Default heads */
+ 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 */
@@ -270,9 +270,9 @@ static void ide_cfata_identify(IDEState *s)
put_le16(p + 51, 0x0002); /* PIO cycle timing mode */
put_le16(p + 52, 0x0001); /* DMA cycle timing mode */
put_le16(p + 53, 0x0003); /* Translation params valid */
- put_le16(p + 54, s->cylinders); /* Current cylinders */
- put_le16(p + 55, s->heads); /* Current heads */
- put_le16(p + 56, s->sectors); /* Current sectors */
+ put_le16(p + 54, s->dev->conf.cyls); /* Current cylinders */
+ put_le16(p + 55, s->dev->conf.heads); /* Current heads */
+ put_le16(p + 56, s->dev->conf.secs); /* Current sectors */
put_le16(p + 57, cur_sec); /* Current capacity */
put_le16(p + 58, cur_sec >> 16); /* Current capacity */
if (s->mult_sectors) /* Multiple sector setting */
@@ -462,8 +462,10 @@ int64_t ide_get_sector(IDEState *s)
((int64_t) s->lcyl << 8) | s->sector;
}
} else {
- sector_num = ((s->hcyl << 8) | s->lcyl) * s->heads * s->sectors +
- (s->select & 0x0f) * s->sectors + (s->sector - 1);
+ sector_num =
+ (((s->hcyl << 8) | s->lcyl) * s->dev->conf.heads
+ + (s->select & 0x0f)) * s->dev->conf.secs
+ + (s->sector - 1);
}
return sector_num;
}
@@ -486,12 +488,12 @@ void ide_set_sector(IDEState *s, int64_t sector_num)
s->hob_hcyl = sector_num >> 40;
}
} else {
- cyl = sector_num / (s->heads * s->sectors);
- r = sector_num % (s->heads * s->sectors);
+ cyl = sector_num / (s->dev->conf.heads * s->dev->conf.secs);
+ r = sector_num % (s->dev->conf.heads * s->dev->conf.secs);
s->hcyl = cyl >> 8;
s->lcyl = cyl;
- s->select = (s->select & 0xf0) | ((r / s->sectors) & 0x0f);
- s->sector = (r % s->sectors) + 1;
+ s->select = (s->select & 0xf0) | ((r / s->dev->conf.secs) & 0x0f);
+ s->sector = (r % s->dev->conf.secs) + 1;
}
}
@@ -2088,9 +2090,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,
- uint64_t wwn,
- uint32_t cylinders, uint32_t heads, uint32_t secs,
- int chs_trans)
+ uint64_t wwn)
{
BlockDriverState *bs = s->dev->conf.bs;
uint64_t nb_sectors;
@@ -2098,10 +2098,6 @@ int ide_init_drive(IDEState *s, IDEDriveKind kind,
s->drive_kind = kind;
bdrv_get_geometry(bs, &nb_sectors);
- s->cylinders = cylinders;
- s->heads = heads;
- s->sectors = secs;
- s->chs_trans = chs_trans;
s->nb_sectors = nb_sectors;
s->wwn = wwn;
/* The SMART values should be preserved across power cycles
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 8f673f8..7a39d44 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -345,7 +345,6 @@ struct IDEState {
uint8_t unit;
/* ide config */
IDEDriveKind drive_kind;
- int cylinders, heads, sectors, chs_trans;
int64_t nb_sectors;
int mult_sectors;
int identify_set;
@@ -547,9 +546,7 @@ 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,
- uint64_t wwn,
- uint32_t cylinders, uint32_t heads, uint32_t secs,
- int chs_trans);
+ 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 f9a4d31..c233d66 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -124,15 +124,15 @@ int ide_get_geometry(BusState *bus, int unit,
return -1;
}
- *cyls = s->cylinders;
- *heads = s->heads;
- *secs = s->sectors;
+ *cyls = s->dev->conf.cyls;
+ *heads = s->dev->conf.heads;
+ *secs = s->dev->conf.secs;
return 0;
}
int ide_get_bios_chs_trans(BusState *bus, int unit)
{
- return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].chs_trans;
+ return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].dev->chs_trans;
}
/* --------------------------------- */
@@ -161,9 +161,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
}
if (ide_init_drive(s, kind,
- dev->version, dev->serial, dev->model, dev->wwn,
- dev->conf.cyls, dev->conf.heads, dev->conf.secs,
- dev->chs_trans) < 0) {
+ dev->version, dev->serial, dev->model, dev->wwn) < 0) {
return -1;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 6/9] ide: Drop redundant IDEState member version
2013-11-20 9:55 [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification armbru
` (4 preceding siblings ...)
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 5/9] ide: Drop redundant IDEState geometry members armbru
@ 2013-11-20 9:55 ` armbru
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 7/9] ide: Drop redundant IDEState member drive_serial_str armbru
` (3 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: armbru @ 2013-11-20 9:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, afaerber
From: Markus Armbruster <armbru@redhat.com>
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 <armbru@redhat.com>
---
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 3dc2de0..f20b3a6 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 275df4b..31e696c 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);
@@ -2089,7 +2089,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;
@@ -2141,12 +2141,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 7a39d44..4c0fb8e 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -372,7 +372,6 @@ struct IDEState {
/* set for lba48 access */
uint8_t lba48;
- char version[9];
/* ATAPI specific */
struct unreported_events events;
uint8_t sense_key;
@@ -545,7 +544,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_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index c233d66..0326360 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -160,14 +160,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.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 7/9] ide: Drop redundant IDEState member drive_serial_str
2013-11-20 9:55 [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification armbru
` (5 preceding siblings ...)
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 6/9] ide: Drop redundant IDEState member version armbru
@ 2013-11-20 9:55 ` armbru
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 8/9] ide: Drop redundant IDEState member model armbru
` (2 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: armbru @ 2013-11-20 9:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, afaerber
From: 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 31e696c..153e7cf 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 8/9] ide: Drop redundant IDEState member model
2013-11-20 9:55 [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification armbru
` (6 preceding siblings ...)
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 7/9] ide: Drop redundant IDEState member drive_serial_str armbru
@ 2013-11-20 9:55 ` armbru
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 9/9] ide: Drop redundant IDEState member wwn armbru
2013-12-18 14:54 ` [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification Markus Armbruster
9 siblings, 0 replies; 14+ messages in thread
From: armbru @ 2013-11-20 9:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, afaerber
From: 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 | 22 +++-------------------
hw/ide/internal.h | 2 --
hw/ide/qdev.c | 16 +++++++++++++++-
3 files changed, 18 insertions(+), 22 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 153e7cf..be5be61 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -99,7 +99,7 @@ static void ide_identify(IDEState *s)
put_le16(p + 21, 512); /* cache size in sectors */
put_le16(p + 22, 4); /* ecc bytes */
padstr((char *)(p + 23), s->dev->version, 8); /* firmware version */
- padstr((char *)(p + 27), s->drive_model_str, 40); /* model */
+ padstr((char *)(p + 27), s->dev->model, 40); /* model */
#if MAX_MULT_SECTORS > 1
put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS);
#endif
@@ -203,7 +203,7 @@ static void ide_atapi_identify(IDEState *s)
put_le16(p + 21, 512); /* cache size in sectors */
put_le16(p + 22, 4); /* ecc bytes */
padstr((char *)(p + 23), s->dev->version, 8); /* firmware version */
- padstr((char *)(p + 27), s->drive_model_str, 40); /* model */
+ padstr((char *)(p + 27), s->dev->model, 40); /* model */
put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */
#ifdef USE_DMA_CDROM
put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */
@@ -260,7 +260,7 @@ static void ide_cfata_identify(IDEState *s)
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 */
+ padstr((char *) (p + 27), s->dev->model, 40); /* Model number */
#if MAX_MULT_SECTORS > 1
put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS);
#else
@@ -2089,7 +2089,6 @@ static const BlockDevOps ide_cd_block_ops = {
};
int ide_init_drive(IDEState *s, IDEDriveKind kind,
- const char *model,
uint64_t wwn)
{
BlockDriverState *bs = s->dev->conf.bs;
@@ -2119,21 +2118,6 @@ int ide_init_drive(IDEState *s, IDEDriveKind kind,
return -1;
}
}
- if (model) {
- pstrcpy(s->drive_model_str, sizeof(s->drive_model_str), model);
- } else {
- switch (kind) {
- case IDE_CD:
- strcpy(s->drive_model_str, "QEMU DVD-ROM");
- break;
- case IDE_CFATA:
- strcpy(s->drive_model_str, "QEMU MICRODRIVE");
- break;
- default:
- strcpy(s->drive_model_str, "QEMU HARDDISK");
- break;
- }
- }
ide_reset(s);
bdrv_iostatus_enable(bs);
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index f6b5b7a..c4a5773 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_model_str[41];
uint64_t wwn;
/* ide regs */
uint8_t feature;
@@ -543,7 +542,6 @@ 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 *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 c1a4cb6..915bc27 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -168,8 +168,22 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
dev->version = g_strdup(qemu_get_version());
}
+ if (!dev->model) {
+ switch (kind) {
+ case IDE_CD:
+ dev->model = g_strdup("QEMU DVD-ROM");
+ break;
+ case IDE_CFATA:
+ dev->model = g_strdup("QEMU MICRODRIVE");
+ break;
+ default:
+ dev->model = g_strdup("QEMU HARDDISK");
+ break;
+ }
+ }
+
if (ide_init_drive(s, kind,
- dev->model, dev->wwn) < 0) {
+ dev->wwn) < 0) {
return -1;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 9/9] ide: Drop redundant IDEState member wwn
2013-11-20 9:55 [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification armbru
` (7 preceding siblings ...)
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 8/9] ide: Drop redundant IDEState member model armbru
@ 2013-11-20 9:55 ` armbru
2013-12-18 14:54 ` [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification Markus Armbruster
9 siblings, 0 replies; 14+ messages in thread
From: armbru @ 2013-11-20 9:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, afaerber
From: Markus Armbruster <armbru@redhat.com>
It's a copy of dev->wwn. 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 | 18 ++++++++----------
hw/ide/internal.h | 4 +---
hw/ide/qdev.c | 3 +--
3 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index be5be61..008d9bc 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -142,7 +142,7 @@ static void ide_identify(IDEState *s)
/* 13=flush_cache_ext,12=flush_cache,10=lba48 */
put_le16(p + 83, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10));
/* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */
- if (s->wwn) {
+ if (s->dev->wwn) {
put_le16(p + 84, (1 << 14) | (1 << 8) | 0);
} else {
put_le16(p + 84, (1 << 14) | 0);
@@ -156,7 +156,7 @@ static void ide_identify(IDEState *s)
/* 13=flush_cache_ext,12=flush_cache,10=lba48 */
put_le16(p + 86, (1 << 13) | (1 <<12) | (1 << 10));
/* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */
- if (s->wwn) {
+ if (s->dev->wwn) {
put_le16(p + 87, (1 << 14) | (1 << 8) | 0);
} else {
put_le16(p + 87, (1 << 14) | 0);
@@ -170,12 +170,12 @@ static void ide_identify(IDEState *s)
if (dev && dev->conf.physical_block_size)
put_le16(p + 106, 0x6000 | get_physical_block_exp(&dev->conf));
- if (s->wwn) {
+ if (s->dev->wwn) {
/* LE 16-bit words 111-108 contain 64-bit World Wide Name */
- put_le16(p + 108, s->wwn >> 48);
- put_le16(p + 109, s->wwn >> 32);
- put_le16(p + 110, s->wwn >> 16);
- put_le16(p + 111, s->wwn);
+ put_le16(p + 108, s->dev->wwn >> 48);
+ put_le16(p + 109, s->dev->wwn >> 32);
+ put_le16(p + 110, s->dev->wwn >> 16);
+ put_le16(p + 111, s->dev->wwn);
}
if (dev && dev->conf.discard_granularity) {
put_le16(p + 169, 1); /* TRIM support */
@@ -2088,8 +2088,7 @@ static const BlockDevOps ide_cd_block_ops = {
.is_medium_locked = ide_cd_is_medium_locked,
};
-int ide_init_drive(IDEState *s, IDEDriveKind kind,
- uint64_t wwn)
+int ide_init_drive(IDEState *s, IDEDriveKind kind)
{
BlockDriverState *bs = s->dev->conf.bs;
uint64_t nb_sectors;
@@ -2098,7 +2097,6 @@ int ide_init_drive(IDEState *s, IDEDriveKind kind,
bdrv_get_geometry(bs, &nb_sectors);
s->nb_sectors = nb_sectors;
- s->wwn = wwn;
/* The SMART values should be preserved across power cycles
but they aren't. */
s->smart_enabled = 1;
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index c4a5773..95ea75c 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;
- uint64_t wwn;
/* ide regs */
uint8_t feature;
uint8_t error;
@@ -541,8 +540,7 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr);
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,
- uint64_t wwn);
+int ide_init_drive(IDEState *s, IDEDriveKind kind);
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 915bc27..17404b8 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -182,8 +182,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
}
}
- if (ide_init_drive(s, kind,
- dev->wwn) < 0) {
+ if (ide_init_drive(s, kind) < 0) {
return -1;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification
2013-11-20 9:55 [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification armbru
` (8 preceding siblings ...)
2013-11-20 9:55 ` [Qemu-devel] [PATCH v2 9/9] ide: Drop redundant IDEState member wwn armbru
@ 2013-12-18 14:54 ` Markus Armbruster
2013-12-18 15:38 ` Andreas Färber
9 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2013-12-18 14:54 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, afaerber
Ping?
armbru@redhat.com writes:
> From: Markus Armbruster <armbru@redhat.com>
>
> Obvious cleanups possible since we no longer have the special case of
> a non-qdevified controller.
>
> v2:
> * Dropped PATCH 1/10 ide: Break all non-qdevified controllers
> Andreas qdevified them since; thanks!
> * Series renamed from "Drop code for non-qdevified IDE, and clean up"
> * Trivially rebased
>
> Markus Armbruster (9):
> ide: Move IDEDevice pointer from IDEBus to IDEState
> ide: Use IDEState member dev for "device connected" test
> ide: Don't block-align IDEState member smart_selftest_data
> ide: Drop redundant IDEState member bs
> ide: Drop redundant IDEState geometry members
> ide: Drop redundant IDEState member version
> ide: Drop redundant IDEState member drive_serial_str
> ide: Drop redundant IDEState member model
> ide: Drop redundant IDEState member wwn
>
> hw/ide/ahci.c | 19 +++--
> hw/ide/atapi.c | 39 +++++----
> hw/ide/core.c | 235 +++++++++++++++++++++++-----------------------------
> hw/ide/internal.h | 15 +---
> hw/ide/macio.c | 26 +++---
> hw/ide/microdrive.c | 2 +-
> hw/ide/piix.c | 4 -
> hw/ide/qdev.c | 50 ++++++-----
> 8 files changed, 181 insertions(+), 209 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification
2013-12-18 14:54 ` [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification Markus Armbruster
@ 2013-12-18 15:38 ` Andreas Färber
2013-12-18 17:05 ` Markus Armbruster
0 siblings, 1 reply; 14+ messages in thread
From: Andreas Färber @ 2013-12-18 15:38 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: Kevin Wolf, Alexander Graf, Stefan Hajnoczi
I had a brief look at this series. Dropping redundant fields certainly
sounds good and after the lengthy QOM'ifications of IDE devices we seem
to no longer break any devices, but whether to check for device or
BlockDriverState sounds more like a block topic to me...
Regards,
Andreas
Am 18.12.2013 15:54, schrieb Markus Armbruster:
> Ping?
>
> armbru@redhat.com writes:
>
>> From: Markus Armbruster <armbru@redhat.com>
>>
>> Obvious cleanups possible since we no longer have the special case of
>> a non-qdevified controller.
>>
>> v2:
>> * Dropped PATCH 1/10 ide: Break all non-qdevified controllers
>> Andreas qdevified them since; thanks!
>> * Series renamed from "Drop code for non-qdevified IDE, and clean up"
>> * Trivially rebased
>>
>> Markus Armbruster (9):
>> ide: Move IDEDevice pointer from IDEBus to IDEState
>> ide: Use IDEState member dev for "device connected" test
>> ide: Don't block-align IDEState member smart_selftest_data
>> ide: Drop redundant IDEState member bs
>> ide: Drop redundant IDEState geometry members
>> ide: Drop redundant IDEState member version
>> ide: Drop redundant IDEState member drive_serial_str
>> ide: Drop redundant IDEState member model
>> ide: Drop redundant IDEState member wwn
>>
>> hw/ide/ahci.c | 19 +++--
>> hw/ide/atapi.c | 39 +++++----
>> hw/ide/core.c | 235 +++++++++++++++++++++++-----------------------------
>> hw/ide/internal.h | 15 +---
>> hw/ide/macio.c | 26 +++---
>> hw/ide/microdrive.c | 2 +-
>> hw/ide/piix.c | 4 -
>> hw/ide/qdev.c | 50 ++++++-----
>> 8 files changed, 181 insertions(+), 209 deletions(-)
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification
2013-12-18 15:38 ` Andreas Färber
@ 2013-12-18 17:05 ` Markus Armbruster
2013-12-19 9:30 ` Stefan Hajnoczi
0 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2013-12-18 17:05 UTC (permalink / raw)
To: Andreas Färber
Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Alexander Graf
Andreas Färber <afaerber@suse.de> writes:
> I had a brief look at this series. Dropping redundant fields certainly
> sounds good and after the lengthy QOM'ifications of IDE devices we seem
> to no longer break any devices, but whether to check for device or
> BlockDriverState sounds more like a block topic to me...
I read that as "ack from a block maintainer wanted for '[PATCH v2 2/9]
ide: Use IDEState member dev for "device connected" test'". Correct?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification
2013-12-18 17:05 ` Markus Armbruster
@ 2013-12-19 9:30 ` Stefan Hajnoczi
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2013-12-19 9:30 UTC (permalink / raw)
To: Markus Armbruster
Cc: Kevin Wolf, Alexander Graf, Andreas Färber, Stefan Hajnoczi,
qemu-devel
On Wed, Dec 18, 2013 at 06:05:39PM +0100, Markus Armbruster wrote:
> Andreas Färber <afaerber@suse.de> writes:
>
> > I had a brief look at this series. Dropping redundant fields certainly
> > sounds good and after the lengthy QOM'ifications of IDE devices we seem
> > to no longer break any devices, but whether to check for device or
> > BlockDriverState sounds more like a block topic to me...
>
> I read that as "ack from a block maintainer wanted for '[PATCH v2 2/9]
> ide: Use IDEState member dev for "device connected" test'". Correct?
IDE is not something I'm familiar with so I suggest we wait for Kevin to
return from vacation.
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread