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
>
> >
> > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> > Cc: Andreas Färber <afaer...@suse.de>
> > Cc: Aneesh Kumar K.V <aneesh.ku...@linux.vnet.ibm.com>
> > Cc: Alexander Graf <ag...@suse.de>
> > Cc: Marcel Apfelbaum <marce...@redhat.com>
> > ---
> > Tested in a x86 machine only. Help would be welcome to test it on a PPC
> > machine using -machine spapr and KVM.
> >
> > Before this patch:
> >
> > $ qemu-system-x86_64 -machine pc,kvm-type=hv,accel=kvm
> > qemu-system-x86_64: Property '.kvm-type' not found
> >
> > (This means the option won't work even for sPAPR machines.)
> >
> > After applying this patch:
> >
> > $ qemu-system-x86_64 -machine pc,kvm-type=hv,accel=kvm
> > Invalid argument kvm-type=hv
> >
> > (This means the x86 KVM init code is seeing (and rejecting) the option,
> > and the sPAPR code can use it.)
> >
> > Note that qemu-system-x86_64 will segfault with the above command-line
> > 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(+)
> >
> > 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 = g_strdup(value);
> > }
> >
> > +static char *machine_get_kvm_type(Object *obj, Error **errp)
> > +{
> > + MachineState *ms = MACHINE(obj);
> > +
> > + return g_strdup(ms->kvm_type);
> > +}
> > +
> > +static void machine_set_kvm_type(Object *obj, const char *value, Error
> > **errp)
> > +{
> > + MachineState *ms = 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
> > + ms->kvm_type = 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_firmware,
> > NULL);
> > + object_property_add_str(obj, "kvm-type",
> > + machine_get_kvm_type, machine_set_kvm_type,
> > NULL);
> > }
> >
> > 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;
> >
> > ram_addr_t ram_size;
> > const char *boot_order;
>
>
>
>
