Re: [PATCH] Don't warn about nonexisting connection drivers

[Cole Robinson <crobinso@xxxxxxxxxx>]

On 03/17/2016 03:02 PM, Jovanka Gulicoska wrote:
> ---
>  src/driver.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/driver.c b/src/driver.c
> index 2985538..1514a3b 100644
> --- a/src/driver.c
> +++ b/src/driver.c
> @@ -62,7 +62,7 @@ virDriverLoadModule(const char *name)
>          return NULL;
>  
>      if (access(modfile, R_OK) < 0) {
> -        VIR_WARN("Module %s not accessible", modfile);
> +        VIR_INFO("Module %s not accessible", modfile);
>          goto cleanup;
>      }
>  
> 

Thanks for the patch! The actual change is fine, but I suggest a commit
message like:

  driver: log missing modules as INFO, not WARN

  Missing modules is a common expected scenario for most libvirt usage on
  RPM distributions like Fedora, so it doesn't really warrant logging at
  WARN level. Use INFO instead

  https://bugzilla.redhat.com/show_bug.cgi?id=1274849


General rules to follow:

- The commit message should be more than one line. Too much info is always
better than too little.
- The commit message should explain the motivation behind the change, see my
explanation above, and check git log for other examples
- If there's an associated bug, list it in the commit message (I had pointed
Jovanka at that bug offlist)
- the first line of most commit messages generally starts with a prefix like
'qemu:' or 'docs:' to give a quick indication what part of the code it
touches. 'driver:' here isn't really that meaningful since it's a small part
of the code but personally I always try to stick to the convention.

Generally if you're looking for examples, check 'git log' or 'git log
[filename]' for the file(s) you are touching. Also some more reading for git
commit advice that largely applies to libvirt, including good and bad examples:

https://wiki.openstack.org/wiki/GitCommitMessages


Anyways, I pushed your patch with that commit message updated!

commit 9a0c7f5f834185db9017c34aabc03ad99cf37bed
Author: Jovanka Gulicoska <jovanka.gulicoska@xxxxxxxxx>
Date:   Thu Mar 17 20:02:20 2016 +0100

    driver: log missing modules as INFO, not WARN

    Missing modules is a common expected scenario for most libvirt usage on
    RPM distributions like Fedora, so it doesn't really warrant logging at
    WARN level. Use INFO instead

    https://bugzilla.redhat.com/show_bug.cgi?id=1274849


Thanks,
Cole

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list