From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:38447)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1SqfY0-0001xv-AF
	for qemu-devel@nongnu.org; Mon, 16 Jul 2012 03:16:52 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1SqfXz-0005Dn-AD
	for qemu-devel@nongnu.org; Mon, 16 Jul 2012 03:16:48 -0400
Received: from mail-yw0-f45.google.com ([209.85.213.45]:46439)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1SqfXz-0005Dh-56
	for qemu-devel@nongnu.org; Mon, 16 Jul 2012 03:16:47 -0400
Received: by yhpp34 with SMTP id p34so3177000yhp.4
	for <qemu-devel@nongnu.org>; Mon, 16 Jul 2012 00:16:46 -0700 (PDT)
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
Message-ID: <5003BFD7.2010703@redhat.com>
Date: Mon, 16 Jul 2012 09:16:39 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1342383911-6094-1-git-send-email-minyard@acm.org>
	<1342383911-6094-12-git-send-email-minyard@acm.org>
In-Reply-To: <1342383911-6094-12-git-send-email-minyard@acm.org>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 12/16] IPMI: Add a PC ISA type structure
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: minyard@acm.org
Cc: Corey Minyard <cminyard@mvista.com>, qemu-devel@nongnu.org

Il 15/07/2012 22:25, minyard@acm.org ha scritto:
> +    /* Clear the property from this device so we can put it elsewhere */
> +    chr = isa->chr;
> +    qdev_prop_set_chr(&dev->qdev, "chardev", NULL);
> +
> +    if (chr) {
> +	bdev = qdev_create(NULL, "ipmi-bmc-extern");
> +	qdev_prop_set_chr(bdev, "chardev", chr);
> +    } else {
> +	bdev = qdev_create(NULL, "ipmi-bmc-sim");
> +    }
> +    snprintf(typename, sizeof(typename), "ipmi-interface-%s", isa->interface);
> +    idev = qdev_create(&isa_bus->qbus, typename);
> +    qdev_prop_set_ptr(bdev, "ipmiif", idev);
> +    qdev_prop_set_uint64(idev, "iobase", isa->iobase);
> +    qdev_prop_set_ptr(idev, "bmc", bdev);
> +    qdev_prop_set_uint8(idev, "slave-addr", isa->slave_addr);
> +    rc = qdev_init(idev);

I think this is adding a bit of extra complication.  You do not need to
add explicit devices for this.

If you want to make ipmi-bmc-* and ipmi-interface-* objects, you can
create them with object_new, set the properties manually with the
returned pointer and add the object to the parent with
object_property_add_child.

Otherwise, it should also be fine if you just add them as function pointers.

Either way also lets you keep the ownership of the chardev in the ISA
IPMI device, and avoids that "info qtree" looks strange and does not
show the chardev you assigned.

Paolo