From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53733) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YS7Yn-0003hB-PN for qemu-devel@nongnu.org; Sun, 01 Mar 2015 12:21:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YS7Yj-00050Q-62 for qemu-devel@nongnu.org; Sun, 01 Mar 2015 12:21:45 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40055) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YS7Yi-000508-Tn for qemu-devel@nongnu.org; Sun, 01 Mar 2015 12:21:41 -0500 Date: Sun, 1 Mar 2015 18:21:31 +0100 From: "Michael S. Tsirkin" Message-ID: <20150301172131.GD8233@redhat.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v3 06/10] acpi, mem-hotplug: Add unplug cb for memory device. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhu Guihua 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 Thu, Feb 26, 2015 at 09:16:48AM +0800, Zhu Guihua wrote: > From: Tang Chen > > Reset all memory status, and unparent the memory device. > > Signed-off-by: Tang Chen > Signed-off-by: Zhu Guihua Please, stop breaking patches up like this. You add new a function, with zero documentation, and no users until later in series. Does it do what it should do? Why are there now both acpi_memory_unplug_request_cb and acpi_memory_unplug_cb? When is this called? No way to tell from documentation since there's none. I guess one is supposed to look at the follow-up patches to figure it out. Just include this in the next patch then. Same applies everywhere really. Split patches along functional lines, not by affected file. > --- > hw/acpi/memory_hotplug.c | 14 ++++++++++++++ > include/hw/acpi/memory_hotplug.h | 2 ++ > 2 files changed, 16 insertions(+) > > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c > index 02231d2..ffacecf 100644 > --- a/hw/acpi/memory_hotplug.c > +++ b/hw/acpi/memory_hotplug.c > @@ -223,6 +223,20 @@ void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq, > acpi_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS); > } > > +void acpi_memory_unplug_cb(MemHotplugState *mem_st, > + DeviceState *dev, Error **errp) > +{ > + MemStatus *mdev; > + > + mdev = acpi_memory_slot_status(mem_st, dev, errp); > + if (!mdev) { > + return; > + } > + > + mdev->is_enabled = false; > + mdev->dimm = NULL; > +} > + > static const VMStateDescription vmstate_memhp_sts = { > .name = "memory hotplug device state", > .version_id = 1, > diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h > index c437a85..15deae0 100644 > --- a/include/hw/acpi/memory_hotplug.h > +++ b/include/hw/acpi/memory_hotplug.h > @@ -32,6 +32,8 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st, > void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq, > MemHotplugState *mem_st, > DeviceState *dev, Error **errp); > +void acpi_memory_unplug_cb(MemHotplugState *mem_st, > + DeviceState *dev, Error **errp); > > extern const VMStateDescription vmstate_memory_hotplug; > #define VMSTATE_MEMORY_HOTPLUG(memhp, state) \ > -- > 1.9.3