Re: [Qemu-devel] [PATCH] usb: ohci: limit the number of link eds

[Li Qiang]

Hello Gerd,

Ping...

2017-02-07 18:23 GMT+08:00 Li Qiang :

> From: Li Qiang 
>
> The guest may builds an infinite loop with link eds. This patch
> limit the number of linked ed to avoid this.
>
> Signed-off-by: Li Qiang 
> ---
>  hw/usb/hcd-ohci.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index c82a92f..4a63f3b 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -42,6 +42,8 @@
>
>  #define OHCI_MAX_PORTS 15
>
> +#define ED_LINK_LIMIT 4
> +
>  static int64_t usb_frame_time;
>  static int64_t usb_bit_time;
>
> @@ -1184,7 +1186,7 @@ static int ohci_service_ed_list(OHCIState *ohci,
> uint32_t head, int completion)
>  uint32_t next_ed;
>  uint32_t cur;
>  int active;
> -
> +uint32_t link_cnt = 0;
>  active = 0;
>
>  if (head == 0)
> @@ -1199,6 +1201,11 @@ static int ohci_service_ed_list(OHCIState *ohci,
> uint32_t head, int completion)
>
>  next_ed = ed.next & OHCI_DPTR_MASK;
>
> +if (++link_cnt > ED_LINK_LIMIT) {
> +ohci_die(ohci);
> +return 0;
> +}
> +
>  if ((ed.head & OHCI_ED_H) || (ed.flags & OHCI_ED_K)) {
>  uint32_t addr;
>  /* Cancel pending packets for ED that have been paused.  */
> --
> 1.8.3.1
>
>

Re: [Qemu-devel] [PATCH] usb: ohci: fix error return code in servicing iso td

[Li Qiang]

Ping...

2017-02-07 19:15 GMT+08:00 Li Qiang :

> From: Li Qiang 
>
> It should return 1 if an error occurs when reading iso td.
> This will avoid an infinite loop issue in ohci_service_ed_list.
>
> Signed-off-by: Li Qiang 
> ---
>  hw/usb/hcd-ohci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index 4a63f3b..21c93e0 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -727,7 +727,7 @@ static int ohci_service_iso_td(OHCIState *ohci, struct
> ohci_ed *ed,
>  if (ohci_read_iso_td(ohci, addr, _td)) {
>  trace_usb_ohci_iso_td_read_failed(addr);
>  ohci_die(ohci);
> -return 0;
> +return 1;
>  }
>
>  starting_frame = OHCI_BM(iso_td.flags, TD_SF);
> --
> 1.8.3.1
>
>

[Qemu-devel] [PATCH v6 1/7] linker-loader: Add new 'write pointer' command

[ben]

From: Ben Warren 

This is similar to the existing 'add pointer' functionality, but instead
of instructing the guest (BIOS or UEFI) to patch memory, it instructs
the guest to write the pointer back to QEMU via a writeable fw_cfg file.

Signed-off-by: Ben Warren 
---
 hw/acpi/bios-linker-loader.c | 58 ++--
 include/hw/acpi/bios-linker-loader.h |  6 
 2 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index d963ebe..5030cf1 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -78,6 +78,19 @@ struct BiosLinkerLoaderEntry {
 uint32_t length;
 } cksum;
 
+/*
+ * COMMAND_WRITE_POINTER - write the fw_cfg file (originating from
+ * @dest_file) at @wr_pointer.offset, by adding a pointer to the table
+ * originating from @src_file. 1,2,4 or 8 byte unsigned
+ * addition is used depending on @wr_pointer.size.
+ */
+struct {
+char dest_file[BIOS_LINKER_LOADER_FILESZ];
+char src_file[BIOS_LINKER_LOADER_FILESZ];
+uint32_t offset;
+uint8_t size;
+} wr_pointer;
+
 /* padding */
 char pad[124];
 };
@@ -85,9 +98,10 @@ struct BiosLinkerLoaderEntry {
 typedef struct BiosLinkerLoaderEntry BiosLinkerLoaderEntry;
 
 enum {
-BIOS_LINKER_LOADER_COMMAND_ALLOCATE = 0x1,
-BIOS_LINKER_LOADER_COMMAND_ADD_POINTER  = 0x2,
-BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3,
+BIOS_LINKER_LOADER_COMMAND_ALLOCATE  = 0x1,
+BIOS_LINKER_LOADER_COMMAND_ADD_POINTER   = 0x2,
+BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM  = 0x3,
+BIOS_LINKER_LOADER_COMMAND_WRITE_POINTER = 0x4,
 };
 
 enum {
@@ -278,3 +292,41 @@ void bios_linker_loader_add_pointer(BIOSLinker *linker,
 
 g_array_append_vals(linker->cmd_blob, , sizeof entry);
 }
+
+/*
+ * bios_linker_loader_write_pointer: ask guest to write a pointer to the
+ * source file into the destination file, and write it back to QEMU via
+ * fw_cfg DMA.
+ *
+ * @linker: linker object instance
+ * @dest_file: destination file that must be written
+ * @dst_patched_offset: location within destination file blob to be patched
+ *  with the pointer to @src_file, in bytes
+ * @dst_patched_offset_size: size of the pointer to be patched
+ *  at @dst_patched_offset in @dest_file blob, in bytes
+ * @src_file: source file who's address must be taken
+ */
+void bios_linker_loader_write_pointer(BIOSLinker *linker,
+const char *dest_file,
+uint32_t dst_patched_offset,
+uint8_t dst_patched_size,
+const char *src_file)
+{
+BiosLinkerLoaderEntry entry;
+const BiosLinkerFileEntry *source_file =
+bios_linker_find_file(linker, src_file);
+
+assert(source_file);
+memset(, 0, sizeof entry);
+strncpy(entry.wr_pointer.dest_file, dest_file,
+sizeof entry.wr_pointer.dest_file - 1);
+strncpy(entry.wr_pointer.src_file, src_file,
+sizeof entry.wr_pointer.src_file - 1);
+entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_WRITE_POINTER);
+entry.wr_pointer.offset = cpu_to_le32(dst_patched_offset);
+entry.wr_pointer.size = dst_patched_size;
+assert(dst_patched_size == 1 || dst_patched_size == 2 ||
+   dst_patched_size == 4 || dst_patched_size == 8);
+
+g_array_append_vals(linker->cmd_blob, , sizeof entry);
+}
diff --git a/include/hw/acpi/bios-linker-loader.h 
b/include/hw/acpi/bios-linker-loader.h
index fa1e5d1..f9ba5d6 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -26,5 +26,11 @@ void bios_linker_loader_add_pointer(BIOSLinker *linker,
 const char *src_file,
 uint32_t src_offset);
 
+void bios_linker_loader_write_pointer(BIOSLinker *linker,
+  const char *dest_file,
+  uint32_t dst_patched_offset,
+  uint8_t dst_patched_size,
+  const char *src_file);
+
 void bios_linker_loader_cleanup(BIOSLinker *linker);
 #endif
-- 
2.7.4

[Qemu-devel] [PATCH v6 4/7] ACPI: Add Virtual Machine Generation ID support

[ben]

From: Ben Warren 

This implements the VM Generation ID feature by passing a 128-bit
GUID to the guest via a fw_cfg blob.
Any time the GUID changes, an ACPI notify event is sent to the guest

The user interface is a simple device with one parameter:
 - guid (string, must be "auto" or in UUID format
   ----)

Signed-off-by: Ben Warren 
---
 default-configs/i386-softmmu.mak |   1 +
 default-configs/x86_64-softmmu.mak   |   1 +
 hw/acpi/Makefile.objs|   1 +
 hw/acpi/vmgenid.c| 237 +++
 hw/i386/acpi-build.c |  16 +++
 include/hw/acpi/acpi_dev_interface.h |   1 +
 include/hw/acpi/vmgenid.h|  35 ++
 7 files changed, 292 insertions(+)
 create mode 100644 hw/acpi/vmgenid.c
 create mode 100644 include/hw/acpi/vmgenid.h

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 48b07a4..029e952 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -59,3 +59,4 @@ CONFIG_I82801B11=y
 CONFIG_SMBIOS=y
 CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
 CONFIG_PXB=y
+CONFIG_ACPI_VMGENID=y
diff --git a/default-configs/x86_64-softmmu.mak 
b/default-configs/x86_64-softmmu.mak
index fd96345..d1d7432 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -59,3 +59,4 @@ CONFIG_I82801B11=y
 CONFIG_SMBIOS=y
 CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
 CONFIG_PXB=y
+CONFIG_ACPI_VMGENID=y
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 6acf798..11c35bc 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -5,6 +5,7 @@ common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
 common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
 common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
 common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
+common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
 common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
 
 common-obj-y += acpi_interface.o
diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
new file mode 100644
index 000..b1b7b32
--- /dev/null
+++ b/hw/acpi/vmgenid.c
@@ -0,0 +1,237 @@
+/*
+ *  Virtual Machine Generation ID Device
+ *
+ *  Copyright (C) 2017 Skyport Systems.
+ *
+ *  Author: Ben Warren 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qmp-commands.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/vmgenid.h"
+#include "hw/nvram/fw_cfg.h"
+#include "sysemu/sysemu.h"
+
+void vmgenid_build_acpi(VmGenIdState *vms, GArray *table_data, GArray *guid,
+BIOSLinker *linker)
+{
+Aml *ssdt, *dev, *scope, *method, *addr, *if_ctx;
+uint32_t vgia_offset;
+QemuUUID guid_le;
+
+/* Fill in the GUID values.  These need to be converted to little-endian
+ * first, since that's what the guest expects
+ */
+g_array_set_size(guid, VMGENID_FW_CFG_SIZE);
+memcpy(_le.data, >guid.data, sizeof(vms->guid.data));
+qemu_uuid_bswap(_le);
+/* The GUID is written at a fixed offset into the fw_cfg file
+ * in order to implement the "OVMF SDT Header probe suppressor"
+ * see docs/specs/vmgenid.txt for more details
+ */
+g_array_insert_vals(guid, VMGENID_GUID_OFFSET, guid_le.data,
+ARRAY_SIZE(guid_le.data));
+
+/* Put this in a separate SSDT table */
+ssdt = init_aml_allocator();
+
+/* Reserve space for header */
+acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
+
+/* Storage for the GUID address */
+vgia_offset = table_data->len +
+build_append_named_dword(ssdt->buf, "VGIA");
+scope = aml_scope("\\_SB");
+dev = aml_device("VGEN");
+aml_append(dev, aml_name_decl("_HID", aml_string("QEMUVGID")));
+aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter")));
+aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter")));
+
+/* Simple status method to check that address is linked and non-zero */
+method = aml_method("_STA", 0, AML_NOTSERIALIZED);
+addr = aml_local(0);
+aml_append(method, aml_store(aml_int(0xf), addr));
+if_ctx = aml_if(aml_equal(aml_name("VGIA"), aml_int(0)));
+aml_append(if_ctx, aml_store(aml_int(0), addr));
+aml_append(method, if_ctx);
+aml_append(method, aml_return(addr));
+aml_append(dev, method);
+
+/* the ADDR method returns two 32-bit words representing the lower and
+ * upper halves * of the physical address of the fw_cfg blob
+ * (holding the GUID)
+ */
+method = aml_method("ADDR", 0, AML_NOTSERIALIZED);
+
+addr = aml_local(0);
+aml_append(method, aml_store(aml_package(2), addr));
+
+aml_append(method, aml_store(aml_add(aml_name("VGIA"),
+ 

[Qemu-devel] [PATCH v6 7/7] tests: Add unit tests for the VM Generation ID feature

[ben]

From: Ben Warren 

The following tests are implemented:
* test that a GUID passed in by command line is propagated to the guest.
* test that changing the GUID at runtime via the monitor is reflected in
  the guest.
* test that the "auto" argument to the GUID generates a different, and
  correct GUID as seen by the guest.

  This patch is loosely based on a previous patch from:
  Gal Hammer   and Igor Mammedov 

Signed-off-by: Ben Warren 
---
 tests/Makefile.include |   2 +
 tests/vmgenid-test.c   | 195 +
 2 files changed, 197 insertions(+)
 create mode 100644 tests/vmgenid-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 634394a..ca4b3f7 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -241,6 +241,7 @@ check-qtest-i386-y += tests/usb-hcd-xhci-test$(EXESUF)
 gcov-files-i386-y += hw/usb/hcd-xhci.c
 check-qtest-i386-y += tests/pc-cpu-test$(EXESUF)
 check-qtest-i386-y += tests/q35-test$(EXESUF)
+check-qtest-i386-y += tests/vmgenid-test$(EXESUF)
 gcov-files-i386-y += hw/pci-host/q35.c
 check-qtest-i386-$(CONFIG_VHOST_NET_TEST_i386) += 
tests/vhost-user-test$(EXESUF)
 ifeq ($(CONFIG_VHOST_NET_TEST_i386),)
@@ -726,6 +727,7 @@ tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o 
contrib/ivshmem-server/ivshmem
 tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o 
contrib/libvhost-user/libvhost-user.o $(test-util-obj-y)
 tests/test-uuid$(EXESUF): tests/test-uuid.o $(test-util-obj-y)
 tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
+tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< 
,"LINK","$(TARGET_DIR)$@")
diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
new file mode 100644
index 000..721ba05
--- /dev/null
+++ b/tests/vmgenid-test.c
@@ -0,0 +1,195 @@
+/*
+ * QTest testcase for VM Generation ID
+ *
+ * Copyright (c) 2016 Red Hat, Inc.
+ * Copyright (c) 2017 Skyport Systems
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include 
+#include 
+#include 
+#include "qemu/osdep.h"
+#include "qemu/bitmap.h"
+#include "qemu/uuid.h"
+#include "hw/acpi/acpi-defs.h"
+#include "acpi-utils.h"
+#include "libqtest.h"
+
+#define VGID_GUID "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
+#define VMGENID_GUID_OFFSET  40   /* allow space for
+   * OVMF SDT Header Probe Supressor
+   */
+
+static uint32_t vgia;
+
+typedef struct {
+AcpiTableHeader header;
+gchar name_op;
+gchar vgia[4];
+gchar val_op;
+uint32_t vgia_val;
+} QEMU_PACKED VgidTable;
+
+static uint32_t find_vgia(void)
+{
+uint32_t off;
+AcpiRsdpDescriptor rsdp_table;
+uint32_t rsdt;
+AcpiRsdtDescriptorRev1 rsdt_table;
+int tables_nr;
+uint32_t *tables;
+AcpiTableHeader ssdt_table;
+VgidTable vgid_table;
+int i;
+
+/* First, find the RSDP */
+for (off = 0xf; off < 0x10; off += 0x10) {
+uint8_t sig[] = "RSD PTR ";
+
+for (i = 0; i < sizeof sig - 1; ++i) {
+sig[i] = readb(off + i);
+}
+
+if (!memcmp(sig, "RSD PTR ", sizeof sig)) {
+break;
+}
+}
+g_assert_cmphex(off, <, 0x10);
+
+/* Parse the RSDP header so we can find the RSDT */
+ACPI_READ_FIELD(rsdp_table.signature, off);
+ACPI_ASSERT_CMP64(rsdp_table.signature, "RSD PTR ");
+
+ACPI_READ_FIELD(rsdp_table.checksum, off);
+ACPI_READ_ARRAY(rsdp_table.oem_id, off);
+ACPI_READ_FIELD(rsdp_table.revision, off);
+ACPI_READ_FIELD(rsdp_table.rsdt_physical_address, off);
+
+rsdt = rsdp_table.rsdt_physical_address;
+/* read the header */
+ACPI_READ_TABLE_HEADER(_table, rsdt);
+ACPI_ASSERT_CMP(rsdt_table.signature, "RSDT");
+
+/* compute the table entries in rsdt */
+tables_nr = (rsdt_table.length - sizeof(AcpiRsdtDescriptorRev1)) /
+sizeof(uint32_t);
+g_assert_cmpint(tables_nr, >, 0);
+
+/* get the addresses of the tables pointed by rsdt */
+tables = g_new0(uint32_t, tables_nr);
+ACPI_READ_ARRAY_PTR(tables, tables_nr, rsdt);
+
+for (i = 0; i < tables_nr; i++) {
+ACPI_READ_TABLE_HEADER(_table, tables[i]);
+if (!strncmp((char *)ssdt_table.oem_table_id, "VMGENID", 7)) {
+/* the first entry in the table should be VGIA
+ * That's all we need
+ */
+ACPI_READ_FIELD(vgid_table.name_op, tables[i]);
+g_assert(vgid_table.name_op == 0x08);  /* name */
+ACPI_READ_ARRAY(vgid_table.vgia, tables[i]);
+g_assert(memcmp(vgid_table.vgia, "VGIA", 4) == 0);
+ACPI_READ_FIELD(vgid_table.val_op, tables[i]);
+

[Qemu-devel] [PATCH v6 3/7] ACPI: Add vmgenid blob storage to the build tables

[ben]

From: Ben Warren 

This allows them to be centrally initialized and destroyed

The "AcpiBuildTables.vmgenid" array will be used to construct the
"etc/vmgenid" fw_cfg blob.

Its contents will be linked into fw_cfg after being built on the
pc_machine_done() -> acpi_setup() -> acpi_build() call path, and dropped
without use on the subsequent, guest triggered, acpi_build_update() ->
acpi_build() call path.

Signed-off-by: Ben Warren 
Reviewed-by: Laszlo Ersek 
---
 hw/acpi/aml-build.c | 2 ++
 include/hw/acpi/aml-build.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index b2a1e40..c6f2032 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1559,6 +1559,7 @@ void acpi_build_tables_init(AcpiBuildTables *tables)
 tables->rsdp = g_array_new(false, true /* clear */, 1);
 tables->table_data = g_array_new(false, true /* clear */, 1);
 tables->tcpalog = g_array_new(false, true /* clear */, 1);
+tables->vmgenid = g_array_new(false, true /* clear */, 1);
 tables->linker = bios_linker_loader_init();
 }
 
@@ -1568,6 +1569,7 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, 
bool mfre)
 g_array_free(tables->rsdp, true);
 g_array_free(tables->table_data, true);
 g_array_free(tables->tcpalog, mfre);
+g_array_free(tables->vmgenid, mfre);
 }
 
 /* Build rsdt table */
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 559326c..00c21f1 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -210,6 +210,7 @@ struct AcpiBuildTables {
 GArray *table_data;
 GArray *rsdp;
 GArray *tcpalog;
+GArray *vmgenid;
 BIOSLinker *linker;
 } AcpiBuildTables;
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH v4 2/4] sd: sdhci: check transfer mode register in multi block transfer

[P J P]

  Hello Alistair,

+-- On Tue, 14 Feb 2017, Alistair Francis wrote --+
| On Tue, Feb 14, 2017 at 10:52 AM, P J P  wrote:
| > From: Prasad J Pandit 
| >
| > In the SDHCI protocol, the transfer mode register value
| > is used during multi block transfer to check if block count
| > register is enabled and should be updated. Transfer mode
| > register could be set such that, block count register would
| > not be updated, thus leading to an infinite loop. Add check
| > to avoid it.
| >
| > Reported-by: Wjjzhang 
| > Reported-by: Jiang Xin 
| > Signed-off-by: Prasad J Pandit 
| 
| Didn't I already review this?

Yes, you did. I resent it owing to thread

  -> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg02898.html

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

[Qemu-devel] [PULL 4/5] net: imx: limit buffer descriptor count

[Jason Wang]

From: Prasad J Pandit 

i.MX Fast Ethernet Controller uses buffer descriptors to manage
data flow to/fro receive & transmit queues. While transmitting
packets, it could continue to read buffer descriptors if a buffer
descriptor has length of zero and has crafted values in bd.flags.
Set an upper limit to number of buffer descriptors.

Reported-by: Li Qiang 
Signed-off-by: Prasad J Pandit 
Signed-off-by: Jason Wang 
---
 hw/net/imx_fec.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 50c7564..90e6ee3 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -55,6 +55,8 @@
 } \
 } while (0)
 
+#define IMX_MAX_DESC1024
+
 static const char *imx_default_reg_name(IMXFECState *s, uint32_t index)
 {
 static char tmp[20];
@@ -402,12 +404,12 @@ static void imx_eth_update(IMXFECState *s)
 
 static void imx_fec_do_tx(IMXFECState *s)
 {
-int frame_size = 0;
+int frame_size = 0, descnt = 0;
 uint8_t frame[ENET_MAX_FRAME_SIZE];
 uint8_t *ptr = frame;
 uint32_t addr = s->tx_descriptor;
 
-while (1) {
+while (descnt++ < IMX_MAX_DESC) {
 IMXFECBufDesc bd;
 int len;
 
@@ -453,12 +455,12 @@ static void imx_fec_do_tx(IMXFECState *s)
 
 static void imx_enet_do_tx(IMXFECState *s)
 {
-int frame_size = 0;
+int frame_size = 0, descnt = 0;
 uint8_t frame[ENET_MAX_FRAME_SIZE];
 uint8_t *ptr = frame;
 uint32_t addr = s->tx_descriptor;
 
-while (1) {
+while (descnt++ < IMX_MAX_DESC) {
 IMXENETBufDesc bd;
 int len;
 
-- 
2.7.4

[Qemu-devel] [PULL 0/5] Net patches

[Jason Wang]

The following changes since commit 5dae13cd71f0755a1395b5a4cde635b8a6ee3f58:

  Merge remote-tracking branch 'remotes/rth/tags/pull-or-20170214' into staging 
(2017-02-14 09:55:48 +)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to 4154c7e03fa55b4cf52509a83d50d6c09d743b77:

  net: e1000e: fix an infinite loop issue (2017-02-15 11:18:57 +0800)




Li Qiang (1):
  net: e1000e: fix an infinite loop issue

Paolo Bonzini (1):
  net: e1000e: fix dead code in e1000e_write_packet_to_guest

Prasad J Pandit (1):
  net: imx: limit buffer descriptor count

Thomas Huth (1):
  net: Mark 'vlan' parameter as deprecated

Zhang Chen (1):
  colo-compare: sort TCP packet queue by sequence number

 hw/net/e1000e_core.c |  9 +++--
 hw/net/imx_fec.c | 10 ++
 net/colo-compare.c   | 19 +++
 net/net.c|  6 ++
 4 files changed, 38 insertions(+), 6 deletions(-)

Re: [Qemu-devel] [PATCH] Add PowerPC 32-bit guest memory dump support

[Nawrocki, Michael]

Hi Philippe,

I rebased on the latest master, but the only difference between that patch and 
the emailed patch was the commit hash. I’m using master at 
git://git.qemu.org/qemu.git (latest commit 5dae13c). It seems to cleanly apply 
on my end… is there a different repo I should be working against, or am I 
missing something else?

Thanks

On 2/14/17, 18:18, "Philippe Mathieu-Daudé"  wrote:

Hi Mike,

I failed to apply your patch on master:

error: patch failed: target/ppc/Makefile.objs:1
error: target/ppc/Makefile.objs: patch does not apply
error: patch failed: target/ppc/arch_dump.c:1
error: target/ppc/arch_dump.c: patch does not apply
error: patch failed: target/ppc/cpu.h:1225
error: target/ppc/cpu.h: patch does not apply
error: patch failed: target/ppc/translate_init.c:10478
error: target/ppc/translate_init.c: patch does not apply
Patch failed at 0001 Add PowerPC 32-bit guest memory dump support

Can you rebase/fix?

Thanks

On 02/08/2017 05:39 PM, Nawrocki, Michael wrote:
> This patch extends support for the `dump-guest-memory` command to the 
32-bit PowerPC architecture. It relies on the assumption that a 64-bit guest 
will not dump a 32-bit core file (and vice versa); if this assumption is 
invalid, please let me know.
>
> Signed-off-by: Mike Nawrocki 
> ---
>  target/ppc/Makefile.objs|   4 +-
>  target/ppc/arch_dump.c  | 154 

>  target/ppc/cpu.h|   2 +
>  target/ppc/translate_init.c |   5 +-
>  4 files changed, 91 insertions(+), 74 deletions(-)
>
> diff --git a/target/ppc/Makefile.objs b/target/ppc/Makefile.objs
> index a8c7a30..f50ffac 100644
> --- a/target/ppc/Makefile.objs
> +++ b/target/ppc/Makefile.objs
> @@ -1,8 +1,8 @@
>  obj-y += cpu-models.o
>  obj-y += translate.o
>  ifeq ($(CONFIG_SOFTMMU),y)
> -obj-y += machine.o mmu_helper.o mmu-hash32.o monitor.o
> -obj-$(TARGET_PPC64) += mmu-hash64.o arch_dump.o compat.o
> +obj-y += machine.o mmu_helper.o mmu-hash32.o monitor.o arch_dump.o
> +obj-$(TARGET_PPC64) += mmu-hash64.o compat.o
>  endif
>  obj-$(CONFIG_KVM) += kvm.o
>  obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> index 40282a1..28d9cc7 100644
> --- a/target/ppc/arch_dump.c
> +++ b/target/ppc/arch_dump.c
> @@ -1,5 +1,5 @@
>  /*
> - * writing ELF notes for ppc64 arch
> + * writing ELF notes for ppc{64,} arch
>   *
>   *
>   * Copyright IBM, Corp. 2013
> @@ -19,36 +19,48 @@
>  #include "sysemu/dump.h"
>  #include "sysemu/kvm.h"
>
> -struct PPC64UserRegStruct {
> -uint64_t gpr[32];
> -uint64_t nip;
> -uint64_t msr;
> -uint64_t orig_gpr3;
> -uint64_t ctr;
> -uint64_t link;
> -uint64_t xer;
> -uint64_t ccr;
> -uint64_t softe;
> -uint64_t trap;
> -uint64_t dar;
> -uint64_t dsisr;
> -uint64_t result;
> +#ifdef TARGET_PPC64
> +#define ELFCLASS ELFCLASS64
> +#define cpu_to_dump_reg cpu_to_dump64
> +typedef uint64_t reg_t;
> +typedef Elf64_Nhdr Elf_Nhdr;
> +#else
> +#define ELFCLASS ELFCLASS32
> +#define cpu_to_dump_reg cpu_to_dump32
> +typedef uint32_t reg_t;
> +typedef Elf32_Nhdr Elf_Nhdr;
> +#endif /* TARGET_PPC64 */
> +
> +struct PPCUserRegStruct {
> +reg_t gpr[32];
> +reg_t nip;
> +reg_t msr;
> +reg_t orig_gpr3;
> +reg_t ctr;
> +reg_t link;
> +reg_t xer;
> +reg_t ccr;
> +reg_t softe;
> +reg_t trap;
> +reg_t dar;
> +reg_t dsisr;
> +reg_t result;
>  } QEMU_PACKED;
>
> -struct PPC64ElfPrstatus {
> +struct PPCElfPrstatus {
>  char pad1[112];
> -struct PPC64UserRegStruct pr_reg;
> -uint64_t pad2[4];
> +struct PPCUserRegStruct pr_reg;
> +reg_t pad2[4];
>  } QEMU_PACKED;
>
>
> -struct PPC64ElfFpregset {
> +struct PPCElfFpregset {
>  uint64_t fpr[32];
> -uint64_t fpscr;
> +reg_t fpscr;
>  }  QEMU_PACKED;
>
>
> -struct PPC64ElfVmxregset {
> +struct PPCElfVmxregset {
>  ppc_avr_t avr[32];
>  ppc_avr_t vscr;
>  union {
> @@ -57,26 +69,26 @@ struct PPC64ElfVmxregset {
>  } vrsave;
>  }  QEMU_PACKED;
>
> -struct PPC64ElfVsxregset {
> +struct PPCElfVsxregset {
>  uint64_t vsr[32];
>  }  QEMU_PACKED;
>
> -struct PPC64ElfSperegset {
> +struct PPCElfSperegset {
>  uint32_t evr[32];
>  uint64_t spe_acc;
>  uint32_t spe_fscr;
>  }  QEMU_PACKED;
>
>  typedef struct 

Re: [Qemu-devel] [PATCH] osdep.h: pull in sys/sysmacros.h for major/minor/makedev

[Mike Frysinger]

On 10 Feb 2017 09:04, Peter Maydell wrote:
> I still think that it's a shame that glibc is breaking
> compatibility with where these macros have always been
> kept on every OS that's implemented them back to 2BSD...

i don't think that characterization is accurate.  if it were,
autoconf wouldn't have a AC_HEADER_MAJOR helper.
https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Particular-Headers.html#index-AC_005fHEADER_005fMAJOR-616
-mike


signature.asc
Description: Digital signature

Re: [Qemu-devel] [PATCH 1/2] ppc/xics: remove set_nr_irqs() handler from XICSStateClass

[David Gibson]

On Tue, Feb 14, 2017 at 03:52:09PM +0100, Cédric Le Goater wrote:
> On 02/14/2017 08:04 AM, Cédric Le Goater wrote:
> > On 02/14/2017 06:02 AM, David Gibson wrote:
> >> On Mon, Feb 13, 2017 at 03:09:16PM +0100, Cédric Le Goater wrote:
> >>> Today, the ICS (Interrupt Controller Source) object is created and
> >>> realized by the init and realize routines of the XICS object, but some
> >>> of the parameters are only known at the machine level.
> >>>
> >>> These parameters are passed from the sPAPR machine to the ICS object
> >>> in a rather convoluted way using property handlers and a class handler
> >>> of the XICS object. The number of irqs required to allocate the IRQ
> >>> state objects in the ICS realize routine is one of them.
> >>>
> >>> Let's simplify the process by creating the ICS object along with the
> >>> XICS object at the machine level and link the ICS into the XICS list
> >>> of ICSs at this level also. In the sPAPR machine, there is only a
> >>> single ICS but that will change with the PowerNV machine.
> >>>
> >>> Also, QOMify the creation of the objects and get rid of the
> >>> superfluous code.
> >>>
> >>> Signed-off-by: Cédric Le Goater 
> >>
> >> I like the basic idea here: while the ics and icp objects are pretty
> >> straightforward, the "xics" object has always been a bit of a hack,
> >> with logic that really belongs in the machine.
> >>
> >> But.. I don't think the approach here really works.  Specifically..
> >>
> >> [snip]
> >>> -static XICSState *try_create_xics(const char *type, int nr_servers,
> >>> -  int nr_irqs, Error **errp)
> >>> -{
> >>> -Error *err = NULL;
> >>> -DeviceState *dev;
> >>> +static XICSState *try_create_xics(const char *type, const char *type_ics,
> >>> +  int nr_servers, int nr_irqs, Error 
> >>> **errp)
> >>> +{
> >>> +Error *err = NULL, *local_err = NULL;
> >>> +XICSState *xics;
> >>> +ICSState *ics = NULL;
> >>> +
> >>> +xics = XICS_COMMON(object_new(type));
> >>> +qdev_set_parent_bus(DEVICE(xics), sysbus_get_default());
> >>> +object_property_set_int(OBJECT(xics), nr_servers, "nr_servers", 
> >>> );
> >>> +object_property_set_bool(OBJECT(xics), true, "realized", _err);
> >>> +error_propagate(, local_err);
> >>> +if (err) {
> >>> +goto error;
> >>> +}
> >>>  
> >>> -dev = qdev_create(NULL, type);
> >>> -qdev_prop_set_uint32(dev, "nr_servers", nr_servers);
> >>> -qdev_prop_set_uint32(dev, "nr_irqs", nr_irqs);
> >>> -object_property_set_bool(OBJECT(dev), true, "realized", );
> >>> +ics = ICS_SIMPLE(object_new(type_ics));
> >>> +object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL);
> >>> +object_property_set_int(OBJECT(ics), nr_irqs, "nr-irqs", );
> >>> +object_property_set_bool(OBJECT(ics), true, "realized", _err);
> >>> +error_propagate(, local_err);
> >>>  if (err) {
> >>> -error_propagate(errp, err);
> >>> -object_unparent(OBJECT(dev));
> >>> -return NULL;
> >>> +goto error;
> >>> +}
> >>> +
> >>> +ics->xics = xics;
> >>> +QLIST_INSERT_HEAD(>ics, ics, list);
> >>
> >> Poking into the ics and xics objects directly from the machine here
> >> violates abstraction even worse than the existing xics device does.
> >> In fact, avoiding that is basically why the xics device exists in the
> >> first place.
> > 
> > Well, currently, xics_set_nr_servers() also does a :
> > 
> > icp->xics = xics;
> > 
> > So, I think we can live with it until we move the ICS and ICP objects 
> > out of XICS ?
> > 
> > if this is the only worrisome problem in the patch, I can start the
> > series with a QOM Interface handler implemented at the machine level, 
> > something like : 
> > 
> > void (*ics_insert)(ICSState *ics);
> > 
> > doing the insert above to hide the temporary hideousness. Is that ok ? 
> 
> After some thought, I don't think this one is important. At the 
> machine level, it seems OK to me to insert an ICS in the XICS list. 
> I agree that XICS should disappear, but it is still there for the 
> moment. 
> 
> > And, as you propose below, I think we can add the 'xics' link property 
> > right now to get rid of :
> > 
> > ic[ps]->xics = xics;
> 
> I have a v2 ready adding the 'xics' link property but keeping the 
> QLIST_INSERT_HEAD() call in try_create_xics(). Do you think it is 
> worth sending as an initial cleanup :
> 
>  5 files changed, 96 insertions(+), 225 deletions(-)
> 
> or do you want the full picture to be addressed ? 

I don't think I'll want to merge until the full picture is addressed.
However, I'm fine with posting incomplete versions for earlier review
- just label them RFC and note in the 0/N comment that there's more
work to be done to complete the picture.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_

Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

[David Gibson]

On Tue, Feb 14, 2017 at 02:53:08PM +0200, Marcel Apfelbaum wrote:
> On 02/14/2017 06:15 AM, David Gibson wrote:
> > On Mon, Feb 13, 2017 at 12:14:23PM +0200, Marcel Apfelbaum wrote:
> > > On 02/13/2017 06:33 AM, David Gibson wrote:
> > > > On Sun, Feb 12, 2017 at 09:05:46PM +0200, Marcel Apfelbaum wrote:
> > > > > On 02/10/2017 02:37 AM, David Gibson wrote:
> > > > > > On Thu, Feb 09, 2017 at 10:04:47AM +0100, Laszlo Ersek wrote:
> > > > > > > On 02/09/17 05:16, David Gibson wrote:
> > > > > > > > On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek wrote:
> > > > > > > > > On 02/08/17 07:16, David Gibson wrote:
> > > > > > > > > > Marcel,
> > > > > > > > > > 
> > > > > > > > > > Your original patch adding PCIe support to virtio-pci.c has 
> > > > > > > > > > the
> > > > > > > > > > limitation noted below that PCIe won't be enabled if the 
> > > > > > > > > > device is on
> > > > > > > > > > the root bus (rather than under a root or downstream port). 
> > > > > > > > > >  As
> > > > > > > > > > reasoned below, I think removing the check is correct, even 
> > > > > > > > > > for x86
> > > > > > > > > > (though it would rarely be useful there).  But I could well 
> > > > > > > > > > have
> > > > > > > > > > missed something.  Let me know if so...
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > Virtio devices can appear as either vanilla PCI or 
> > > > > > > > > > PCI-Express devices
> > > > > > > > > > depending on the bus they're connected to.  At the moment 
> > > > > > > > > > it will only
> > > > > > > > > > appear as vanilla PCI if connected to the root bus of a 
> > > > > > > > > > PCIe host bridge.
> > > > > > > > > > 
> > > > > > > > > > Presumably this is to reflect the fact that PCIe devices 
> > > > > > > > > > usually need to
> > > > > > > > > > be connected to a root (or further downstream) port rather 
> > > > > > > > > > than directly
> > > > > > > > > > on the root bus.  However, due to the odd requirements of 
> > > > > > > > > > the PAPR spec on the 'pseries'
> > > > > > > > > > machine type, it's normal for PCIe devices to appear on the 
> > > > > > > > > > root bus
> > > > > > > > > > without root ports.
> > > > > > > > > > 
> > > > > > > > > > Further, even on x86, there's no inherent reason we 
> > > > > > > > > > couldn't present a
> > > > > > > > > > virtio device as an "integrated device" (typically used for 
> > > > > > > > > > things built
> > > > > > > > > > into the PCI chipset), and those devices *do* typically 
> > > > > > > > > > appear on the root
> > > > > > > > > > bus.
> > > > > > > > > 
> > > > > > > > > I'm not personally making a counter-argument, just qouting 
> > > > > > > > > some of
> > > > > > > > > the relevant parts of "docs/pcie.txt" ("PCI EXPRESS 
> > > > > > > > > GUIDELINES"):
> > > > > > > > 
> > > > > > > > So, an earlier discussion more or less concluded that the PCIe
> > > > > > > > guidelines don't really work with PAPR guests.  That comes 
> > > > > > > > because
> > > > > > > > PAPR was designed with PowerVM in mind which allows PCI 
> > > > > > > > passthrough
> > > > > > > > but doesn't do any emulated PCI devices.  So they wanted to 
> > > > > > > > present
> > > > > > > > passed through devices (virtual or phyical) to the guest without
> > > > > > > > inserting virtual root ports.
> > > > > > > > 
> > > > > > > > Now, you can argue that this was a silly decision in PAPR, and 
> > > > > > > > you
> > > > > > > > could well be right, but there it is.
> > > > > > > 
> > > > > > > I can totally accept this, but then we should state it as a fact 
> > > > > > > near
> > > > > > > the top of "docs/pcie.txt".
> > > > > > > 
> > > > > > > > 
> > > > > > > > > > Place only the following kinds of devices directly on the 
> > > > > > > > > > Root Complex:
> > > > > > > > > > (1) PCI Devices (e.g. network card, graphics card, IDE 
> > > > > > > > > > controller),
> > > > > > > > > > not controllers. Place only legacy PCI devices on
> > > > > > > > > > the Root Complex. These will be considered 
> > > > > > > > > > Integrated Endpoints.
> > > > > > > > > > Note: Integrated Endpoints are not hot-pluggable.
> > > > > > > > > > 
> > > > > > > > > > Although the PCI Express spec does not forbid PCI 
> > > > > > > > > > Express devices as
> > > > > > > > > > Integrated Endpoints, existing hardware mostly 
> > > > > > > > > > integrates legacy PCI
> > > > > > > > > > devices with the Root Complex.
> > > > > > > > 
> > > > > > > > "Mostly".. on my laptop at least the GPU shows up as an 
> > > > > > > > integrated PCI
> > > > > > > > Express endpoint, so it's certainly not the case that *all* 
> > > > > > > > root bus
> > > > > > > > devices are legacy.
> > > > > > > > 
> > > > > > > > > Guest OSes are suspected to behave
> > > > > > > > > > strangely when PCI Express devices are integrated
> > > > > > > > > > with the Root Complex.
> > > > > > > > 
> > > > > > > > 

Re: [Qemu-devel] [PATCH v3 3/3] .shippable.yml: new CI provider

[Fam Zheng]

On Tue, 02/14 15:56, Alex Bennée wrote:
> > May I propose we merge 'docker testing' section of MAINTAINERS into 'build 
> > and
> > test automation' section? I don't know as much about travis (and shippable) 
> > but
> > I'm totally fine if you want to have docker tests under your umbrella, and
> > it seems a logical step judging from their names.
> 
> So:
> 
> Build and test automation
> -
> M: Alex Bennée 
> M: Fam Zheng 
> L: qemu-devel@nongnu.org
> S: Maintained
> F: .travis.yml
> F: .shippable.yml
> F: tests/docker/

Yes, looks good. Please include a patch in v4.

> 
> Or did you want to drop out of the review cycle and concentrate on patchew?

I would still keep my eyes on tests/docker/, the intent is just for being more
compact on MAINTAINERS.

Fam

Re: [Qemu-devel] [PATCH 25/25] qcow2-bitmap: improve check_constraints_on_bitmap

[John Snow]

On 02/14/2017 11:59 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add detailed error messages.
> 

yay

> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/qcow2-bitmap.c | 63 
> ++--
>  1 file changed, 41 insertions(+), 22 deletions(-)
> 
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 7ad1f7c..113d48c 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -160,28 +160,49 @@ static int check_table_entry(uint64_t entry, int 
> cluster_size)
>  
>  static int check_constraints_on_bitmap(BlockDriverState *bs,
> const char *name,
> -   uint32_t granularity)
> +   uint32_t granularity,
> +   Error **errp)
>  {
>  BDRVQcow2State *s = bs->opaque;
>  int granularity_bits = ctz32(granularity);
>  int64_t len = bdrv_getlength(bs);
> -bool fail;
>  
>  assert(granularity > 0);
>  assert((granularity & (granularity - 1)) == 0);
>  
>  if (len < 0) {
> +error_setg_errno(errp, -len, "Failed to get size of '%s'",
> + bdrv_get_device_or_node_name(bs));
>  return len;
>  }
>  
> -fail = (granularity_bits > BME_MAX_GRANULARITY_BITS) ||
> -   (granularity_bits < BME_MIN_GRANULARITY_BITS) ||
> -   (len > (uint64_t)BME_MAX_PHYS_SIZE << granularity_bits) ||
> -   (len > (uint64_t)BME_MAX_TABLE_SIZE * s->cluster_size <<
> -  granularity_bits) ||
> -   (strlen(name) > BME_MAX_NAME_SIZE);
> +if (granularity_bits > BME_MAX_GRANULARITY_BITS) {
> +error_setg(errp, "Granularity exceeds maximum (%u bytes)",
> +   1 << BME_MAX_GRANULARITY_BITS);
> +return -EINVAL;
> +}
> +if (granularity_bits < BME_MIN_GRANULARITY_BITS) {
> +error_setg(errp, "Granularity is under minimum (%u bytes)",
> +   1 << BME_MIN_GRANULARITY_BITS);
> +return -EINVAL;
> +}
>  
> -return fail ? -EINVAL : 0;
> +if ((len > (uint64_t)BME_MAX_PHYS_SIZE << granularity_bits) ||
> +(len > (uint64_t)BME_MAX_TABLE_SIZE * s->cluster_size <<
> +   granularity_bits))
> +{
> +error_setg(errp, "Too much space will be occupied by the bitmap. "
> +   "Use larger granularity");
> +return -EINVAL;
> +}
> +
> +if (strlen(name) > BME_MAX_NAME_SIZE) {
> +error_setg(errp, "Name length exceeds maximum (%u characters)",
> +   BME_MAX_NAME_SIZE);
> +return -EINVAL;
> +}
> +
> +return 0;
>  }
>  
>  static void clear_bitmap_table(BlockDriverState *bs, uint64_t *bitmap_table,
> @@ -1142,9 +1163,9 @@ void 
> qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
>  continue;
>  }
>  
> -if (check_constraints_on_bitmap(bs, name, granularity) < 0) {
> -error_setg(errp, "Bitmap '%s' doesn't satisfy the constraints",
> -   name);
> +if (check_constraints_on_bitmap(bs, name, granularity, errp) < 0) {
> +error_prepend(errp, "Bitmap '%s' doesn't satisfy the 
> constraints: ",
> +  name);
>  goto fail;
>  }
>  
> @@ -1230,13 +1251,11 @@ bool 
> qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
>Error **errp)
>  {
>  BDRVQcow2State *s = bs->opaque;
> -const char *reason = NULL;
>  bool found;
>  Qcow2BitmapList *bm_list;
>  
> -if (check_constraints_on_bitmap(bs, name, granularity) != 0) {
> -reason = "it doesn't satisfy the constraints";
> -goto common_errp;
> +if (check_constraints_on_bitmap(bs, name, granularity, errp) != 0) {
> +goto fail;
>  }
>  
>  if (s->nb_bitmaps == 0) {
> @@ -1246,21 +1265,21 @@ bool 
> qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
>  bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
> s->bitmap_directory_size, errp);
>  if (bm_list == NULL) {
> -return false;
> +goto fail;
>  }
>  
>  found = find_bitmap_by_name(bm_list, name);
>  bitmap_list_free(bm_list);
>  if (found) {
> -reason = "bitmap with the same name is already stored";
> -goto common_errp;
> +error_setg(errp, "Bitmap with the same name is already stored");
> +goto fail;
>  }
>  
>  return true;
>  
> -common_errp:
> -error_setg(errp, "Can't make bitmap '%s' persistent in '%s', as %s.",
> -   name, bdrv_get_device_or_node_name(bs), reason);
> +fail:
> +error_prepend(errp, "Can't make bitmap '%s' persistent in '%s': ",
> +  name, bdrv_get_device_or_node_name(bs));
>  return false;
>  }
>  
> 

Sorry I made it less efficient :)

Re: [Qemu-devel] [PATCH 22/25] block/dirty-bitmap: add bdrv_remove_persistent_dirty_bitmap

[John Snow]

On 02/14/2017 11:59 AM, Vladimir Sementsov-Ogievskiy wrote:
> Interface for removing persistent bitmap from its storage.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: Max Reitz 
> ---
>  block/dirty-bitmap.c | 18 ++
>  include/block/block_int.h|  3 +++
>  include/block/dirty-bitmap.h |  3 +++
>  3 files changed, 24 insertions(+)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index b684d8b..05e0aae 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -327,12 +327,30 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, 
> BdrvDirtyBitmap *bitmap)
>  /**
>   * Release all named dirty bitmaps attached to a BDS (for use in 
> bdrv_close()).
>   * There must not be any frozen bitmaps attached.
> + * This function does not remove persistent bitmaps from the storage.
>   */
>  void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs)
>  {
>  bdrv_do_release_matching_dirty_bitmap(bs, NULL, true);
>  }
>  
> +/**
> + * Remove persistent dirty bitmap from the storage if it exists.
> + * Absence of bitmap is not an error, because we have the following scenario:
> + * BdrvDirtyBitmap can have .persistent = true but not yet saved and have no
> + * stored version. For such bitmap bdrv_remove_persistent_dirty_bitmap() 
> should
> + * not fail.
> + * This function doesn't release corresponding BdrvDirtyBitmap.
> + */
> +void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs,
> + const char *name,
> + Error **errp)
> +{
> +if (bs->drv && bs->drv->bdrv_remove_persistent_dirty_bitmap) {
> +bs->drv->bdrv_remove_persistent_dirty_bitmap(bs, name, errp);
> +}
> +}
> +
>  void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>  {
>  assert(!bdrv_dirty_bitmap_frozen(bitmap));
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index db68067..d138555 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -328,6 +328,9 @@ struct BlockDriver {
>  const char *name,
>  uint32_t granularity,
>  Error **errp);
> +void (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs,
> +const char *name,
> +Error **errp);
>  
>  QLIST_ENTRY(BlockDriver) list;
>  };
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index b022b34..ce12610 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -25,6 +25,9 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState 
> *bs,
>  void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap);
>  void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap 
> *bitmap);
>  void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs);
> +void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs,
> + const char *name,
> + Error **errp);
>  void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>  void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>  BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
> 

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH] vmstate-static-checker: update white list with spapr_pci

[David Gibson]

On Tue, Feb 14, 2017 at 02:33:31PM +0100, Laurent Vivier wrote:
> To fix migration between 2.7 and 2.8, some fields have
> been renamed and managed with the help of a PHB property
> (pre_2_8_migration):
> 
> 5c4537b spapr: Fix 2.7<->2.8 migration of PCI host bridge
> 
> So we need to add them to the white list:
> 
> dma_liobn[0],
> mem_win_addr, mem_win_size,
> io_win_addr, io_win_size
> 
> become
> 
> mig_liobn,
> mig_mem_win_addr, mig_mem_win_size,
> mig_io_win_addr, mig_io_win_size
> 
> CC: David Gibson 
> CC: Dr. David Alan Gilbert 
> CC: Thomas Huth 
> CC: Greg Kurz 
> CC: Alexey Kardashevskiy 
> Signed-off-by: Laurent Vivier 
> ---
>  scripts/vmstate-static-checker.py | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/scripts/vmstate-static-checker.py 
> b/scripts/vmstate-static-checker.py
> index 14a27e7..bcef7ee 100755
> --- a/scripts/vmstate-static-checker.py
> +++ b/scripts/vmstate-static-checker.py
> @@ -85,6 +85,11 @@ def check_fields_match(name, s_field, d_field):
>  'xio3130-express-upstream-port': ['br.dev', 'parent_obj.parent_obj',
>'br.dev.exp.aer_log',
>
> 'parent_obj.parent_obj.exp.aer_log'],
> +'spapr_pci': ['dma_liobn[0]', 'mig_liobn',
> +  'mem_win_addr', 'mig_mem_win_addr',
> +  'mem_win_size', 'mig_mem_win_size',
> +  'io_win_addr', 'mig_io_win_addr',
> +  'io_win_size', 'mig_io_win_size'],
>  }
>  
>  if not name in changed_names:

Reviewed-by: David Gibson 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 21/25] qcow2-bitmap: refcounts

[John Snow]

On 02/14/2017 11:59 AM, Vladimir Sementsov-Ogievskiy wrote:
> Calculate refcounts for qcow2 bitmaps. It is needed for qcow2's qemu-img
> check implementation.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: Max Reitz 
> ---
>  block/qcow2-bitmap.c   | 76 
> ++
>  block/qcow2-refcount.c |  6 
>  block/qcow2.h  |  3 ++
>  3 files changed, 85 insertions(+)
> 
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 419d51e..326a799 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -1222,3 +1222,79 @@ common_errp:
> name, bdrv_get_device_or_node_name(bs), reason);
>  return false;
>  }
> +
> +int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> +  void **refcount_table,
> +  int64_t *refcount_table_size)
> +{
> +int ret;
> +BDRVQcow2State *s = bs->opaque;
> +Qcow2BitmapList *bm_list;
> +Qcow2Bitmap *bm;
> +
> +if (s->nb_bitmaps == 0) {
> +return 0;
> +}
> +
> +ret = qcow2_inc_refcounts_imrt(bs, res, refcount_table, 
> refcount_table_size,
> +   s->bitmap_directory_offset,
> +   s->bitmap_directory_size);
> +if (ret < 0) {
> +return ret;
> +}
> +
> +bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
> +   s->bitmap_directory_size, NULL);
> +if (bm_list == NULL) {
> +res->corruptions++;
> +return -EINVAL;
> +}
> +
> +QSIMPLEQ_FOREACH(bm, bm_list, entry) {
> +uint64_t *bitmap_table = NULL;
> +int i;
> +
> +ret = qcow2_inc_refcounts_imrt(bs, res,
> +   refcount_table, refcount_table_size,
> +   bm->table.offset,
> +   bm->table.size * sizeof(uint64_t));
> +if (ret < 0) {
> +goto out;
> +}
> +
> +ret = bitmap_table_load(bs, >table, _table);
> +if (ret < 0) {
> +res->corruptions++;
> +goto out;
> +}
> +
> +for (i = 0; i < bm->table.size; ++i) {
> +uint64_t entry = bitmap_table[i];
> +uint64_t offset = entry & BME_TABLE_ENTRY_OFFSET_MASK;
> +
> +if (check_table_entry(entry, s->cluster_size) < 0) {
> +res->corruptions++;
> +continue;
> +}
> +
> +if (offset == 0) {
> +continue;
> +}
> +
> +ret = qcow2_inc_refcounts_imrt(bs, res,
> +   refcount_table, 
> refcount_table_size,
> +   offset, s->cluster_size);
> +if (ret < 0) {
> +g_free(bitmap_table);
> +goto out;
> +}
> +}
> +
> +g_free(bitmap_table);
> +}
> +
> +out:
> +bitmap_list_free(bm_list);
> +
> +return ret;
> +}
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 14a736d..cdb74ba 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1846,6 +1846,12 @@ static int calculate_refcounts(BlockDriverState *bs, 
> BdrvCheckResult *res,
>  return ret;
>  }
>  
> +/* bitmaps */
> +ret = qcow2_check_bitmaps_refcounts(bs, res, refcount_table, 
> nb_clusters);
> +if (ret < 0) {
> +return ret;
> +}
> +
>  return check_refblocks(bs, res, fix, rebuild, refcount_table, 
> nb_clusters);
>  }
>  
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 0a3708d..eaad34a 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -624,5 +624,8 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState 
> *bs,
>const char *name,
>uint32_t granularity,
>Error **errp);
> +int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> +  void **refcount_table,
> +  int64_t *refcount_table_size);
>  
>  #endif
> 

Looks OK.

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH] Add PowerPC 32-bit guest memory dump support

[Philippe Mathieu-Daudé]

Hi Mike,

I failed to apply your patch on master:

error: patch failed: target/ppc/Makefile.objs:1
error: target/ppc/Makefile.objs: patch does not apply
error: patch failed: target/ppc/arch_dump.c:1
error: target/ppc/arch_dump.c: patch does not apply
error: patch failed: target/ppc/cpu.h:1225
error: target/ppc/cpu.h: patch does not apply
error: patch failed: target/ppc/translate_init.c:10478
error: target/ppc/translate_init.c: patch does not apply
Patch failed at 0001 Add PowerPC 32-bit guest memory dump support

Can you rebase/fix?

Thanks

On 02/08/2017 05:39 PM, Nawrocki, Michael wrote:

This patch extends support for the `dump-guest-memory` command to the 32-bit 
PowerPC architecture. It relies on the assumption that a 64-bit guest will not 
dump a 32-bit core file (and vice versa); if this assumption is invalid, please 
let me know.

Signed-off-by: Mike Nawrocki 
---
 target/ppc/Makefile.objs|   4 +-
 target/ppc/arch_dump.c  | 154 
 target/ppc/cpu.h|   2 +
 target/ppc/translate_init.c |   5 +-
 4 files changed, 91 insertions(+), 74 deletions(-)

diff --git a/target/ppc/Makefile.objs b/target/ppc/Makefile.objs
index a8c7a30..f50ffac 100644
--- a/target/ppc/Makefile.objs
+++ b/target/ppc/Makefile.objs
@@ -1,8 +1,8 @@
 obj-y += cpu-models.o
 obj-y += translate.o
 ifeq ($(CONFIG_SOFTMMU),y)
-obj-y += machine.o mmu_helper.o mmu-hash32.o monitor.o
-obj-$(TARGET_PPC64) += mmu-hash64.o arch_dump.o compat.o
+obj-y += machine.o mmu_helper.o mmu-hash32.o monitor.o arch_dump.o
+obj-$(TARGET_PPC64) += mmu-hash64.o compat.o
 endif
 obj-$(CONFIG_KVM) += kvm.o
 obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
index 40282a1..28d9cc7 100644
--- a/target/ppc/arch_dump.c
+++ b/target/ppc/arch_dump.c
@@ -1,5 +1,5 @@
 /*
- * writing ELF notes for ppc64 arch
+ * writing ELF notes for ppc{64,} arch
  *
  *
  * Copyright IBM, Corp. 2013
@@ -19,36 +19,48 @@
 #include "sysemu/dump.h"
 #include "sysemu/kvm.h"

-struct PPC64UserRegStruct {
-uint64_t gpr[32];
-uint64_t nip;
-uint64_t msr;
-uint64_t orig_gpr3;
-uint64_t ctr;
-uint64_t link;
-uint64_t xer;
-uint64_t ccr;
-uint64_t softe;
-uint64_t trap;
-uint64_t dar;
-uint64_t dsisr;
-uint64_t result;
+#ifdef TARGET_PPC64
+#define ELFCLASS ELFCLASS64
+#define cpu_to_dump_reg cpu_to_dump64
+typedef uint64_t reg_t;
+typedef Elf64_Nhdr Elf_Nhdr;
+#else
+#define ELFCLASS ELFCLASS32
+#define cpu_to_dump_reg cpu_to_dump32
+typedef uint32_t reg_t;
+typedef Elf32_Nhdr Elf_Nhdr;
+#endif /* TARGET_PPC64 */
+
+struct PPCUserRegStruct {
+reg_t gpr[32];
+reg_t nip;
+reg_t msr;
+reg_t orig_gpr3;
+reg_t ctr;
+reg_t link;
+reg_t xer;
+reg_t ccr;
+reg_t softe;
+reg_t trap;
+reg_t dar;
+reg_t dsisr;
+reg_t result;
 } QEMU_PACKED;

-struct PPC64ElfPrstatus {
+struct PPCElfPrstatus {
 char pad1[112];
-struct PPC64UserRegStruct pr_reg;
-uint64_t pad2[4];
+struct PPCUserRegStruct pr_reg;
+reg_t pad2[4];
 } QEMU_PACKED;


-struct PPC64ElfFpregset {
+struct PPCElfFpregset {
 uint64_t fpr[32];
-uint64_t fpscr;
+reg_t fpscr;
 }  QEMU_PACKED;


-struct PPC64ElfVmxregset {
+struct PPCElfVmxregset {
 ppc_avr_t avr[32];
 ppc_avr_t vscr;
 union {
@@ -57,26 +69,26 @@ struct PPC64ElfVmxregset {
 } vrsave;
 }  QEMU_PACKED;

-struct PPC64ElfVsxregset {
+struct PPCElfVsxregset {
 uint64_t vsr[32];
 }  QEMU_PACKED;

-struct PPC64ElfSperegset {
+struct PPCElfSperegset {
 uint32_t evr[32];
 uint64_t spe_acc;
 uint32_t spe_fscr;
 }  QEMU_PACKED;

 typedef struct noteStruct {
-Elf64_Nhdr hdr;
+Elf_Nhdr hdr;
 char name[5];
 char pad3[3];
 union {
-struct PPC64ElfPrstatus  prstatus;
-struct PPC64ElfFpregset  fpregset;
-struct PPC64ElfVmxregset vmxregset;
-struct PPC64ElfVsxregset vsxregset;
-struct PPC64ElfSperegset speregset;
+struct PPCElfPrstatus  prstatus;
+struct PPCElfFpregset  fpregset;
+struct PPCElfVmxregset vmxregset;
+struct PPCElfVsxregset vsxregset;
+struct PPCElfSperegset speregset;
 } contents;
 } QEMU_PACKED Note;

@@ -85,12 +97,12 @@ typedef struct NoteFuncArg {
 DumpState *state;
 } NoteFuncArg;

-static void ppc64_write_elf64_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
+static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
 {
 int i;
-uint64_t cr;
-struct PPC64ElfPrstatus *prstatus;
-struct PPC64UserRegStruct *reg;
+reg_t cr;
+struct PPCElfPrstatus *prstatus;
+struct PPCUserRegStruct *reg;
 Note *note = >note;
 DumpState *s = arg->state;

@@ -101,25 +113,25 @@ static void ppc64_write_elf64_prstatus(NoteFuncArg *arg, 
PowerPCCPU *cpu)
 reg = >pr_reg;

 for (i = 0; i < 32; i++) {
-reg->gpr[i] = cpu_to_dump64(s, 

Re: [Qemu-devel] [PATCH RFC v2 0/2] block: Crude initial implementation of -blockdev

[Markus Armbruster]

Markus Armbruster  writes:

> This is based on "[PATCH 00/24] QemuOpts util/cutils: Fix and clean up
> number conversions".
>
> v2: Support KEY=VALUE,... syntax as well.

Forgot to mention: the thing collapses when KEY has a type other than
string.  There's one reason this is marked RFC...

Re: [Qemu-devel] [PATCH 17/25] qmp: add autoload parameter to block-dirty-bitmap-add

[John Snow]

On 02/14/2017 11:59 AM, Vladimir Sementsov-Ogievskiy wrote:
> Optional. Default is false.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Signed-off-by: Denis V. Lunev 
> Reviewed-by: Max Reitz 
> ---
>  blockdev.c   | 18 --
>  qapi/block-core.json |  6 +-
>  2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 40605fa..e32ac69 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1968,6 +1968,7 @@ static void 
> block_dirty_bitmap_add_prepare(BlkActionState *common,
>  qmp_block_dirty_bitmap_add(action->node, action->name,
> action->has_granularity, action->granularity,
> action->has_persistent, action->persistent,
> +   action->has_autoload, action->autoload,
> _err);
>  
>  if (!local_err) {
> @@ -2698,6 +2699,7 @@ out:
>  void qmp_block_dirty_bitmap_add(const char *node, const char *name,
>  bool has_granularity, uint32_t granularity,
>  bool has_persistent, bool persistent,
> +bool has_autoload, bool autoload,
>  Error **errp)
>  {
>  AioContext *aio_context;
> @@ -2731,6 +2733,15 @@ void qmp_block_dirty_bitmap_add(const char *node, 
> const char *name,
>  if (!has_persistent) {
>  persistent = false;
>  }
> +if (!has_autoload) {
> +autoload = false;
> +}
> +
> +if (has_autoload && !persistent) {
> +error_setg(errp, "Autoload flag must be used only for persistent "
> + "bitmaps");
> +goto out;
> +}
>  

Good, glad we have this case covered.

>  if (persistent &&
>  !bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
> @@ -2739,10 +2750,13 @@ void qmp_block_dirty_bitmap_add(const char *node, 
> const char *name,
>  }
>  
>  bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
> -if (bitmap != NULL) {
> -bdrv_dirty_bitmap_set_persistance(bitmap, persistent);
> +if (bitmap == NULL) {
> +goto out;
>  }
>  
> +bdrv_dirty_bitmap_set_persistance(bitmap, persistent);
> +bdrv_dirty_bitmap_set_autoload(bitmap, autoload);
> +
>   out:
>  aio_context_release(aio_context);
>  }
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 535df20..09dcf4e 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1564,11 +1564,15 @@
>  #  Qcow2 disks support persistent bitmaps. Default is false.
>  #  (Since 2.9)
>  #
> +# @autoload: #optional the bitmap will be automatically loaded when the image
> +#it is stored in is opened. This flag may only be specified for
> +#persistent bitmaps. Default is false. (Since 2.9)
> +#
>  # Since: 2.4
>  ##
>  { 'struct': 'BlockDirtyBitmapAdd',
>'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
> -'*persistent': 'bool' } }
> +'*persistent': 'bool', '*autoload': 'bool' } }
>  
>  ##
>  # @block-dirty-bitmap-add:
> 

Looks good to me.

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH 16/25] qmp: add persistent flag to block-dirty-bitmap-add

[John Snow]

On 02/14/2017 11:59 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add optional 'persistent' flag to qmp command block-dirty-bitmap-add.
> Default is false.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Signed-off-by: Denis V. Lunev 
> Reviewed-by: Max Reitz 
> ---
>  blockdev.c   | 18 +-
>  qapi/block-core.json |  8 +++-
>  2 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 245e1e1..40605fa 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1967,6 +1967,7 @@ static void 
> block_dirty_bitmap_add_prepare(BlkActionState *common,
>  /* AIO context taken and released within qmp_block_dirty_bitmap_add */
>  qmp_block_dirty_bitmap_add(action->node, action->name,
> action->has_granularity, action->granularity,
> +   action->has_persistent, action->persistent,
> _err);
>  
>  if (!local_err) {
> @@ -2696,10 +2697,12 @@ out:
>  
>  void qmp_block_dirty_bitmap_add(const char *node, const char *name,
>  bool has_granularity, uint32_t granularity,
> +bool has_persistent, bool persistent,
>  Error **errp)
>  {
>  AioContext *aio_context;
>  BlockDriverState *bs;
> +BdrvDirtyBitmap *bitmap;
>  
>  if (!name || name[0] == '\0') {
>  error_setg(errp, "Bitmap name cannot be empty");
> @@ -2725,7 +2728,20 @@ void qmp_block_dirty_bitmap_add(const char *node, 
> const char *name,
>  granularity = bdrv_get_default_bitmap_granularity(bs);
>  }
>  
> -bdrv_create_dirty_bitmap(bs, granularity, name, errp);
> +if (!has_persistent) {
> +persistent = false;
> +}
> +
> +if (persistent &&
> +!bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
> +{
> +goto out;
> +}
> +

Yeah, I don't see where you're checking against MAX_BITMAPS in the qcow2
implementation for can_store.

> +bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
> +if (bitmap != NULL) {
> +bdrv_dirty_bitmap_set_persistance(bitmap, persistent);
> +}
>  
>   out:
>  aio_context_release(aio_context);
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 932f5bb..535df20 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1559,10 +1559,16 @@
>  # @granularity: #optional the bitmap granularity, default is 64k for
>  #   block-dirty-bitmap-add
>  #
> +# @persistent: #optional the bitmap is persistent, i.e. it will be saved to 
> the
> +#  corresponding block device image file on its close. For now 
> only
> +#  Qcow2 disks support persistent bitmaps. Default is false.
> +#  (Since 2.9)
> +#
>  # Since: 2.4
>  ##
>  { 'struct': 'BlockDirtyBitmapAdd',
> -  'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32' } }
> +  'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
> +'*persistent': 'bool' } }
>  
>  ##
>  # @block-dirty-bitmap-add:
> 

Looks good otherwise.

Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"

[ashish mittal]

On Tue, Feb 14, 2017 at 12:51 PM, Jeff Cody  wrote:
> On Thu, Feb 09, 2017 at 01:24:58AM -0800, ashish mittal wrote:
>> On Wed, Feb 8, 2017 at 10:29 PM, Jeff Cody  wrote:
>> > On Wed, Feb 08, 2017 at 09:23:33PM -0800, Ashish Mittal wrote:
>> >> From: Ashish Mittal 
>> >>
>> >> Source code for the qnio library that this code loads can be downloaded 
>> >> from:
>> >> https://github.com/VeritasHyperScale/libqnio.git
>> >>
>> >> Sample command line using JSON syntax:
>> >> ./x86_64-softmmu/qemu-system-x86_64 -name instance-0008 -S -vnc 
>> >> 0.0.0.0:0
>> >> -k en-us -vga cirrus -device 
>> >> virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
>> >> -msg timestamp=on
>> >> 'json:{"driver":"vxhs","vdisk-id":"c3e9095a-a5ee-4dce-afeb-2a59fb387410",
>> >> "server":{"host":"172.172.17.4","port":""}}'
>> >>
>> >> Sample command line using URI syntax:
>> >> qemu-img convert -f raw -O raw -n
>> >> /var/lib/nova/instances/_base/0c5eacd5ebea5ed914b6a3e7b18f1ce734c386ad
>> >> vxhs://192.168.0.1:/c6718f6b-0401-441d-a8c3-1f0064d75ee0
>> >>
>
> [...]
>
>> >> +#define VXHS_OPT_FILENAME   "filename"
>> >> +#define VXHS_OPT_VDISK_ID   "vdisk-id"
>> >> +#define VXHS_OPT_SERVER "server"
>> >> +#define VXHS_OPT_HOST   "host"
>> >> +#define VXHS_OPT_PORT   "port"
>> >> +#define VXHS_UUID_DEF "12345678-1234-1234-1234-123456789012"
>> >
>> > What is this?  It is not a valid UUID; is the value significant?
>> >
>>
>> This value gets passed to libvxhs for binaries like qemu-io, qemu-img
>> that do not have an Instance ID. We can use this default ID to control
>> access to specific vdisks by these binaries. qemu-kvm will pass the
>> actual instance ID, and therefore will not use this default value.
>>
>> Will reply to other queries soon.
>>
>
> If you are going to call it a UUID, it should adhere to the RFC 4122 spec.
> You can easily generate a compliant UUID with uuidgen.  However:
>
> Can you explain more about how you are using this to control access by
> qemu-img and qemu-io?  Looking at libqnio, it looks like this is used to
> determine at runtime which TLS certs to use based off of a
> pathname/filename, which is then how I presume you are controlling access.
> See Daniel's email regarding TLS certificates.
>

(1) The default behavior would be to disallow access to any vdisks by
the non qemu-kvm binaries. qemu-kvm would use the actual instance ID
for authentication.
(2) Depending on the workflow, HyperScale controller can choose to
grant *temporary* access to specific vdisks by qemu-img, qemu-io
binaries (identified by the default VXHS_UUID_DEF above).
(3) This information, described in #2, would be communicated by the
HyperScale controller to the actual proprietary VxHS server (running
on each compute) that does the authentication/SSL.
(4) The HyperScale controller, in this way, can grant/revoke access
for specific vdisks not just to clients with VXHS_UUID_DEF instance
ID, but also the actual VM instances.

> [...]
>
>> >> +
>> >> +static void bdrv_vxhs_init(void)
>> >> +{
>> >> +char out[37];
>
> Additional point: this should be sized as UUID_FMT_LEN + 1, not 37, but I
> suspect this code is changing anyways.
>
>> >> +
>> >> +if (qemu_uuid_is_null(_uuid)) {
>> >> +lib_init_failed = iio_init(QNIO_VERSION, vxhs_iio_callback, 
>> >> VXHS_UUID_DEF);
>> >> +} else {
>> >> +qemu_uuid_unparse(_uuid, out);
>> >> +lib_init_failed = iio_init(QNIO_VERSION, vxhs_iio_callback, out);
>> >> +}
>> >> +
>> >
>> > [1]
>> >
>> > Can you explain what is going on here with the qemu_uuid check?
>> >

(1) qemu_uuid_is_null(_uuid) is true for qemu-io, qemu-img that
do not define a instance ID. We end up using the default VXHS_UUID_DEF
ID for them, and authenticating them as described above.

(2) For the other case 'else', we convert the uuid to a char * using
qemu_uuid_unparse(), and pass the resulting char * uuid in variable
'out' to libvxhs.

>> >
>> > You also can't do this here.  This init function is just to register the
>> > driver (e.g. populate the BlockDriver list).  You shouldn't be doing
>> > anything other than the bdrv_register() call here.
>> >
>> > Since you want to run this iio_init only once, I would recommend doing it 
>> > in
>> > the vxhs_open() call, and using a ref counter.  That way, you can also call
>> > iio_fini() to finish releasing resources once the last device is closed.
>> >
>> > This was actually a suggestion I had before, which you then incorporated
>> > into v6, but it appears all the refcnt code has gone away for v7/v8.
>> >
>> >
>> >> +bdrv_register(_vxhs);
>> >> +}
>> >> +

Will consider this in the next patch.

Regards,
Ashish

Re: [Qemu-devel] [PATCH 10/24] tests/test-cutils: Add missing qemu_strtosz()... endptr checks

[Eric Blake]

On 02/14/2017 04:25 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster 
> ---
>  tests/test-cutils.c | 37 ++---
>  1 file changed, 26 insertions(+), 11 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 15/25] qcow2: add .bdrv_can_store_new_dirty_bitmap

[John Snow]

On 02/14/2017 11:59 AM, Vladimir Sementsov-Ogievskiy wrote:
> Realize .bdrv_can_store_new_dirty_bitmap interface.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: Max Reitz 
> ---
>  block/qcow2-bitmap.c | 40 
>  block/qcow2.c|  1 +
>  block/qcow2.h|  4 
>  3 files changed, 45 insertions(+)
> 
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index b177a95..419d51e 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -1182,3 +1182,43 @@ fail:
>  
>  bitmap_list_free(bm_list);
>  }
> +
> +bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
> +  const char *name,
> +  uint32_t granularity,
> +  Error **errp)
> +{
> +BDRVQcow2State *s = bs->opaque;
> +const char *reason = NULL;
> +bool found;
> +Qcow2BitmapList *bm_list;
> +
> +if (check_constraints_on_bitmap(bs, name, granularity) != 0) {
> +reason = "it doesn't satisfy the constraints";
> +goto common_errp;
> +}

"the constraints" are still entirely opaque to the caller, it may not be
obvious that it's because of the name length.

(But maybe you fix this in patch 25.)

> +
> +if (s->nb_bitmaps == 0) {
> +return true;
> +}
> +
> +bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
> +   s->bitmap_directory_size, errp);
> +if (bm_list == NULL) {
> +return false;
> +}

Another opportunity for caching the bm_list.

> +
> +found = find_bitmap_by_name(bm_list, name);
> +bitmap_list_free(bm_list);
> +if (found) {
> +reason = "bitmap with the same name is already stored";
> +goto common_errp;
> +}
> +
> +return true;
> +
> +common_errp:
> +error_setg(errp, "Can't make bitmap '%s' persistent in '%s', as %s.",
> +   name, bdrv_get_device_or_node_name(bs), reason);
> +return false;
> +}

We don't check total numbers of bitmaps here? (I'll keep reading ahead
in the series.)

> diff --git a/block/qcow2.c b/block/qcow2.c
> index d0e41bf..6e1fe53 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3541,6 +3541,7 @@ BlockDriver bdrv_qcow2 = {
>  
>  .bdrv_load_autoloading_dirty_bitmaps = 
> qcow2_load_autoloading_dirty_bitmaps,
>  .bdrv_store_persistent_dirty_bitmaps = 
> qcow2_store_persistent_dirty_bitmaps,
> +.bdrv_can_store_new_dirty_bitmap = qcow2_can_store_new_dirty_bitmap,
>  };
>  
>  static void bdrv_qcow2_init(void)
> diff --git a/block/qcow2.h b/block/qcow2.h
> index d9a7643..749710d 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -616,5 +616,9 @@ void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, 
> void **table);
>  /* qcow2-bitmap.c functions */
>  void qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error 
> **errp);
>  void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error 
> **errp);
> +bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
> +  const char *name,
> +  uint32_t granularity,
> +  Error **errp);
>  
>  #endif
> 

Benefit of the doubt:

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH 09/24] QemuOpts: Fix to reject numbers that overflow uint64_t

[Eric Blake]

On 02/14/2017 04:25 AM, Markus Armbruster wrote:
> parse_option_number() fails to check for overflow after strtoull().
> Has always been broken.  Fix that.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  tests/test-qemu-opts.c | 14 ++
>  util/qemu-option.c | 11 ---
>  2 files changed, 14 insertions(+), 11 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v14 00/24] qcow2: persistent dirty bitmaps

[Eric Blake]

On 02/14/2017 01:02 PM, John Snow wrote:
> 
> 
> On 02/14/2017 11:59 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
> 
> Hi! :)
> 
>> There is a new update of qcow2-bitmap series - v14.
>>
> 
> Having the cover letter be 00/24 but including 25 patches confuses the
> patch scraping tool a good deal. Also, can you include the "v14" in the
> patch emails themselves, too?

If you do have a reason for v15, using 'qemu send-email -v15 ...' should
automatically put the prefix on all the mails.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 07/24] util/cutils: Clean up variable names around qemu_strtol()

[Eric Blake]

On 02/14/2017 04:25 AM, Markus Armbruster wrote:
> Name same things the same, different things differently.
> 
> * qemu_strtol()'s parameter @nptr is called @p in
>   check_strtox_error().  Rename the latter.
> 
> * qemu_strtol()'s parameter @endptr is called @next in
>   check_strtox_error().  Rename the latter.
> 
> * qemu_strtol()'s variable @p is called @endptr in
>   check_strtox_error().  Rename both to @ep.
> 
> * qemu_strtol()'s variable @err is *negative* errno,
>   check_strtox_error()'s parameter @err is *positive*.  Rename the
>   latter to @eno.

Whether or not you rename @eno based on Paolo's comments,

> 
> Same for qemu_strtoul(), qemu_strtoi64(), qemu_strtou64(), of course.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  util/cutils.c | 42 +-
>  1 file changed, 21 insertions(+), 21 deletions(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 06/24] util/cutils: Rename qemu_strtoll(), qemu_strtoull()

[Eric Blake]

On 02/14/2017 04:25 AM, Markus Armbruster wrote:
> The name qemu_strtoll() suggests conversion to long long, but it
> actually converts to int64_t.  Rename to qemu_strtoi64().
> 
> The name qemu_strtoull() suggests conversion to unsigned long long,
> but it actually converts to uint64_t.  Rename to qemu_strtou64().
> 
> Signed-off-by: Markus Armbruster 
> ---
>  include/qemu/cutils.h |   6 +-
>  qobject/qdict.c   |   2 +-
>  qtest.c   |  34 ++---
>  tests/test-cutils.c   | 366 
> +++---
>  util/cutils.c |   4 +-
>  util/log.c|   4 +-
>  6 files changed, 224 insertions(+), 192 deletions(-)

Thankfully, not too many existing clients (except in the testsuite).
Mechanical search-and-replace, and I don't see anything missed, so:

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 05/24] util/cutils: Rewrite documentation of qemu_strtol() & friends

[Eric Blake]

On 02/14/2017 04:25 AM, Markus Armbruster wrote:
> Fixes the following documentation bugs:
> 
> * Fails to document that null @nptr is safe.
> 
> * Fails to document that we return -EINVAL when no conversion could be
>   performed (commit 47d4be1).
> 
> * Confuses long long with int64_t, and unsigned long long with
>   uint64_t.
> 
> * Claims the unsigned conversions can underflow.  They can't.
> 
> While there, mark problematic assumptions that int64_t is long long,
> and uint64_t is unsigned long long with FIXME comments.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  util/cutils.c | 80 
> +--
>  1 file changed, 45 insertions(+), 35 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 04/24] tests/test-cutils: Clean up qemu_strtoul() result checks

[Eric Blake]

On 02/14/2017 04:25 AM, Markus Armbruster wrote:
> Use unsigned comparisons to check the result of qemu_strtoul() and
> strtoull().
> 
> Signed-off-by: Markus Armbruster 
> ---
>  tests/test-cutils.c | 60 
> ++---
>  1 file changed, 30 insertions(+), 30 deletions(-)
> 

No real difference except when the test fails (better printed result if
the number being printed exceeds the max signed counterpart value), but
never hurts to be more precise.

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"

[Jeff Cody]

On Thu, Feb 09, 2017 at 01:24:58AM -0800, ashish mittal wrote:
> On Wed, Feb 8, 2017 at 10:29 PM, Jeff Cody  wrote:
> > On Wed, Feb 08, 2017 at 09:23:33PM -0800, Ashish Mittal wrote:
> >> From: Ashish Mittal 
> >>
> >> Source code for the qnio library that this code loads can be downloaded 
> >> from:
> >> https://github.com/VeritasHyperScale/libqnio.git
> >>
> >> Sample command line using JSON syntax:
> >> ./x86_64-softmmu/qemu-system-x86_64 -name instance-0008 -S -vnc 
> >> 0.0.0.0:0
> >> -k en-us -vga cirrus -device 
> >> virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
> >> -msg timestamp=on
> >> 'json:{"driver":"vxhs","vdisk-id":"c3e9095a-a5ee-4dce-afeb-2a59fb387410",
> >> "server":{"host":"172.172.17.4","port":""}}'
> >>
> >> Sample command line using URI syntax:
> >> qemu-img convert -f raw -O raw -n
> >> /var/lib/nova/instances/_base/0c5eacd5ebea5ed914b6a3e7b18f1ce734c386ad
> >> vxhs://192.168.0.1:/c6718f6b-0401-441d-a8c3-1f0064d75ee0
> >>

[...]

> >> +#define VXHS_OPT_FILENAME   "filename"
> >> +#define VXHS_OPT_VDISK_ID   "vdisk-id"
> >> +#define VXHS_OPT_SERVER "server"
> >> +#define VXHS_OPT_HOST   "host"
> >> +#define VXHS_OPT_PORT   "port"
> >> +#define VXHS_UUID_DEF "12345678-1234-1234-1234-123456789012"
> >
> > What is this?  It is not a valid UUID; is the value significant?
> >
> 
> This value gets passed to libvxhs for binaries like qemu-io, qemu-img
> that do not have an Instance ID. We can use this default ID to control
> access to specific vdisks by these binaries. qemu-kvm will pass the
> actual instance ID, and therefore will not use this default value.
> 
> Will reply to other queries soon.
>

If you are going to call it a UUID, it should adhere to the RFC 4122 spec.
You can easily generate a compliant UUID with uuidgen.  However:

Can you explain more about how you are using this to control access by
qemu-img and qemu-io?  Looking at libqnio, it looks like this is used to
determine at runtime which TLS certs to use based off of a
pathname/filename, which is then how I presume you are controlling access.
See Daniel's email regarding TLS certificates.

[...]

> >> +
> >> +static void bdrv_vxhs_init(void)
> >> +{
> >> +char out[37];

Additional point: this should be sized as UUID_FMT_LEN + 1, not 37, but I
suspect this code is changing anyways.

> >> +
> >> +if (qemu_uuid_is_null(_uuid)) {
> >> +lib_init_failed = iio_init(QNIO_VERSION, vxhs_iio_callback, 
> >> VXHS_UUID_DEF);
> >> +} else {
> >> +qemu_uuid_unparse(_uuid, out);
> >> +lib_init_failed = iio_init(QNIO_VERSION, vxhs_iio_callback, out);
> >> +}
> >> +
> >
> > [1]
> >
> > Can you explain what is going on here with the qemu_uuid check?
> >
> >
> > You also can't do this here.  This init function is just to register the
> > driver (e.g. populate the BlockDriver list).  You shouldn't be doing
> > anything other than the bdrv_register() call here.
> >
> > Since you want to run this iio_init only once, I would recommend doing it in
> > the vxhs_open() call, and using a ref counter.  That way, you can also call
> > iio_fini() to finish releasing resources once the last device is closed.
> >
> > This was actually a suggestion I had before, which you then incorporated
> > into v6, but it appears all the refcnt code has gone away for v7/v8.
> >
> >
> >> +bdrv_register(_vxhs);
> >> +}
> >> +

Re: [Qemu-devel] [PATCH 02/24] QemuOpts: Assert value string isn't null

[Eric Blake]

On 02/14/2017 04:25 AM, Markus Armbruster wrote:
> Plenty of code relies on QemuOpt member @str not being null, including
> qemu_opts_print(), qemu_opts_to_qdict(), and callbacks passed to
> qemu_opt_foreach().
> 

> 
> Assert member @str isn't null, so that misuse is caught right away.
> 
> Simplify parse_option_bool(), parse_option_number() and
> parse_option_size() accordingly.  Best viewed with whitespace changes
> ignored.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  util/qemu-option.c | 89 
> --
>  1 file changed, 39 insertions(+), 50 deletions(-)
> 

> @@ -180,39 +172,35 @@ void parse_option_size(const char *name, const char 
> *value,
>  char *postfix;
>  double sizef;
>  

> +sizef = strtod(value, );
> +if (sizef < 0 || sizef > UINT64_MAX) {
> +error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
> +   "a non-negative number below 2^64");
> +return;
> +}
> +switch (*postfix) {
> +case 'T':
...
> +default:
>  error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
> +error_append_hint(errp, "You may use k, M, G or T suffixes for "
> +  "kilobytes, megabytes, gigabytes and 
> terabytes.\n");
> +return;
>  }

Unrelated to this patch, but noticing it now: it looks like we blindly
accept "qemu-system-x86_64 -nodefaults -m 1Mgarbage" as meaning the same
as "... -m 1M".  Looking back at 1/24, looks like you marked that as one
of the buggy cases.  Good - I guess I'll get to comment more on it in a
later patch.

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 00/16] Postcopy: Hugepage support

[Dr. David Alan Gilbert]

* Alexey Perevalov (a.pereva...@samsung.com) wrote:
> Hi David,
> 
> Thank your, now it's clear.
> 
> On Mon, Feb 13, 2017 at 06:16:02PM +, Dr. David Alan Gilbert wrote:
> > * Alexey Perevalov (a.pereva...@samsung.com) wrote:
> > >  Hello David!
> > 
> > Hi Alexey,
> > 
> > > I have checked you series with 1G hugepage, but only in 1 Gbit/sec network
> > > environment.
> > 
> > Can you show the qemu command line you're using?  I'm just trying
> > to make sure I understand where your hugepages are; running 1G hostpages
> > across a 1Gbit/sec network for postcopy would be pretty poor - it would take
> > ~10 seconds to transfer the page.
> 
> sure
> -hda ./Ubuntu.img -name PAU,debug-threads=on -boot d -net nic -net user
> -m 1024 -localtime -nographic -enable-kvm -incoming tcp:0: -object
> memory-backend-file,id=mem,size=1G,mem-path=/dev/hugepages -mem-prealloc
> -numa node,memdev=mem -trace events=/tmp/events -chardev
> socket,id=charmonitor,path=/var/lib/migrate-vm-monitor.sock,server,nowait
> -mon chardev=charmonitor,id=monitor,mode=control

OK, it's a pretty unusual setup - a 1G page guest with 1G of guest RAM.

> > 
> > > I started Ubuntu just with console interface and gave to it only 1G of
> > > RAM, inside Ubuntu I started stress command
> > 
> > > (stress --cpu 4 --io 4 --vm 4 --vm-bytes 25600 &)
> > > in such environment precopy live migration was impossible, it never
> > > being finished, in this case it infinitely sends pages (it looks like
> > > dpkg scenario).
> > > 
> > > Also I modified stress utility
> > > http://people.seas.harvard.edu/~apw/stress/stress-1.0.4.tar.gz
> > > due to it wrote into memory every time the same value `Z`. My
> > > modified version writes every allocation new incremented value.
> > 
> > I use google's stressapptest normally; although remember to turn
> > off the bit where it pauses.
> 
> I decided to use it too
> stressapptest -s 300 -M 256 -m 8 -W
> 
> > 
> > > I'm using Arcangeli's kernel only at the destination.
> > > 
> > > I got controversial results. Downtime for 1G hugepage is close to 2Mb
> > > hugepage and it took around 7 ms (in 2Mb hugepage scenario downtime was
> > > around 8 ms).
> > > I made that opinion by query-migrate.
> > > {"return": {"status": "completed", "setup-time": 6, "downtime": 6, 
> > > "total-time": 9668, "ram": {"total": 1091379200, "postcopy-requests": 1, 
> > > "dirty-sync-count": 2, "remaining": 0, "mbps": 879.786851, "transferred": 
> > > 1063007296, "duplicate": 7449, "dirty-pages-rate": 0, "skipped": 0, 
> > > "normal-bytes": 1060868096, "normal": 259001}}}
> > > 
> > > Documentation says about downtime field - measurement unit is ms.
> > 
> > The downtime measurement field is pretty meaningless for postcopy; it's only
> > the time from stopping the VM until the point where we tell the destination 
> > it
> > can start running.  Meaningful measurements are only from inside the guest
> > really, or the place latencys.
> >
> 
> Maybe improve it by receiving such information from destination?
> I wish to do that.
> > > So I traced it (I added additional trace into postcopy_place_page
> > > trace_postcopy_place_page_start(host, from, pagesize); )
> > > 
> > > postcopy_ram_fault_thread_request Request for HVA=7f6dc000 
> > > rb=/objects/mem offset=0
> > > postcopy_place_page_start host=0x7f6dc000 from=0x7f6d7000, 
> > > pagesize=4000
> > > postcopy_place_page_start host=0x7f6e0e80 from=0x55b665969619, 
> > > pagesize=1000
> > > postcopy_place_page_start host=0x7f6e0e801000 from=0x55b6659684e8, 
> > > pagesize=1000
> > > several pages with 4Kb step ...
> > > postcopy_place_page_start host=0x7f6e0e817000 from=0x55b6659694f0, 
> > > pagesize=1000
> > > 
> > > 4K pages, started from 0x7f6e0e80 address it's
> > > vga.ram, /rom@etc/acpi/tables etc.
> > > 
> > > Frankly saying, right now, I don't have any ideas why hugepage wasn't
> > > resent. Maybe my expectation of it is wrong as well as understanding )
> > 
> > That's pretty much what I expect to see - before you get into postcopy
> > mode everything is sent as individual 4k pages (in order); once we're
> > in postcopy mode we send each page no more than once.  So you're
> > huge page comes across once - and there it is.
> > 
> > > stress utility also duplicated for me value into appropriate file:
> > > sec_since_epoch.microsec:value
> > > 1487003192.728493:22
> > > 1487003197.335362:23
> > > *1487003213.367260:24*
> > > *1487003238.480379:25*
> > > 1487003243.315299:26
> > > 1487003250.775721:27
> > > 1487003255.473792:28
> > > 
> > > It mean rewriting 256Mb of memory per byte took around 5 sec, but at
> > > the moment of migration it took 25 sec.
> > 
> > right, now this is the thing that's more useful to measure.
> > That's not too surprising; when it migrates that data is changing rapidly
> > so it's going to have to pause and wait for that whole 1GB to be 
> > transferred.
> > Your 1Gbps network is going to take about 10 seconds to transfer that
> > 

[Qemu-devel] [PATCH v5 5/7] blkdebug: Simplify override logic

[Eric Blake]

Rather than store into a local variable, then copy to the struct
if the value is valid, then reporting errors otherwise, it is
simpler to just store into the struct and report errors if the
value is invalid.  This however requires that the struct store
a 64-bit number, rather than a narrower type.  Move the errno
assignment into a label that will be reused from more places in
the next patch.

Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 

---
v5: no change
v4: fix typo in commit message, move errno assignment
v3: new patch
---
 block/blkdebug.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index b2d5f7d..2996152 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -38,7 +38,7 @@
 typedef struct BDRVBlkdebugState {
 int state;
 int new_state;
-int align;
+uint64_t align;

 /* For blkdebug_refresh_filename() */
 char *config_file;
@@ -353,7 +353,6 @@ static int blkdebug_open(BlockDriverState *bs, QDict 
*options, int flags,
 BDRVBlkdebugState *s = bs->opaque;
 QemuOpts *opts;
 Error *local_err = NULL;
-uint64_t align;
 int ret;

 opts = qemu_opts_create(_opts, NULL, 0, _abort);
@@ -389,12 +388,9 @@ static int blkdebug_open(BlockDriverState *bs, QDict 
*options, int flags,
 bs->file->bs->supported_zero_flags;

 /* Set request alignment */
-align = qemu_opt_get_size(opts, "align", 0);
-if (align < INT_MAX && is_power_of_2(align)) {
-s->align = align;
-} else if (align) {
-error_setg(errp, "Invalid alignment");
-ret = -EINVAL;
+s->align = qemu_opt_get_size(opts, "align", 0);
+if (s->align && (s->align >= INT_MAX || !is_power_of_2(s->align))) {
+error_setg(errp, "Invalid alignment, align must be integer power of 
2");
 goto fail_unref;
 }

@@ -402,6 +398,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto out;

 fail_unref:
+ret = -EINVAL;
 bdrv_unref_child(bs, bs->file);
 out:
 if (ret < 0) {
-- 
2.9.3

[Qemu-devel] [PATCH v5 7/7] tests: Add coverage for recent block geometry fixes

[Eric Blake]

Use blkdebug's new geometry constraints to emulate setups that
have caused recent regression fixes: write zeroes asserting
when running through a loopback block device with max-transfer
smaller than cluster size, and discard rounding away portions
of requests not aligned to preferred boundaries.  Also, add
coverage that the block layer is honoring max transfer limits.

For now, a single iotest performs all actions, with the idea
that we can add future blkdebug constraint test cases in the
same file; but it can be split into multiple iotests if we find
reason to run one portion of the test in more setups than what
are possible in the other.

For reference, the final portion of the test (checking whether
discard passes as much as possible to the lowest layers of the
stack) works as follows:

qemu-io: discard 30M at 8001, passed to blkdebug
  blkdebug: discard 511 bytes at 8001, -ENOTSUP (smaller than
blkdebug's 512 align)
  blkdebug: discard 14371328 bytes at 8512, passed to qcow2
qcow2: discard 739840 bytes at 8512, -ENOTSUP (smaller than
qcow2's 1M align)
qcow2: discard 13M bytes at 77M, succeeds
  blkdebug: discard 15M bytes at 90M, passed to qcow2
qcow2: discard 15M bytes at 90M, succeeds
  blkdebug: discard 1356800 bytes at 105M, passed to qcow2
qcow2: discard 1M at 105M, succeeds
qcow2: discard 308224 bytes at 106M, -ENOTSUP (smaller than qcow2's
1M align)
  blkdebug: discard 1 byte at 111457280, -ENOTSUP (smaller than
blkdebug's 512 align)

Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 

---
v5: rebase to master by renumbering s/173/175/
v4: clean up some comments, nicer backing file creation, more commit message
v3: make comments tied more to test at hand, rather than the
particular hardware that led to the earlier patches being tested
v2: new patch
---
 tests/qemu-iotests/175 | 114 +
 tests/qemu-iotests/175.out |  49 +++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 164 insertions(+)
 create mode 100755 tests/qemu-iotests/175
 create mode 100644 tests/qemu-iotests/175.out

diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
new file mode 100755
index 000..867c9ca
--- /dev/null
+++ b/tests/qemu-iotests/175
@@ -0,0 +1,114 @@
+#!/bin/bash
+#
+# Test corner cases with unusual block geometries
+#
+# Copyright (C) 2016-2017 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=ebl...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+
+CLUSTER_SIZE=1M
+size=128M
+options=driver=blkdebug,image.driver=qcow2
+
+echo
+echo "== setting up files =="
+
+TEST_IMG="$TEST_IMG.base" _make_test_img $size
+$QEMU_IO -c "write -P 11 0 $size" "$TEST_IMG.base" | _filter_qemu_io
+_make_test_img -b "$TEST_IMG.base"
+$QEMU_IO -c "write -P 22 0 $size" "$TEST_IMG" | _filter_qemu_io
+
+# Limited to 64k max-transfer
+echo
+echo "== constrained alignment and max-transfer =="
+limits=align=4k,max-transfer=64k
+$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \
+ -c "write -P 33 1000 128k" -c "read -P 33 1000 128k" | _filter_qemu_io
+
+echo
+echo "== write zero with constrained max-transfer =="
+limits=align=512,max-transfer=64k,opt-write-zero=$CLUSTER_SIZE
+$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \
+ -c "write -z 8003584 2093056" | _filter_qemu_io
+
+# non-power-of-2 write-zero/discard alignments
+echo
+echo "== non-power-of-2 write zeroes limits =="
+
+limits=align=512,opt-write-zero=15M,max-write-zero=15M,opt-discard=15M,max-discard=15M
+$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \
+ -c "write -z 32M 32M" | _filter_qemu_io
+
+echo
+echo "== non-power-of-2 discard limits =="
+
+limits=align=512,opt-write-zero=15M,max-write-zero=15M,opt-discard=15M,max-discard=15M
+$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \
+ -c "discard 8001 30M" | _filter_qemu_io
+
+echo
+echo "== verify image content =="
+
+function verify_io()
+{
+if ($QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" |

Re: [Qemu-devel] [PATCH 13/25] qcow2: add .bdrv_store_persistent_dirty_bitmaps()

[John Snow]

On 02/14/2017 11:59 AM, Vladimir Sementsov-Ogievskiy wrote:
> Realize block bitmap storing interface, to allow qcow2 images store
> persistent bitmaps.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: Max Reitz 

Reviewed-by: John Snow 

[Qemu-devel] [PATCH v4 4/4] sd: sdhci: Remove block count enable check in single block transfers

[P J P]

From: Prasad J Pandit 

In SDHCI protocol, the 'Block count enable' bit of the Transfer
Mode register is relevant only in multi block transfers. We need
not check it in single block transfers.

Signed-off-by: Prasad J Pandit 
---
 hw/sd/sdhci.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 8ae75fe..05558d3 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -570,7 +570,6 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
 }
 
 /* single block SDMA transfer */
-
 static void sdhci_sdma_transfer_single_block(SDHCIState *s)
 {
 int n;
@@ -589,10 +588,7 @@ static void sdhci_sdma_transfer_single_block(SDHCIState *s)
 sdbus_write_data(>sdbus, s->fifo_buffer[n]);
 }
 }
-
-if (s->trnmod & SDHC_TRNS_BLK_CNT_EN) {
-s->blkcnt--;
-}
+s->blkcnt--;
 
 sdhci_end_transfer(s);
 }
-- 
2.9.3

[Qemu-devel] [PATCH v4 3/4] sd: sdhci: conditionally invoke multi block transfer

[P J P]

From: Prasad J Pandit 

In sdhci_write invoke multi block transfer if it is enabled
in the transfer mode register 's->trnmod'.

Signed-off-by: Prasad J Pandit 
---
 hw/sd/sdhci.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index f8220c0..8ae75fe 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1023,7 +1023,11 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
 /* Writing to last byte of sdmasysad might trigger transfer */
 if (!(mask & 0xFF00) && TRANSFERRING_DATA(s->prnsts) && s->blkcnt 
&&
 s->blksize && SDHC_DMA_TYPE(s->hostctl) == SDHC_CTRL_SDMA) {
-sdhci_sdma_transfer_multi_blocks(s);
+if (s->trnmod & SDHC_TRNS_MULTI) {
+sdhci_sdma_transfer_multi_blocks(s);
+} else {
+sdhci_sdma_transfer_single_block(s);
+}
 }
 break;
 case SDHC_BLKSIZE:
-- 
2.9.3

[Qemu-devel] [PATCH v4 2/4] sd: sdhci: check transfer mode register in multi block transfer

[P J P]

From: Prasad J Pandit 

In the SDHCI protocol, the transfer mode register value
is used during multi block transfer to check if block count
register is enabled and should be updated. Transfer mode
register could be set such that, block count register would
not be updated, thus leading to an infinite loop. Add check
to avoid it.

Reported-by: Wjjzhang 
Reported-by: Jiang Xin 
Signed-off-by: Prasad J Pandit 
---
 hw/sd/sdhci.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index cf647fa..f8220c0 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -487,6 +487,11 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
 uint32_t boundary_chk = 1 << (((s->blksize & 0xf000) >> 12) + 12);
 uint32_t boundary_count = boundary_chk - (s->sdmasysad % boundary_chk);
 
+if (!(s->trnmod & SDHC_TRNS_BLK_CNT_EN) || !s->blkcnt) {
+qemu_log_mask(LOG_UNIMP, "infinite transfer is not supported\n");
+return;
+}
+
 /* XXX: Some sd/mmc drivers (for example, u-boot-slp) do not account for
  * possible stop at page boundary if initial address is not page aligned,
  * allow them to work properly */
@@ -798,11 +803,6 @@ static void sdhci_data_transfer(void *opaque)
 if (s->trnmod & SDHC_TRNS_DMA) {
 switch (SDHC_DMA_TYPE(s->hostctl)) {
 case SDHC_CTRL_SDMA:
-if ((s->trnmod & SDHC_TRNS_MULTI) &&
-(!(s->trnmod & SDHC_TRNS_BLK_CNT_EN) || s->blkcnt == 0)) {
-break;
-}
-
 if ((s->blkcnt == 1) || !(s->trnmod & SDHC_TRNS_MULTI)) {
 sdhci_sdma_transfer_single_block(s);
 } else {
-- 
2.9.3

[Qemu-devel] [PATCH v4 0/4] sd: sdhci: correct transfer mode register usage

[P J P]

From: Prasad J Pandit 

Hello,

In SDHCI protocol, the 'Block Count Enable' bit of the Transfer Mode
register is used to control 's->blkcnt' value. This bit is not relevant
in single block transfers. Also, Transfer Mode register value could be
set such that 's->blkcnt' would not see an update during multi block
transfers. Thus leading to an infinite loop.

This patch set attempts to correct 'Block Count Enable' bit usage.

This series incorporates changes suggested in patch set v3:
  -> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg02376.html
  -> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg02905.html

Thank you.
--
Prasad J Pandit (4):
  sd: sdhci: mask transfer mode register value
  sd: sdhci: check transfer mode register in multi block transfer
  sd: sdhci: conditionally invoke multi block transfer
  sd: sdhci: Remove block count enable check in single block transfers

 hw/sd/sdhci.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH v4 1/4] sd: sdhci: mask transfer mode register value

[P J P]

From: Prasad J Pandit 

In SDHCI protocol, the transfer mode register is defined
to be of 6 bits. Mask its value with '0x0037' so that an
invalid value could not be assigned.

Signed-off-by: Prasad J Pandit 
---
 hw/sd/sdhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Update per: use macro for the mask value
  -> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg02774.html

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 5bd5ab6..cf647fa 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -119,6 +119,7 @@
 (SDHC_CAPAB_BASECLKFREQ << 8) | (SDHC_CAPAB_TOUNIT << 7) | \
 (SDHC_CAPAB_TOCLKFREQ))
 
+#define MASK_TRNMOD 0x0037
 #define MASKED_WRITE(reg, mask, val)  (reg = (reg & (mask)) | (val))
 
 static uint8_t sdhci_slotint(SDHCIState *s)
@@ -1050,7 +1051,7 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
 if (!(s->capareg & SDHC_CAN_DO_DMA)) {
 value &= ~SDHC_TRNS_DMA;
 }
-MASKED_WRITE(s->trnmod, mask, value);
+MASKED_WRITE(s->trnmod, mask, value & MASK_TRNMOD);
 MASKED_WRITE(s->cmdreg, mask >> 16, value >> 16);
 
 /* Writing to the upper byte of CMDREG triggers SD command generation 
*/
-- 
2.9.3

Re: [Qemu-devel] [PATCH 1/1] qemu-iotests: redirect nbd server stdout to /dev/null

[Eric Blake]

On 02/14/2017 12:15 PM, Jeff Cody wrote:
> Some iotests (e.g. 174) try to filter the output of _make_test_image by
> piping the stdout.  Pipe the server stdout to /dev/null, so that filter
> pipe does not need to wait until process completion.
> 
> Signed-off-by: Jeff Cody 
> ---
>  tests/qemu-iotests/common.rc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake 

> 
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index 6c0fd4c..08065dc 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -172,7 +172,7 @@ _make_test_img()
>  
>  # Start an NBD server on the image file, which is what we'll be talking 
> to
>  if [ $IMGPROTO = "nbd" ]; then
> -eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT  
> $TEST_IMG_FILE &"
> +eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT  
> $TEST_IMG_FILE >/dev/null &"
>  sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
>  fi
>  }
> 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH V8] qqq: module for synchronizing with a simulation

[James J. Nutaro]

This patch adds an interface for pacing the execution of QEMU to match an 
external
simulation clock. Its aim is to permit QEMU to be used as a module within a
larger simulation system.

v6 -> v7 Added a check for command line arguments that are consistent
with -qqq and print an error message if incompatible arguments are
provide. [Paolo]

v7 -> v8 Updated patch to track changes in the main branch of qemu
development.

Signed-off-by: James J. Nutaro 
---
 Makefile.target  |   3 +
 cpus.c   |   8 +++
 docs/simulation-sync.txt |  59 
 include/sysemu/cpus.h|   1 +
 kvm-all.c|  10 +++
 qemu-options.hx  |  15 
 qqq.c| 177 +++
 qqq.h|  37 ++
 vl.c |  35 ++
 9 files changed, 345 insertions(+)
 create mode 100644 docs/simulation-sync.txt
 create mode 100644 qqq.c
 create mode 100644 qqq.h

diff --git a/Makefile.target b/Makefile.target
index 924304c..19703ca 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -149,6 +149,9 @@ obj-y += dump.o
 obj-y += migration/ram.o migration/savevm.o
 LIBS := $(libs_softmmu) $(LIBS)
 
+# qqq support
+obj-y += qqq.o
+
 # xen support
 obj-$(CONFIG_XEN) += xen-common.o
 obj-$(CONFIG_XEN_I386) += xen-hvm.o xen-mapcache.o
diff --git a/cpus.c b/cpus.c
index 71a82e5..dd659f1 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1765,3 +1765,11 @@ void dump_drift_info(FILE *f, fprintf_function 
cpu_fprintf)
 cpu_fprintf(f, "Max guest advance   NA\n");
 }
 }
+
+void kick_all_vcpus(void)
+{
+CPUState *cpu;
+CPU_FOREACH(cpu) {
+qemu_cpu_kick(cpu);
+}
+}
diff --git a/docs/simulation-sync.txt b/docs/simulation-sync.txt
new file mode 100644
index 000..de4dd34
--- /dev/null
+++ b/docs/simulation-sync.txt
@@ -0,0 +1,59 @@
+= Synchronizing the virtual clock with an external source =
+
+QEMU has a protocol for synchronizing its virtual clock
+with the clock of a simulator in which QEMU is embedded
+as a component. This options is enabled with the -qqq
+argument, and it should generally be accompanied by the
+following additional command line arguments:
+
+-icount 1,sleep=off -rtc clock=vm
+  or
+-enable-kvm -rtc clock=vm
+
+The -qqq argument is used to supply a file descriptor
+for a Unix socket, which is used for synchronization.
+The procedure for launching QEMU in is synchronization
+mode has three steps:
+
+(1) Create a socket pair with the Linux socketpair function.
+The code segment that does this might look like
+
+   int socks[2];
+   socketpair(AF_UNIX,SOCK_STREAM,0,socks);
+
+(2) Fork QEMU with the appropriate command line arguments.
+The -qqq part of the argument will look something like
+
+   -qqq sock=socks[1]
+
+(3) After forking QEMU, close sock[1] and retain the
+sock[0] for communicating with QEMU.
+
+The synchronization protocol is very simple. To start, the
+external simulator writes an integer to its socket with
+the amount of time in microseconds that QEMU is allowed to
+advance. The code segment that does this might look like:
+
+uint32_t ta = htonl(1000); // Advance by 1 millisecond
+write(sock[0],,sizeof(uint32_t));
+
+The external simulator can then advance its clock by this
+same amount. During this time, QEMU and the external simulator
+will be executing in parallel. When the external simulator
+completes its time advance, it waits for QEMU by reading from
+its socket. The value read will be the actual number of
+virtual microseconds by which QEMU has advanced its virtual clock.
+This will be greater than or equal to the requested advance.
+The code that does this might look like:
+
+   uint32_t ta;
+   read(fd,,sizeof(uint32_t));
+   ta = ntohl(ta);
+
+These steps are repeated until either (1) the external simulator
+closes its socket thereby causing QEMU to terminate or (2) QEMU
+stops executing (e.g., if the emulated computer is shutdown) and
+causes a read or write error on the simulator's socket.
+
+You can find an example of a simulator using this protocol in
+the adevs simulation package at http://sourceforge.net/projects/adevs/
diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index 3728a1e..bdf22c9 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -2,6 +2,7 @@
 #define QEMU_CPUS_H
 
 /* cpus.c */
+void kick_all_vcpus(void);
 bool qemu_in_vcpu_thread(void);
 void qemu_init_cpu_loop(void);
 void resume_all_vcpus(void);
diff --git a/kvm-all.c b/kvm-all.c
index a27c880..b361cb5 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -18,6 +18,7 @@
 
 #include 
 
+#include "qqq.h"
 #include "qemu-common.h"
 #include "qemu/atomic.h"
 #include "qemu/option.h"
@@ -1926,6 +1927,15 @@ int kvm_cpu_exec(CPUState *cpu)
 qemu_cpu_kick_self();
 }
 
+if (qqq_enabled()) {
+/* Pause here while qqq is synchronizing with a simulation clock.
+ * We do not want 

Re: [Qemu-devel] [PATCH v6 2/2] block/vxhs.c: Add qemu-iotests for new block device type "vxhs"

[ashish mittal]

On Tue, Feb 14, 2017 at 10:12 AM, Jeff Cody  wrote:
> On Mon, Nov 07, 2016 at 04:59:45PM -0800, Ashish Mittal wrote:
>> These changes use a vxhs test server that is a part of the following
>> repository:
>> https://github.com/MittalAshish/libqnio.git
>>
>> Signed-off-by: Ashish Mittal 
>> ---
>> v6 changelog:
>> (1) Added iotests for VxHS block device.
>>
>>  tests/qemu-iotests/common|  6 ++
>>  tests/qemu-iotests/common.config | 13 +
>>  tests/qemu-iotests/common.filter |  1 +
>>  tests/qemu-iotests/common.rc | 19 +++
>>  4 files changed, 39 insertions(+)
>>
>
> [...]
>
>> diff --git a/tests/qemu-iotests/common.filter 
>> b/tests/qemu-iotests/common.filter
>> index 240ed06..a8a4d0e 100644
>> --- a/tests/qemu-iotests/common.filter
>> +++ b/tests/qemu-iotests/common.filter
>> @@ -123,6 +123,7 @@ _filter_img_info()
>>  -e "s#$TEST_DIR#TEST_DIR#g" \
>>  -e "s#$IMGFMT#IMGFMT#g" \
>>  -e 's#nbd://127.0.0.1:10810$#TEST_DIR/t.IMGFMT#g' \
>> +-e 's#json.*vdisk-id.*vxhs"}}#TEST_DIR/t.IMGFMT#' \
>>  -e "/encrypted: yes/d" \
>>  -e "/cluster_size: [0-9]\\+/d" \
>>  -e "/table_size: [0-9]\\+/d" \
>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>> index 3213765..06a3164 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -89,6 +89,9 @@ else
>>  TEST_IMG=$TEST_DIR/t.$IMGFMT
>>  elif [ "$IMGPROTO" = "archipelago" ]; then
>>  TEST_IMG="archipelago:at.$IMGFMT"
>> +elif [ "$IMGPROTO" = "vxhs" ]; then
>> +TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
>> +TEST_IMG="vxhs://127.0.0.1:/t.$IMGFMT"
>>  else
>>  TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
>>  fi
>> @@ -175,6 +178,12 @@ _make_test_img()
>>  eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT  
>> $TEST_IMG_FILE &"
>>  sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
>>  fi
>> +
>> +# Start QNIO server on image directory for vxhs protocol
>> +if [ $IMGPROTO = "vxhs" ]; then
>> +eval "$QEMU_VXHS -d  $TEST_DIR &"
>
> So I spoke too soon about tests passing, but it is not really your fault :)
>
> After rebasing to master, there is a new test 174 that now hangs (and hangs
> for nbd, as well).  This is because the test is piping the results of
> _make_test_image to sed to filter it.
>
> This line should redirect stdout to /dev/null, so that the pipe does not
> need to wait until process completion:
>
> eval "$QEMU_VXHS -d  $TEST_DIR > /dev/null &"
>
>

Will make this change in the next series. Thanks again!

Re: [Qemu-devel] [PATCH 11/24] block: introduce persistent dirty bitmaps

[John Snow]

On 02/14/2017 07:05 AM, Vladimir Sementsov-Ogievskiy wrote:
> 11.02.2017 02:20, John Snow wrote:
>> On 02/03/2017 04:40 AM, Vladimir Sementsov-Ogievskiy wrote:

>>> +void bdrv_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error
>>> **errp)
>>> +{
>>> +BlockDriver *drv = bs->drv;
>>> +
>>> +if (!bdrv_has_persistent_bitmaps(bs)) {
>>> +return;
>>> +}
>>> +
>>> +if (!drv) {
>>> +error_setg_errno(errp, ENOMEDIUM,
>>> + "Can't store persistent bitmaps to %s",
>>> + bdrv_get_device_or_node_name(bs));
>>> +return;
>>> +}
>>> +
>>> +if (!drv->bdrv_store_persistent_dirty_bitmaps) {
>>> +error_setg_errno(errp, ENOTSUP,
>>> + "Can't store persistent bitmaps to %s",
>>> + bdrv_get_device_or_node_name(bs));
>>> +return;
>>> +}
>>> +
>> I suppose this is for the case for where we have added a persistent
>> bitmap during runtime, but the driver does not support it?
>>
>> I'd rather fail this type of thing earlier if possible, but I'm not that
>> far in your series yet.
> 
> qmp bitmap add checks for availability of
> drv->bdrv_can_store_new_dirty_bitmap,
> and loaded bitmaps of course should be attached to bds with appropriate
> driver.
> So, it is mostly a paranoic check.
> 

OK, understood. Not a problem, then.

--js

Re: [Qemu-devel] [PATCH v8 0/2] Qemu: gdbstub: fix vCont

[Paolo Bonzini]

Thanks, I hope to send a pull request this week, including this patch.

Paolo

On 14/02/2017 18:07, Claudio Imbrenda wrote:
> This small patchset fixes the incorrect behaviour of the vCont command
> in the gdb stub. 
> 
> The first patch, as suggested be Paolo, refactors some code. The most
> visible change is that it moves vm_start to cpus.c 
> 
> The second one fixes the incorrect behaviour of the vCont command.
> Previously, continuing or stepping a single thread (CPU) caused all
> other CPUs to be started too, whereas the GDB specification clearly
> states that without a default action all threads not explicitly
> mentioned in the command should stay stopped.
> 
> So if the Qemu gdbstub receives a  vCont;c:1  packet, no other CPU
> should be restarted except the first, and when a  vCont;s:1  is
> received, the first CPU should be stepped without restarting the others.
> With this patchset Qemu now behaves as expected.
> 
> See here for reference material about the packets: 
> https://sourceware.org/gdb/current/onlinedocs/gdb/Packets.html
> https://sourceware.org/gdb/onlinedocs/gdb/Packets.html
> 
> v7 -> v8
> * fixed and added some comments
> * fixed vCont a little bit for user mode
> * use cpu->cpu_index directly when possible
> 
> v6 -> v7
> * fixed description of patch 1 to reflect what is actually happening
>   and improved description of patch 2
> * removed leftover header declaration of resume_some_vcpus which had
>   been removed a few versions ago
> * fixed a compilation issue when compiling userspace-mode only
>   (global variable max_cpus is not defined when not in system-mode)
> 
> v4 -> v6
> * rebased on master after target-s390x was moved
> * put qemu_clock_enable back into resume_all_vcpus
> * improved the parsing function of the vCont packet
> * added qemu_clock_enable to gdb_continue_partial
> 
> v3 -> v4
> * rebased on v2.8.0-rc2, no changes needed
> 
> v2 -> v3
> * removed resume_some_vcpus
> * cleared up the code and simplified the implementation in light of the 
>   clarification in the specification of the vCont packet
> 
> Claudio Imbrenda (2):
>   move vm_start to cpus.c
>   gdbstub: Fix vCont behaviour
> 
>  cpus.c  |  42 ++
>  gdbstub.c   | 209 
> +---
>  include/sysemu/sysemu.h |   2 +
>  vl.c|  30 +--
>  4 files changed, 207 insertions(+), 76 deletions(-)
> 

[Qemu-devel] [PATCH v8 1/2] move vm_start to cpus.c

[Claudio Imbrenda]

This patch:

* moves vm_start to cpus.c.
* exports qemu_vmstop_requested, since it's needed by vm_start.
* extracts vm_prepare_start from vm_start; it does what vm_start did,
  except restarting the cpus.
* vm_start now calls vm_prepare_start and then restarts the cpus.

Signed-off-by: Claudio Imbrenda 
---
 cpus.c  | 42 ++
 include/sysemu/sysemu.h |  2 ++
 vl.c| 30 +-
 3 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/cpus.c b/cpus.c
index 71a82e5..0bcb5b5 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1578,6 +1578,48 @@ int vm_stop(RunState state)
 return do_vm_stop(state);
 }
 
+/**
+ * Prepare for (re)starting the VM.
+ * Returns -1 if the vCPUs are not to be restarted (e.g. if they are already
+ * running or in case of an error condition), 0 otherwise.
+ */
+int vm_prepare_start(void)
+{
+RunState requested;
+int res = 0;
+
+qemu_vmstop_requested();
+if (runstate_is_running() && requested == RUN_STATE__MAX) {
+return -1;
+}
+
+/* Ensure that a STOP/RESUME pair of events is emitted if a
+ * vmstop request was pending.  The BLOCK_IO_ERROR event, for
+ * example, according to documentation is always followed by
+ * the STOP event.
+ */
+if (runstate_is_running()) {
+qapi_event_send_stop(_abort);
+res = -1;
+} else {
+replay_enable_events();
+cpu_enable_ticks();
+runstate_set(RUN_STATE_RUNNING);
+vm_state_notify(1, RUN_STATE_RUNNING);
+}
+
+/* We are sending this now, but the CPUs will be resumed shortly later */
+qapi_event_send_resume(_abort);
+return res;
+}
+
+void vm_start(void)
+{
+if (!vm_prepare_start()) {
+resume_all_vcpus();
+}
+}
+
 /* does a state transition even if the VM is already stopped,
current state is forgotten forever */
 int vm_stop_force_state(RunState state)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ff8ffb5..0bb0c40 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -37,6 +37,7 @@ void vm_state_notify(int running, RunState state);
 #define VMRESET_REPORT   true
 
 void vm_start(void);
+int vm_prepare_start(void);
 int vm_stop(RunState state);
 int vm_stop_force_state(RunState state);
 
@@ -60,6 +61,7 @@ void qemu_register_powerdown_notifier(Notifier *notifier);
 void qemu_system_debug_request(void);
 void qemu_system_vmstop_request(RunState reason);
 void qemu_system_vmstop_request_prepare(void);
+bool qemu_vmstop_requested(RunState *r);
 int qemu_shutdown_requested_get(void);
 int qemu_reset_requested_get(void);
 void qemu_system_killed(int signal, pid_t pid);
diff --git a/vl.c b/vl.c
index abb0900..7a32a30 100644
--- a/vl.c
+++ b/vl.c
@@ -757,7 +757,7 @@ StatusInfo *qmp_query_status(Error **errp)
 return info;
 }
 
-static bool qemu_vmstop_requested(RunState *r)
+bool qemu_vmstop_requested(RunState *r)
 {
 qemu_mutex_lock(_lock);
 *r = vmstop_requested;
@@ -778,34 +778,6 @@ void qemu_system_vmstop_request(RunState state)
 qemu_notify_event();
 }
 
-void vm_start(void)
-{
-RunState requested;
-
-qemu_vmstop_requested();
-if (runstate_is_running() && requested == RUN_STATE__MAX) {
-return;
-}
-
-/* Ensure that a STOP/RESUME pair of events is emitted if a
- * vmstop request was pending.  The BLOCK_IO_ERROR event, for
- * example, according to documentation is always followed by
- * the STOP event.
- */
-if (runstate_is_running()) {
-qapi_event_send_stop(_abort);
-} else {
-replay_enable_events();
-cpu_enable_ticks();
-runstate_set(RUN_STATE_RUNNING);
-vm_state_notify(1, RUN_STATE_RUNNING);
-resume_all_vcpus();
-}
-
-qapi_event_send_resume(_abort);
-}
-
-
 /***/
 /* real time host monotonic timer */
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH 2/9] armv7m: Implement reading and writing of PRIGROUP

[Alex Bennée]

Peter Maydell  writes:

> Add a state field for the v7M PRIGROUP register and implent
> reading and writing it. The current NVIC doesn't honour
> the values written, but the new version will.
>
> Signed-off-by: Peter Maydell 
> ---
>  hw/intc/armv7m_nvic.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index 09975f3..ce22001 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -24,6 +24,9 @@
>  typedef struct NVICState {
>  GICState gic;
>  ARMCPU *cpu;
> +
> +uint32_t prigroup;

It might be worth mentioning the field resets to 0b000 here.
Nevertheless:

Reviewed-by: Alex Bennée 

> +
>  struct {
>  uint32_t control;
>  uint32_t reload;
> @@ -223,7 +226,7 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
>  case 0xd08: /* Vector Table Offset.  */
>  return cpu->env.v7m.vecbase;
>  case 0xd0c: /* Application Interrupt/Reset Control.  */
> -return 0xfa05;
> +return 0xfa05 | (s->prigroup << 8);
>  case 0xd10: /* System Control.  */
>  /* TODO: Implement SLEEPONEXIT.  */
>  return 0;
> @@ -362,9 +365,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
> uint32_t value)
>  if (value & 1) {
>  qemu_log_mask(LOG_UNIMP, "AIRCR system reset 
> unimplemented\n");
>  }
> -if (value & 0x700) {
> -qemu_log_mask(LOG_UNIMP, "PRIGROUP unimplemented\n");
> -}
> +s->prigroup = extract32(value, 8, 3);
>  }
>  break;
>  case 0xd10: /* System Control.  */
> @@ -483,13 +484,14 @@ static const MemoryRegionOps nvic_sysreg_ops = {
>
>  static const VMStateDescription vmstate_nvic = {
>  .name = "armv7m_nvic",
> -.version_id = 1,
> -.minimum_version_id = 1,
> +.version_id = 2,
> +.minimum_version_id = 2,
>  .fields = (VMStateField[]) {
>  VMSTATE_UINT32(systick.control, NVICState),
>  VMSTATE_UINT32(systick.reload, NVICState),
>  VMSTATE_INT64(systick.tick, NVICState),
>  VMSTATE_TIMER_PTR(systick.timer, NVICState),
> +VMSTATE_UINT32(prigroup, NVICState),
>  VMSTATE_END_OF_LIST()
>  }
>  };


--
Alex Bennée

[Qemu-devel] [Bug 886255] Re: Qemu master branch - RHEL 6.1 guest fails to boot

[Thomas Huth]

Marking as fixed, according to comment #5

** Changed in: qemu
   Status: New => Fix Released

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/886255

Title:
  Qemu master branch - RHEL 6.1 guest fails to boot

Status in QEMU:
  Fix Released

Bug description:
  Guest: RHEL 6.1 64 bit DVD

  Kernel: Latest Fedora, also reproduces with Avi's kvm.git kernel based
  on 3.1: 2.6.40.6-0.fc15.x86_64

  qemu version:

  11/04 00:25:30 DEBUG|virt_utils:2587| Git repo qemu uri: 
git://git.qemu.org/qemu.git
  11/04 00:25:30 DEBUG|virt_utils:2590| Git repo qemu branch: master
  11/04 00:25:30 DEBUG|virt_utils:3189| Configure options for git_repo_qemu: 
['--target-list=x86_64-softmmu']
  11/04 00:25:30 DEBUG|virt_utils:2496| Initializing new git repo at 
/usr/local/autotest/tests/kvm/src/qemu for receiving git repo 11/04 00:25:30 
INFO |virt_utils:2505| Fetching git [REP 'git://git.qemu.org/qemu.git' BRANCH 
'master'] -> /usr/local/autotest/tests/kvm/src/qemu
  11/04 00:25:30 DEBUG|base_utils:0074| Running 'git fetch -q -f -u -t 
git://git.qemu.org/qemu.git master:master'
  11/04 00:34:41 INFO |virt_utils:2531| Commit hash for qemu is 
932eacc158c064935c7bab920c88a93a629e1ca4 (no tag found)

  On guest boot screenshots (one of them attached), you can see the
  message

  "Bringing up interface eth0: Device eth0 does not seem to be present,
  delaying initialization"

  Network card info
  11/04 00:44:55 DEBUG| virt_test:0041| bridge = virbr0
  11/04 00:44:55 DEBUG| virt_test:0041| nic_mode = tap
  11/04 00:44:55 DEBUG| virt_test:0041| nic_model = virtio

  Commented excerpt of the test log:

  11/04 00:44:57 INFO |kvm_vm:0790| Running qemu command:
  /usr/local/autotest/tests/kvm/qemu -name 'vm1' -nodefaults -vga std -monitor 
unix:'/tmp/monitor-humanmonitor1-2004-003602-LPJY',server,nowait -qmp 
unix:'/tmp/monitor-qmpmonitor1-2004-003602-LPJY',server,nowait -serial 
unix:'/tmp/serial-2004-003602-LPJY',server,nowait -drive 
file='/tmp/kvm_autotest_root/images/rhel6.1-64.qcow2',index=0,if=virtio,cache=none
 -device virtio-net-pci,netdev=id3HQgQx,mac='9a:16:a5:3c:05:25',id='id0AfUVE' 
-netdev tap,id=id3HQgQx,fd=23 -m 2048 -smp 2 -vnc :0  -enable-kvm
  11/04 00:44:59 DEBUG|kvm_monito:0624| (monitor qmpmonitor1) Sending command 
'qmp_capabilities'
  11/04 00:44:59 DEBUG|kvm_vm:0851| VM appears to be alive with PID 9827
  11/04 00:44:59 DEBUG|virt_env_p:0318| Starting screendump thread
  11/04 00:44:59 DEBUG|   virt_vm:0654| Attempting to log into 'vm1' (timeout 
720s)

  ... here it starts booting ...

  11/04 00:44:59 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25
  11/04 00:45:01 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25
  11/04 00:45:03 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25
  11/04 00:45:05 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25
  11/04 00:45:07 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25
  11/04 00:45:09 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25
  11/04 00:45:11 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25
  11/04 00:45:13 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25
  11/04 00:45:15 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25
  11/04 00:45:17 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25
  11/04 00:45:19 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25
  11/04 00:45:21 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25
  11/04 00:45:23 DEBUG|   virt_vm:0660| Cannot find IP address for MAC address 
9a:16:a5:3c:05:25

  ... here it gets an IP from the DHCP server ...

  11/04 00:45:24 DEBUG|virt_env_p:0438| (address cache) Adding cache
  entry: 9a:16:a5:3c:05:25 ---> 192.168.122.195

  ... ok, let's try to login ...

  11/04 00:45:25 DEBUG|virt_utils:0710| Trying to login with command
  'ssh -o UserKnownHostsFile=/dev/null -o
  PreferredAuthentications=password -p 22 root@192.168.122.195'

  ... ouch, not possible to login ...

  11/04 00:45:25 DEBUG|   virt_vm:0660| Client said 'connection refused'
  (output: 'ssh: connect to host 192.168.122.195 port 22: Connection
  refused\n')

  ... message above repeats until ...

  11/04 00:56:59 ERROR| virt_test:0095| Test failed: LoginError: Client said 
'connection refused'(output: 'ssh: connect to host 192.168.122.195 port 22: 
Connection refused\n')
  11/04 00:56:59 DEBUG|virt_env_p:0147| Postprocessing VM 'vm1'
  11/04 00:56:59 DEBUG|virt_env_p:0166| Param 'kill_vm' specified, killing VM
  11/04 00:56:59 DEBUG|kvm_vm:0885| Destroying VM with PID 9827
  11/04 00:56:59 DEBUG|kvm_vm:0889| Trying to 

[Qemu-devel] [PATCH v14 00/24] qcow2: persistent dirty bitmaps

[Vladimir Sementsov-Ogievskiy]

Hi all!

There is a new update of qcow2-bitmap series - v14.

web: 
https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=qcow2-bitmap-v14
git: https://src.openvz.org/scm/~vsementsov/qemu.git (tag qcow2-bitmap-v14)

v14:

07: use '|=' to update need_update_header
add John's r-b
add Max's r-b
09: remove unused bitmap_table_to_cpu()
left Max's r-b, hope it's ok
add John's r-b
10: remove extra new line
add John's r-b
11: add John's r-b
12: add John's r-b
13: small fixes by John's review:
   - remove weird g_free of NULL pointer from
   if (tb == NULL) {
   g_free(tb);
   return -EINVAL;
   }
   - remove extra comment "/* errp is already set */"
   - s/"Too large bitmap directory"/"Bitmap directory is too large"/
left Max's r-b, hope you don't mind 
22: add Max's r-b
23: add Max's r-b
24: add Max's r-b
25: new patch to improve error message on check_constraints_on_bitmap fail


v13: Just a fix for style checker.
13: line over 80
14: line over 80
22: s/if () \n{/if () {/

v12:
07: do not update header in qcow2_read_extensions, instead do it in the
end of qcow2_open, where it is updated also to clear unknown
autoclear features.
Wrong bdrv_is_root_node used instead of bdrv_is_read_only is fixed
automatically.
08: add Max's r-b
09: contextual: s/raw_bsd/raw-format/
qcow2-bitmap.c: copyright s/2016/2017/
fix compilation: move update_header_sync() to this patch
add Max's r-b
13: update_header_sync() is moved to 09
add Max's r-b
15: add Max's r-b
16: As qmp-commands.txt is removed from master, remove it here too.
Sentence "For now only Qcow2 disks support persistent bitmaps.
 Default is false." moved to qapi/block-core.json.
Hope that is OK, Max's r-b is not dropped.
17: qmp-commands.txt deleted, r-b is not dropped too.
18: s/is failed/has failed/, add Max's r-b
19: copyright: s/2016/2017/
21: drop "res->corruptions > 0 => ret = -EINVAL", add Max's r-b
22: actually, patch is replaced with a new one, however some parts are the same.
instead of changing bdrv_release_dirty_bitmap behaviour, create new
bdrv_remove_persistent_dirty_bitmap
23: improve comment, about not-exists is not an error
24: new patch

Old 24 (qcow2-bitmap: cache bitmap list in BDRVQcow2State) will be sent
separately, to be applied after these series.

Also: about bdrv_dirty_bitmap_set_persistance: I've decided not change its 
behaviour
for now and just call bdrv_remove_persistent_dirty_bitmap from
qmp_block_dirty_bitmap_remove. Reasons:
1. Do not change reviewed part.
2. I'm not sure, that this complication of BdrvDirtyBitmap.persistent semantics
   is good (current semantics are just: .persistent means that bitmap should be
   saved on disk close). .persistent actually is not very related to "is there 
   stored version of the bitmap in the image".
3. It may be done later.
4. It is not actually needed for now, as the only usage is dropping bitmap in
   bdrv_remove_persistent_dirty_bitmap. May be in future it will be needed,
   when we will have qmp interfaces for finer control of persistent dirty 
bitmaps.


v11:

Fix automatic build test fail.

18: wording - "ASCII representation of SHA256 ..."
24: fix redeclaration error. (This is still RFC, may be not needed patch,
see description in v10 below).

v10:

07: rm John's r-b
not add Max's r-b
fix subject s/dirty bitmaps/bitmaps
improve WARNING message
remove header ext if autoclear bit is unset, through qcow2_updata_header() 
- r-b blocking change
08: move bdrv_load_autoloading_dirty_bitmaps under asserts block (Max)
fix typo in commit message
move bdrv_load_autoloading_dirty_bitmaps after asserts
John's r-b
09: rewrite check_dir_entry
remove extra feature cluster_size=0 from check_table_entry
which clears autoclear bit before trying to update bitmap directory
bitmap_list_store - do not free old clusters if in_place=true
changes in comments, fix indents
add functions
  bitmap_list_count
  update_ext_header_and_dir_in_place (unset autoclear bit before trying to
  modify bitmap directory)
   (for usage in qcow2_load_autoloading_dirty_bitmaps)
11: add node name to error report
Max's r-b
13: add type Qcow2BitmapTable
move from bm.table_* to bm.table.*
rewrite check_constraints_on_bitmap
flush(bs->file) instead of flush(bs) in update_ext_header_and_dir
assert(DIV_ROUND_UP(bm_size, dsc) == tb_size);
  and assert(write_size <= s->cluster_size); in store_bitmap_data
fix: s/tb_size/tb_size * sizeof(tb[0]) in store_bitmap
in qcow2_store_persistent_dirty_bitmaps:
  fail if !can_write 
  fix counting of new_nb_bitmaps and new_dir_size
  free old clusters _after_ successful directory update (aggregate old 
bitmap
tables into drop_tables)
2 14: rename to can_store_new_dirty_bitmap
Max's r-b
15: rename to 

Re: [Qemu-devel] [PATCH v6 2/2] block/vxhs.c: Add qemu-iotests for new block device type "vxhs"

[Jeff Cody]

On Tue, Feb 14, 2017 at 04:51:09PM +, Daniel P. Berrange wrote:
> On Tue, Feb 14, 2017 at 11:35:17AM -0500, Jeff Cody wrote:
> > From 7c135439fd0151860cb0f6ef1a857dfbee6e6317 Mon Sep 17 00:00:00 2001
> > From: Jeff Cody 
> > Date: Tue, 14 Feb 2017 09:51:42 -0500
> > Subject: [PATCH] qemu-iotests: exclude vxhs from image creation via protocol
> > 
> > The protocol VXHS does not support image creation.  Some tests expect
> > to be able to create images through the protocol.  Exclude VXHS from
> > these tests.
> 
> [snip]
> 
> > diff --git a/tests/qemu-iotests/017 b/tests/qemu-iotests/017
> > index e3f9e75..4f9302d 100755
> > --- a/tests/qemu-iotests/017
> > +++ b/tests/qemu-iotests/017
> > @@ -41,6 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> >  # Any format supporting backing files
> >  _supported_fmt qcow qcow2 vmdk qed
> >  _supported_proto generic
> > +_unsupported_proto vxhs
> >  _supported_os Linux
> >  _unsupported_imgopts "subformat=monolithicFlat" 
> > "subformat=twoGbMaxExtentFlat"
> 
> I've no objection to your patch as is, just a thought for future
> improvements.
> 
> There's quite a few block protos that doen't support image
> creation. Rather than listing protocols in _unsupported_proto
> it would be more scalable if we could list features. eg if
> each test had
> 
>   _supported_feature imagecreate
> 
> Then, the tests/qemu-iotests/common file could set the list
> of features supported by each protocol/format. Thus avoiding
> the need to update all iotests when adding new protocols. We
> could have features for "encryption", and "snapshots" and
> "backing_files", etc too
>

I like that approach, good idea.

-Jeff

[Qemu-devel] [PATCH 11/25] block: introduce persistent dirty bitmaps

[Vladimir Sementsov-Ogievskiy]

New field BdrvDirtyBitmap.persistent means, that bitmap should be saved
on bdrv_close, using format driver. Format driver should maintain bitmap
storing.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 block.c  | 32 
 block/dirty-bitmap.c | 26 ++
 block/qcow2-bitmap.c |  1 +
 include/block/block.h|  1 +
 include/block/block_int.h|  2 ++
 include/block/dirty-bitmap.h |  6 ++
 6 files changed, 68 insertions(+)

diff --git a/block.c b/block.c
index 56f030c..970e4ca 100644
--- a/block.c
+++ b/block.c
@@ -2321,6 +2321,7 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
 static void bdrv_close(BlockDriverState *bs)
 {
 BdrvAioNotifier *ban, *ban_next;
+Error *local_err = NULL;
 
 assert(!bs->job);
 assert(!bs->refcnt);
@@ -2329,6 +2330,12 @@ static void bdrv_close(BlockDriverState *bs)
 bdrv_flush(bs);
 bdrv_drain(bs); /* in case flush left pending I/O */
 
+bdrv_store_persistent_dirty_bitmaps(bs, _err);
+if (local_err != NULL) {
+error_report_err(local_err);
+error_report("Persistent bitmaps are lost for node '%s'",
+ bdrv_get_device_or_node_name(bs));
+}
 bdrv_release_named_dirty_bitmaps(bs);
 assert(QLIST_EMPTY(>dirty_bitmaps));
 
@@ -4107,3 +4114,28 @@ void 
bdrv_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp)
 bs->drv->bdrv_load_autoloading_dirty_bitmaps(bs, errp);
 }
 }
+
+void bdrv_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
+{
+BlockDriver *drv = bs->drv;
+
+if (!bdrv_has_persistent_bitmaps(bs)) {
+return;
+}
+
+if (!drv) {
+error_setg_errno(errp, ENOMEDIUM,
+ "Can't store persistent bitmaps to %s",
+ bdrv_get_device_or_node_name(bs));
+return;
+}
+
+if (!drv->bdrv_store_persistent_dirty_bitmaps) {
+error_setg_errno(errp, ENOTSUP,
+ "Can't store persistent bitmaps to %s",
+ bdrv_get_device_or_node_name(bs));
+return;
+}
+
+drv->bdrv_store_persistent_dirty_bitmaps(bs, errp);
+}
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index a9dfce8..d2fbf55 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -44,6 +44,7 @@ struct BdrvDirtyBitmap {
 int64_t size;   /* Size of the bitmap (Number of sectors) */
 bool disabled;  /* Bitmap is read-only */
 int active_iterators;   /* How many iterators are active */
+bool persistent;/* bitmap must be saved to owner disk image */
 bool autoload;  /* For persistent bitmaps: bitmap must be
autoloaded on image opening */
 QLIST_ENTRY(BdrvDirtyBitmap) list;
@@ -72,6 +73,7 @@ void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
 assert(!bdrv_dirty_bitmap_frozen(bitmap));
 g_free(bitmap->name);
 bitmap->name = NULL;
+bitmap->persistent = false;
 bitmap->autoload = false;
 }
 
@@ -241,6 +243,8 @@ BdrvDirtyBitmap 
*bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
 bitmap->name = NULL;
 successor->name = name;
 bitmap->successor = NULL;
+successor->persistent = bitmap->persistent;
+bitmap->persistent = false;
 successor->autoload = bitmap->autoload;
 bitmap->autoload = false;
 bdrv_release_dirty_bitmap(bs, bitmap);
@@ -555,3 +559,25 @@ bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap 
*bitmap)
 {
 return bitmap->autoload;
 }
+
+void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap, bool 
persistent)
+{
+bitmap->persistent = persistent;
+}
+
+bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap)
+{
+return bitmap->persistent;
+}
+
+bool bdrv_has_persistent_bitmaps(BlockDriverState *bs)
+{
+BdrvDirtyBitmap *bm;
+QLIST_FOREACH(bm, >dirty_bitmaps, list) {
+if (bm->persistent) {
+return true;
+}
+}
+
+return false;
+}
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index f1ca799..1b5ae2d 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -698,6 +698,7 @@ void qcow2_load_autoloading_dirty_bitmaps(BlockDriverState 
*bs, Error **errp)
 goto fail;
 }
 
+bdrv_dirty_bitmap_set_persistance(bitmap, true);
 bdrv_dirty_bitmap_set_autoload(bitmap, true);
 bm->flags |= BME_FLAG_IN_USE;
 created_dirty_bitmaps =
diff --git a/include/block/block.h b/include/block/block.h
index 154ac7f..0a20d68 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -552,5 +552,6 @@ void bdrv_add_child(BlockDriverState *parent, 
BlockDriverState *child,
 void bdrv_del_child(BlockDriverState *parent, BdrvChild *child, 

[Qemu-devel] [PATCH 22/25] block/dirty-bitmap: add bdrv_remove_persistent_dirty_bitmap

[Vladimir Sementsov-Ogievskiy]

Interface for removing persistent bitmap from its storage.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
---
 block/dirty-bitmap.c | 18 ++
 include/block/block_int.h|  3 +++
 include/block/dirty-bitmap.h |  3 +++
 3 files changed, 24 insertions(+)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index b684d8b..05e0aae 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -327,12 +327,30 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap)
 /**
  * Release all named dirty bitmaps attached to a BDS (for use in bdrv_close()).
  * There must not be any frozen bitmaps attached.
+ * This function does not remove persistent bitmaps from the storage.
  */
 void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs)
 {
 bdrv_do_release_matching_dirty_bitmap(bs, NULL, true);
 }
 
+/**
+ * Remove persistent dirty bitmap from the storage if it exists.
+ * Absence of bitmap is not an error, because we have the following scenario:
+ * BdrvDirtyBitmap can have .persistent = true but not yet saved and have no
+ * stored version. For such bitmap bdrv_remove_persistent_dirty_bitmap() should
+ * not fail.
+ * This function doesn't release corresponding BdrvDirtyBitmap.
+ */
+void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ Error **errp)
+{
+if (bs->drv && bs->drv->bdrv_remove_persistent_dirty_bitmap) {
+bs->drv->bdrv_remove_persistent_dirty_bitmap(bs, name, errp);
+}
+}
+
 void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 {
 assert(!bdrv_dirty_bitmap_frozen(bitmap));
diff --git a/include/block/block_int.h b/include/block/block_int.h
index db68067..d138555 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -328,6 +328,9 @@ struct BlockDriver {
 const char *name,
 uint32_t granularity,
 Error **errp);
+void (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs,
+const char *name,
+Error **errp);
 
 QLIST_ENTRY(BlockDriver) list;
 };
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index b022b34..ce12610 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -25,6 +25,9 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
 void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs);
+void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ Error **errp);
 void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
-- 
1.8.3.1

[Qemu-devel] [PATCH 10/25] block/dirty-bitmap: add autoload field to BdrvDirtyBitmap

[Vladimir Sementsov-Ogievskiy]

Mirror AUTO flag from Qcow2 bitmap in BdrvDirtyBitmap. This will be
needed in future, to save this flag back to Qcow2 for persistent
bitmaps.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 block/dirty-bitmap.c | 15 +++
 block/qcow2-bitmap.c |  2 ++
 include/block/dirty-bitmap.h |  2 ++
 3 files changed, 19 insertions(+)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 90af372..a9dfce8 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -44,6 +44,8 @@ struct BdrvDirtyBitmap {
 int64_t size;   /* Size of the bitmap (Number of sectors) */
 bool disabled;  /* Bitmap is read-only */
 int active_iterators;   /* How many iterators are active */
+bool autoload;  /* For persistent bitmaps: bitmap must be
+   autoloaded on image opening */
 QLIST_ENTRY(BdrvDirtyBitmap) list;
 };
 
@@ -70,6 +72,7 @@ void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
 assert(!bdrv_dirty_bitmap_frozen(bitmap));
 g_free(bitmap->name);
 bitmap->name = NULL;
+bitmap->autoload = false;
 }
 
 BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
@@ -238,6 +241,8 @@ BdrvDirtyBitmap 
*bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
 bitmap->name = NULL;
 successor->name = name;
 bitmap->successor = NULL;
+successor->autoload = bitmap->autoload;
+bitmap->autoload = false;
 bdrv_release_dirty_bitmap(bs, bitmap);
 
 return successor;
@@ -540,3 +545,13 @@ int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap)
 {
 return hbitmap_count(bitmap->meta);
 }
+
+void bdrv_dirty_bitmap_set_autoload(BdrvDirtyBitmap *bitmap, bool autoload)
+{
+bitmap->autoload = autoload;
+}
+
+bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap)
+{
+return bitmap->autoload;
+}
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index e08e46e..f1ca799 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -697,6 +697,8 @@ void qcow2_load_autoloading_dirty_bitmaps(BlockDriverState 
*bs, Error **errp)
 if (bitmap == NULL) {
 goto fail;
 }
+
+bdrv_dirty_bitmap_set_autoload(bitmap, true);
 bm->flags |= BME_FLAG_IN_USE;
 created_dirty_bitmaps =
 g_slist_append(created_dirty_bitmaps, bitmap);
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 1e17729..45a389a 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -75,4 +75,6 @@ void bdrv_dirty_bitmap_deserialize_ones(BdrvDirtyBitmap 
*bitmap,
 bool finish);
 void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
 
+void bdrv_dirty_bitmap_set_autoload(BdrvDirtyBitmap *bitmap, bool autoload);
+bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 02/25] specs/qcow2: do not use wording 'bitmap header'

[Vladimir Sementsov-Ogievskiy]

A bitmap directory entry is sometimes called a 'bitmap header'. This
patch leaves only one name - 'bitmap directory entry'. The name 'bitmap
header' creates misunderstandings with 'qcow2 header' and 'qcow2 bitmap
header extension' (which is extension of qcow2 header)

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Eric Blake 
Reviewed-by: John Snow 
---
 docs/specs/qcow2.txt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index dda53dd..8874e8c 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -201,7 +201,7 @@ The fields of the bitmaps extension are:
 
   8 - 15:  bitmap_directory_size
Size of the bitmap directory in bytes. It is the cumulative
-   size of all (nb_bitmaps) bitmap headers.
+   size of all (nb_bitmaps) bitmap directory entries.
 
  16 - 23:  bitmap_directory_offset
Offset into the image file at which the bitmap directory
@@ -426,8 +426,7 @@ Each bitmap saved in the image is described in a bitmap 
directory entry. The
 bitmap directory is a contiguous area in the image file, whose starting offset
 and length are given by the header extension fields bitmap_directory_offset and
 bitmap_directory_size. The entries of the bitmap directory have variable
-length, depending on the lengths of the bitmap name and extra data. These
-entries are also called bitmap headers.
+length, depending on the lengths of the bitmap name and extra data.
 
 Structure of a bitmap directory entry:
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 15/25] qcow2: add .bdrv_can_store_new_dirty_bitmap

[Vladimir Sementsov-Ogievskiy]

Realize .bdrv_can_store_new_dirty_bitmap interface.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
---
 block/qcow2-bitmap.c | 40 
 block/qcow2.c|  1 +
 block/qcow2.h|  4 
 3 files changed, 45 insertions(+)

diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index b177a95..419d51e 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1182,3 +1182,43 @@ fail:
 
 bitmap_list_free(bm_list);
 }
+
+bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
+  const char *name,
+  uint32_t granularity,
+  Error **errp)
+{
+BDRVQcow2State *s = bs->opaque;
+const char *reason = NULL;
+bool found;
+Qcow2BitmapList *bm_list;
+
+if (check_constraints_on_bitmap(bs, name, granularity) != 0) {
+reason = "it doesn't satisfy the constraints";
+goto common_errp;
+}
+
+if (s->nb_bitmaps == 0) {
+return true;
+}
+
+bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
+   s->bitmap_directory_size, errp);
+if (bm_list == NULL) {
+return false;
+}
+
+found = find_bitmap_by_name(bm_list, name);
+bitmap_list_free(bm_list);
+if (found) {
+reason = "bitmap with the same name is already stored";
+goto common_errp;
+}
+
+return true;
+
+common_errp:
+error_setg(errp, "Can't make bitmap '%s' persistent in '%s', as %s.",
+   name, bdrv_get_device_or_node_name(bs), reason);
+return false;
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index d0e41bf..6e1fe53 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3541,6 +3541,7 @@ BlockDriver bdrv_qcow2 = {
 
 .bdrv_load_autoloading_dirty_bitmaps = 
qcow2_load_autoloading_dirty_bitmaps,
 .bdrv_store_persistent_dirty_bitmaps = 
qcow2_store_persistent_dirty_bitmaps,
+.bdrv_can_store_new_dirty_bitmap = qcow2_can_store_new_dirty_bitmap,
 };
 
 static void bdrv_qcow2_init(void)
diff --git a/block/qcow2.h b/block/qcow2.h
index d9a7643..749710d 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -616,5 +616,9 @@ void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, 
void **table);
 /* qcow2-bitmap.c functions */
 void qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp);
 void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp);
+bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
+  const char *name,
+  uint32_t granularity,
+  Error **errp);
 
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 06/25] block/dirty-bitmap: add deserialize_ones func

[Vladimir Sementsov-Ogievskiy]

Add bdrv_dirty_bitmap_deserialize_ones() function, which is needed for
qcow2 bitmap loading, to handle unallocated bitmap parts, marked as
all-ones.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Kevin Wolf 
Reviewed-by: John Snow 
---
 block/dirty-bitmap.c |  7 +++
 include/block/dirty-bitmap.h |  3 +++
 include/qemu/hbitmap.h   | 15 +++
 util/hbitmap.c   | 17 +
 4 files changed, 42 insertions(+)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 186941c..90af372 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -499,6 +499,13 @@ void bdrv_dirty_bitmap_deserialize_zeroes(BdrvDirtyBitmap 
*bitmap,
 hbitmap_deserialize_zeroes(bitmap->bitmap, start, count, finish);
 }
 
+void bdrv_dirty_bitmap_deserialize_ones(BdrvDirtyBitmap *bitmap,
+uint64_t start, uint64_t count,
+bool finish)
+{
+hbitmap_deserialize_ones(bitmap->bitmap, start, count, finish);
+}
+
 void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap)
 {
 hbitmap_deserialize_finish(bitmap->bitmap);
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 7cbe623..1e17729 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -70,6 +70,9 @@ void bdrv_dirty_bitmap_deserialize_part(BdrvDirtyBitmap 
*bitmap,
 void bdrv_dirty_bitmap_deserialize_zeroes(BdrvDirtyBitmap *bitmap,
   uint64_t start, uint64_t count,
   bool finish);
+void bdrv_dirty_bitmap_deserialize_ones(BdrvDirtyBitmap *bitmap,
+uint64_t start, uint64_t count,
+bool finish);
 void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
 
 #endif
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 6b04391..b52304a 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -229,6 +229,21 @@ void hbitmap_deserialize_zeroes(HBitmap *hb, uint64_t 
start, uint64_t count,
 bool finish);
 
 /**
+ * hbitmap_deserialize_ones
+ * @hb: HBitmap to operate on.
+ * @start: First bit to restore.
+ * @count: Number of bits to restore.
+ * @finish: Whether to call hbitmap_deserialize_finish automatically.
+ *
+ * Fills the bitmap with ones.
+ *
+ * If @finish is false, caller must call hbitmap_serialize_finish before using
+ * the bitmap.
+ */
+void hbitmap_deserialize_ones(HBitmap *hb, uint64_t start, uint64_t count,
+  bool finish);
+
+/**
  * hbitmap_deserialize_finish
  * @hb: HBitmap to operate on.
  *
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 0b38817..0c1591a 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -551,6 +551,23 @@ void hbitmap_deserialize_zeroes(HBitmap *hb, uint64_t 
start, uint64_t count,
 }
 }
 
+void hbitmap_deserialize_ones(HBitmap *hb, uint64_t start, uint64_t count,
+  bool finish)
+{
+uint64_t el_count;
+unsigned long *first;
+
+if (!count) {
+return;
+}
+serialization_chunk(hb, start, count, , _count);
+
+memset(first, 0xff, el_count * sizeof(unsigned long));
+if (finish) {
+hbitmap_deserialize_finish(hb);
+}
+}
+
 void hbitmap_deserialize_finish(HBitmap *bitmap)
 {
 int64_t i, size, prev_size;
-- 
1.8.3.1

[Qemu-devel] [PATCH 03/25] hbitmap: improve dirty iter

[Vladimir Sementsov-Ogievskiy]

Make dirty iter resistant to resetting bits in corresponding HBitmap.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 include/qemu/hbitmap.h | 26 --
 util/hbitmap.c | 23 ++-
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 9239fe5..6b04391 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -256,10 +256,9 @@ void hbitmap_free(HBitmap *hb);
  * the lowest-numbered bit that is set in @hb, starting at @first.
  *
  * Concurrent setting of bits is acceptable, and will at worst cause the
- * iteration to miss some of those bits.  Resetting bits before the current
- * position of the iterator is also okay.  However, concurrent resetting of
- * bits can lead to unexpected behavior if the iterator has not yet reached
- * those bits.
+ * iteration to miss some of those bits.
+ *
+ * The concurrent resetting of bits is OK.
  */
 void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first);
 
@@ -298,24 +297,7 @@ void hbitmap_free_meta(HBitmap *hb);
  * Return the next bit that is set in @hbi's associated HBitmap,
  * or -1 if all remaining bits are zero.
  */
-static inline int64_t hbitmap_iter_next(HBitmapIter *hbi)
-{
-unsigned long cur = hbi->cur[HBITMAP_LEVELS - 1];
-int64_t item;
-
-if (cur == 0) {
-cur = hbitmap_iter_skip_words(hbi);
-if (cur == 0) {
-return -1;
-}
-}
-
-/* The next call will resume work from the next bit.  */
-hbi->cur[HBITMAP_LEVELS - 1] = cur & (cur - 1);
-item = ((uint64_t)hbi->pos << BITS_PER_LEVEL) + ctzl(cur);
-
-return item << hbi->granularity;
-}
+int64_t hbitmap_iter_next(HBitmapIter *hbi);
 
 /**
  * hbitmap_iter_next_word:
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 35088e1..0b38817 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -106,8 +106,9 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi)
 
 unsigned long cur;
 do {
-cur = hbi->cur[--i];
+i--;
 pos >>= BITS_PER_LEVEL;
+cur = hbi->cur[i] & hb->levels[i][pos];
 } while (cur == 0);
 
 /* Check for end of iteration.  We always use fewer than BITS_PER_LONG
@@ -139,6 +140,26 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi)
 return cur;
 }
 
+int64_t hbitmap_iter_next(HBitmapIter *hbi)
+{
+unsigned long cur = hbi->cur[HBITMAP_LEVELS - 1] &
+hbi->hb->levels[HBITMAP_LEVELS - 1][hbi->pos];
+int64_t item;
+
+if (cur == 0) {
+cur = hbitmap_iter_skip_words(hbi);
+if (cur == 0) {
+return -1;
+}
+}
+
+/* The next call will resume work from the next bit.  */
+hbi->cur[HBITMAP_LEVELS - 1] = cur & (cur - 1);
+item = ((uint64_t)hbi->pos << BITS_PER_LEVEL) + ctzl(cur);
+
+return item << hbi->granularity;
+}
+
 void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first)
 {
 unsigned i, bit;
-- 
1.8.3.1

[Qemu-devel] How to upgrade QEMU?

[Bob Chen]

Hi folks,

I am about to upgrade my QEMU version from an ancient 1.1.2 to the latest.
My plan is to override all the installing files with the new ones. Since I
used to rename all the qemu-xxx binaries under /usr/local/bin with a
specified version suffix, so they are not my concern.

 Just wondering if it is safe to replace those /usr/local/share files (ROM
and keymaps?), while some some of my QEMU guests are still running
in-flight.

Regards,
Bob

Re: [Qemu-devel] [PATCH v6 2/2] block/vxhs.c: Add qemu-iotests for new block device type "vxhs"

[Jeff Cody]

On Mon, Feb 13, 2017 at 11:43:17PM -0800, ashish mittal wrote:
> On Mon, Feb 13, 2017 at 7:02 PM, Ketan Nilangekar
>  wrote:
> >
> >
> > On 2/13/17, 3:23 PM, "Jeff Cody"  wrote:
> >
> > On Mon, Feb 13, 2017 at 10:36:53PM +, Ketan Nilangekar wrote:
> > >
> > >
> > > On 2/13/17, 8:32 AM, "Jeff Cody"  wrote:
> > >
> > > On Mon, Feb 13, 2017 at 01:37:25PM +, Stefan Hajnoczi wrote:
> > > > On Tue, Feb 07, 2017 at 03:12:36PM -0800, ashish mittal wrote:
> > > > > On Tue, Nov 8, 2016 at 12:44 PM, Jeff Cody  
> > wrote:
> > > > > > On Mon, Nov 07, 2016 at 04:59:45PM -0800, Ashish Mittal 
> > wrote:
> > > > > >> These changes use a vxhs test server that is a part of the 
> > following
> > > > > >> repository:
> > > > > >> https://github.com/MittalAshish/libqnio.git
> > > > > >>
> > > > > >> Signed-off-by: Ashish Mittal 
> > > > > >> ---
> > > > > >> v6 changelog:
> > > > > >> (1) Added iotests for VxHS block device.
> > > > > >>
> > > > > >>  tests/qemu-iotests/common|  6 ++
> > > > > >>  tests/qemu-iotests/common.config | 13 +
> > > > > >>  tests/qemu-iotests/common.filter |  1 +
> > > > > >>  tests/qemu-iotests/common.rc | 19 +++
> > > > > >>  4 files changed, 39 insertions(+)
> > > > > >>
> > > > > >> diff --git a/tests/qemu-iotests/common 
> > b/tests/qemu-iotests/common
> > > > > >> index d60ea2c..41430d8 100644
> > > > > >> --- a/tests/qemu-iotests/common
> > > > > >> +++ b/tests/qemu-iotests/common
> > > > > >
> > > > > > When using raw format, I was able to run the test 
> > successfully for all
> > > > > > supported test cases (26 of them).
> > > > > >
> > > > > > With qcow2, they fail - but not the fault of this patch, I 
> > think; but
> > > > > > rather, the fault of the test server.  Can qnio_server be 
> > modified so that
> > > > > > it does not work on just raw files?
> > > > > >
> > > > > >
> > > > >
> > > > > VxHS supports and uses only the raw format.
> > > >
> > > > That's like saying only ext4 guest file systems are supported 
> > on VxHS
> > > > and not ZFS.  The VxHS driver should not care what file system 
> > is used,
> > > > it just does block I/O without interpreting the data.
> > > >
> > > > It must be possible to use any format on top of the VxHS 
> > protocol.
> > > > After all, the image format drivers just do block I/O.  If 
> > there is a
> > > > case where qcow2 on VxHS fails then it needs to be investigated.
> > > >
> > > > The VxHS driver can't be merged until we at least understand 
> > the cause
> > > > of the qcow2 test failures.
> > > >
> > >
> > > A quick run with the test server and a QEMU process showed an 
> > abort() in the
> > > test server, so I just sent a pull req to libqnio to fix that.
> > >
> > > But playing with it in gdb right now with a test qcow2 file, I 
> > see that we
> > > are waiting in aio_poll() forever for the test server to respond 
> > to a read
> > > request, when using qcow2 format.
> > >
> > > As Stefan said, this doesn't really make any sense - why would 
> > VXHS behave
> > > differently based on the file contents?
> > >
> > > [Ketan] To read/write a qcow2 backed device VxHS server implementation
> > > will need to understand the qcow2 format. This is not just block IO 
> > but
> > > actually does involve interpreting the qcow2 header and cluster 
> > formats.
> > > Clearly the test server implementation does not handle it as it was 
> > never
> > > intended to. VxHS backend won't handle it either because VxHS virtual
> > > disks are written as non-sparse files.  There are space savings with 
> > the
> > > qcow2 format but performance penalties as well because of the metadata
> > > overhead. As a block storage provider, VxHS does not support sparse 
> > file
> > > formats like qcow2 primarily because of performance reasons.  
> > Implementing
> > > a qcow2 backend in the test server would be a non-trivial and 
> > non-useful
> > > exercise since the VxHS server won't support it.
> > >
> >
> > What?  Why would the backend need to know anything about qcow2 formats; 
> > are
> > you manipulating the guest image data directly yourself?  But 
> > regardless,
> > since the test server is naive and just reads and writes data, the fact 
> > that
> > the test server breaks on qcow2 image formats means that the test 
> > server is
> > broken for raw images, as well [1].
> >
> > 

[Qemu-devel] [PATCH RFC v2 2/2] block: Crude initial implementation of -blockdev

[Markus Armbruster]

The new command line option -blockdev works like QMP command
blockdev-add.

The option argument may be given in JSON syntax, exactly as in QMP.
Example usage:

-blockdev '{"node-name": "foo", "driver": "raw", "file": {"driver": "file", 
"filename": "foo.img"} }'

The JSON argument doesn't exactly blend into the existing option
syntax, so the traditional KEY=VALUE,... syntax is also supported,
using dotted keys to do the nesting:

-blockdev node-name=foo,driver=raw,file.driver=file,file.filename=foo.img

Note that calling qmp_blockdev_add() (say via qmp_marshal_block_add())
right away would crash.  We need to stash the configuration for later
instead.  This is crudely done, and bypasses QemuOpts, even though
storing configuration is what QemuOpts is for.  Need to revamp option
infrastructure to support QAPI types like BlockdevOptions.

Signed-off-by: Markus Armbruster 
---
 qemu-options.hx |  3 +++
 vl.c| 51 +++
 2 files changed, 54 insertions(+)

diff --git a/qemu-options.hx b/qemu-options.hx
index ad2f8fc..a66bd65 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -512,6 +512,9 @@ Use @var{file} as CD-ROM image (you cannot use 
@option{-hdc} and
 using @file{/dev/cdrom} as filename (@pxref{host_drives}).
 ETEXI
 
+DEF("blockdev", HAS_ARG, QEMU_OPTION_blockdev,
+"-blockdev FIXME document\n", QEMU_OPTION_blockdev)
+
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
 "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
 "   [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
diff --git a/vl.c b/vl.c
index b4eaf03..f2d46c3 100644
--- a/vl.c
+++ b/vl.c
@@ -94,6 +94,9 @@ int main(int argc, char **argv)
 #include "migration/colo.h"
 #include "sysemu/kvm.h"
 #include "sysemu/hax.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi-visit.h"
 #include "qapi/qmp/qjson.h"
 #include "qemu/option.h"
 #include "qemu/config-file.h"
@@ -2965,6 +2968,12 @@ int main(int argc, char **argv, char **envp)
 Error *main_loop_err = NULL;
 Error *err = NULL;
 bool list_data_dirs = false;
+typedef struct BlockdevOptions_queue {
+BlockdevOptions *bdo;
+Location loc;
+QSIMPLEQ_ENTRY(BlockdevOptions_queue) entry;
+} BlockdevOptions_queue;
+QSIMPLEQ_HEAD(, BlockdevOptions_queue) bdo_queue = 
QSIMPLEQ_HEAD_INITIALIZER(bdo_queue);
 
 module_call_init(MODULE_INIT_TRACE);
 
@@ -3106,6 +3115,38 @@ int main(int argc, char **argv, char **envp)
 drive_add(IF_DEFAULT, popt->index - QEMU_OPTION_hda, optarg,
   HD_OPTS);
 break;
+case QEMU_OPTION_blockdev:
+{
+bool is_json = optarg[0] == '{';
+QObject *obj;
+QDict *args;
+Visitor *v;
+BlockdevOptions_queue *bdo;
+
+if (is_json) {
+obj = qobject_from_json(optarg);
+// TODO get error out of parser
+if (!obj) {
+error_report("invalid JSON");
+exit(1);
+}
+args = qobject_to_qdict(obj);
+assert(args);
+} else {
+args = opt_parse_qdict(optarg, "driver",
+   _fatal);
+}
+
+bdo = g_new(BlockdevOptions_queue, 1);
+v = qobject_input_visitor_new(QOBJECT(args), true);
+visit_type_BlockdevOptions(v, NULL, >bdo,
+   _fatal);
+visit_free(v);
+QDECREF(args);
+loc_save(>loc);
+QSIMPLEQ_INSERT_TAIL(_queue, bdo, entry);
+break;
+}
 case QEMU_OPTION_drive:
 if (drive_def(optarg) == NULL) {
 exit(1);
@@ -4418,6 +4459,16 @@ int main(int argc, char **argv, char **envp)
 qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
   NULL, NULL);
 }
+while (!QSIMPLEQ_EMPTY(_queue)) {
+BlockdevOptions_queue *bdo = QSIMPLEQ_FIRST(_queue);
+
+QSIMPLEQ_REMOVE_HEAD(_queue, entry);
+loc_push_restore(>loc);
+qmp_blockdev_add(bdo->bdo, _fatal);
+loc_pop(>loc);
+qapi_free_BlockdevOptions(bdo->bdo);
+g_free(bdo);
+}
 if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
   _class->block_default_type, NULL)) {
 exit(1);
-- 
2.7.4

Re: [Qemu-devel] Fw: qemu integrator RAM size can't reach 128M when emulate the arm926

[Peter Maydell]

On 14 February 2017 at 13:45, heavybird <4345...@qq.com> wrote:
> i tried use the integrator.c to emulate the arm926 which has 128M message but 
> found the RAM size can use is less than 32M. steps below. the qemu usding is 
> 2.8.
>
> 1  define the RAM from 0-128M in ld file.
> 2 write the test program to verify i can write/read the memory from 32M to 
> 64M continuously with address directly.
> 3 always failed when the size is close to 32M even though i changed start 
> address.
> 4 error message:
>
> qemu: hardware error: integratorcm_write3: Unimplemented offset 0x0
>
> CPU #0:
> R00=0028 R01=00019000 R02=0055 R03=1000
> R04=ea9c R05=0505 R06=0606 R07=0707
> R08=0808 R09=0909 R10=1010 R11=feb4
> R12= R13=fea8 R14=0338 R15=0300

You don't give your test program, but I'm guessing
from this register dump that the address it is
trying to read is in r3 which is 0x1000.
That is off the end of the RAM (which starts at 0
and goes up to just under 0x1000 in the case when
you have the maximum possible 256MB RAM in this board).
In particular 0x1000 is where the board's system
registers start, and the error above indicates that you
have attempted to write to one of them, not to RAM.

I suspect you have a bug in your test code.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 2/3] new: debian docker targets for cross-compiling

[Alex Bennée]

Peter Maydell  writes:

> On 14 February 2017 at 10:07, Alex Bennée  wrote:
>> This provides a basic Debian install with access to the emdebian cross
>> compilers. The debian-armhf-cross and debian-arm64-cross targets build
>> on the basic Debian image to allow cross compiling to those targets.
>
> Is there a particular reason for creating different docker images
> for each cross target rather than just having one image with
> all the cross compilers in it?

Mainly the clashing of build-dependencies. Debian's multi-arch is pretty
good but you still can't generally have two arches worth of complex
dependencies installed at once.

I did originally have one base docker image with all the compilers and
intended to do the dependencies step as part of test-build. However when
running as builders the docker images don't actually have network
ability so couldn't download them.

--
Alex Bennée

Re: [Qemu-devel] [PATCH] backup: allow target without .bdrv_get_info

[Denis V. Lunev]

On 02/14/2017 06:05 PM, Fam Zheng wrote:
> On Tue, 02/14 17:48, Vladimir Sementsov-Ogievskiy wrote:
>> Currently backup to nbd target is broken, as nbd doesn't have
>> .bdrv_get_info realization.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>> ---
>>
>> Hi all!
>>
>> Since commit
>>
>> commit 4c9bca7e39a6e07ad02c1dcde3478363344ec60b
>> Author: John Snow 
>> Date:   Thu Feb 25 15:58:30 2016 -0500
>>
>> block/backup: avoid copying less than full target clusters
>>
>> backup to nbd target is broken, we have "Couldn't determine the cluster size 
>> of
>> the target image".
>>
>> Proposed NBD protocol extension - NBD_OPT_INFO should finally solve this 
>> problem.
>> But until it is not realized, we need allow backup to nbd target due to 
>> backward
>> compatibility.
>>
>> Furthermore, is it entirely ok to disallow backup if bds lacks 
>> .bdrv_get_info?
>> Which behavior should be default: to fail backup or to use default cluster 
>> size?
>>
>>  block/backup.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/backup.c b/block/backup.c
>> index ea38733..095a390 100644
>> --- a/block/backup.c
>> +++ b/block/backup.c
>> @@ -638,7 +638,10 @@ BlockJob *backup_job_create(const char *job_id, 
>> BlockDriverState *bs,
>>   * backup cluster size is smaller than the target cluster size. Even for
>>   * targets with a backing file, try to avoid COW if possible. */
>>  ret = bdrv_get_info(target, );
>> -if (ret < 0 && !target->backing) {
>> +if (ret == -ENOTSUP) {
>> +/* Cluster size is not defined */
>> +job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
>> +} else if (ret < 0 && !target->backing) {
>>  error_setg_errno(errp, -ret,
>>  "Couldn't determine the cluster size of the target image, "
>>  "which has no backing file");
>> -- 
>> 1.8.3.1
>>
> Target not having .bdrv_get_info means 'some sane assumptions are okay' to 
> me, so
>
> Reviewed-by: Fam Zheng 
>
> (But we are going to have a .bdrv_get_info after NBD_OPT_INFO, right?)
>
yes, sooner or later :)

[Qemu-devel] [PATCH] backup: allow target without .bdrv_get_info

[Vladimir Sementsov-Ogievskiy]

Currently backup to nbd target is broken, as nbd doesn't have
.bdrv_get_info realization.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Hi all!

Since commit

commit 4c9bca7e39a6e07ad02c1dcde3478363344ec60b
Author: John Snow 
Date:   Thu Feb 25 15:58:30 2016 -0500

block/backup: avoid copying less than full target clusters

backup to nbd target is broken, we have "Couldn't determine the cluster size of
the target image".

Proposed NBD protocol extension - NBD_OPT_INFO should finally solve this 
problem.
But until it is not realized, we need allow backup to nbd target due to backward
compatibility.

Furthermore, is it entirely ok to disallow backup if bds lacks .bdrv_get_info?
Which behavior should be default: to fail backup or to use default cluster size?

 block/backup.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/backup.c b/block/backup.c
index ea38733..095a390 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -638,7 +638,10 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
  * backup cluster size is smaller than the target cluster size. Even for
  * targets with a backing file, try to avoid COW if possible. */
 ret = bdrv_get_info(target, );
-if (ret < 0 && !target->backing) {
+if (ret == -ENOTSUP) {
+/* Cluster size is not defined */
+job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
+} else if (ret < 0 && !target->backing) {
 error_setg_errno(errp, -ret,
 "Couldn't determine the cluster size of the target image, "
 "which has no backing file");
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 2/3] new: debian docker targets for cross-compiling

[Peter Maydell]

On 14 February 2017 at 10:07, Alex Bennée  wrote:
> This provides a basic Debian install with access to the emdebian cross
> compilers. The debian-armhf-cross and debian-arm64-cross targets build
> on the basic Debian image to allow cross compiling to those targets.

Is there a particular reason for creating different docker images
for each cross target rather than just having one image with
all the cross compilers in it?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 3/3] .shippable.yml: new CI provider

[Fam Zheng]

On Tue, 02/14 10:07, Alex Bennée wrote:
> Ostensibly Shippable offers a similar set of services as Travis.
> However they are focused on Docker container based work-flows so we
> can use our existing containers to run a few extra builds - in this
> case a bunch of cross-compiled targets on a Debian multiarch system.
> 
> Signed-off-by: Alex Bennée 
> 
> ---
> v3
>   - reduce matrix to armhf/arm64 which currently work
>   - use the make docker-image-* build stanzas
>   - add TARGET_LIST to each build
> ---
>  .shippable.yml | 23 +++
>  MAINTAINERS|  1 +
>  2 files changed, 24 insertions(+)
>  create mode 100644 .shippable.yml
> 
> diff --git a/.shippable.yml b/.shippable.yml
> new file mode 100644
> index 00..e4fa159481
> --- /dev/null
> +++ b/.shippable.yml
> @@ -0,0 +1,23 @@
> +language: c
> +env:
> +  matrix:
> +- IMAGE=debian-armhf-cross
> +  CROSS_PREFIX=arm-linux-gnueabihf-
> +  TARGET_LIST=arm-softmmu,arm-linux-user
> +- IMAGE=debian-arm64-cross
> +  CROSS_PREFIX=aarch64-linux-gnu-
> +  TARGET_LIST=aarch64-softmmu,aarch64-linux-user
> +- IMAGE=centos6
> +  TARGET_LIST=i386-softmmu,x86_64-softmmu
> +build:
> +  pre_ci:
> +- make docker-image-${IMAGE}
> +  pre_ci_boot:
> +image_name: qemu
> +image_tag: ${IMAGE}
> +pull: false
> +options: "-e HOME=/root"
> +  ci:
> +- unset CC
> +- ./configure --cross-prefix=${CROSS_PREFIX} --target-list=${TARGET_LIST}
> +- make -j2

Looks cool!

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7afbadaa15..57d32d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1805,6 +1805,7 @@ M: Alex Bennée 
>  L: qemu-devel@nongnu.org
>  S: Supported
>  F: .travis.yml
> +F: .shippable.yml
>  
>  Documentation
>  -
> -- 
> 2.11.0
> 
> 

May I propose we merge 'docker testing' section of MAINTAINERS into 'build and
test automation' section? I don't know as much about travis (and shippable) but
I'm totally fine if you want to have docker tests under your umbrella, and
it seems a logical step judging from their names.

Fam

Re: [Qemu-devel] [PATCH v2 0/3] migration capability to discard the migrated ram pages

[Dr. David Alan Gilbert]

Hi Pavel,
  I was curious, having merged this, how you're using postcopy; do you switch
into postcopy mode immediately or wait until the first sync or what?
Do you find yourself in postcopy mode long enough that it's worth
doing the release?  If so on what size VMs are you working with?

Dave

* Pavel Butsykin (pbutsy...@virtuozzo.com) wrote:
> This feature frees the migrated memory on the source during postcopy-ram
> migration. In the second step of postcopy-ram migration when the source vm
> is put on pause we can free unnecessary memory. It will allow, in particular,
> to start relaxing the memory stress on the source host in a load-balancing
> scenario.
> 
> Changes from v1:
> - changed name of the interfaces (discard to release)
> - fix make check error
> - add more comments to qemu_iovec_release_ram()
> - rebase on "Postcopy: Hugepage support" (David's patch series)
> - removed ram_discard_page for xbzrle 
> - fix erroneous release memory in complete precopy (tie release-ram to 
> postcopy)
> 
> Pavel Butsykin (3):
>   migration: add MigrationState arg for ram_save_/compressed_/page()
>   add 'release-ram' migrate capability
>   migration: discard non-dirty ram pages after the start of postcopy
> 
>  include/migration/migration.h |  2 ++
>  include/migration/qemu-file.h |  3 ++-
>  migration/migration.c | 13 ++
>  migration/qemu-file.c | 59 
> ++-
>  migration/ram.c   | 56 ++--
>  qapi-schema.json  |  5 +++-
>  6 files changed, 121 insertions(+), 17 deletions(-)
> 
> -- 
> 2.11.0
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PULL 07/12] migration: Start of multiple fd work

[Juan Quintela]

Paolo Bonzini  wrote:
> On 14/02/2017 14:12, Juan Quintela wrote:
>>> On 13/02/2017 18:19, Juan Quintela wrote:
 +qemu_sem_init(>init, 0);
  p->quit = false;
 +p->c = socket_send_channel_create();
 +if (!p->c) {
 +error_report("Error creating a send channel");
 +exit(0);
 +}
  snprintf(thread_name, 15, "multifd_send_%d", i);
  qemu_thread_create(>thread, thread_name, multifd_send_thread, 
 p,
 QEMU_THREAD_JOINABLE);
 +qemu_sem_wait(>init);
>>> Why do you need p->init here?  Could initialization proceed in parallel
>>> for all the threads?
>>
>> We need to make sure that the send thread number 2 goes to thread number
>> 2 on destination.  Yes, we could do a more complicated algorithm, but we
>> really care so much about this initialization time?
>
> I was wondering if p->init was needed in general, so it would simplify.
> But without a design document I cannot really understand the logic---as
> I said, I don't really grok the need for RAM_SAVE_FLAG_MULTIFD_PAGE.

will get some general documentation in /doc/.

Basically what we had on the only stream was:


{[page header][page]}+


And we moved to:

{[page header]+[where to receive]}: on the principal stream

[page]+: on the rest of the multifd

All nicely aligned and so.

My understanding is that we could optimize the receiving with splice to
not even touch userspace? (that part is not done).  That was the reason
why I didn't want to put header's footers there.  As the headers are so
small compared with the pages payload, the transmission of them should
be lost on the noise, no?

Later, Juan.

Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices

[Stefan Hajnoczi]

On Tue, Feb 07, 2017 at 05:29:33PM +0100, Greg Kurz wrote:
> Cc'ing Stefan who reviewed patch 2/2.
> 
> On Tue, 7 Feb 2017 09:56:08 -0600
> Eric Blake  wrote:
> 
> > On 02/07/2017 04:32 AM, Greg Kurz wrote:
> > >>
> > >> I'm not aware of anything related to fsdev in QMP... and libvirt seems to
> > >> only parse the output of -help to guess fsdev capabilities.  
> > > 
> > > Oops, reading some more libvirt code I now see that libvirt doesn't parse
> > > -help anymore with QEMU >= 1.2.0... sorry for the noise :)
> > >   
> > >> And indeed,
> > >> qemu-options.hx doesn't expose this new feature.
> > >>  
> > >>> Please make sure we don't reach 2.9 with only a half-baked feature;
> > >>> whether that means finishing the QMP work or temporarily disabling the
> > >>> cli additions until a later release can finish the work.
> > >>> 
> > >>
> > >> Would this be ok to add the missing bits in qemu-options.hx or do you
> > >> expect more ?  
> > 
> > If it cannot be probed via QMP, then libvirt will most likely assume
> > that it does not exist.  I guess we're okay having command line only in
> > 2.9 if you can't get QMP working, because libvirt will just never drive
> > the feature until 2.10 when QMP is available; but then we risk the
> > command line subtly changing and breaking someone else that was using
> > the command line without QMP.  Maybe the safest approach is to just use
> > the 'x-' prefix to the command line portion, until the feature is complete.
> > 
> 
> The semantics here are exactly the same as for block devices. The
> command line options added to -fsdev are the very same already used
> by -drive for years.
> 
> Patch 2/2 in this series even factors them out to a common header file
> to be used by fsdev and blockdev. I really don't expect any modification
> at all on the command line (nor the other people who reviewed that patch
> obviously)... are you suggesting that we should put 2/2 on hold and
> use the 'x-' prefix anyway ?

I see these parameter names as stable.  There is little risk that they
would change.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PULL 07/12] migration: Start of multiple fd work

[Juan Quintela]

Paolo Bonzini  wrote:
> On 13/02/2017 18:19, Juan Quintela wrote:
>> +qemu_sem_init(>init, 0);
>>  p->quit = false;
>> +p->c = socket_send_channel_create();
>> +if (!p->c) {
>> +error_report("Error creating a send channel");
>> +exit(0);
>> +}
>>  snprintf(thread_name, 15, "multifd_send_%d", i);
>>  qemu_thread_create(>thread, thread_name, multifd_send_thread, p,
>> QEMU_THREAD_JOINABLE);
>> +qemu_sem_wait(>init);
>
> Why do you need p->init here?  Could initialization proceed in parallel
> for all the threads?

We need to make sure that the send thread number 2 goes to thread number
2 on destination.  Yes, we could do a more complicated algorithm, but we
really care so much about this initialization time?

Later, Juan.

Re: [Qemu-devel] [PATCH 00/12] Multifd v4

[Paolo Bonzini]

On 13/02/2017 18:19, Juan Quintela wrote:
> [v4]
> 
> - Address reviews
> - move synchronization to semaphores (faster).  Paolo suggestion
> - improvements overall (see invidiual patches)
> - fix all the checkpatch warnings
> - fix all [HACKS] except for one
> 
> Please review.

I think you can simplify the receive threads much more.  I left comments
on patch 11.

Paolo

Re: [Qemu-devel] [PULL 11/12] migration: Send the fd number which we are going to use for this page

[Paolo Bonzini]

On 13/02/2017 18:19, Juan Quintela wrote:
>  case RAM_SAVE_FLAG_MULTIFD_PAGE:
> +fd_num = qemu_get_be16(f);
> +multifd_recv_page(host, fd_num);
>  qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
>  break;

Why do you need RAM_SAVE_FLAG_MULTIFD_PAGE?  I understand the
orchestration of sent pages from a single thread, but could the receive
threads proceed independently, each reading its own socket?  They do not
even need to tell the central thread "I'm done" (they can do so just by
exiting, and the central thread does qemu_thread_join when it sees the
marker for end of live data).

Paolo

Re: [Qemu-devel] [PATCH 08/24] util/cutils: Clean up control flow around qemu_strtol() a bit

[Peter Maydell]

On 14 February 2017 at 12:58, Markus Armbruster  wrote:
> Peter Maydell  writes:
>> Doesn't this change the semantics? Previously, for the
>> case of (eno == 0 && ep == nptr) we would both set
>> *endptr to ep and return -EINVAL. Now we only return -EINVAL
>> and leave *endptr alone.

> No change, as far as I can tell.

Ah, you're right, and I misread the diff. Sorry.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 08/24] util/cutils: Clean up control flow around qemu_strtol() a bit

[Markus Armbruster]

Peter Maydell  writes:

> On 14 February 2017 at 10:25, Markus Armbruster  wrote:
>> Reorder check_strtox_error() to make it obvious that we always store
>> through a non-null @endptr.
>>
>> Transform
>>
>> if (some error) {
>> error case ...
>> err = value for error case;
>> } else {
>> normal case ...
>> err = value for normal case;
>> }
>> return err;
>>
>> to
>>
>> if (some error) {
>> error case ...
>> return value for error case;
>> }
>> normal case ...
>> return value for normal case;
>>
>> Signed-off-by: Markus Armbruster 
>> ---
>>  util/cutils.c | 89 
>> ++-
>>  1 file changed, 45 insertions(+), 44 deletions(-)
>>
>> diff --git a/util/cutils.c b/util/cutils.c
>> index 7d165bc..7442d46 100644
>> --- a/util/cutils.c
>> +++ b/util/cutils.c
>> @@ -265,15 +265,20 @@ int64_t qemu_strtosz(const char *nptr, char **end)
>>  static int check_strtox_error(const char *nptr, char *ep,
>>const char **endptr, int eno)
>>  {
>> -if (eno == 0 && ep == nptr) {
>> -eno = EINVAL;
>> -}
>> -if (!endptr && *ep) {
>> -return -EINVAL;
>> -}
>>  if (endptr) {
>>  *endptr = ep;
>>  }
>> +
>> +/* Turn "no conversion" into an error */
>> +if (eno == 0 && ep == nptr) {
>> +return -EINVAL;
>> +}
>> +
>> +/* Fail when we're expected to consume the string, but didn't */
>> +if (!endptr && *ep) {
>> +return -EINVAL;
>> +}
>> +
>>  return -eno;
>>  }
>
> Doesn't this change the semantics? Previously, for the
> case of (eno == 0 && ep == nptr) we would both set
> *endptr to ep and return -EINVAL. Now we only return -EINVAL
> and leave *endptr alone.

Behavior before the patch:

if (eno == 0 && ep == nptr) {
eno = EINVAL;   // set return value to EINVAL, but continue
}
if (!endptr && *ep) {
return -EINVAL; // return -EINVAL without setting *endptr
// correct because endptr is null
}
if (endptr) {
*endptr = ep;   // set *endptr unless endptr is null
}
return -eno;// *endptr got set unless endptr is null

As you say, we set *endptr unless it's null.

After the patch:

if (endptr) {
*endptr = ep;   // set *endptr unless endptr is null
}

// no matter what happens below, *endptr got set unless endptr is null

/* Turn "no conversion" into an error */
if (eno == 0 && ep == nptr) {
return -EINVAL;
}

/* Fail when we're expected to consume the string, but didn't */
if (!endptr && *ep) {
return -EINVAL;
}

return -eno;

No change, as far as I can tell.

>  The comment describing the
> qemu_strtol() API is a bit vague about what exactly we keep
> from the strtol() semantics for "no convertable characters"
> but I would assume that retaining "*endptr is written with
> the original value of nptr" is intentional.

I rewrite the comment PATCH 05.  Relevant part:

 * @nptr may be null, and no conversion is performed then.
 *
 * If no conversion is performed, store @nptr in *@endptr and return
 * -EINVAL.

I guess I should qualify "store @nptr in *@endptr" with "unless @endptr
is null" for completeness.  Anything else to improve?

[Qemu-devel] VirtIO Windows Driver repository

[Yan Vugenfirer]

Hello All,

The official upstream repository moved from 
https://github.com/YanVugenfirer/kvm-guest-drivers-windows to 
https://github.com/virtio-win/kvm-guest-drivers-windows

Best regards,
Yan Vugenfirer.

Re: [Qemu-devel] [PATCH 15/24] util/cutils: Rename qemu_strtosz() to qemu_strtosz_mebi()

[Paolo Bonzini]

On 14/02/2017 11:26, Markus Armbruster wrote:
> With qemu_strtosz(), no suffix means mebibytes.  It's used rarely.
> I'm going to add a similar function where no suffix means bytes.
> Rename qemu_strtosz() to qemu_strtosz_mebi() to make the name
> qemu_strtosz() available for the new function.

What about qemu_strtosz_MiB (yes, camelcase intended).

Paolo

> 
> Signed-off-by: Markus Armbruster 
> ---
>  hmp.c |  2 +-
>  hw/misc/ivshmem.c |  2 +-
>  include/qemu/cutils.h |  2 +-
>  monitor.c |  2 +-
>  tests/test-cutils.c   | 38 +++---
>  util/cutils.c |  2 +-
>  6 files changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 2bc4f06..6d0d05b 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1379,7 +1379,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const 
> QDict *qdict)
>  break;
>  case MIGRATION_PARAMETER_MAX_BANDWIDTH:
>  p.has_max_bandwidth = true;
> -valuebw = qemu_strtosz(valuestr, );
> +valuebw = qemu_strtosz_mebi(valuestr, );
>  if (valuebw < 0 || (size_t)valuebw != valuebw
>  || *endp != '\0') {
>  error_setg(, "Invalid size %s", valuestr);
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index bf57e63..382dd8b 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -1268,7 +1268,7 @@ static void ivshmem_realize(PCIDevice *dev, Error 
> **errp)
>  s->legacy_size = 4 << 20; /* 4 MB default */
>  } else {
>  char *end;
> -int64_t size = qemu_strtosz(s->sizearg, );
> +int64_t size = qemu_strtosz_mebi(s->sizearg, );
>  if (size < 0 || (size_t)size != size || *end != '\0'
>  || !is_power_of_2(size)) {
>  error_setg(errp, "Invalid size %s", s->sizearg);
> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> index 81613d0..5f4e138 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -153,9 +153,9 @@ int parse_uint_full(const char *s, unsigned long long 
> *value, int base);
>  #define QEMU_STRTOSZ_DEFSUFFIX_MB 'M'
>  #define QEMU_STRTOSZ_DEFSUFFIX_KB 'K'
>  #define QEMU_STRTOSZ_DEFSUFFIX_B 'B'
> -int64_t qemu_strtosz(const char *nptr, char **end);
>  int64_t qemu_strtosz_suffix(const char *nptr, char **end,
>  const char default_suffix);
> +int64_t qemu_strtosz_mebi(const char *nptr, char **end);
>  int64_t qemu_strtosz_metric(const char *nptr, char **end);
>  
>  #define K_BYTE (1ULL << 10)
> diff --git a/monitor.c b/monitor.c
> index 3cd72a9..1f8c031 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2785,7 +2785,7 @@ static QDict *monitor_parse_arguments(Monitor *mon,
>  break;
>  }
>  }
> -val = qemu_strtosz(p, );
> +val = qemu_strtosz_mebi(p, );
>  if (val < 0) {
>  monitor_printf(mon, "invalid size\n");
>  goto fail;
> diff --git a/tests/test-cutils.c b/tests/test-cutils.c
> index 383f812..86d36b7 100644
> --- a/tests/test-cutils.c
> +++ b/tests/test-cutils.c
> @@ -1376,16 +1376,16 @@ static void test_qemu_strtosz_simple(void)
>  int64_t res;
>  
>  str = "0";
> -res = qemu_strtosz(str, );
> +res = qemu_strtosz_mebi(str, );
>  g_assert_cmpint(res, ==, 0);
>  g_assert(endptr == str + 1);
>  
>  str = "12345M";
> -res = qemu_strtosz(str, );
> +res = qemu_strtosz_mebi(str, );
>  g_assert_cmpint(res, ==, 12345 * M_BYTE);
>  g_assert(endptr == str + 6);
>  
> -res = qemu_strtosz(str, NULL);
> +res = qemu_strtosz_mebi(str, NULL);
>  g_assert_cmpint(res, ==, 12345 * M_BYTE);
>  
>  /* Note: precision is 53 bits since we're parsing with strtod() */
> @@ -1433,35 +1433,35 @@ static void test_qemu_strtosz_units(void)
>  int64_t res;
>  
>  /* default is M */
> -res = qemu_strtosz(none, );
> +res = qemu_strtosz_mebi(none, );
>  g_assert_cmpint(res, ==, M_BYTE);
>  g_assert(endptr == none + 1);
>  
> -res = qemu_strtosz(b, );
> +res = qemu_strtosz_mebi(b, );
>  g_assert_cmpint(res, ==, 1);
>  g_assert(endptr == b + 2);
>  
> -res = qemu_strtosz(k, );
> +res = qemu_strtosz_mebi(k, );
>  g_assert_cmpint(res, ==, K_BYTE);
>  g_assert(endptr == k + 2);
>  
> -res = qemu_strtosz(m, );
> +res = qemu_strtosz_mebi(m, );
>  g_assert_cmpint(res, ==, M_BYTE);
>  g_assert(endptr == m + 2);
>  
> -res = qemu_strtosz(g, );
> +res = qemu_strtosz_mebi(g, );
>  g_assert_cmpint(res, ==, G_BYTE);
>  g_assert(endptr == g + 2);
>  
> -res = qemu_strtosz(t, );
> +res = qemu_strtosz_mebi(t, );
>  g_assert_cmpint(res, ==, T_BYTE);
>  g_assert(endptr == t + 2);
>  
> -res = qemu_strtosz(p, );
> +res = qemu_strtosz_mebi(p, );
>  

Re: [Qemu-devel] [PATCH 07/24] util/cutils: Clean up variable names around qemu_strtol()

[Paolo Bonzini]

On 14/02/2017 11:25, Markus Armbruster wrote:
> * qemu_strtol()'s variable @err is *negative* errno,
>   check_strtox_error()'s parameter @err is *positive*.  Rename the
>   latter to @eno.

What about in_errno or libc_errno?  No need to repost of course.

Paolo

Re: [Qemu-devel] [PATCH 01/18] block: move AioContext, QEMUTimer, main-loop to libqemuutil

[Daniel P. Berrange]

On Tue, Feb 14, 2017 at 03:48:31PM +0800, Fam Zheng wrote:
> On Mon, 02/13 14:52, Paolo Bonzini wrote:
> > --- /dev/null
> > +++ b/util/aiocb.c
> > @@ -0,0 +1,55 @@
> > +/*
> > + * BlockAIOCB allocation
> > + *
> > + * Copyright (c) 2003-2017 Fabrice Bellard and the QEMU team
> 
> Hmm, I'm not lawyer, just wondering if the QEMU team is a legal entity that 
> can
> hold copyright? :)

Reword it to say

  "Copyright (c) 2003-2017 Fabrice Bellard and other QEMU contributors"

so it is referring to individual contributors as distinct copyright
holders, as opposed to a single entity called "QEMU team"

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [PATCH 11/24] block: introduce persistent dirty bitmaps

[Vladimir Sementsov-Ogievskiy]

11.02.2017 02:20, John Snow wrote:


On 02/03/2017 04:40 AM, Vladimir Sementsov-Ogievskiy wrote:

New field BdrvDirtyBitmap.persistent means, that bitmap should be saved
on bdrv_close, using format driver. Format driver should maintain bitmap
storing.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
---
  block.c  | 32 
  block/dirty-bitmap.c | 26 ++
  block/qcow2-bitmap.c |  1 +
  include/block/block.h|  1 +
  include/block/block_int.h|  2 ++
  include/block/dirty-bitmap.h |  6 ++
  6 files changed, 68 insertions(+)

diff --git a/block.c b/block.c
index 56f030c562..970e4ca50e 100644
--- a/block.c
+++ b/block.c
@@ -2321,6 +2321,7 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
  static void bdrv_close(BlockDriverState *bs)
  {
  BdrvAioNotifier *ban, *ban_next;
+Error *local_err = NULL;
  
  assert(!bs->job);

  assert(!bs->refcnt);
@@ -2329,6 +2330,12 @@ static void bdrv_close(BlockDriverState *bs)
  bdrv_flush(bs);
  bdrv_drain(bs); /* in case flush left pending I/O */
  
+bdrv_store_persistent_dirty_bitmaps(bs, _err);

+if (local_err != NULL) {
+error_report_err(local_err);
+error_report("Persistent bitmaps are lost for node '%s'",
+ bdrv_get_device_or_node_name(bs));
+}

Ouch! I guess there's not much else we can do here, eh?


  bdrv_release_named_dirty_bitmaps(bs);
  assert(QLIST_EMPTY(>dirty_bitmaps));
  
@@ -4107,3 +4114,28 @@ void bdrv_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp)

  bs->drv->bdrv_load_autoloading_dirty_bitmaps(bs, errp);
  }
  }
+
+void bdrv_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
+{
+BlockDriver *drv = bs->drv;
+
+if (!bdrv_has_persistent_bitmaps(bs)) {
+return;
+}
+
+if (!drv) {
+error_setg_errno(errp, ENOMEDIUM,
+ "Can't store persistent bitmaps to %s",
+ bdrv_get_device_or_node_name(bs));
+return;
+}
+
+if (!drv->bdrv_store_persistent_dirty_bitmaps) {
+error_setg_errno(errp, ENOTSUP,
+ "Can't store persistent bitmaps to %s",
+ bdrv_get_device_or_node_name(bs));
+return;
+}
+

I suppose this is for the case for where we have added a persistent
bitmap during runtime, but the driver does not support it?

I'd rather fail this type of thing earlier if possible, but I'm not that
far in your series yet.


qmp bitmap add checks for availability of 
drv->bdrv_can_store_new_dirty_bitmap,
and loaded bitmaps of course should be attached to bds with appropriate 
driver.

So, it is mostly a paranoic check.




+drv->bdrv_store_persistent_dirty_bitmaps(bs, errp);
+}
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 2d27494dc7..4d026df1bd 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -44,6 +44,7 @@ struct BdrvDirtyBitmap {
  int64_t size;   /* Size of the bitmap (Number of sectors) */
  bool disabled;  /* Bitmap is read-only */
  int active_iterators;   /* How many iterators are active */
+bool persistent;/* bitmap must be saved to owner disk image */
  bool autoload;  /* For persistent bitmaps: bitmap must be
 autoloaded on image opening */
  QLIST_ENTRY(BdrvDirtyBitmap) list;
@@ -73,6 +74,7 @@ void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
  g_free(bitmap->name);
  bitmap->name = NULL;
  
+bitmap->persistent = false;

  bitmap->autoload = false;
  }
  
@@ -242,6 +244,8 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,

  bitmap->name = NULL;
  successor->name = name;
  bitmap->successor = NULL;
+successor->persistent = bitmap->persistent;
+bitmap->persistent = false;
  successor->autoload = bitmap->autoload;
  bitmap->autoload = false;
  bdrv_release_dirty_bitmap(bs, bitmap);
@@ -556,3 +560,25 @@ bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap 
*bitmap)
  {
  return bitmap->autoload;
  }
+
+void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap, bool 
persistent)
+{
+bitmap->persistent = persistent;
+}
+
+bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap)
+{
+return bitmap->persistent;
+}
+
+bool bdrv_has_persistent_bitmaps(BlockDriverState *bs)
+{
+BdrvDirtyBitmap *bm;
+QLIST_FOREACH(bm, >dirty_bitmaps, list) {
+if (bm->persistent) {
+return true;
+}
+}
+
+return false;
+}

Probably not worth optimizing.


diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index bcbb0491ee..9af658a0f4 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -707,6 +707,7 @@ void 

Re: [Qemu-devel] [PATCH 14/17] migration: Create thread infrastructure for multifd recv side

[Dr. David Alan Gilbert]

* Juan Quintela (quint...@redhat.com) wrote:
> "Dr. David Alan Gilbert"  wrote:
> > * Juan Quintela (quint...@redhat.com) wrote:
> >> We make the locking and the transfer of information specific, even if we
> >> are still receiving things through the main thread.
> >> 
> >> Signed-off-by: Juan Quintela 
> >> ---
> >>  migration/ram.c | 77 
> >> +
> >>  1 file changed, 67 insertions(+), 10 deletions(-)
> >> 
> >> diff --git a/migration/ram.c b/migration/ram.c
> >> index ca94704..4e530ea 100644
> >> --- a/migration/ram.c
> >> +++ b/migration/ram.c
> >> @@ -523,7 +523,7 @@ void migrate_multifd_send_threads_create(void)
> >>  }
> >>  }
> >> 
> >> -static int multifd_send_page(uint8_t *address)
> >> +static uint16_t multifd_send_page(uint8_t *address, bool last_page)
> >>  {
> >>  int i, j, thread_count;
> >>  bool found = false;
> >> @@ -538,8 +538,10 @@ static int multifd_send_page(uint8_t *address)
> >>  pages.address[pages.num] = address;
> >>  pages.num++;
> >> 
> >> -if (pages.num < (pages.size - 1)) {
> >> -return UINT16_MAX;
> >> +if (!last_page) {
> >> +if (pages.num < (pages.size - 1)) {
> >> +return UINT16_MAX;
> >> +}
> >>  }
> >
> > This should be in the previous patch?
> > (and the place that adds the last_page parameter below)?
> 
> ok.
> 
> >> @@ -2920,10 +2980,7 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> >> version_id)
> >> 
> >>  case RAM_SAVE_FLAG_MULTIFD_PAGE:
> >>  fd_num = qemu_get_be16(f);
> >> -if (fd_num != 0) {
> >> -/* this is yet an unused variable, changed later */
> >> -fd_num = fd_num;
> >> -}
> >> +multifd_recv_page(host, fd_num);
> >
> > This is going to be quite tricky to fit into ram_load_postcopy
> > in this form; somehow it's going to have to find addresses to use for place 
> > page
> > and with anything with a page size != target page size it gets messy.
> 
> What do you have in mind?

The problem is that for postcopy we read the data into a temporary buffer
and then call a system call to 'place' the page atomically in memory.
At the moment there's a single temporary buffer; for x86 this is easy -
read a page into buffer; place it.  For Power/ARM or hugepages we
read consecutive target-pages into the temporary buffer and at the end
of the page place the whole host/huge page at once.
If you're reading multiple pages in parallel then you're going to need
to take care with multiple temporary buffers; having one hugepage/hostpage
per fd would probably be the easiest way.

A related thing to take care of is that when switching to postcopy mode
we probably need to take care to sync all of the fds to make sure
any outstanding RAM load has completed before we start doing any postcopy
magic.

Dave

> Later, Juan.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 10/17] migration: create ram_multifd_page

[Dr. David Alan Gilbert]

* Juan Quintela (quint...@redhat.com) wrote:
> "Dr. David Alan Gilbert"  wrote:
> > * Juan Quintela (quint...@redhat.com) wrote:
> >> The function still don't use multifd, but we have simplified
> >> ram_save_page, xbzrle and RDMA stuff is gone.  We have added a new
> >> counter and a new flag for this type of pages.
> 
> 
> >> +static int ram_multifd_page(QEMUFile *f, PageSearchStatus *pss,
> >> +bool last_stage, uint64_t *bytes_transferred)
> >> +{
> >> +int pages;
> >> +uint8_t *p;
> >> +RAMBlock *block = pss->block;
> >> +ram_addr_t offset = pss->offset;
> >> +
> >> +p = block->host + offset;
> >> +
> >> +if (block == last_sent_block) {
> >> +offset |= RAM_SAVE_FLAG_CONTINUE;
> >> +}
> >> +pages = save_zero_page(f, block, offset, p, bytes_transferred);
> >> +if (pages == -1) {
> >> +*bytes_transferred +=
> >> +save_page_header(f, block, offset | 
> >> RAM_SAVE_FLAG_MULTIFD_PAGE);
> >> +qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
> >> +*bytes_transferred += TARGET_PAGE_SIZE;
> >> +pages = 1;
> >> +acct_info.norm_pages++;
> >> +acct_info.multifd_pages++;
> >> +}
> >> +
> >> +return pages;
> >> +}
> >> +
> >>  static int do_compress_ram_page(QEMUFile *f, RAMBlock *block,
> >>  ram_addr_t offset)
> >>  {
> >> @@ -1427,6 +1461,8 @@ static int ram_save_target_page(MigrationState *ms, 
> >> QEMUFile *f,
> >>  res = ram_save_compressed_page(f, pss,
> >> last_stage,
> >> bytes_transferred);
> >> +} else if (migrate_use_multifd()) {
> >> +res = ram_multifd_page(f, pss, last_stage, bytes_transferred);
> >
> > I'm curious whether it's best to pick the destination fd at this level or 
> > one level
> > higher; for example would it be good to keep all the components of a
> > host page or huge
> > page together on the same fd? If so then it would be best to pick the fd
> > at ram_save_host_page level.
> 
> my plan here would be to change the migration code to be able to call
> with a bigger sizes, not page by page, and then the problem is solved by
> itself?

Yes it might be, but you may have to be careful to keep all your FDs busy;
for example, imagine that we're using huge pages, and you try and stuff
an entire hugepage down one FD, for 2MB hugepages that's about half of the
write buffer (tcp-wmem?) so there's a fair chance it'll block.  But thereagain
I think it's probably the right thing to do as long as you can get
different pages down different FDs.

Dave

> Later, Juan.
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4] sd: sdhci: mask transfer mode register value

[P J P]

  Hello Petr,

+-- On Tue, 14 Feb 2017, Peter Maydell wrote --+
| Yes, exactly. (We document this in 
| 
http://wiki.qemu-project.org/Contribute/SubmitAPatch#Participating_in_Code_Review
 
| but of course that page is huge...)

  Right, okay. I'll resend the patch set later today.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PULL 07/12] migration: Start of multiple fd work

[Daniel P. Berrange]

On Mon, Feb 13, 2017 at 06:19:43PM +0100, Juan Quintela wrote:
> We create new channels for each new thread created. We only send through
> them a character to be sure that we are creating the channels in the
> right order.
> 
> Signed-off-by: Juan Quintela 
> ---
>  include/migration/migration.h |  7 +
>  migration/ram.c   | 33 ++
>  migration/socket.c| 64 
> +--
>  3 files changed, 101 insertions(+), 3 deletions(-)

[snip]

> diff --git a/migration/socket.c b/migration/socket.c
> index 13966f1..1c764f1 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -24,6 +24,62 @@
>  #include "io/channel-socket.h"
>  #include "trace.h"
> 
> +struct SocketArgs {
> +QIOChannelSocket *ioc;
> +SocketAddress *saddr;
> +Error **errp;
> +} socket_args;
> +
> +QIOChannel *socket_recv_channel_create(void)
> +{
> +QIOChannelSocket *sioc;
> +Error *err = NULL;
> +
> +sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(socket_args.ioc),
> + );
> +if (!sioc) {
> +error_report("could not accept migration connection (%s)",
> + error_get_pretty(err));
> +return NULL;
> +}
> +return QIO_CHANNEL(sioc);
> +}
> +
> +int socket_recv_channel_destroy(QIOChannel *recv)
> +{
> +/* Remove channel */
> +object_unref(OBJECT(send));
> +return 0;
> +}
> +
> +/* we have created all the recv channels, we can close the main one */
> +int socket_recv_channel_close_listening(void)
> +{
> +/* Close listening socket as its no longer needed */
> +qio_channel_close(QIO_CHANNEL(socket_args.ioc), NULL);
> +return 0;
> +}
> +
> +QIOChannel *socket_send_channel_create(void)
> +{
> +QIOChannelSocket *sioc = qio_channel_socket_new();
> +
> +qio_channel_socket_connect_sync(sioc, socket_args.saddr,
> +socket_args.errp);
> +qio_channel_set_delay(QIO_CHANNEL(sioc), false);
> +return QIO_CHANNEL(sioc);
> +}
> +
> +int socket_send_channel_destroy(QIOChannel *send)
> +{
> +/* Remove channel */
> +object_unref(OBJECT(send));
> +if (socket_args.saddr) {
> +qapi_free_SocketAddress(socket_args.saddr);
> +socket_args.saddr = NULL;
> +}
> +return 0;
> +}
> 
>  static SocketAddress *tcp_build_address(const char *host_port, Error **errp)
>  {
> @@ -97,6 +153,10 @@ static void 
> socket_start_outgoing_migration(MigrationState *s,
>  struct SocketConnectData *data = g_new0(struct SocketConnectData, 1);
> 
>  data->s = s;
> +
> +socket_args.saddr = saddr;
> +socket_args.errp = errp;
> +
>  if (saddr->type == SOCKET_ADDRESS_KIND_INET) {
>  data->hostname = g_strdup(saddr->u.inet.data->host);
>  }
> @@ -107,7 +167,6 @@ static void 
> socket_start_outgoing_migration(MigrationState *s,
>   socket_outgoing_migration,
>   data,
>   socket_connect_data_free);
> -qapi_free_SocketAddress(saddr);
>  }
> 
>  void tcp_start_outgoing_migration(MigrationState *s,
> @@ -154,8 +213,6 @@ static gboolean 
> socket_accept_incoming_migration(QIOChannel *ioc,
>  object_unref(OBJECT(sioc));
> 
>  out:
> -/* Close listening socket as its no longer needed */
> -qio_channel_close(ioc, NULL);
>  return FALSE; /* unregister */
>  }
> 
> @@ -164,6 +221,7 @@ static void socket_start_incoming_migration(SocketAddress 
> *saddr,
>  Error **errp)
>  {
>  QIOChannelSocket *listen_ioc = qio_channel_socket_new();
> +socket_args.ioc = listen_ioc;
> 
>  qio_channel_set_name(QIO_CHANNEL(listen_ioc),
>   "migration-socket-listener");

FYI I put some comments against v3 on this patch just as you sent this v4,
as I don't think the changes here are desirable in this format.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [PATCH 00/24] QemuOpts util/cutils: Fix and clean up number conversions

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH 00/24] QemuOpts util/cutils: Fix and clean up 
number conversions
Message-id: 1487067971-10443-1-git-send-email-arm...@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] 
patchew/1487067971-10443-1-git-send-email-arm...@redhat.com -> 
patchew/1487067971-10443-1-git-send-email-arm...@redhat.com
Switched to a new branch 'test'
855de21 QemuOpts: Fix checking of sizes for overflow and trailing crap
704c59b util/cutils: Change qemu_strtosz*() from int64_t to uint64_t
926a170 util/cutils: Return qemu_strtosz*() error and value separately
3efa6d4 util/cutils: Let qemu_strtosz*() optionally reject trailing crap
bd4e31d qemu-img: Wrap cvtnum() around qemu_strtosz()
18a3da0 tests/test-cutils: Drop suffix from test_qemu_strtosz_simple()
f2880dd tests/test-cutils: Use qemu_strtosz() more often
8e04b3b util/cutils: Drop QEMU_STRTOSZ_DEFSUFFIX_* macros
25f2bf6 util/cutils: New qemu_strtosz()
d133023 util/cutils: Rename qemu_strtosz() to qemu_strtosz_mebi()
5c33f1b util/cutils: New qemu_strtosz_metric()
567a856 tests/test-cutils: Cover qemu_strtosz() around range limits
5eea8e0 tests/test-cutils: Cover qemu_strtosz() with trailing crap
db765f5 tests/test-cutils: Cover qemu_strtosz() invalid input
958d0f3 tests/test-cutils: Add missing qemu_strtosz()... endptr checks
7c61791 QemuOpts: Fix to reject numbers that overflow uint64_t
29872d3 util/cutils: Clean up control flow around qemu_strtol() a bit
3a696e2 util/cutils: Clean up variable names around qemu_strtol()
b2e8b59 util/cutils: Rename qemu_strtoll(), qemu_strtoull()
5153908 util/cutils: Rewrite documentation of qemu_strtol() & friends
32a7846 tests/test-cutils: Clean up qemu_strtoul() result checks
2a602fb tests/test-cutils: Add missing qemu_strtol()... endptr checks
ebda9f0 QemuOpts: Assert value string isn't null
aced107 tests/test-qemu-opts: Cover qemu_opts_parse()

=== OUTPUT BEGIN ===
Checking PATCH 1/24: tests/test-qemu-opts: Cover qemu_opts_parse()...
Checking PATCH 2/24: QemuOpts: Assert value string isn't null...
ERROR: consider using qemu_strtoull in preference to strtoull
#75: FILE: util/qemu-option.c:147:
+number = strtoull(value, , 0);

total: 1 errors, 0 warnings, 117 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/24: tests/test-cutils: Add missing qemu_strtol()... endptr 
checks...
Checking PATCH 4/24: tests/test-cutils: Clean up qemu_strtoul() result checks...
Checking PATCH 5/24: util/cutils: Rewrite documentation of qemu_strtol() & 
friends...
Checking PATCH 6/24: util/cutils: Rename qemu_strtoll(), qemu_strtoull()...
Checking PATCH 7/24: util/cutils: Clean up variable names around 
qemu_strtol()...
ERROR: consider using qemu_strtol in preference to strtol
#76: FILE: util/cutils.c:316:
+*result = strtol(nptr, , base);

ERROR: consider using qemu_strtoul in preference to strtoul
#95: FILE: util/cutils.c:359:
+*result = strtoul(nptr, , base);

ERROR: consider using qemu_strtoll in preference to strtoll
#120: FILE: util/cutils.c:388:
+*result = strtoll(nptr, , base);

ERROR: consider using qemu_strtoull in preference to strtoull
#139: FILE: util/cutils.c:412:
+*result = strtoull(nptr, , base);

total: 4 errors, 0 warnings, 110 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 8/24: util/cutils: Clean up control flow around qemu_strtol() a 
bit...
ERROR: consider using qemu_strtol in preference to strtol
#81: FILE: util/cutils.c:322:
+*result = strtol(nptr, , base);

ERROR: consider using qemu_strtoul in preference to strtoul
#109: FILE: util/cutils.c:364:
+*result = strtoul(nptr, , base);

ERROR: consider using qemu_strtoll in preference to strtoll
#141: FILE: util/cutils.c:392:
+*result = strtoll(nptr, , base);

ERROR: consider using qemu_strtoull in preference to strtoull
#171: FILE: util/cutils.c:415:
+*result = strtoull(nptr, , base);

total: 4 errors, 0 warnings, 140 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, 

Re: [Qemu-devel] [PATCH v4] sd: sdhci: mask transfer mode register value

[Peter Maydell]

On 14 February 2017 at 11:02, P J P  wrote:
>   Hello Peter,
>
> +-- On Tue, 14 Feb 2017, Peter Maydell wrote --+
> | Please keep sending them as a single series, though. Otherwise I
> | won't be able to easily apply them in the right order and track
> | their current status.
>
>   Okay, got it. Ie. to re-send entire series even for change in one of the
> patches?

Yes, exactly. (We document this in
http://wiki.qemu-project.org/Contribute/SubmitAPatch#Participating_in_Code_Review
but of course that page is huge...)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4] sd: sdhci: mask transfer mode register value

[Peter Maydell]

On 14 February 2017 at 10:44, P J P  wrote:
> +-- On Tue, 14 Feb 2017, Peter Maydell wrote --+
> | What has happened to the other patches that were in this patchset in v3 ?
>
> v3 1/4
>   -> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg02776.html
>
> v3 3/4
>   -> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg02775.html
>
> These two are acked.
>
>
> v3 2/4
>   -> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg02846.html
>   -> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg02844.html
>
> v3 4/4
>   -> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg02380.html
>   -> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg01568.html
>
> These two were acked with few changes, which are incorporated in the second
> link above. Acks pending.

Please keep sending them as a single series, though. Otherwise I
won't be able to easily apply them in the right order and track
their current status.

thanks
-- PMM

[Qemu-devel] [PATCH 22/24] util/cutils: Return qemu_strtosz*() error and value separately

[Markus Armbruster]

This makes qemu_strtosz(), qemu_strtosz_mebi() and
qemu_strtosz_metric() similar to qemu_strtoi64(), except negative
values are rejected.

Cc: Dr. David Alan Gilbert 
Cc: Eduardo Habkost  (maintainer:X86)
Cc: Kevin Wolf  (supporter:Block layer core)
Cc: Max Reitz  (supporter:Block layer core)
Cc: qemu-bl...@nongnu.org (open list:Block layer core)
Signed-off-by: Markus Armbruster 
---
 hmp.c |   6 +--
 hw/misc/ivshmem.c |   7 ++-
 include/qemu/cutils.h |   6 +--
 monitor.c |   5 ++-
 qapi/opts-visitor.c   |   5 ++-
 qemu-img.c|  10 +++--
 qemu-io-cmds.c|  10 +++--
 target/i386/cpu.c |   5 ++-
 tests/test-cutils.c   | 120 ++
 util/cutils.c |  22 -
 10 files changed, 119 insertions(+), 77 deletions(-)

diff --git a/hmp.c b/hmp.c
index 0eb5b6d..9846fa4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1342,7 +1342,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
*qdict)
 long valueint = 0;
 Error *err = NULL;
 bool use_int_value = false;
-int i;
+int i, ret;
 
 for (i = 0; i < MIGRATION_PARAMETER__MAX; i++) {
 if (strcmp(param, MigrationParameter_lookup[i]) == 0) {
@@ -1378,8 +1378,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
*qdict)
 break;
 case MIGRATION_PARAMETER_MAX_BANDWIDTH:
 p.has_max_bandwidth = true;
-valuebw = qemu_strtosz_mebi(valuestr, NULL);
-if (valuebw < 0 || (size_t)valuebw != valuebw) {
+ret = qemu_strtosz_mebi(valuestr, NULL, );
+if (ret < 0 || (size_t)valuebw != valuebw) {
 error_setg(, "Invalid size %s", valuestr);
 goto cleanup;
 }
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index f00cd75..3dc04f4 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -1267,8 +1267,11 @@ static void ivshmem_realize(PCIDevice *dev, Error **errp)
 if (s->sizearg == NULL) {
 s->legacy_size = 4 << 20; /* 4 MB default */
 } else {
-int64_t size = qemu_strtosz_mebi(s->sizearg, NULL);
-if (size < 0 || (size_t)size != size || !is_power_of_2(size)) {
+int ret;
+int64_t size;
+
+ret = qemu_strtosz_mebi(s->sizearg, NULL, );
+if (ret < 0 || (size_t)size != size || !is_power_of_2(size)) {
 error_setg(errp, "Invalid size %s", s->sizearg);
 return;
 }
diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 4184851..c91649b 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -139,9 +139,9 @@ int parse_uint(const char *s, unsigned long long *value, 
char **endptr,
int base);
 int parse_uint_full(const char *s, unsigned long long *value, int base);
 
-int64_t qemu_strtosz(const char *nptr, char **end);
-int64_t qemu_strtosz_mebi(const char *nptr, char **end);
-int64_t qemu_strtosz_metric(const char *nptr, char **end);
+int qemu_strtosz(const char *nptr, char **end, int64_t *result);
+int qemu_strtosz_mebi(const char *nptr, char **end, int64_t *result);
+int qemu_strtosz_metric(const char *nptr, char **end, int64_t *result);
 
 #define K_BYTE (1ULL << 10)
 #define M_BYTE (1ULL << 20)
diff --git a/monitor.c b/monitor.c
index 1f8c031..85b1b61 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2773,6 +2773,7 @@ static QDict *monitor_parse_arguments(Monitor *mon,
 break;
 case 'o':
 {
+int ret;
 int64_t val;
 char *end;
 
@@ -2785,8 +2786,8 @@ static QDict *monitor_parse_arguments(Monitor *mon,
 break;
 }
 }
-val = qemu_strtosz_mebi(p, );
-if (val < 0) {
+ret = qemu_strtosz_mebi(p, , );
+if (ret < 0) {
 monitor_printf(mon, "invalid size\n");
 goto fail;
 }
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 911a0ee..aac2e09 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -482,14 +482,15 @@ opts_type_size(Visitor *v, const char *name, uint64_t 
*obj, Error **errp)
 OptsVisitor *ov = to_ov(v);
 const QemuOpt *opt;
 int64_t val;
+int err;
 
 opt = lookup_scalar(ov, name, errp);
 if (!opt) {
 return;
 }
 
-val = qemu_strtosz(opt->str ? opt->str : "", NULL);
-if (val < 0) {
+err = qemu_strtosz(opt->str ? opt->str : "", NULL, );
+if (err < 0) {
 error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
"a size value representible as a non-negative int64");
 return;
diff --git a/qemu-img.c b/qemu-img.c
index adcb511..89ced5a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -370,10 +370,14 @@ static int 

[Qemu-devel] [PATCH 02/24] QemuOpts: Assert value string isn't null

[Markus Armbruster]

Plenty of code relies on QemuOpt member @str not being null, including
qemu_opts_print(), qemu_opts_to_qdict(), and callbacks passed to
qemu_opt_foreach().

Begs the question whether it can be null.  Only opt_set() creates
QemuOpt.  It sets member @str to its argument @value.  Passing null
for @value would plant a time bomb.  Callers:

* opts_do_parse() can't pass null.

* qemu_opt_set() passes its argument @value.  Callers:

  - qemu_opts_from_qdict_1() can't pass null

  - qemu_opts_set() passes its argument @value, but none of its
callers pass null.

  - Many more outside qemu-option.c, but they shouldn't pass null,
either.

Assert member @str isn't null, so that misuse is caught right away.

Simplify parse_option_bool(), parse_option_number() and
parse_option_size() accordingly.  Best viewed with whitespace changes
ignored.

Signed-off-by: Markus Armbruster 
---
 util/qemu-option.c | 89 --
 1 file changed, 39 insertions(+), 50 deletions(-)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index 3467dc2..2b4e0c9 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -128,17 +128,13 @@ int get_param_value(char *buf, int buf_size,
 static void parse_option_bool(const char *name, const char *value, bool *ret,
   Error **errp)
 {
-if (value != NULL) {
-if (!strcmp(value, "on")) {
-*ret = 1;
-} else if (!strcmp(value, "off")) {
-*ret = 0;
-} else {
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-   name, "'on' or 'off'");
-}
-} else {
+if (!strcmp(value, "on")) {
 *ret = 1;
+} else if (!strcmp(value, "off")) {
+*ret = 0;
+} else {
+error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+   name, "'on' or 'off'");
 }
 }
 
@@ -148,16 +144,12 @@ static void parse_option_number(const char *name, const 
char *value,
 char *postfix;
 uint64_t number;
 
-if (value != NULL) {
-number = strtoull(value, , 0);
-if (*postfix != '\0') {
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
-return;
-}
-*ret = number;
-} else {
+number = strtoull(value, , 0);
+if (*postfix != '\0') {
 error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
+return;
 }
+*ret = number;
 }
 
 static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc,
@@ -180,39 +172,35 @@ void parse_option_size(const char *name, const char 
*value,
 char *postfix;
 double sizef;
 
-if (value != NULL) {
-sizef = strtod(value, );
-if (sizef < 0 || sizef > UINT64_MAX) {
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
- "a non-negative number below 2^64");
-return;
-}
-switch (*postfix) {
-case 'T':
-sizef *= 1024;
-/* fall through */
-case 'G':
-sizef *= 1024;
-/* fall through */
-case 'M':
-sizef *= 1024;
-/* fall through */
-case 'K':
-case 'k':
-sizef *= 1024;
-/* fall through */
-case 'b':
-case '\0':
-*ret = (uint64_t) sizef;
-break;
-default:
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
-error_append_hint(errp, "You may use k, M, G or T suffixes for "
-"kilobytes, megabytes, gigabytes and terabytes.\n");
-return;
-}
-} else {
+sizef = strtod(value, );
+if (sizef < 0 || sizef > UINT64_MAX) {
+error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
+   "a non-negative number below 2^64");
+return;
+}
+switch (*postfix) {
+case 'T':
+sizef *= 1024;
+/* fall through */
+case 'G':
+sizef *= 1024;
+/* fall through */
+case 'M':
+sizef *= 1024;
+/* fall through */
+case 'K':
+case 'k':
+sizef *= 1024;
+/* fall through */
+case 'b':
+case '\0':
+*ret = (uint64_t) sizef;
+break;
+default:
 error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
+error_append_hint(errp, "You may use k, M, G or T suffixes for "
+  "kilobytes, megabytes, gigabytes and terabytes.\n");
+return;
 }
 }
 
@@ -547,6 +535,7 @@ static void opt_set(QemuOpts *opts, const char *name, const 
char *value,
 }
 opt->desc = desc;
 opt->str = g_strdup(value);
+assert(opt->str);
 qemu_opt_parse(opt, _err);
 if (local_err) {
 error_propagate(errp, local_err);
-- 
2.7.4

[Qemu-devel] [PATCH 20/24] qemu-img: Wrap cvtnum() around qemu_strtosz()

[Markus Armbruster]

Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: qemu-bl...@nongnu.org
Signed-off-by: Markus Armbruster 
---
 qemu-img.c | 58 +++---
 1 file changed, 31 insertions(+), 27 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 4407244..2a47966 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -368,6 +368,19 @@ static int add_old_style_options(const char *fmt, QemuOpts 
*opts,
 return 0;
 }
 
+static int64_t cvtnum(const char *s)
+{
+char *end;
+int64_t ret;
+
+ret = qemu_strtosz(s, );
+if (*end != '\0') {
+/* Detritus at the end of the string */
+return -EINVAL;
+}
+return ret;
+}
+
 static int img_create(int argc, char **argv)
 {
 int c;
@@ -461,9 +474,9 @@ static int img_create(int argc, char **argv)
 /* Get image size, if specified */
 if (optind < argc) {
 int64_t sval;
-char *end;
-sval = qemu_strtosz(argv[optind++], );
-if (sval < 0 || *end) {
+
+sval = cvtnum(argv[optind++]);
+if (sval < 0) {
 if (sval == -ERANGE) {
 error_report("Image size must be less than 8 EiB!");
 } else {
@@ -1861,9 +1874,9 @@ static int img_convert(int argc, char **argv)
 case 'S':
 {
 int64_t sval;
-char *end;
-sval = qemu_strtosz(optarg, );
-if (sval < 0 || *end) {
+
+sval = cvtnum(optarg);
+if (sval < 0) {
 error_report("Invalid minimum zero buffer size for sparse 
output specified");
 ret = -1;
 goto fail_getopt;
@@ -3648,10 +3661,8 @@ static int img_bench(int argc, char **argv)
 break;
 case 'o':
 {
-char *end;
-errno = 0;
-offset = qemu_strtosz(optarg, );
-if (offset < 0|| *end) {
+offset = cvtnum(optarg);
+if (offset < 0) {
 error_report("Invalid offset specified");
 return 1;
 }
@@ -3664,10 +3675,9 @@ static int img_bench(int argc, char **argv)
 case 's':
 {
 int64_t sval;
-char *end;
 
-sval = qemu_strtosz(optarg, );
-if (sval < 0 || sval > INT_MAX || *end) {
+sval = cvtnum(optarg);
+if (sval < 0 || sval > INT_MAX) {
 error_report("Invalid buffer size specified");
 return 1;
 }
@@ -3678,10 +3688,9 @@ static int img_bench(int argc, char **argv)
 case 'S':
 {
 int64_t sval;
-char *end;
 
-sval = qemu_strtosz(optarg, );
-if (sval < 0 || sval > INT_MAX || *end) {
+sval = cvtnum(optarg);
+if (sval < 0 || sval > INT_MAX) {
 error_report("Invalid step size specified");
 return 1;
 }
@@ -3840,12 +3849,11 @@ static int img_dd_bs(const char *arg,
  struct DdIo *in, struct DdIo *out,
  struct DdInfo *dd)
 {
-char *end;
 int64_t res;
 
-res = qemu_strtosz(arg, );
+res = cvtnum(arg);
 
-if (res <= 0 || res > INT_MAX || *end) {
+if (res <= 0 || res > INT_MAX) {
 error_report("invalid number: '%s'", arg);
 return 1;
 }
@@ -3858,11 +3866,9 @@ static int img_dd_count(const char *arg,
 struct DdIo *in, struct DdIo *out,
 struct DdInfo *dd)
 {
-char *end;
+dd->count = cvtnum(arg);
 
-dd->count = qemu_strtosz(arg, );
-
-if (dd->count < 0 || *end) {
+if (dd->count < 0) {
 error_report("invalid number: '%s'", arg);
 return 1;
 }
@@ -3892,11 +3898,9 @@ static int img_dd_skip(const char *arg,
struct DdIo *in, struct DdIo *out,
struct DdInfo *dd)
 {
-char *end;
+in->offset = cvtnum(arg);
 
-in->offset = qemu_strtosz(arg, );
-
-if (in->offset < 0 || *end) {
+if (in->offset < 0) {
 error_report("invalid number: '%s'", arg);
 return 1;
 }
-- 
2.7.4