Re: [Qemu-devel] [PATCH v5 00/10] Add support for VM Generation ID

[Igor Mammedov]

On Mon, 13 Feb 2017 15:00:22 +0200
"Michael S. Tsirkin" <address@hidden> wrote:

> On Mon, Feb 13, 2017 at 12:00:36PM +0100, Igor Mammedov wrote:
> > On Fri, 10 Feb 2017 19:27:21 +0100
> > Andreas Färber <address@hidden> wrote:
> >   
> > > Am 10.02.2017 um 19:18 schrieb Andrew Jones:  
> > > > On Fri, Feb 10, 2017 at 05:16:59PM +0100, Igor Mammedov wrote:    
> > > >> On Fri, 10 Feb 2017 17:31:33 +0200
> > > >> "Michael S. Tsirkin" <address@hidden> wrote:
> > > >>    
> > > >>> On Fri, Feb 10, 2017 at 11:12:13AM +0100, Laszlo Ersek wrote:    
> > > >>>> On 02/05/17 10:11, address@hidden wrote:      
> > > >>>>> From: Ben Warren <address@hidden>    
> > > >> [...]
> > > >>    
> > > >>>>
> > > >>>> - or else, add another boolean property to vmgenid, one that 
> > > >>>> parallels
> > > >>>> "dma-enabled" of "fw-cfg" precisely, in HW_COMPAT*. Then simply fail
> > > >>>> realize() when this property is false.      
> > > >>>
> > > >>> That's probably the easiest way. x-fw-cfg-dma-enabled.
> > > >>> Won't delay merging because of this, can be done with
> > > >>> patch on top.    
> > > >> (not related to this series)
> > > >>
> > > >> I've thought that there still were no consensus on x-foo prefix,
> > > >> not to mention that x- might be legitimate prefix for some properties.
*1

> > > >>
> > > >> Maybe we should add a flag to property like INTERNAL_PROPERTY
> > > >> and then set it explicitly on for internal stuff.
> > > >>
> > > >> That way we could cleanly exclude internal properties from
> > > >>  -device foo,help and make sure that user won't set them from CLI.
> > > >> I'd even volunteer to add this API to Object    
> > > > 
> > > > Yes, please. I know of a property or two where it would be nice to
> > > > have that flag.    
> > > 
> > > Apart from documentation, what effect would such a flag have?
> > > 
> > > With QOM I don't really see "internal" as being a thing: Besides -device
> > > and the likes, we expose qom-set operation and -global option, and I
> > > don't think it makes sense to restrict the latter two. For -device,
> > > "realized" is a property I would classify as non-user maybe.  
> > I'd propose to start with this flag hiding/forbiding usage of
> > property in following interfaces:
> > 
> >   * -device
> >   * -device foo,help
> >   * device_add
> > 
> > With docs mentioning that usage of hidden properties with
> >  -globals/qom-set isn't recommended.  
> 
> I'm not sure I agree. These properties can be useful e.g. for debugging,
> the only issue is we don't guarantee to support them across QEMU
> releases. If you won't want something exposed to users, don't make it a
> property at all.
Property approach is cleaner especially in cases where
it's used to tie together generic with target-specific code
+ automatically makes feature introspect-able via qom-get
(I've used it for debugging to poke into object properties
without need for gdb).
Replacing property with direct function call makes one
to create stubs (I've seen Paolo working on removing stubs)
for targets that do not implement it or uglifying headers with
target specific ifdefs.

> Producing a warning when a property starting with x- is set
> might not be a bad idea.
I don't like x- prefix for above [1] and
we already have a bunch of properties that shouldn't be
user settable that makes x- convention inconsistent.
so how about following:
   * -device foo,help:
       * hide non user flagged properties or
       *less preferable mark them as non stable/supportable
   * issue warning in case such property is used
     with -device/device_add
   * since it's warning we could probably put it in -globals as well
     so user would notice probable misuse
  

> > > 
> > > Regards,
> > > Andreas
> > >   
>