[Qemu-devel] [PULL 00/33] virtio, pc: fixes and features

[Qemu-devel] [PULL 00/33] virtio, pc: fixes and features

[Michael S. Tsirkin]

The following changes since commit 48f592118ab42f83a1a7561c4bfd2b72a100f241:

  bsd-user: fix FreeBSD build after d148d90e (2016-10-07 15:17:53 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to dea651a95af6dad0997b840241a0bf6059d9a776:

  intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE (2016-10-10 02:38:14 +0300)

----------------------------------------------------------------
virtio, pc: fixes and features

more guest error handling for virtio devices
virtio migration rework
pc fixes

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Daniel P. Berrange (1):
      net: don't poke at chardev internal QemuOpts

Feng Wu (1):
      intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE

Greg Kurz (9):
      virtio-9p: add parentheses to sizeof operator
      virtio-blk: make some functions static
      virtio-9p: handle handle_9p_output() error
      virtio-blk: handle virtio_blk_handle_request() errors
      virtio-net: handle virtio_net_handle_ctrl() error
      virtio-net: handle virtio_net_receive() errors
      virtio-net: handle virtio_net_flush_tx() errors
      virtio-scsi: convert virtio_scsi_bad_req() to use virtio_error()
      virtio-scsi: handle virtio_scsi_set_config() error

Halil Pasic (12):
      virtio: prepare change VMSTATE_VIRTIO_DEVICE macro
      virtio-blk: convert VMSTATE_VIRTIO_DEVICE
      virtio-net: convert VMSTATE_VIRTIO_DEVICE
      virtio-9p: convert VMSTATE_VIRTIO_DEVICE
      virtio-serial: convert VMSTATE_VIRTIO_DEVICE
      virtio-gpu: convert VMSTATE_VIRTIO_DEVICE
      virtio-input: convert VMSTATE_VIRTIO_DEVICE
      virtio-scsi: convert VMSTATE_VIRTIO_DEVICE
      virtio-balloon: convert VMSTATE_VIRTIO_DEVICE
      virtio-rng: convert VMSTATE_VIRTIO_DEVICE
      vhost-vsock: convert VMSTATE_VIRTIO_DEVICE
      virtio: cleanup VMSTATE_VIRTIO_DEVICE

Igor Mammedov (4):
      numa: reduce code duplication by adding helper numa_get_node_for_cpu()
      acpi: provide _PXM method for CPU devices if QEMU is started numa enabled
      tests: acpi: extend cphp testcase with numa check
      tests: acpi tables expected blobs update

Liang Li (1):
      virtio-balloon: Remove needless precompiled directive

Sascha Silbe (2):
      virtio-serial: add plumbing for virtio console emergency write support
      virtio-serial: enable virtio console emergency write feature

Stefan Hajnoczi (3):
      virtio: add virtio_detach_element()
      virtio-blk: add missing virtio_detach_element() call
      virtio-serial: add missing virtio_detach_element() call

 include/hw/compat.h                |   4 ++
 include/hw/virtio/virtio-blk.h     |   8 ---
 include/hw/virtio/virtio-serial.h  |   2 +
 include/hw/virtio/virtio.h         |  29 ++++-------
 include/sysemu/char.h              |  21 +++++++-
 include/sysemu/numa.h              |   3 ++
 hmp.c                              |   1 +
 hw/9pfs/virtio-9p-device.c         |  45 ++++++++++++-----
 hw/acpi/cpu.c                      |  12 +++++
 hw/arm/virt-acpi-build.c           |   6 +--
 hw/arm/virt.c                      |   7 ++-
 hw/block/virtio-blk.c              |  72 +++++++++++++++-----------
 hw/char/virtio-serial-bus.c        |  79 +++++++++++++++++++++++++----
 hw/display/virtio-gpu.c            |  39 ++++++++++-----
 hw/i386/acpi-build.c               |   7 +--
 hw/i386/intel_iommu.c              |  12 +++++
 hw/i386/pc.c                       |   8 ++-
 hw/input/virtio-input.c            |  21 ++++----
 hw/net/virtio-net.c                | 100 +++++++++++++++++++++----------------
 hw/ppc/spapr_cpu_core.c            |   6 +--
 hw/scsi/virtio-scsi.c              |  77 ++++++++++++++++------------
 hw/virtio/vhost-vsock.c            |  42 ++++++++--------
 hw/virtio/virtio-balloon.c         |  17 ++++---
 hw/virtio/virtio-rng.c             |  19 ++++---
 hw/virtio/virtio.c                 |  44 ++++++++++++++--
 net/colo-compare.c                 |  30 +----------
 net/vhost-user.c                   |  41 +++------------
 numa.c                             |  12 +++++
 qemu-char.c                        |  22 ++++++--
 tests/bios-tables-test.c           |   6 ++-
 tests/acpi-test-data/pc/DSDT.cphp  | Bin 6435 -> 6471 bytes
 tests/acpi-test-data/pc/SRAT.cphp  | Bin 0 -> 304 bytes
 tests/acpi-test-data/q35/DSDT.cphp | Bin 9197 -> 9233 bytes
 tests/acpi-test-data/q35/SRAT.cphp | Bin 0 -> 304 bytes
 34 files changed, 484 insertions(+), 308 deletions(-)
 create mode 100644 tests/acpi-test-data/pc/SRAT.cphp
 create mode 100644 tests/acpi-test-data/q35/SRAT.cphp

[Qemu-devel] [PULL 01/33] virtio-balloon: Remove needless precompiled directive

[Michael S. Tsirkin]

From: Liang Li <liang.z.li@intel.com>

Since there in wrapper around madvise(), the virtio-balloon
code is able to work without the precompiled directive, the
directive can be removed.

Signed-off-by: Liang Li <liang.z.li@intel.com>
Suggested-by: Thomas Huth <thuth@redhat.com>
Reviewd-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-balloon.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 49a2f4a..eb572e6 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -34,13 +34,11 @@
 
 static void balloon_page(void *addr, int deflate)
 {
-#if defined(__linux__)
     if (!qemu_balloon_is_inhibited() && (!kvm_enabled() ||
                                          kvm_has_sync_mmu())) {
         qemu_madvise(addr, BALLOON_PAGE_SIZE,
                 deflate ? QEMU_MADV_WILLNEED : QEMU_MADV_DONTNEED);
     }
-#endif
 }
 
 static const char *balloon_stat_names[] = {
-- 
MST

[Qemu-devel] [PULL 02/33] virtio-serial: add plumbing for virtio console emergency write support

[Michael S. Tsirkin]

From: Sascha Silbe <silbe@linux.vnet.ibm.com>

Add the infrastructure required for the virtio 1.0 "emergency write"
(VIRTIO_CONSOLE_F_EMERG_WRITE) feature. Because we don't touch the
size of the configuration area, guests will not be able to actually
make use of this without further patches.

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/char/virtio-serial-bus.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index db57a38..57419b2 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -75,6 +75,19 @@ static VirtIOSerialPort *find_port_by_name(char *name)
     return NULL;
 }
 
+static VirtIOSerialPort *find_first_connected_console(VirtIOSerial *vser)
+{
+    VirtIOSerialPort *port;
+
+    QTAILQ_FOREACH(port, &vser->ports, next) {
+        VirtIOSerialPortClass const *vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+        if (vsc->is_console && port->host_connected) {
+            return port;
+        }
+    }
+    return NULL;
+}
+
 static bool use_multiport(VirtIOSerial *vser)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(vser);
@@ -547,6 +560,29 @@ static void get_config(VirtIODevice *vdev, uint8_t *config_data)
                                           vser->serial.max_virtserial_ports);
 }
 
+/* Guest sent new config info */
+static void set_config(VirtIODevice *vdev, const uint8_t *config_data)
+{
+    VirtIOSerial *vser = VIRTIO_SERIAL(vdev);
+    struct virtio_console_config *config =
+        (struct virtio_console_config *)config_data;
+    uint8_t emerg_wr_lo = le32_to_cpu(config->emerg_wr);
+    VirtIOSerialPort *port = find_first_connected_console(vser);
+    VirtIOSerialPortClass *vsc;
+
+    if (!config->emerg_wr) {
+        return;
+    }
+    /* Make sure we don't misdetect an emergency write when the guest
+     * does a short config write after an emergency write. */
+    config->emerg_wr = 0;
+    if (!port) {
+        return;
+    }
+    vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+    (void)vsc->have_data(port, &emerg_wr_lo, 1);
+}
+
 static void guest_reset(VirtIOSerial *vser)
 {
     VirtIOSerialPort *port;
@@ -1098,6 +1134,7 @@ static void virtio_serial_class_init(ObjectClass *klass, void *data)
     vdc->unrealize = virtio_serial_device_unrealize;
     vdc->get_features = get_features;
     vdc->get_config = get_config;
+    vdc->set_config = set_config;
     vdc->set_status = set_status;
     vdc->reset = vser_reset;
     vdc->save = virtio_serial_save_device;
-- 
MST

[Qemu-devel] [PULL 03/33] virtio-serial: enable virtio console emergency write feature

[Michael S. Tsirkin]

From: Sascha Silbe <silbe@linux.vnet.ibm.com>

Add support for enabling the virtio 1.0 "emergency write"
(VIRTIO_CONSOLE_F_EMERG_WRITE) feature. The previous patch introduced
the plumbing required for this; now we expose the virtio feature to
the guest. The feature is disabled for compatibility machines to avoid
exposing a new feature to existing guests.

As required by the virtio 1.0 spec, the emergency write functionality
is available to the guest even if the guest doesn't negotatiate the
feature, as well as before feature negotation.

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/compat.h               |  4 ++++
 include/hw/virtio/virtio-serial.h |  2 ++
 hw/char/virtio-serial-bus.c       | 12 +++++++++---
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/include/hw/compat.h b/include/hw/compat.h
index 46412b2..ef3fae3 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -7,6 +7,10 @@
         .property = "page-per-vq",\
         .value    = "on",\
     },{\
+        .driver   = "virtio-serial-device",\
+        .property = "emergency-write",\
+        .value    = "off",\
+    },{\
         .driver   = "ioapic",\
         .property = "version",\
         .value    = "0x11",\
diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
index 730c88d..b19c447 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -184,6 +184,8 @@ struct VirtIOSerial {
     struct VirtIOSerialPostLoad *post_load;
 
     virtio_serial_conf serial;
+
+    uint64_t host_features;
 };
 
 /* Interface to the virtio-serial bus */
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 57419b2..db2a9f1 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -541,6 +541,7 @@ static uint64_t get_features(VirtIODevice *vdev, uint64_t features,
 
     vser = VIRTIO_SERIAL(vdev);
 
+    features |= vser->host_features;
     if (vser->bus.max_nr_ports > 1) {
         virtio_add_feature(&features, VIRTIO_CONSOLE_F_MULTIPORT);
     }
@@ -1003,6 +1004,7 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOSerial *vser = VIRTIO_SERIAL(dev);
     uint32_t i, max_supported_ports;
+    size_t config_size = sizeof(struct virtio_console_config);
 
     if (!vser->serial.max_virtserial_ports) {
         error_setg(errp, "Maximum number of serial ports not specified");
@@ -1017,10 +1019,12 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    /* We don't support emergency write, skip it for now. */
-    /* TODO: cleaner fix, depending on host features. */
+    if (!virtio_has_feature(vser->host_features,
+                            VIRTIO_CONSOLE_F_EMERG_WRITE)) {
+        config_size = offsetof(struct virtio_console_config, emerg_wr);
+    }
     virtio_init(vdev, "virtio-serial", VIRTIO_ID_CONSOLE,
-                offsetof(struct virtio_console_config, emerg_wr));
+                config_size);
 
     /* Spawn a new virtio-serial bus on which the ports will ride as devices */
     qbus_create_inplace(&vser->bus, sizeof(vser->bus), TYPE_VIRTIO_SERIAL_BUS,
@@ -1116,6 +1120,8 @@ VMSTATE_VIRTIO_DEVICE(console, 3, virtio_serial_load, virtio_vmstate_save);
 static Property virtio_serial_properties[] = {
     DEFINE_PROP_UINT32("max_ports", VirtIOSerial, serial.max_virtserial_ports,
                                                   31),
+    DEFINE_PROP_BIT64("emergency-write", VirtIOSerial, host_features,
+                      VIRTIO_CONSOLE_F_EMERG_WRITE, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
MST

[Qemu-devel] [PULL 04/33] numa: reduce code duplication by adding helper numa_get_node_for_cpu()

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

Replace repeated pattern

    for (i = 0; i < nb_numa_nodes; i++) {
        if (test_bit(idx, numa_info[i].node_cpu)) {
           ...
           break;

with a helper function to lookup numa node index for cpu.

Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/sysemu/numa.h    |  3 +++
 hw/arm/virt-acpi-build.c |  6 ++----
 hw/arm/virt.c            |  7 +++----
 hw/i386/acpi-build.c     |  7 ++-----
 hw/i386/pc.c             |  8 +++-----
 hw/ppc/spapr_cpu_core.c  |  6 ++----
 numa.c                   | 12 ++++++++++++
 7 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index bb184c9..4da808a 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -32,4 +32,7 @@ void numa_set_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node);
 void numa_unset_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node);
 uint32_t numa_get_node(ram_addr_t addr, Error **errp);
 
+/* on success returns node index in numa_info,
+ * on failure returns nb_numa_nodes */
+int numa_get_node_for_cpu(int idx);
 #endif
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 7b39b1d..c77525d 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -427,11 +427,9 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
     uint32_t *cpu_node = g_malloc0(guest_info->smp_cpus * sizeof(uint32_t));
 
     for (i = 0; i < guest_info->smp_cpus; i++) {
-        for (j = 0; j < nb_numa_nodes; j++) {
-            if (test_bit(i, numa_info[j].node_cpu)) {
+        j = numa_get_node_for_cpu(i);
+        if (j < nb_numa_nodes) {
                 cpu_node[i] = j;
-                break;
-            }
         }
     }
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0f6305d..795740d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -413,10 +413,9 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
                                   armcpu->mp_affinity);
         }
 
-        for (i = 0; i < nb_numa_nodes; i++) {
-            if (test_bit(cpu, numa_info[i].node_cpu)) {
-                qemu_fdt_setprop_cell(vbi->fdt, nodename, "numa-node-id", i);
-            }
+        i = numa_get_node_for_cpu(cpu);
+        if (i < nb_numa_nodes) {
+            qemu_fdt_setprop_cell(vbi->fdt, nodename, "numa-node-id", i);
         }
 
         g_free(nodename);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index c20bc71..e999654 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2410,18 +2410,15 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
     srat->reserved1 = cpu_to_le32(1);
 
     for (i = 0; i < apic_ids->len; i++) {
-        int j;
+        int j = numa_get_node_for_cpu(i);
         int apic_id = apic_ids->cpus[i].arch_id;
 
         core = acpi_data_push(table_data, sizeof *core);
         core->type = ACPI_SRAT_PROCESSOR_APIC;
         core->length = sizeof(*core);
         core->local_apic_id = apic_id;
-        for (j = 0; j < nb_numa_nodes; j++) {
-            if (test_bit(i, numa_info[j].node_cpu)) {
+        if (j < nb_numa_nodes) {
                 core->proximity_lo = j;
-                break;
-            }
         }
         memset(core->proximity_hi, 0, 3);
         core->local_sapic_eid = 0;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2d6d792..93ff49c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -779,11 +779,9 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
     for (i = 0; i < max_cpus; i++) {
         unsigned int apic_id = x86_cpu_apic_id_from_index(i);
         assert(apic_id < pcms->apic_id_limit);
-        for (j = 0; j < nb_numa_nodes; j++) {
-            if (test_bit(i, numa_info[j].node_cpu)) {
-                numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
-                break;
-            }
+        j = numa_get_node_for_cpu(i);
+        if (j < nb_numa_nodes) {
+            numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
         }
     }
     for (i = 0; i < nb_numa_nodes; i++) {
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 35d1873..bc922bc 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -69,11 +69,9 @@ void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp)
     }
 
     /* Set NUMA node for the added CPUs  */
-    for (i = 0; i < nb_numa_nodes; i++) {
-        if (test_bit(cs->cpu_index, numa_info[i].node_cpu)) {
+    i = numa_get_node_for_cpu(cs->cpu_index);
+    if (i < nb_numa_nodes) {
             cs->numa_node = i;
-            break;
-        }
     }
 
     xics_cpu_setup(spapr->xics, cpu);
diff --git a/numa.c b/numa.c
index 6289f46..9c09e45 100644
--- a/numa.c
+++ b/numa.c
@@ -550,3 +550,15 @@ MemdevList *qmp_query_memdev(Error **errp)
     object_child_foreach(obj, query_memdev, &list);
     return list;
 }
+
+int numa_get_node_for_cpu(int idx)
+{
+    int i;
+
+    for (i = 0; i < nb_numa_nodes; i++) {
+        if (test_bit(idx, numa_info[i].node_cpu)) {
+            break;
+        }
+    }
+    return i;
+}
-- 
MST

[Qemu-devel] [PULL 05/33] acpi: provide _PXM method for CPU devices if QEMU is started numa enabled

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

Workaround for long standing issue where Linux kernel
assigns hotplugged CPU to 1st numa node as it discards
proximity for possible CPUs from SRAT after it's parsed.

_PXM method allows linux query proximity directly from
hotplugged CPU object, which allows Linux to assing CPU
to the correct numa node.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/cpu.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index c13b65c..902f5c9 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -4,6 +4,7 @@
 #include "qapi/error.h"
 #include "qapi-event.h"
 #include "trace.h"
+#include "sysemu/numa.h"
 
 #define ACPI_CPU_HOTPLUG_REG_LEN 12
 #define ACPI_CPU_SELECTOR_OFFSET_WR 0
@@ -503,6 +504,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
 
         /* build Processor object for each processor */
         for (i = 0; i < arch_ids->len; i++) {
+            int j;
             Aml *dev;
             Aml *uid = aml_int(i);
             GArray *madt_buf = g_array_new(0, 1, 1);
@@ -546,6 +548,16 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
                           aml_arg(1), aml_arg(2))
             );
             aml_append(dev, method);
+
+            /* Linux guests discard SRAT info for non-present CPUs
+             * as a result _PXM is required for all CPUs which might
+             * be hot-plugged. For simplicity, add it for all CPUs.
+             */
+            j = numa_get_node_for_cpu(i);
+            if (j < nb_numa_nodes) {
+                aml_append(dev, aml_name_decl("_PXM", aml_int(j)));
+            }
+
             aml_append(cpus_dev, dev);
         }
     }
-- 
MST

[Qemu-devel] [PULL 06/33] tests: acpi: extend cphp testcase with numa check

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

so it would be possible to verify _PXM generation in
DSDT and SRAT tables.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/bios-tables-test.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 7e27ea9..6ea2b6d 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -811,7 +811,8 @@ static void test_acpi_piix4_tcg_cphp(void)
     memset(&data, 0, sizeof(data));
     data.machine = MACHINE_PC;
     data.variant = ".cphp";
-    test_acpi_one("-smp 2,cores=3,sockets=2,maxcpus=6",
+    test_acpi_one("-smp 2,cores=3,sockets=2,maxcpus=6"
+                  " -numa node -numa node",
                   &data);
     free_test_data(&data);
 }
@@ -823,7 +824,8 @@ static void test_acpi_q35_tcg_cphp(void)
     memset(&data, 0, sizeof(data));
     data.machine = MACHINE_Q35;
     data.variant = ".cphp";
-    test_acpi_one(" -smp 2,cores=3,sockets=2,maxcpus=6",
+    test_acpi_one(" -smp 2,cores=3,sockets=2,maxcpus=6"
+                  " -numa node -numa node",
                   &data);
     free_test_data(&data);
 }
-- 
MST

[Qemu-devel] [PULL 07/33] tests: acpi tables expected blobs update

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/acpi-test-data/pc/DSDT.cphp  | Bin 6435 -> 6471 bytes
 tests/acpi-test-data/pc/SRAT.cphp  | Bin 0 -> 304 bytes
 tests/acpi-test-data/q35/DSDT.cphp | Bin 9197 -> 9233 bytes
 tests/acpi-test-data/q35/SRAT.cphp | Bin 0 -> 304 bytes
 4 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/acpi-test-data/pc/SRAT.cphp
 create mode 100644 tests/acpi-test-data/q35/SRAT.cphp

diff --git a/tests/acpi-test-data/pc/DSDT.cphp b/tests/acpi-test-data/pc/DSDT.cphp
index e8b146208eb3877e1cde2cc361c5afd270d194c6..9f405cfd83d39a8e06bc08428e160a0192fc9704 100644
GIT binary patch
delta 122
zcmZ2%blix`CD<jzU6O%;F?AzXB|DQx$mCY`)lA-=n~gXQGBJ8j{=((L$q^qA;mZ)+
u>^<3<8^QCN+{En;m-Cx^2F7EIZuXlj#sifD^AdR6*}$eSZeGq)!vg?MeIj%K

delta 85
zcmX?ZwAhHtCD<iIS(1T)@ySN6N_HlfpvkT5tC`&0Hyd#rWMXuk{DsSfIl9?(vLiQ$
a_L$tq?GC2zL1{f62)~ZUee-Fa8Xf>!Wg5-^

diff --git a/tests/acpi-test-data/pc/SRAT.cphp b/tests/acpi-test-data/pc/SRAT.cphp
new file mode 100644
index 0000000000000000000000000000000000000000..ff2137642f488ec70b85207ed6c20e7351d61e98
GIT binary patch
literal 304
zcmWFzattwGWME)4bMklg2v%^42yhMtiUEZfKx_~V!f+sf!DmF1XF}yOvY_!<(fDl0
pd`1npO;83GTmZW|po75R12aq^syaB21u74tQT&BzFU&Ml8UVWm2>}2A

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/q35/DSDT.cphp b/tests/acpi-test-data/q35/DSDT.cphp
index 6cc28c6daec2b331030ab0600a4d79034c1dfc40..a0ce6b3264c69999c6e82a8ae7bab49338e4819b 100644
GIT binary patch
delta 122
zcmaFsKGB2ACD<iIP=$ekaoa|&MoA`*kjcH0tC_q#H(N;^WMcH5{71%xlOsML!j~bs
u*?Y3HEQ04Zxl7g^F6TG-3XI1X-Rw76P7W#u=4Hsavw=-v+`L}Sjuim7cO#ep

delta 85
zcmbQ}@z$NoCD<k8tug}xquEBTMoA`@pvk?GtC`&0H(N;^WMXuk{71%xIl9?(vZpMF
a_Lw|P)*VbggVJVl5PqAS`{v7XcB}xQ78~*a

diff --git a/tests/acpi-test-data/q35/SRAT.cphp b/tests/acpi-test-data/q35/SRAT.cphp
new file mode 100644
index 0000000000000000000000000000000000000000..ff2137642f488ec70b85207ed6c20e7351d61e98
GIT binary patch
literal 304
zcmWFzattwGWME)4bMklg2v%^42yhMtiUEZfKx_~V!f+sf!DmF1XF}yOvY_!<(fDl0
pd`1npO;83GTmZW|po75R12aq^syaB21u74tQT&BzFU&Ml8UVWm2>}2A

literal 0
HcmV?d00001

-- 
MST

[Qemu-devel] [PULL 08/33] virtio: add virtio_detach_element()

[Michael S. Tsirkin]

From: Stefan Hajnoczi <stefanha@redhat.com>

During device reset or similar situations a VirtQueueElement needs to be
freed without pushing it onto the used ring or rewinding the virtqueue.
Extract a new function to do this.

Later patches add virtio_detach_element() calls to existing device so
that scatter-gather lists are unmapped and vq->inuse goes back to zero
during device reset.  Currently some devices don't bother and simply
call g_free(elem) which is not a clean way to throw away a
VirtQueueElement.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Ladi Prosek <lprosek@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/virtio.h |  2 ++
 hw/virtio/virtio.c         | 27 +++++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 888c8de..e25ec4f 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -155,6 +155,8 @@ void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_num);
 void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
                     unsigned int len);
 void virtqueue_flush(VirtQueue *vq, unsigned int count);
+void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
+                              unsigned int len);
 void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
                        unsigned int len);
 bool virtqueue_rewind(VirtQueue *vq, unsigned int num);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 18ce333..46f79c9 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -264,12 +264,35 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
                                   0, elem->out_sg[i].iov_len);
 }
 
+/* virtqueue_detach_element:
+ * @vq: The #VirtQueue
+ * @elem: The #VirtQueueElement
+ * @len: number of bytes written
+ *
+ * Detach the element from the virtqueue.  This function is suitable for device
+ * reset or other situations where a #VirtQueueElement is simply freed and will
+ * not be pushed or discarded.
+ */
+void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
+                              unsigned int len)
+{
+    vq->inuse--;
+    virtqueue_unmap_sg(vq, elem, len);
+}
+
+/* virtqueue_discard:
+ * @vq: The #VirtQueue
+ * @elem: The #VirtQueueElement
+ * @len: number of bytes written
+ *
+ * Pretend the most recent element wasn't popped from the virtqueue.  The next
+ * call to virtqueue_pop() will refetch the element.
+ */
 void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
                        unsigned int len)
 {
     vq->last_avail_idx--;
-    vq->inuse--;
-    virtqueue_unmap_sg(vq, elem, len);
+    virtqueue_detach_element(vq, elem, len);
 }
 
 /* virtqueue_rewind:
-- 
MST

[Qemu-devel] [PULL 09/33] virtio-blk: add missing virtio_detach_element() call

[Michael S. Tsirkin]

From: Stefan Hajnoczi <stefanha@redhat.com>

Make sure to unmap the scatter-gather list and decrement vq->inuse
before freeing requests in virtio_blk_reset().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Ladi Prosek <lprosek@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/block/virtio-blk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 3a6112f..c7ca4d6 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -665,6 +665,7 @@ static void virtio_blk_reset(VirtIODevice *vdev)
     while (s->rq) {
         req = s->rq;
         s->rq = req->next;
+        virtqueue_detach_element(req->vq, &req->elem, 0);
         virtio_blk_free_request(req);
     }
 
-- 
MST

[Qemu-devel] [PULL 10/33] virtio-serial: add missing virtio_detach_element() call

[Michael S. Tsirkin]

From: Stefan Hajnoczi <stefanha@redhat.com>

Ports enter a "throttled" state when writing to the chardev would block.
The current output VirtQueueElement is kept around until the chardev
becomes writable again.

There are several places in the virtio-serial lifecycle where the
VirtQueueElement should be thrown away.  For example, if the virtio
device is reset then virtqueue elements are no longer valid.

This patch adds the discard_throttle_data() function to unmap the
scatter-gather list and decrement vq->inuse.  This ensures that the
VirtQueueElement is freed properly.

Cc: amit.shah@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Ladi Prosek <lprosek@redhat.com>
Reviewed-by: Ladi Prosek <lprosek@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/char/virtio-serial-bus.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index db2a9f1..3955f0f 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -145,6 +145,15 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice *vdev)
     virtio_notify(vdev, vq);
 }
 
+static void discard_throttle_data(VirtIOSerialPort *port)
+{
+    if (port->elem) {
+        virtqueue_detach_element(port->ovq, port->elem, 0);
+        g_free(port->elem);
+        port->elem = NULL;
+    }
+}
+
 static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
                                  VirtIODevice *vdev)
 {
@@ -267,6 +276,7 @@ int virtio_serial_close(VirtIOSerialPort *port)
      * consume, reset the throttling flag and discard the data.
      */
     port->throttled = false;
+    discard_throttle_data(port);
     discard_vq_data(port->ovq, VIRTIO_DEVICE(port->vser));
 
     send_control_event(port->vser, port->id, VIRTIO_CONSOLE_PORT_OPEN, 0);
@@ -591,6 +601,9 @@ static void guest_reset(VirtIOSerial *vser)
 
     QTAILQ_FOREACH(port, &vser->ports, next) {
         vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+
+        discard_throttle_data(port);
+
         if (port->guest_connected) {
             port->guest_connected = false;
             if (vsc->set_guest_connected) {
@@ -901,6 +914,7 @@ static void remove_port(VirtIOSerial *vser, uint32_t port_id)
     assert(port);
 
     /* Flush out any unconsumed buffers first */
+    discard_throttle_data(port);
     discard_vq_data(port->ovq, VIRTIO_DEVICE(port->vser));
 
     send_control_event(vser, port->id, VIRTIO_CONSOLE_PORT_REMOVE, 1);
-- 
MST

[Qemu-devel] [PULL 11/33] virtio-9p: add parentheses to sizeof operator

[Michael S. Tsirkin]

From: Greg Kurz <groug@kaod.org>

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/9pfs/virtio-9p-device.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 009b43f..e7ea0e4 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -57,12 +57,12 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
         }
 
         BUG_ON(elem->out_num == 0 || elem->in_num == 0);
-        QEMU_BUILD_BUG_ON(sizeof out != 7);
+        QEMU_BUILD_BUG_ON(sizeof(out) != 7);
 
         v->elems[pdu->idx] = elem;
         len = iov_to_buf(elem->out_sg, elem->out_num, 0,
-                         &out, sizeof out);
-        BUG_ON(len != sizeof out);
+                         &out, sizeof(out));
+        BUG_ON(len != sizeof(out));
 
         pdu->size = le32_to_cpu(out.size_le);
 
-- 
MST

[Qemu-devel] [PULL 12/33] virtio-blk: make some functions static

[Michael S. Tsirkin]

From: Greg Kurz <groug@kaod.org>

Some functions that were called from the dataplane code are now only used
locally:

virtio_blk_init_request()
virtio_blk_handle_request()
virtio_blk_submit_multireq()

since commit "03de2f527499 virtio-blk: do not use vring in dataplane", and

virtio_blk_free_request()

since commit "6aa46d8ff1ee virtio: move VirtQueueElement at the beginning
of the structs".

This patch converts them to static.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/virtio-blk.h |  8 --------
 hw/block/virtio-blk.c          | 10 +++++-----
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 180bd8d..9734b4c 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -80,14 +80,6 @@ typedef struct MultiReqBuffer {
     bool is_write;
 } MultiReqBuffer;
 
-void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
-                             VirtIOBlockReq *req);
-void virtio_blk_free_request(VirtIOBlockReq *req);
-
-void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb);
-
-void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb);
-
 void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq);
 
 #endif
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index c7ca4d6..bbacd56 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -29,8 +29,8 @@
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
 
-void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
-                             VirtIOBlockReq *req)
+static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
+                                    VirtIOBlockReq *req)
 {
     req->dev = s;
     req->vq = vq;
@@ -40,7 +40,7 @@ void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
     req->mr_next = NULL;
 }
 
-void virtio_blk_free_request(VirtIOBlockReq *req)
+static void virtio_blk_free_request(VirtIOBlockReq *req)
 {
     if (req) {
         g_free(req);
@@ -381,7 +381,7 @@ static int multireq_compare(const void *a, const void *b)
     }
 }
 
-void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb)
+static void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb)
 {
     int i = 0, start = 0, num_reqs = 0, niov = 0, nb_sectors = 0;
     uint32_t max_transfer;
@@ -468,7 +468,7 @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
     return true;
 }
 
-void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
+static void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
 {
     uint32_t type;
     struct iovec *in_iov = req->elem.in_sg;
-- 
MST

[Qemu-devel] [PULL 13/33] virtio-9p: handle handle_9p_output() error

[Michael S. Tsirkin]

From: Greg Kurz <groug@kaod.org>

A broken guest may send a request without providing buffers for the reply
or for the request itself, and virtqueue_pop() will return an element with
either in_num == 0 or out_num == 0.

All 9P requests are expected to start with the following 7-byte header:

            uint32_t size_le;
            uint8_t id;
            uint16_t tag_le;

If iov_to_buf() fails to return these 7 bytes, then something is wrong in
the guest.

In both cases, it is wrong to crash QEMU, since the root cause lies in the
guest.

This patch hence does the following:
- keep the check of in_num since pdu_complete() assumes it has enough
  space to store the reply and we will send something broken to the guest
- let iov_to_buf() handle out_num == 0, since it will return 0 just like
  if the guest had provided an zero-sized buffer.
- call virtio_error() to inform the guest that the device is now broken,
  instead of aborting
- detach the request from the virtqueue and free it

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/9pfs/virtio-9p-device.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index e7ea0e4..a338f64 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -41,6 +41,7 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
     V9fsState *s = &v->state;
     V9fsPDU *pdu;
     ssize_t len;
+    VirtQueueElement *elem;
 
     while ((pdu = pdu_alloc(s))) {
         struct {
@@ -48,21 +49,28 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
             uint8_t id;
             uint16_t tag_le;
         } QEMU_PACKED out;
-        VirtQueueElement *elem;
 
         elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
         if (!elem) {
-            pdu_free(pdu);
-            break;
+            goto out_free_pdu;
         }
 
-        BUG_ON(elem->out_num == 0 || elem->in_num == 0);
+        if (elem->in_num == 0) {
+            virtio_error(vdev,
+                         "The guest sent a VirtFS request without space for "
+                         "the reply");
+            goto out_free_req;
+        }
         QEMU_BUILD_BUG_ON(sizeof(out) != 7);
 
         v->elems[pdu->idx] = elem;
         len = iov_to_buf(elem->out_sg, elem->out_num, 0,
                          &out, sizeof(out));
-        BUG_ON(len != sizeof(out));
+        if (len != sizeof(out)) {
+            virtio_error(vdev, "The guest sent a malformed VirtFS request: "
+                         "header size is %zd, should be 7", len);
+            goto out_free_req;
+        }
 
         pdu->size = le32_to_cpu(out.size_le);
 
@@ -72,6 +80,14 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
         qemu_co_queue_init(&pdu->complete);
         pdu_submit(pdu);
     }
+
+    return;
+
+out_free_req:
+    virtqueue_detach_element(vq, elem, 0);
+    g_free(elem);
+out_free_pdu:
+    pdu_free(pdu);
 }
 
 static uint64_t virtio_9p_get_features(VirtIODevice *vdev, uint64_t features,
-- 
MST

[Qemu-devel] [PULL 14/33] virtio-blk: handle virtio_blk_handle_request() errors

[Michael S. Tsirkin]

From: Greg Kurz <groug@kaod.org>

All these errors are caused by a buggy guest: QEMU should not exit.

With this patch, if virtio_blk_handle_request() detects a buggy request, it
marks the device as broken and returns an error to the caller so it takes
appropriate action.

In the case of virtio_blk_handle_vq(), we detach the request from the
virtqueue, free its allocated memory and stop popping new requests.
We don't need to bother about multireq since virtio_blk_handle_request()
errors out early and mrb.num_reqs == 0.

In the case of virtio_blk_dma_restart_bh(), we need to detach and free all
queued requests as well.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/block/virtio-blk.c | 38 ++++++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index bbacd56..0ddd7fb 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -468,30 +468,32 @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
     return true;
 }
 
-static void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
+static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
 {
     uint32_t type;
     struct iovec *in_iov = req->elem.in_sg;
     struct iovec *iov = req->elem.out_sg;
     unsigned in_num = req->elem.in_num;
     unsigned out_num = req->elem.out_num;
+    VirtIOBlock *s = req->dev;
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
 
     if (req->elem.out_num < 1 || req->elem.in_num < 1) {
-        error_report("virtio-blk missing headers");
-        exit(1);
+        virtio_error(vdev, "virtio-blk missing headers");
+        return -1;
     }
 
     if (unlikely(iov_to_buf(iov, out_num, 0, &req->out,
                             sizeof(req->out)) != sizeof(req->out))) {
-        error_report("virtio-blk request outhdr too short");
-        exit(1);
+        virtio_error(vdev, "virtio-blk request outhdr too short");
+        return -1;
     }
 
     iov_discard_front(&iov, &out_num, sizeof(req->out));
 
     if (in_iov[in_num - 1].iov_len < sizeof(struct virtio_blk_inhdr)) {
-        error_report("virtio-blk request inhdr too short");
-        exit(1);
+        virtio_error(vdev, "virtio-blk request inhdr too short");
+        return -1;
     }
 
     /* We always touch the last byte, so just see how big in_iov is.  */
@@ -529,7 +531,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
             block_acct_invalid(blk_get_stats(req->dev->blk),
                                is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ);
             virtio_blk_free_request(req);
-            return;
+            return 0;
         }
 
         block_acct_start(blk_get_stats(req->dev->blk),
@@ -576,6 +578,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
         virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
         virtio_blk_free_request(req);
     }
+    return 0;
 }
 
 void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq)
@@ -586,7 +589,11 @@ void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq)
     blk_io_plug(s->blk);
 
     while ((req = virtio_blk_get_request(s, vq))) {
-        virtio_blk_handle_request(req, &mrb);
+        if (virtio_blk_handle_request(req, &mrb)) {
+            virtqueue_detach_element(req->vq, &req->elem, 0);
+            virtio_blk_free_request(req);
+            break;
+        }
     }
 
     if (mrb.num_reqs) {
@@ -625,7 +632,18 @@ static void virtio_blk_dma_restart_bh(void *opaque)
 
     while (req) {
         VirtIOBlockReq *next = req->next;
-        virtio_blk_handle_request(req, &mrb);
+        if (virtio_blk_handle_request(req, &mrb)) {
+            /* Device is now broken and won't do any processing until it gets
+             * reset. Already queued requests will be lost: let's purge them.
+             */
+            while (req) {
+                next = req->next;
+                virtqueue_detach_element(req->vq, &req->elem, 0);
+                virtio_blk_free_request(req);
+                req = next;
+            }
+            break;
+        }
         req = next;
     }
 
-- 
MST

[Qemu-devel] [PULL 15/33] virtio-net: handle virtio_net_handle_ctrl() error

[Michael S. Tsirkin]

From: Greg Kurz <groug@kaod.org>

This error is caused by a buggy guest: let's switch the device to the
broken state instead of terminating QEMU. Also we detach the element
from the virtqueue and free it.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/virtio-net.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 6b8ae2c..a1584e1 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -880,6 +880,7 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
 
     return VIRTIO_NET_OK;
 }
+
 static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
@@ -897,8 +898,10 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         }
         if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
             iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
-            error_report("virtio-net ctrl missing headers");
-            exit(1);
+            virtio_error(vdev, "virtio-net ctrl missing headers");
+            virtqueue_detach_element(vq, elem, 0);
+            g_free(elem);
+            break;
         }
 
         iov_cnt = elem->out_num;
-- 
MST

[Qemu-devel] [PULL 16/33] virtio-net: handle virtio_net_receive() errors

[Michael S. Tsirkin]

From: Greg Kurz <groug@kaod.org>

All these errors are caused by a buggy guest: let's switch the device to
the broken state instead of terminating QEMU. Also we detach the element
from the virtqueue and free it.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/virtio-net.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index a1584e1..5c0b2e0 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1130,21 +1130,24 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
 
         elem = virtqueue_pop(q->rx_vq, sizeof(VirtQueueElement));
         if (!elem) {
-            if (i == 0)
-                return -1;
-            error_report("virtio-net unexpected empty queue: "
-                         "i %zd mergeable %d offset %zd, size %zd, "
-                         "guest hdr len %zd, host hdr len %zd "
-                         "guest features 0x%" PRIx64,
-                         i, n->mergeable_rx_bufs, offset, size,
-                         n->guest_hdr_len, n->host_hdr_len,
-                         vdev->guest_features);
-            exit(1);
+            if (i) {
+                virtio_error(vdev, "virtio-net unexpected empty queue: "
+                             "i %zd mergeable %d offset %zd, size %zd, "
+                             "guest hdr len %zd, host hdr len %zd "
+                             "guest features 0x%" PRIx64,
+                             i, n->mergeable_rx_bufs, offset, size,
+                             n->guest_hdr_len, n->host_hdr_len,
+                             vdev->guest_features);
+            }
+            return -1;
         }
 
         if (elem->in_num < 1) {
-            error_report("virtio-net receive queue contains no in buffers");
-            exit(1);
+            virtio_error(vdev,
+                         "virtio-net receive queue contains no in buffers");
+            virtqueue_detach_element(q->rx_vq, elem, 0);
+            g_free(elem);
+            return -1;
         }
 
         sg = elem->in_sg;
-- 
MST

[Qemu-devel] [PULL 17/33] virtio-net: handle virtio_net_flush_tx() errors

[Michael S. Tsirkin]

From: Greg Kurz <groug@kaod.org>

All these errors are caused by a buggy guest: let's switch the device to
the broken state instead of terminating QEMU. Also we detach the element
from the virtqueue and free it.

If this happens, virtio_net_flush_tx() also returns -EINVAL, so that all
callers can stop processing the virtqueue immediatly.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/virtio-net.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 5c0b2e0..ca1b469 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1249,15 +1249,19 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
         out_num = elem->out_num;
         out_sg = elem->out_sg;
         if (out_num < 1) {
-            error_report("virtio-net header not in first element");
-            exit(1);
+            virtio_error(vdev, "virtio-net header not in first element");
+            virtqueue_detach_element(q->tx_vq, elem, 0);
+            g_free(elem);
+            return -EINVAL;
         }
 
         if (n->has_vnet_hdr) {
             if (iov_to_buf(out_sg, out_num, 0, &mhdr, n->guest_hdr_len) <
                 n->guest_hdr_len) {
-                error_report("virtio-net header incorrect");
-                exit(1);
+                virtio_error(vdev, "virtio-net header incorrect");
+                virtqueue_detach_element(q->tx_vq, elem, 0);
+                g_free(elem);
+                return -EINVAL;
             }
             if (n->needs_vnet_hdr_swap) {
                 virtio_net_hdr_swap(vdev, (void *) &mhdr);
@@ -1325,7 +1329,9 @@ static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq)
         virtio_queue_set_notification(vq, 1);
         timer_del(q->tx_timer);
         q->tx_waiting = 0;
-        virtio_net_flush_tx(q);
+        if (virtio_net_flush_tx(q) == -EINVAL) {
+            return;
+        }
     } else {
         timer_mod(q->tx_timer,
                        qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + n->tx_timeout);
@@ -1396,8 +1402,9 @@ static void virtio_net_tx_bh(void *opaque)
     }
 
     ret = virtio_net_flush_tx(q);
-    if (ret == -EBUSY) {
-        return; /* Notification re-enable handled by tx_complete */
+    if (ret == -EBUSY || ret == -EINVAL) {
+        return; /* Notification re-enable handled by tx_complete or device
+                 * broken */
     }
 
     /* If we flush a full burst of packets, assume there are
@@ -1412,7 +1419,10 @@ static void virtio_net_tx_bh(void *opaque)
      * anything that may have come in while we weren't looking.  If
      * we find something, assume the guest is still active and reschedule */
     virtio_queue_set_notification(q->tx_vq, 1);
-    if (virtio_net_flush_tx(q) > 0) {
+    ret = virtio_net_flush_tx(q);
+    if (ret == -EINVAL) {
+        return;
+    } else if (ret > 0) {
         virtio_queue_set_notification(q->tx_vq, 0);
         qemu_bh_schedule(q->tx_bh);
         q->tx_waiting = 1;
-- 
MST

[Qemu-devel] [PULL 18/33] virtio-scsi: convert virtio_scsi_bad_req() to use virtio_error()

[Michael S. Tsirkin]

From: Greg Kurz <groug@kaod.org>

The virtio_scsi_bad_req() function is called when a guest sends a
request with missing or ill-sized headers. This generally happens
when the virtio_scsi_parse_req() function returns an error.

With this patch, virtio_scsi_bad_req() will mark the device as broken,
detach the request from the virtqueue and free it, instead of forcing
QEMU to exit.

In nearly all locations where virtio_scsi_bad_req() is called, the only
thing to do next is to return to the caller.

The virtio_scsi_handle_cmd_req_prepare() function is an exception though.

It is called in a loop by virtio_scsi_handle_cmd_vq() and passed requests
freshly popped from a cmd virtqueue; virtio_scsi_handle_cmd_req_prepare()
does some sanity checks on the request and returns a boolean flag to
indicate whether the request should be queued or not. In the latter case,
virtio_scsi_handle_cmd_req_prepare() has detected a non-fatal error and
sent a response back to the guest.

We have now a new condition to take into account: the device is broken
and should stop all processing.

The return value of virtio_scsi_handle_cmd_req_prepare() is hence changed
to an int. A return value of zero means that the request should be queued.
Other non-fatal error cases where the request shoudn't be queued  return
a negative errno (values are vaguely inspired by the error condition, but
the only goal here is to discriminate the case we're interested in).

And finally, if virtio_scsi_bad_req() was called, -EINVAL is returned. In
this case, virtio_scsi_handle_cmd_vq() detaches and frees already queued
requests, instead of submitting them.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/scsi/virtio-scsi.c | 46 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index e596b64..b58de95 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -81,10 +81,11 @@ static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
     virtio_scsi_free_req(req);
 }
 
-static void virtio_scsi_bad_req(void)
+static void virtio_scsi_bad_req(VirtIOSCSIReq *req)
 {
-    error_report("wrong size for virtio-scsi headers");
-    exit(1);
+    virtio_error(VIRTIO_DEVICE(req->dev), "wrong size for virtio-scsi headers");
+    virtqueue_detach_element(req->vq, &req->elem, 0);
+    virtio_scsi_free_req(req);
 }
 
 static size_t qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *iov,
@@ -387,7 +388,7 @@ static void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
 
     if (iov_to_buf(req->elem.out_sg, req->elem.out_num, 0,
                 &type, sizeof(type)) < sizeof(type)) {
-        virtio_scsi_bad_req();
+        virtio_scsi_bad_req(req);
         return;
     }
 
@@ -395,7 +396,8 @@ static void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
     if (type == VIRTIO_SCSI_T_TMF) {
         if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq),
                     sizeof(VirtIOSCSICtrlTMFResp)) < 0) {
-            virtio_scsi_bad_req();
+            virtio_scsi_bad_req(req);
+            return;
         } else {
             r = virtio_scsi_do_tmf(s, req);
         }
@@ -404,7 +406,8 @@ static void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
                type == VIRTIO_SCSI_T_AN_SUBSCRIBE) {
         if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlANReq),
                     sizeof(VirtIOSCSICtrlANResp)) < 0) {
-            virtio_scsi_bad_req();
+            virtio_scsi_bad_req(req);
+            return;
         } else {
             req->resp.an.event_actual = 0;
             req->resp.an.response = VIRTIO_SCSI_S_OK;
@@ -521,7 +524,7 @@ static void virtio_scsi_fail_cmd_req(VirtIOSCSIReq *req)
     virtio_scsi_complete_cmd_req(req);
 }
 
-static bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
+static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
 {
     VirtIOSCSICommon *vs = &s->parent_obj;
     SCSIDevice *d;
@@ -532,17 +535,18 @@ static bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req
     if (rc < 0) {
         if (rc == -ENOTSUP) {
             virtio_scsi_fail_cmd_req(req);
+            return -ENOTSUP;
         } else {
-            virtio_scsi_bad_req();
+            virtio_scsi_bad_req(req);
+            return -EINVAL;
         }
-        return false;
     }
 
     d = virtio_scsi_device_find(s, req->req.cmd.lun);
     if (!d) {
         req->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
         virtio_scsi_complete_cmd_req(req);
-        return false;
+        return -ENOENT;
     }
     virtio_scsi_ctx_check(s, d);
     req->sreq = scsi_req_new(d, req->req.cmd.tag,
@@ -554,11 +558,11 @@ static bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req
             req->sreq->cmd.xfer > req->qsgl.size)) {
         req->resp.cmd.response = VIRTIO_SCSI_S_OVERRUN;
         virtio_scsi_complete_cmd_req(req);
-        return false;
+        return -ENOBUFS;
     }
     scsi_req_ref(req->sreq);
     blk_io_plug(d->conf.blk);
-    return true;
+    return 0;
 }
 
 static void virtio_scsi_handle_cmd_req_submit(VirtIOSCSI *s, VirtIOSCSIReq *req)
@@ -574,11 +578,24 @@ static void virtio_scsi_handle_cmd_req_submit(VirtIOSCSI *s, VirtIOSCSIReq *req)
 void virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq)
 {
     VirtIOSCSIReq *req, *next;
+    int ret;
+
     QTAILQ_HEAD(, VirtIOSCSIReq) reqs = QTAILQ_HEAD_INITIALIZER(reqs);
 
     while ((req = virtio_scsi_pop_req(s, vq))) {
-        if (virtio_scsi_handle_cmd_req_prepare(s, req)) {
+        ret = virtio_scsi_handle_cmd_req_prepare(s, req);
+        if (!ret) {
             QTAILQ_INSERT_TAIL(&reqs, req, next);
+        } else if (ret == -EINVAL) {
+            /* The device is broken and shouldn't process any request */
+            while (!QTAILQ_EMPTY(&reqs)) {
+                req = QTAILQ_FIRST(&reqs);
+                QTAILQ_REMOVE(&reqs, req, next);
+                blk_io_unplug(req->sreq->dev->conf.blk);
+                scsi_req_unref(req->sreq);
+                virtqueue_detach_element(req->vq, &req->elem, 0);
+                virtio_scsi_free_req(req);
+            }
         }
     }
 
@@ -708,7 +725,8 @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
     }
 
     if (virtio_scsi_parse_req(req, 0, sizeof(VirtIOSCSIEvent))) {
-        virtio_scsi_bad_req();
+        virtio_scsi_bad_req(req);
+        goto out;
     }
 
     evt = &req->resp.event;
-- 
MST

[Qemu-devel] [PULL 19/33] virtio-scsi: handle virtio_scsi_set_config() error

[Michael S. Tsirkin]

From: Greg Kurz <groug@kaod.org>

This error is caused by a buggy guest: let's switch the device to the
broken state instead of terminating QEMU.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/scsi/virtio-scsi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index b58de95..6eaadd8 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -644,8 +644,9 @@ static void virtio_scsi_set_config(VirtIODevice *vdev,
 
     if ((uint32_t) virtio_ldl_p(vdev, &scsiconf->sense_size) >= 65536 ||
         (uint32_t) virtio_ldl_p(vdev, &scsiconf->cdb_size) >= 256) {
-        error_report("bad data written to virtio-scsi configuration space");
-        exit(1);
+        virtio_error(vdev,
+                     "bad data written to virtio-scsi configuration space");
+        return;
     }
 
     vs->sense_size = virtio_ldl_p(vdev, &scsiconf->sense_size);
-- 
MST

[Qemu-devel] [PULL 20/33] net: don't poke at chardev internal QemuOpts

[Michael S. Tsirkin]

From: "Daniel P. Berrange" <berrange@redhat.com>

The vhost-user & colo code is poking at the QemuOpts instance
in the CharDriverState struct, not realizing that it is valid
for this to be NULL. e.g. the following crash shows a codepath
where it will be NULL:

 Program terminated with signal SIGSEGV, Segmentation fault.
 #0  0x000055baf6ab4adc in qemu_opt_foreach (opts=0x0, func=0x55baf696b650 <net_vhost_chardev_opts>, opaque=0x7ffc51368c00, errp=0x7ffc51368e48) at util/qemu-option.c:617
 617         QTAILQ_FOREACH(opt, &opts->head, next) {
 [Current thread is 1 (Thread 0x7f1d4970bb40 (LWP 6603))]
 (gdb) bt
 #0  0x000055baf6ab4adc in qemu_opt_foreach (opts=0x0, func=0x55baf696b650 <net_vhost_chardev_opts>, opaque=0x7ffc51368c00, errp=0x7ffc51368e48) at util/qemu-option.c:617
 #1  0x000055baf696b7da in net_vhost_parse_chardev (opts=0x55baf8ff9260, errp=0x7ffc51368e48) at net/vhost-user.c:314
 #2  0x000055baf696b985 in net_init_vhost_user (netdev=0x55baf8ff9250, name=0x55baf879d270 "hostnet2", peer=0x0, errp=0x7ffc51368e48) at net/vhost-user.c:360
 #3  0x000055baf6960216 in net_client_init1 (object=0x55baf8ff9250, is_netdev=true, errp=0x7ffc51368e48) at net/net.c:1051
 #4  0x000055baf6960518 in net_client_init (opts=0x55baf776e7e0, is_netdev=true, errp=0x7ffc51368f00) at net/net.c:1108
 #5  0x000055baf696083f in netdev_add (opts=0x55baf776e7e0, errp=0x7ffc51368f00) at net/net.c:1186
 #6  0x000055baf69608c7 in qmp_netdev_add (qdict=0x55baf7afaf60, ret=0x7ffc51368f50, errp=0x7ffc51368f48) at net/net.c:1205
 #7  0x000055baf6622135 in handle_qmp_command (parser=0x55baf77fb590, tokens=0x7f1d24011960) at /path/to/qemu.git/monitor.c:3978
 #8  0x000055baf6a9d099 in json_message_process_token (lexer=0x55baf77fb598, input=0x55baf75acd20, type=JSON_RCURLY, x=113, y=19) at qobject/json-streamer.c:105
 #9  0x000055baf6abf7aa in json_lexer_feed_char (lexer=0x55baf77fb598, ch=125 '}', flush=false) at qobject/json-lexer.c:319
 #10 0x000055baf6abf8f2 in json_lexer_feed (lexer=0x55baf77fb598, buffer=0x7ffc51369170 "}R\204\367\272U", size=1) at qobject/json-lexer.c:369
 #11 0x000055baf6a9d13c in json_message_parser_feed (parser=0x55baf77fb590, buffer=0x7ffc51369170 "}R\204\367\272U", size=1) at qobject/json-streamer.c:124
 #12 0x000055baf66221f7 in monitor_qmp_read (opaque=0x55baf77fb530, buf=0x7ffc51369170 "}R\204\367\272U", size=1) at /path/to/qemu.git/monitor.c:3994
 #13 0x000055baf6757014 in qemu_chr_be_write_impl (s=0x55baf7610a40, buf=0x7ffc51369170 "}R\204\367\272U", len=1) at qemu-char.c:387
 #14 0x000055baf6757076 in qemu_chr_be_write (s=0x55baf7610a40, buf=0x7ffc51369170 "}R\204\367\272U", len=1) at qemu-char.c:399
 #15 0x000055baf675b3b0 in tcp_chr_read (chan=0x55baf90244b0, cond=G_IO_IN, opaque=0x55baf7610a40) at qemu-char.c:2927
 #16 0x000055baf6a5d655 in qio_channel_fd_source_dispatch (source=0x55baf7610df0, callback=0x55baf675b25a <tcp_chr_read>, user_data=0x55baf7610a40) at io/channel-watch.c:84
 #17 0x00007f1d3e80cbbd in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
 #18 0x000055baf69d3720 in glib_pollfds_poll () at main-loop.c:213
 #19 0x000055baf69d37fd in os_host_main_loop_wait (timeout=126000000) at main-loop.c:258
 #20 0x000055baf69d38ad in main_loop_wait (nonblocking=0) at main-loop.c:506
 #21 0x000055baf676587b in main_loop () at vl.c:1908
 #22 0x000055baf676d3bf in main (argc=101, argv=0x7ffc5136a6c8, envp=0x7ffc5136a9f8) at vl.c:4604
 (gdb) p opts
 $1 = (QemuOpts *) 0x0

The crash occurred when attaching vhost-user net via QMP:

{
    "execute": "chardev-add",
    "arguments": {
        "id": "charnet2",
        "backend": {
            "type": "socket",
            "data": {
                "addr": {
                    "type": "unix",
                    "data": {
                        "path": "/var/run/openvswitch/vhost-user1"
                    }
                },
                "wait": false,
                "server": false
            }
        }
    },
    "id": "libvirt-19"
}
{
    "return": {

    },
    "id": "libvirt-19"
}
{
    "execute": "netdev_add",
    "arguments": {
        "type": "vhost-user",
        "chardev": "charnet2",
        "id": "hostnet2"
    },
    "id": "libvirt-20"
}

Code using chardevs should not be poking at the internals of the
CharDriverState struct. What vhost-user wants is a chardev that is
operating as reconnectable network service, along with the ability
to do FD passing over the connection. The colo code simply wants
a network service. Add a feature concept to the char drivers so
that chardev users can query the actual features they wish to have
supported. The QemuOpts member is removed to prevent future mistakes
in this area.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/sysemu/char.h | 21 ++++++++++++++++++++-
 hmp.c                 |  1 +
 net/colo-compare.c    | 30 ++----------------------------
 net/vhost-user.c      | 41 +++++++----------------------------------
 qemu-char.c           | 22 +++++++++++++++++++---
 5 files changed, 49 insertions(+), 66 deletions(-)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 0d0465a..19dad3f 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -9,6 +9,7 @@
 #include "qapi/qmp/qobject.h"
 #include "qapi/qmp/qstring.h"
 #include "qemu/main-loop.h"
+#include "qemu/bitmap.h"
 
 /* character device */
 
@@ -58,6 +59,20 @@ struct ParallelIOArg {
 
 typedef void IOEventHandler(void *opaque, int event);
 
+typedef enum {
+    /* Whether the chardev peer is able to close and
+     * reopen the data channel, thus requiring support
+     * for qemu_chr_wait_connected() to wait for a
+     * valid connection */
+    QEMU_CHAR_FEATURE_RECONNECTABLE,
+    /* Whether it is possible to send/recv file descriptors
+     * over the data channel */
+    QEMU_CHAR_FEATURE_FD_PASS,
+
+    QEMU_CHAR_FEATURE_LAST,
+} CharDriverFeature;
+
+
 struct CharDriverState {
     QemuMutex chr_write_lock;
     void (*init)(struct CharDriverState *s);
@@ -93,8 +108,8 @@ struct CharDriverState {
     int avail_connections;
     int is_mux;
     guint fd_in_tag;
-    QemuOpts *opts;
     bool replay;
+    DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST);
     QTAILQ_ENTRY(CharDriverState) next;
 };
 
@@ -437,6 +452,10 @@ int qemu_chr_add_client(CharDriverState *s, int fd);
 CharDriverState *qemu_chr_find(const char *name);
 bool chr_is_ringbuf(const CharDriverState *chr);
 
+bool qemu_chr_has_feature(CharDriverState *chr,
+                          CharDriverFeature feature);
+void qemu_chr_set_feature(CharDriverState *chr,
+                          CharDriverFeature feature);
 QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
 
 void register_char_driver(const char *name, ChardevBackendKind kind,
diff --git a/hmp.c b/hmp.c
index 336e7bf..3c42182 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1909,6 +1909,7 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict)
         error_setg(&err, "Parsing chardev args failed");
     } else {
         qemu_chr_new_from_opts(opts, NULL, &err);
+        qemu_opts_del(opts);
     }
     hmp_handle_error(mon, &err);
 }
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 22b1da1..47703c5 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -564,29 +564,6 @@ static void compare_sec_rs_finalize(SocketReadState *sec_rs)
     }
 }
 
-static int compare_chardev_opts(void *opaque,
-                                const char *name, const char *value,
-                                Error **errp)
-{
-    CompareChardevProps *props = opaque;
-
-    if (strcmp(name, "backend") == 0 &&
-        strcmp(value, "socket") == 0) {
-        props->is_socket = true;
-        return 0;
-    } else if (strcmp(name, "host") == 0 ||
-              (strcmp(name, "port") == 0) ||
-              (strcmp(name, "server") == 0) ||
-              (strcmp(name, "wait") == 0) ||
-              (strcmp(name, "path") == 0)) {
-        return 0;
-    } else {
-        error_setg(errp,
-                   "COLO-compare does not support a chardev with option %s=%s",
-                   name, value);
-        return -1;
-    }
-}
 
 /*
  * Return 0 is success.
@@ -606,12 +583,9 @@ static int find_and_check_chardev(CharDriverState **chr,
     }
 
     memset(&props, 0, sizeof(props));
-    if (qemu_opt_foreach((*chr)->opts, compare_chardev_opts, &props, errp)) {
-        return 1;
-    }
 
-    if (!props.is_socket) {
-        error_setg(errp, "chardev \"%s\" is not a tcp socket",
+    if (!qemu_chr_has_feature(*chr, QEMU_CHAR_FEATURE_RECONNECTABLE)) {
+        error_setg(errp, "chardev \"%s\" is not reconnectable",
                    chr_name);
         return 1;
     }
diff --git a/net/vhost-user.c b/net/vhost-user.c
index b0595f8..5b94c84 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -27,11 +27,6 @@ typedef struct VhostUserState {
     bool started;
 } VhostUserState;
 
-typedef struct VhostUserChardevProps {
-    bool is_socket;
-    bool is_unix;
-} VhostUserChardevProps;
-
 VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
 {
     VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
@@ -278,45 +273,23 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
     return 0;
 }
 
-static int net_vhost_chardev_opts(void *opaque,
-                                  const char *name, const char *value,
-                                  Error **errp)
-{
-    VhostUserChardevProps *props = opaque;
-
-    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
-        props->is_socket = true;
-    } else if (strcmp(name, "path") == 0) {
-        props->is_unix = true;
-    } else if (strcmp(name, "server") == 0) {
-    } else {
-        error_setg(errp,
-                   "vhost-user does not support a chardev with option %s=%s",
-                   name, value);
-        return -1;
-    }
-    return 0;
-}
-
-static CharDriverState *net_vhost_parse_chardev(
+static CharDriverState *net_vhost_claim_chardev(
     const NetdevVhostUserOptions *opts, Error **errp)
 {
     CharDriverState *chr = qemu_chr_find(opts->chardev);
-    VhostUserChardevProps props;
 
     if (chr == NULL) {
         error_setg(errp, "chardev \"%s\" not found", opts->chardev);
         return NULL;
     }
 
-    /* inspect chardev opts */
-    memset(&props, 0, sizeof(props));
-    if (qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, errp)) {
+    if (!qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_RECONNECTABLE)) {
+        error_setg(errp, "chardev \"%s\" is not reconnectable",
+                   opts->chardev);
         return NULL;
     }
-
-    if (!props.is_socket || !props.is_unix) {
-        error_setg(errp, "chardev \"%s\" is not a unix socket",
+    if (!qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_FD_PASS)) {
+        error_setg(errp, "chardev \"%s\" does not support FD passing",
                    opts->chardev);
         return NULL;
     }
@@ -357,7 +330,7 @@ int net_init_vhost_user(const Netdev *netdev, const char *name,
     assert(netdev->type == NET_CLIENT_DRIVER_VHOST_USER);
     vhost_user_opts = &netdev->u.vhost_user;
 
-    chr = net_vhost_parse_chardev(vhost_user_opts, errp);
+    chr = net_vhost_claim_chardev(vhost_user_opts, errp);
     if (!chr) {
         return -1;
     }
diff --git a/qemu-char.c b/qemu-char.c
index fb456ce..768150d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3996,7 +3996,6 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
     }
 
     chr = qemu_chr_find(id);
-    chr->opts = opts;
 
 qapi_out:
     qapi_free_ChardevBackend(backend);
@@ -4005,7 +4004,6 @@ qapi_out:
     return chr;
 
 err:
-    qemu_opts_del(opts);
     return NULL;
 }
 
@@ -4033,6 +4031,7 @@ CharDriverState *qemu_chr_new_noreplay(const char *label, const char *filename,
         qemu_chr_fe_claim_no_fail(chr);
         monitor_init(chr, MONITOR_USE_READLINE);
     }
+    qemu_opts_del(opts);
     return chr;
 }
 
@@ -4132,7 +4131,6 @@ static void qemu_chr_free_common(CharDriverState *chr)
 {
     g_free(chr->filename);
     g_free(chr->label);
-    qemu_opts_del(chr->opts);
     if (chr->logfd != -1) {
         close(chr->logfd);
     }
@@ -4513,6 +4511,11 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
 
     s->addr = QAPI_CLONE(SocketAddress, sock->addr);
 
+    qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_RECONNECTABLE);
+    if (s->is_unix) {
+        qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS);
+    }
+
     chr->opaque = s;
     chr->chr_wait_connected = tcp_chr_wait_connected;
     chr->chr_write = tcp_chr_write;
@@ -4596,6 +4599,19 @@ static CharDriverState *qmp_chardev_open_udp(const char *id,
     return qemu_chr_open_udp(sioc, common, errp);
 }
 
+
+bool qemu_chr_has_feature(CharDriverState *chr,
+                          CharDriverFeature feature)
+{
+    return test_bit(feature, chr->features);
+}
+
+void qemu_chr_set_feature(CharDriverState *chr,
+                           CharDriverFeature feature)
+{
+    return set_bit(feature, chr->features);
+}
+
 ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
                                Error **errp)
 {
-- 
MST

[Qemu-devel] [PULL 21/33] virtio: prepare change VMSTATE_VIRTIO_DEVICE macro

[Michael S. Tsirkin]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

In most cases the functions passed to VMSTATE_VIRTIO_DEVICE
only call the virtio_load and virtio_save wrappers. Some include some
pre- and post- massaging too. The massaging is better expressed
as such in the VMStateDescription.

Let us prepare for changing the semantic of the VMSTATE_VIRTIO_DEVICE
macro so that it is more similar to the other VMSTATE_*_DEVICE macros
in a sense that it is a field definition.

The preprocessor conditionals are going to be removed as soon as
every usage is converted to the new semantic.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/virtio.h | 16 ++++++++++++++++
 hw/virtio/virtio.c         | 21 +++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index e25ec4f..929fa92 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -179,6 +179,20 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
 void virtio_save(VirtIODevice *vdev, QEMUFile *f);
 void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size);
 
+extern const VMStateInfo virtio_vmstate_info;
+
+#ifdef VMSTATE_VIRTIO_DEVICE_USE_NEW
+
+#define VMSTATE_VIRTIO_DEVICE \
+    {                                         \
+        .name = "virtio",                     \
+        .info = &virtio_vmstate_info,         \
+        .flags = VMS_SINGLE,                  \
+    }
+
+#else
+/* TODO remove conditional as soon as all users are converted */
+
 #define VMSTATE_VIRTIO_DEVICE(devname, v, getf, putf) \
     static const VMStateDescription vmstate_virtio_ ## devname = { \
         .name = "virtio-" #devname ,          \
@@ -198,6 +212,8 @@ void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size);
         }                                     \
     }
 
+#endif
+
 int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id);
 
 void virtio_notify_config(VirtIODevice *vdev);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 46f79c9..62b9c00 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1645,6 +1645,27 @@ void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size)
     virtio_save(VIRTIO_DEVICE(opaque), f);
 }
 
+/* A wrapper for use as a VMState .put function */
+static void virtio_device_put(QEMUFile *f, void *opaque, size_t size)
+{
+    virtio_save(VIRTIO_DEVICE(opaque), f);
+}
+
+/* A wrapper for use as a VMState .get function */
+static int virtio_device_get(QEMUFile *f, void *opaque, size_t size)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
+    DeviceClass *dc = DEVICE_CLASS(VIRTIO_DEVICE_GET_CLASS(vdev));
+
+    return virtio_load(vdev, f, dc->vmsd->version_id);
+}
+
+const VMStateInfo  virtio_vmstate_info = {
+    .name = "virtio",
+    .get = virtio_device_get,
+    .put = virtio_device_put,
+};
+
 static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
 {
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-- 
MST

[Qemu-devel] [PULL 22/33] virtio-blk: convert VMSTATE_VIRTIO_DEVICE

[Michael S. Tsirkin]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/block/virtio-blk.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 0ddd7fb..10c5794 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -11,6 +11,8 @@
  *
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
@@ -822,13 +824,6 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
     }
 }
 
-static void virtio_blk_save(QEMUFile *f, void *opaque, size_t size)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
-
-    virtio_save(vdev, f);
-}
-    
 static void virtio_blk_save_device(VirtIODevice *vdev, QEMUFile *f)
 {
     VirtIOBlock *s = VIRTIO_BLK(vdev);
@@ -847,14 +842,6 @@ static void virtio_blk_save_device(VirtIODevice *vdev, QEMUFile *f)
     qemu_put_sbyte(f, 0);
 }
 
-static int virtio_blk_load(QEMUFile *f, void *opaque, size_t size)
-{
-    VirtIOBlock *s = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(s);
-
-    return virtio_load(vdev, f, 2);
-}
-
 static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
                                   int version_id)
 {
@@ -975,7 +962,15 @@ static void virtio_blk_instance_init(Object *obj)
                                   DEVICE(obj), NULL);
 }
 
-VMSTATE_VIRTIO_DEVICE(blk, 2, virtio_blk_load, virtio_blk_save);
+static const VMStateDescription vmstate_virtio_blk = {
+    .name = "virtio-blk",
+    .minimum_version_id = 2,
+    .version_id = 2,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static Property virtio_blk_properties[] = {
     DEFINE_BLOCK_PROPERTIES(VirtIOBlock, conf.conf),
-- 
MST

[Qemu-devel] [PULL 23/33] virtio-net: convert VMSTATE_VIRTIO_DEVICE

[Michael S. Tsirkin]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/virtio-net.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index ca1b469..b2198a5 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -11,6 +11,8 @@
  *
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qemu/iov.h"
 #include "hw/virtio/virtio.h"
@@ -1514,17 +1516,6 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue)
     virtio_net_set_queues(n);
 }
 
-static void virtio_net_save(QEMUFile *f, void *opaque, size_t size)
-{
-    VirtIONet *n = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(n);
-
-    /* At this point, backend must be stopped, otherwise
-     * it might keep writing to memory. */
-    assert(!n->vhost_started);
-    virtio_save(vdev, f);
-}
-
 static void virtio_net_save_device(VirtIODevice *vdev, QEMUFile *f)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
@@ -1560,14 +1551,6 @@ static void virtio_net_save_device(VirtIODevice *vdev, QEMUFile *f)
     }
 }
 
-static int virtio_net_load(QEMUFile *f, void *opaque, size_t size)
-{
-    VirtIONet *n = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(n);
-
-    return virtio_load(vdev, f, VIRTIO_NET_VM_VERSION);
-}
-
 static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f,
                                   int version_id)
 {
@@ -1870,8 +1853,25 @@ static void virtio_net_instance_init(Object *obj)
                                   DEVICE(n), NULL);
 }
 
-VMSTATE_VIRTIO_DEVICE(net, VIRTIO_NET_VM_VERSION, virtio_net_load,
-                      virtio_net_save);
+static void virtio_net_pre_save(void *opaque)
+{
+    VirtIONet *n = opaque;
+
+    /* At this point, backend must be stopped, otherwise
+     * it might keep writing to memory. */
+    assert(!n->vhost_started);
+}
+
+static const VMStateDescription vmstate_virtio_net = {
+    .name = "virtio-net",
+    .minimum_version_id = VIRTIO_NET_VM_VERSION,
+    .version_id = VIRTIO_NET_VM_VERSION,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+    .pre_save = virtio_net_pre_save,
+};
 
 static Property virtio_net_properties[] = {
     DEFINE_PROP_BIT("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, true),
-- 
MST

[Qemu-devel] [PULL 24/33] virtio-9p: convert VMSTATE_VIRTIO_DEVICE

[Michael S. Tsirkin]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/9pfs/virtio-9p-device.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index a338f64..526ec7d 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -11,6 +11,8 @@
  *
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "hw/virtio/virtio.h"
 #include "qemu/sockets.h"
@@ -113,11 +115,6 @@ static void virtio_9p_get_config(VirtIODevice *vdev, uint8_t *config)
     g_free(cfg);
 }
 
-static int virtio_9p_load(QEMUFile *f, void *opaque, size_t size)
-{
-    return virtio_load(VIRTIO_DEVICE(opaque), f, 1);
-}
-
 static void virtio_9p_device_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -184,7 +181,15 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
 
 /* virtio-9p device */
 
-VMSTATE_VIRTIO_DEVICE(9p, 1, virtio_9p_load, virtio_vmstate_save);
+static const VMStateDescription vmstate_virtio_9p = {
+    .name = "virtio-9p",
+    .minimum_version_id = 1,
+    .version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static Property virtio_9p_properties[] = {
     DEFINE_PROP_STRING("mount_tag", V9fsVirtioState, state.fsconf.tag),
-- 
MST

[Qemu-devel] [PULL 25/33] virtio-serial: convert VMSTATE_VIRTIO_DEVICE

[Michael S. Tsirkin]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/char/virtio-serial-bus.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 3955f0f..c9b0fc8 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -18,6 +18,8 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/iov.h"
@@ -778,12 +780,6 @@ static int fetch_active_ports_list(QEMUFile *f,
     return 0;
 }
 
-static int virtio_serial_load(QEMUFile *f, void *opaque, size_t size)
-{
-    /* The virtio device */
-    return virtio_load(VIRTIO_DEVICE(opaque), f, 3);
-}
-
 static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
                                      int version_id)
 {
@@ -1129,7 +1125,15 @@ static void virtio_serial_device_unrealize(DeviceState *dev, Error **errp)
 }
 
 /* Note: 'console' is used for backwards compatibility */
-VMSTATE_VIRTIO_DEVICE(console, 3, virtio_serial_load, virtio_vmstate_save);
+static const VMStateDescription vmstate_virtio_console = {
+    .name = "virtio-console",
+    .minimum_version_id = 3,
+    .version_id = 3,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static Property virtio_serial_properties[] = {
     DEFINE_PROP_UINT32("max_ports", VirtIOSerial, serial.max_virtserial_ports,
-- 
MST

[Qemu-devel] [PULL 26/33] virtio-gpu: convert VMSTATE_VIRTIO_DEVICE

[Michael S. Tsirkin]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Use the new VMSTATE_VIRTIO_DEVICE macro. The device virtio-gpu is
special because it actually does not adhere to the virtio migration
schema, because device state is last.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/display/virtio-gpu.c | 41 +++++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 7fe6ed8..4fcd63c 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -11,6 +11,8 @@
  * See the COPYING file in the top-level directory.
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/iov.h"
@@ -990,12 +992,9 @@ static const VMStateDescription vmstate_virtio_gpu_scanouts = {
 static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size)
 {
     VirtIOGPU *g = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(g);
     struct virtio_gpu_simple_resource *res;
     int i;
 
-    virtio_save(vdev, f);
-
     /* in 2d mode we should never find unprocessed commands here */
     assert(QTAILQ_EMPTY(&g->cmdq));
 
@@ -1020,16 +1019,10 @@ static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size)
 static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size)
 {
     VirtIOGPU *g = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(g);
     struct virtio_gpu_simple_resource *res;
     struct virtio_gpu_scanout *scanout;
     uint32_t resource_id, pformat;
-    int i, ret;
-
-    ret = virtio_load(vdev, f, VIRTIO_GPU_VM_VERSION);
-    if (ret) {
-        return ret;
-    }
+    int i;
 
     resource_id = qemu_get_be32(f);
     while (resource_id != 0) {
@@ -1219,8 +1212,32 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
 #endif
 }
 
-VMSTATE_VIRTIO_DEVICE(gpu, VIRTIO_GPU_VM_VERSION, virtio_gpu_load,
-                      virtio_gpu_save);
+/*
+ * For historical reasons virtio_gpu does not adhere to virtio migration
+ * scheme as described in doc/virtio-migration.txt, in a sense that no
+ * save/load callback are provided to the core. Instead the device data
+ * is saved/loaded after the core data.
+ *
+ * Because of this we need a special vmsd.
+ */
+static const VMStateDescription vmstate_virtio_gpu = {
+    .name = "virtio-gpu",
+    .minimum_version_id = VIRTIO_GPU_VM_VERSION,
+    .version_id = VIRTIO_GPU_VM_VERSION,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE /* core */,
+        {
+            .name = "virtio-gpu",
+            .info = &(const VMStateInfo) {
+                        .name = "virtio-gpu",
+                        .get = virtio_gpu_load,
+                        .put = virtio_gpu_save,
+            },
+            .flags = VMS_SINGLE,
+        } /* device */,
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static Property virtio_gpu_properties[] = {
     DEFINE_PROP_UINT32("max_outputs", VirtIOGPU, conf.max_outputs, 1),
-- 
MST

[Qemu-devel] [PULL 27/33] virtio-input: convert VMSTATE_VIRTIO_DEVICE

[Michael S. Tsirkin]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/input/virtio-input.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index ccdf730..5e31033 100644
--- a/hw/input/virtio-input.c
+++ b/hw/input/virtio-input.c
@@ -4,6 +4,8 @@
  * top-level directory.
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/iov.h"
@@ -217,19 +219,12 @@ static void virtio_input_reset(VirtIODevice *vdev)
     }
 }
 
-static int virtio_input_load(QEMUFile *f, void *opaque, size_t size)
+static int virtio_input_post_load(void *opaque, int version_id)
 {
     VirtIOInput *vinput = opaque;
     VirtIOInputClass *vic = VIRTIO_INPUT_GET_CLASS(vinput);
     VirtIODevice *vdev = VIRTIO_DEVICE(vinput);
-    int ret;
-
-    ret = virtio_load(vdev, f, VIRTIO_INPUT_VM_VERSION);
-    if (ret) {
-        return ret;
-    }
 
-    /* post_load() */
     vinput->active = vdev->status & VIRTIO_CONFIG_S_DRIVER_OK;
     if (vic->change_active) {
         vic->change_active(vinput);
@@ -296,8 +291,16 @@ static void virtio_input_device_unrealize(DeviceState *dev, Error **errp)
     virtio_cleanup(vdev);
 }
 
-VMSTATE_VIRTIO_DEVICE(input, VIRTIO_INPUT_VM_VERSION, virtio_input_load,
-                      virtio_vmstate_save);
+static const VMStateDescription vmstate_virtio_input = {
+    .name = "virtio-input",
+    .minimum_version_id = VIRTIO_INPUT_VM_VERSION,
+    .version_id = VIRTIO_INPUT_VM_VERSION,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+    .post_load = virtio_input_post_load,
+};
 
 static Property virtio_input_properties[] = {
     DEFINE_PROP_STRING("serial", VirtIOInput, serial),
-- 
MST

[Qemu-devel] [PULL 28/33] virtio-scsi: convert VMSTATE_VIRTIO_DEVICE

[Michael S. Tsirkin]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/scsi/virtio-scsi.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 6eaadd8..9473e10 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -13,6 +13,8 @@
  *
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "standard-headers/linux/virtio_ids.h"
@@ -681,22 +683,6 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
     s->events_dropped = false;
 }
 
-/* The device does not have anything to save beyond the virtio data.
- * Request data is saved with callbacks from SCSI devices.
- */
-static void virtio_scsi_save(QEMUFile *f, void *opaque, size_t size)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
-    virtio_save(vdev, f);
-}
-
-static int virtio_scsi_load(QEMUFile *f, void *opaque, size_t size)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
-
-    return virtio_load(vdev, f, 1);
-}
-
 void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
                             uint32_t event, uint32_t reason)
 {
@@ -940,7 +926,15 @@ static Property virtio_scsi_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-VMSTATE_VIRTIO_DEVICE(scsi, 1, virtio_scsi_load, virtio_scsi_save);
+static const VMStateDescription vmstate_virtio_scsi = {
+    .name = "virtio-scsi",
+    .minimum_version_id = 1,
+    .version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static void virtio_scsi_common_class_init(ObjectClass *klass, void *data)
 {
-- 
MST

[Qemu-devel] [PULL 29/33] virtio-balloon: convert VMSTATE_VIRTIO_DEVICE

[Michael S. Tsirkin]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-balloon.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index eb572e6..2c68d3d 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -13,6 +13,8 @@
  *
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qemu/iov.h"
 #include "qemu/timer.h"
@@ -402,11 +404,6 @@ static void virtio_balloon_save_device(VirtIODevice *vdev, QEMUFile *f)
     qemu_put_be32(f, s->actual);
 }
 
-static int virtio_balloon_load(QEMUFile *f, void *opaque, size_t size)
-{
-    return virtio_load(VIRTIO_DEVICE(opaque), f, 1);
-}
-
 static int virtio_balloon_load_device(VirtIODevice *vdev, QEMUFile *f,
                                       int version_id)
 {
@@ -492,7 +489,15 @@ static void virtio_balloon_instance_init(Object *obj)
                         NULL, s, NULL);
 }
 
-VMSTATE_VIRTIO_DEVICE(balloon, 1, virtio_balloon_load, virtio_vmstate_save);
+static const VMStateDescription vmstate_virtio_balloon = {
+    .name = "virtio-balloon",
+    .minimum_version_id = 1,
+    .version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static Property virtio_balloon_properties[] = {
     DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
-- 
MST

[Qemu-devel] [PULL 30/33] virtio-rng: convert VMSTATE_VIRTIO_DEVICE

[Michael S. Tsirkin]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-rng.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index cd8ca10..62867d1 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -9,6 +9,8 @@
  * top-level directory.
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/iov.h"
@@ -120,15 +122,9 @@ static uint64_t get_features(VirtIODevice *vdev, uint64_t f, Error **errp)
     return f;
 }
 
-static int virtio_rng_load(QEMUFile *f, void *opaque, size_t size)
+static int virtio_rng_post_load(void *opaque, int version_id)
 {
     VirtIORNG *vrng = opaque;
-    int ret;
-
-    ret = virtio_load(VIRTIO_DEVICE(vrng), f, 1);
-    if (ret != 0) {
-        return ret;
-    }
 
     /* We may have an element ready but couldn't process it due to a quota
      * limit.  Make sure to try again after live migration when the quota may
@@ -216,7 +212,16 @@ static void virtio_rng_device_unrealize(DeviceState *dev, Error **errp)
     virtio_cleanup(vdev);
 }
 
-VMSTATE_VIRTIO_DEVICE(rng, 1, virtio_rng_load, virtio_vmstate_save);
+static const VMStateDescription vmstate_virtio_rng = {
+    .name = "virtio-rng",
+    .minimum_version_id = 1,
+    .version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+    .post_load =  virtio_rng_post_load,
+};
 
 static Property virtio_rng_properties[] = {
     /* Set a default rate limit of 2^47 bytes per minute or roughly 2TB/s.  If
-- 
MST

[Qemu-devel] [PULL 31/33] vhost-vsock: convert VMSTATE_VIRTIO_DEVICE

[Michael S. Tsirkin]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/vhost-vsock.c | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index bde2456..99cb216 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -11,6 +11,8 @@
  * top-level directory.
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include <sys/ioctl.h>
 #include "qemu/osdep.h"
 #include "standard-headers/linux/virtio_vsock.h"
@@ -236,17 +238,6 @@ out:
     g_free(elem);
 }
 
-static void vhost_vsock_save(QEMUFile *f, void *opaque, size_t size)
-{
-    VHostVSock *vsock = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(vsock);
-
-    /* At this point, backend must be stopped, otherwise
-     * it might keep writing to memory. */
-    assert(!vsock->vhost_dev.started);
-    virtio_save(vdev, f);
-}
-
 static void vhost_vsock_post_load_timer_cleanup(VHostVSock *vsock)
 {
     if (!vsock->post_load_timer) {
@@ -266,16 +257,19 @@ static void vhost_vsock_post_load_timer_cb(void *opaque)
     vhost_vsock_send_transport_reset(vsock);
 }
 
-static int vhost_vsock_load(QEMUFile *f, void *opaque, size_t size)
+static void vhost_vsock_pre_save(void *opaque)
 {
     VHostVSock *vsock = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(vsock);
-    int ret;
 
-    ret = virtio_load(vdev, f, VHOST_VSOCK_SAVEVM_VERSION);
-    if (ret) {
-        return ret;
-    }
+    /* At this point, backend must be stopped, otherwise
+     * it might keep writing to memory. */
+    assert(!vsock->vhost_dev.started);
+}
+
+static int vhost_vsock_post_load(void *opaque, int version_id)
+{
+    VHostVSock *vsock = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(vsock);
 
     if (virtio_queue_get_addr(vdev, 2)) {
         /* Defer transport reset event to a vm clock timer so that virtqueue
@@ -288,12 +282,20 @@ static int vhost_vsock_load(QEMUFile *f, void *opaque, size_t size)
                          vsock);
         timer_mod(vsock->post_load_timer, 1);
     }
-
     return 0;
 }
 
-VMSTATE_VIRTIO_DEVICE(vhost_vsock, VHOST_VSOCK_SAVEVM_VERSION,
-                      vhost_vsock_load, vhost_vsock_save);
+static const VMStateDescription vmstate_virtio_vhost_vsock = {
+    .name = "virtio-vhost_vsock",
+    .minimum_version_id = VHOST_VSOCK_SAVEVM_VERSION,
+    .version_id = VHOST_VSOCK_SAVEVM_VERSION,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+    .pre_save = vhost_vsock_pre_save,
+    .post_load = vhost_vsock_post_load,
+};
 
 static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
 {
-- 
MST

[Qemu-devel] [PULL 32/33] virtio: cleanup VMSTATE_VIRTIO_DEVICE

[Michael S. Tsirkin]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Now all the usages of the old version of VMSTATE_VIRTIO_DEVICE are gone,
so we can get rid of the conditionals, and the old macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/virtio.h  | 27 ---------------------------
 hw/9pfs/virtio-9p-device.c  |  2 --
 hw/block/virtio-blk.c       |  2 --
 hw/char/virtio-serial-bus.c |  2 --
 hw/display/virtio-gpu.c     |  2 --
 hw/input/virtio-input.c     |  2 --
 hw/net/virtio-net.c         |  2 --
 hw/scsi/virtio-scsi.c       |  2 --
 hw/virtio/vhost-vsock.c     |  2 --
 hw/virtio/virtio-balloon.c  |  2 --
 hw/virtio/virtio-rng.c      |  2 --
 hw/virtio/virtio.c          |  6 ------
 12 files changed, 53 deletions(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 929fa92..b913aac 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -177,12 +177,9 @@ bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq);
 void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
 
 void virtio_save(VirtIODevice *vdev, QEMUFile *f);
-void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size);
 
 extern const VMStateInfo virtio_vmstate_info;
 
-#ifdef VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #define VMSTATE_VIRTIO_DEVICE \
     {                                         \
         .name = "virtio",                     \
@@ -190,30 +187,6 @@ extern const VMStateInfo virtio_vmstate_info;
         .flags = VMS_SINGLE,                  \
     }
 
-#else
-/* TODO remove conditional as soon as all users are converted */
-
-#define VMSTATE_VIRTIO_DEVICE(devname, v, getf, putf) \
-    static const VMStateDescription vmstate_virtio_ ## devname = { \
-        .name = "virtio-" #devname ,          \
-        .minimum_version_id = v,              \
-        .version_id = v,                      \
-        .fields = (VMStateField[]) {          \
-            {                                 \
-                .name = "virtio",             \
-                .info = &(const VMStateInfo) {\
-                        .name = "virtio",     \
-                        .get = getf,          \
-                        .put = putf,          \
-                    },                        \
-                .flags = VMS_SINGLE,          \
-            },                                \
-            VMSTATE_END_OF_LIST()             \
-        }                                     \
-    }
-
-#endif
-
 int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id);
 
 void virtio_notify_config(VirtIODevice *vdev);
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 526ec7d..e98dd0c 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -11,8 +11,6 @@
  *
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "hw/virtio/virtio.h"
 #include "qemu/sockets.h"
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 10c5794..37fe72b 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -11,8 +11,6 @@
  *
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index c9b0fc8..7975c2c 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -18,8 +18,6 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/iov.h"
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 4fcd63c..fa6fd0e 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -11,8 +11,6 @@
  * See the COPYING file in the top-level directory.
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/iov.h"
diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index 5e31033..b678ee9 100644
--- a/hw/input/virtio-input.c
+++ b/hw/input/virtio-input.c
@@ -4,8 +4,6 @@
  * top-level directory.
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/iov.h"
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index b2198a5..06bfe4b 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -11,8 +11,6 @@
  *
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qemu/iov.h"
 #include "hw/virtio/virtio.h"
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 9473e10..4762f05 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -13,8 +13,6 @@
  *
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "standard-headers/linux/virtio_ids.h"
diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index 99cb216..b481562 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -11,8 +11,6 @@
  * top-level directory.
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include <sys/ioctl.h>
 #include "qemu/osdep.h"
 #include "standard-headers/linux/virtio_vsock.h"
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 2c68d3d..1d77028 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -13,8 +13,6 @@
  *
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qemu/iov.h"
 #include "qemu/timer.h"
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 62867d1..9639f4e 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -9,8 +9,6 @@
  * top-level directory.
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/iov.h"
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 62b9c00..d48d1a9 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1640,12 +1640,6 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
 }
 
 /* A wrapper for use as a VMState .put function */
-void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size)
-{
-    virtio_save(VIRTIO_DEVICE(opaque), f);
-}
-
-/* A wrapper for use as a VMState .put function */
 static void virtio_device_put(QEMUFile *f, void *opaque, size_t size)
 {
     virtio_save(VIRTIO_DEVICE(opaque), f);
-- 
MST

[Qemu-devel] [PULL 33/33] intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE

[Michael S. Tsirkin]

From: Feng Wu <feng.wu@intel.com>

The Trigger Mode field of IOAPIC must match the Trigger Mode in
the IRTE according to VT-d Spec 5.1.5.1.

Signed-off-by: Feng Wu <feng.wu@intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/intel_iommu.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 9f4e64a..2efd69b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -27,6 +27,7 @@
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_bus.h"
 #include "hw/i386/pc.h"
+#include "hw/i386/apic-msidef.h"
 #include "hw/boards.h"
 #include "hw/i386/x86-iommu.h"
 #include "hw/pci-host/q35.h"
@@ -2209,6 +2210,8 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
         }
     } else {
         uint8_t vector = origin->data & 0xff;
+        uint8_t trigger_mode = (origin->data >> MSI_DATA_TRIGGER_SHIFT) & 0x1;
+
         VTD_DPRINTF(IR, "received IOAPIC interrupt");
         /* IOAPIC entry vector should be aligned with IRTE vector
          * (see vt-d spec 5.1.5.1). */
@@ -2217,6 +2220,15 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
                         "entry: %d, IRTE: %d, index: %d",
                         vector, irq.vector, index);
         }
+
+        /* The Trigger Mode field must match the Trigger Mode in the IRTE.
+         * (see vt-d spec 5.1.5.1). */
+        if (trigger_mode != irq.trigger_mode) {
+            VTD_DPRINTF(GENERAL, "IOAPIC trigger mode inconsistent: "
+                        "entry: %u, IRTE: %u, index: %d",
+                        trigger_mode, irq.trigger_mode, index);
+        }
+
     }
 
     /*
-- 
MST

Re: [Qemu-devel] [PULL 00/33] virtio, pc: fixes and features

[Peter Maydell]

On 10 October 2016 at 03:57, Michael S. Tsirkin <mst@redhat.com> wrote:
> The following changes since commit 48f592118ab42f83a1a7561c4bfd2b72a100f241:
>
>   bsd-user: fix FreeBSD build after d148d90e (2016-10-07 15:17:53 +0100)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to dea651a95af6dad0997b840241a0bf6059d9a776:
>
>   intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE (2016-10-10 02:38:14 +0300)
>
> ----------------------------------------------------------------
> virtio, pc: fixes and features
>
> more guest error handling for virtio devices
> virtio migration rework
> pc fixes
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

This failed 'make check' on aarch64 host (everything else was ok):

TEST: tests/pxe-test... (pid=11699)
  /ppc64/pxe/virtio:                                                   **
ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
assertion failed (signature == SIGN
ATURE): (0x00002020 == 0x0000dead)
FAIL
GTester: last random seed: R02S87a02de849c98998299177b1a4c7a31b
(pid=19477)
  /ppc64/pxe/spapr-vlan:                                               **
ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
assertion failed (signature == SIGN
ATURE): (0x00002020 == 0x0000dead)
FAIL
GTester: last random seed: R02Sf9cf55ad239a137dd20d3085abf91524
(pid=24055)
FAIL: tests/pxe-test

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/33] virtio, pc: fixes and features

[Peter Maydell]

On 10 October 2016 at 15:13, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 10 October 2016 at 03:57, Michael S. Tsirkin <mst@redhat.com> wrote:
>> The following changes since commit 48f592118ab42f83a1a7561c4bfd2b72a100f241:
>>
>>   bsd-user: fix FreeBSD build after d148d90e (2016-10-07 15:17:53 +0100)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>>
>> for you to fetch changes up to dea651a95af6dad0997b840241a0bf6059d9a776:
>>
>>   intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE (2016-10-10 02:38:14 +0300)
>>
>> ----------------------------------------------------------------
>> virtio, pc: fixes and features
>>
>> more guest error handling for virtio devices
>> virtio migration rework
>> pc fixes
>>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> This failed 'make check' on aarch64 host (everything else was ok):
>
> TEST: tests/pxe-test... (pid=11699)
>   /ppc64/pxe/virtio:                                                   **
> ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
> assertion failed (signature == SIGN
> ATURE): (0x00002020 == 0x0000dead)
> FAIL
> GTester: last random seed: R02S87a02de849c98998299177b1a4c7a31b
> (pid=19477)
>   /ppc64/pxe/spapr-vlan:                                               **
> ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
> assertion failed (signature == SIGN
> ATURE): (0x00002020 == 0x0000dead)
> FAIL
> GTester: last random seed: R02Sf9cf55ad239a137dd20d3085abf91524
> (pid=24055)
> FAIL: tests/pxe-test

Several subsequent test runs passed, so I'm inclined to suspect
this is not related to the pull request but is actually an
over-enthusiastic timeout and the build machine was heavily
loaded or something.

Anyway, I've pushed the merge to master.

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/33] virtio, pc: fixes and features

[Michael S. Tsirkin]

On Mon, Oct 10, 2016 at 07:32:32PM +0100, Peter Maydell wrote:
> On 10 October 2016 at 15:13, Peter Maydell <peter.maydell@linaro.org> wrote:
> > On 10 October 2016 at 03:57, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> The following changes since commit 48f592118ab42f83a1a7561c4bfd2b72a100f241:
> >>
> >>   bsd-user: fix FreeBSD build after d148d90e (2016-10-07 15:17:53 +0100)
> >>
> >> are available in the git repository at:
> >>
> >>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >>
> >> for you to fetch changes up to dea651a95af6dad0997b840241a0bf6059d9a776:
> >>
> >>   intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE (2016-10-10 02:38:14 +0300)
> >>
> >> ----------------------------------------------------------------
> >> virtio, pc: fixes and features
> >>
> >> more guest error handling for virtio devices
> >> virtio migration rework
> >> pc fixes
> >>
> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > This failed 'make check' on aarch64 host (everything else was ok):
> >
> > TEST: tests/pxe-test... (pid=11699)
> >   /ppc64/pxe/virtio:                                                   **
> > ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
> > assertion failed (signature == SIGN
> > ATURE): (0x00002020 == 0x0000dead)
> > FAIL
> > GTester: last random seed: R02S87a02de849c98998299177b1a4c7a31b
> > (pid=19477)
> >   /ppc64/pxe/spapr-vlan:                                               **
> > ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
> > assertion failed (signature == SIGN
> > ATURE): (0x00002020 == 0x0000dead)
> > FAIL
> > GTester: last random seed: R02Sf9cf55ad239a137dd20d3085abf91524
> > (pid=24055)
> > FAIL: tests/pxe-test
> 
> Several subsequent test runs passed, so I'm inclined to suspect
> this is not related to the pull request but is actually an
> over-enthusiastic timeout and the build machine was heavily
> loaded or something.
> 
> Anyway, I've pushed the merge to master.
> 
> thanks
> -- PMM


Could be. I think we should have code to detect this though.
How about using e.g. time to report elapsed system and user time?

-- 
MST

Re: [Qemu-devel] [PULL 00/33] virtio, pc: fixes and features

[Sascha Silbe]

[-- Attachment #1: Type: text/plain, Size: 1979 bytes --]

Dear Peter,

Peter Maydell <peter.maydell@linaro.org> writes:

> On 10 October 2016 at 15:13, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This failed 'make check' on aarch64 host (everything else was ok):
>>
>> TEST: tests/pxe-test... (pid=11699)
>>   /ppc64/pxe/virtio:                                                   **
>> ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
>> assertion failed (signature == SIGN
>> ATURE): (0x00002020 == 0x0000dead)
>> FAIL
>> GTester: last random seed: R02S87a02de849c98998299177b1a4c7a31b
>> (pid=19477)
>>   /ppc64/pxe/spapr-vlan:                                               **
>> ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
>> assertion failed (signature == SIGN
>> ATURE): (0x00002020 == 0x0000dead)
>> FAIL
>> GTester: last random seed: R02Sf9cf55ad239a137dd20d3085abf91524
>> (pid=24055)
>> FAIL: tests/pxe-test
>
> Several subsequent test runs passed, so I'm inclined to suspect
> this is not related to the pull request but is actually an
> over-enthusiastic timeout and the build machine was heavily
> loaded or something.

There's a race condition in the tests. Both the "i386" and the "x86_64"
set of tests are creating and removing the same set of files:
tests/acpi-test-disk.raw (hard-coded in tests/bios-tables-test.c) and
tests/pxe-test-disk.raw (hard-coded in tests/pxe-test.c).

FWIW, it's not obvious at first sight that the tests added to
check-qtest-i386-y will also be included in check-qtest-x86_64-y. The
line responsible for that is hiding in the midst of other assignments,
without even a blank line separating it from the rest.

Assigning to a common variable first and then including it in both
check-qtest-i386-y and check-qtest-x86_64-y would make it a lot easier
to understand IMO.

Sascha
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr.: DE281696641

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [Qemu-devel] [PULL 00/33] virtio, pc: fixes and features

[Peter Maydell]

On 11 October 2016 at 09:27, Sascha Silbe <x-qemu@se-silbe.de> wrote:
> There's a race condition in the tests. Both the "i386" and the "x86_64"
> set of tests are creating and removing the same set of files:
> tests/acpi-test-disk.raw (hard-coded in tests/bios-tables-test.c) and
> tests/pxe-test-disk.raw (hard-coded in tests/pxe-test.c).

Ooh, thanks for tracking that down. I was really not looking forward
to trying to figure out why this wasn't passing...

-- PMM

Re: [Qemu-devel] [PULL 00/33] virtio, pc: fixes and features

[Thomas Huth]

[-- Attachment #1: Type: text/plain, Size: 2445 bytes --]

On 11.10.2016 10:27, Sascha Silbe wrote:
> Dear Peter,
> 
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
>> On 10 October 2016 at 15:13, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> This failed 'make check' on aarch64 host (everything else was ok):
>>>
>>> TEST: tests/pxe-test... (pid=11699)
>>>   /ppc64/pxe/virtio:                                                   **
>>> ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
>>> assertion failed (signature == SIGN
>>> ATURE): (0x00002020 == 0x0000dead)
>>> FAIL
>>> GTester: last random seed: R02S87a02de849c98998299177b1a4c7a31b
>>> (pid=19477)
>>>   /ppc64/pxe/spapr-vlan:                                               **
>>> ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
>>> assertion failed (signature == SIGN
>>> ATURE): (0x00002020 == 0x0000dead)
>>> FAIL
>>> GTester: last random seed: R02Sf9cf55ad239a137dd20d3085abf91524
>>> (pid=24055)
>>> FAIL: tests/pxe-test
>>
>> Several subsequent test runs passed, so I'm inclined to suspect
>> this is not related to the pull request but is actually an
>> over-enthusiastic timeout and the build machine was heavily
>> loaded or something.
> 
> There's a race condition in the tests. Both the "i386" and the "x86_64"
> set of tests are creating and removing the same set of files:
> tests/acpi-test-disk.raw (hard-coded in tests/bios-tables-test.c) and
> tests/pxe-test-disk.raw (hard-coded in tests/pxe-test.c).

Hmm, it's likely even worse - the new ppc64 test is using the same file
name, too - with different content in the file. So if the tests are
running in parallel, they will likely disturb each other.

I think we should use mkstemp() instead for creating a unique file. Is
anybody working on a patch already? If not, I could have a look...

> FWIW, it's not obvious at first sight that the tests added to
> check-qtest-i386-y will also be included in check-qtest-x86_64-y. The
> line responsible for that is hiding in the midst of other assignments,
> without even a blank line separating it from the rest.
> 
> Assigning to a common variable first and then including it in both
> check-qtest-i386-y and check-qtest-x86_64-y would make it a lot easier
> to understand IMO.

Could you maybe provide a patch for this, Sascha? (at least for adding
some blank lines inbetween - currently it is really hard to read)

 Thomas



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PULL 00/33] virtio, pc: fixes and features

[Sascha Silbe]

[-- Attachment #1: Type: text/plain, Size: 1960 bytes --]

Dear Thomas,

Thomas Huth <thuth@redhat.com> writes:

> On 11.10.2016 10:27, Sascha Silbe wrote:
[pxe-test failing during "make check"]
>> There's a race condition in the tests. Both the "i386" and the "x86_64"
>> set of tests are creating and removing the same set of files:
>> tests/acpi-test-disk.raw (hard-coded in tests/bios-tables-test.c) and
>> tests/pxe-test-disk.raw (hard-coded in tests/pxe-test.c).
>
> Hmm, it's likely even worse - the new ppc64 test is using the same file
> name, too - with different content in the file. So if the tests are
> running in parallel, they will likely disturb each other.

Ouch.

> I think we should use mkstemp() instead for creating a unique file. Is
> anybody working on a patch already? If not, I could have a look...

Feel free to go ahead. In the long run we'll probably want a more
systematic approach; I've fixed several instances in the past (see
50455700, 5f1525a6, 6bb6f6cd, 0145b4e1). But that shouldn't prevent us
From fixing a known issue in the meantime.


>> FWIW, it's not obvious at first sight that the tests added to
>> check-qtest-i386-y will also be included in check-qtest-x86_64-y. The
>> line responsible for that is hiding in the midst of other assignments,
>> without even a blank line separating it from the rest.
>> 
>> Assigning to a common variable first and then including it in both
>> check-qtest-i386-y and check-qtest-x86_64-y would make it a lot easier
>> to understand IMO.
>
> Could you maybe provide a patch for this, Sascha? (at least for adding
> some blank lines inbetween - currently it is really hard to read)

I'll try to whip up a patch in the next few days. Unless someone has a
strong preference to stay with the current scheme, I'll implement the
common variable approach I outlined above.

Sascha
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr.: DE281696641

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]