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

Pavel Hrdina phrdina at redhat.com
Wed Apr 21 14:23:19 UTC 2021


On Wed, Apr 21, 2021 at 04:06:05PM +0200, Michal Privoznik wrote:
> 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

I wanted to avoid using #ifdef MKFS because once this series [1] is
merged it will be always true. In addition using WITH_STORAGE_FS is what
we do for other functions in this file as well so I wanted to keep it
consistent.

If you are OK with using WITH_STORAGE_FS instead of MKFS in the ifdef
we can push this version.

Pavel

[1] <https://listman.redhat.com/archives/libvir-list/2021-April/msg00973.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210421/d3097097/attachment-0001.sig>


More information about the libvir-list mailing list