From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56213) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UhZrh-0000oC-C8 for qemu-devel@nongnu.org; Wed, 29 May 2013 02:28:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UhZrc-00013P-Mn for qemu-devel@nongnu.org; Wed, 29 May 2013 02:28:05 -0400 Sender: Paolo Bonzini Message-ID: <51A59FE4.30308@redhat.com> Date: Wed, 29 May 2013 08:27:48 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1369738859-16557-1-git-send-email-akong@redhat.com> <51A491ED.2080504@redhat.com> <20130529044137.GB2432@t430s.nay.redhat.com> In-Reply-To: <20130529044137.GB2432@t430s.nay.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] qdev: don't add typename to fw_dev_path when get_fw_dev_path isn't implemented List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong Cc: aliguori@us.ibm.com, kevin@koconnor.net, seabios@seabios.org, qemu-devel@nongnu.org, qemu-stable@nongnu.org Il 29/05/2013 06:41, Amos Kong ha scritto: > On Tue, May 28, 2013 at 01:15:57PM +0200, Paolo Bonzini wrote: >> Il 28/05/2013 13:00, Amos Kong ha scritto: >>> Recent virtio refactoring in QEMU made virtio-bus become the parent bus >>> of scsi-bus, and virtio-bus doesn't have get_fw_dev_path implementation, >>> so redundant typename will be added to fw_dev_path. It causes that >>> bootindex parameter of scsi device doesn't work. >>> >>> This patch changes qdev_get_fw_dev_path_helper() to add nothing if >>> implemented get_fw_dev_path() returns NULL. >>> >>> Signed-off-by: Amos Kong >>> -- >>> v2: only add nothing to fw_dev_path when get_fw_dev_path() is >>> implemented and return NULL. then it will not effect other devices >>> don't have get_fw_dev_path() implementation. > > Hi Paolo, > >> To achieve the same result, you can simply ensure that all classes >> implement the method. > > Do you mean if some classes didn't have an implement, I need to add > a implement (just returns NULL) for all of them, then nothing will be > added to fw_dev_path for those classes? No, you can have an implementation in BusClass. See how device_class_init provides a default implementation for realize/unrealize, and do the same in bus_class_init. > Currently I'm sure 'typename' is redundant for virio-bus. 'typename' > might be needed for other class (have no get_fw_dev_path implementation) > > Applied patch v2, there are three conditions: > 1) implement get_fw_dev_path, return specific string, add to > fw_dev_path (same as original) > 2) don't implement get_fw_dev_path, add typename to fw_dev_path. > (same as original) > 3) implement get_fw_dev_path and return NULL, nothing is added > to fw_dev_path (new behavior) > > If some other class wants to add nothing to path, just define a method > to return NULL. > >> The default implementation is just >> g_strdup(object_get_typename(OBJECT(dev))). > > If we implement the method for all classes, the default implementation > in qdev_get_fw_dev_path_helper() will not be called. Yes, but we shouldn't implement the method for all classes. We should implement it just once in BusClass. Then the default implementation in qdev_get_fw_dev_path_helper() can be removed. It will live in BusClass instead Paolo >> Remember that if you do not specify a method in a BusClass's class_init >> function, it is inherited from the superclass. >> >> Paolo >