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

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(a)redhat.com&gt; >>> 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.