[libvirt PATCH] storage_backend_fs: use MKFS ony if WITH_STORAGE_FS is defined

Michal Privoznik mprivozn at redhat.com
Wed Apr 21 14:06:05 UTC 2021


On 4/21/21 3:33 PM, Pavel Hrdina wrote:
> The code in storage_backend_fs is used for storage_dir and storage_fs
> drivers so some parts need to be guarded by checking for
> WITH_STORAGE_FS.
> 
> Fixes: 16c69e7aaed4cabfd8e8c19cc326854d4c480437
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
> 
> GitLab CI faild on MacOS and FreeBSD where storage_fs is disabled.
> 
>   src/storage/storage_backend_fs.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index af645ea9de..e6a348521d 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -401,6 +401,7 @@ static int
>   virStorageBackendExecuteMKFS(const char *device,
>                                const char *format)
>   {
> +#if WITH_STORAGE_FS
>       g_autoptr(virCommand) cmd = NULL;
>       g_autofree char *mkfs = virFindFileInPath(MKFS);
>   
> @@ -431,6 +432,7 @@ virStorageBackendExecuteMKFS(const char *device,
>       if (virCommandRun(cmd, NULL) < 0)
>           return -1;
>   
> +#endif /* WITH_STORAGE_FS */
>       return 0;
>   }
>   
> 

This function has only one caller so we can be sure for now that it is 
not called when !WITH_STORAGE_FS but I think we would be much more safe 
if in that case an error would be returned, instead of return 0.

Can't we do something like this?

diff --git c/src/storage/storage_backend_fs.c 
w/src/storage/storage_backend_fs.c
index af645ea9de..09da8ec8b6 100644
--- c/src/storage/storage_backend_fs.c
+++ w/src/storage/storage_backend_fs.c
@@ -402,7 +402,11 @@ virStorageBackendExecuteMKFS(const char *device,
                               const char *format)
  {
      g_autoptr(virCommand) cmd = NULL;
-    g_autofree char *mkfs = virFindFileInPath(MKFS);
+    g_autofree char *mkfs = NULL;
+
+#ifdef MKFS
+    mkfs = virFindFileInPath(MKFS);
+#endif

      if (!mkfs) {
          virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -412,7 +416,7 @@ virStorageBackendExecuteMKFS(const char *device,
          return -1;
      }

-    cmd = virCommandNewArgList(MKFS, "-t", format, NULL);
+    cmd = virCommandNewArgList(mkfs, "-t", format, NULL);

      /* use the force, otherwise mkfs.xfs won't overwrite existing fs.
       * Similarly mkfs.ext2, mkfs.ext3, and mkfs.ext4 require supplying -F

Michal




More information about the libvir-list mailing list