From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52650) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SqvT8-0000XF-S8 for qemu-devel@nongnu.org; Mon, 16 Jul 2012 20:16:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SqvT7-0008RB-Ni for qemu-devel@nongnu.org; Mon, 16 Jul 2012 20:16:50 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:46161) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SqvT7-0008R6-Ie for qemu-devel@nongnu.org; Mon, 16 Jul 2012 20:16:49 -0400 Received: by obbta14 with SMTP id ta14so10436922obb.4 for ; Mon, 16 Jul 2012 17:16:49 -0700 (PDT) Message-ID: <5004AEEE.9030308@acm.org> Date: Mon, 16 Jul 2012 19:16:46 -0500 From: Corey Minyard MIME-Version: 1.0 References: <1342383911-6094-1-git-send-email-minyard@acm.org> <1342383911-6094-12-git-send-email-minyard@acm.org> <5003BFD7.2010703@redhat.com> In-Reply-To: <5003BFD7.2010703@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 12/16] IPMI: Add a PC ISA type structure Reply-To: minyard@acm.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Corey Minyard , qemu-devel@nongnu.org On 07/16/2012 02:16 AM, Paolo Bonzini wrote: > 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. Yeah, but the qdevs did a lot of nice things for me :). But I understand. > > 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. The main reason I went with qdevs originally was because all the properties are defined for qdevs. I'd have to do all my own properties otherwise. However, I think all the properties are represented by what is already there in isa-ipmi, so just having pointers to the interface and bmc objects is ok, I think. > > 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. Yeah, I see that now. Makes sense. BTW, where do you want me to document this? I didn't see an obvious place in qemu-options.hx. I'm working on tests, too. I didn't find much there already, but I'll see what I can add. Thanks, -corey