qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: Marcel Apfelbaum <marcel.a@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>
Cc: vasilis.liaskovitis@profitbricks.com, pbonzini@redhat.com,
	qemu-devel@nongnu.org, aliguori@amazon.com, mst@redhat.com
Subject: Re: [Qemu-devel] [RFC 3/8] pc: prepare PC for custom machine state
Date: Mon, 24 Mar 2014 11:52:45 +0100	[thread overview]
Message-ID: <53300E7D.8080501@suse.de> (raw)
In-Reply-To: <1395587617.5673.55.camel@localhost.localdomain>

Am 23.03.2014 16:13, schrieb Marcel Apfelbaum:
> On Thu, 2014-03-20 at 16:01 +0100, Igor Mammedov wrote:
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>>  hw/i386/pc.c         |   26 ++++++++++++++++++++++++++
>>  hw/i386/pc_piix.c    |   34 +++++++++++++++++-----------------
>>  hw/i386/pc_q35.c     |   10 +++++-----
>>  include/hw/i386/pc.h |   14 ++++++++++++++
>>  4 files changed, 62 insertions(+), 22 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index e715a33..e0bc3a2 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1413,3 +1413,29 @@ void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
>>          gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
>>      }
>>  }
>> +
>> +void qemu_register_pc_machine(QEMUMachine *m)
> I am not so comfortable with this approach because
> every subsystem (e.g pc) will have to duplicate the
> "register machine" code until the conversion from
> QEMUMachine to MachineClass is over. (which I hope
> it will not take too much time)
> 
> I propose a patch already in the list which does that
> automatically by moving this logic into hw/core/machine.c .
> In this way it will be much less code "touched" during conversion. 
> 
> Andreas, did you have anything against the usage of 'class_base_init' ?

Yes, I do, .class_base_init is wrong for this, it would be .class_init;
but please avoid making a base class' .class_init (or .class_base_init)
depend on a subtype specifying .class_data.

I believe I asked you to post patches that finish your conversion so
that MachineClass no longer needs this pointer to QEMUMachine and
actually uses the fields you already prepared. In my mind the next
logical step of QOM'ification is to have each machine specify what is
now in a QEMUMachine struct in its own type's class_init, then there is
no duplication of such a general assignment any more.

Since this patch is for one machine only, I would much prefer to have
the PC duplicate the class_init like we did for sPAPR machine
(hw/ppc/spapr.c) over exposing the class_init across files - if this
series cannot wait to be ordered after the machine series.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

  parent reply	other threads:[~2014-03-24 10:53 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-20 15:01 [Qemu-devel] [RFC 0/8] bus-less device hotplug Igor Mammedov
2014-03-20 15:01 ` [Qemu-devel] [RFC 1/8] vl.c: extend -m option to support options for memory hotplug Igor Mammedov
2014-03-20 15:01 ` [Qemu-devel] [RFC 2/8] make machine_class_init() accessible outside of vl.c Igor Mammedov
2014-03-23 15:27   ` Marcel Apfelbaum
2014-03-23 16:22     ` Marcel Apfelbaum
2014-03-20 15:01 ` [Qemu-devel] [RFC 3/8] pc: prepare PC for custom machine state Igor Mammedov
2014-03-23 15:13   ` Marcel Apfelbaum
2014-03-24 10:34     ` Igor Mammedov
2014-03-24 12:28       ` Marcel Apfelbaum
2014-03-24 10:52     ` Andreas Färber [this message]
2014-03-24 11:20       ` Michael S. Tsirkin
2014-03-24 11:57         ` Andreas Färber
2014-03-24 12:13       ` Marcel Apfelbaum
2014-03-20 15:01 ` [Qemu-devel] [RFC 4/8] qdev: link based hotplug Igor Mammedov
2014-03-20 16:12   ` Paolo Bonzini
2014-03-20 16:20     ` Igor Mammedov
2014-03-20 18:03       ` Paolo Bonzini
2014-03-21 10:35         ` Igor Mammedov
2014-03-21 11:45           ` Paolo Bonzini
2014-03-24  9:10             ` Igor Mammedov
2014-03-24 12:20           ` Andreas Färber
2014-03-24 13:12             ` Michael S. Tsirkin
2014-03-20 15:01 ` [Qemu-devel] [RFC 5/8] dimm: implement dimm device abstraction Igor Mammedov
2014-03-20 15:01 ` [Qemu-devel] [RFC 6/8] pc: preallocate hotplug links for DIMMDevices Igor Mammedov
2014-03-20 15:01 ` [Qemu-devel] [RFC 7/8] pc: initialize memory hotplug address space Igor Mammedov
2014-03-20 15:01 ` [Qemu-devel] [RFC 8/8] pc: make PC_MACHINE memory hotplug controller Igor Mammedov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53300E7D.8080501@suse.de \
    --to=afaerber@suse.de \
    --cc=aliguori@amazon.com \
    --cc=imammedo@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=vasilis.liaskovitis@profitbricks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).