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@xxxxxxxxxx>
>>>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list