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

[Martin Kletzander]

On 06/06/2013 12:13 PM, Daniel P. Berrange wrote:
> On Thu, Jun 06, 2013 at 11:19:18AM +0200, Guido Günther wrote:
>> On Mon, Jun 03, 2013 at 11:47:10PM +0800, Osier Yang wrote:
>>> On 01/06/13 16:05, Martin Kletzander wrote:
>>>> commit 416247880399f88ec382a16c7cebc5460d6b67ad
>>>> Author: Martin Kletzander <mkletzan at redhat.com>
>>>> Date:   Fri May 31 14:25:48 2013 +0200
>>>>
>>>>     test
>>>>
>>>> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
>>>> index 1a85afc..a119fc4 100644
>>>> --- a/src/storage/storage_backend_fs.c
>>>> +++ b/src/storage/storage_backend_fs.c
>>>> @@ -1,7 +1,7 @@
>>>>  /*
>>>>   * storage_backend_fs.c: storage backend for FS and directory handling
>>>>   *
>>>> - * Copyright (C) 2007-2012 Red Hat, Inc.
>>>> + * Copyright (C) 2007-2013 Red Hat, Inc.
>>>>   * Copyright (C) 2007-2008 Daniel P. Berrange
>>>>   *
>>>>   * This library is free software; you can redistribute it and/or
>>>> @@ -797,6 +797,27 @@ error:
>>>>  }
>>>>
>>>>
>>>> +static int
>>>> +virStorageBackendStatVFS(virStoragePoolDefPtr def)
>>>> +{
>>>> +    struct statvfs sb;
>>>> +
>>>> +    if (statvfs(def->target.path, &sb) < 0) {
>>>> +        virReportSystemError(errno,
>>>> +                             _("cannot statvfs path '%s'"),
>>>> +                             def->target.path);
>>>> +        return -1;
>>>> +    }
>>>> +    def->capacity = ((unsigned long long)sb.f_frsize *
>>>> +                           (unsigned long long)sb.f_blocks);
>>>> +    def->available = ((unsigned long long)sb.f_bfree *
>>>> +                            (unsigned long long)sb.f_frsize);
>>>> +    def->allocation = def->capacity - def->available;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>
>>> Though this is only for fs backend,  I could see your thought (only
>>> refresh the
>>> pool's capacity, available, and allocation, without collecting the
>>> volumes), right?
>>> This makes sense for fs backend indeed, but not for some of the other
>>> backends, for instance, scsi backend pool, it populates the pool's capacity,
>>> available, allocation while scanning the LUNs...
>>
>> This would also mean that we do heaps of pool refreshes when creating
>> lots of volumes which can be an expensive operation. Shouldn't we rather
>> only update the information from the information we have from the newly
>> created volume?
> 
> Yeah, when creating a volume, we certainly shouldn't do a full pool
> refresh. We want to update the pool from the metadata that we already
> have about the volume we're creating.
> 

That is what I'm trying to suggest the whole time :)  One check for the
volume and one statvfs call is very reasonable (I'd say) to run when
creating a volume.  And it removes similar problems with information
freshness.

Martin