Re: [libvirt] [PATCH v2 5/8] storageVolWipePattern: Don't take shortcut to refreshPool()
On 3/15/19 3:14 PM, Pavel Hrdina wrote:
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(a)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.
Ah, good point. I'm adding it here on the list.
I don't like this ugly hack, but I guess there is no better and easy
way how to refactor it.
Right. I don't see any neither.
Michal