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

Re: [libvirt] --checksum-fill error on Ubuntu 10.04



 On 10/26/2010 09:04 PM, Justin Clift wrote:
On 10/26/2010 01:41 PM, Osier wrote:
<snip>
Could we treat the error message as warnings? or make it only works for newer
iptables?


The only method to determine if --checksum-fill is in iptables or not is to execute the command, and if it's not there, iptables will exit with a non-0 status. It's not like, eg, qemu, where you can determine what's supported by execing a 'help' command first.


Yeah, the "error" message that we generate is a bit misleading, since
Laine says it's effectively not stopping things from working.

If you know how to easily fix the code so it classifies this as a
warning, then I reckon "go for it". :)

It's not all that simple, or it would have been that way from the start. The problem is that the function that execs this iptables command is shared with other code, and changing its error reporting would change what happens when the other iptables commands fail (and doing the exact right thing in all cases in the short term would require ugly code that would just become obsolete very quickly).

There was a discussion about this when I first added in the --checksum-fill rule, and we agreed that, because it was a temporary problem that would only exist for a short while until a distro's iptables version caught up to the libvirt version, rather than mess up good clean code for something that was going to go away, it would be acceptable to just add an extra warning level log describing the likely cause of the problem, thus heading off any uninformed bug report before it was made.

Of course 1) that's assuming that people actually look at all the log messages, and 2) at the time I'm sure nobody thought Fedora 14 would be released without this code in iptables.

(BTW, examples escape me right now, but I have noticed other cases of commands run by libvirtd exiting with non-0 status and causing an error log, but otherwise being harmless, so this isn't without precedence. Maybe that's one of the things that can be fixed with virRun/virExec are replaced with the new exec library.)


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