[libvirt] [PATCH v2 5/8] storageVolWipePattern: Don't take shortcut to refreshPool()

Pavel Hrdina phrdina at redhat.com
Fri Mar 15 14:14:54 UTC 2019


On Wed, Mar 06, 2019 at 03:59:15PM +0100, Michal Privoznik wrote:
> In d16f803d780 we've tried to solve an issue that after wiping an
> image its format might have changed (e.g. from qcow2 to raw) but
> libvirt wasn't probing the image format. We fixed this by calling
> virStorageBackendRefreshVolTargetUpdate() which is what
> refreshPool() would end up calling. But this shortcut is not good
> enough because the function is called only for local types of
> volumes (like dir, fs, netfs). But now that more backends support
> volume wiping we have to call the function with more caution.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/storage/storage_driver.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 98be434005..cefffdfd64 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -2525,11 +2525,15 @@ storageVolWipePattern(virStorageVolPtr vol,
>      if (rc < 0)
>          goto cleanup;
>  
> -    /* Instead of using the refreshVol, since much changes on the target
> -     * volume, let's update using the same function as refreshPool would
> -     * use when it discovers a volume. The only failure to capture is -1,
> -     * we can ignore -2. */
> -    if (virStorageBackendRefreshVolTargetUpdate(voldef) == -1)
> +    /* For local volumes, Instead of using the refreshVol, since
> +     * much changes on the target volume, let's update using the
> +     * same function as refreshPool would use when it discovers a
> +     * volume. The only failure to capture is -1, we can ignore
> +     * -2. */
> +    if ((backend->type == VIR_STORAGE_POOL_DIR ||
> +         backend->type == VIR_STORAGE_POOL_FS ||
> +         backend->type == VIR_STORAGE_POOL_NETFS) &&
> +        virStorageBackendRefreshVolTargetUpdate(voldef) == -1)

virStorageBackendRefreshLocal() is used for vstorage as well and
vstorage supports wiping so this should probably include vstorage.

I don't like this ugly hack, but I guess there is no better and easy
way how to refactor it.

Pavel

>          goto cleanup;
>  
>      ret = 0;
> -- 
> 2.19.2
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- 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/20190315/b8a904ca/attachment-0001.sig>


More information about the libvir-list mailing list