From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59553) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YSzc8-0007Wp-TQ for qemu-devel@nongnu.org; Tue, 03 Mar 2015 22:04:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YSzc5-000165-NZ for qemu-devel@nongnu.org; Tue, 03 Mar 2015 22:04:48 -0500 Received: from [59.151.112.132] (port=37571 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YSzc5-00014L-Bu for qemu-devel@nongnu.org; Tue, 03 Mar 2015 22:04:45 -0500 Message-ID: <54F675F6.9050509@cn.fujitsu.com> Date: Wed, 4 Mar 2015 11:03:18 +0800 From: Zhu Guihua MIME-Version: 1.0 References: <26bd642f7692697d24cc6c8ffae8bb4b2cc94e7b.1424912878.git.zhugh.fnst@cn.fujitsu.com> <20150301173139.GG8233@redhat.com> <54F519E4.6090502@cn.fujitsu.com> <20150303134353.GB17209@redhat.com> In-Reply-To: <20150303134353.GB17209@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 02/10] acpi, mem-hotplug: Add acpi_memory_slot_status() to get MemStatus. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: hutao@cn.fujitsu.com, qemu-devel@nongnu.org, tangchen@cn.fujitsu.com, isimatu.yasuaki@jp.fujitsu.com, pbonzini@redhat.com, guz.fnst@cn.fujitsu.com, imammedo@redhat.com On 03/03/2015 09:43 PM, Michael S. Tsirkin wrote: > On Tue, Mar 03, 2015 at 10:18:12AM +0800, Zhu Guihua wrote: >> On 03/02/2015 01:31 AM, Michael S. Tsirkin wrote: >>> On Thu, Feb 26, 2015 at 09:16:44AM +0800, Zhu Guihua wrote: >>>> From: Tang Chen >>>> >>>> Add a new API named acpi_memory_slot_status() to obtain a single memory >>>> slot status. Doing this is because this procedure will be used by other >>>> functions in the next coming patches. >>>> >>>> Signed-off-by: Tang Chen >>>> Signed-off-by: Zhu Guihua >>> Same comments as everywhere: include implementation with >>> users. >>> In this case, I can't tell whether error_setg is >>> appropriate since I don't know whether the error >>> ever comes from user. >> Do you have any suggestions about this error_setg? >> >> Thanks, >> Zhu > If it's guest triggered, it probably shouldn't > print to monitor. If it's user triggered, then it should. Oh, I understand. In this case, the error is not triggered by guest. Thanks, Zhu >>>> --- >>>> hw/acpi/memory_hotplug.c | 26 +++++++++++++++++++------- >>>> 1 file changed, 19 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c >>>> index c6580da..6d91a0d 100644 >>>> --- a/hw/acpi/memory_hotplug.c >>>> +++ b/hw/acpi/memory_hotplug.c >>>> @@ -163,29 +163,41 @@ void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner, >>>> memory_region_add_subregion(as, ACPI_MEMORY_HOTPLUG_BASE, &state->io); >>>> } >>>> -void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st, >>>> - DeviceState *dev, Error **errp) >>>> +static MemStatus * >>>> +acpi_memory_slot_status(MemHotplugState *mem_st, >>>> + DeviceState *dev, Error **errp) >>>> { >>>> - MemStatus *mdev; >>>> Error *local_err = NULL; >>>> int slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP, >>>> &local_err); >>>> if (local_err) { >>>> error_propagate(errp, local_err); >>>> - return; >>>> + return NULL; >>>> } >>>> if (slot >= mem_st->dev_count) { >>>> char *dev_path = object_get_canonical_path(OBJECT(dev)); >>>> - error_setg(errp, "acpi_memory_plug_cb: " >>>> + error_setg(errp, "acpi_memory_get_slot_status_descriptor: " >>>> "device [%s] returned invalid memory slot[%d]", >>>> - dev_path, slot); >>>> + dev_path, slot); >>>> g_free(dev_path); >>>> + return NULL; >>>> + } >>>> + >>>> + return &mem_st->devs[slot]; >>>> +} >>>> + >>>> +void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st, >>>> + DeviceState *dev, Error **errp) >>>> +{ >>>> + MemStatus *mdev; >>>> + >>>> + mdev = acpi_memory_slot_status(mem_st, dev, errp); >>>> + if (!mdev) { >>>> return; >>>> } >>>> - mdev = &mem_st->devs[slot]; >>>> mdev->dimm = dev; >>>> mdev->is_enabled = true; >>>> mdev->is_inserting = true; >>>> -- >>>> 1.9.3 >>> . >>> > . >