From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48138) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UhGTc-0007D0-4P for qemu-devel@nongnu.org; Tue, 28 May 2013 05:46:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UhGTV-0003H8-MF for qemu-devel@nongnu.org; Tue, 28 May 2013 05:45:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:29065) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UhGTU-0003GW-Rz for qemu-devel@nongnu.org; Tue, 28 May 2013 05:45:49 -0400 Message-ID: <51A47CBC.1090906@redhat.com> Date: Tue, 28 May 2013 11:45:32 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1369726806-9715-1-git-send-email-akong@redhat.com> <51A4659B.8060601@redhat.com> <20130528093501.GB2825@t430s.nay.redhat.com> In-Reply-To: <20130528093501.GB2825@t430s.nay.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] boot: fix path pattern of scsi device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong Cc: sluo@redhat.com, kevin@koconnor.net, seabios@seabios.org, qemu-devel@nongnu.org, =?ISO-8859-1?Q?KONRAD_Fr=E9d=E9ric?= Il 28/05/2013 11:35, Amos Kong ha scritto: > On Tue, May 28, 2013 at 10:06:51AM +0200, Paolo Bonzini wrote: >> Il 28/05/2013 09:40, Amos Kong ha scritto: >>> bootindex parameter of scsi device doesn't work, it causes >>> by wrong pattern in seabios. >>> >>> qemu passes the following firmware dev_path to seabios: >>> /pci@i0cf8/scsi@4/virtio-scsi-device/channel@0/disk@0,0 >> >> No, this is another unexpected change due to the virtio refactoring in >> QEMU. The right fix is in QEMU, by adding a get_fw_dev_path >> implementation in hw/virtio/virtio-bus.c. > > Hi Paolo, > > We could not fix this by implementing get_fw_dev_path in > hw/virtio/virtio-bus.c > > virtio-bus is the parent bus of scsi-bus, scsibus_get_fw_dev_path() > will be called after called virtio_bus_get_fw_dev_path(). > > str0 = returns of parents of virtio-bus /* eg: /pci@i0cf8/scsi@4 */ > str1 = return of virtio_bus_get_fw_dev_path() /* eg: .. */ > str2 = scsibus_get_fw_dev_path() /* eg: channel@0/disk@0,0 */ > > The final fw_dev_path should be $str0/$str1/$str2 > > | static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size) > | { > | int l = 0; > | > | if (dev && dev->parent_bus) { > | char *d; > | l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size); > | d = bus_get_fw_dev_path(dev->parent_bus, dev); > | if (d) { > | l += snprintf(p + l, size - l, "%s", d); > | g_free(d); > > if we implement virtio_bus_get_fw_dev_path(), the return value will be set to $str1 > > | } else { > | l += snprintf(p + l, size - l, "%s", object_get_typename(OBJECT(dev))); > > Currently we didn't implement virtio_bus_get_fw_dev_path(), so 'virtio-scsi-device' was set to $str1 > > | } > | > > However, we need to change the pattern. Turn this: l += snprintf(p + l, size - l, "%s", object_get_typename(OBJECT(dev))); into the default implementation of get_fw_dev_path (in TYPE_BUS), and change qdev_get_fw_dev_path_helper to if (d) { l += snprintf(p + l, size - l, "%s", d); g_free(d); } else { return l; } Then virtio_bus_get_fw_dev_path can just return NULL. Paolo > Amos. > >> We fixed it already for migration paths, it should be easy to do the >> same for this. >> >> Please Cc qemu-stable@nongnu.org when sending the QEMU patch. >> >> Thanks, >> >> Paolo >> >>> Signed-off-by: Amos Kong >>> --- >>> src/boot.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/src/boot.c b/src/boot.c >>> index c308602..cd9d784 100644 >>> --- a/src/boot.c >>> +++ b/src/boot.c >>> @@ -138,10 +138,11 @@ int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun) >>> if (!pci) >>> // support only pci machine for now >>> return -1; >>> - // Find scsi drive - for example: /pci@i0cf8/scsi@5/channel@0/disk@1,0 >>> + /* Find scsi drive - for example: >>> + /pci@i0cf8/scsi@5/virtio-scsi-device/channel@0/disk@1,0 */ >>> char desc[256], *p; >>> p = build_pci_path(desc, sizeof(desc), "*", pci); >>> - snprintf(p, desc+sizeof(desc)-p, "/*@0/*@%d,%d", target, lun); >>> + snprintf(p, desc+sizeof(desc)-p, "/*/*@0/*@%d,%d", target, lun); >>> return find_prio(desc); >>> } >>> >>>