Skip to contentSkip to conversation

: Fix i2c_bus_save, which fixes KVM live migration

PATCH In review Chris Lalancette 5 participants 7 messages Jul 29, 2008 KVM

lore
Share & tools
lore

Chris Lalancette

author
#0 lore raw

PATCH 1/1 : Fix i2c_bus_save, which fixes KVM live migration

Attached is a simple patch to make i2c_bus_save() put a 32-bit quantity in the save file, which matches what i2c_bus_load() expects to pull out of the save file later. Without this fix in place, KVM live migration fails since the sender is only sending 1 byte while the receiver is waiting to receive 4 bytes.

Avi, I don't know when you plan to next rebase KVM to upstream QEMU, but it's probably a good idea to carry this patch so that live migration works at all.

Signed-off-by Chris Lalancette <[email protected]>
qemu/hw/i2c.c +1 -1
index 5d283fb..f711db7 100644
@@ -21,7 +21,7 @@ static void i2c_bus_save(QEMUFile *f, void *opaque)
 {
     i2c_bus *bus = (i2c_bus *)opaque;
 
-    qemu_put_byte(f, bus->current_dev ? bus->current_dev->address : -1);
+    qemu_put_be32(f, bus->current_dev ? bus->current_dev->address : -1);
 }
 
 static int i2c_bus_load(QEMUFile *f, void *opaque, int version_id)
#1 lore raw
Quoted 9 lines · Chris Lalancette

Attached is a simple patch to make i2c_bus_save() put a 32-bit quantity in the save file, which matches what i2c_bus_load() expects to pull out of the save file later. Without this fix in place, KVM live migration fails since the sender is only sending 1 byte while the receiver is waiting to receive 4 bytes.

Avi, I don't know when you plan to next rebase KVM to upstream QEMU, but it's probably a good idea to carry this patch so that live migration works at all.

I'll merge qemu upstream as soon as I see this go in.

Signature · 1 line
error compiling committee.c: too many arguments to function
#2 lore raw
In reply to Chris Lalancette
Quoted 15 lines · Chris Lalancette

Attached is a simple patch to make i2c_bus_save() put a 32-bit quantity in the save file, which matches what i2c_bus_load() expects to pull out of the save file later. Without this fix in place, KVM live migration fails since the sender is only sending 1 byte while the receiver is waiting to receive 4 bytes.

Avi, I don't know when you plan to next rebase KVM to upstream QEMU, but it's probably a good idea to carry this patch so that live migration works at all.

Signed-off-by Chris Lalancette <[email protected]>
qemu/hw/i2c.c +0 -0
index 5d283fb..f711db7 100644

qemu/hw/i2c.c static void i2c_bus_save(QEMUFile *f, void *opaque) · line 26

Quoted 7 lines
qemu/hw/i2c.c +1 -1
@@ -21,7 +21,7 @@ static void i2c_bus_save(QEMUFile *f, void *opaque)
 {
     i2c_bus *bus = (i2c_bus *)opaque;
 
-    qemu_put_byte(f, bus->current_dev ? bus->current_dev->address : -1);
+    qemu_put_be32(f, bus->current_dev ? bus->current_dev->address : -1);
 }

Looking at the code, I suspect it's really supposed to be a byte. Perhaps pbrook can clarify?

Regards,

Anthony Liguori

Quoted 2 lines
qemu/hw/i2c.c +0 -0
 static int i2c_bus_load(QEMUFile *f, void *opaque, int version_id)
#3 lore raw
In reply to Chris Lalancette

2008/7/29 Chris Lalancette <[email protected]>:

Quoted 4 lines

Attached is a simple patch to make i2c_bus_save() put a 32-bit quantity in the save file, which matches what i2c_bus_load() expects to pull out of the save file later. Without this fix in place, KVM live migration fails since the sender is only sending 1 byte while the receiver is waiting to receive 4 bytes.

Thanks, I committed this modified to use byte width afterall since the addresses are 7-bit quantities, as noted by Anthony. Means that existing snapshots are not invalidated.

Cheers

#4 lore raw
Quoted 9 lines · andrzej zaborowski

2008/7/29 Chris Lalancette <[email protected]>:

Quoted 5 lines

Attached is a simple patch to make i2c_bus_save() put a 32-bit quantity in the save file, which matches what i2c_bus_load() expects to pull out of the save file later. Without this fix in place, KVM live migration fails since the sender is only sending 1 byte while the receiver is waiting to receive 4 bytes.

Thanks, I committed this modified to use byte width afterall since the addresses are 7-bit quantities.

No they aren't. I2C also supports 10-bit addressing.

Paul

#5 lore raw

2008/7/29 Paul Brook <[email protected]>:

Quoted 8 lines · andrzej zaborowski
Quoted 7 lines

2008/7/29 Chris Lalancette <[email protected]>:

Quoted 5 lines

Attached is a simple patch to make i2c_bus_save() put a 32-bit quantity in the save file, which matches what i2c_bus_load() expects to pull out of the save file later. Without this fix in place, KVM live migration fails since the sender is only sending 1 byte while the receiver is waiting to receive 4 bytes.

Quoted 4 lines
Quoted 2 lines

Thanks, I committed this modified to use byte width afterall since the addresses are 7-bit quantities.

No they aren't. I2C also supports 10-bit addressing.

Addresses in qemu are still <= 8-bit, for example i2c_slave_load() assumes this and current i2c api users do.

Regards

#6 lore raw
Quoted 8 lines · andrzej zaborowski

2008/7/29 Paul Brook <[email protected]>:

Quoted 2 lines

No they aren't. I2C also supports 10-bit addressing.

Addresses in qemu are still <= 8-bit, for example i2c_slave_load() assumes this and current i2c api users do.

But for the save/restore format, if the spec can support 10-bit addressing, then we should use be16s to avoid having to change save/restore format if we ever expand the supported address size.

Regards,

Anthony Liguori

Quoted 6 lines

Regards
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html