[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4] make windows notice media change
From: |
Filip Navara |
Subject: |
Re: [Qemu-devel] [PATCH v4] make windows notice media change |
Date: |
Sun, 2 Aug 2009 10:21:00 +0200 |
On Sun, Aug 2, 2009 at 10:06 AM, Gleb Natapov<address@hidden> wrote:
> Windows seems to be very stupid about cdrom media change. It polls
> cdrom status and if status goes ready->media not present->ready
> it assumes that media was changed. If "media not present" step doesn't
> happen even if "medium may have changed" was seen it assumes media
> haven't changed. Fake "media not present" step.
>
> Filip Navara did a great job debugging this issue in Windows and this is
> what he found out:
>
> BINGO! ... The media present notifications were broken ever since
> Windows 2000 it seems. The media change is detected properly and it's
> passed to ClassSetMediaChangeState function which in turn calls
> ClasspInternalSetMediaChangeState. This function is responsible for
> changing some internal state of the device object and sending the PnP
> events which later result in application notifications. It has this
> tiny bit of code (not copied byte for byte):
>
> if (oldMediaState == NewState) {
> // Media is in the same state it was before.
> return;
> }
>
> so the end result is that for the case of UNIT NEEDS ATTENTION /
> MEDIUM MAY HAVE CHANGED without NOT READY in-between is really broken.
> It results in the internal media change counter incremented, so the
> media contents are re-read when necessary, instead of relying on the
> cache, but the notifications to applications are never sent.
>
> Signed-off-by: Gleb Natapov <address@hidden>
> ---
> v1->v2:
> do not reuse media_changed. Save/restore on migration.
>
> v2->v3:
> allow migration from 2 to 3.
>
> v3->v4:
> fix migration from 2 to 3 for pmac and md.
>
> diff --git a/hw/ide.c b/hw/ide.c
> index 6cf04a6..268e589 100644
> --- a/hw/ide.c
> +++ b/hw/ide.c
> @@ -418,6 +418,7 @@ typedef struct IDEState {
> /* ATAPI specific */
> uint8_t sense_key;
> uint8_t asc;
> + uint8_t cdrom_changed;
> int packet_transfer_size;
> int elementary_transfer_size;
> int io_buffer_index;
> @@ -1660,9 +1661,10 @@ static void ide_atapi_cmd(IDEState *s)
> }
> switch(s->io_buffer[0]) {
> case GPCMD_TEST_UNIT_READY:
> - if (bdrv_is_inserted(s->bs)) {
> + if (bdrv_is_inserted(s->bs) && !s->cdrom_changed) {
> ide_atapi_cmd_ok(s);
> } else {
> + s->cdrom_changed = 0;
> ide_atapi_cmd_error(s, SENSE_NOT_READY,
> ASC_MEDIUM_NOT_PRESENT);
> }
> @@ -2122,7 +2124,7 @@ static void cdrom_change_cb(void *opaque)
>
> s->sense_key = SENSE_UNIT_ATTENTION;
> s->asc = ASC_MEDIUM_MAY_HAVE_CHANGED;
> -
> + s->cdrom_changed = 1;
> ide_set_irq(s);
> }
>
> @@ -2890,7 +2892,7 @@ static void ide_save(QEMUFile* f, IDEState *s)
> }
>
> /* load per IDE drive data */
> -static void ide_load(QEMUFile* f, IDEState *s)
> +static void ide_load(QEMUFile* f, IDEState *s, int version_id)
> {
> s->mult_sectors=qemu_get_be32(f);
> s->identify_set=qemu_get_be32(f);
> @@ -2914,6 +2916,13 @@ static void ide_load(QEMUFile* f, IDEState *s)
>
> qemu_get_8s(f, &s->sense_key);
> qemu_get_8s(f, &s->asc);
> + if (version_id == 3) {
> + qemu_get_8s(f, &s->cdrom_changed);
> + } else {
> + if (s->sense_key == SENSE_UNIT_ATTENTION &&
> + s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED)
> + s->cdrom_changed = 1;
> + }
> /* XXX: if a transfer is pending, we do not save it yet */
> }
>
> @@ -3235,7 +3244,7 @@ static int pci_ide_load(QEMUFile* f, void *opaque, int
> version_id)
> PCIIDEState *d = opaque;
> int ret, i;
>
> - if (version_id != 2)
> + if (version_id != 2 || version_id != 3)
> return -EINVAL;
Should be &&
> ret = pci_device_load(&d->dev, f);
> if (ret < 0)
> @@ -3265,7 +3274,7 @@ static int pci_ide_load(QEMUFile* f, void *opaque, int
> version_id)
>
> /* per IDE drive data */
> for(i = 0; i < 4; i++) {
> - ide_load(f, &d->ide_if[i]);
> + ide_load(f, &d->ide_if[i], version_id);
> }
> return 0;
> }
> @@ -3355,7 +3364,7 @@ void pci_cmd646_ide_init(PCIBus *bus, BlockDriverState
> **hd_table,
> ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], irq[0]);
> ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], irq[1]);
>
> - register_savevm("ide", 0, 2, pci_ide_save, pci_ide_load, d);
> + register_savevm("ide", 0, 3, pci_ide_save, pci_ide_load, d);
> qemu_register_reset(cmd646_reset, d);
> cmd646_reset(d);
> }
> @@ -3414,7 +3423,7 @@ void pci_piix3_ide_init(PCIBus *bus, BlockDriverState
> **hd_table, int devfn,
> if (hd_table[i])
> hd_table[i]->private = &d->dev;
>
> - register_savevm("ide", 0, 2, pci_ide_save, pci_ide_load, d);
> + register_savevm("ide", 0, 3, pci_ide_save, pci_ide_load, d);
> }
>
> /* hd_table must contain 4 block drivers */
> @@ -3450,7 +3459,7 @@ void pci_piix4_ide_init(PCIBus *bus, BlockDriverState
> **hd_table, int devfn,
> ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
> ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
>
> - register_savevm("ide", 0, 2, pci_ide_save, pci_ide_load, d);
> + register_savevm("ide", 0, 3, pci_ide_save, pci_ide_load, d);
> }
>
> #if defined(TARGET_PPC)
> @@ -3738,7 +3747,7 @@ static int pmac_ide_load(QEMUFile *f, void *opaque, int
> version_id)
> uint8_t drive1_selected;
> unsigned int i;
>
> - if (version_id != 1)
> + if (version_id != 1 || version_id != 3)
> return -EINVAL;
Should be &&
>
> /* per IDE interface data */
> @@ -3748,7 +3757,7 @@ static int pmac_ide_load(QEMUFile *f, void *opaque, int
> version_id)
>
> /* per IDE drive data */
> for(i = 0; i < 2; i++) {
> - ide_load(f, &s[i]);
> + ide_load(f, &s[i], version_id);
> }
> return 0;
> }
> @@ -3779,7 +3788,7 @@ int pmac_ide_init (BlockDriverState **hd_table,
> qemu_irq irq,
>
> pmac_ide_memory = cpu_register_io_memory(pmac_ide_read,
> pmac_ide_write, d);
> - register_savevm("ide", 0, 1, pmac_ide_save, pmac_ide_load, d);
> + register_savevm("ide", 0, 3, pmac_ide_save, pmac_ide_load, d);
> qemu_register_reset(pmac_ide_reset, d);
> pmac_ide_reset(d);
>
> @@ -4172,6 +4181,9 @@ static int md_load(QEMUFile *f, void *opaque, int
> version_id)
> int i;
> uint8_t drive1_selected;
>
> + if (version_id != 0 || version_id != 3)
> + return -EINVAL;
> +
Should be &&
> qemu_get_8s(f, &s->opt);
> qemu_get_8s(f, &s->stat);
> qemu_get_8s(f, &s->pins);
> @@ -4185,7 +4197,7 @@ static int md_load(QEMUFile *f, void *opaque, int
> version_id)
> s->ide->cur_drive = &s->ide[(drive1_selected != 0)];
>
> for (i = 0; i < 2; i ++)
> - ide_load(f, &s->ide[i]);
> + ide_load(f, &s->ide[i], version_id);
>
> return 0;
> }
> @@ -4416,7 +4428,7 @@ PCMCIACardState *dscm1xxxx_init(BlockDriverState *bdrv)
> md->ide->mdata_size = METADATA_SIZE;
> md->ide->mdata_storage = (uint8_t *) qemu_mallocz(METADATA_SIZE);
>
> - register_savevm("microdrive", -1, 0, md_save, md_load, md);
> + register_savevm("microdrive", -1, 3, md_save, md_load, md);
>
> return &md->card;
> }
> --
> Gleb.
Otherwise it looks good. Thanks for taking the time to help me with
the debugging.
Best regards,
Filip Navara