From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:40802)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1W8unG-0005oQ-HH
	for qemu-devel@nongnu.org; Thu, 30 Jan 2014 11:48:52 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1W8unA-0003ar-IY
	for qemu-devel@nongnu.org; Thu, 30 Jan 2014 11:48:46 -0500
Received: from mx1.redhat.com ([209.132.183.28]:52291)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1W8unA-0003aY-B4
	for qemu-devel@nongnu.org; Thu, 30 Jan 2014 11:48:40 -0500
Message-ID: <52EA825D.5030900@redhat.com>
Date: Thu, 30 Jan 2014 17:48:29 +0100
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1391093245-14277-1-git-send-email-marcel.a@redhat.com>
	<1391093245-14277-5-git-send-email-marcel.a@redhat.com>
In-Reply-To: <1391093245-14277-5-git-send-email-marcel.a@redhat.com>
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH RFC 4/5] hw/machine: add qemu machine opts
 as properties to QemuMachineState
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Marcel Apfelbaum <marcel.a@redhat.com>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, mst@redhat.com, armbru@redhat.com, lcapitulino@redhat.com, blauwirbel@gmail.com, aliguori@amazon.com, afaerber@suse.de

Il 30/01/2014 15:47, Marcel Apfelbaum ha scritto:
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3cd48fe..51bcaba 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -86,6 +86,21 @@ struct QemuMachineState {
>      Object parent;
>      /* public */
>
> +    char *accel;
> +    bool kernel_irqchip;
> +    int kvm_shadow_mem;
> +    char *kernel;
> +    char *initrd;
> +    char *append;

Many of these are in init_args as well.

Perhaps you can include the init_args by value instead of having a 
pointer, and make the setters store into the init_args.  It should be 
fairly easy to use &current_machine->init_args in vl.c instead of the 
current

     QEMUMachineInitArgs args = { .machine = machine,
                                  .ram_size = ram_size,
                                  .boot_order = boot_order,
                                  .kernel_filename = kernel_filename,
                                  .kernel_cmdline = kernel_cmdline,
                                  .initrd_filename = initrd_filename,
                                  .cpu_model = cpu_model };
     machine->init(&args);

Otherwise the series is nice!

Do you think it makes sense to prepend something like "machine::" or 
"machine-" to the class name?

Paolo

> +    char *dtb;
> +    char *dumpdtb;
> +    int phandle_start;
> +    char *dt_compatible;
> +    bool dump_guest_core;
> +    bool mem_merge;
> +    bool usb;
> +    char *firmware;
> +
>      QEMUMachineInitArgs *init_args;