* [Qemu-devel] [PATCH] memory_region: Fix name comments @ 2017-03-08 10:54 Dr. David Alan Gilbert (git) 2017-03-08 11:15 ` Philippe Mathieu-Daudé 2017-03-08 11:47 ` Peter Maydell 0 siblings, 2 replies; 6+ messages in thread From: Dr. David Alan Gilbert (git) @ 2017-03-08 10:54 UTC (permalink / raw) To: qemu-devel, pbonzini, peter.maydell From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> The 'name' parameter to memory_region_init_* had been marked as debug only, however vmstate_region_ram uses it as a parameter to qemu_ram_set_idstr to set RAMBlock names and these form part of the migration stream. Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- include/exec/memory.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 6911023..de8f69e 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -307,7 +307,7 @@ struct MemoryRegionSection { * * @mr: the #MemoryRegion to be initialized * @owner: the object that tracks the region's reference count - * @name: used for debugging; not visible to the user or ABI + * @name: Region name, becomes part of RAMBlock name used in migration stream * @size: size of the region; any subregions beyond this size will be clipped */ void memory_region_init(MemoryRegion *mr, @@ -355,7 +355,7 @@ void memory_region_unref(MemoryRegion *mr); * @ops: a structure containing read and write callbacks to be used when * I/O is performed on the region. * @opaque: passed to the read and write callbacks of the @ops structure. - * @name: used for debugging; not visible to the user or ABI + * @name: Region name, becomes part of RAMBlock name used in migration stream * @size: size of the region. */ void memory_region_init_io(MemoryRegion *mr, @@ -474,7 +474,7 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr, * * @mr: the #MemoryRegion to be initialized. * @owner: the object that tracks the region's reference count - * @name: used for debugging; not visible to the user or ABI + * @name: Region name, becomes part of RAMBlock name used in migration stream * @orig: the region to be referenced; @mr will be equivalent to * @orig between @offset and @offset + @size - 1. * @offset: start of the section in @orig to be referenced. @@ -537,7 +537,7 @@ void memory_region_init_rom_device(MemoryRegion *mr, * * @mr: the #MemoryRegion to be initialized * @owner: the object that tracks the region's reference count - * @name: used for debugging; not visible to the user or ABI + * @name: Region name, becomes part of RAMBlock name used in migration stream * @size: size of the region. */ static inline void memory_region_init_reservation(MemoryRegion *mr, @@ -558,7 +558,7 @@ static inline void memory_region_init_reservation(MemoryRegion *mr, * @mr: the #MemoryRegion to be initialized * @owner: the object that tracks the region's reference count * @ops: a function that translates addresses into the @target region - * @name: used for debugging; not visible to the user or ABI + * @name: Region name, becomes part of RAMBlock name used in migration stream * @size: size of the region. */ void memory_region_init_iommu(MemoryRegion *mr, -- 2.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] memory_region: Fix name comments 2017-03-08 10:54 [Qemu-devel] [PATCH] memory_region: Fix name comments Dr. David Alan Gilbert (git) @ 2017-03-08 11:15 ` Philippe Mathieu-Daudé 2017-03-08 11:47 ` Peter Maydell 1 sibling, 0 replies; 6+ messages in thread From: Philippe Mathieu-Daudé @ 2017-03-08 11:15 UTC (permalink / raw) To: Dr. David Alan Gilbert (git), qemu-devel, pbonzini, peter.maydell On 03/08/2017 07:54 AM, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > The 'name' parameter to memory_region_init_* had been marked as debug > only, however vmstate_region_ram uses it as a parameter to > qemu_ram_set_idstr to set RAMBlock names and these form part of the > migration stream. > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > include/exec/memory.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 6911023..de8f69e 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -307,7 +307,7 @@ struct MemoryRegionSection { > * > * @mr: the #MemoryRegion to be initialized > * @owner: the object that tracks the region's reference count > - * @name: used for debugging; not visible to the user or ABI > + * @name: Region name, becomes part of RAMBlock name used in migration stream > * @size: size of the region; any subregions beyond this size will be clipped > */ > void memory_region_init(MemoryRegion *mr, > @@ -355,7 +355,7 @@ void memory_region_unref(MemoryRegion *mr); > * @ops: a structure containing read and write callbacks to be used when > * I/O is performed on the region. > * @opaque: passed to the read and write callbacks of the @ops structure. > - * @name: used for debugging; not visible to the user or ABI > + * @name: Region name, becomes part of RAMBlock name used in migration stream > * @size: size of the region. > */ > void memory_region_init_io(MemoryRegion *mr, > @@ -474,7 +474,7 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr, > * > * @mr: the #MemoryRegion to be initialized. > * @owner: the object that tracks the region's reference count > - * @name: used for debugging; not visible to the user or ABI > + * @name: Region name, becomes part of RAMBlock name used in migration stream > * @orig: the region to be referenced; @mr will be equivalent to > * @orig between @offset and @offset + @size - 1. > * @offset: start of the section in @orig to be referenced. > @@ -537,7 +537,7 @@ void memory_region_init_rom_device(MemoryRegion *mr, > * > * @mr: the #MemoryRegion to be initialized > * @owner: the object that tracks the region's reference count > - * @name: used for debugging; not visible to the user or ABI > + * @name: Region name, becomes part of RAMBlock name used in migration stream > * @size: size of the region. > */ > static inline void memory_region_init_reservation(MemoryRegion *mr, > @@ -558,7 +558,7 @@ static inline void memory_region_init_reservation(MemoryRegion *mr, > * @mr: the #MemoryRegion to be initialized > * @owner: the object that tracks the region's reference count > * @ops: a function that translates addresses into the @target region > - * @name: used for debugging; not visible to the user or ABI > + * @name: Region name, becomes part of RAMBlock name used in migration stream > * @size: size of the region. > */ > void memory_region_init_iommu(MemoryRegion *mr, > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] memory_region: Fix name comments 2017-03-08 10:54 [Qemu-devel] [PATCH] memory_region: Fix name comments Dr. David Alan Gilbert (git) 2017-03-08 11:15 ` Philippe Mathieu-Daudé @ 2017-03-08 11:47 ` Peter Maydell 2017-03-09 11:26 ` Dr. David Alan Gilbert 1 sibling, 1 reply; 6+ messages in thread From: Peter Maydell @ 2017-03-08 11:47 UTC (permalink / raw) To: Dr. David Alan Gilbert (git); +Cc: QEMU Developers, Paolo Bonzini On 8 March 2017 at 11:54, Dr. David Alan Gilbert (git) <dgilbert@redhat.com> wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > The 'name' parameter to memory_region_init_* had been marked as debug > only, however vmstate_region_ram uses it as a parameter to > qemu_ram_set_idstr to set RAMBlock names and these form part of the > migration stream. > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > include/exec/memory.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 6911023..de8f69e 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -307,7 +307,7 @@ struct MemoryRegionSection { > * > * @mr: the #MemoryRegion to be initialized > * @owner: the object that tracks the region's reference count > - * @name: used for debugging; not visible to the user or ABI > + * @name: Region name, becomes part of RAMBlock name used in migration stream ...for RAM-backend MRs only, presumably? Also, what are the uniqueness constraints? > * @size: size of the region; any subregions beyond this size will be clipped > */ > void memory_region_init(MemoryRegion *mr, > @@ -355,7 +355,7 @@ void memory_region_unref(MemoryRegion *mr); > * @ops: a structure containing read and write callbacks to be used when > * I/O is performed on the region. > * @opaque: passed to the read and write callbacks of the @ops structure. > - * @name: used for debugging; not visible to the user or ABI > + * @name: Region name, becomes part of RAMBlock name used in migration stream ditto. > * @size: size of the region. > */ > void memory_region_init_io(MemoryRegion *mr, > @@ -474,7 +474,7 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr, > * > * @mr: the #MemoryRegion to be initialized. > * @owner: the object that tracks the region's reference count > - * @name: used for debugging; not visible to the user or ABI > + * @name: Region name, becomes part of RAMBlock name used in migration stream > * @orig: the region to be referenced; @mr will be equivalent to > * @orig between @offset and @offset + @size - 1. > * @offset: start of the section in @orig to be referenced. The diff here is rather lacking in context, but this comment change is for memory_region_init_alias(). Aliases presumably don't get migrated (the thing they alias into will be migrated if it's RAM), so the comment seems out of place here. > @@ -537,7 +537,7 @@ void memory_region_init_rom_device(MemoryRegion *mr, > * > * @mr: the #MemoryRegion to be initialized > * @owner: the object that tracks the region's reference count > - * @name: used for debugging; not visible to the user or ABI > + * @name: Region name, becomes part of RAMBlock name used in migration stream > * @size: size of the region. > */ > static inline void memory_region_init_reservation(MemoryRegion *mr, Similarly, a reservation MR isn't RAM-backed. > @@ -558,7 +558,7 @@ static inline void memory_region_init_reservation(MemoryRegion *mr, > * @mr: the #MemoryRegion to be initialized > * @owner: the object that tracks the region's reference count > * @ops: a function that translates addresses into the @target region > - * @name: used for debugging; not visible to the user or ABI > + * @name: Region name, becomes part of RAMBlock name used in migration stream > * @size: size of the region. > */ > void memory_region_init_iommu(MemoryRegion *mr, ...and nor is an IOMMU MR. > -- > 2.9.3 > thanks -- PMM -- 12345678901234567890123456789012345678901234567890123456789012345678901234567890 1 2 3 4 5 6 7 8 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] memory_region: Fix name comments 2017-03-08 11:47 ` Peter Maydell @ 2017-03-09 11:26 ` Dr. David Alan Gilbert 2017-03-09 12:01 ` Paolo Bonzini 0 siblings, 1 reply; 6+ messages in thread From: Dr. David Alan Gilbert @ 2017-03-09 11:26 UTC (permalink / raw) To: Peter Maydell; +Cc: QEMU Developers, Paolo Bonzini * Peter Maydell (peter.maydell@linaro.org) wrote: > On 8 March 2017 at 11:54, Dr. David Alan Gilbert (git) > <dgilbert@redhat.com> wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > The 'name' parameter to memory_region_init_* had been marked as debug > > only, however vmstate_region_ram uses it as a parameter to > > qemu_ram_set_idstr to set RAMBlock names and these form part of the > > migration stream. > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > --- > > include/exec/memory.h | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/include/exec/memory.h b/include/exec/memory.h > > index 6911023..de8f69e 100644 > > --- a/include/exec/memory.h > > +++ b/include/exec/memory.h > > @@ -307,7 +307,7 @@ struct MemoryRegionSection { > > * > > * @mr: the #MemoryRegion to be initialized > > * @owner: the object that tracks the region's reference count > > - * @name: used for debugging; not visible to the user or ABI > > + * @name: Region name, becomes part of RAMBlock name used in migration stream > > ...for RAM-backend MRs only, presumably? Also, what are the > uniqueness constraints? Hmm; yes RAM backed or things that behave like RAM backed (e.g. file-backed). Uniqueness isn't required in all cases; qemu_ram_set_idstr prefixes this name by a device name if it's given a device - which it isn't normally for stuff on boards. However the code will at least fail if there's a clash. How about: * @name: Region name, becomes part of RAMBlock name used in migration stream * must be unique within any device > > * @size: size of the region; any subregions beyond this size will be clipped > > */ > > void memory_region_init(MemoryRegion *mr, > > @@ -355,7 +355,7 @@ void memory_region_unref(MemoryRegion *mr); > > * @ops: a structure containing read and write callbacks to be used when > > * I/O is performed on the region. > > * @opaque: passed to the read and write callbacks of the @ops structure. > > - * @name: used for debugging; not visible to the user or ABI > > + * @name: Region name, becomes part of RAMBlock name used in migration stream > > ditto. > > > * @size: size of the region. > > */ > > void memory_region_init_io(MemoryRegion *mr, > > @@ -474,7 +474,7 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr, > > * > > * @mr: the #MemoryRegion to be initialized. > > * @owner: the object that tracks the region's reference count > > - * @name: used for debugging; not visible to the user or ABI > > + * @name: Region name, becomes part of RAMBlock name used in migration stream > > * @orig: the region to be referenced; @mr will be equivalent to > > * @orig between @offset and @offset + @size - 1. > > * @offset: start of the section in @orig to be referenced. > > The diff here is rather lacking in context, but this comment change > is for memory_region_init_alias(). Aliases presumably don't get > migrated (the thing they alias into will be migrated if it's RAM), > so the comment seems out of place here. > > > @@ -537,7 +537,7 @@ void memory_region_init_rom_device(MemoryRegion *mr, > > * > > * @mr: the #MemoryRegion to be initialized > > * @owner: the object that tracks the region's reference count > > - * @name: used for debugging; not visible to the user or ABI > > + * @name: Region name, becomes part of RAMBlock name used in migration stream > > * @size: size of the region. > > */ > > static inline void memory_region_init_reservation(MemoryRegion *mr, > > Similarly, a reservation MR isn't RAM-backed. > > > @@ -558,7 +558,7 @@ static inline void memory_region_init_reservation(MemoryRegion *mr, > > * @mr: the #MemoryRegion to be initialized > > * @owner: the object that tracks the region's reference count > > * @ops: a function that translates addresses into the @target region > > - * @name: used for debugging; not visible to the user or ABI > > + * @name: Region name, becomes part of RAMBlock name used in migration stream > > * @size: size of the region. > > */ > > void memory_region_init_iommu(MemoryRegion *mr, > > ...and nor is an IOMMU MR. OK, so I think we need it on: memory_region_init, memory_region_init_ram, memory_region_init_resizeable_ram, memory_region_init_ram_from_file, memory_region_init_ram_ptr, memory_region_init_rom, memory_region_init_rom_device (?) I don't think it's needed in memory_region_init_ram_device_ptr Dave > > -- > > 2.9.3 > > > > > thanks > -- PMM > > -- > 12345678901234567890123456789012345678901234567890123456789012345678901234567890 > 1 2 3 4 5 6 7 8 -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] memory_region: Fix name comments 2017-03-09 11:26 ` Dr. David Alan Gilbert @ 2017-03-09 12:01 ` Paolo Bonzini 2017-03-09 12:11 ` Dr. David Alan Gilbert 0 siblings, 1 reply; 6+ messages in thread From: Paolo Bonzini @ 2017-03-09 12:01 UTC (permalink / raw) To: Dr. David Alan Gilbert, Peter Maydell; +Cc: QEMU Developers On 09/03/2017 12:26, Dr. David Alan Gilbert wrote: > * @name: Region name, becomes part of RAMBlock name used in migration stream > * must be unique within any device Just a note, the uniqueness constraint also applies for RAM regions only. Paolo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] memory_region: Fix name comments 2017-03-09 12:01 ` Paolo Bonzini @ 2017-03-09 12:11 ` Dr. David Alan Gilbert 0 siblings, 0 replies; 6+ messages in thread From: Dr. David Alan Gilbert @ 2017-03-09 12:11 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Peter Maydell, QEMU Developers * Paolo Bonzini (pbonzini@redhat.com) wrote: > > > On 09/03/2017 12:26, Dr. David Alan Gilbert wrote: > > * @name: Region name, becomes part of RAMBlock name used in migration stream > > * must be unique within any device > > Just a note, the uniqueness constraint also applies for RAM regions only. Yes, I'm only adding this comment to the ram/ram like functions now. Dave > Paolo -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-09 12:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-03-08 10:54 [Qemu-devel] [PATCH] memory_region: Fix name comments Dr. David Alan Gilbert (git) 2017-03-08 11:15 ` Philippe Mathieu-Daudé 2017-03-08 11:47 ` Peter Maydell 2017-03-09 11:26 ` Dr. David Alan Gilbert 2017-03-09 12:01 ` Paolo Bonzini 2017-03-09 12:11 ` Dr. David Alan Gilbert
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).