Hello, This series fixes a few issues pointed out by Avi and Juan. Avi pointed out we should do full scatter/gather processing of guest data even if current (well-behaved) guests don't send multiple iovs per element. Juan pointed out a few migration-related bugs. In handling the migration fixes, I noticed hot-plug/unplug isn't handled perfectly for the migration case: ports are enumerated and the port numbering has to be consistent with the guest's numbering. If there's a mismatch, control messages meant for one port could be interpreted for another. To solve this issue, I go back to maintaining a bitmap in the config space for active ports. Hot-plug and unplug can be added easily via the config space as a result. The kernel driver has to be changed as well so that the changes are in sync with the changes here. I've tested these patches on my test suite that tests for correctness and also hot-plug/unplug cases and fixes presented here. Amit Shah (9): virtio-serial-bus: save/load: Ensure target has enough ports virtio-serial-bus: save/load: Ensure nr_ports on src and dest are same. virtio-serial: save/load: Ensure we have hot-plugged ports instantiated virtio-serial: Handle scatter-gather buffers for control messages virtio-serial: Handle scatter/gather input from the guest virtio-serial: Remove redundant check for 0-sized write request virtio-serial: Update copyright year to 2010 virtio-serial-bus: Use a bitmap in virtio config space for active ports virtio-serial-bus: Let the guest know of host connection changes after migration hw/virtio-console.c | 4 +- hw/virtio-serial-bus.c | 205 ++++++++++++++++++++++++++++++++++++------------ hw/virtio-serial.h | 8 +- 3 files changed, 161 insertions(+), 56 deletions(-)
Amit Shah
2010-Mar-19 11:58 UTC
[PATCH 1/9] virtio-serial-bus: save/load: Ensure target has enough ports
The target could be started with max_nr_ports for a virtio-serial device lesser than what was available on the source machine. Fail the migration in such a case. Signed-off-by: Amit Shah <amit.shah at redhat.com> Reported-by: Juan Quintela <quintela at redhat.com> --- hw/virtio-serial-bus.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index 17c1ec1..36985a1 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -374,6 +374,8 @@ static void virtio_serial_save(QEMUFile *f, void *opaque) /* Items in struct VirtIOSerial */ + qemu_put_be32s(f, &s->bus->max_nr_ports); + /* Do this because we might have hot-unplugged some ports */ nr_active_ports = 0; QTAILQ_FOREACH(port, &s->ports, next) @@ -399,7 +401,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id) { VirtIOSerial *s = opaque; VirtIOSerialPort *port; - uint32_t nr_active_ports; + uint32_t max_nr_ports, nr_active_ports; unsigned int i; if (version_id > 2) { @@ -420,6 +422,12 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id) /* Items in struct VirtIOSerial */ + qemu_get_be32s(f, &max_nr_ports); + if (max_nr_ports > s->bus->max_nr_ports) { + /* Source could have more ports than us. Fail migration. */ + return -EINVAL; + } + qemu_get_be32s(f, &nr_active_ports); /* Items in struct VirtIOSerialPort */ -- 1.6.2.5
On Fri, Mar 19, 2010 at 05:28:37PM +0530, Amit Shah wrote:> Hello, > > This series fixes a few issues pointed out by Avi and Juan. Avi > pointed out we should do full scatter/gather processing of guest data > even if current (well-behaved) guests don't send multiple iovs per > element. > > Juan pointed out a few migration-related bugs. > > In handling the migration fixes, I noticed hot-plug/unplug isn't > handled perfectly for the migration case: ports are enumerated and the > port numbering has to be consistent with the guest's numbering. If > there's a mismatch, control messages meant for one port could be > interpreted for another.BTW, I think virtio serial migration code needs to be fixed to be backwards compatible with old qemu if multiport feature is off.> To solve this issue, I go back to maintaining a bitmap in the config > space for active ports. Hot-plug and unplug can be added easily via > the config space as a result.As I commented on the kernel driver, I'm not sure this is a good choice.> The kernel driver has to be changed as well so that the changes are in > sync with the changes here. > > I've tested these patches on my test suite that tests for correctness > and also hot-plug/unplug cases and fixes presented here. > > Amit Shah (9): > virtio-serial-bus: save/load: Ensure target has enough ports > virtio-serial-bus: save/load: Ensure nr_ports on src and dest are > same. > virtio-serial: save/load: Ensure we have hot-plugged ports > instantiated > virtio-serial: Handle scatter-gather buffers for control messages > virtio-serial: Handle scatter/gather input from the guest > virtio-serial: Remove redundant check for 0-sized write request > virtio-serial: Update copyright year to 2010 > virtio-serial-bus: Use a bitmap in virtio config space for active > ports > virtio-serial-bus: Let the guest know of host connection changes > after migration > > hw/virtio-console.c | 4 +- > hw/virtio-serial-bus.c | 205 ++++++++++++++++++++++++++++++++++++------------ > hw/virtio-serial.h | 8 +- > 3 files changed, 161 insertions(+), 56 deletions(-)