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

[Osier Yang]

On 30/05/13 00:04, Martin Kletzander wrote:
> 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.

Right in principle.

> But when the driver does something (with the
> pool), it should update its info accordingly without the need to refresh it.

Right too. This applies to deleteVol/resizeVol too. But as I said, though
calliing refreshPool in these APIs is not the best method, but it's the 
generic
thing what current storage driver does. Assuming that we won't
reply on 3rd project/tools, we have to introduce something like event to
let the pool refresh itself. It's just not much better than calling 
refreshPool
in the APIs, as it should call refreshPool anyway finally.

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