On Thu, 12 Apr 2018 11:15:04 +0200 Paolo Bonzini <pbonz...@redhat.com> wrote:
> On 29/03/2018 05:21, Alexey Kardashevskiy wrote: > > + DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, > > TYPE_DEVICE); > > + const char *id = object_property_print(obj, "id", true, NULL); > > I learnt now about commit e1ff3c67e8544f41f1bea76ba76385faee0d2bb7 and I > find it a mistake. The "id" is available through > object_get_canonical_path_component. Can you change the "id" assignment > to use object_get_canonical_path_component instead? object_get_canonical_path_component() should be used with care as it assumes that object HAS parent. It will crash if parent is NULL or hasn't been assigned yet. Generally object doesn't need to know its own name, we use it only for debugging and nice error reporting so far. I'd rather have 'id' property at Object level so we won't have to fish out ID from parent /which we aren't supposed to do and which doesn't work in some cases/ when it's needed within object itself. > > + const char *name = object_property_print(obj, "name", true, NULL); > > Who defines a name property? > > > + mon_printf(f, " %s:{", label); > > + if (dev) { > > + mon_printf(f, "dev"); > > + if (dev->id) { > > + mon_printf(f, " id=%s", dev->id); > > + } > > + } else { > > + mon_printf(f, "obj"); > > > + } > > + if (name) { > > + mon_printf(f, " name=%s ", name); > > + } > > + if (id) { > > + mon_printf(f, " id=%s ", id); > > + } > > + if (dev && (!name && !id && !dev->id)) { > > + mon_printf(f, " path=%s", dev->canonical_path); > > Why not print the path for the !dev case? I think we can just have > > mon_prinf(f, " %s:{%s", label, dev ? "dev" : "obj"); > if (dev ? dev->id : id) { > mon_printf(f, " id=%s", dev ? dev->id : id); > } else { > canonical_path = object_get_canonical_path(...) > mon_printf(f, " path=%s", canonical_path); > g_free(canonical_path); > } > > Thanks, > > Paolo