From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33271) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1duezg-00011z-Rw for qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:26:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dueyc-0001Is-6G for qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:24:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43696) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dueyU-00017V-8r for qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:23:40 -0400 References: <1505811353-29151-1-git-send-email-thuth@redhat.com> <84d1e5a0-141f-cd91-f62c-14fe58e73b4b@redhat.com> From: Marcel Apfelbaum Message-ID: <54a1ab1c-4da0-ded1-c225-f50d4756b773@redhat.com> Date: Wed, 20 Sep 2017 13:57:16 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH] qdev: Mark devices as non-hotpluggable by default List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , Eduardo Habkost Cc: Thomas Huth , QEMU Developers , Igor Mammedov , Paolo Bonzini , Xiao Guangrong , "Michael S. Tsirkin" , Cornelia Huck , Christian Borntraeger , Gerd Hoffmann , Stefano Stabellini , Anthony Perard On 20/09/2017 13:07, Peter Maydell wrote: > On 20 September 2017 at 08:50, Marcel Apfelbaum wrote: >> Hi Thomas, >> >> >> On 19/09/2017 11:55, Thomas Huth wrote: >>> >>> Historically we've marked all devices as hotpluggable by default. However, >>> most devices are not hotpluggable, and you also need a HotplugHandler to >>> support these devices. So if the user tries to "device_add" or >>> "device_del" >>> such a non-hotpluggable device during runtime, either nothing really >>> usable >>> happens, or QEMU even crashes/aborts unexpectedly (see for example commit >>> 84ebd3e8c7d4fe955b - "Mark diag288 watchdog as non-hotpluggable"). >>> So let's change this dangerous default behaviour and mark the devices as >>> non-hotpluggable by default. Certain parent devices classes which are >>> known >>> as hotpluggable (e.g. PCI, USB, etc.) are marked with "hotpluggable = >>> true", >>> so that devices that are derived from these classes continue to work as >>> expected. >>> >>> Signed-off-by: Thomas Huth >>> --- >>> Note: I've marked the patch as RFC since I'm not 100% sure whether I've >>> correctly identified all devices that should still be marked as hot- >>> pluggable. Feedback is welcome! >>> >>> hw/core/qdev.c | 10 ++++------ >>> hw/cpu/core.c | 1 + >>> hw/mem/nvdimm.c | 3 +++ >>> hw/mem/pc-dimm.c | 1 + >>> hw/pci/pci.c | 1 + >>> hw/s390x/ccw-device.c | 1 + >>> hw/scsi/scsi-bus.c | 1 + >>> hw/usb/bus.c | 1 + >>> 8 files changed, 13 insertions(+), 6 deletions(-) >>> >>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >>> index 606ab53..c4f1902 100644 >>> --- a/hw/core/qdev.c >>> +++ b/hw/core/qdev.c >>> @@ -1120,13 +1120,11 @@ static void device_class_init(ObjectClass *class, >>> void *data) >>> dc->realize = device_realize; >>> dc->unrealize = device_unrealize; >>> - /* by default all devices were considered as hotpluggable, >>> - * so with intent to check it in generic qdev_unplug() / >>> - * device_set_realized() functions make every device >>> - * hotpluggable. Devices that shouldn't be hotpluggable, >>> - * should override it in their class_init() >>> + /* >>> + * All devices are considered as cold-pluggable by default. The >>> devices >>> + * that are hotpluggable should override it in their class_init(). >>> */ >>> - dc->hotpluggable = true; >>> + dc->hotpluggable = false; >> >> >> I agree with the defensive mode as long as we don't >> introduce any regression... > > Is it possible to hack together some kind of test code that > can give us a list of the devices compiled in that have > hotpluggable enabled? Then we could compare before and > after to see which devices we've changed. > Eduardo came up with some cool automated tests not long ago. Eduardo, can your tests help? Thanks, Marcel > thanks > -- PMM >