[PATCH] the leading space in volmode check will never match the leading tab output from zfs get

richardburakowski@gmail.com posted 1 patch 3 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200924004810.3647088-1-richard.burakowski@groupm.com
src/storage/storage_backend_zfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] the leading space in volmode check will never match the leading tab output from zfs get
Posted by richardburakowski@gmail.com 3 years, 6 months ago
Signed-off-by: Richard Burakowski <richard.burakowski@gmail.com>
---
 src/storage/storage_backend_zfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c
index dc692f47ed..439f5b2fd5 100644
--- a/src/storage/storage_backend_zfs.c
+++ b/src/storage/storage_backend_zfs.c
@@ -71,7 +71,7 @@ virStorageBackendZFSVolModeNeeded(void)
         return ret;
     }
 
-    if (strstr(error, " volmode "))
+    if (strstr(error, "volmode "))
         return 1;
     else
         return 0;
-- 
2.26.2

Re: [PATCH] the leading space in volmode check will never match the leading tab output from zfs get
Posted by Daniel Henrique Barboza 3 years, 6 months ago
The code is ok but the commit message can be improved. Usually
we want a commit title with a single-line summary of what you're
changing, a blank line, and a description describes in more
the change, and your signed-off tag. This guideline can be
found here:

https://libvirt.org/best-practices.html


For this patch, one possibility would be:

-------
storage_backend_zfs.c: remove leading space in volmode check

The leading space in volmode check will never match the leading
tab output from zfs get.


Signed-off-by: Richard Burakowski <richard.burakowski@gmail.com>
-------


Thanks,


DHB

On 9/23/20 9:48 PM, richardburakowski@gmail.com wrote:
> Signed-off-by: Richard Burakowski <richard.burakowski@gmail.com>
> ---
>   src/storage/storage_backend_zfs.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c
> index dc692f47ed..439f5b2fd5 100644
> --- a/src/storage/storage_backend_zfs.c
> +++ b/src/storage/storage_backend_zfs.c
> @@ -71,7 +71,7 @@ virStorageBackendZFSVolModeNeeded(void)
>           return ret;
>       }
>   
> -    if (strstr(error, " volmode "))
> +    if (strstr(error, "volmode "))
>           return 1;
>       else
>           return 0;
> 

Re: [PATCH] the leading space in volmode check will never match the leading tab output from zfs get
Posted by Michal Privoznik 3 years, 6 months ago
On 10/2/20 4:42 PM, Daniel Henrique Barboza wrote:
> The code is ok but the commit message can be improved. Usually
> we want a commit title with a single-line summary of what you're
> changing, a blank line, and a description describes in more
> the change, and your signed-off tag. This guideline can be
> found here:
> 
> https://libvirt.org/best-practices.html
> 
> 
> For this patch, one possibility would be:
> 
> -------
> storage_backend_zfs.c: remove leading space in volmode check
> 
> The leading space in volmode check will never match the leading
> tab output from zfs get.
> 
> 
> Signed-off-by: Richard Burakowski <richard.burakowski@gmail.com>
> -------
> 
> 

Agree with Daniel here. Richard, can you please send v2 with the commit 
message fixed? We can merge it then.

Michal