Re: [libvirt] [V2 Patch] virsh: Change log level order

On 05/20/2011 03:36 AM, Daniel P. Berrange wrote: > On Fri, May 20, 2011 at 03:05:28PM +0530, Supriya Kannery wrote: >> Change log level order so that messages at all other levels get logged >> for "DEBUG" level. Replace magic numbers with corresponding VSH_ERR_xx. >> >> } vshErrorLevel; > NACK I don't think this bit is a good idea. In fact I think it > should be changed to match the log levels in src/util/logging.h > exactly, so we have the same behaviour for virsh and the rest > of libvirt. > >> @@ -2146,7 +2146,8 @@ cmdDominfo(vshControl *ctl, const vshCmd >> >> /* Check and display whether the domain is persistent or not */ >> persistent = virDomainIsPersistent(dom); >> - vshDebug(ctl, 5, "Domain persistent flag value: %d\n", persistent); >> + vshDebug(ctl, VSH_ERR_DEBUG, "Domain persistent flag value: %d\n", >> + persistent); >> if (persistent< 0) > ACK to all these changes though, hardcoding the numbers instead of using > the enums was clearly bad. Unfortunately, I tried to apply this patch in order to pick out just the magic number removals, but your mailer mungled it beyond the point that git could understand it. Can you fix the issues we've identified, including splitting this patch into two pieces (one with just the magic number removal, another with my suggestion of factoring the log level comparison into a helper function, and then just tweaking the helper function and command line -d parsing to nicely map log levels to the same values as for libvirtd)?

Also, since your mailer seems to be conspiring against us, have you tried 'git send-email'? Or attaching rather than sending inline? Practice sending mail to yourself, and running that file through 'git am' to see what sort of problems we might have applying your patch. Or, you could post a link to a public repository (github or repo.or.cz are both decent choices) with your commits pre-applied, to bypass your mailer idiosyncrasies.