From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcel Apfelbaum Subject: Re: [Qemu-devel] [PATCH] machine: Add kvm-type property Date: Mon, 02 Jun 2014 11:56:59 +0300 Message-ID: <1401699419.2875.117.camel@localhost.localdomain> References: <1401482467-31550-1-git-send-email-ehabkost@redhat.com> <1401611464.2875.99.camel@localhost.localdomain> Reply-To: marcel.a@redhat.com Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, Alexander Graf , qemu-devel@nongnu.org, "Aneesh Kumar K.V" , Paolo Bonzini , Andreas =?ISO-8859-1?Q?F=E4rber?= To: Eduardo Habkost Return-path: Received: from mail-we0-f181.google.com ([74.125.82.181]:59220 "EHLO mail-we0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752815AbaFBI5E (ORCPT ); Mon, 2 Jun 2014 04:57:04 -0400 Received: by mail-we0-f181.google.com with SMTP id w61so4834093wes.12 for ; Mon, 02 Jun 2014 01:57:02 -0700 (PDT) In-Reply-To: <1401611464.2875.99.camel@localhost.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: On Sun, 2014-06-01 at 11:31 +0300, Marcel Apfelbaum wrote: > On Fri, 2014-05-30 at 17:41 -0300, Eduardo Habkost wrote: > > The kvm-type machine option was left out when MachineState was > > introduced, preventing the kvm-type option from being used. Add the > > missing property. > Very interesting how did I miss that. > Thanks! > Marcel >=20 > >=20 > > Signed-off-by: Eduardo Habkost > > Cc: Andreas F=C3=A4rber > > Cc: Aneesh Kumar K.V > > Cc: Alexander Graf > > Cc: Marcel Apfelbaum > > --- > > Tested in a x86 machine only. Help would be welcome to test it on a= PPC > > machine using -machine spapr and KVM. > >=20 > > Before this patch: > >=20 > > $ qemu-system-x86_64 -machine pc,kvm-type=3Dhv,accel=3Dkvm > > qemu-system-x86_64: Property '.kvm-type' not found > >=20 > > (This means the option won't work even for sPAPR machines.) > >=20 > > After applying this patch: > >=20 > > $ qemu-system-x86_64 -machine pc,kvm-type=3Dhv,accel=3Dkvm > > Invalid argument kvm-type=3Dhv > >=20 > > (This means the x86 KVM init code is seeing (and rejecting) the opt= ion, > > and the sPAPR code can use it.) > >=20 > > Note that qemu-system-x86_64 will segfault with the above command-l= ine > > unless an additional fix (submitted today) is applied (kvm: Ensure > > negative return value on kvm_init() error handling path). > > --- > > hw/core/machine.c | 17 +++++++++++++++++ > > include/hw/boards.h | 1 + > > 2 files changed, 18 insertions(+) > >=20 > > diff --git a/hw/core/machine.c b/hw/core/machine.c > > index cbba679..ed47b3a 100644 > > --- a/hw/core/machine.c > > +++ b/hw/core/machine.c > > @@ -235,6 +235,21 @@ static void machine_set_firmware(Object *obj, = const char *value, Error **errp) > > ms->firmware =3D g_strdup(value); > > } > > =20 > > +static char *machine_get_kvm_type(Object *obj, Error **errp) > > +{ > > + MachineState *ms =3D MACHINE(obj); > > + > > + return g_strdup(ms->kvm_type); > > +} > > + > > +static void machine_set_kvm_type(Object *obj, const char *value, E= rror **errp) > > +{ > > + MachineState *ms =3D MACHINE(obj); > > + > > + g_free(ms->kvm_type); Hi, Here also, like in my other replies, I think it should be the caller responsibility to free the string. A set method should not touch its parameter. Thanks, Marcel =20 > > + ms->kvm_type =3D g_strdup(value); > > +} > > + > > static void machine_initfn(Object *obj) > > { > > object_property_add_str(obj, "accel", > > @@ -274,6 +289,8 @@ static void machine_initfn(Object *obj) > > object_property_add_bool(obj, "usb", machine_get_usb, machine_= set_usb, NULL); > > object_property_add_str(obj, "firmware", > > machine_get_firmware, machine_set_firm= ware, NULL); > > + object_property_add_str(obj, "kvm-type", > > + machine_get_kvm_type, machine_set_kvm_= type, NULL); > > } > > =20 > > static void machine_finalize(Object *obj) > > diff --git a/include/hw/boards.h b/include/hw/boards.h > > index 2d2e2be..44956d6 100644 > > --- a/include/hw/boards.h > > +++ b/include/hw/boards.h > > @@ -111,6 +111,7 @@ struct MachineState { > > bool mem_merge; > > bool usb; > > char *firmware; > > + char *kvm_type; > > =20 > > ram_addr_t ram_size; > > const char *boot_order; >=20 >=20 >=20 >=20