Re: [Qemu-devel] [PATCH v3] make windows notice media change

[Filip Navara <filip.navara@gmail.com>]

On Wed, Jul 29, 2009 at 6:09 PM, Gleb Natapov<gleb@redhat.com> wrote:
> @@ -3250,6 +3253,8 @@ 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]);
> +        if (version_id == 3)
> +            qemu_get_8s(f, &d->ide_if[i].cdrom_changed);
>     }
>     return 0;
>  }

I'd prefer passing the version to ide_load and doing the actual load there...

... but the patch is all wrong and based on wrong assumptions, which is
much more fundamental problem. Windows cdrom driver is not that stupid
about the change as you think.

The cdrom driver really has a timer and polls the IDE controller, but it
doesn't require the intermediate ASC_MEDIUM_NOT_PRESENT state
you introduced. It's perfectly ok to return SENSE_UNIT_ATTENTION /
ASC_MEDIUM_MAY_HAVE_CHANGED from GPCMD_TEST_UNIT_READY
and Windows will recognize it as medium change.

Something like this should work:
        if (bdrv_is_inserted(s->bs)) {
            if (s->cdrom_changed) {
                ide_atapi_cmd_error(s, SENSE_UNIT_ATTENTION,
                                 ASC_MEDIUM_MAY_HAVE_CHANGED);
                s->cdrom_changed = 0;
            } else {
                ide_atapi_cmd_ok(s);
            }
        } else {
            ide_atapi_cmd_error(s, SENSE_NOT_READY,
                                ASC_MEDIUM_NOT_PRESENT);
        }

The benefit is that it will not break guests which issue the request only
once.

Best regards,
Filip Navara