* [Qemu-devel] [PATCH] pc-dimm: use same mechanism for [get|set]_addr
@ 2019-02-01 1:08 Wei Yang
2019-02-06 11:59 ` Igor Mammedov
0 siblings, 1 reply; 4+ messages in thread
From: Wei Yang @ 2019-02-01 1:08 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, imammedo, Wei Yang
[get|set]_addr are two counterpart to access PCDIMMDevice.addr.
Since we have already set up a property PC_DIMM_ADDR_PROP for this
field and use this mechanism in set_addr, it would be more proper to use
the same mechanism in set_addr.
This patch uses object_property_get_uint() to replace the direct memory
access to make [get|set]_addr with the same mechanism.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
hw/mem/pc-dimm.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 0c9b9e8292..c0658b9b88 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -204,9 +204,7 @@ static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm, Error **errp)
static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md)
{
- const PCDIMMDevice *dimm = PC_DIMM(md);
-
- return dimm->addr;
+ return object_property_get_uint(OBJECT(md), PC_DIMM_ADDR_PROP, NULL);
}
static void pc_dimm_md_set_addr(MemoryDeviceState *md, uint64_t addr,
--
2.19.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] pc-dimm: use same mechanism for [get|set]_addr
2019-02-01 1:08 [Qemu-devel] [PATCH] pc-dimm: use same mechanism for [get|set]_addr Wei Yang
@ 2019-02-06 11:59 ` Igor Mammedov
2019-02-06 23:55 ` Wei Yang
0 siblings, 1 reply; 4+ messages in thread
From: Igor Mammedov @ 2019-02-06 11:59 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, mst
On Fri, 1 Feb 2019 09:08:37 +0800
Wei Yang <richardw.yang@linux.intel.com> wrote:
> [get|set]_addr are two counterpart to access PCDIMMDevice.addr.
>
> Since we have already set up a property PC_DIMM_ADDR_PROP for this
> field and use this mechanism in set_addr, it would be more proper to use
> the same mechanism in set_addr.
>
> This patch uses object_property_get_uint() to replace the direct memory
> access to make [get|set]_addr with the same mechanism.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
> hw/mem/pc-dimm.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 0c9b9e8292..c0658b9b88 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -204,9 +204,7 @@ static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm, Error **errp)
>
> static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md)
> {
> - const PCDIMMDevice *dimm = PC_DIMM(md);
> -
> - return dimm->addr;
> + return object_property_get_uint(OBJECT(md), PC_DIMM_ADDR_PROP, NULL);
it's not good to ignore errors
s/NULL/error_abort/
so we would notice if error ever happened.
> }
>
> static void pc_dimm_md_set_addr(MemoryDeviceState *md, uint64_t addr,
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] pc-dimm: use same mechanism for [get|set]_addr
2019-02-06 11:59 ` Igor Mammedov
@ 2019-02-06 23:55 ` Wei Yang
2019-02-07 10:49 ` Igor Mammedov
0 siblings, 1 reply; 4+ messages in thread
From: Wei Yang @ 2019-02-06 23:55 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Wei Yang, qemu-devel, mst
On Wed, Feb 06, 2019 at 12:59:49PM +0100, Igor Mammedov wrote:
>On Fri, 1 Feb 2019 09:08:37 +0800
>Wei Yang <richardw.yang@linux.intel.com> wrote:
>
>> [get|set]_addr are two counterpart to access PCDIMMDevice.addr.
>>
>> Since we have already set up a property PC_DIMM_ADDR_PROP for this
>> field and use this mechanism in set_addr, it would be more proper to use
>> the same mechanism in set_addr.
>>
>> This patch uses object_property_get_uint() to replace the direct memory
>> access to make [get|set]_addr with the same mechanism.
>>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> ---
>> hw/mem/pc-dimm.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
>> index 0c9b9e8292..c0658b9b88 100644
>> --- a/hw/mem/pc-dimm.c
>> +++ b/hw/mem/pc-dimm.c
>> @@ -204,9 +204,7 @@ static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm, Error **errp)
>>
>> static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md)
>> {
>> - const PCDIMMDevice *dimm = PC_DIMM(md);
>> -
>> - return dimm->addr;
>> + return object_property_get_uint(OBJECT(md), PC_DIMM_ADDR_PROP, NULL);
>it's not good to ignore errors
>
>s/NULL/error_abort/
>
>so we would notice if error ever happened.
You mean something like this?
Error *err;
uint64_t addr;
addr = object_property_get_uint(OBJECT(md), PC_DIMM_ADDR_PROP, &err);
if (err) {
warn_report_err(&err);
return 0;
}
return addr;
>
>> }
>>
>> static void pc_dimm_md_set_addr(MemoryDeviceState *md, uint64_t addr,
>
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] pc-dimm: use same mechanism for [get|set]_addr
2019-02-06 23:55 ` Wei Yang
@ 2019-02-07 10:49 ` Igor Mammedov
0 siblings, 0 replies; 4+ messages in thread
From: Igor Mammedov @ 2019-02-07 10:49 UTC (permalink / raw)
To: Wei Yang; +Cc: Wei Yang, qemu-devel, mst
On Wed, 6 Feb 2019 23:55:15 +0000
Wei Yang <richard.weiyang@gmail.com> wrote:
> On Wed, Feb 06, 2019 at 12:59:49PM +0100, Igor Mammedov wrote:
> >On Fri, 1 Feb 2019 09:08:37 +0800
> >Wei Yang <richardw.yang@linux.intel.com> wrote:
> >
> >> [get|set]_addr are two counterpart to access PCDIMMDevice.addr.
> >>
> >> Since we have already set up a property PC_DIMM_ADDR_PROP for this
> >> field and use this mechanism in set_addr, it would be more proper to use
> >> the same mechanism in set_addr.
> >>
> >> This patch uses object_property_get_uint() to replace the direct memory
> >> access to make [get|set]_addr with the same mechanism.
> >>
> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> >> ---
> >> hw/mem/pc-dimm.c | 4 +---
> >> 1 file changed, 1 insertion(+), 3 deletions(-)
> >>
> >> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> >> index 0c9b9e8292..c0658b9b88 100644
> >> --- a/hw/mem/pc-dimm.c
> >> +++ b/hw/mem/pc-dimm.c
> >> @@ -204,9 +204,7 @@ static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm, Error **errp)
> >>
> >> static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md)
> >> {
> >> - const PCDIMMDevice *dimm = PC_DIMM(md);
> >> -
> >> - return dimm->addr;
> >> + return object_property_get_uint(OBJECT(md), PC_DIMM_ADDR_PROP, NULL);
> >it's not good to ignore errors
> >
> >s/NULL/error_abort/
> >
> >so we would notice if error ever happened.
>
> You mean something like this?
>
> Error *err;
> uint64_t addr;
>
> addr = object_property_get_uint(OBJECT(md), PC_DIMM_ADDR_PROP, &err);
see how error_abort is used in QEMU,
point is if there is a error that should never happen it is programming error so
QEMU should crash and we should fix it.
>
> if (err) {
> warn_report_err(&err);
> return 0;
> }
>
> return addr;
>
> >
> >> }
> >>
> >> static void pc_dimm_md_set_addr(MemoryDeviceState *md, uint64_t addr,
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-02-07 10:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-01 1:08 [Qemu-devel] [PATCH] pc-dimm: use same mechanism for [get|set]_addr Wei Yang
2019-02-06 11:59 ` Igor Mammedov
2019-02-06 23:55 ` Wei Yang
2019-02-07 10:49 ` Igor Mammedov
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).