[libvirt] [PATCH 4/4] storage: Add check for valid FS types in checkPool callback

[Erik Skultety]

On 06/03/2015 01:44 PM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1181087
> 
> The virStorageBackendFileSystemIsMounted is called from three source paths
> checkPool, startPool, and stopPool. Both start and stop validate the FS
> fields before calling *IsMounted; however the check path there is no call.
I'd adjust this "however the check path there is no call" to something
like this "however there is no FS validation call in the check path"
> This could lead the code into returning a true in "isActive" if for some
> reason the target path for the pool was mounted. The assumption being
> that if it was mounted, then we believe we started/mounted it.
> 
> It's also of note that commit id '81165294' added an error message for
> the start/mount path regarding that the target is already mounted so
> fail the start. That check was adjusted by commit id '13fde7ce' to
> only message if actually mounted.
> 
> At one time this led to the libvirtd restart autostart code to declare
> that the pool was active even though the startPool would inhibit startup
> and the stopPool would inhibit shutdown. The autostart path changed as
> of commit id '2a31c5f0' as part of the keep storage pools started between
> libvirtd restarts.
> 
> This patch adds the same check made prior to start/mount and stop/unmount
> to ensure we have a valid configuration before attempting to see if the
> target is already mounted to declare "isActive" or not. Finding an improper
> configuration will now cause an error at checkPool, which should make it
> so we can no longer be left in a situation where the pool was started and
> we have no way to stop it.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/storage/storage_backend_fs.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index 3c39646..b8c5a59 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -537,6 +537,10 @@ virStorageBackendFileSystemCheck(virStoragePoolObjPtr pool,
>      } else {
>          int ret;
>          *isActive = false;
> +
> +        if (virStorageBackendFileSystemValidateFS(pool) < 0)
> +            return -1;
> +
>          if ((ret = virStorageBackendFileSystemIsMounted(pool)) != 0) {
>              if (ret < 0)
>                  return -1;
> 
ACK.
Erik