[Qemu-devel] [PULL 00/20] SCSI and memory changes for 2014-08-18

[Qemu-devel] [PULL 00/20] SCSI and memory changes for 2014-08-18

[Paolo Bonzini]

The following changes since commit 5a7348045091a2bc15d85bb177e5956aa6114e5a:

  Update version for v2.1.0-rc2 release (2014-07-15 18:55:37 +0100)

are available in the git repository at:

  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to f54bb15f9d373877954e44db3a8bb368aff45b42:

  mtree: remove write-only field (2014-08-18 12:06:21 +0200)

----------------------------------------------------------------
SCSI changes that enable sending vendor-specific commands via virtio-scsi.

Memory changes for QOMification and automatic tracking of MR lifetime.

----------------------------------------------------------------
Paolo Bonzini (15):
      scsi-bus: prepare scsi_req_new for introduction of parse_cdb
      scsi-bus: introduce parse_cdb in SCSIDeviceClass and SCSIBusInfo
      scsi-block: extract scsi_block_is_passthrough
      scsi-block, scsi-generic: implement parse_cdb
      virtio-scsi: implement parse_cdb
      qom: object: delete properties before calling instance_finalize
      qom: object: move unparenting to the child property's release callback
      sysbus: remove unused function sysbus_del_io
      vga: do not dynamically allocate chain4_alias
      nic: do not destroy memory regions in cleanup functions
      ioport: split deletion and destruction
      memory: convert memory_region_destroy to object_unparent
      memory: remove memory_region_destroy
      tpm_tis: remove instance_finalize callback
      mtree: remove write-only field

Peter Crosthwaite (5):
      loader: Abstract away ref to memory region names
      exec: Abstract away ref to memory region names
      memory: constify memory_region_name
      memory: Use memory_region_name for name access
      memory: Use canonical path component as the name

 backends/hostmem.c             | 10 ------
 docs/memory.txt                | 15 ++++++---
 exec.c                         |  4 +--
 hw/audio/ac97.c                |  9 -----
 hw/audio/es1370.c              |  8 -----
 hw/audio/intel-hda.c           |  1 -
 hw/block/nvme.c                |  1 -
 hw/block/pflash_cfi01.c        |  1 -
 hw/block/pflash_cfi02.c        |  1 -
 hw/char/serial-pci.c           |  3 --
 hw/core/loader.c               |  2 +-
 hw/core/sysbus.c               |  5 ---
 hw/display/vga.c               | 24 ++++++--------
 hw/display/vga_int.h           |  3 +-
 hw/i386/kvm/pci-assign.c       |  8 -----
 hw/i386/kvmvapic.c             |  2 +-
 hw/ide/ahci.c                  |  2 --
 hw/ide/cmd646.c                |  5 ---
 hw/ide/piix.c                  |  3 --
 hw/ide/via.c                   |  3 --
 hw/ipack/tpci200.c             | 13 --------
 hw/mips/gt64xxx_pci.c          |  2 +-
 hw/misc/ivshmem.c              |  3 --
 hw/misc/omap_gpmc.c            |  2 +-
 hw/misc/pci-testdev.c          |  2 --
 hw/misc/vfio.c                 | 11 ++-----
 hw/net/dp8393x.c               |  3 --
 hw/net/e1000.c                 |  2 --
 hw/net/eepro100.c              |  3 --
 hw/net/mcf_fec.c               |  3 --
 hw/net/ne2000.c                |  1 -
 hw/net/pcnet-pci.c             |  2 --
 hw/net/rtl8139.c               |  2 --
 hw/net/stellaris_enet.c        |  8 -----
 hw/net/vmxnet3.c               |  4 ---
 hw/pci-bridge/pci_bridge_dev.c |  2 --
 hw/pci/msix.c                  |  4 ---
 hw/pci/pci.c                   |  2 --
 hw/pci/pci_bridge.c            |  8 -----
 hw/pci/pcie_host.c             |  1 -
 hw/pci/shpc.c                  |  1 -
 hw/ppc/ppc4xx_devs.c           |  2 +-
 hw/scsi/esp-pci.c              |  1 -
 hw/scsi/lsi53c895a.c           | 10 ------
 hw/scsi/megasas.c              |  3 --
 hw/scsi/scsi-bus.c             | 74 +++++++++++++++++++++++++++++-------------
 hw/scsi/scsi-disk.c            | 52 ++++++++++++++++++++++-------
 hw/scsi/scsi-generic.c         |  7 ++++
 hw/scsi/virtio-scsi.c          | 25 ++++++++++++++
 hw/scsi/vmw_pvscsi.c           |  3 --
 hw/tpm/tpm_tis.c               |  9 -----
 hw/usb/hcd-uhci.c              |  8 -----
 hw/virtio/virtio-pci.c         |  3 --
 hw/watchdog/wdt_i6300esb.c     |  8 -----
 hw/xen/xen_pt.c                | 20 ------------
 hw/xen/xen_pt_msi.c            |  2 --
 include/exec/memory.h          | 12 +------
 include/hw/scsi/scsi.h         |  7 ++++
 include/hw/sysbus.h            |  1 -
 ioport.c                       | 11 +++++--
 memory.c                       | 28 +++++-----------
 qom/object.c                   | 16 +++------
 62 files changed, 183 insertions(+), 308 deletions(-)
-- 
1.8.3.1

[Qemu-devel] [PULL 01/20] scsi-bus: prepare scsi_req_new for introduction of parse_cdb

[Paolo Bonzini]

The per-SCSIDevice parse_cdb callback must not be called if the
request will go through special SCSIReqOps, so detect the special
cases early enough.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c | 51 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 4341754..ca4e9f3 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -561,13 +561,38 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
                           uint8_t *buf, void *hba_private)
 {
     SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus);
+    const SCSIReqOps *ops;
     SCSIRequest *req;
-    SCSICommand cmd;
+    SCSICommand cmd = { .len = 0 };
+    int ret;
+
+    if ((d->unit_attention.key == UNIT_ATTENTION ||
+         bus->unit_attention.key == UNIT_ATTENTION) &&
+        (buf[0] != INQUIRY &&
+         buf[0] != REPORT_LUNS &&
+         buf[0] != GET_CONFIGURATION &&
+         buf[0] != GET_EVENT_STATUS_NOTIFICATION &&
+
+         /*
+          * If we already have a pending unit attention condition,
+          * report this one before triggering another one.
+          */
+         !(buf[0] == REQUEST_SENSE && d->sense_is_ua))) {
+        ops = &reqops_unit_attention;
+    } else if (lun != d->lun ||
+               buf[0] == REPORT_LUNS ||
+               (buf[0] == REQUEST_SENSE && d->sense_len)) {
+        ops = &reqops_target_command;
+    } else {
+        ops = NULL;
+    }
 
-    if (scsi_req_parse(&cmd, d, buf) != 0) {
+    ret = scsi_req_parse(&cmd, d, buf);
+    if (ret != 0) {
         trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
         req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private);
     } else {
+        assert(cmd.len != 0);
         trace_scsi_req_parsed(d->id, lun, tag, buf[0],
                               cmd.mode, cmd.xfer);
         if (cmd.lba != -1) {
@@ -577,25 +602,8 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
 
         if (cmd.xfer > INT32_MAX) {
             req = scsi_req_alloc(&reqops_invalid_field, d, tag, lun, hba_private);
-        } else if ((d->unit_attention.key == UNIT_ATTENTION ||
-                   bus->unit_attention.key == UNIT_ATTENTION) &&
-                  (buf[0] != INQUIRY &&
-                   buf[0] != REPORT_LUNS &&
-                   buf[0] != GET_CONFIGURATION &&
-                   buf[0] != GET_EVENT_STATUS_NOTIFICATION &&
-
-                   /*
-                    * If we already have a pending unit attention condition,
-                    * report this one before triggering another one.
-                    */
-                   !(buf[0] == REQUEST_SENSE && d->sense_is_ua))) {
-            req = scsi_req_alloc(&reqops_unit_attention, d, tag, lun,
-                                 hba_private);
-        } else if (lun != d->lun ||
-                   buf[0] == REPORT_LUNS ||
-                   (buf[0] == REQUEST_SENSE && d->sense_len)) {
-            req = scsi_req_alloc(&reqops_target_command, d, tag, lun,
-                                 hba_private);
+        } else if (ops) {
+            req = scsi_req_alloc(ops, d, tag, lun, hba_private);
         } else {
             req = scsi_device_alloc_req(d, tag, lun, buf, hba_private);
         }
@@ -1186,6 +1194,7 @@ static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
 {
     int rc;
 
+    cmd->lba = -1;
     switch (buf[0] >> 5) {
     case 0:
         cmd->len = 6;
-- 
1.8.3.1

[Qemu-devel] [PULL 02/20] scsi-bus: introduce parse_cdb in SCSIDeviceClass and SCSIBusInfo

[Paolo Bonzini]

These callbacks will let devices do their own request parsing, or
defer it to the bus.  If the bus does not provide an implementation,
in turn, fall back to the default parsing routine.

Swap the first two arguments to scsi_req_parse, and rename it to
scsi_req_parse_cdb, for consistency.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c     | 26 +++++++++++++++++++++++---
 include/hw/scsi/scsi.h |  6 ++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index ca4e9f3..d97d18a 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -9,7 +9,7 @@
 
 static char *scsibus_get_dev_path(DeviceState *dev);
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
-static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
+static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf);
 static void scsi_req_dequeue(SCSIRequest *req);
 static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len);
 static void scsi_target_free_buf(SCSIRequest *req);
@@ -54,6 +54,20 @@ static void scsi_device_destroy(SCSIDevice *s)
     }
 }
 
+int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+                       void *hba_private)
+{
+    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
+    int rc;
+
+    assert(cmd->len == 0);
+    rc = scsi_req_parse_cdb(dev, cmd, buf);
+    if (bus->info->parse_cdb) {
+        rc = bus->info->parse_cdb(dev, cmd, buf, hba_private);
+    }
+    return rc;
+}
+
 static SCSIRequest *scsi_device_alloc_req(SCSIDevice *s, uint32_t tag, uint32_t lun,
                                           uint8_t *buf, void *hba_private)
 {
@@ -562,6 +576,7 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
 {
     SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus);
     const SCSIReqOps *ops;
+    SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(d);
     SCSIRequest *req;
     SCSICommand cmd = { .len = 0 };
     int ret;
@@ -587,7 +602,12 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
         ops = NULL;
     }
 
-    ret = scsi_req_parse(&cmd, d, buf);
+    if (ops != NULL || !sc->parse_cdb) {
+        ret = scsi_req_parse_cdb(d, &cmd, buf);
+    } else {
+        ret = sc->parse_cdb(d, &cmd, buf, hba_private);
+    }
+
     if (ret != 0) {
         trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
         req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private);
@@ -1190,7 +1210,7 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd)
     return lba;
 }
 
-static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
+static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf)
 {
     int rc;
 
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 1adb549..4a0b860 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -76,6 +76,8 @@ typedef struct SCSIDeviceClass {
     DeviceClass parent_class;
     int (*init)(SCSIDevice *dev);
     void (*destroy)(SCSIDevice *s);
+    int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+                     void *hba_private);
     SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
                               uint8_t *buf, void *hba_private);
     void (*unit_attention_reported)(SCSIDevice *s);
@@ -131,6 +133,8 @@ struct SCSIReqOps {
 struct SCSIBusInfo {
     int tcq;
     int max_channel, max_target, max_lun;
+    int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+                     void *hba_private);
     void (*transfer_data)(SCSIRequest *req, uint32_t arg);
     void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid);
     void (*cancel)(SCSIRequest *req);
@@ -244,6 +248,8 @@ void scsi_req_free(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
 void scsi_req_unref(SCSIRequest *req);
 
+int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+                       void *hba_private);
 void scsi_req_build_sense(SCSIRequest *req, SCSISense sense);
 void scsi_req_print(SCSIRequest *req);
 void scsi_req_continue(SCSIRequest *req);
-- 
1.8.3.1

[Qemu-devel] [PULL 03/20] scsi-block: extract scsi_block_is_passthrough

[Paolo Bonzini]

This will be used for both scsi_block_new_request and the scsi-block
implementation of parse_cdb.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-disk.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index d47ecd6..81b7276 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2501,12 +2501,8 @@ static int scsi_block_initfn(SCSIDevice *dev)
     return scsi_initfn(&s->qdev);
 }
 
-static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
-                                           uint32_t lun, uint8_t *buf,
-                                           void *hba_private)
+static bool scsi_block_is_passthrough(SCSIDiskState *s, uint8_t *buf)
 {
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
-
     switch (buf[0]) {
     case READ_6:
     case READ_10:
@@ -2523,9 +2519,9 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
     case WRITE_VERIFY_12:
     case WRITE_VERIFY_16:
         /* If we are not using O_DIRECT, we might read stale data from the
-	 * host cache if writes were made using other commands than these
-	 * ones (such as WRITE SAME or EXTENDED COPY, etc.).  So, without
-	 * O_DIRECT everything must go through SG_IO.
+         * host cache if writes were made using other commands than these
+         * ones (such as WRITE SAME or EXTENDED COPY, etc.).  So, without
+         * O_DIRECT everything must go through SG_IO.
          */
         if (!(bdrv_get_flags(s->qdev.conf.bs) & BDRV_O_NOCACHE)) {
             break;
@@ -2542,13 +2538,31 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
          * just make scsi-block operate the same as scsi-generic for them.
          */
         if (s->qdev.type != TYPE_ROM) {
-            return scsi_req_alloc(&scsi_disk_dma_reqops, &s->qdev, tag, lun,
-                                  hba_private);
+            return false;
         }
+        break;
+
+    default:
+        break;
     }
 
-    return scsi_req_alloc(&scsi_generic_req_ops, &s->qdev, tag, lun,
-                          hba_private);
+    return true;
+}
+
+
+static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
+                                           uint32_t lun, uint8_t *buf,
+                                           void *hba_private)
+{
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+
+    if (scsi_block_is_passthrough(s, buf)) {
+        return scsi_req_alloc(&scsi_generic_req_ops, &s->qdev, tag, lun,
+                              hba_private);
+    } else {
+        return scsi_req_alloc(&scsi_disk_dma_reqops, &s->qdev, tag, lun,
+                              hba_private);
+    }
 }
 #endif
 
-- 
1.8.3.1

[Qemu-devel] [PULL 04/20] scsi-block, scsi-generic: implement parse_cdb

[Paolo Bonzini]

The callback lets the bus provide the direction and transfer count
for passthrough commands, enabling passthrough of vendor-specific
commands.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c     |  3 +--
 hw/scsi/scsi-disk.c    | 14 ++++++++++++++
 hw/scsi/scsi-generic.c |  7 +++++++
 include/hw/scsi/scsi.h |  1 +
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index d97d18a..6f4462b 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -9,7 +9,6 @@
 
 static char *scsibus_get_dev_path(DeviceState *dev);
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
-static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf);
 static void scsi_req_dequeue(SCSIRequest *req);
 static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len);
 static void scsi_target_free_buf(SCSIRequest *req);
@@ -1210,7 +1209,7 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd)
     return lba;
 }
 
-static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf)
+int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf)
 {
     int rc;
 
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 81b7276..d55521d 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2564,6 +2564,19 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
                               hba_private);
     }
 }
+
+static int scsi_block_parse_cdb(SCSIDevice *d, SCSICommand *cmd,
+                                  uint8_t *buf, void *hba_private)
+{
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+
+    if (scsi_block_is_passthrough(s, buf)) {
+        return scsi_bus_parse_cdb(&s->qdev, cmd, buf, hba_private);
+    } else {
+        return scsi_req_parse_cdb(&s->qdev, cmd, buf);
+    }
+}
+
 #endif
 
 #define DEFINE_SCSI_DISK_PROPERTIES()                                \
@@ -2672,6 +2685,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, void *data)
     sc->init         = scsi_block_initfn;
     sc->destroy      = scsi_destroy;
     sc->alloc_req    = scsi_block_new_request;
+    sc->parse_cdb    = scsi_block_parse_cdb;
     dc->fw_name = "disk";
     dc->desc = "SCSI block device passthrough";
     dc->reset = scsi_disk_reset;
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 3733d2c..0b2ff90 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -490,6 +490,12 @@ static Property scsi_generic_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static int scsi_generic_parse_cdb(SCSIDevice *dev, SCSICommand *cmd,
+                                  uint8_t *buf, void *hba_private)
+{
+    return scsi_bus_parse_cdb(dev, cmd, buf, hba_private);
+}
+
 static void scsi_generic_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -498,6 +504,7 @@ static void scsi_generic_class_initfn(ObjectClass *klass, void *data)
     sc->init         = scsi_generic_initfn;
     sc->destroy      = scsi_destroy;
     sc->alloc_req    = scsi_new_request;
+    sc->parse_cdb    = scsi_generic_parse_cdb;
     dc->fw_name = "disk";
     dc->desc = "pass through generic scsi device (/dev/sg*)";
     dc->reset = scsi_generic_reset;
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 4a0b860..a7a28e6 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -250,6 +250,7 @@ void scsi_req_unref(SCSIRequest *req);
 
 int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
                        void *hba_private);
+int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf);
 void scsi_req_build_sense(SCSIRequest *req, SCSISense sense);
 void scsi_req_print(SCSIRequest *req);
 void scsi_req_continue(SCSIRequest *req);
-- 
1.8.3.1

[Qemu-devel] [PULL 05/20] virtio-scsi: implement parse_cdb

[Paolo Bonzini]

Enable passthrough of vendor-specific commands.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/virtio-scsi.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 0eb069a..2dd9255 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -406,6 +406,30 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
     virtio_scsi_complete_cmd_req(req);
 }
 
+static int virtio_scsi_parse_cdb(SCSIDevice *dev, SCSICommand *cmd,
+                                 uint8_t *buf, void *hba_private)
+{
+    VirtIOSCSIReq *req = hba_private;
+
+    if (cmd->len == 0) {
+        cmd->len = MIN(VIRTIO_SCSI_CDB_SIZE, SCSI_CMD_BUF_SIZE);
+        memcpy(cmd->buf, buf, cmd->len);
+    }
+
+    /* Extract the direction and mode directly from the request, for
+     * host device passthrough.
+     */
+    cmd->xfer = req->qsgl.size;
+    if (cmd->xfer == 0) {
+        cmd->mode = SCSI_XFER_NONE;
+    } else if (iov_size(req->elem.in_sg, req->elem.in_num) > req->resp_size) {
+        cmd->mode = SCSI_XFER_FROM_DEV;
+    } else {
+        cmd->mode = SCSI_XFER_TO_DEV;
+    }
+    return 0;
+}
+
 static QEMUSGList *virtio_scsi_get_sg_list(SCSIRequest *r)
 {
     VirtIOSCSIReq *req = r->hba_private;
@@ -658,6 +682,7 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
     .change = virtio_scsi_change,
     .hotplug = virtio_scsi_hotplug,
     .hot_unplug = virtio_scsi_hot_unplug,
+    .parse_cdb = virtio_scsi_parse_cdb,
     .get_sg_list = virtio_scsi_get_sg_list,
     .save_request = virtio_scsi_save_request,
     .load_request = virtio_scsi_load_request,
-- 
1.8.3.1

[Qemu-devel] [PULL 06/20] qom: object: delete properties before calling instance_finalize

[Paolo Bonzini]

This ensures that the children's unparent callback will still
have a usable parent.

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qom/object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 0e8267b..f301bc2 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -418,8 +418,8 @@ static void object_finalize(void *data)
     Object *obj = data;
     TypeImpl *ti = obj->class->type;
 
-    object_deinit(obj, ti);
     object_property_del_all(obj);
+    object_deinit(obj, ti);
 
     g_assert(obj->ref == 0);
     if (obj->free) {
-- 
1.8.3.1

[Qemu-devel] [PULL 07/20] qom: object: move unparenting to the child property's release callback

[Paolo Bonzini]

This ensures that the unparent callback is called automatically
when the parent object is finalized.

Note that there's no need to keep a reference neither in
object_unparent nor in object_finalize_child_property.  The
reference held by the child property itself will do.

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qom/object.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index f301bc2..1b00831 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -387,19 +387,9 @@ static void object_property_del_child(Object *obj, Object *child, Error **errp)
 
 void object_unparent(Object *obj)
 {
-    if (!obj->parent) {
-        return;
-    }
-
-    object_ref(obj);
-    if (obj->class->unparent) {
-        (obj->class->unparent)(obj);
-    }
     if (obj->parent) {
         object_property_del_child(obj->parent, obj, NULL);
-        obj->parent = NULL;
     }
-    object_unref(obj);
 }
 
 static void object_deinit(Object *obj, TypeImpl *type)
@@ -1042,6 +1032,10 @@ static void object_finalize_child_property(Object *obj, const char *name,
 {
     Object *child = opaque;
 
+    if (child->class->unparent) {
+        (child->class->unparent)(child);
+    }
+    child->parent = NULL;
     object_unref(child);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 08/20] sysbus: remove unused function sysbus_del_io

[Paolo Bonzini]

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/sysbus.c    | 5 -----
 include/hw/sysbus.h | 1 -
 2 files changed, 6 deletions(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index f4e760d..414e2a1 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -242,11 +242,6 @@ void sysbus_add_io(SysBusDevice *dev, hwaddr addr,
     memory_region_add_subregion(get_system_io(), addr, mem);
 }
 
-void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem)
-{
-    memory_region_del_subregion(get_system_io(), mem);
-}
-
 MemoryRegion *sysbus_address_space(SysBusDevice *dev)
 {
     return get_system_memory();
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index f5aaa05..0bb91a8 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -71,7 +71,6 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, hwaddr addr,
                              int priority);
 void sysbus_add_io(SysBusDevice *dev, hwaddr addr,
                    MemoryRegion *mem);
-void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem);
 MemoryRegion *sysbus_address_space(SysBusDevice *dev);
 
 /* Legacy helper function for creating devices.  */
-- 
1.8.3.1

[Qemu-devel] [PULL 09/20] vga: do not dynamically allocate chain4_alias

[Paolo Bonzini]

Instead, add a boolean variable to indicate the presence of the region.
This avoids a repeated malloc/free (later we can also avoid the
add_child/unparent by changing the offset/size of the alias).

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/display/vga.c     | 24 ++++++++++--------------
 hw/display/vga_int.h |  3 ++-
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 4b089a3..68cfee2 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -168,15 +168,18 @@ static uint8_t expand4to8[16];
 
 static void vga_update_memory_access(VGACommonState *s)
 {
-    MemoryRegion *region, *old_region = s->chain4_alias;
     hwaddr base, offset, size;
 
     if (s->legacy_address_space == NULL) {
         return;
     }
 
-    s->chain4_alias = NULL;
-
+    if (s->has_chain4_alias) {
+        memory_region_del_subregion(s->legacy_address_space, &s->chain4_alias);
+        memory_region_destroy(&s->chain4_alias);
+        s->has_chain4_alias = false;
+        s->plane_updated = 0xf;
+    }
     if ((s->sr[VGA_SEQ_PLANE_WRITE] & VGA_SR02_ALL_PLANES) ==
         VGA_SR02_ALL_PLANES && s->sr[VGA_SEQ_MEMORY_MODE] & VGA_SR04_CHN_4M) {
         offset = 0;
@@ -201,18 +204,11 @@ static void vga_update_memory_access(VGACommonState *s)
             break;
         }
         base += isa_mem_base;
-        region = g_malloc(sizeof(*region));
-        memory_region_init_alias(region, memory_region_owner(&s->vram),
+        memory_region_init_alias(&s->chain4_alias, memory_region_owner(&s->vram),
                                  "vga.chain4", &s->vram, offset, size);
         memory_region_add_subregion_overlap(s->legacy_address_space, base,
-                                            region, 2);
-        s->chain4_alias = region;
-    }
-    if (old_region) {
-        memory_region_del_subregion(s->legacy_address_space, old_region);
-        memory_region_destroy(old_region);
-        g_free(old_region);
-        s->plane_updated = 0xf;
+                                            &s->chain4_alias, 2);
+        s->has_chain4_alias = true;
     }
 }
 
@@ -1321,7 +1317,7 @@ static void vga_draw_text(VGACommonState *s, int full_update)
         s->font_offsets[1] = offset;
         full_update = 1;
     }
-    if (s->plane_updated & (1 << 2) || s->chain4_alias) {
+    if (s->plane_updated & (1 << 2) || s->has_chain4_alias) {
         /* if the plane 2 was modified since the last display, it
            indicates the font may have been modified */
         s->plane_updated = 0;
diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
index 5320abd..641f8f4 100644
--- a/hw/display/vga_int.h
+++ b/hw/display/vga_int.h
@@ -94,7 +94,8 @@ typedef struct VGACommonState {
     uint32_t vram_size;
     uint32_t vram_size_mb; /* property */
     uint32_t latch;
-    MemoryRegion *chain4_alias;
+    bool has_chain4_alias;
+    MemoryRegion chain4_alias;
     uint8_t sr_index;
     uint8_t sr[256];
     uint8_t gr_index;
-- 
1.8.3.1

[Qemu-devel] [PULL 10/20] nic: do not destroy memory regions in cleanup functions

[Paolo Bonzini]

The memory regions should be destroyed in the unrealize function;
since these NICs are not even qdev-ified, they cannot be unplugged
and they do not have to do anything to destroy their memory regions.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/net/dp8393x.c | 3 ---
 hw/net/mcf_fec.c | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 789d385..7eab7ad 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -863,9 +863,6 @@ static void nic_cleanup(NetClientState *nc)
 {
     dp8393xState *s = qemu_get_nic_opaque(nc);
 
-    memory_region_del_subregion(s->address_space, &s->mmio);
-    memory_region_destroy(&s->mmio);
-
     timer_del(s->watchdog);
     timer_free(s->watchdog);
 
diff --git a/hw/net/mcf_fec.c b/hw/net/mcf_fec.c
index 4bff3de..22cd7cf 100644
--- a/hw/net/mcf_fec.c
+++ b/hw/net/mcf_fec.c
@@ -443,9 +443,6 @@ static void mcf_fec_cleanup(NetClientState *nc)
 {
     mcf_fec_state *s = qemu_get_nic_opaque(nc);
 
-    memory_region_del_subregion(s->sysmem, &s->iomem);
-    memory_region_destroy(&s->iomem);
-
     g_free(s);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 11/20] ioport: split deletion and destruction

[Paolo Bonzini]

Of the two functions portio_list_del and portio_list_destroy,
the latter is just freeing a memory area.  However, portio_list_del
is the logical equivalent of memory_region_del_subregion so
destruction of memory regions does not belong there.

Actually, neither of these APIs are in use; portio is mostly used by
ISA devices or VGAs, and neither of these is currently hot-unpluggable.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 ioport.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ioport.c b/ioport.c
index 3d91e79..dce94a3 100644
--- a/ioport.c
+++ b/ioport.c
@@ -149,6 +149,14 @@ void portio_list_set_flush_coalesced(PortioList *piolist)
 
 void portio_list_destroy(PortioList *piolist)
 {
+    MemoryRegionPortioList *mrpio;
+    unsigned i;
+
+    for (i = 0; i < piolist->nr; ++i) {
+        mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, mr);
+        memory_region_destroy(&mrpio->mr);
+        g_free(mrpio);
+    }
     g_free(piolist->regions);
 }
 
@@ -291,8 +299,5 @@ void portio_list_del(PortioList *piolist)
     for (i = 0; i < piolist->nr; ++i) {
         mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, mr);
         memory_region_del_subregion(piolist->address_space, &mrpio->mr);
-        memory_region_destroy(&mrpio->mr);
-        g_free(mrpio);
-        piolist->regions[i] = NULL;
     }
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 12/20] memory: convert memory_region_destroy to object_unparent

[Paolo Bonzini]

Explicitly call object_unparent in the few places where we
will re-create the memory region.  If the memory region is
simply being destroyed as part of device teardown, let QOM
handle it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/memory.txt       | 15 ++++++++++-----
 hw/display/vga.c      |  2 +-
 hw/i386/kvmvapic.c    |  2 +-
 hw/mips/gt64xxx_pci.c |  2 +-
 hw/misc/omap_gpmc.c   |  2 +-
 hw/misc/vfio.c        |  4 ++--
 hw/ppc/ppc4xx_devs.c  |  2 +-
 ioport.c              |  2 +-
 memory.c              |  1 -
 9 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/docs/memory.txt b/docs/memory.txt
index 5bdbdb3..b12f1f0 100644
--- a/docs/memory.txt
+++ b/docs/memory.txt
@@ -74,11 +74,16 @@ Region lifecycle
 ----------------
 
 A region is created by one of the constructor functions (memory_region_init*())
-and destroyed by the destructor (memory_region_destroy()).  In between,
-a region can be added to an address space by using memory_region_add_subregion()
-and removed using memory_region_del_subregion().  Region attributes may be
-changed at any point; they take effect once the region becomes exposed to the
-guest.
+and attached to an object.  It is then destroyed by object_unparent() or simply
+when the parent object dies.
+
+In between, a region can be added to an address space
+by using memory_region_add_subregion() and removed using
+memory_region_del_subregion().  Destroying the region implicitly
+removes the region from the address space.
+
+Region attributes may be changed at any point; they take effect once
+the region becomes exposed to the guest.
 
 Overlapping regions and priority
 --------------------------------
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 68cfee2..65dab8d 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -176,7 +176,7 @@ static void vga_update_memory_access(VGACommonState *s)
 
     if (s->has_chain4_alias) {
         memory_region_del_subregion(s->legacy_address_space, &s->chain4_alias);
-        memory_region_destroy(&s->chain4_alias);
+        object_unparent(OBJECT(&s->chain4_alias));
         s->has_chain4_alias = false;
         s->plane_updated = 0xf;
     }
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index cb855c7..ee95963 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -584,7 +584,7 @@ static int vapic_map_rom_writable(VAPICROMState *s)
 
     if (s->rom_mapped_writable) {
         memory_region_del_subregion(as, &s->rom);
-        memory_region_destroy(&s->rom);
+        object_unparent(OBJECT(&s->rom));
     }
 
     /* grab RAM memory region (region @rom_paddr may still be pc.rom) */
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 22f63ce..1f2fe5f 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -297,7 +297,7 @@ static void gt64120_pci_mapping(GT64120State *s)
       if (s->PCI0IO_length)
       {
           memory_region_del_subregion(get_system_memory(), &s->PCI0IO_mem);
-          memory_region_destroy(&s->PCI0IO_mem);
+          object_unparent(OBJECT(&s->PCI0IO_mem));
       }
       /* Map new IO address */
       s->PCI0IO_start = s->regs[GT_PCI0IOLD] << 21;
diff --git a/hw/misc/omap_gpmc.c b/hw/misc/omap_gpmc.c
index cddea24..fbbe2ff 100644
--- a/hw/misc/omap_gpmc.c
+++ b/hw/misc/omap_gpmc.c
@@ -436,7 +436,7 @@ static void omap_gpmc_cs_unmap(struct omap_gpmc_s *s, int cs)
     }
     memory_region_del_subregion(get_system_memory(), &f->container);
     memory_region_del_subregion(&f->container, omap_gpmc_cs_memregion(s, cs));
-    memory_region_destroy(&f->container);
+    object_unparent(OBJECT(&f->container));
 }
 
 void omap_gpmc_reset(struct omap_gpmc_s *s)
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 0b9eba0..af9ae1f 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -2266,7 +2266,7 @@ static void vfio_vga_quirk_teardown(VFIODevice *vdev)
         while (!QLIST_EMPTY(&vdev->vga.region[i].quirks)) {
             VFIOQuirk *quirk = QLIST_FIRST(&vdev->vga.region[i].quirks);
             memory_region_del_subregion(&vdev->vga.region[i].mem, &quirk->mem);
-            memory_region_destroy(&quirk->mem);
+            object_unparent(OBJECT(&quirk->mem));
             QLIST_REMOVE(quirk, next);
             g_free(quirk);
         }
@@ -2290,7 +2290,7 @@ static void vfio_bar_quirk_teardown(VFIODevice *vdev, int nr)
     while (!QLIST_EMPTY(&bar->quirks)) {
         VFIOQuirk *quirk = QLIST_FIRST(&bar->quirks);
         memory_region_del_subregion(&bar->mem, &quirk->mem);
-        memory_region_destroy(&quirk->mem);
+        object_unparent(OBJECT(&quirk->mem));
         QLIST_REMOVE(quirk, next);
         g_free(quirk);
     }
diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index 07f9d00..405bbe7 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -422,7 +422,7 @@ static void sdram_set_bcr(ppc4xx_sdram_t *sdram,
                                     &sdram->containers[n]);
         memory_region_del_subregion(&sdram->containers[n],
                                     &sdram->ram_memories[n]);
-        memory_region_destroy(&sdram->containers[n]);
+        object_unparent(OBJECT(&sdram->containers[n]));
     }
     *bcrp = bcr & 0xFFDEE001;
     if (enabled && (bcr & 0x00000001)) {
diff --git a/ioport.c b/ioport.c
index dce94a3..783a3ae 100644
--- a/ioport.c
+++ b/ioport.c
@@ -154,7 +154,7 @@ void portio_list_destroy(PortioList *piolist)
 
     for (i = 0; i < piolist->nr; ++i) {
         mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, mr);
-        memory_region_destroy(&mrpio->mr);
+        object_unparent(OBJECT(&mrpio->mr));
         g_free(mrpio);
     }
     g_free(piolist->regions);
diff --git a/memory.c b/memory.c
index 64d7176..23c5269 100644
--- a/memory.c
+++ b/memory.c
@@ -1266,7 +1266,6 @@ static void memory_region_finalize(Object *obj)
 
 void memory_region_destroy(MemoryRegion *mr)
 {
-    object_unparent(OBJECT(mr));
 }
 
 
-- 
1.8.3.1

[Qemu-devel] [PULL 13/20] memory: remove memory_region_destroy

[Paolo Bonzini]

The function is empty after the previous patch, so remove it.

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 backends/hostmem.c             | 10 ----------
 hw/audio/ac97.c                |  9 ---------
 hw/audio/es1370.c              |  8 --------
 hw/audio/intel-hda.c           |  1 -
 hw/block/nvme.c                |  1 -
 hw/block/pflash_cfi01.c        |  1 -
 hw/block/pflash_cfi02.c        |  1 -
 hw/char/serial-pci.c           |  3 ---
 hw/i386/kvm/pci-assign.c       |  8 --------
 hw/ide/ahci.c                  |  2 --
 hw/ide/cmd646.c                |  5 -----
 hw/ide/piix.c                  |  3 ---
 hw/ide/via.c                   |  3 ---
 hw/ipack/tpci200.c             | 13 -------------
 hw/misc/ivshmem.c              |  3 ---
 hw/misc/pci-testdev.c          |  2 --
 hw/misc/vfio.c                 |  7 -------
 hw/net/e1000.c                 |  2 --
 hw/net/eepro100.c              |  3 ---
 hw/net/ne2000.c                |  1 -
 hw/net/pcnet-pci.c             |  2 --
 hw/net/rtl8139.c               |  2 --
 hw/net/stellaris_enet.c        |  8 --------
 hw/net/vmxnet3.c               |  4 ----
 hw/pci-bridge/pci_bridge_dev.c |  2 --
 hw/pci/msix.c                  |  4 ----
 hw/pci/pci.c                   |  2 --
 hw/pci/pci_bridge.c            |  8 --------
 hw/pci/pcie_host.c             |  1 -
 hw/pci/shpc.c                  |  1 -
 hw/scsi/esp-pci.c              |  1 -
 hw/scsi/lsi53c895a.c           | 10 ----------
 hw/scsi/megasas.c              |  3 ---
 hw/scsi/vmw_pvscsi.c           |  3 ---
 hw/tpm/tpm_tis.c               |  1 -
 hw/usb/hcd-uhci.c              |  8 --------
 hw/virtio/virtio-pci.c         |  3 ---
 hw/watchdog/wdt_i6300esb.c     |  8 --------
 hw/xen/xen_pt.c                | 20 --------------------
 hw/xen/xen_pt_msi.c            |  2 --
 include/exec/memory.h          |  9 ---------
 memory.c                       |  5 -----
 42 files changed, 193 deletions(-)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index ca10c51..e7eec37 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -257,15 +257,6 @@ static void host_memory_backend_init(Object *obj)
                         host_memory_backend_set_policy, NULL, NULL, NULL);
 }
 
-static void host_memory_backend_finalize(Object *obj)
-{
-    HostMemoryBackend *backend = MEMORY_BACKEND(obj);
-
-    if (memory_region_size(&backend->mr)) {
-        memory_region_destroy(&backend->mr);
-    }
-}
-
 MemoryRegion *
 host_memory_backend_get_memory(HostMemoryBackend *backend, Error **errp)
 {
@@ -360,7 +351,6 @@ static const TypeInfo host_memory_backend_info = {
     .class_init = host_memory_backend_class_init,
     .instance_size = sizeof(HostMemoryBackend),
     .instance_init = host_memory_backend_init,
-    .instance_finalize = host_memory_backend_finalize,
     .interfaces = (InterfaceInfo[]) {
         { TYPE_USER_CREATABLE },
         { }
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 45cb118..0e22bb9 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1388,14 +1388,6 @@ static int ac97_initfn (PCIDevice *dev)
     return 0;
 }
 
-static void ac97_exitfn (PCIDevice *dev)
-{
-    AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev);
-
-    memory_region_destroy (&s->io_nam);
-    memory_region_destroy (&s->io_nabm);
-}
-
 static int ac97_init (PCIBus *bus)
 {
     pci_create_simple (bus, -1, "AC97");
@@ -1413,7 +1405,6 @@ static void ac97_class_init (ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS (klass);
 
     k->init = ac97_initfn;
-    k->exit = ac97_exitfn;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82801AA_5;
     k->revision = 0x01;
diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index 5dbf803..e67d1ea 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -1042,13 +1042,6 @@ static int es1370_initfn (PCIDevice *dev)
     return 0;
 }
 
-static void es1370_exitfn (PCIDevice *dev)
-{
-    ES1370State *s = DO_UPCAST (ES1370State, dev, dev);
-
-    memory_region_destroy (&s->io);
-}
-
 static int es1370_init (PCIBus *bus)
 {
     pci_create_simple (bus, -1, "ES1370");
@@ -1061,7 +1054,6 @@ static void es1370_class_init (ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS (klass);
 
     k->init = es1370_initfn;
-    k->exit = es1370_exitfn;
     k->vendor_id = PCI_VENDOR_ID_ENSONIQ;
     k->device_id = PCI_DEVICE_ID_ENSONIQ_ES1370;
     k->class_id = PCI_CLASS_MULTIMEDIA_AUDIO;
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index aa49b47..aba45fc 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -1155,7 +1155,6 @@ static void intel_hda_exit(PCIDevice *pci)
     IntelHDAState *d = INTEL_HDA(pci);
 
     msi_uninit(&d->pci);
-    memory_region_destroy(&d->mmio);
 }
 
 static int intel_hda_post_load(void *opaque, int version)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5fd8f89..6d9a065 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -839,7 +839,6 @@ static void nvme_exit(PCIDevice *pci_dev)
     g_free(n->cq);
     g_free(n->sq);
     msix_uninit_exclusive_bar(pci_dev);
-    memory_region_destroy(&n->iomem);
 }
 
 static Property nvme_props[] = {
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index f9507b4..2238f39 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -781,7 +781,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
 
         if (ret < 0) {
             vmstate_unregister_ram(&pfl->mem, DEVICE(pfl));
-            memory_region_destroy(&pfl->mem);
             error_setg(errp, "failed to read the initial flash content");
             return;
         }
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 8d4b828..e196f4d 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -617,7 +617,6 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
         ret = bdrv_read(pfl->bs, 0, pfl->storage, chip_len >> 9);
         if (ret < 0) {
             vmstate_unregister_ram(&pfl->orig_mem, DEVICE(pfl));
-            memory_region_destroy(&pfl->orig_mem);
             error_setg(errp, "failed to read the initial flash content");
             return;
         }
diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
index c133c33..f05c9b4 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -135,7 +135,6 @@ static void serial_pci_exit(PCIDevice *dev)
     SerialState *s = &pci->state;
 
     serial_exit_core(s);
-    memory_region_destroy(&s->io);
     qemu_free_irq(s->irq);
 }
 
@@ -149,10 +148,8 @@ static void multi_serial_pci_exit(PCIDevice *dev)
         s = pci->state + i;
         serial_exit_core(s);
         memory_region_del_subregion(&pci->iobar, &s->io);
-        memory_region_destroy(&s->io);
         g_free(pci->name[i]);
     }
-    memory_region_destroy(&pci->iobar);
     qemu_free_irqs(pci->irqs, pci->ports);
 }
 
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index de33657..17c7d6dc 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -697,8 +697,6 @@ static void free_assigned_device(AssignedDevice *dev)
             if (region->u.r_baseport) {
                 memory_region_del_subregion(&region->container,
                                             &region->real_iomem);
-                memory_region_destroy(&region->real_iomem);
-                memory_region_destroy(&region->container);
             }
         } else if (pci_region->type & IORESOURCE_MEM) {
             if (region->u.r_virtbase) {
@@ -712,9 +710,6 @@ static void free_assigned_device(AssignedDevice *dev)
                     memory_region_del_subregion(&region->container,
                                                 &dev->mmio);
                 }
-
-                memory_region_destroy(&region->real_iomem);
-                memory_region_destroy(&region->container);
                 if (munmap(region->u.r_virtbase,
                            (pci_region->size + 0xFFF) & 0xFFFFF000)) {
                     error_report("Failed to unmap assigned device region: %s",
@@ -1680,8 +1675,6 @@ static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev)
         return;
     }
 
-    memory_region_destroy(&dev->mmio);
-
     if (munmap(dev->msix_table, MSIX_PAGE_SIZE) == -1) {
         error_report("error unmapping msix_table! %s", strerror(errno));
     }
@@ -1953,7 +1946,6 @@ static void assigned_dev_load_option_rom(AssignedDevice *dev)
         error_printf("Device option ROM contents are probably invalid "
                      "(check dmesg).\nSkip option ROM probe with rombar=0, "
                      "or load from file with romfile=\n");
-        memory_region_destroy(&dev->dev.rom);
         goto close_rom;
     }
 
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 604152a..aa0ef42 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1197,8 +1197,6 @@ void ahci_init(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
 
 void ahci_uninit(AHCIState *s)
 {
-    memory_region_destroy(&s->mem);
-    memory_region_destroy(&s->idp);
     g_free(s->dev);
 }
 
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index a8e35fe..1295ed0 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -310,13 +310,8 @@ static void pci_cmd646_ide_exitfn(PCIDevice *dev)
 
     for (i = 0; i < 2; ++i) {
         memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io);
-        memory_region_destroy(&d->bmdma[i].extra_io);
         memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport);
-        memory_region_destroy(&d->bmdma[i].addr_ioport);
-        memory_region_destroy(&d->cmd646_bar[i].cmd);
-        memory_region_destroy(&d->cmd646_bar[i].data);
     }
-    memory_region_destroy(&d->bmdma_bar);
 }
 
 void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 8651726..59319eb 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -207,11 +207,8 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
 
     for (i = 0; i < 2; ++i) {
         memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io);
-        memory_region_destroy(&d->bmdma[i].extra_io);
         memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport);
-        memory_region_destroy(&d->bmdma[i].addr_ioport);
     }
-    memory_region_destroy(&d->bmdma_bar);
 }
 
 /* hd_table must contain 4 block drivers */
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 198123b..89d27b4 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -198,11 +198,8 @@ static void vt82c686b_ide_exitfn(PCIDevice *dev)
 
     for (i = 0; i < 2; ++i) {
         memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io);
-        memory_region_destroy(&d->bmdma[i].extra_io);
         memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport);
-        memory_region_destroy(&d->bmdma[i].addr_ioport);
     }
-    memory_region_destroy(&d->bmdma_bar);
 }
 
 void vt82c686b_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
diff --git a/hw/ipack/tpci200.c b/hw/ipack/tpci200.c
index 42ca923..b7031a0 100644
--- a/hw/ipack/tpci200.c
+++ b/hw/ipack/tpci200.c
@@ -613,18 +613,6 @@ static int tpci200_initfn(PCIDevice *pci_dev)
     return 0;
 }
 
-static void tpci200_exitfn(PCIDevice *pci_dev)
-{
-    TPCI200State *s = TPCI200(pci_dev);
-
-    memory_region_destroy(&s->mmio);
-    memory_region_destroy(&s->io);
-    memory_region_destroy(&s->las0);
-    memory_region_destroy(&s->las1);
-    memory_region_destroy(&s->las2);
-    memory_region_destroy(&s->las3);
-}
-
 static const VMStateDescription vmstate_tpci200 = {
     .name = "tpci200",
     .version_id = 1,
@@ -645,7 +633,6 @@ static void tpci200_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->init = tpci200_initfn;
-    k->exit = tpci200_exitfn;
     k->vendor_id = PCI_VENDOR_ID_TEWS;
     k->device_id = PCI_DEVICE_ID_TEWS_TPCI200;
     k->class_id = PCI_CLASS_BRIDGE_OTHER;
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 768e528..156edd2 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -789,11 +789,8 @@ static void pci_ivshmem_uninit(PCIDevice *dev)
         error_free(s->migration_blocker);
     }
 
-    memory_region_destroy(&s->ivshmem_mmio);
     memory_region_del_subregion(&s->bar, &s->ivshmem);
     vmstate_unregister_ram(&s->ivshmem, DEVICE(dev));
-    memory_region_destroy(&s->ivshmem);
-    memory_region_destroy(&s->bar);
     unregister_savevm(DEVICE(dev), "ivshmem", s);
 }
 
diff --git a/hw/misc/pci-testdev.c b/hw/misc/pci-testdev.c
index ca53b3f..c78a63e 100644
--- a/hw/misc/pci-testdev.c
+++ b/hw/misc/pci-testdev.c
@@ -293,8 +293,6 @@ pci_testdev_uninit(PCIDevice *dev)
         g_free(d->tests[i].hdr);
     }
     g_free(d->tests);
-    memory_region_destroy(&d->mmio);
-    memory_region_destroy(&d->portio);
 }
 
 static void qdev_pci_testdev_reset(DeviceState *dev)
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index af9ae1f..813c2cc 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -2857,15 +2857,11 @@ static void vfio_unmap_bar(VFIODevice *vdev, int nr)
 
     memory_region_del_subregion(&bar->mem, &bar->mmap_mem);
     munmap(bar->mmap, memory_region_size(&bar->mmap_mem));
-    memory_region_destroy(&bar->mmap_mem);
 
     if (vdev->msix && vdev->msix->table_bar == nr) {
         memory_region_del_subregion(&bar->mem, &vdev->msix->mmap_mem);
         munmap(vdev->msix->mmap, memory_region_size(&vdev->msix->mmap_mem));
-        memory_region_destroy(&vdev->msix->mmap_mem);
     }
-
-    memory_region_destroy(&bar->mem);
 }
 
 static int vfio_mmap_bar(VFIODevice *vdev, VFIOBAR *bar,
@@ -3018,9 +3014,6 @@ static void vfio_unmap_bars(VFIODevice *vdev)
     if (vdev->has_vga) {
         vfio_vga_quirk_teardown(vdev);
         pci_unregister_vga(&vdev->pdev);
-        memory_region_destroy(&vdev->vga.region[QEMU_PCI_VGA_MEM].mem);
-        memory_region_destroy(&vdev->vga.region[QEMU_PCI_VGA_IO_LO].mem);
-        memory_region_destroy(&vdev->vga.region[QEMU_PCI_VGA_IO_HI].mem);
     }
 }
 
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 0fc29a0..21c38fa 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1496,8 +1496,6 @@ pci_e1000_uninit(PCIDevice *dev)
     timer_free(d->autoneg_timer);
     timer_del(d->mit_timer);
     timer_free(d->mit_timer);
-    memory_region_destroy(&d->mmio);
-    memory_region_destroy(&d->io);
     qemu_del_nic(d->nic);
 }
 
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 3263e3f..3cd826a 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1843,9 +1843,6 @@ static void pci_nic_uninit(PCIDevice *pci_dev)
 {
     EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
 
-    memory_region_destroy(&s->mmio_bar);
-    memory_region_destroy(&s->io_bar);
-    memory_region_destroy(&s->flash_bar);
     vmstate_unregister(&pci_dev->qdev, s->vmstate, s);
     eeprom93xx_free(&pci_dev->qdev, s->eeprom);
     qemu_del_nic(s->nic);
diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index d558b8c..a62d12d 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -748,7 +748,6 @@ static void pci_ne2000_exit(PCIDevice *pci_dev)
     PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
     NE2000State *s = &d->ne2000;
 
-    memory_region_destroy(&s->io);
     qemu_del_nic(s->nic);
     qemu_free_irq(s->irq);
 }
diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
index b25d789..50ffe91 100644
--- a/hw/net/pcnet-pci.c
+++ b/hw/net/pcnet-pci.c
@@ -282,8 +282,6 @@ static void pci_pcnet_uninit(PCIDevice *dev)
     PCIPCNetState *d = PCI_PCNET(dev);
 
     qemu_free_irq(d->state.irq);
-    memory_region_destroy(&d->state.mmio);
-    memory_region_destroy(&d->io_bar);
     timer_del(d->state.poll_timer);
     timer_free(d->state.poll_timer);
     qemu_del_nic(d->state.nic);
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 90bc5ec..6e59f38 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -3462,8 +3462,6 @@ static void pci_rtl8139_uninit(PCIDevice *dev)
 {
     RTL8139State *s = RTL8139(dev);
 
-    memory_region_destroy(&s->bar_io);
-    memory_region_destroy(&s->bar_mem);
     if (s->cplus_txbuffer) {
         g_free(s->cplus_txbuffer);
         s->cplus_txbuffer = NULL;
diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
index c9ee5d3..c07e513 100644
--- a/hw/net/stellaris_enet.c
+++ b/hw/net/stellaris_enet.c
@@ -485,13 +485,6 @@ static int stellaris_enet_init(SysBusDevice *sbd)
     return 0;
 }
 
-static void stellaris_enet_unrealize(DeviceState *dev, Error **errp)
-{
-    stellaris_enet_state *s = STELLARIS_ENET(dev);
-
-    memory_region_destroy(&s->mmio);
-}
-
 static Property stellaris_enet_properties[] = {
     DEFINE_NIC_PROPERTIES(stellaris_enet_state, conf),
     DEFINE_PROP_END_OF_LIST(),
@@ -503,7 +496,6 @@ static void stellaris_enet_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = stellaris_enet_init;
-    dc->unrealize = stellaris_enet_unrealize;
     dc->props = stellaris_enet_properties;
     dc->vmsd = &vmstate_stellaris_enet;
 }
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 77bea6f..f11525c 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2182,10 +2182,6 @@ static void vmxnet3_pci_uninit(PCIDevice *pci_dev)
     vmxnet3_cleanup_msix(s);
 
     vmxnet3_cleanup_msi(s);
-
-    memory_region_destroy(&s->bar0);
-    memory_region_destroy(&s->bar1);
-    memory_region_destroy(&s->msix_bar);
 }
 
 static void vmxnet3_qdev_reset(DeviceState *dev)
diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index e68145c..92799d0 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -81,7 +81,6 @@ msi_error:
 slotid_error:
     shpc_cleanup(dev, &bridge_dev->bar);
 shpc_error:
-    memory_region_destroy(&bridge_dev->bar);
     pci_bridge_exitfn(dev);
 bridge_error:
     return err;
@@ -95,7 +94,6 @@ static void pci_bridge_dev_exitfn(PCIDevice *dev)
     }
     slotid_cap_cleanup(dev);
     shpc_cleanup(dev, &bridge_dev->bar);
-    memory_region_destroy(&bridge_dev->bar);
     pci_bridge_exitfn(dev);
 }
 
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 5c49bfc..3c07d22 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -319,7 +319,6 @@ int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries,
                     bar_nr, MSIX_EXCLUSIVE_BAR_PBA_OFFSET,
                     MSIX_EXCLUSIVE_CAP_OFFSET);
     if (ret) {
-        memory_region_destroy(&dev->msix_exclusive_bar);
         return ret;
     }
 
@@ -359,11 +358,9 @@ void msix_uninit(PCIDevice *dev, MemoryRegion *table_bar, MemoryRegion *pba_bar)
     msix_free_irq_entries(dev);
     dev->msix_entries_nr = 0;
     memory_region_del_subregion(pba_bar, &dev->msix_pba_mmio);
-    memory_region_destroy(&dev->msix_pba_mmio);
     g_free(dev->msix_pba);
     dev->msix_pba = NULL;
     memory_region_del_subregion(table_bar, &dev->msix_table_mmio);
-    memory_region_destroy(&dev->msix_table_mmio);
     g_free(dev->msix_table);
     dev->msix_table = NULL;
     g_free(dev->msix_entry_used);
@@ -375,7 +372,6 @@ void msix_uninit_exclusive_bar(PCIDevice *dev)
 {
     if (msix_present(dev)) {
         msix_uninit(dev, &dev->msix_exclusive_bar, &dev->msix_exclusive_bar);
-        memory_region_destroy(&dev->msix_exclusive_bar);
     }
 }
 
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 351d320..daeaeac 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -799,7 +799,6 @@ static void do_pci_unregister_device(PCIDevice *pci_dev)
     pci_config_free(pci_dev);
 
     address_space_destroy(&pci_dev->bus_master_as);
-    memory_region_destroy(&pci_dev->bus_master_enable_region);
 }
 
 /* -1 for devfn means auto assign */
@@ -1996,7 +1995,6 @@ static void pci_del_option_rom(PCIDevice *pdev)
         return;
 
     vmstate_unregister_ram(&pdev->rom, &pdev->qdev);
-    memory_region_destroy(&pdev->rom);
     pdev->has_rom = false;
 }
 
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 4becdc1..1307265 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -219,12 +219,6 @@ static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
 
 static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
 {
-    memory_region_destroy(&w->alias_io);
-    memory_region_destroy(&w->alias_mem);
-    memory_region_destroy(&w->alias_pref_mem);
-    memory_region_destroy(&w->alias_vga[QEMU_PCI_VGA_IO_LO]);
-    memory_region_destroy(&w->alias_vga[QEMU_PCI_VGA_IO_HI]);
-    memory_region_destroy(&w->alias_vga[QEMU_PCI_VGA_MEM]);
     g_free(w);
 }
 
@@ -389,8 +383,6 @@ void pci_bridge_exitfn(PCIDevice *pci_dev)
     QLIST_REMOVE(&s->sec_bus, sibling);
     pci_bridge_region_del(s, s->windows);
     pci_bridge_region_cleanup(s, s->windows);
-    memory_region_destroy(&s->address_space_mem);
-    memory_region_destroy(&s->address_space_io);
     /* object_unparent() is called automatically during device deletion */
 }
 
diff --git a/hw/pci/pcie_host.c b/hw/pci/pcie_host.c
index 7c88a1d..3db038f 100644
--- a/hw/pci/pcie_host.c
+++ b/hw/pci/pcie_host.c
@@ -94,7 +94,6 @@ void pcie_host_mmcfg_unmap(PCIExpressHost *e)
 {
     if (e->base_addr != PCIE_BASE_ADDR_UNMAPPED) {
         memory_region_del_subregion(get_system_memory(), &e->mmio);
-        memory_region_destroy(&e->mmio);
         e->base_addr = PCIE_BASE_ADDR_UNMAPPED;
     }
 }
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 180faa7..1fcb8c4 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -667,7 +667,6 @@ void shpc_cleanup(PCIDevice *d, MemoryRegion *bar)
     g_free(shpc->cmask);
     g_free(shpc->wmask);
     g_free(shpc->w1cmask);
-    memory_region_destroy(&shpc->mmio);
     g_free(shpc);
 }
 
diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 9971bbf..82795e6 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -378,7 +378,6 @@ static void esp_pci_scsi_uninit(PCIDevice *d)
     PCIESPState *pci = PCI_ESP(d);
 
     qemu_free_irq(pci->esp.irq);
-    memory_region_destroy(&pci->io);
 }
 
 static void esp_pci_class_init(ObjectClass *klass, void *data)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 786d848..513ea47 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2072,15 +2072,6 @@ static const VMStateDescription vmstate_lsi_scsi = {
     }
 };
 
-static void lsi_scsi_uninit(PCIDevice *d)
-{
-    LSIState *s = LSI53C895A(d);
-
-    memory_region_destroy(&s->mmio_io);
-    memory_region_destroy(&s->ram_io);
-    memory_region_destroy(&s->io_io);
-}
-
 static const struct SCSIBusInfo lsi_scsi_info = {
     .tcq = true,
     .max_target = LSI_MAX_DEVS,
@@ -2134,7 +2125,6 @@ static void lsi_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->init = lsi_scsi_init;
-    k->exit = lsi_scsi_uninit;
     k->vendor_id = PCI_VENDOR_ID_LSI_LOGIC;
     k->device_id = PCI_DEVICE_ID_LSI_53C895A;
     k->class_id = PCI_CLASS_STORAGE_SCSI;
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index c68a873..eedc992 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2129,9 +2129,6 @@ static void megasas_scsi_uninit(PCIDevice *d)
     if (megasas_use_msi(s)) {
         msi_uninit(d);
     }
-    memory_region_destroy(&s->mmio_io);
-    memory_region_destroy(&s->port_io);
-    memory_region_destroy(&s->queue_io);
 }
 
 static const struct SCSIBusInfo megasas_scsi_info = {
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index f9ed926..5734d19 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -1087,7 +1087,6 @@ pvscsi_init(PCIDevice *pci_dev)
     s->completion_worker = qemu_bh_new(pvscsi_process_completion_queue, s);
     if (!s->completion_worker) {
         pvscsi_cleanup_msi(s);
-        memory_region_destroy(&s->io_space);
         return -ENOMEM;
     }
 
@@ -1107,8 +1106,6 @@ pvscsi_uninit(PCIDevice *pci_dev)
     qemu_bh_delete(s->completion_worker);
 
     pvscsi_cleanup_msi(s);
-
-    memory_region_destroy(&s->io_space);
 }
 
 static void
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 6f0a4d2..d398c16 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -901,7 +901,6 @@ static void tpm_tis_uninitfn(Object *obj)
     TPMState *s = TPM(obj);
 
     memory_region_del_subregion(get_system_memory(), &s->mmio);
-    memory_region_destroy(&s->mmio);
 }
 
 static void tpm_tis_class_init(ObjectClass *klass, void *data)
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index c3bf72c..ee5f112 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -1256,13 +1256,6 @@ static int usb_uhci_vt82c686b_initfn(PCIDevice *dev)
     return usb_uhci_common_initfn(dev);
 }
 
-static void usb_uhci_exit(PCIDevice *dev)
-{
-    UHCIState *s = DO_UPCAST(UHCIState, dev, dev);
-
-    memory_region_destroy(&s->io_bar);
-}
-
 static Property uhci_properties[] = {
     DEFINE_PROP_STRING("masterbus", UHCIState, masterbus),
     DEFINE_PROP_UINT32("firstport", UHCIState, firstport, 0),
@@ -1279,7 +1272,6 @@ static void uhci_class_init(ObjectClass *klass, void *data)
     UHCIInfo *info = data;
 
     k->init = info->initfn ? info->initfn : usb_uhci_common_initfn;
-    k->exit = info->unplug ? usb_uhci_exit : NULL;
     k->vendor_id = info->vendor_id;
     k->device_id = info->device_id;
     k->revision  = info->revision;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 3007319..ddb5da1 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1020,10 +1020,7 @@ static int virtio_pci_init(PCIDevice *pci_dev)
 
 static void virtio_pci_exit(PCIDevice *pci_dev)
 {
-    VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev);
-
     msix_uninit_exclusive_bar(pci_dev);
-    memory_region_destroy(&proxy->bar);
 }
 
 static void virtio_pci_reset(DeviceState *qdev)
diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
index b3d6f39..687c8b1 100644
--- a/hw/watchdog/wdt_i6300esb.c
+++ b/hw/watchdog/wdt_i6300esb.c
@@ -425,13 +425,6 @@ static int i6300esb_init(PCIDevice *dev)
     return 0;
 }
 
-static void i6300esb_exit(PCIDevice *dev)
-{
-    I6300State *d = DO_UPCAST(I6300State, dev, dev);
-
-    memory_region_destroy(&d->io_mem);
-}
-
 static WatchdogTimerModel model = {
     .wdt_name = "i6300esb",
     .wdt_description = "Intel 6300ESB",
@@ -445,7 +438,6 @@ static void i6300esb_class_init(ObjectClass *klass, void *data)
     k->config_read = i6300esb_config_read;
     k->config_write = i6300esb_config_write;
     k->init = i6300esb_init;
-    k->exit = i6300esb_exit;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_ESB_9;
     k->class_id = PCI_CLASS_SYSTEM_OTHER;
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index be4220b..c1bf357 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -453,25 +453,6 @@ static int xen_pt_register_regions(XenPCIPassthroughState *s)
     return 0;
 }
 
-static void xen_pt_unregister_regions(XenPCIPassthroughState *s)
-{
-    XenHostPCIDevice *d = &s->real_device;
-    int i;
-
-    for (i = 0; i < PCI_NUM_REGIONS - 1; i++) {
-        XenHostPCIIORegion *r = &d->io_regions[i];
-
-        if (r->base_addr == 0 || r->size == 0) {
-            continue;
-        }
-
-        memory_region_destroy(&s->bar[i]);
-    }
-    if (d->rom.base_addr && d->rom.size) {
-        memory_region_destroy(&s->rom);
-    }
-}
-
 /* region mapping */
 
 static int xen_pt_bar_from_region(XenPCIPassthroughState *s, MemoryRegion *mr)
@@ -810,7 +791,6 @@ static void xen_pt_unregister_device(PCIDevice *d)
     /* delete all emulated config registers */
     xen_pt_config_delete(s);
 
-    xen_pt_unregister_regions(s);
     memory_listener_unregister(&s->memory_listener);
     memory_listener_unregister(&s->io_listener);
 
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index 12b4c45..9ed9321 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -593,7 +593,6 @@ int xen_pt_msix_init(XenPCIPassthroughState *s, uint32_t base)
     return 0;
 
 error_out:
-    memory_region_destroy(&msix->mmio);
     g_free(s->msix);
     s->msix = NULL;
     return rc;
@@ -616,7 +615,6 @@ void xen_pt_msix_delete(XenPCIPassthroughState *s)
     }
 
     memory_region_del_subregion(&s->bar[msix->bar_index], &msix->mmio);
-    memory_region_destroy(&msix->mmio);
 
     g_free(s->msix);
     s->msix = NULL;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index e2c8e3e..5bd10d1 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -430,15 +430,6 @@ void memory_region_init_iommu(MemoryRegion *mr,
                               uint64_t size);
 
 /**
- * memory_region_destroy: Destroy a memory region and reclaim all resources.
- *
- * @mr: the region to be destroyed.  May not currently be a subregion
- *      (see memory_region_add_subregion()) or referenced in an alias
- *      (see memory_region_init_alias()).
- */
-void memory_region_destroy(MemoryRegion *mr);
-
-/**
  * memory_region_owner: get a memory region's owner.
  *
  * @mr: the memory region being queried.
diff --git a/memory.c b/memory.c
index 23c5269..f11c035 100644
--- a/memory.c
+++ b/memory.c
@@ -1264,11 +1264,6 @@ static void memory_region_finalize(Object *obj)
     g_free(mr->ioeventfds);
 }
 
-void memory_region_destroy(MemoryRegion *mr)
-{
-}
-
-
 Object *memory_region_owner(MemoryRegion *mr)
 {
     Object *obj = OBJECT(mr);
-- 
1.8.3.1

[Qemu-devel] [PULL 14/20] tpm_tis: remove instance_finalize callback

[Paolo Bonzini]

It is never used, since ISA device are not hot-unpluggable.

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/tpm/tpm_tis.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index d398c16..82747ee 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -896,13 +896,6 @@ static void tpm_tis_initfn(Object *obj)
                                 &s->mmio);
 }
 
-static void tpm_tis_uninitfn(Object *obj)
-{
-    TPMState *s = TPM(obj);
-
-    memory_region_del_subregion(get_system_memory(), &s->mmio);
-}
-
 static void tpm_tis_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -918,7 +911,6 @@ static const TypeInfo tpm_tis_info = {
     .parent = TYPE_ISA_DEVICE,
     .instance_size = sizeof(TPMState),
     .instance_init = tpm_tis_initfn,
-    .instance_finalize = tpm_tis_uninitfn,
     .class_init  = tpm_tis_class_init,
 };
 
-- 
1.8.3.1

[Qemu-devel] [PULL 15/20] loader: Abstract away ref to memory region names

[Paolo Bonzini]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Use the function provided rather than spying on the struct.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2bf6b8f..1a53f0f 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -955,7 +955,7 @@ void do_info_roms(Monitor *mon, const QDict *qdict)
         if (rom->mr) {
             monitor_printf(mon, "%s"
                            " size=0x%06zx name=\"%s\"\n",
-                           rom->mr->name,
+                           memory_region_name(rom->mr),
                            rom->romsize,
                            rom->name);
         } else if (!rom->fw_file) {
-- 
1.8.3.1

[Qemu-devel] [PULL 16/20] exec: Abstract away ref to memory region names

[Paolo Bonzini]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Use the function provided rather than spying on the struct.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index 5a2a25e..42688b6 100644
--- a/exec.c
+++ b/exec.c
@@ -1044,7 +1044,7 @@ static void *file_ram_alloc(RAMBlock *block,
     }
 
     /* Make name safe to use with mkstemp by replacing '/' with '_'. */
-    sanitized_name = g_strdup(block->mr->name);
+    sanitized_name = g_strdup(memory_region_name(block->mr));
     for (c = sanitized_name; *c != '\0'; c++) {
         if (*c == '/')
             *c = '_';
@@ -1242,7 +1242,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block)
             new_block->host = phys_mem_alloc(new_block->length);
             if (!new_block->host) {
                 fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
-                        new_block->mr->name, strerror(errno));
+                        memory_region_name(new_block->mr), strerror(errno));
                 exit(1);
             }
             memory_try_enable_merging(new_block->host, new_block->length);
-- 
1.8.3.1

[Qemu-devel] [PULL 17/20] memory: constify memory_region_name

[Paolo Bonzini]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

It doesn't change the MR and some prospective call sites will have
const MRs at hand.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/memory.h | 2 +-
 memory.c              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 5bd10d1..d165b27 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -511,7 +511,7 @@ void memory_region_unregister_iommu_notifier(Notifier *n);
  *
  * @mr: the memory region being queried
  */
-const char *memory_region_name(MemoryRegion *mr);
+const char *memory_region_name(const MemoryRegion *mr);
 
 /**
  * memory_region_is_logging: return whether a memory region is logging writes
diff --git a/memory.c b/memory.c
index f11c035..1cde6ff 100644
--- a/memory.c
+++ b/memory.c
@@ -1308,7 +1308,7 @@ uint64_t memory_region_size(MemoryRegion *mr)
     return int128_get64(mr->size);
 }
 
-const char *memory_region_name(MemoryRegion *mr)
+const char *memory_region_name(const MemoryRegion *mr)
 {
     return mr->name;
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 18/20] memory: Use memory_region_name for name access

[Paolo Bonzini]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Despite being local to memory.c, use the helper function. This prepares
support for fully QOMifiying the name field of MR (which will remove
this state from MR completely).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/memory.c b/memory.c
index 1cde6ff..b6b208f 100644
--- a/memory.c
+++ b/memory.c
@@ -2027,8 +2027,8 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
                    mr->romd_mode ? 'R' : '-',
                    !mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W'
                                                                        : '-',
-                   mr->name,
-                   mr->alias->name,
+                   memory_region_name(mr),
+                   memory_region_name(mr->alias),
                    mr->alias_offset,
                    mr->alias_offset
                    + (int128_nz(mr->size) ?
@@ -2046,7 +2046,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
                    mr->romd_mode ? 'R' : '-',
                    !mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W'
                                                                        : '-',
-                   mr->name);
+                   memory_region_name(mr));
     }
 
     QTAILQ_INIT(&submr_print_queue);
@@ -2095,7 +2095,7 @@ void mtree_info(fprintf_function mon_printf, void *f)
     /* print aliased regions */
     QTAILQ_FOREACH(ml, &ml_head, queue) {
         if (!ml->printed) {
-            mon_printf(f, "%s\n", ml->mr->name);
+            mon_printf(f, "%s\n", memory_region_name(ml->mr));
             mtree_print_mr(mon_printf, f, ml->mr, 0, 0, &ml_head);
         }
     }
-- 
1.8.3.1

[Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name

[Paolo Bonzini]

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Rather than having the name as separate state. This prepares support
for creating a MemoryRegion dynamically (i.e. without
memory_region_init() and friends) and the MemoryRegion still getting
a usable name.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/memory.h | 1 -
 memory.c              | 4 +---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index d165b27..10f73d9 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -162,7 +162,6 @@ struct MemoryRegion {
     QTAILQ_HEAD(subregions, MemoryRegion) subregions;
     QTAILQ_ENTRY(MemoryRegion) subregions_link;
     QTAILQ_HEAD(coalesced_ranges, CoalescedMemoryRange) coalesced;
-    const char *name;
     uint8_t dirty_log_mask;
     unsigned ioeventfd_nb;
     MemoryRegionIoeventfd *ioeventfds;
diff --git a/memory.c b/memory.c
index b6b208f..8da29af 100644
--- a/memory.c
+++ b/memory.c
@@ -915,7 +915,6 @@ void memory_region_init(MemoryRegion *mr,
     if (size == UINT64_MAX) {
         mr->size = int128_2_64();
     }
-    mr->name = g_strdup(name);
 
     if (name) {
         object_property_add_child_array(owner, name, OBJECT(mr));
@@ -1260,7 +1259,6 @@ static void memory_region_finalize(Object *obj)
     assert(memory_region_transaction_depth == 0);
     mr->destructor(mr);
     memory_region_clear_coalescing(mr);
-    g_free((char *)mr->name);
     g_free(mr->ioeventfds);
 }
 
@@ -1310,7 +1308,7 @@ uint64_t memory_region_size(MemoryRegion *mr)
 
 const char *memory_region_name(const MemoryRegion *mr)
 {
-    return mr->name;
+    return object_get_canonical_path_component(OBJECT(mr));
 }
 
 bool memory_region_is_ram(MemoryRegion *mr)
-- 
1.8.3.1

[Qemu-devel] [PULL 20/20] mtree: remove write-only field

[Paolo Bonzini]

ml->printed is never set to true.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/memory.c b/memory.c
index 8da29af..031ff51 100644
--- a/memory.c
+++ b/memory.c
@@ -1972,7 +1972,6 @@ typedef struct MemoryRegionList MemoryRegionList;
 
 struct MemoryRegionList {
     const MemoryRegion *mr;
-    bool printed;
     QTAILQ_ENTRY(MemoryRegionList) queue;
 };
 
@@ -2002,7 +2001,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
 
         /* check if the alias is already in the queue */
         QTAILQ_FOREACH(ml, alias_print_queue, queue) {
-            if (ml->mr == mr->alias && !ml->printed) {
+            if (ml->mr == mr->alias) {
                 found = true;
             }
         }
@@ -2010,7 +2009,6 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
         if (!found) {
             ml = g_new(MemoryRegionList, 1);
             ml->mr = mr->alias;
-            ml->printed = false;
             QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
         }
         mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx
@@ -2092,10 +2090,8 @@ void mtree_info(fprintf_function mon_printf, void *f)
     mon_printf(f, "aliases\n");
     /* print aliased regions */
     QTAILQ_FOREACH(ml, &ml_head, queue) {
-        if (!ml->printed) {
-            mon_printf(f, "%s\n", memory_region_name(ml->mr));
-            mtree_print_mr(mon_printf, f, ml->mr, 0, 0, &ml_head);
-        }
+        mon_printf(f, "%s\n", memory_region_name(ml->mr));
+        mtree_print_mr(mon_printf, f, ml->mr, 0, 0, &ml_head);
     }
 
     QTAILQ_FOREACH_SAFE(ml, &ml_head, queue, ml2) {
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 00/20] SCSI and memory changes for 2014-08-18

[Peter Maydell]

On 19 August 2014 11:42, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit 5a7348045091a2bc15d85bb177e5956aa6114e5a:
>
>   Update version for v2.1.0-rc2 release (2014-07-15 18:55:37 +0100)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to f54bb15f9d373877954e44db3a8bb368aff45b42:
>
>   mtree: remove write-only field (2014-08-18 12:06:21 +0200)
>
> ----------------------------------------------------------------
> SCSI changes that enable sending vendor-specific commands via virtio-scsi.
>
> Memory changes for QOMification and automatic tracking of MR lifetime.

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name

[Peter Maydell]

On 19 August 2014 11:43, Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Rather than having the name as separate state. This prepares support
> for creating a MemoryRegion dynamically (i.e. without
> memory_region_init() and friends) and the MemoryRegion still getting
> a usable name.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

This commit seems to have broken builds with Xen:

  CC    i386-softmmu/xen-hvm.o
/home/petmay01/linaro/qemu-for-merges/xen-hvm.c: In function
‘xen_add_to_physmap’:
/home/petmay01/linaro/qemu-for-merges/xen-hvm.c:333:31: error:
‘MemoryRegion’ has no member named ‘name’
     physmap->name = (char *)mr->name;
                               ^
/home/petmay01/linaro/qemu-for-merges/xen-hvm.c:357:11: error:
‘MemoryRegion’ has no member named ‘name’
     if (mr->name) {
           ^
/home/petmay01/linaro/qemu-for-merges/xen-hvm.c:361:51: error:
‘MemoryRegion’ has no member named ‘name’
         if (!xs_write(state->xenstore, 0, path, mr->name, strlen(mr->name))) {
                                                   ^
/home/petmay01/linaro/qemu-for-merges/xen-hvm.c:361:68: error:
‘MemoryRegion’ has no member named ‘name’
         if (!xs_write(state->xenstore, 0, path, mr->name, strlen(mr->name))) {
                                                                    ^
make[1]: *** [xen-hvm.o] Error 1
make: *** [subdir-i386-softmmu] Error 2


I should have spotted this as part of my pre-merge
tests, but somehow it slipped through -- sorry.

-- PMM

Re: [Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name

[Peter Maydell]

On 19 August 2014 11:43, Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Rather than having the name as separate state. This prepares support
> for creating a MemoryRegion dynamically (i.e. without
> memory_region_init() and friends) and the MemoryRegion still getting
> a usable name.
> @@ -1310,7 +1308,7 @@ uint64_t memory_region_size(MemoryRegion *mr)
>
>  const char *memory_region_name(const MemoryRegion *mr)
>  {
> -    return mr->name;
> +    return object_get_canonical_path_component(OBJECT(mr));
>  }

This doesn't look right. It changes the semantics of this function
from "returns a string which you don't own and can't change
but don't need to free" to "returns a copy of a string which you
have to free with g_free() when you're done". Unsurprisingly,
the callsites aren't expecting this and we leak the string all
over the place.

I think we need to revert this (commit b0225c2c0d8) until
both the Xen callsites are fixed and the leak issue is
dealt with.

thanks
-- PMM

Re: [Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name

[Peter Crosthwaite]

On Wed, Aug 20, 2014 at 4:51 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 19 August 2014 11:43, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>> Rather than having the name as separate state. This prepares support
>> for creating a MemoryRegion dynamically (i.e. without
>> memory_region_init() and friends) and the MemoryRegion still getting
>> a usable name.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> This commit seems to have broken builds with Xen:
>
>   CC    i386-softmmu/xen-hvm.o
> /home/petmay01/linaro/qemu-for-merges/xen-hvm.c: In function
> ‘xen_add_to_physmap’:
> /home/petmay01/linaro/qemu-for-merges/xen-hvm.c:333:31: error:
> ‘MemoryRegion’ has no member named ‘name’
>      physmap->name = (char *)mr->name;
>                                ^
> /home/petmay01/linaro/qemu-for-merges/xen-hvm.c:357:11: error:
> ‘MemoryRegion’ has no member named ‘name’
>      if (mr->name) {
>            ^
> /home/petmay01/linaro/qemu-for-merges/xen-hvm.c:361:51: error:
> ‘MemoryRegion’ has no member named ‘name’
>          if (!xs_write(state->xenstore, 0, path, mr->name, strlen(mr->name))) {
>                                                    ^
> /home/petmay01/linaro/qemu-for-merges/xen-hvm.c:361:68: error:
> ‘MemoryRegion’ has no member named ‘name’
>          if (!xs_write(state->xenstore, 0, path, mr->name, strlen(mr->name))) {
>                                                                     ^
> make[1]: *** [xen-hvm.o] Error 1
> make: *** [subdir-i386-softmmu] Error 2
>
>
> I should have spotted this as part of my pre-merge
> tests, but somehow it slipped through -- sorry.
>

You and me both. Added Xen to my pre-send compile testing.

The fix for this is reasonably trivial and just follows the change
pattern for the other usages as fixed in original series. Patch on
list imminently.

Regards,
Peter

> -- PMM
>

Re: [Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name

[Peter Crosthwaite]

On Wed, Aug 20, 2014 at 5:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 19 August 2014 11:43, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>> Rather than having the name as separate state. This prepares support
>> for creating a MemoryRegion dynamically (i.e. without
>> memory_region_init() and friends) and the MemoryRegion still getting
>> a usable name.
>> @@ -1310,7 +1308,7 @@ uint64_t memory_region_size(MemoryRegion *mr)
>>
>>  const char *memory_region_name(const MemoryRegion *mr)
>>  {
>> -    return mr->name;
>> +    return object_get_canonical_path_component(OBJECT(mr));
>>  }
>
> This doesn't look right. It changes the semantics of this function
> from "returns a string which you don't own and can't change
> but don't need to free" to "returns a copy of a string which you
> have to free with g_free() when you're done". Unsurprisingly,
> the callsites aren't expecting this and we leak the string all
> over the place.
>
> I think we need to revert this (commit b0225c2c0d8) until
> both the Xen callsites are fixed and the leak issue is
> dealt with.
>

Have half a plan on the leak issue. With
object_get_canonical_path_component() being used increasing as a "get
me the already set, name of this object", I think it's better to just
change to API to return const and remove the free burden from all call
sites completely. If call sites really want a fresh copy they can take
one themselves. Patch on list imminently.

Regards,
Peter

> thanks
> -- PMM
>

Re: [Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name

[Peter Maydell]

On 20 August 2014 06:04, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> On Wed, Aug 20, 2014 at 5:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> I think we need to revert this (commit b0225c2c0d8) until
>> both the Xen callsites are fixed and the leak issue is
>> dealt with.
>>
>
> Have half a plan on the leak issue. With
> object_get_canonical_path_component() being used increasing as a "get
> me the already set, name of this object", I think it's better to just
> change to API to return const and remove the free burden from all call
> sites completely. If call sites really want a fresh copy they can take
> one themselves. Patch on list imminently.

I'm going to go ahead and revert this patch for the moment so
we can take our time reviewing that patch set.

thanks
-- PMM

Re: [Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name

[Paolo Bonzini]

Il 20/08/2014 09:50, Peter Maydell ha scritto:
> On 20 August 2014 06:04, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> On Wed, Aug 20, 2014 at 5:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> I think we need to revert this (commit b0225c2c0d8) until
>>> both the Xen callsites are fixed and the leak issue is
>>> dealt with.
>>>
>>
>> Have half a plan on the leak issue. With
>> object_get_canonical_path_component() being used increasing as a "get
>> me the already set, name of this object", I think it's better to just
>> change to API to return const and remove the free burden from all call
>> sites completely. If call sites really want a fresh copy they can take
>> one themselves. Patch on list imminently.
> 
> I'm going to go ahead and revert this patch for the moment so
> we can take our time reviewing that patch set.

Yes, thanks.

Paolo