iov_to_buf() has an 'offset' parameter, iov_from_buf() hasn't.
This patch adds the missing parameter to iov_from_buf().
It also renames the 'offset' parameter to 'iov_off' to
emphasize it's the offset into the iovec and not the buffer.
Signed-off-by: NHannes Reinecke <hare@suse.de>
Acked-by: NAlexander Graf <agraf@suse.de>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

If I start qemu with:

  # qemu -hda disks/test.img -enable-kvm -m 1G -snapshot \
         -device virtio-serial \
         -chardev socket,host=localhost,port=1234,server,nowait,id=foo \
         -device virtserialport,chardev=foo,name=org.qemu.guest_agent

I get a segfault when booting a Fedora 14 guest. The backtrace says:

  Program terminated with signal 11, Segmentation fault.
    #0  0x0000000000420850 in handle_control_message (vser=0x3732bd0, buf=0x2c173e0, len=8) at /home/lcapitulino/src/qmp-unstable/hw/virtio-serial-bus.c:335
    335     info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info);

What's happening is VIRTIO_CONSOLE_DEVICE_READY is a message for the
whole device, not for an individual port. So port is NULL. This bug was
introduced by commit a15bb0d6.

This commit fixes that by making the port returned by find_port_by_id()
be used only by the VIRTIO_CONSOLE_PORT_READY and
VIRTIO_CONSOLE_PORT_OPEN messages.
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>

error_report() prepends location, and appends a newline.  The message
constructed from the arguments should not contain a newline.  Fix the
obvious offenders.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>

virtio-serial-bus needs to treat "virtconsole" devices specially.  It
uses VirtIOSerialPort member is_console to recognize them.  It gets
its value via property initialization.  Cute hack, except it lets
users mess with it: "-device virtconsole,is_console=0" isn't plugged
into port 0 as it should.

Move the flag to VirtIOSerialPortInfo.  Keep the property for backward
compatibility; its value has no effect.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>

virtio_serial_init() allocates the VirtIOSerialBus dynamically, but
virtio_serial_exit() doesn't free it.

Fix by getting rid of the allocation.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>

Instead of calling flush_queued_data when unthrottling, schedule
a bh. That way we can return immediately to the caller, and the
flush uses the same call path as a have_data for callbackee.

No migration change is required because bh are called from vm_stop.
Signed-off-by: NAlon Levy <alevy@redhat.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>

The virtio serial specification requres that the values in the config
space are encoded in native endian of the guest.

The qemu virtio-serial code did not do conversion to the guest endian
format what caused problems when host and guest use different format.

This patch corrects the qemu side, correctly doing host-native <->
guest-native conversions when accessing the config space. This won't
break any setups that aren't already broken, and fixes the case
of different host and guest endianness.
Signed-off-by: NAlexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: NDavid Gibson <david@gibson.dropbear.id.au>
Reviewed-by: NJuan Quintela <quintela@redhat.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>

Fix crash on invalid input in virtio-serial.
Discovered by code review, untested.
Signed-off-by: NMichael S. Tsirkin <mst@redhat.com>

There's no code change, just re-arrangement to simplify the function
after recent modifications.
Reported-by: NJuan Quintela <quintela@redhat.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>

Instead of using a single variable to pass to the virtio_serial_init
function, use a struct so that expanding the number of variables to be
passed on later is easier.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>

VirtIOSerialDevice is like VirtIOSerialPort with just the first two
fields, which makes it pretty pointless.  Using VirtIOSerialPort
directly works equally well and is less confusing.

[Amit: - rebase
       - rename 'dev' to 'port' in function params in virtio-serial.h ]
Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

This can happen if a port gets unplugged before guest has chance to
initialise vqs.
Reported-by: NJuan Quintela <quintela@redhat.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

The new fields that got added as part of not copying over the guest
buffer to the host need to be saved/restored across migration.  Do that
and bump up the version number.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>

This commit lets apps signal an incomplete write.  When that happens,
stop sending out any more data to the app and wait for it to unthrottle
the port.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>

When the guest writes something to a host, we copied over the entire
buffer first into the host and then processed it.  Do away with that, it
could result in a malicious guest causing a DoS on the host.
Reported-by: NPaul Brook <paul@codesourcery.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>

Instead of combining flush logic into the discard case and not discard
case, have one function doing discard case.  This will help later when
adding flow control logic to the do_flush_queued_data() function.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>

The current default of 16 buffers for the control vq is too small.  We
can get more entries in there, for example when asking the guest to add
max. allowed ports.

Note: a more robust solution would involve some kind of event queueing
in host to guarantee no event loss. Added a TODO to look into
this later.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NMichael S. Tsirkin <mst@redhat.com>

Free malloc'ed memory, unregister from savevm and clean up virtio-common
bits on device hot-unplug.

This was found performing a migration after device hot-unplug.

Reported-by: <lihuang@redhat.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

Currently virtio-serial supports a maximum of 31 ports. Specifying the
'max_ports' parameter to be > 31 on the cmd line causes badness.

Ensure we initialise virtio-serial only if max_ports is within the
supported range.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAurelien Jarno <aurelien@aurel32.net>

In addition to the previous fix for calling do_flush_queued_data() only
when the virtqueue is ready, ensure do_flush_queued_data() gets a vq
that's suitably initialised.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

If a virtio-serial port is removed before the guest comes up and
initialises the virtqueues, qemu exits with the message

Guest moved used index from 0 to 61440

This happens because we try to clear any pending buffers from the
virtqueue.

Ensure the virtqueue is initialised before calling any virtqueue
operations.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

When available, we'd like to be able to access the DeviceState
when registering a savevm.  For buses with a get_dev_path()
function, this will allow us to create more unique savevm
id strings.
Signed-off-by: NAlex Williamson <alex.williamson@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

For all i, ports_map[i] is used in and only in the i-th iteration.
Replace the dynamic array by a scalar variable.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Acked-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAurelien Jarno <aurelien@aurel32.net>

Fix for too small allocation to ports_map
Signed-off-by: NAlon Levy <alevy@redhat.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

In the flush_queued_data() function, we expect port to be valid. Assert
only for port and not port || discard.
Reported-by: NJuan Quintela <quintela@redhat.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

Individual ports can now signal to the virtio-serial core to stop
sending data if the ports cannot immediately handle new data.  When a
port later unthrottles, any data queued up in the virtqueue are sent to
the port.

Disable throttling once a port is closed (and we discard all the
unconsumed buffers in the vq).

The guest kernel can reclaim the buffers when it receives the port close
event or when a port is being removed. Ensure we free up the buffers
before we send out any events to the guest.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

Before the earlier patch, we relied on incorrect virtio api usage to
signal to the guest that a particular buffer wasn't consumed by the
host.

After fixing that, we now just discard the data the guest sends us while
a host port is disconnected or doesn't have a handler registered for
consuming data.

This commit really doesn't change anything from the current behaviour,
just makes the code slightly better by spinning off data handling to
ports in another function.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

We cannot indicate to the guest how much data was consumed by an app for
out_bufs.  So we just have to assume the apps will consume all the data
that are handed over to them.

Fix the virtio api abuse in control_out() and handle_output().
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

Current guests don't send more than one iov but it can change later.
Ensure we handle that case.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
CC: Avi Kivity <avi@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

Current control messages are small enough to not be split into multiple
buffers but we could run into such a situation in the future or a
malicious guest could cause such a situation.

So handle the entire iov request for control messages.

Also ensure the size of the control request is >= what we expect
otherwise we risk accessing memory that we don't own.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
CC: Avi Kivity <avi@redhat.com>
Reported-by: NAvi Kivity <avi@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

The virtio-net code uses iov_fill() which fills an iov from a linear
buffer. The virtio-serial-bus code does something similar in an
open-coded function.

Create a new iov.c file that has iov_from_buf().

Convert virtio-net and virtio-serial-bus over to use this functionality.
virtio-net used ints to hold sizes, the new function is going to use
size_t types.

Later commits will add the opposite functionality -- going from an iov
to a linear buffer.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

Data should be written only when ports are open.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

If adding of ports or devices in the guest fails we can send out a QMP
event so that management software can deal with it.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

The check for a 0-sized write request to a guest port is not necessary;
the while loop below won't be executed in this case and all will be
fine.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

The virtio-serial code doesn't mix declarations and definitions, so
separate them out on different lines.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

Allow the port 'id's to be set by a user on the command line. This is
needed by management apps that will want a stable port numbering scheme
for hot-plug/unplug and migration.

Since the port numbers are shared with the guest (to identify ports in
control messages), we just send a control message to the guest
indicating addition of new ports (hot-plug) or notifying the guest of
the available ports when the guest sends us a DEVICE_READY control
message.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

If the host connection to a port is closed on the destination machine
after migration, whereas the connection was open on the source, the
guest has to be informed of that.

Similar for a host connection open on the destination.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

If some ports that were hot-plugged on the source are not available on
the destination, fail migration instead of trying to deref a NULL
pointer.
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Reported-by: NJuan Quintela <quintela@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>