On Thu, 12 Nov 2009 08:44:03 -0600 Anthony Liguori <aligu...@linux.vnet.ibm.com> wrote:
> Luiz Capitulino wrote: > > > >> #define QERR_DEVICE_ALREADY_OPEN "{'class': 'DeviceAlreadyOpen', 'data' > >> : {'bus_num': %d, 'addr': %d}" > >> > >> qemu_error_new(QERR_DEVICE_ALREADY_OPEN, bus_num, addr); > >> > > > > What about DeviceAlreadyOpen errors with a different argument list? > > > > Why would you have this? That would seem like a problem to me. I think > the errors need to be very well structured (just like everything else in > QMP). This can happen with errors that carry specific info which are different among subsystems, eg. USB device info vs. PCI device info. We could have 'USBDeviceAlreadyOpen', but then I think the class attribute will lose generality. > >> That gives us a nice simple interface with full error checking on the > >> parameters. > >> > > > > I've said this is not so simple because people writing those macros > > would find out that the 'class' or 'data' _keys_ are missing or incorrect > > only at run-time, when the error is triggered. > > > > Sure but introducing new types of errors is not the common case. Using > existing errors is the common case. Right, although there's a long road until we stabilize. > >> For human readable strings, I'd suggest making a table somewhere else > >> that looked like: > >> > >> QErrorStringTable qerror_descriptions[] = { > >> { QERR_DEVICE_ALREADY_OPEN, "This device at %(bus_num)d.%(addr)d is > >> already open." }, > >> ... > >> }; > >> > > > > How do you suggest we lookup the table? Doing a strcmp() on > > QERR_DEVICE_ALREADY_OPEN? > > > > We can either change the index on the table to be just the class code or > find something more clever. I'm working on this and trying to find something. > >> There are a number of advantages to an approach like this. The table > >> can be reused by both in the server and by a client. > >> > > > > My suggestions on both problems makes me willing go back to my initial > > series, which had a table indexed by an error number. > > > > I don't understand why. I've found other problems with it, let's pretend I didn't mention it. :)