Il 22/09/2014 14:34, Michael S. Tsirkin ha scritto: > On Mon, Sep 22, 2014 at 02:06:38PM +0200, Paolo Bonzini wrote: >> Il 22/09/2014 13:22, Gonglei (Arei) ha scritto: >>>> This doesn't change the fact that ObjectProperty is a generic struct, >>>> and adding alias-specific fields there is wrong. >>> >>> OK, Maybe I should find other ways to attach this purpose and >>> avoid layering violation. Thanks! >> >> Unfortunately I cannot think of any. >> >> We could add a description field to ObjectProperty, and replace >> legacy_name with a description. The output then would be >> >> virtio-blk.drive=str (drive) >> That's a bit hackish, but perhaps it would be good enough for mst and >> Markus? > > I would just drop "str" and replace it with drive.
In the above example, the format would be CLASS.PROPERTY=TYPE (DESCRIPTION) where I just put "drive" as the description to resemble the old output. You cannot just have foo.drive=drive because the type is *not* "drive", it's "str" (if anything, we could make it link<BlockBackend> which does not really show the connection between BlockBackend and -drive). The problem is that "drive" as the type (the legacy_name) is simply not available for the virtio-blk-pci.drive property. It's hidden beneath an opaque pointer. Gonglei's patches were adding fields to ObjectProperty just to avoid visiting that opaque pointer, which I consider a layering violation. > I seems to convey zero information. > Description should be something more informative, e.g. > (ID of a drive to use as a backend) That would of course be fine too. The problem is then that we have 644 properties (DEFINE_PROP_* macro invocations) that someone must add a description to. > Help for -device could add "Must match ID of a -drive option". > Help for device HMP command could add "Must match ID of a drive command." You are turning a bug that nobody noticed until now, and that really doesn't break anything, into a month or more worth of work. Paolo
