Message ID | 5AB9C3BF-1A4F-4942-A433-1724BBFC9D23@alex.org.uk |
---|---|
State | New |
Headers | show |
On (Tue) 22 Jul 2014 [12:38:14], Alex Bligh wrote: > > On 22 Jul 2014, at 11:54, Amit Shah <amit.shah@redhat.com> wrote: > >> > >> This one, together with the PIIX4 one (which for some reason doesn't > >> show up) where the two I hit, after manually fixing the rom sizes > >> stuff on the command line. > >> > >> Apparently "flags" and "channels" are pseudonyms. > > > > No, they're not; flags is an extra 4-byte param in qemu-kvm-1.0, which > > doesn't exist in qemu-1.0. > > > > In qemu-2.1 -M 1.0, "flags" got renamed to "channels[0].irq_disabled", > > which is also flagged. > > I was basing that on the comment in Cole Robinson's patch here: > http://pkgs.fedoraproject.org/cgit/qemu.git/tree/0001-Fix-migration-from-qemu-kvm.patch?h=f20 > > --- a/hw/timer/i8254_common.c > +++ b/hw/timer/i8254_common.c > @@ -267,7 +267,12 @@ static const VMStateDescription vmstate_pit_common = { > .pre_save = pit_dispatch_pre_save, > .post_load = pit_dispatch_post_load, > .fields = (VMStateField[]) { > - VMSTATE_UINT32_V(channels[0].irq_disabled, PITCommonState, 3), > + /* qemu-kvm version_id=2 had 'flags' here which is equivalent > + * This fixes incoming migration from qemu-kvm 1.0, but breaks > + * incoming migration from qemu < 1.1 > + */ > + //VMSTATE_UINT32_V(channels[0].irq_disabled, PITCommonState, 3), > + VMSTATE_UINT32(channels[0].irq_disabled, PITCommonState), > VMSTATE_STRUCT_ARRAY(channels, PITCommonState, 3, 2, > vmstate_pit_channel, PITChannelState), > VMSTATE_INT64(channels[0].next_transition_time, > > I think what this means is that flags is equivalent to channels[0].irq_disabled, > yes? Yes, that's what I said above. > >> The piix4_m thing > >> is that qemu-kvm-1.0 is actually using version 3 but says it's > >> using version 2, which is perhaps why it doesn't show up in your > >> test. > > > > You're looking at the wrong output. You should see one message back, > > where I compare qemu-1.0 with qemu-2.1 -M pc-1.0. That has the > > piix4_pm version mismatch flagged. > > Don't think I am > > a) because I'm using qemu-kvm-1.0, not qemu-1.0. You looked at qemu-1.0 > in the previous message; that's not directly relevant for what I'm > looking at as my source is qemu-kvm-1.0 (qemu vs qemu-kvm). Sigh; please read both my replies. The only difference between qemu-kvm-1.0 and qemu-1.0 is the presence of the pci-assign section in qemu-kvm-1.0. All other output from qemu-1.0 -> qemu-2.1 is equally applicable to qemu-kvm-1.0 -> qemu-2.1. > b) because I wasn't actually using your Json, but testing what made the > thing load in real life! I know that; but you brought up the comparison of your experiments with the output of the checker. I just pointed out the checker flagged both those things you saw (and a few others too). Amit
Il 22/07/2014 13:54, Amit Shah ha scritto: >> > a) because I'm using qemu-kvm-1.0, not qemu-1.0. You looked at qemu-1.0 >> > in the previous message; that's not directly relevant for what I'm >> > looking at as my source is qemu-kvm-1.0 (qemu vs qemu-kvm). > Sigh; please read both my replies. > > The only difference between qemu-kvm-1.0 and qemu-1.0 is the presence > of the pci-assign section in qemu-kvm-1.0. All other output from > qemu-1.0 -> qemu-2.1 is equally applicable to qemu-kvm-1.0 -> > qemu-2.1. Not really true. qemu 1.0 didn't have neither channels[0].irq_disabled nor flags and is version 2. qemu-2.1 has channels[0].irq_disabled and is version 3. qemu-kvm-1.0 had flags, which is now called channels[0].irq_disabled, and is version 2. qemu-2.1 loads version 2 as something that doesn't have channels[0].irq_disabled. So it breaks if you feed it with qemu-kvm-1.0 data. There's something similar going on with PIIX4_PM but I don't remember the details. Paolo
On 22 Jul 2014, at 12:54, Amit Shah <amit.shah@redhat.com> wrote: > Sigh; please read both my replies. > > The only difference between qemu-kvm-1.0 and qemu-1.0 is the presence > of the pci-assign section in qemu-kvm-1.0. All other output from > qemu-1.0 -> qemu-2.1 is equally applicable to qemu-kvm-1.0 -> > qemu-2.1. ... > I know that; but you brought up the comparison of your experiments > with the output of the checker. I just pointed out the checker > flagged both those things you saw (and a few others too). Apologies for my naivety here as it's the first time I've looked at this stuff. So sorry if I'm getting the wrong end of the stick. However, am I correct in assuming your JSON checker is only checking for the field structure, and not for things like changed defaults? For instance the cirrus vga video ram size appears to be different between qemu-kvm-1.0 and qemu-1.0, but this isn't something brought up in your email highlighting the difference between a 2.1->qemu-1.0 comparison and a 2.1->qemu-kvm-1.0 comparison. I know you haven't directly compared qemu-1.0 and qemu-1.0-kvm, but if this difference does exist (which I think it does) I'd expect to see it in the list of differences (i.e. your second email), if your tool looks at that too.
On 22 Jul 2014, at 13:12, Paolo Bonzini <pbonzini@redhat.com> wrote: > There's something similar going on with PIIX4_PM but I don't remember > the details. From memory: * qemu-1.0 uses the v2 format * qemu-kvm-1.0 uses the v3 format but advertises itself as v2 * qemu-2.1 uses the v3 format I don't quite understand whether Amit's checker is designed to pick up all three situations as different, or whether it (quite reasonably) expects people to play ball with version numbers.
On (Tue) 22 Jul 2014 [14:12:00], Paolo Bonzini wrote: > Il 22/07/2014 13:54, Amit Shah ha scritto: > >> > a) because I'm using qemu-kvm-1.0, not qemu-1.0. You looked at qemu-1.0 > >> > in the previous message; that's not directly relevant for what I'm > >> > looking at as my source is qemu-kvm-1.0 (qemu vs qemu-kvm). > > Sigh; please read both my replies. > > > > The only difference between qemu-kvm-1.0 and qemu-1.0 is the presence > > of the pci-assign section in qemu-kvm-1.0. All other output from > > qemu-1.0 -> qemu-2.1 is equally applicable to qemu-kvm-1.0 -> > > qemu-2.1. > > Not really true. Here's the complete output: Section "xio3130-downstream" Description "xio3130-express-downstream-port": version error: 2 > 0 Section "usb-host" Section "usb-host" Description "usb-host": minimum version error: 0 < 1 Section "ich9-usb-ehci1" Section "ich9-usb-ehci1" Description "ehci": minimum version error: 0 < 1 Section "x3130-upstream" Description "xio3130-express-upstream-port": version error: 2 > 0 Section "usb-ehci" Section "usb-ehci" Description "ehci": minimum version error: 0 < 1 Section "ioh3420" Description "ioh-3240-express-root-port": version error: 2 > 0 Section "pci-assign" does not exist in dest Section "PIIX4_PM" Section "PIIX4_PM" Description "piix4_pm": minimum version error: 2 < 3 > qemu 1.0 didn't have neither channels[0].irq_disabled nor flags and is > version 2. > > qemu-2.1 has channels[0].irq_disabled and is version 3. > > qemu-kvm-1.0 had flags, which is now called channels[0].irq_disabled, > and is version 2. Right - 'flags' here is renamed, and doesn't appear in that output; but the src version was 0 according to the json output. > qemu-2.1 loads version 2 as something that doesn't have > channels[0].irq_disabled. So it breaks if you feed it with qemu-kvm-1.0 > data. OK - that's not something the static checker can flag, though.. > There's something similar going on with PIIX4_PM but I don't remember > the details. According to the output above, minimum version that qemu-2.1 accepts changed to 3, and qemu-kvm-1.0 sends version 2. Amit
On (Tue) 22 Jul 2014 [13:15:41], Alex Bligh wrote: > > On 22 Jul 2014, at 12:54, Amit Shah <amit.shah@redhat.com> wrote: > > > Sigh; please read both my replies. > > > > The only difference between qemu-kvm-1.0 and qemu-1.0 is the presence > > of the pci-assign section in qemu-kvm-1.0. All other output from > > qemu-1.0 -> qemu-2.1 is equally applicable to qemu-kvm-1.0 -> > > qemu-2.1. > ... > > I know that; but you brought up the comparison of your experiments > > with the output of the checker. I just pointed out the checker > > flagged both those things you saw (and a few others too). > > Apologies for my naivety here as it's the first time I've looked > at this stuff. So sorry if I'm getting the wrong end of the stick. > > However, am I correct in assuming your JSON checker is only > checking for the field structure, and not for things like > changed defaults? It only looks at the vmstate structs. > For instance the cirrus vga video ram size > appears to be different between qemu-kvm-1.0 and qemu-1.0, Right, this won't get flagged. > but this isn't something brought up in your email highlighting > the difference between a 2.1->qemu-1.0 comparison and a > 2.1->qemu-kvm-1.0 comparison. I know you haven't directly > compared qemu-1.0 and qemu-1.0-kvm, but if this difference > does exist (which I think it does) I'd expect to see it > in the list of differences (i.e. your second email), if your > tool looks at that too. I did compare qemu-1.0 and qemu-kvm-1.0, but this difference won't pop up. The other differences that the checker spotted may not be relevant to you, as your VMs don't have those devices enabled. Amit
On (Tue) 22 Jul 2014 [13:19:43], Alex Bligh wrote: > > On 22 Jul 2014, at 13:12, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > There's something similar going on with PIIX4_PM but I don't remember > > the details. > > From memory: > > * qemu-1.0 uses the v2 format > * qemu-kvm-1.0 uses the v3 format but advertises itself as v2 > * qemu-2.1 uses the v3 format Yea; I vaguely recall that happening. Quite unfortunate. > I don't quite understand whether Amit's checker is designed to pick > up all three situations as different, or whether it (quite reasonably) > expects people to play ball with version numbers. This is what it says: Section "PIIX4_PM" Section "PIIX4_PM" Description "piix4_pm": minimum version error: 2 < 3 Which means qemu-2.1 expects a minimum version of 3, but qemu-kvm-1.0 is sending it version 2. Now even though they may be compatible, version-number-wise they're not, as the dest qemu will also complain about. Amit
--- a/hw/timer/i8254_common.c +++ b/hw/timer/i8254_common.c @@ -267,7 +267,12 @@ static const VMStateDescription vmstate_pit_common = { .pre_save = pit_dispatch_pre_save, .post_load = pit_dispatch_post_load, .fields = (VMStateField[]) { - VMSTATE_UINT32_V(channels[0].irq_disabled, PITCommonState, 3), + /* qemu-kvm version_id=2 had 'flags' here which is equivalent + * This fixes incoming migration from qemu-kvm 1.0, but breaks + * incoming migration from qemu < 1.1 + */ + //VMSTATE_UINT32_V(channels[0].irq_disabled, PITCommonState, 3), + VMSTATE_UINT32(channels[0].irq_disabled, PITCommonState), VMSTATE_STRUCT_ARRAY(channels, PITCommonState, 3, 2, vmstate_pit_channel, PITChannelState), VMSTATE_INT64(channels[0].next_transition_time,