Re: [Qemu-devel] [PATCH v2 10/10] hostmem: use object id for memory region name with >= 3.1

[Eduardo Habkost]

On Thu, Nov 01, 2018 at 04:16:12PM +0100, Igor Mammedov wrote:
> On Wed, 31 Oct 2018 17:27:57 -0300
> Eduardo Habkost <address@hidden> wrote:
> 
> > On Tue, Oct 30, 2018 at 07:04:53PM +0400, Marc-André Lureau wrote:
> > > hostmem-file and hostmem-memfd use the whole object path for the
> > > memory region name, and hostname-ram uses only the path component (the
> > > object id, or canonical path basename):
> > > 
> > > qemu -m 1024 -object memory-backend-file,id=mem,size=1G,mem-path=/tmp/foo 
> > > -numa node,memdev=mem -monitor stdio
> > > (qemu) info ramblock
> > >               Block Name    PSize              Offset               Used  
> > >             Total
> > >             /objects/mem    4 KiB  0x0000000000000000 0x0000000040000000 
> > > 0x0000000040000000
> > > 
> > > qemu -m 1024 -object memory-backend-memfd,id=mem,size=1G -numa 
> > > node,memdev=mem -monitor stdio
> > > (qemu) info ramblock
> > >               Block Name    PSize              Offset               Used  
> > >             Total
> > >             /objects/mem    4 KiB  0x0000000000000000 0x0000000040000000 
> > > 0x0000000040000000
> > > 
> > > qemu -m 1024 -object memory-backend-ram,id=mem,size=1G -numa 
> > > node,memdev=mem -monitor stdio
> > > (qemu) info ramblock
> > >               Block Name    PSize              Offset               Used  
> > >             Total
> > >                      mem    4 KiB  0x0000000000000000 0x0000000040000000 
> > > 0x0000000040000000
> > > 
> > > Use the object id for -file and -memfd with >= 3.1 for consistency.
> > > Having a consistent naming allow to migrate to different hostmem
> > > backends.
> > > 
> > > Signed-off-by: Marc-André Lureau <address@hidden>  
> > 
> > I don't want to make you feel like you wasted your time on the
> > global property system refactor, but:
> > 
> > Maybe it would be simpler to add a
> >   bool MachineClass::canonical_path_for_ramblock_id
> I dislike adding compat flags in random places in this case 
> it has nothing to do with machine.

I don't see why it would be a problem.  If you really believe
this compat flag doesn't belong to a MachineClass field, it would
not belong to MachineClass::compat_props either.


> 
> > field, instead of refactoring the global property system,
> > considering that we're past soft freeze?
> We can merge it for 3.2,
> I have another potential usage for Mark's work,
> converting initial memory to memory devices (so far only ARM),
> where we would need to keep ramblock id's compatible across
> different machine versions and a different way we were creating
> initial memory memory regions so migration would not fail.

Right, so allowing backend objects to be affected by
MachineClass::compat_props would be nice to have, long term.

> 
> > Sometimes I think the global property system was a mistake, and
> > that we should avoid spreading it to other subsystems.
> Well user accessible globals might be a mistake,
> but it's quite useful/convenient API to fix compatibility parameters
> for different machines/versions in uniform way without hacking
> unrelated code.

Agreed.  Having compat_props is nice.  Mixing -global and
MachineClass::compat_props was a mistake, and I want to revert
that.  See the suggestion on the reply to 05/10 on how we could
do this.

-- 
Eduardo