From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gleb Natapov <gleb@redhat.com>
Subject: Re: [PATCHv3 10/14] Add get_dev_path callback for usb bus.
Date: Wed, 10 Nov 2010 21:11:07 +0200
Message-ID: <20101110191107.GE6798@redhat.com>
References: <1289409261-5418-1-git-send-email-gleb@redhat.com>
 <1289409261-5418-11-git-send-email-gleb@redhat.com>
 <AANLkTi=iDcW3F2yOp3besuw2ay8kr6AncNuNKrK3MCft@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=koi8-r
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, armbru@redhat.com,
	alex.williamson@redhat.com, mst@redhat.com
To: Blue Swirl <blauwirbel@gmail.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:42215 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1755613Ab0KJTLM convert rfc822-to-8bit (ORCPT
	<rfc822;kvm@vger.kernel.org>); Wed, 10 Nov 2010 14:11:12 -0500
Content-Disposition: inline
In-Reply-To: <AANLkTi=iDcW3F2yOp3besuw2ay8kr6AncNuNKrK3MCft@mail.gmail.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, Nov 10, 2010 at 06:37:45PM +0000, Blue Swirl wrote:
> On Wed, Nov 10, 2010 at 5:14 PM, Gleb Natapov <gleb@redhat.com> wrote=
:
> >
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > ---
> > =9Ahw/usb-bus.c | =9A 35 +++++++++++++++++++++++++++++++++++
> > =9A1 files changed, 35 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/usb-bus.c b/hw/usb-bus.c
> > index 256b881..6292282 100644
> > --- a/hw/usb-bus.c
> > +++ b/hw/usb-bus.c
> > @@ -5,11 +5,13 @@
> > =9A#include "monitor.h"
> >
> > =9Astatic void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, i=
nt indent);
> > +static char *usbbus_get_fw_dev_path(DeviceState *dev);
> >
> > =9Astatic struct BusInfo usb_bus_info =3D {
> > =9A =9A .name =9A =9A =9A=3D "USB",
> > =9A =9A .size =9A =9A =9A=3D sizeof(USBBus),
> > =9A =9A .print_dev =3D usb_bus_dev_print,
> > + =9A =9A.get_fw_dev_path =3D usbbus_get_fw_dev_path,
> > =9A};
> > =9Astatic int next_usb_bus =3D 0;
> > =9Astatic QTAILQ_HEAD(, USBBus) busses =3D QTAILQ_HEAD_INITIALIZER(=
busses);
> > @@ -307,3 +309,36 @@ USBDevice *usbdevice_create(const char *cmdlin=
e)
> > =9A =9A }
> > =9A =9A return usb->usbdevice_init(params);
> > =9A}
> > +
> > +static int usbbus_get_fw_dev_path_helper(USBDevice *d, USBBus *bus=
, char *p,
> > + =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A=
 =9A =9A =9A int len)
> > +{
> > + =9A =9Aint l =3D 0;
> > + =9A =9AUSBPort *port;
> > +
> > + =9A =9AQTAILQ_FOREACH(port, &bus->used, next) {
> > + =9A =9A =9A =9Aif (port->dev !=3D d)
>=20
> Braces.
>=20
> > + =9A =9A =9A =9A =9A =9Acontinue;
> > +
> > + =9A =9A =9A =9Aif (port->pdev) {
> > + =9A =9A =9A =9A =9A =9Al =3D usbbus_get_fw_dev_path_helper(port->=
pdev, bus, p, len);
> > + =9A =9A =9A =9A}
> > + =9A =9A =9A =9Al +=3D snprintf(p + l, len - l, "%s@%x/", qdev_fw_=
name(&d->qdev),
> > + =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9Aport->index);
> > + =9A =9A =9A =9Areturn l;
> > + =9A =9A}
> > + =9A =9Areturn 0;
>=20
> With this return value...
>=20
> > +}
This can only happen if device is not attached to parent bus, so we
should never get here I think. I do not know USB bus enough to be sure.
May be we should abort here.

> > +
> > +static char *usbbus_get_fw_dev_path(DeviceState *dev)
> > +{
> > + =9A =9AUSBDevice *d =3D (USBDevice*)dev;
> > + =9A =9AUSBBus *bus =3D usb_bus_from_device(d);
> > + =9A =9Achar path[100];
> > + =9A =9Aint l;
> > +
> > + =9A =9Al =3D usbbus_get_fw_dev_path_helper(d, bus, path, sizeof(p=
ath));
> > + =9A =9Apath[l-1] =3D '\0';
>=20
> ...this statement will have undesirable effects...
>=20
> > +
> > + =9A =9Areturn strdup(path);
>=20
> ...like this.

--
			Gleb.