Re: [libvirt PATCH v2 13/21] storage: use virFindFileInPath to validate presence of mkfs

[Pavel Hrdina]

On Tue, Apr 20, 2021 at 02:44:53PM +0100, Daniel P. Berrangé wrote:
> On Mon, Apr 19, 2021 at 07:14:16PM +0200, Pavel Hrdina wrote:
> > Future patch will remove MKFS define as we will no longer check it
> > during compilation.
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  src/storage/storage_backend_fs.c | 24 +---
> >  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> Reviewed-by: Daniel P. Berrangé 

Thanks for the review. I've pushed the cleanup patches and will post v3
without the virFindFileInPath move from testutilsqemu.c.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 13/21] storage: use virFindFileInPath to validate presence of mkfs

[Daniel P . Berrangé]

On Mon, Apr 19, 2021 at 07:14:16PM +0200, Pavel Hrdina wrote:
> Future patch will remove MKFS define as we will no longer check it
> during compilation.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  src/storage/storage_backend_fs.c | 24 +---
>  1 file changed, 9 insertions(+), 15 deletions(-)

Reviewed-by: Daniel P. Berrangé 

> 
> diff --git a/src/storage/storage_backend_fs.c 
> b/src/storage/storage_backend_fs.c
> index bb93d64bbe..b0f8070c6c 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -397,13 +397,20 @@ virStorageBackendFileSystemCheck(virStoragePoolObj 
> *pool,
>  return 0;
>  }
>  
> -/* some platforms don't support mkfs */
> -#ifdef MKFS
>  static int
>  virStorageBackendExecuteMKFS(const char *device,
>   const char *format)
>  {
>  g_autoptr(virCommand) cmd = NULL;
> +g_autofree char *mkfs = virFindFileInPath(MKFS);
> +
> +if (!mkfs) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("mkfs is not supported on this platform: "

s/supported/available/ since it may simply be uninstalled now.

> + "Failed to make filesystem of type '%s' on device 
> '%s'"),
> +   format, device);
> +return -1;
> +}

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[libvirt PATCH v2 13/21] storage: use virFindFileInPath to validate presence of mkfs

[Pavel Hrdina]

Future patch will remove MKFS define as we will no longer check it
during compilation.

Signed-off-by: Pavel Hrdina 
---
 src/storage/storage_backend_fs.c | 24 +---
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index bb93d64bbe..b0f8070c6c 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -397,13 +397,20 @@ virStorageBackendFileSystemCheck(virStoragePoolObj *pool,
 return 0;
 }
 
-/* some platforms don't support mkfs */
-#ifdef MKFS
 static int
 virStorageBackendExecuteMKFS(const char *device,
  const char *format)
 {
 g_autoptr(virCommand) cmd = NULL;
+g_autofree char *mkfs = virFindFileInPath(MKFS);
+
+if (!mkfs) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("mkfs is not supported on this platform: "
+ "Failed to make filesystem of type '%s' on device 
'%s'"),
+   format, device);
+return -1;
+}
 
 cmd = virCommandNewArgList(MKFS, "-t", format, NULL);
 
@@ -426,19 +433,6 @@ virStorageBackendExecuteMKFS(const char *device,
 
 return 0;
 }
-#else /* #ifdef MKFS */
-static int
-virStorageBackendExecuteMKFS(const char *device G_GNUC_UNUSED,
- const char *format G_GNUC_UNUSED)
-{
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("mkfs is not supported on this platform: "
- "Failed to make filesystem of "
- "type '%s' on device '%s'"),
-   format, device);
-return -1;
-}
-#endif /* #ifdef MKFS */
 
 static int
 virStorageBackendMakeFileSystem(virStoragePoolObj *pool,
-- 
2.30.2