MemoryRegion Name QOMification

PATCH Applied Peter Crosthwaite 2 participants 7 messages Aug 15, 2014 – Aug 17, 2014

lore

Share & tools

lore

Peter Crosthwaite

author

2014-08-15 06:54 UTC #0 lore raw

cover MemoryRegion Name QOMification

Hi Paolo,

Another piece to the Memory Region QOMification puzzle. This series gets rid of the name field of MemoryRegion and converts all users to the QOM managed Object name.

Regards,
Peter

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

 exec.c                |  4 ++--
 hw/core/loader.c      |  2 +-
 include/exec/memory.h |  3 +--
 memory.c              | 14 ++++++--------
 4 files changed, 10 insertions(+), 13 deletions(-)

Signature · 1 line

2.0.1.1.gfbfc394

Peter Crosthwaite

author

2014-08-15 06:54 UTC #1 lore raw

PATCH 1/5 loader: Abstract away ref to memory region names

Use the function provided rather than spying on the struct.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/core/loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

hw/core/loader.c +1 -1

index 2bf6b8f..1a53f0f 100644
@@ -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) {

Signature · 1 line

2.0.1.1.gfbfc394

Peter Crosthwaite

author

2014-08-15 06:55 UTC #2 lore raw

PATCH 2/5 exec: Abstract away ref to memory region names

In reply to Peter Crosthwaite

Use the function provided rather than spying on the struct.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 exec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

exec.c +2 -2

index 765bd94..5f9857c 100644
@@ -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);

Signature · 1 line

2.0.1.1.gfbfc394

Peter Crosthwaite

author

2014-08-15 06:55 UTC #3 lore raw

PATCH 3/5 memory: constify memory_region_name

In reply to Peter Crosthwaite

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

Signed-off-by Peter Crosthwaite <[email protected]>

---

 include/exec/memory.h | 2 +-
 memory.c              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

include/exec/memory.h +1 -1

index e2c8e3e..081f85b 100644
@@ -520,7 +520,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

memory.c +1 -1

index 64d7176..8d88cc4 100644
@@ -1314,7 +1314,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;
 }

Signature · 1 line

2.0.1.1.gfbfc394

Peter Crosthwaite

author

2014-08-15 06:56 UTC #4 lore raw

PATCH 4/5 memory: Use memory_region_name for name access

In reply to Peter Crosthwaite

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 <[email protected]>

---

 memory.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

memory.c +4 -4

index 8d88cc4..e3ee338 100644
@@ -2033,8 +2033,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) ?

@@ -2052,7 +2052,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);

@@ -2101,7 +2101,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);
         }
     }

Signature · 1 line

2.0.1.1.gfbfc394

Peter Crosthwaite

author

2014-08-15 06:56 UTC #5 lore raw

PATCH 5/5 memory: Use canonical path component as the name

In reply to Peter Crosthwaite

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 <[email protected]>

---

 include/exec/memory.h | 1 -
 memory.c              | 4 +---
 2 files changed, 1 insertion(+), 4 deletions(-)

include/exec/memory.h +0 -1

index 081f85b..9b3e6f7 100644
@@ -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;

memory.c +1 -3

index e3ee338..d4de571 100644
@@ -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);
 }

@@ -1316,7 +1314,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)

Signature · 1 line

2.0.1.1.gfbfc394

Paolo Bonzini

Applied

2014-08-17 20:43 UTC #6 lore raw

In reply to Peter Crosthwaite

Il 15/08/2014 08:53, Peter Crosthwaite ha scritto:

Quoted 17 lines

Hi Paolo,
Another piece to the Memory Region QOMification puzzle. This series gets rid of the name field of MemoryRegion and converts all users to the QOM managed Object name.
Regards,
Peter
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

Quoted 6 lines

 exec.c                |  4 ++--
 hw/core/loader.c      |  2 +-
 include/exec/memory.h |  3 +--
 memory.c              | 14 ++++++--------
 4 files changed, 10 insertions(+), 13 deletions(-)

Thanks, applied all to memory branch.

Paolo