On Tue, 09 Sep 2014 06:37:31 -0600 Eric Blake <ebl...@redhat.com> wrote:
> On 09/09/2014 02:27 AM, Kevin Wolf wrote: > > >>> > >>> What was our conclusion wrt the human-readable strerror() string for > >>> debugging? Didn't we want to add that as well? > >> > >> I can do it on top of this patch. So, just adding a new field for this > >> is fine? > > > > I think so. Perhaps we should give it an 'x-' name to make clear that > > it's a debugging help and not supposed to be parsed by management tools. > > Or would that be abuse of that namespace? > > I think using x- would be okay for the namespace, but am not sure we > need to go that far. We already have other fields without x- that are > documented as human-readable only; for example, CommandLineParameterInfo > has: > > # @help: #optional human readable text string, not suitable for parsing. > > or in our events, QUORUM_REPORT_BAD has: > > # @error: #optional, error message. Only present on failure. This field > # contains a human-readable error message. There are no > semantics other > # than that the block layer reported an error and clients should not > # try to interpret the error string. > > So I'd be fine with something as simple as 'message' or 'error'. > > > > > The alternative solution (or actually we could do both) would be to > > store it somewhere in bs and put it into query-block. > > Enhancing query-block in addition to the event makes sense, if it is > easy enough to do. At this point, we are talking about debugging aids, > so as long as they are documented appropriately, I won't be too fussy. OK, but I'm wondering if we need to add the string field to both, BLOCK_IO_ERROR and query-block, or only to one to the other. In my opinion, we should only add it to BLOCK_IO_ERROR if libvirt is going to consume. Otherwise, it makes more sense to add it to query-block because that's where we'll meet the user. Btw, by "consume" I mean read it and make it available to libvirt clients so that they can print it to their users. If we don't want libvirt to consume that field then I think we should only add it to query-block and info block.