Re: [Qemu-devel] [PATCH v4 19/24] virtio-pmem: prototype

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 26 Sep 2018 11:42:14 +0200
David Hildenbrand <david@redhat.com> wrote:

> From: Pankaj Gupta <pagupta@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 <pagupta@redhat.com>
> [ MemoryDevice/MemoryRegion changes, cleanups, addr property "memaddr",
>   split up patches ]
> Signed-off-by: David Hildenbrand <david@redhat.com>
not an actual review, but a small nit/no/ below

[...]
> +
> +static const char *virtio_pmem_md_get_device_id(const MemoryDeviceState *md)
> +{
> +    Object *obj = OBJECT(md);
> +
> +    /* always return the ID of the proxy device the user configured */
> +    if (obj->parent && object_dynamic_cast(obj->parent, TYPE_DEVICE)) {
> +        const DeviceState *parent_dev = DEVICE(obj->parent);
> +
> +        return parent_dev->id;
It's layer violation, child device shouldn't ever access it's parent fields,
in general it's not acceptable practice.
To workaround parent could set a property for a child with this id to use,
but I'm not sure if it would work in this case.

Anyways you might not need this callback at all if proxy would implement
memory-device interface as mentioned in another comment.


> +    }
> +    return NULL;
> +}
> +
[...]