On Mon, 2012-11-26 at 16:08 +0100, Kevin Wolf wrote: > Am 26.11.2012 15:56, schrieb Pavel Hrdina: > > On Mon, 2012-11-26 at 14:55 +0100, Kevin Wolf wrote: > >> Am 26.11.2012 13:43, schrieb Pavel Hrdina: > >>> On Fri, 2012-11-23 at 13:09 +0100, Kevin Wolf wrote: > >>>> Another thing I would consider is using cdrom_changed = 0/1/2 instead of > >>>> adding fake_cdrom_eject to add another state. Migration would > >>>> automatically do the right thing for it, old versions would in both 1 > >>>> and 2 state switch to ASC_MEDIUM_MAY_HAVE_CHANGED next, which is correct > >>>> in the latter case and is in the former case the same bug as the old > >>>> qemu we're migrating to has anyway. > >>>> > >>> > >>> I do it that way at first, but then I rewrite it, because I thought that > >>> using new state would be better. But if you agree with cdrom_changed = > >>> 0/1/2, I'll change it. > >> > >> I think it's nicer to have only one state. And if I'm not mistaken, we > >> can even do without the pre_save/post_load handlers then. > >> > >> Kevin > > > > I don't think that we can do it without the pre_save/post_load handlers, > > because if the cdrom_changed could be set also to 2 then in case, that > > you migrate immediately after the change from "buggy" qemu to the > > "fixed" qemu we have in cdrom_changed state 1, but we need state 2. > > But you can't tell which is the right state anyway, because the buggy > qemu on your source has only one "changed" state. Choosing state 1 seems > fine to me. The worst thing that can happen is that you return > ASC_MEDIUM_NOT_PRESENT twice - not a big deal.
At first I wanted to make state 1 as ASC_MEDIUM_MAY_HAVE_CHANGED and state 2 as ASC_MEDIUM_NOT_PRESENT. In that case I would have to in post_load and in ide_cd_change_cb set cdrom_changed to 2. But if we switch these states, then only what we have to do is fix the code in atapi.c. I'll sent the patch. > > > And > > otherwise, if you want to migrate from "fixed" qemu to "buggy" qemu > > where version_id is lower then 3, you have to set the asc to > > ASC_MEDIUM_MAY_HAVE_CHANGED. > > This part is true. However, I'm pretty sure that migration from 1.3 to > 0.11 doesn't work anyway, so why bother? Yes, you're right. Pavel > > Kevin >
