[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 00/10]: QError v4

From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH 00/10]: QError v4
Date: Wed, 18 Nov 2009 12:08:58 -0600
User-agent: Thunderbird (X11/20090825)

Markus Armbruster wrote:
1. QError feels overengineered, particularly for a first iteration of a

   We go to great lengths to build highly structured error objects.
   There is only one sane reason for doing that: a demonstrated client
   need.  I can't see that need.

A GUI wants to present the user a mechanism to reduce the memory consumption of a VM. There are three reasons why this would not work. The first is that ballooning was not configured for this guest. The second is that ballooning was configured but the version of the KVM kernel module does not have a sync mmu. The third is that an appropriate guest driver has not been loaded for the device.

If we don't specify a type of error, all the GUI can do is fail right before trying to balloon the guest. That offers the user no insight why it failed other than popping up a message box with an error message (that may not even be in their locale). For a non-english speaker, it's gibberish.

If you use three types of errors, then a GUI can differentiate these cases and presents the user different feedback. For the first case, it can simply gray out the menu item. For the second can, it can also gray out the menu item but it can provide a one time indication (in say, a status bar), that ballooning is unavailable because the kernel version does not support it. For the third error, you would want to prompt the user to indicate the driver is not loaded in the guest.

It's highly likely that for this last case, you'd want generic code to generate this error. Further more, in order to generate the error message for a user, you need to know what device does not have a functioning driver. You may say it's obvious for something like info balloon but it's not so obvious if you assume we support things other than just virtio-pci-balloon. For instance, with s390, it's a different balloon driver. For xenner/xenpv, it's a different driver. It really suggests we should send the qdev device name for the current balloon driver.

2. It falls short of the requirement that clients can reasonably
   classify errors they don't know.

I think this is wishful thinking. I strongly suspect that you can't find a reasonably popular web browser that doesn't know all of the error codes that web servers generate.

We should document all of the errors that each QMP function can return.

That said, if you thought there was 4-5 common classes of errors, adding an additional 'base_class' field could be a completely reasonable thing to do. As long as you're adding information vs. taking it away, I'm quite happy.

3. It falls short of the requirement that clients can easily present a
   human-readable error description to their human users, regardless of
   whether they know the error or not.

That's just incorrect. We provide an example conversion table that's akin to strerror() or a __repr__ for an exception in Python.

In more detail[*]:

There are more than one way to screw up here.  One is certainly to fall
short of client requirements in a way that is costly to fix (think
incompatible protocol revision).  Another is to overshoot them in a way
that is costly to maintain.  A third one is to spend too much time on
figuring out the perfect solution.

I believe our true problem is that we're still confused and/or
disagreeing on client requirements, and this has led to a design that
tries to cover all the conceivable bases, and feels overengineered to

I've described my requirements for what a client can do. I'd like to understand how you disagree.

There are only so many complexity credits you can spend in a program,
both globally and individually.  I'm very, very wary of making error
reporting more complex than absolutely, desperately necessary.

We're following a very well established model of error reporting (object-based exceptions). This is well established and well understood.

Reporting errors should remain as easy as we can make it.  The more
cumbersome it is to report an error, the less it is done, and the more
vaguely it is done.  If you have to edit more than the error site to
report an error accurately, then chances skyrocket that it won't be
reported, or it'll be reported inaccurately.

I don't buy these sort of arguments. We are not designing a library that is open to abuse. If our developers are fall victim to this, then we need to retrain them to be less lazy by not accepting patches that do a poor job reporting errors.

 And not because coders are
stupid or lazy, but because they make sensible use of their very limited
complexity credits: if you can either get the job done with lousy error
messages, or not get it done at all, guess what the smart choice is.

Error reporting is extremely important in a management scenario. You are severely limited by what actions you can take based on the amount of error information returned. This is particularly when dealing with a multi-client management mechanism. IMHO, this is a sensible place to spend complexity credits.

If I understand Dan correctly, machine-readable error code +
human-readable description is just fine, as long as the error code is
reasonably specific and the description is precise and complete.  Have
we heard anything else from client developers?

There are no client developers because QMP doesn't exist. Historically, we haven't provided high quality error messages so of course libvirt makes due without them today.

But I can answer on behalf of the management work we've done based on libvirt.

If you attempt to do a virDomainSetMemory() to a qemu domain, you only get an error if you're using a version of qemu that doesn't support the balloon command. You do not get an error at all for the case where the kernel module doesn't support ballooning or a guest driver isn't loaded.

The concrete use case here is building an autoballoon mechanism part of a larger management suite. If you're in an environment where memory overcommit is important, then you really do want to know whether ballooning isn't functioning because appropriate guest drivers aren't loaded.

I'm not a client developer, but let me make a few propositions on client
needs anyway:

* Clients are just as complexity-bound as the server.  They prefer their
  errors as simple as possible, but no simpler.

I agree but the problem is that every client has a different definition of "simple as possible". My client doesn't necessarily care about PCI hotplug error messages because it's not a function that we use. However, ballooning is tremendously important and I want to know everything I can about why it failed.

* The crucial question for the client isn't "what exactly went wrong".
  It is "how should I handle this error".  Answering that question
  should be easy (say, check the error code).  Figuring out what went
  wrong should still be possible for a human operator of the client.

I disagree. The client needs to know what went wrong if they are going to take a corrective action.

* Clients don't want to be tightly coupled to the server.

I don't follow this.

* No matter how smart and up-to-date the client is, there will always be
  errors it doesn't know.  And it needs to answer the "how to handle"
  question whether it knows the error code or not!  That's why protocols
  like HTTP have simple rules to classify error codes.

HTTP is the exception and the HTTP error classes really leave an awful lot to be desired. If you feel it's important to add error classes, I'd be happy to consider those patches.

  Likewise, it needs to be able to give a human operator enough
  information to figure out what went wrong whether it knows the error
  or not.  How do you expect clients to format a structured error object
  for an error they don't know into human-readable text?  Isn't it much
  easier and more robust to cut out the formatting middle-man and send
  the text along with the error?

We do that by providing a conversion table. The fundamental problem here is localization.

* Clients need to present a human-readable error description to their
  human users, regardless of whether they know the error or not.

I would never pass an error string from a third party directly to a user. I doubt you'll find a lot of good applications that do. From a usability perspective, you need to be in control of your interactions with the user. They grammar needs to be consistent and it has to be localized. The best you would do with a third party string is log it to some log file for later examination by support. In that case, dumping the class code and the supporting information in JSON form is just as good as a text description.

There's a general rule of programming that I've found quite hard to
learn and quite painful to disobey: always try the stupidest solution
that could possibly work first.

Based on what I've learned about client requirements so far, I figure
that solution is "easily classified error code + human-readable

I appreciate you feel strongly that this is the right thing to do. That said, I disagree and I have specific use cases in mind that are not satisfied by the above mechanism. In a situation like this, I tend to think the best thing to do is provide more information such that everyone can get the function they want.

If we add a base_class member to the error structure and we have the table for formatting error messages, then that should satisfy your requirements. If you're concerned about a client having to have an up-to-date version of the table, we can introduce a monitor command to pretty print a json error object which would address that concern.

Then I think your argument boils down to, you think the remaining infrastructure is unnecessary for what you anticipate the uses to be. But hopefully, you'll at least concede that there are valid use cases beyond what you anticipate to be the normal case that do need this information.

If we later realize that this solution was *too* stupid, we can simply
add a data member to the error object.

It's never that easy because a management tool has to support a least common denominator.


Anthony Liguori

reply via email to

[Prev in Thread] Current Thread [Next in Thread]