Re: [Qemu-devel] [PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

2011-06-06 Thread Gerd Hoffmann 

  Hi,


Let me know if you'd rather have me resend 12 + 14 to the list.


I'd suggest to look at the leftover stuff after the next usb merge.

cheers,
  Gerd

PS: /me is busy doing tests, preparing the next usb pull which hopefully 
goes out later today.

Re: [Qemu-devel] [PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

2011-06-01 Thread Gerd Hoffmann 

  Hi,


I'll wait a bit for more feedback and then change
[PATCH 14/14] usb: Proper error propagation for usb_device_attach errors


I'll wait for new revisions of patches 12+14 then.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

2011-06-01 Thread Hans de Goede 

Hi,

On 06/01/2011 02:50 PM, Gerd Hoffmann wrote:

Hi,


I'll wait a bit for more feedback and then change
[PATCH 14/14] usb: Proper error propagation for usb_device_attach errors


I'll wait for new revisions of patches 12+14 then.


I already fixed this yeterday, I opted to leave patch 12 unmodified and
replace all fprintf(stderr, ... calls with error_report calls in one go
in a new patch 14.

You can find the new version here:
http://cgit.freedesktop.org/~jwrdegoede/qemu/commit/?h=usb-patchesid=3c0be7730ff74a5cdac6aa100179b15c9392ab5f

Let me know if you'd rather have me resend 12 + 14 to the list.

Regards,

Hans

[Qemu-devel] [PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

2011-05-31 Thread Hans de Goede 
---
 hw/usb-bus.c |   23 ---
 hw/usb-msd.c |5 +++--
 usb-linux.c  |6 +-
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 0a49921..2ae2678 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -75,7 +75,7 @@ static int usb_qdev_init(DeviceState *qdev, DeviceInfo *base)
 QLIST_INIT(dev-strings);
 rc = dev-info-init(dev);
 if (rc == 0  dev-auto_attach)
-usb_device_attach(dev);
+rc = usb_device_attach(dev);
 return rc;
 }
 
@@ -171,20 +171,20 @@ void usb_unregister_port(USBBus *bus, USBPort *port)
 bus-nfree--;
 }
 
-static void do_attach(USBDevice *dev)
+static int do_attach(USBDevice *dev)
 {
 USBBus *bus = usb_bus_from_device(dev);
 USBPort *port;
 
 if (dev-attached) {
-fprintf(stderr, Warning: tried to attach usb device %s twice\n,
+fprintf(stderr, Error: tried to attach usb device %s twice\n,
 dev-product_desc);
-return;
+return -1;
 }
 if (bus-nfree == 0) {
-fprintf(stderr, Warning: tried to attach usb device %s to a bus with 
no free ports\n,
+fprintf(stderr, Error: tried to attach usb device %s to a bus with no 
free ports\n,
 dev-product_desc);
-return;
+return -1;
 }
 if (dev-port_path) {
 QTAILQ_FOREACH(port, bus-free, next) {
@@ -193,9 +193,9 @@ static void do_attach(USBDevice *dev)
 }
 }
 if (port == NULL) {
-fprintf(stderr, Warning: usb port %s (bus %s) not found\n,
+fprintf(stderr, Error: usb port %s (bus %s) not found\n,
 dev-port_path, bus-qbus.name);
-return;
+return -1;
 }
 } else {
 port = QTAILQ_FIRST(bus-free);
@@ -209,6 +209,8 @@ static void do_attach(USBDevice *dev)
 
 QTAILQ_INSERT_TAIL(bus-used, port, next);
 bus-nused++;
+
+return 0;
 }
 
 int usb_device_attach(USBDevice *dev)
@@ -220,8 +222,7 @@ int usb_device_attach(USBDevice *dev)
(unless a physical port location is specified). */
 usb_create_simple(bus, usb-hub);
 }
-do_attach(dev);
-return 0;
+return do_attach(dev);
 }
 
 int usb_device_detach(USBDevice *dev)
@@ -230,7 +231,7 @@ int usb_device_detach(USBDevice *dev)
 USBPort *port;
 
 if (!dev-attached) {
-fprintf(stderr, Warning: tried to detach unattached usb device %s\n,
+fprintf(stderr, Error: tried to detach unattached usb device %s\n,
 dev-product_desc);
 return -1;
 }
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index c8b7d41..68b46fa 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -481,8 +481,9 @@ static void usb_msd_password_cb(void *opaque, int err)
 MSDState *s = opaque;
 
 if (!err)
-usb_device_attach(s-dev);
-else
+err = usb_device_attach(s-dev);
+
+if (err)
 qdev_unplug(s-dev.qdev);
 }
 
diff --git a/usb-linux.c b/usb-linux.c
index 490c49f..fe21da1 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1152,10 +1152,14 @@ static int usb_host_open(USBHostDevice *dev, int 
bus_num,
 prod_name);
 }
 
+ret = usb_device_attach(dev-dev);
+if (ret) {
+goto fail;
+}
+
 /* USB devio uses 'write' flag to check for async completions */
 qemu_set_fd_handler(dev-fd, NULL, async_complete, dev);
 
-usb_device_attach(dev-dev);
 return 0;
 
 fail:
-- 
1.7.5.1

Re: [Qemu-devel] [PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

2011-05-31 Thread Michael Tokarev 
31.05.2011 13:35, Hans de Goede wrote:
 ---
  hw/usb-bus.c |   23 ---
  hw/usb-msd.c |5 +++--
  usb-linux.c  |6 +-
  3 files changed, 20 insertions(+), 14 deletions(-)
 
 diff --git a/hw/usb-bus.c b/hw/usb-bus.c
 index 0a49921..2ae2678 100644
 --- a/hw/usb-bus.c
 +++ b/hw/usb-bus.c

  if (dev-attached) {
 -fprintf(stderr, Warning: tried to attach usb device %s twice\n,
 +fprintf(stderr, Error: tried to attach usb device %s twice\n,
  dev-product_desc);

qemu_error() maybe, while we're at it?
Here and in a few other places.

Thanks!

/mjt

Re: [Qemu-devel] [PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

2011-05-31 Thread Hans de Goede 

Hi,

On 05/31/2011 11:42 AM, Michael Tokarev wrote:

31.05.2011 13:35, Hans de Goede wrote:

---
  hw/usb-bus.c |   23 ---
  hw/usb-msd.c |5 +++--
  usb-linux.c  |6 +-
  3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 0a49921..2ae2678 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c



  if (dev-attached) {
-fprintf(stderr, Warning: tried to attach usb device %s twice\n,
+fprintf(stderr, Error: tried to attach usb device %s twice\n,
  dev-product_desc);


qemu_error() maybe, while we're at it?
Here and in a few other places.


That does not seem to exist, do you perhaps mean error_printf() ?

Regards,

Hans

Re: [Qemu-devel] [PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

2011-05-31 Thread Kevin Wolf 
Am 31.05.2011 11:51, schrieb Hans de Goede:
 Hi,
 
 On 05/31/2011 11:42 AM, Michael Tokarev wrote:
 31.05.2011 13:35, Hans de Goede wrote:
 ---
   hw/usb-bus.c |   23 ---
   hw/usb-msd.c |5 +++--
   usb-linux.c  |6 +-
   3 files changed, 20 insertions(+), 14 deletions(-)

 diff --git a/hw/usb-bus.c b/hw/usb-bus.c
 index 0a49921..2ae2678 100644
 --- a/hw/usb-bus.c
 +++ b/hw/usb-bus.c

   if (dev-attached) {
 -fprintf(stderr, Warning: tried to attach usb device %s twice\n,
 +fprintf(stderr, Error: tried to attach usb device %s twice\n,
   dev-product_desc);

 qemu_error() maybe, while we're at it?
 Here and in a few other places.
 
 That does not seem to exist, do you perhaps mean error_printf() ?

error_report() is what you should use, so that messages go to the
monitor if the function is called from a monitor command. error_printf()
is used by it internally, but usually isn't used directly.

Kevin

Re: [Qemu-devel] [PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

2011-05-31 Thread Hans de Goede 

Hi,

On 05/31/2011 11:56 AM, Kevin Wolf wrote:

Am 31.05.2011 11:51, schrieb Hans de Goede:

Hi,

On 05/31/2011 11:42 AM, Michael Tokarev wrote:

31.05.2011 13:35, Hans de Goede wrote:

---
   hw/usb-bus.c |   23 ---
   hw/usb-msd.c |5 +++--
   usb-linux.c  |6 +-
   3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 0a49921..2ae2678 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c



   if (dev-attached) {
-fprintf(stderr, Warning: tried to attach usb device %s twice\n,
+fprintf(stderr, Error: tried to attach usb device %s twice\n,
   dev-product_desc);


qemu_error() maybe, while we're at it?
Here and in a few other places.


That does not seem to exist, do you perhaps mean error_printf() ?


error_report() is what you should use, so that messages go to the
monitor if the function is called from a monitor command. error_printf()
is used by it internally, but usually isn't used directly.



I've looked at error_report, but IMHO it is made of crazy, I'm not going
to construct a json dict every time I need to log some simple error message
(and the existing ones are not suitable for many error messages).

Regards,

Hans

Re: [Qemu-devel] [PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

2011-05-31 Thread Kevin Wolf 
Am 31.05.2011 12:05, schrieb Hans de Goede:
 Hi,
 
 On 05/31/2011 11:56 AM, Kevin Wolf wrote:
 Am 31.05.2011 11:51, schrieb Hans de Goede:
 Hi,

 On 05/31/2011 11:42 AM, Michael Tokarev wrote:
 31.05.2011 13:35, Hans de Goede wrote:
 ---
hw/usb-bus.c |   23 ---
hw/usb-msd.c |5 +++--
usb-linux.c  |6 +-
3 files changed, 20 insertions(+), 14 deletions(-)

 diff --git a/hw/usb-bus.c b/hw/usb-bus.c
 index 0a49921..2ae2678 100644
 --- a/hw/usb-bus.c
 +++ b/hw/usb-bus.c

if (dev-attached) {
 -fprintf(stderr, Warning: tried to attach usb device %s twice\n,
 +fprintf(stderr, Error: tried to attach usb device %s twice\n,
dev-product_desc);

 qemu_error() maybe, while we're at it?
 Here and in a few other places.

 That does not seem to exist, do you perhaps mean error_printf() ?

 error_report() is what you should use, so that messages go to the
 monitor if the function is called from a monitor command. error_printf()
 is used by it internally, but usually isn't used directly.

 
 I've looked at error_report, but IMHO it is made of crazy, I'm not going
 to construct a json dict every time I need to log some simple error message
 (and the existing ones are not suitable for many error messages).

error_report() works with plain strings. Maybe you confuse it with the
QMP error reporting function?

Kevin

Re: [Qemu-devel] [PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

2011-05-31 Thread Hans de Goede 

Hi,

On 05/31/2011 12:12 PM, Kevin Wolf wrote:

Am 31.05.2011 12:05, schrieb Hans de Goede:

Hi,

On 05/31/2011 11:56 AM, Kevin Wolf wrote:

Am 31.05.2011 11:51, schrieb Hans de Goede:

Hi,

On 05/31/2011 11:42 AM, Michael Tokarev wrote:

31.05.2011 13:35, Hans de Goede wrote:

---
hw/usb-bus.c |   23 ---
hw/usb-msd.c |5 +++--
usb-linux.c  |6 +-
3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 0a49921..2ae2678 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c



if (dev-attached) {
-fprintf(stderr, Warning: tried to attach usb device %s twice\n,
+fprintf(stderr, Error: tried to attach usb device %s twice\n,
dev-product_desc);


qemu_error() maybe, while we're at it?
Here and in a few other places.


That does not seem to exist, do you perhaps mean error_printf() ?


error_report() is what you should use, so that messages go to the
monitor if the function is called from a monitor command. error_printf()
is used by it internally, but usually isn't used directly.



I've looked at error_report, but IMHO it is made of crazy, I'm not going
to construct a json dict every time I need to log some simple error message
(and the existing ones are not suitable for many error messages).


error_report() works with plain strings. Maybe you confuse it with the
QMP error reporting function?


Ah yes I was looking at qerror_report (who ever named that, having just
one letter difference in the function names is a bad idea). error_report
looks fine.

I'll wait a bit for more feedback and then change
[PATCH 14/14] usb: Proper error propagation for usb_device_attach errors

To turn the fprintf(stderr, ... calls into error_report calls.

Thanks  Regards,

Hans





Kevin