Series comparison

-[PATCH 00/14] Fix check-qtest-ppc64 sanitizer errors
+[PATCH v7 0/2] Fix check-qtest-ppc64 sanitizer errors
 I saw various sanitizer errors when running check-qtest-ppc64. While
 I could just turn off sanitizers, I decided to tackle them this time.
-Unfortunately, GLib does not free test data in some cases so some
+Unfortunately, GLib versions older than 2.81.0 do not free test data in
-sanitizer errors remain. All sanitizer errors will be gone with this
+some cases so some sanitizer errors remain. All sanitizer errors will be
-patch series combined with the following change for GLib:
+gone with this patch series combined with the following change for GLib:
 https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4120
 Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
 ---
-Akihiko Odaki (14):
+Changes in v7:
-      hw/core: Free CPUState allocations
+- Don't open code memory_region_ref(). (Peter Xu)
-      hw/ide: Free macio-ide IRQs
+- Link to v6: https://lore.kernel.org/r/20250105-san-v6-0-11fc859b99b7@daynix.com
-      hw/isa/vt82c686: Free irqs
-      spapr: Free stdout path
+Changes in v6:
-      ppc/vof: Fix unaligned FDT property access
+- Avoid referring owner as "the object that tracks the region's
-      hw/virtio: Free vqs before vhost_dev_cleanup()
+  reference count".
-      migration: Free removed SaveStateEntry
+- Noted that memroy_region_ref() and memroy_region_unref() do nothing
   if the owner is not present.
 - Explicitly stated that memory_region_unref() may destroy the owner
   along with the memory region itself.
 - Link to v5: https://lore.kernel.org/r/20250104-san-v5-0-8b430457b09d@daynix.com
 Changes in v5:
 - Rebased.
 - Merged four patches to update inline documentation into one
 - Link to v4: https://lore.kernel.org/r/20240823-san-v4-0-a24c6dfa4ceb@daynix.com
 Changes in v4:
 - Changed to create a reference to the subregion instead of its owner
   when its owner equals to the container's owner.
 - Dropped R-b from patch "memory: Do not create circular reference with
   subregion".
 - Rebased.
 - Link to v3: https://lore.kernel.org/r/20240708-san-v3-0-b03f671c40c6@daynix.com
 Changes in v3:
 - Added patch "memory: Clarify that we use owner's reference count".
 - Added patch "memory: Refer to docs/devel/memory.rst for 'owner'".
 - Fixed the message of patch
   "memory: Do not create circular reference with subregion".
 - Dropped patch "cpu: Free cpu_ases" in favor of:
   https://lore.kernel.org/r/20240607115649.214622-7-salil.mehta@huawei.com/
   ("[PATCH V13 6/8] physmem: Add helper function to destroy CPU
   AddressSpace")
 - Dropped patches "hw/ide: Convert macio ide_irq into GPIO line" and
   "hw/ide: Remove internal DMA qemu_irq" in favor of commit efb359346c7a
   ("hw/ide/macio: switch from using qemu_allocate_irq() to qdev input
   GPIOs")
 - Dropped patch "hw/isa/vt82c686: Define a GPIO line between vt82c686
   and i8259" in favor of:
   https://patchew.org/QEMU/20240704205854.18537-1-shentey@gmail.com/
   ("[PATCH 0/3] Resolve vt82c686 and piix4 qemu_irq memory leaks")
 - Dropped pulled patches.
 - Link to v2: https://lore.kernel.org/r/20240627-san-v2-0-750bb0946dbd@daynix.com
 Changes in v2:
 - Rebased to "[PATCH] cpu: fix memleak of 'halt_cond' and 'thread'".
   (Philippe Mathieu-Daudé)
 - Converted IRQs into GPIO lines and removed one qemu_irq usage.
   (Peter Maydell)
 - s/suppresses/fixes/ (Michael S. Tsirkin)
 - Corrected title of patch "hw/virtio: Free vqs after vhost_dev_cleanup()"
   (was "hw/virtio: Free vqs before vhost_dev_cleanup()")
 - Link to v1: https://lore.kernel.org/r/20240626-san-v1-0-f3cc42302189@daynix.com
 ---
 Akihiko Odaki (2):
       memory: Update inline documentation
       memory: Do not create circular reference with subregion
-      tests/qtest: Use qtest_add_data_func_full()
-      tests/qtest: Free unused QMP response
-      tests/qtest: Free old machine variable name
-      tests/qtest: Delete previous boot file
-      tests/qtest: Free paths
-      tests/qtest: Free GThread
- hw/core/cpu-common.c                 |  3 +++
+ include/exec/memory.h | 59 ++++++++++++++++++++++++---------------------------
- hw/ide/macio.c                       |  9 +++++++++
+ system/memory.c       | 24 +++++++++++++++++++--
- hw/isa/vt82c686.c                    |  3 ++-
+files changed, 50 insertions(+), 33 deletions(-)
  hw/ppc/spapr_vof.c                   |  2 +-
  hw/ppc/vof.c                         |  2 +-
  hw/virtio/vhost-user-base.c          |  2 ++
  migration/savevm.c                   |  2 ++
  system/memory.c                      | 11 +++++++++--
  tests/qtest/device-introspect-test.c |  7 +++----
  tests/qtest/libqtest.c               |  3 +++
  tests/qtest/migration-test.c         | 18 +++++++++++-------
  tests/qtest/qos-test.c               | 16 ++++++++++++----
  tests/qtest/vhost-user-test.c        |  6 +++---
 files changed, 61 insertions(+), 23 deletions(-)
 ---
-base-commit: 74abb45dac6979e7ff76172b7f0a24e869405184
+base-commit: 38d0939b86e2eef6f6a622c6f1f7befda0146595
 change-id: 20240625-san-097afaf4f1c2
 Best regards,
 --
 Akihiko Odaki <akihiko.odaki@daynix.com>

-[PATCH 01/14] hw/core: Free CPUState allocations
+Deleted patch
-This suppresses LeakSanitizer warnings.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
----
- hw/core/cpu-common.c | 3 +++
-file changed, 3 insertions(+)
-diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
-index XXXXXXX..XXXXXXX 100644
---- a/hw/core/cpu-common.c
-+++ b/hw/core/cpu-common.c
-@@ -XXX,XX +XXX,XX @@ static void cpu_common_finalize(Object *obj)
- {
-     CPUState *cpu = CPU(obj);
-+    g_free(cpu->thread);
-+    g_free(cpu->halt_cond);
-+    g_free(cpu->cpu_ases);
-     g_array_free(cpu->gdb_regs, TRUE);
-     qemu_lockcnt_destroy(&cpu->in_ioctl_lock);
-     qemu_mutex_destroy(&cpu->work_mutex);
---
-.45.2

-[PATCH 02/14] hw/ide: Free macio-ide IRQs
+Deleted patch
-This suppresses LeakSanitizer warnings.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
----
- hw/ide/macio.c | 9 +++++++++
-file changed, 9 insertions(+)
-diff --git a/hw/ide/macio.c b/hw/ide/macio.c
-index XXXXXXX..XXXXXXX 100644
---- a/hw/ide/macio.c
-+++ b/hw/ide/macio.c
-@@ -XXX,XX +XXX,XX @@ static void macio_ide_initfn(Object *obj)
-                              qdev_prop_allow_set_link_before_realize, 0);
- }
-+static void macio_ide_finalize(Object *obj)
-+{
-+    MACIOIDEState *s = MACIO_IDE(obj);
-+
-+    qemu_free_irq(s->dma_irq);
-+    qemu_free_irq(s->ide_irq);
-+}
-+
- static Property macio_ide_properties[] = {
-     DEFINE_PROP_UINT32("channel", MACIOIDEState, channel, 0),
-     DEFINE_PROP_UINT32("addr", MACIOIDEState, addr, -1),
-@@ -XXX,XX +XXX,XX @@ static const TypeInfo macio_ide_type_info = {
-     .parent = TYPE_SYS_BUS_DEVICE,
-     .instance_size = sizeof(MACIOIDEState),
-     .instance_init = macio_ide_initfn,
-+    .instance_finalize = macio_ide_finalize,
-     .class_init = macio_ide_class_init,
- };
---
-.45.2

-[PATCH 03/14] hw/isa/vt82c686: Free irqs
+Deleted patch
-This suppresses LeakSanitizer warnings.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
----
- hw/isa/vt82c686.c | 3 ++-
-file changed, 2 insertions(+), 1 deletion(-)
-diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
-index XXXXXXX..XXXXXXX 100644
---- a/hw/isa/vt82c686.c
-+++ b/hw/isa/vt82c686.c
-@@ -XXX,XX +XXX,XX @@ static void via_isa_realize(PCIDevice *d, Error **errp)
-     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
-     qdev_init_gpio_in_named(dev, via_isa_pirq, "pirq", PCI_NUM_PINS);
--    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
-     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
-                           errp);
-@@ -XXX,XX +XXX,XX @@ static void via_isa_realize(PCIDevice *d, Error **errp)
-         return;
-     }
-+    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
-     s->isa_irqs_in = i8259_init(isa_bus, *isa_irq);
-+    qemu_free_irqs(isa_irq, 1);
-     isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
-     i8254_pit_init(isa_bus, 0x40, 0, NULL);
-     i8257_dma_init(OBJECT(d), isa_bus, 0);
---
-.45.2

-[PATCH 04/14] spapr: Free stdout path
+Deleted patch
-This suppresses LeakSanitizer warnings.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
----
- hw/ppc/spapr_vof.c | 2 +-
-file changed, 1 insertion(+), 1 deletion(-)
-diff --git a/hw/ppc/spapr_vof.c b/hw/ppc/spapr_vof.c
-index XXXXXXX..XXXXXXX 100644
---- a/hw/ppc/spapr_vof.c
-+++ b/hw/ppc/spapr_vof.c
-@@ -XXX,XX +XXX,XX @@ target_ulong spapr_h_vof_client(PowerPCCPU *cpu, SpaprMachineState *spapr,
- void spapr_vof_client_dt_finalize(SpaprMachineState *spapr, void *fdt)
- {
--    char *stdout_path = spapr_vio_stdout_path(spapr->vio_bus);
-+    g_autofree char *stdout_path = spapr_vio_stdout_path(spapr->vio_bus);
-     vof_build_dt(fdt, spapr->vof);
---
-.45.2

-[PATCH 05/14] ppc/vof: Fix unaligned FDT property access
+Deleted patch
-FDT properties are aligned by 4 bytes, not 8 bytes.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
----
- hw/ppc/vof.c | 2 +-
-file changed, 1 insertion(+), 1 deletion(-)
-diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
-index XXXXXXX..XXXXXXX 100644
---- a/hw/ppc/vof.c
-+++ b/hw/ppc/vof.c
-@@ -XXX,XX +XXX,XX @@ static void vof_dt_memory_available(void *fdt, GArray *claimed, uint64_t base)
-     mem0_reg = fdt_getprop(fdt, offset, "reg", &proplen);
-     g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc));
-     if (sc == 2) {
--        mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg + sizeof(uint32_t) * ac));
-+        mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac);
-     } else {
-         mem0_end = be32_to_cpu(*(uint32_t *)(mem0_reg + sizeof(uint32_t) * ac));
-     }
---
-.45.2

-[PATCH 06/14] hw/virtio: Free vqs before vhost_dev_cleanup()
+Deleted patch
-This suppresses LeakSanitizer warnings.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
----
- hw/virtio/vhost-user-base.c | 2 ++
-file changed, 2 insertions(+)
-diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
-index XXXXXXX..XXXXXXX 100644
---- a/hw/virtio/vhost-user-base.c
-+++ b/hw/virtio/vhost-user-base.c
-@@ -XXX,XX +XXX,XX @@ static void vub_disconnect(DeviceState *dev)
- {
-     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-     VHostUserBase *vub = VHOST_USER_BASE(vdev);
-+    struct vhost_virtqueue *vhost_vqs = vub->vhost_dev.vqs;
-     if (!vub->connected) {
-         return;
-@@ -XXX,XX +XXX,XX @@ static void vub_disconnect(DeviceState *dev)
-     vub_stop(vdev);
-     vhost_dev_cleanup(&vub->vhost_dev);
-+    g_free(vhost_vqs);
-     /* Re-instate the event handler for new connections */
-     qemu_chr_fe_set_handlers(&vub->chardev,
---
-.45.2

-[PATCH 07/14] migration: Free removed SaveStateEntry
+Deleted patch
-This suppresses LeakSanitizer warnings.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
----
- migration/savevm.c | 2 ++
-file changed, 2 insertions(+)
-diff --git a/migration/savevm.c b/migration/savevm.c
-index XXXXXXX..XXXXXXX 100644
---- a/migration/savevm.c
-+++ b/migration/savevm.c
-@@ -XXX,XX +XXX,XX @@ int vmstate_replace_hack_for_ppc(VMStateIf *obj, int instance_id,
-     if (se) {
-         savevm_state_handler_remove(se);
-+        g_free(se->compat);
-+        g_free(se);
-     }
-     return vmstate_register(obj, instance_id, vmsd, opaque);
- }
---
-.45.2

-[PATCH 14/14] tests/qtest: Free GThread
+[PATCH v7 1/2] memory: Update inline documentation
-These GThreads are never referenced.
+Do not refer to "memory region's reference count"
 -------------------------------------------------
 Now MemoryRegions do have their own reference counts, but they will not
 be used when their owners are not themselves. However, the documentation
 of memory_region_ref() says it adds "1 to a memory region's reference
 count", which is confusing. Avoid referring to "memory region's
 reference count" and just say: "Add a reference to a memory region".
 Make a similar change to memory_region_unref() too.
 Refer to docs/devel/memory.rst for "owner"
 ------------------------------------------
 memory_region_ref() and memory_region_unref() used to have their own
 descriptions of "owner", but they are somewhat out-of-date and
 misleading.
 In particular, they say "whenever memory regions are accessed outside
 the BQL, they need to be preserved against hot-unplug", but protecting
 against hot-unplug is not mandatory if it is known that they will never
 be hot-unplugged. They also say "MemoryRegions actually do not have
 their own reference count", but they actually do. They just will not be
 used unless their owners are not themselves.
 Refer to docs/devel/memory.rst as the single source of truth instead of
 maintaining duplicate descriptions of "owner".
 Clarify that owner may be missing
 ---------------------------------
 A memory region may not have an owner, and memory_region_ref() and
 memory_region_unref() do nothing for such.
 memory: Clarify owner must not call memory_region_ref()
 --------------------------------------------------------
 The owner must not call this function as it results in a circular
 reference.
 Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
+Reviewed-by: Peter Xu <peterx@redhat.com>
 ---
- tests/qtest/vhost-user-test.c | 6 +++---
+ include/exec/memory.h | 59 ++++++++++++++++++++++++---------------------------
-file changed, 3 insertions(+), 3 deletions(-)
+file changed, 28 insertions(+), 31 deletions(-)
-diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
+diff --git a/include/exec/memory.h b/include/exec/memory.h
 index XXXXXXX..XXXXXXX 100644
---- a/tests/qtest/vhost-user-test.c
+--- a/include/exec/memory.h
-+++ b/tests/qtest/vhost-user-test.c
++++ b/include/exec/memory.h
-@@ -XXX,XX +XXX,XX @@ static void *vhost_user_test_setup_reconnect(GString *cmd_line, void *arg)
+@@ -XXX,XX +XXX,XX @@ void memory_region_section_free_copy(MemoryRegionSection *s);
- {
+  * memory_region_add_subregion() to add subregions.
-     TestServer *s = test_server_new("reconnect", arg);
+  *
+  * @mr: the #MemoryRegion to be initialized
--    g_thread_new("connect", connect_thread, s);
+- * @owner: the object that tracks the region's reference count
-+    g_thread_unref(g_thread_new("connect", connect_thread, s));
++ * @owner: the object that keeps the region alive
-     append_mem_opts(s, cmd_line, 256, TEST_MEMFD_AUTO);
+  * @name: used for debugging; not visible to the user or ABI
-     s->vu_ops->append_opts(s, cmd_line, ",server=on");
+  * @size: size of the region; any subregions beyond this size will be clipped
+  */
-@@ -XXX,XX +XXX,XX @@ static void *vhost_user_test_setup_connect_fail(GString *cmd_line, void *arg)
+@@ -XXX,XX +XXX,XX @@ void memory_region_init(MemoryRegion *mr,
+                         uint64_t size);
-     s->test_fail = true;
+ /**
--    g_thread_new("connect", connect_thread, s);
+- * memory_region_ref: Add 1 to a memory region's reference count
-+    g_thread_unref(g_thread_new("connect", connect_thread, s));
++ * memory_region_ref: Add a reference to the owner of a memory region
-     append_mem_opts(s, cmd_line, 256, TEST_MEMFD_AUTO);
+  *
-     s->vu_ops->append_opts(s, cmd_line, ",server=on");
+- * Whenever memory regions are accessed outside the BQL, they need to be
+- * preserved against hot-unplug.  MemoryRegions actually do not have their
-@@ -XXX,XX +XXX,XX @@ static void *vhost_user_test_setup_flags_mismatch(GString *cmd_line, void *arg)
+- * own reference count; they piggyback on a QOM object, their "owner".
+- * This function adds a reference to the owner.
-     s->test_flags = TEST_FLAGS_DISCONNECT;
+- *
+- * All MemoryRegions must have an owner if they can disappear, even if the
--    g_thread_new("connect", connect_thread, s);
+- * device they belong to operates exclusively under the BQL.  This is because
-+    g_thread_unref(g_thread_new("connect", connect_thread, s));
+- * the region could be returned at any time by memory_region_find, and this
-     append_mem_opts(s, cmd_line, 256, TEST_MEMFD_AUTO);
+- * is usually under guest control.
-     s->vu_ops->append_opts(s, cmd_line, ",server=on");
++ * This function adds a reference to the owner of a memory region to keep the
++ * memory region alive. It does nothing if the owner is not present as a memory
 + * region without owner will never die.
 + * For references internal to the owner, use object_ref() instead to avoid a
 + * circular reference.
 + * See docs/devel/memory.rst to know about owner.
   *
   * @mr: the #MemoryRegion
   */
  void memory_region_ref(MemoryRegion *mr);
  /**
 - * memory_region_unref: Remove 1 to a memory region's reference count
 + * memory_region_unref: Remove a reference to the memory region of the owner
   *
 - * Whenever memory regions are accessed outside the BQL, they need to be
 - * preserved against hot-unplug.  MemoryRegions actually do not have their
 - * own reference count; they piggyback on a QOM object, their "owner".
 - * This function removes a reference to the owner and possibly destroys it.
 + * This function removes a reference to the owner of a memory region and
 + * possibly destroys the owner along with the memory region. It does nothing if
 + * the owner is not present.
 + * See docs/devel/memory.rst to know about owner.
   *
   * @mr: the #MemoryRegion
   */
@@ -XXX,XX +XXX,XX @@ void memory_region_unref(MemoryRegion *mr);
   * if @size is nonzero, subregions will be clipped to @size.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @ops: a structure containing read and write callbacks to be used when
   *       I/O is performed on the region.
   * @opaque: passed to the read and write callbacks of the @ops structure.
@@ -XXX,XX +XXX,XX @@ void memory_region_init_io(MemoryRegion *mr,
   *                                    directly.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @name: Region name, becomes part of RAMBlock name used in migration stream
   *        must be unique within any device
   * @size: size of the region.
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_ram_nomigrate(MemoryRegion *mr,
   *                                          modify memory directly.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @name: Region name, becomes part of RAMBlock name used in migration stream
   *        must be unique within any device
   * @size: size of the region.
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
   *                                     canceled.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @name: Region name, becomes part of RAMBlock name used in migration stream
   *        must be unique within any device
   * @size: used size of the region.
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_resizeable_ram(MemoryRegion *mr,
   *                                    mmap-ed backend.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @name: Region name, becomes part of RAMBlock name used in migration stream
   *        must be unique within any device
   * @size: size of the region.
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_ram_from_file(MemoryRegion *mr,
   *                                  mmap-ed backend.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @name: the name of the region.
   * @size: size of the region.
   * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM,
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_ram_from_fd(MemoryRegion *mr,
   *                              region will modify memory directly.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @name: Region name, becomes part of RAMBlock name used in migration stream
   *        must be unique within any device
   * @size: size of the region.
@@ -XXX,XX +XXX,XX @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
   * skip_dump flag.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @name: the name of the region.
   * @size: size of the region.
   * @ptr: memory to be mapped; must contain at least @size bytes.
@@ -XXX,XX +XXX,XX @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr,
   *                           part of another memory region.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @name: used for debugging; not visible to the user or ABI
   * @orig: the region to be referenced; @mr will be equivalent to
   *        @orig between @offset and @offset + @size - 1.
@@ -XXX,XX +XXX,XX @@ void memory_region_init_alias(MemoryRegion *mr,
   * of the caller.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @name: Region name, becomes part of RAMBlock name used in migration stream
   *        must be unique within any device
   * @size: size of the region.
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_rom_nomigrate(MemoryRegion *mr,
   * of the caller.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @ops: callbacks for write access handling (must not be NULL).
   * @opaque: passed to the read and write callbacks of the @ops structure.
   * @name: Region name, becomes part of RAMBlock name used in migration stream
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
   * @_iommu_mr: the #IOMMUMemoryRegion to be initialized
   * @instance_size: the IOMMUMemoryRegion subclass instance size
   * @mrtypename: the type name of the #IOMMUMemoryRegion
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @name: used for debugging; not visible to the user or ABI
   * @size: size of the region.
   */
@@ -XXX,XX +XXX,XX @@ void memory_region_init_iommu(void *_iommu_mr,
   *                          region will modify memory directly.
   *
   * @mr: the #MemoryRegion to be initialized
 - * @owner: the object that tracks the region's reference count (must be
 + * @owner: the object that keeps the region alive (must be
   *         TYPE_DEVICE or a subclass of TYPE_DEVICE, or NULL)
   * @name: name of the memory region
   * @size: size of the region in bytes
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_ram_guest_memfd(MemoryRegion *mr,
   * If you pass a non-NULL non-device @owner then we will assert.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @name: Region name, becomes part of RAMBlock name used in migration stream
   *        must be unique within any device
   * @size: size of the region.
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_rom(MemoryRegion *mr,
   * If you pass a non-NULL non-device @owner then we will assert.
   *
   * @mr: the #MemoryRegion to be initialized.
 - * @owner: the object that tracks the region's reference count
 + * @owner: the object that keeps the region alive
   * @ops: callbacks for write access handling (must not be NULL).
   * @opaque: passed to the read and write callbacks of the @ops structure.
   * @name: Region name, becomes part of RAMBlock name used in migration stream
 --
-.45.2
+.47.1

-[PATCH 08/14] memory: Do not create circular reference with subregion
+[PATCH v7 2/2] memory: Do not create circular reference with subregion
-A memory region does not use their own reference counters, but instead
+memory_region_update_container_subregions() used to call
-piggybacks on another QOM object, "owner" (unless the owner is not the
+memory_region_ref(), which creates a reference to the owner of the
-memory region itself). When creating a subregion, a new reference to the
+subregion, on behalf of the owner of the container. This results in a
-owner of the container must be created. However, if the subregion is
+circular reference if the subregion and container have the same owner.
-owned by the same QOM object, this result in a self-reference, and make
-the owner immortal. Avoid such a self-reference.
+memory_region_ref() creates a reference to the owner instead of the
 memory region to match the lifetime of the owner and memory region. We
 do not need such a hack if the subregion and container have the same
 owner because the owner will be alive as long as the container is.
 Therefore, create a reference to the subregion itself instead ot its
 owner in such a case; the reference to the subregion is still necessary
 to ensure that the subregion gets finalized after the container.
 Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
 ---
- system/memory.c | 11 +++++++++--
+ system/memory.c | 24 ++++++++++++++++++++++--
-file changed, 9 insertions(+), 2 deletions(-)
+file changed, 22 insertions(+), 2 deletions(-)
 diff --git a/system/memory.c b/system/memory.c
 index XXXXXXX..XXXXXXX 100644
 --- a/system/memory.c
 +++ b/system/memory.c
-@@ -XXX,XX +XXX,XX @@ static void memory_region_update_container_subregions(MemoryRegion *subregion)
+@@ -XXX,XX +XXX,XX @@ void memory_region_unref(MemoryRegion *mr)
+     }
-     memory_region_transaction_begin();
+ }
--    memory_region_ref(subregion);
++static void memory_region_ref_subregion(MemoryRegion *subregion)
-+    if (mr->owner != subregion->owner) {
++{
 +    if (subregion->container->owner == subregion->owner) {
 +        object_ref(OBJECT(subregion));
 +    } else {
 +        memory_region_ref(subregion);
 +    }
++}
++
++static void memory_region_unref_subregion(MemoryRegion *subregion)
++{
++    if (subregion->container->owner == subregion->owner) {
++        object_unref(OBJECT(subregion));
++     } else {
++        memory_region_unref(subregion);
++     }
++}
++
+ uint64_t memory_region_size(MemoryRegion *mr)
+ {
+     if (int128_eq(mr->size, int128_2_64())) {
+@@ -XXX,XX +XXX,XX @@ static void memory_region_update_container_subregions(MemoryRegion *subregion)
+     memory_region_transaction_begin();
+-    memory_region_ref(subregion);
++    memory_region_ref_subregion(subregion);
 +
      QTAILQ_FOREACH(other, &mr->subregions, subregions_link) {
          if (subregion->priority >= other->priority) {
              QTAILQ_INSERT_BEFORE(other, subregion, subregions_link);
 @@ -XXX,XX +XXX,XX @@ void memory_region_del_subregion(MemoryRegion *mr,
          assert(alias->mapped_via_alias >= 0);
      }
      QTAILQ_REMOVE(&mr->subregions, subregion, subregions_link);
 -    memory_region_unref(subregion);
-+
++    memory_region_unref_subregion(subregion);
 +    if (mr->owner != subregion->owner) {
 +        memory_region_unref(subregion);
 +    }
 +
      memory_region_update_pending |= mr->enabled && subregion->enabled;
      memory_region_transaction_commit();
  }
 --
-.45.2
+.47.1

-[PATCH 09/14] tests/qtest: Use qtest_add_data_func_full()
+Deleted patch
-A test function may not be executed depending on the test command line
-so it is wrong to free data with a test function. Use
-qtest_add_data_func_full() to register a function to free data.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
----
- tests/qtest/device-introspect-test.c | 7 +++----
-file changed, 3 insertions(+), 4 deletions(-)
-diff --git a/tests/qtest/device-introspect-test.c b/tests/qtest/device-introspect-test.c
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qtest/device-introspect-test.c
-+++ b/tests/qtest/device-introspect-test.c
-@@ -XXX,XX +XXX,XX @@ static void test_device_intro_concrete(const void *args)
-     qobject_unref(types);
-     qtest_quit(qts);
--    g_free((void *)args);
- }
- static void test_abstract_interfaces(void)
-@@ -XXX,XX +XXX,XX @@ static void add_machine_test_case(const char *mname)
-     path = g_strdup_printf("device/introspect/concrete/defaults/%s", mname);
-     args = g_strdup_printf("-M %s", mname);
--    qtest_add_data_func(path, args, test_device_intro_concrete);
-+    qtest_add_data_func_full(path, args, test_device_intro_concrete, g_free);
-     g_free(path);
-     path = g_strdup_printf("device/introspect/concrete/nodefaults/%s", mname);
-     args = g_strdup_printf("-nodefaults -M %s", mname);
--    qtest_add_data_func(path, args, test_device_intro_concrete);
-+    qtest_add_data_func_full(path, args, test_device_intro_concrete, g_free);
-     g_free(path);
- }
-@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
-     qtest_add_func("device/introspect/abstract-interfaces", test_abstract_interfaces);
-     if (g_test_quick()) {
-         qtest_add_data_func("device/introspect/concrete/defaults/none",
--                            g_strdup(common_args), test_device_intro_concrete);
-+                            common_args, test_device_intro_concrete);
-     } else {
-         qtest_cb_for_every_machine(add_machine_test_case, true);
-     }
---
-.45.2

-[PATCH 10/14] tests/qtest: Free unused QMP response
+Deleted patch
-This suppresses LeakSanitizer warnings.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
----
- tests/qtest/libqtest.c | 2 ++
-file changed, 2 insertions(+)
-diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qtest/libqtest.c
-+++ b/tests/qtest/libqtest.c
-@@ -XXX,XX +XXX,XX @@ QDict *qtest_qmp_receive(QTestState *s)
-                         response, s->eventData)) {
-             /* Stash the event for a later consumption */
-             s->pending_events = g_list_append(s->pending_events, response);
-+        } else {
-+            qobject_unref(response);
-         }
-     }
- }
---
-.45.2

-[PATCH 11/14] tests/qtest: Free old machine variable name
+Deleted patch
-This suppresses LeakSanitizer warnings.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
----
- tests/qtest/libqtest.c | 1 +
-file changed, 1 insertion(+)
-diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qtest/libqtest.c
-+++ b/tests/qtest/libqtest.c
-@@ -XXX,XX +XXX,XX @@ static struct MachInfo *qtest_get_machines(const char *var)
-     int idx;
-     if (g_strcmp0(qemu_var, var)) {
-+        g_free(qemu_var);
-         qemu_var = g_strdup(var);
-         /* new qemu, clear the cache */
---
-.45.2

-[PATCH 12/14] tests/qtest: Delete previous boot file
+Deleted patch
-A test run may create boot files several times. Delete the previous boot
-file before creating a new one.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
----
- tests/qtest/migration-test.c | 18 +++++++++++-------
-file changed, 11 insertions(+), 7 deletions(-)
-diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qtest/migration-test.c
-+++ b/tests/qtest/migration-test.c
-@@ -XXX,XX +XXX,XX @@ static char *bootpath;
- #include "tests/migration/aarch64/a-b-kernel.h"
- #include "tests/migration/s390x/a-b-bios.h"
-+static void bootfile_delete(void)
-+{
-+    unlink(bootpath);
-+    g_free(bootpath);
-+    bootpath = NULL;
-+}
-+
- static void bootfile_create(char *dir, bool suspend_me)
- {
-     const char *arch = qtest_get_arch();
-     unsigned char *content;
-     size_t len;
-+    if (bootpath) {
-+        bootfile_delete();
-+    }
-+
-     bootpath = g_strdup_printf("%s/bootsect", dir);
-     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-         /* the assembled x86 boot sector should be exactly one sector large */
-@@ -XXX,XX +XXX,XX @@ static void bootfile_create(char *dir, bool suspend_me)
-     fclose(bootfile);
- }
--static void bootfile_delete(void)
--{
--    unlink(bootpath);
--    g_free(bootpath);
--    bootpath = NULL;
--}
--
- /*
-  * Wait for some output in the serial output file,
-  * we get an 'A' followed by an endless string of 'B's
---
-.45.2

-[PATCH 13/14] tests/qtest: Free paths
+Deleted patch
-This suppresses LeakSanitizer warnings.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
----
- tests/qtest/qos-test.c | 16 ++++++++++++----
-file changed, 12 insertions(+), 4 deletions(-)
-diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qtest/qos-test.c
-+++ b/tests/qtest/qos-test.c
-@@ -XXX,XX +XXX,XX @@
- static char *old_path;
--
- /**
-  * qos_set_machines_devices_available(): sets availability of qgraph
-  * machines and devices.
-@@ -XXX,XX +XXX,XX @@ static void subprocess_run_one_test(const void *arg)
-     g_test_trap_assert_passed();
- }
-+static void destroy_pathv(void *arg)
-+{
-+    g_free(((char **)arg)[0]);
-+    g_free(arg);
-+}
-+
- /*
-  * in this function, 2 path will be built:
-  * path_str, a one-string path (ex "pc/i440FX-pcihost/...")
-@@ -XXX,XX +XXX,XX @@ static void walk_path(QOSGraphNode *orig_path, int len)
-     if (path->u.test.subprocess) {
-         gchar *subprocess_path = g_strdup_printf("/%s/%s/subprocess",
-                                                  qtest_get_arch(), path_str);
--        qtest_add_data_func(path_str, subprocess_path, subprocess_run_one_test);
--        g_test_add_data_func(subprocess_path, path_vec, run_one_test);
-+        qtest_add_data_func_full(path_str, subprocess_path,
-+                                 subprocess_run_one_test, g_free);
-+        g_test_add_data_func_full(subprocess_path, path_vec,
-+                                  run_one_test, destroy_pathv);
-     } else {
--        qtest_add_data_func(path_str, path_vec, run_one_test);
-+        qtest_add_data_func_full(path_str, path_vec,
-+                                 run_one_test, destroy_pathv);
-     }
-     g_free(path_str);
---
-.45.2

I saw various sanitizer errors when running check-qtest-ppc64. While
I could just turn off sanitizers, I decided to tackle them this time.

Unfortunately, GLib does not free test data in some cases so some
sanitizer errors remain. All sanitizer errors will be gone with this
patch series combined with the following change for GLib:
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4120

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Akihiko Odaki (14):
      hw/core: Free CPUState allocations
      hw/ide: Free macio-ide IRQs
      hw/isa/vt82c686: Free irqs
      spapr: Free stdout path
      ppc/vof: Fix unaligned FDT property access
      hw/virtio: Free vqs before vhost_dev_cleanup()
      migration: Free removed SaveStateEntry
      memory: Do not create circular reference with subregion
      tests/qtest: Use qtest_add_data_func_full()
      tests/qtest: Free unused QMP response
      tests/qtest: Free old machine variable name
      tests/qtest: Delete previous boot file
      tests/qtest: Free paths
      tests/qtest: Free GThread

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>

This suppresses LeakSanitizer warnings.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 hw/ide/macio.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -XXX,XX +XXX,XX @@ static void macio_ide_initfn(Object *obj)
                              qdev_prop_allow_set_link_before_realize, 0);
 }
 
+static void macio_ide_finalize(Object *obj)
+{
+    MACIOIDEState *s = MACIO_IDE(obj);
+
+    qemu_free_irq(s->dma_irq);
+    qemu_free_irq(s->ide_irq);
+}
+
 static Property macio_ide_properties[] = {
     DEFINE_PROP_UINT32("channel", MACIOIDEState, channel, 0),
     DEFINE_PROP_UINT32("addr", MACIOIDEState, addr, -1),
@@ -XXX,XX +XXX,XX @@ static const TypeInfo macio_ide_type_info = {
     .parent = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(MACIOIDEState),
     .instance_init = macio_ide_initfn,
+    .instance_finalize = macio_ide_finalize,
     .class_init = macio_ide_class_init,
 };

-- 
2.45.2

This suppresses LeakSanitizer warnings.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 hw/isa/vt82c686.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -XXX,XX +XXX,XX @@ static void via_isa_realize(PCIDevice *d, Error **errp)
 
     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
     qdev_init_gpio_in_named(dev, via_isa_pirq, "pirq", PCI_NUM_PINS);
-    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
                           errp);
 
@@ -XXX,XX +XXX,XX @@ static void via_isa_realize(PCIDevice *d, Error **errp)
         return;
     }
 
+    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
     s->isa_irqs_in = i8259_init(isa_bus, *isa_irq);
+    qemu_free_irqs(isa_irq, 1);
     isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
     i8257_dma_init(OBJECT(d), isa_bus, 0);

-- 
2.45.2

A memory region does not use their own reference counters, but instead
piggybacks on another QOM object, "owner" (unless the owner is not the
memory region itself). When creating a subregion, a new reference to the
owner of the container must be created. However, if the subregion is
owned by the same QOM object, this result in a self-reference, and make
the owner immortal. Avoid such a self-reference.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 system/memory.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index XXXXXXX..XXXXXXX 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -XXX,XX +XXX,XX @@ static void memory_region_update_container_subregions(MemoryRegion *subregion)
 
     memory_region_transaction_begin();
 
-    memory_region_ref(subregion);
+    if (mr->owner != subregion->owner) {
+        memory_region_ref(subregion);
+    }
+
     QTAILQ_FOREACH(other, &mr->subregions, subregions_link) {
         if (subregion->priority >= other->priority) {
             QTAILQ_INSERT_BEFORE(other, subregion, subregions_link);
@@ -XXX,XX +XXX,XX @@ void memory_region_del_subregion(MemoryRegion *mr,
         assert(alias->mapped_via_alias >= 0);
     }
     QTAILQ_REMOVE(&mr->subregions, subregion, subregions_link);
-    memory_region_unref(subregion);
+
+    if (mr->owner != subregion->owner) {
+        memory_region_unref(subregion);
+    }
+
     memory_region_update_pending |= mr->enabled && subregion->enabled;
     memory_region_transaction_commit();
 }

-- 
2.45.2

A test function may not be executed depending on the test command line
so it is wrong to free data with a test function. Use
qtest_add_data_func_full() to register a function to free data.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 tests/qtest/device-introspect-test.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/device-introspect-test.c b/tests/qtest/device-introspect-test.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/qtest/device-introspect-test.c
+++ b/tests/qtest/device-introspect-test.c
@@ -XXX,XX +XXX,XX @@ static void test_device_intro_concrete(const void *args)
 
     qobject_unref(types);
     qtest_quit(qts);
-    g_free((void *)args);
 }
 
 static void test_abstract_interfaces(void)
@@ -XXX,XX +XXX,XX @@ static void add_machine_test_case(const char *mname)
 
     path = g_strdup_printf("device/introspect/concrete/defaults/%s", mname);
     args = g_strdup_printf("-M %s", mname);
-    qtest_add_data_func(path, args, test_device_intro_concrete);
+    qtest_add_data_func_full(path, args, test_device_intro_concrete, g_free);
     g_free(path);
 
     path = g_strdup_printf("device/introspect/concrete/nodefaults/%s", mname);
     args = g_strdup_printf("-nodefaults -M %s", mname);
-    qtest_add_data_func(path, args, test_device_intro_concrete);
+    qtest_add_data_func_full(path, args, test_device_intro_concrete, g_free);
     g_free(path);
 }
 
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
     qtest_add_func("device/introspect/abstract-interfaces", test_abstract_interfaces);
     if (g_test_quick()) {
         qtest_add_data_func("device/introspect/concrete/defaults/none",
-                            g_strdup(common_args), test_device_intro_concrete);
+                            common_args, test_device_intro_concrete);
     } else {
         qtest_cb_for_every_machine(add_machine_test_case, true);
     }

-- 
2.45.2

A test run may create boot files several times. Delete the previous boot
file before creating a new one.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 tests/qtest/migration-test.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -XXX,XX +XXX,XX @@ static char *bootpath;
 #include "tests/migration/aarch64/a-b-kernel.h"
 #include "tests/migration/s390x/a-b-bios.h"
 
+static void bootfile_delete(void)
+{
+    unlink(bootpath);
+    g_free(bootpath);
+    bootpath = NULL;
+}
+
 static void bootfile_create(char *dir, bool suspend_me)
 {
     const char *arch = qtest_get_arch();
     unsigned char *content;
     size_t len;
 
+    if (bootpath) {
+        bootfile_delete();
+    }
+
     bootpath = g_strdup_printf("%s/bootsect", dir);
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         /* the assembled x86 boot sector should be exactly one sector large */
@@ -XXX,XX +XXX,XX @@ static void bootfile_create(char *dir, bool suspend_me)
     fclose(bootfile);
 }
 
-static void bootfile_delete(void)
-{
-    unlink(bootpath);
-    g_free(bootpath);
-    bootpath = NULL;
-}
-
 /*
  * Wait for some output in the serial output file,
  * we get an 'A' followed by an endless string of 'B's

-- 
2.45.2

This suppresses LeakSanitizer warnings.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 tests/qtest/qos-test.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/qtest/qos-test.c
+++ b/tests/qtest/qos-test.c
@@ -XXX,XX +XXX,XX @@
 static char *old_path;
 
 
-
 /**
  * qos_set_machines_devices_available(): sets availability of qgraph
  * machines and devices.
@@ -XXX,XX +XXX,XX @@ static void subprocess_run_one_test(const void *arg)
     g_test_trap_assert_passed();
 }
 
+static void destroy_pathv(void *arg)
+{
+    g_free(((char **)arg)[0]);
+    g_free(arg);
+}
+
 /*
  * in this function, 2 path will be built:
  * path_str, a one-string path (ex "pc/i440FX-pcihost/...")
@@ -XXX,XX +XXX,XX @@ static void walk_path(QOSGraphNode *orig_path, int len)
     if (path->u.test.subprocess) {
         gchar *subprocess_path = g_strdup_printf("/%s/%s/subprocess",
                                                  qtest_get_arch(), path_str);
-        qtest_add_data_func(path_str, subprocess_path, subprocess_run_one_test);
-        g_test_add_data_func(subprocess_path, path_vec, run_one_test);
+        qtest_add_data_func_full(path_str, subprocess_path,
+                                 subprocess_run_one_test, g_free);
+        g_test_add_data_func_full(subprocess_path, path_vec,
+                                  run_one_test, destroy_pathv);
     } else {
-        qtest_add_data_func(path_str, path_vec, run_one_test);
+        qtest_add_data_func_full(path_str, path_vec,
+                                 run_one_test, destroy_pathv);
     }
 
     g_free(path_str);

-- 
2.45.2

These GThreads are never referenced.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 tests/qtest/vhost-user-test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -XXX,XX +XXX,XX @@ static void *vhost_user_test_setup_reconnect(GString *cmd_line, void *arg)
 {
     TestServer *s = test_server_new("reconnect", arg);
 
-    g_thread_new("connect", connect_thread, s);
+    g_thread_unref(g_thread_new("connect", connect_thread, s));
     append_mem_opts(s, cmd_line, 256, TEST_MEMFD_AUTO);
     s->vu_ops->append_opts(s, cmd_line, ",server=on");
 
@@ -XXX,XX +XXX,XX @@ static void *vhost_user_test_setup_connect_fail(GString *cmd_line, void *arg)
 
     s->test_fail = true;
 
-    g_thread_new("connect", connect_thread, s);
+    g_thread_unref(g_thread_new("connect", connect_thread, s));
     append_mem_opts(s, cmd_line, 256, TEST_MEMFD_AUTO);
     s->vu_ops->append_opts(s, cmd_line, ",server=on");
 
@@ -XXX,XX +XXX,XX @@ static void *vhost_user_test_setup_flags_mismatch(GString *cmd_line, void *arg)
 
     s->test_flags = TEST_FLAGS_DISCONNECT;
 
-    g_thread_new("connect", connect_thread, s);
+    g_thread_unref(g_thread_new("connect", connect_thread, s));
     append_mem_opts(s, cmd_line, 256, TEST_MEMFD_AUTO);
     s->vu_ops->append_opts(s, cmd_line, ",server=on");

-- 
2.45.2

I saw various sanitizer errors when running check-qtest-ppc64. While
I could just turn off sanitizers, I decided to tackle them this time.

Unfortunately, GLib versions older than 2.81.0 do not free test data in
some cases so some sanitizer errors remain. All sanitizer errors will be
gone with this patch series combined with the following change for GLib:
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4120

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Changes in v7:
- Don't open code memory_region_ref(). (Peter Xu)
- Link to v6: https://lore.kernel.org/r/20250105-san-v6-0-11fc859b99b7@daynix.com

Changes in v6:
- Avoid referring owner as "the object that tracks the region's
  reference count".
- Noted that memroy_region_ref() and memroy_region_unref() do nothing
  if the owner is not present.
- Explicitly stated that memory_region_unref() may destroy the owner
  along with the memory region itself.
- Link to v5: https://lore.kernel.org/r/20250104-san-v5-0-8b430457b09d@daynix.com

Changes in v5:
- Rebased.
- Merged four patches to update inline documentation into one
- Link to v4: https://lore.kernel.org/r/20240823-san-v4-0-a24c6dfa4ceb@daynix.com

Changes in v4:
- Changed to create a reference to the subregion instead of its owner
  when its owner equals to the container's owner.
- Dropped R-b from patch "memory: Do not create circular reference with
  subregion".
- Rebased.
- Link to v3: https://lore.kernel.org/r/20240708-san-v3-0-b03f671c40c6@daynix.com

Changes in v3:
- Added patch "memory: Clarify that we use owner's reference count".
- Added patch "memory: Refer to docs/devel/memory.rst for 'owner'".
- Fixed the message of patch
  "memory: Do not create circular reference with subregion".
- Dropped patch "cpu: Free cpu_ases" in favor of:
  https://lore.kernel.org/r/20240607115649.214622-7-salil.mehta@huawei.com/
  ("[PATCH V13 6/8] physmem: Add helper function to destroy CPU
  AddressSpace")
- Dropped patches "hw/ide: Convert macio ide_irq into GPIO line" and
  "hw/ide: Remove internal DMA qemu_irq" in favor of commit efb359346c7a
  ("hw/ide/macio: switch from using qemu_allocate_irq() to qdev input
  GPIOs")
- Dropped patch "hw/isa/vt82c686: Define a GPIO line between vt82c686
  and i8259" in favor of:
  https://patchew.org/QEMU/20240704205854.18537-1-shentey@gmail.com/
  ("[PATCH 0/3] Resolve vt82c686 and piix4 qemu_irq memory leaks")
- Dropped pulled patches.
- Link to v2: https://lore.kernel.org/r/20240627-san-v2-0-750bb0946dbd@daynix.com

Changes in v2:
- Rebased to "[PATCH] cpu: fix memleak of 'halt_cond' and 'thread'".
  (Philippe Mathieu-Daudé)
- Converted IRQs into GPIO lines and removed one qemu_irq usage.
  (Peter Maydell)
- s/suppresses/fixes/ (Michael S. Tsirkin)
- Corrected title of patch "hw/virtio: Free vqs after vhost_dev_cleanup()"
  (was "hw/virtio: Free vqs before vhost_dev_cleanup()")
- Link to v1: https://lore.kernel.org/r/20240626-san-v1-0-f3cc42302189@daynix.com

---
Akihiko Odaki (2):
      memory: Update inline documentation
      memory: Do not create circular reference with subregion

include/exec/memory.h | 59 ++++++++++++++++++++++++---------------------------
 system/memory.c       | 24 +++++++++++++++++++--
 2 files changed, 50 insertions(+), 33 deletions(-)
---
base-commit: 38d0939b86e2eef6f6a622c6f1f7befda0146595
change-id: 20240625-san-097afaf4f1c2

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>

Do not refer to "memory region's reference count"
-------------------------------------------------

Now MemoryRegions do have their own reference counts, but they will not
be used when their owners are not themselves. However, the documentation
of memory_region_ref() says it adds "1 to a memory region's reference
count", which is confusing. Avoid referring to "memory region's
reference count" and just say: "Add a reference to a memory region".
Make a similar change to memory_region_unref() too.

Refer to docs/devel/memory.rst for "owner"
------------------------------------------

memory_region_ref() and memory_region_unref() used to have their own
descriptions of "owner", but they are somewhat out-of-date and
misleading.

In particular, they say "whenever memory regions are accessed outside
the BQL, they need to be preserved against hot-unplug", but protecting
against hot-unplug is not mandatory if it is known that they will never
be hot-unplugged. They also say "MemoryRegions actually do not have
their own reference count", but they actually do. They just will not be
used unless their owners are not themselves.

Refer to docs/devel/memory.rst as the single source of truth instead of
maintaining duplicate descriptions of "owner".

Clarify that owner may be missing

---------------------------------
A memory region may not have an owner, and memory_region_ref() and
memory_region_unref() do nothing for such.

memory: Clarify owner must not call memory_region_ref()
--------------------------------------------------------

The owner must not call this function as it results in a circular
reference.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
 include/exec/memory.h | 59 ++++++++++++++++++++++++---------------------------
 1 file changed, 28 insertions(+), 31 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index XXXXXXX..XXXXXXX 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -XXX,XX +XXX,XX @@ void memory_region_section_free_copy(MemoryRegionSection *s);
  * memory_region_add_subregion() to add subregions.
  *
  * @mr: the #MemoryRegion to be initialized
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @name: used for debugging; not visible to the user or ABI
  * @size: size of the region; any subregions beyond this size will be clipped
  */
@@ -XXX,XX +XXX,XX @@ void memory_region_init(MemoryRegion *mr,
                         uint64_t size);
 
 /**
- * memory_region_ref: Add 1 to a memory region's reference count
+ * memory_region_ref: Add a reference to the owner of a memory region
  *
- * Whenever memory regions are accessed outside the BQL, they need to be
- * preserved against hot-unplug.  MemoryRegions actually do not have their
- * own reference count; they piggyback on a QOM object, their "owner".
- * This function adds a reference to the owner.
- *
- * All MemoryRegions must have an owner if they can disappear, even if the
- * device they belong to operates exclusively under the BQL.  This is because
- * the region could be returned at any time by memory_region_find, and this
- * is usually under guest control.
+ * This function adds a reference to the owner of a memory region to keep the
+ * memory region alive. It does nothing if the owner is not present as a memory
+ * region without owner will never die.
+ * For references internal to the owner, use object_ref() instead to avoid a
+ * circular reference.
+ * See docs/devel/memory.rst to know about owner.
  *
  * @mr: the #MemoryRegion
  */
 void memory_region_ref(MemoryRegion *mr);
 
 /**
- * memory_region_unref: Remove 1 to a memory region's reference count
+ * memory_region_unref: Remove a reference to the memory region of the owner
  *
- * Whenever memory regions are accessed outside the BQL, they need to be
- * preserved against hot-unplug.  MemoryRegions actually do not have their
- * own reference count; they piggyback on a QOM object, their "owner".
- * This function removes a reference to the owner and possibly destroys it.
+ * This function removes a reference to the owner of a memory region and
+ * possibly destroys the owner along with the memory region. It does nothing if
+ * the owner is not present.
+ * See docs/devel/memory.rst to know about owner.
  *
  * @mr: the #MemoryRegion
  */
@@ -XXX,XX +XXX,XX @@ void memory_region_unref(MemoryRegion *mr);
  * if @size is nonzero, subregions will be clipped to @size.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @ops: a structure containing read and write callbacks to be used when
  *       I/O is performed on the region.
  * @opaque: passed to the read and write callbacks of the @ops structure.
@@ -XXX,XX +XXX,XX @@ void memory_region_init_io(MemoryRegion *mr,
  *                                    directly.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @name: Region name, becomes part of RAMBlock name used in migration stream
  *        must be unique within any device
  * @size: size of the region.
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_ram_nomigrate(MemoryRegion *mr,
  *                                          modify memory directly.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @name: Region name, becomes part of RAMBlock name used in migration stream
  *        must be unique within any device
  * @size: size of the region.
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
  *                                     canceled.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @name: Region name, becomes part of RAMBlock name used in migration stream
  *        must be unique within any device
  * @size: used size of the region.
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_resizeable_ram(MemoryRegion *mr,
  *                                    mmap-ed backend.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @name: Region name, becomes part of RAMBlock name used in migration stream
  *        must be unique within any device
  * @size: size of the region.
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_ram_from_file(MemoryRegion *mr,
  *                                  mmap-ed backend.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @name: the name of the region.
  * @size: size of the region.
  * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM,
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_ram_from_fd(MemoryRegion *mr,
  *                              region will modify memory directly.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @name: Region name, becomes part of RAMBlock name used in migration stream
  *        must be unique within any device
  * @size: size of the region.
@@ -XXX,XX +XXX,XX @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
  * skip_dump flag.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @name: the name of the region.
  * @size: size of the region.
  * @ptr: memory to be mapped; must contain at least @size bytes.
@@ -XXX,XX +XXX,XX @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr,
  *                           part of another memory region.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @name: used for debugging; not visible to the user or ABI
  * @orig: the region to be referenced; @mr will be equivalent to
  *        @orig between @offset and @offset + @size - 1.
@@ -XXX,XX +XXX,XX @@ void memory_region_init_alias(MemoryRegion *mr,
  * of the caller.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @name: Region name, becomes part of RAMBlock name used in migration stream
  *        must be unique within any device
  * @size: size of the region.
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_rom_nomigrate(MemoryRegion *mr,
  * of the caller.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @ops: callbacks for write access handling (must not be NULL).
  * @opaque: passed to the read and write callbacks of the @ops structure.
  * @name: Region name, becomes part of RAMBlock name used in migration stream
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
  * @_iommu_mr: the #IOMMUMemoryRegion to be initialized
  * @instance_size: the IOMMUMemoryRegion subclass instance size
  * @mrtypename: the type name of the #IOMMUMemoryRegion
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @name: used for debugging; not visible to the user or ABI
  * @size: size of the region.
  */
@@ -XXX,XX +XXX,XX @@ void memory_region_init_iommu(void *_iommu_mr,
  *                          region will modify memory directly.
  *
  * @mr: the #MemoryRegion to be initialized
- * @owner: the object that tracks the region's reference count (must be
+ * @owner: the object that keeps the region alive (must be
  *         TYPE_DEVICE or a subclass of TYPE_DEVICE, or NULL)
  * @name: name of the memory region
  * @size: size of the region in bytes
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_ram_guest_memfd(MemoryRegion *mr,
  * If you pass a non-NULL non-device @owner then we will assert.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @name: Region name, becomes part of RAMBlock name used in migration stream
  *        must be unique within any device
  * @size: size of the region.
@@ -XXX,XX +XXX,XX @@ bool memory_region_init_rom(MemoryRegion *mr,
  * If you pass a non-NULL non-device @owner then we will assert.
  *
  * @mr: the #MemoryRegion to be initialized.
- * @owner: the object that tracks the region's reference count
+ * @owner: the object that keeps the region alive
  * @ops: callbacks for write access handling (must not be NULL).
  * @opaque: passed to the read and write callbacks of the @ops structure.
  * @name: Region name, becomes part of RAMBlock name used in migration stream

-- 
2.47.1

memory_region_update_container_subregions() used to call
memory_region_ref(), which creates a reference to the owner of the
subregion, on behalf of the owner of the container. This results in a
circular reference if the subregion and container have the same owner.

memory_region_ref() creates a reference to the owner instead of the
memory region to match the lifetime of the owner and memory region. We
do not need such a hack if the subregion and container have the same
owner because the owner will be alive as long as the container is.
Therefore, create a reference to the subregion itself instead ot its
owner in such a case; the reference to the subregion is still necessary
to ensure that the subregion gets finalized after the container.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 system/memory.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index XXXXXXX..XXXXXXX 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -XXX,XX +XXX,XX @@ void memory_region_unref(MemoryRegion *mr)
     }
 }
 
+static void memory_region_ref_subregion(MemoryRegion *subregion)
+{
+    if (subregion->container->owner == subregion->owner) {
+        object_ref(OBJECT(subregion));
+    } else {
+        memory_region_ref(subregion);
+    }
+}
+
+static void memory_region_unref_subregion(MemoryRegion *subregion)
+{
+    if (subregion->container->owner == subregion->owner) {
+        object_unref(OBJECT(subregion));
+     } else {
+        memory_region_unref(subregion);
+     }
+}
+
 uint64_t memory_region_size(MemoryRegion *mr)
 {
     if (int128_eq(mr->size, int128_2_64())) {
@@ -XXX,XX +XXX,XX @@ static void memory_region_update_container_subregions(MemoryRegion *subregion)
 
     memory_region_transaction_begin();
 
-    memory_region_ref(subregion);
+    memory_region_ref_subregion(subregion);
+
     QTAILQ_FOREACH(other, &mr->subregions, subregions_link) {
         if (subregion->priority >= other->priority) {
             QTAILQ_INSERT_BEFORE(other, subregion, subregions_link);
@@ -XXX,XX +XXX,XX @@ void memory_region_del_subregion(MemoryRegion *mr,
         assert(alias->mapped_via_alias >= 0);
     }
     QTAILQ_REMOVE(&mr->subregions, subregion, subregions_link);
-    memory_region_unref(subregion);
+    memory_region_unref_subregion(subregion);
+
     memory_region_update_pending |= mr->enabled && subregion->enabled;
     memory_region_transaction_commit();
 }

-- 
2.47.1