[libvirt] [PATCH] storage: Refresh pool after creating volume

[Osier Yang]

On 29/05/13 20:51, Martin Kletzander wrote:
> On 05/29/2013 11:53 AM, Osier Yang wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=965442
>>
>> One has to refresh the pool to get the correct pool info, this
>> patch refreshes the pool after creating a volume in code instead.
>> Pool refreshing failure is fine to ignore with a warning.
>> ---
>>   src/storage/storage_driver.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
>> index a2b0c21..2a55095 100644
>> --- a/src/storage/storage_driver.c
>> +++ b/src/storage/storage_driver.c
>> @@ -1443,6 +1443,9 @@ storageVolCreateXML(virStoragePoolPtr obj,
>>   
>>       }
>>   
>> +    if (backend->refreshPool && backend->refreshPool(obj->conn, pool) < 0)
>> +        VIR_WARN("Failed to refresh pool after creating volume");
>> +
>>       VIR_INFO("Creating volume '%s' in storage pool '%s'",
>>                volobj->name, pool->def->name);
>>       ret = volobj;
>> @@ -1606,6 +1609,9 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
>>           goto cleanup;
>>       }
>>   
>> +    if (backend->refreshPool && backend->refreshPool(obj->conn, pool) < 0)
>> +        VIR_WARN("Failed to refresh pool after creating volume");
>> +
>>       VIR_INFO("Creating volume '%s' in storage pool '%s'",
>>                volobj->name, pool->def->name);
>>       ret = volobj;
>>
> I don't want to say NACK just like that, but I think the bug indicates
> there's a problem in the storage driver.  It should automatically
> reflect the changes made to that pool.

That's the thing I mentioned long time ago. Using things like inotify
to update the pool's info (though inotify doesn't work for all pool
types, such as iscsi).

> What's the structure that gets
> updated only with refresh and not after the vol is created?

Can you explain more about this? I guess you mean the vol is
created outside of libvirt? such as a iscsi target create a new
LUN?

> Does it do
> with all the drivers?

Not sure what do you mean for "drivers".  But I guess you mean
"storage backends" here? if so, yes.  All the current storage backends
support "refreshPool"/


>
> Long story short; I think this bug fixes the symptom, not the problem.

As said  above, you have a right opinion. The pool should be notified
asynchronously, but it's thing I don't have enough time to do currently.

Osier