| Message ID | 1300839376-22520-8-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
Anthony Liguori <aliguori@us.ibm.com> wrote: > I don't fully understand this hack business but we need field to be unique so.. > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > --- > hw/eeprom93xx.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/hw/eeprom93xx.c b/hw/eeprom93xx.c > index cfa695d..f1d75ec 100644 > --- a/hw/eeprom93xx.c > +++ b/hw/eeprom93xx.c > @@ -114,7 +114,7 @@ static const VMStateInfo vmstate_hack_uint16_from_uint8 = { > }; > > #define VMSTATE_UINT16_HACK_TEST(_f, _s, _t) \ > - VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_hack_uint16_from_uint8, uint16_t) > + VMSTATE_SINGLE_TEST_HACK(_f, _s, _t, 0, vmstate_hack_uint16_from_uint8, uint16_t) > > static bool is_old_eeprom_version(void *opaque, int version_id) > { After the fact, we need to promote it as "full types". Basically it is needed when we sent a field with a different size that we use it on the struct. if we have struct FOOState { int32_t bar; .... } and it is sent as VMSTATE_INT8(bar, ....) In this case, I went through the whole device, checed that int8_t was enough and did the change. But if we have: struct FOOState { int8_t bar; .... } and it is sent as VMSTATE_INT32(bar, ....) Then it is not trivial :-( We change FOOState to int32 or we break migration format. Here is where the _HACK suffix appeared. I thought it was not going to be needed a lot, but there are several devices that just sent everything over the wire as uint32, independently of its type. Later, Juan.
On 03/23/2011 04:58 AM, Juan Quintela wrote: > Anthony Liguori<aliguori@us.ibm.com> wrote: >> I don't fully understand this hack business but we need field to be unique so.. >> >> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> >> --- >> hw/eeprom93xx.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/hw/eeprom93xx.c b/hw/eeprom93xx.c >> index cfa695d..f1d75ec 100644 >> --- a/hw/eeprom93xx.c >> +++ b/hw/eeprom93xx.c >> @@ -114,7 +114,7 @@ static const VMStateInfo vmstate_hack_uint16_from_uint8 = { >> }; >> >> #define VMSTATE_UINT16_HACK_TEST(_f, _s, _t) \ >> - VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_hack_uint16_from_uint8, uint16_t) >> + VMSTATE_SINGLE_TEST_HACK(_f, _s, _t, 0, vmstate_hack_uint16_from_uint8, uint16_t) >> >> static bool is_old_eeprom_version(void *opaque, int version_id) >> { > After the fact, we need to promote it as "full types". > > Basically it is needed when we sent a field with a different size that > we use it on the struct. > > if we have > > struct FOOState { > int32_t bar; > .... > } > > and it is sent as > > VMSTATE_INT8(bar, ....) > > In this case, I went through the whole device, checed that int8_t was > enough and did the change. > > But if we have: > > struct FOOState { > int8_t bar; > .... > } > > and it is sent as > > VMSTATE_INT32(bar, ....) > > Then it is not trivial :-( > > We change FOOState to int32 or we break migration format. Here is where > the _HACK suffix appeared. > > I thought it was not going to be needed a lot, but there are several > devices that just sent everything over the wire as uint32, independently > of its type. Could we get away with just doing: VMSTATE_UNUSED(3), VMSTATE_UINT8(bar, ...), That's fully compatible on the wire and seems to be a clearer expression of exactly what the problem is. Regards, Anthony Liguori > Later, Juan. >
Anthony Liguori <anthony@codemonkey.ws> wrote: > On 03/23/2011 04:58 AM, Juan Quintela wrote: >> Anthony Liguori<aliguori@us.ibm.com> wrote: >>> I don't fully understand this hack business but we need field to be unique so.. >>> >>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> >>> --- >>> hw/eeprom93xx.c | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/hw/eeprom93xx.c b/hw/eeprom93xx.c >>> index cfa695d..f1d75ec 100644 >>> --- a/hw/eeprom93xx.c >>> +++ b/hw/eeprom93xx.c >>> @@ -114,7 +114,7 @@ static const VMStateInfo vmstate_hack_uint16_from_uint8 = { >>> }; >>> >>> #define VMSTATE_UINT16_HACK_TEST(_f, _s, _t) \ >>> - VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_hack_uint16_from_uint8, uint16_t) >>> + VMSTATE_SINGLE_TEST_HACK(_f, _s, _t, 0, vmstate_hack_uint16_from_uint8, uint16_t) >>> >>> static bool is_old_eeprom_version(void *opaque, int version_id) >>> { > Could we get away with just doing: > > VMSTATE_UNUSED(3), > VMSTATE_UINT8(bar, ...), Remember that we are "supposed to be" big/little endian safe. > That's fully compatible on the wire and seems to be a clearer > expression of exactly what the problem is. if we are going to break big endian machines, I fully agree. Later, Juan.
On 03/23/2011 09:14 AM, Juan Quintela wrote: > Anthony Liguori<anthony@codemonkey.ws> wrote: >> On 03/23/2011 04:58 AM, Juan Quintela wrote: >>> Anthony Liguori<aliguori@us.ibm.com> wrote: >>>> I don't fully understand this hack business but we need field to be unique so.. >>>> >>>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> >>>> --- >>>> hw/eeprom93xx.c | 2 +- >>>> 1 files changed, 1 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/hw/eeprom93xx.c b/hw/eeprom93xx.c >>>> index cfa695d..f1d75ec 100644 >>>> --- a/hw/eeprom93xx.c >>>> +++ b/hw/eeprom93xx.c >>>> @@ -114,7 +114,7 @@ static const VMStateInfo vmstate_hack_uint16_from_uint8 = { >>>> }; >>>> >>>> #define VMSTATE_UINT16_HACK_TEST(_f, _s, _t) \ >>>> - VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_hack_uint16_from_uint8, uint16_t) >>>> + VMSTATE_SINGLE_TEST_HACK(_f, _s, _t, 0, vmstate_hack_uint16_from_uint8, uint16_t) >>>> >>>> static bool is_old_eeprom_version(void *opaque, int version_id) >>>> { >> Could we get away with just doing: >> >> VMSTATE_UNUSED(3), >> VMSTATE_UINT8(bar, ...), > Remember that we are "supposed to be" big/little endian safe. We always send in network byte order (big endian) so this is safe. >> That's fully compatible on the wire and seems to be a clearer >> expression of exactly what the problem is. > if we are going to break big endian machines, I fully agree. The migration protocol is always big endian, see: void qemu_put_be32(QEMUFile *f, unsigned int v) { qemu_put_byte(f, v >> 24); qemu_put_byte(f, v >> 16); qemu_put_byte(f, v >> 8); qemu_put_byte(f, v); } So this is completely safe. Regards, ANthony Liguori > Later, Juan.
diff --git a/hw/eeprom93xx.c b/hw/eeprom93xx.c index cfa695d..f1d75ec 100644 --- a/hw/eeprom93xx.c +++ b/hw/eeprom93xx.c @@ -114,7 +114,7 @@ static const VMStateInfo vmstate_hack_uint16_from_uint8 = { }; #define VMSTATE_UINT16_HACK_TEST(_f, _s, _t) \ - VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_hack_uint16_from_uint8, uint16_t) + VMSTATE_SINGLE_TEST_HACK(_f, _s, _t, 0, vmstate_hack_uint16_from_uint8, uint16_t) static bool is_old_eeprom_version(void *opaque, int version_id) {
I don't fully understand this hack business but we need field to be unique so.. Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- hw/eeprom93xx.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)