On Mon, Oct 19, 2009 at 10:28:17AM -0200, Luiz Capitulino wrote: > On Mon, 19 Oct 2009 11:25:19 +0100 > "Daniel P. Berrange" <berra...@redhat.com> wrote: > > > On Fri, Oct 16, 2009 at 10:05:44AM -0300, Luiz Capitulino wrote: > > > On Fri, 16 Oct 2009 10:06:10 +0200 > > > Paolo Bonzini <pbonz...@redhat.com> wrote: > > > > > > > On 10/16/2009 12:44 AM, Hollis Blanchard wrote: > > > > > How about this (basically what Paolo suggested): > > > > > > > > > > { "error": { "code": 12, > > > > > "desc": "device %{bus}:%{address} already open", > > > > > "data": { "bus": 0, "address": 12 } } } > > > > > > > > > > 'desc'*may* be used by the client, or may be replaced with a > > > > > localized > > > > > version. > > > > > > > > I would say that desc need not go on the wire too. The client might > > > > not > > > > even want to show the same string to the user, for example they may > > > > want > > > > to say "mouse already" open. > > > > > > > > The "device %{bus}:%{address} already open" would be strictly inside > > > > QEMU, for consumption of the monitor interface. Of course since the > > > > server is in QEMU too it makes sense to consolidate it in the same > > > > struct, but this does not mean that everything in the struct needs to > > > > go > > > > on the wire. > > > > > > This is what my current proposal does, "desc" goes on the wire > > > because it's a simple description but messages for users consumption > > > are printed by .user_print. > > > > > > I think we could make this very simple if we use a solution along > > > the lines suggested by Hollis and do _not_ allow variable information > > > (ie. 'handler data') to go on the wire. > > > > > > I mean, we could let current errors as they are but add error codes > > > and new error descriptions to be used by the protocol only. > > > > > > For example, a call to: > > > > > > monitor_printf(mon, "husb: host usb device %d.%d is already open\n", > > > bus_num, addr); > > > > > > Would become: > > > > > > qemu_error_structed(404, "husb: host usb device %d.%d is already open\n", > > > bus_num, addr); > > > > > > When in user protocol the message is printed normally, when in protocol > > > mode the error code is used to index the error table and on the wire > > > we emit: > > > > > > { "error": { "code": 404, "desc": "device already open" } } > > > > > > Now I need to know if it's ok to have such simple error information > > > on the wire. > > > > In so much as its providing an error code + message I think that is > > sufficient, but I think we should take care to ensure that the error > > description contains enough information to allow useful troubleshooting. > > As such doing a simple index lookup from error code -> message is not > > sufficient, because it would be throwing away potentially important > > error data. > > Yes, it's going to throw away important info. We could have a big enough > table with all possible errors, but this seems insane at first. > > Another solution could be to change the semantics of the error data, > instead of passing to it information which is already there we could pass > additional info which is not part of the original message. > > On the other hand, I think this will make the usage of qemu_error_structed() > a bit confusing.
Why not include all of it, the error code, the generic string for the error code, and then the formatted specific error string the monitor currently has. eg { "error": { "code": 404, "desc": "device already open", "detail": "husb: host usb device 001.003 is already open" } } Since the latter error messages all already exist, it should make it easier to adapt, and allow clients flexibility in how they report/handle the errors whether to show the detail error to users, or just the generic msg Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|