From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:43330) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RbYC4-0007zh-Qg for qemu-devel@nongnu.org; Fri, 16 Dec 2011 08:51:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RbYBy-0001EQ-NW for qemu-devel@nongnu.org; Fri, 16 Dec 2011 08:51:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:10500) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RbYBy-0001EI-H0 for qemu-devel@nongnu.org; Fri, 16 Dec 2011 08:51:18 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id pBGDpHcH000474 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 16 Dec 2011 08:51:17 -0500 Message-ID: <4EEB4CD2.8080907@redhat.com> Date: Fri, 16 Dec 2011 14:51:14 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1324036918-2405-1-git-send-email-pbonzini@redhat.com> <1324036918-2405-7-git-send-email-pbonzini@redhat.com> <4EEB439F.7010601@redhat.com> In-Reply-To: <4EEB439F.7010601@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 6/8] qom: introduce get/set methods for Property List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: kwolf@redhat.com, qemu-devel@nongnu.org On 12/16/2011 02:11 PM, Gerd Hoffmann wrote: > I think we should not touch these. First it doesn't buy us much as we > are using the old parse/print functions for the visitor-based access, > which doesn't look like a good idea to me. Also they will not stay but > will be converted to child<> and link<>, so I think it is better to keep > the old stuff in the legacy<> namespace. I thought the same initially. However, I noticed that the visitor interfaces for links is also a string. So, even if a block/char/netdev property later becomes a link<>, the interface would not change. Using the old parse/print functions and get_set/generic is only to avoid code duplication, I can surely inline everything but it would be uglier. And again, I found an example in the code of using a similar adapter pattern (the string properties). There is one case where I had doubts, namely the PCI address properties. They will be replaced by links that you set in the parent. However, in the end I decided to start this way because: 1) QOM properties can still come and go at this stage; 2) The PCI address property can still stay forever as a synthetic property declared by PCIDevice, so the "qom-get" ABI won't change. The "qom-set" ABI will, so it might be better to do: PropertyInfo qdev_prop_pci_devfn = { .name = "pci-devfn", .type = PROP_TYPE_UINT32, .size = sizeof(uint32_t), .parse = parse_pci_devfn, .print = print_pci_devfn, + .get = get_pci_devfn, + .set = NULL, }; Advantages: it shows that setting the PCI address is (going to be) a legacy feature; Disadvantages: looks a little ad-hoc. See below for an alternative. > Agree on the bit/bool/int types. Although we maybe should apply some > care to integer bus properties, some of them are used for addressing and > will most likely replaced by child<> and link<> too. Yes, these will also become synthetic and read-only. So an alternative could be: for (prop = dev->info->props; prop && prop->name; prop++) { qdev_property_add_legacy(dev, prop, NULL); /* Let the generic initializer register alternative definitions * for qdev properties. */ if (!qdev_property_find(dev, prop->name) { qdev_property_add_static(dev, prop, NULL); } } for (prop = dev->info->bus_info->props; prop && prop->name; prop++) { qdev_property_add_legacy(dev, prop, NULL); if (!qdev_property_find(dev, prop->name) { qdev_property_add_static(dev, prop, NULL); } } For now the pci_devfn property remains read-write, but as soon as the PCIDevice will be able to define it as synthetic, it will become read-only. Paolo