Re: [libvirt] [PATCH] storage: Refresh pool after creating volume
On 05/29/2013 05:55 PM, Osier Yang wrote:
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).
I didn't mean responding to user-created volumes. The user has to do
pool-refresh after that, that's their business as they do something
behind libvirt's back. But when the driver does something (with the
pool), it should update its info accordingly without the need to refresh it.
> 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"/
Yes, backends. I meant what drivers has this issue.
>
> 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.
Not quite what I meant, corrected myself above.
Martin