From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36563) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YbV7c-0005gU-8i for qemu-devel@nongnu.org; Fri, 27 Mar 2015 10:20:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YbV7Y-00021p-Un for qemu-devel@nongnu.org; Fri, 27 Mar 2015 10:20:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60834) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YbV7Y-00021j-Ms for qemu-devel@nongnu.org; Fri, 27 Mar 2015 10:20:24 -0400 Date: Fri, 27 Mar 2015 15:20:18 +0100 From: Igor Mammedov Message-ID: <20150327152018.34c059d4@nial.brq.redhat.com> In-Reply-To: <20150326162617.5b82b604@redhat.com> References: <1427388174-30915-1-git-send-email-imammedo@redhat.com> <20150326155925.50ce214a@redhat.com> <20150326162617.5b82b604@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] fix assertion in "info memory-devices" if memdev isn't accessible List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, afaerber@suse.de, lma@suse.com On Thu, 26 Mar 2015 16:26:17 -0400 Luiz Capitulino wrote: > On Thu, 26 Mar 2015 15:59:25 -0400 > Luiz Capitulino wrote: > > > On Thu, 26 Mar 2015 16:42:54 +0000 > > Igor Mammedov wrote: > > > > > showing a memory device whose memdev is removed leads to an assert: > > > > > > (qemu) object_add memory-backend-ram,id=ram0,size=128M > > > (qemu) device_add pc-dimm,id=d0,memdev=ram0 > > > (qemu) object_del ram0 > > > (qemu) info memory-devices > > > ** > > > ERROR:qom/object.c:1274:object_get_canonical_path_component:\ > > > assertion failed: (obj->parent != NULL) > > > Aborted > > > > > > so check if memdev is present before calling > > > object_get_canonical_path() and in case it's not assign to > > > "memdev:" field "removed" value. > > > > > > Signed-off-by: Igor Mammedov > > > > Igor, do you want to go in through my tree? > > What about the patch, btw? :) > > > > > > --- > > > alternative more generic solution: > > > http://patchwork.ozlabs.org/patch/453343/ > > > --- > > > backends/hostmem.c | 23 +++++++++++++++++++++++ > > > hw/mem/pc-dimm.c | 15 ++++++++++++++- > > > include/sysemu/hostmem.h | 1 + > > > qmp.c | 1 + > > > 4 files changed, 39 insertions(+), 1 deletion(-) > > > > > > diff --git a/backends/hostmem.c b/backends/hostmem.c > > > index 99e8f99..aa88691 100644 > > > --- a/backends/hostmem.c > > > +++ b/backends/hostmem.c > > > @@ -227,6 +227,26 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value, > > > } > > > } > > > > > > +static void host_memory_backend_set_id(Object *obj, const char *id, > > > + Error **errp) > > > +{ > > > + HostMemoryBackend *backend = MEMORY_BACKEND(obj); > > > + > > > + if (backend->id) { > > > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "id", > > > + "can't change already set value"); > > > + return; > > > + } > > > + backend->id = g_strdup(id); > > > +} > > > + > > > +static char *host_memory_backend_get_id(Object *obj, Error **errp) > > > +{ > > > + HostMemoryBackend *backend = MEMORY_BACKEND(obj); > > > + > > > + return g_strdup(backend->id); > > > +} > > > + > > > static void host_memory_backend_init(Object *obj) > > > { > > > HostMemoryBackend *backend = MEMORY_BACKEND(obj); > > > @@ -237,6 +257,9 @@ static void host_memory_backend_init(Object *obj) > > > "dump-guest-core", true); > > > backend->prealloc = mem_prealloc; > > > > > > + object_property_add_str(obj, "id", > > > + host_memory_backend_get_id, > > > + host_memory_backend_set_id, NULL); > > > object_property_add_bool(obj, "merge", > > > host_memory_backend_get_merge, > > > host_memory_backend_set_merge, NULL); > > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c > > > index 39f0c97..482a7a0 100644 > > > --- a/hw/mem/pc-dimm.c > > > +++ b/hw/mem/pc-dimm.c > > > @@ -69,6 +69,8 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque) > > > DeviceState *dev = DEVICE(obj); > > > > > > if (dev->realized) { > > > + char *mdevid, *mdevpath; > > > + Object *mdev; > > > MemoryDeviceInfoList *elem = g_new0(MemoryDeviceInfoList, 1); > > > MemoryDeviceInfo *info = g_new0(MemoryDeviceInfo, 1); > > > PCDIMMDeviceInfo *di = g_new0(PCDIMMDeviceInfo, 1); > > > @@ -86,7 +88,18 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque) > > > di->node = dimm->node; > > > di->size = object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, > > > NULL); > > > - di->memdev = object_get_canonical_path(OBJECT(dimm->hostmem)); > > > + mdevid = object_property_get_str(OBJECT(dimm->hostmem), "id", > > > + &error_abort); > > > + mdevpath = g_strdup_printf("/objects/%s", mdevid); > > > + g_free(mdevid); > > > + mdev = object_resolve_path_type(mdevpath, TYPE_MEMORY_BACKEND, > > > + NULL); > > > + if (mdev == OBJECT(dimm->hostmem)) { > > > + di->memdev = object_get_canonical_path(OBJECT(dimm->hostmem)); > > > + } else { > > > + di->memdev = g_strdup("removed"); > > > + } > > > + g_free(mdevpath); > > > > > > info->dimm = di; > > > elem->value = info; > > > diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h > > > index 1ce4394..63918aa 100644 > > > --- a/include/sysemu/hostmem.h > > > +++ b/include/sysemu/hostmem.h > > > @@ -53,6 +53,7 @@ struct HostMemoryBackend { > > > Object parent; > > > > > > /* protected */ > > > + char *id; > > > uint64_t size; > > > bool merge, dump; > > > bool prealloc, force_prealloc; > > > diff --git a/qmp.c b/qmp.c > > > index c479e77..6f1cca7 100644 > > > --- a/qmp.c > > > +++ b/qmp.c > > > @@ -649,6 +649,7 @@ void object_add(const char *type, const char *id, const QDict *qdict, > > > } > > > } > > > } > > > + object_property_set_str(obj, id, "id", NULL); > > > > > > object_property_add_child(container_get(object_get_root(), "/objects"), > > > id, obj, &local_err); > > > > pls, ignore it there is a better alternative patch on list