From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52716) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b0VdF-0005Ku-IY for qemu-devel@nongnu.org; Wed, 11 May 2016 11:01:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b0Vd9-0008Nr-Az for qemu-devel@nongnu.org; Wed, 11 May 2016 11:01:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50767) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b0Vd9-0008NU-69 for qemu-devel@nongnu.org; Wed, 11 May 2016 11:00:55 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CC17415567 for ; Wed, 11 May 2016 15:00:53 +0000 (UTC) Date: Wed, 11 May 2016 12:00:51 -0300 From: Eduardo Habkost Message-ID: <20160511150051.GG4457@thinpad.lan.raisama.net> References: <1462192431-146342-1-git-send-email-imammedo@redhat.com> <1462192431-146342-21-git-send-email-imammedo@redhat.com> <20160510202718.GE4457@thinpad.lan.raisama.net> <20160511160019.6987eeb8@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160511160019.6987eeb8@nial.brq.redhat.com> Subject: Re: [Qemu-devel] [RFC 20/42] machine: add cpu-hotplug machine option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org, mst@redhat.com, marcel@redhat.com, eblake@redhat.com, armbru@redhat.com, drjones@redhat.com On Wed, May 11, 2016 at 04:00:19PM +0200, Igor Mammedov wrote: > On Tue, 10 May 2016 17:27:18 -0300 > Eduardo Habkost wrote: > > > On Mon, May 02, 2016 at 02:33:29PM +0200, Igor Mammedov wrote: > > > Signed-off-by: Igor Mammedov > > > > If a machine class simply doesn't support CPU hotplug at all, is > > silently ignoring "cpu-hotplug=on" the right thing to do? > > > > Shouldn't we exit with an error if the machine class doesn't > > support CPU hotplug and the user tries to enable it? > We have a bunch of such options in generic MachineClass > and it was upto concrete machine to implement such checks. Yes, and my proposal is to make this more robust from now on, and make machines stop silently ignoring unsupported options. > > So I hadn't even considered to make such check in generic code > nor think that's a right thing to do, if that's what you are asking. If only 1 or 2 machines support CPU hotplug, I don't think it would be reasonable to have the option available at every machine class and require most classes to add an extra check like: if (machine->cpu_hotplug) error_setg(errp, "CPU hotplug not supported"); A check like the following, in common code: if (!machine_class->cpu_hotplug_supported && machine->cpu_hotplug) error_setg(errp, "CPU hotplug not supported") would avoid duplicating code in every machine class, and only require the following extra line in the machine classes that really support CPU hotplug: machine_class->cpu_hotplug_supported = true; Or, even better: we can avoid registering the cpu-hotplug property at all if the subclass doesn't support CPU hotplug. You can do that at machine_class_base_init, e.g.: static void machine_class_base_init(ObjectClass *oc, void *data) { /* [...] */ MachineClass *mc = MACHINE_CLASS(oc); if (mc->cpu_hotplug_supported) { object_class_property_add_str(oc, "cpu-hotplug", machine_get_cpu_hotplug, machine_set_cpu_hotplug, NULL); object_class_property_set_description(oc, "cpu-hotplug", "Set on to enable CPU hotplug", NULL); } } -- Eduardo