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

[Michal Prívozník]

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 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.

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