On 21/09/2018 10:07, Markus Armbruster wrote: > Quick review of just the QAPI part. > > David Hildenbrand <da...@redhat.com> writes: > >> From: Pankaj Gupta <pagu...@redhat.com> >> >> This is the current protoype of virtio-pmem. Support will require >> machine changes for the architectures that will support it, so it will >> not yet be compiled. >> >> Signed-off-by: Pankaj Gupta <pagu...@redhat.com> >> [ MemoryDevice/MemoryRegion changes, cleanups, addr property "memaddr", >> split up patches ] >> Signed-off-by: David Hildenbrand <da...@redhat.com> > [...] >> diff --git a/qapi/misc.json b/qapi/misc.json >> index 7c36de0464..cadbca26ac 100644 >> --- a/qapi/misc.json >> +++ b/qapi/misc.json >> @@ -2907,6 +2907,29 @@ >> } >> } >> >> +## >> +# @VirtioPMemDeviceInfo: >> +# >> +# VirtioPMem state information >> +# >> +# @id: device's ID >> +# >> +# @memaddr: physical address in memory, where device is mapped >> +# >> +# @size: size of memory that the device provides >> +# >> +# @memdev: memory backend linked with device >> +# >> +# Since: 3.1 >> +## >> +{ 'struct': 'VirtioPMemDeviceInfo', >> + 'data': { '*id': 'str', >> + 'memaddr': 'size', >> + 'size': 'size', >> + 'memdev': 'str' >> + } >> +} > > This set of members is a proper subset of PCDIMMDeviceInfo's, except > > * It uses 'size' instead of 'int', which is an improvement. Improve > PCDIMMDeviceInfo as well?
That certainly makes sense. @Pankaj do you want to take care of that? > > * The physical address member is called 'memaddr' instead of 'addr'. > Why? > Very good point. Have a look at "memory-device: add device class function set_addr()" (patch #10). Summary: The property name on the device will be called "memaddr", as "addr" is already (unfortunately) used for virtio addressing, that's why I feel like we should name it here "memaddr", too. ("addr" is too generic, a collision had to happen :( ) -- Thanks, David / dhildenb