Re: [libvirt] [PATCH] Restore skipping of setting capacity

On 04/23/2014 07:28 AM, John Ferlan wrote: Fortunately, we haven't released this regression of mine :) s/vol-list vol-list/vol-list/ More precisely, the code needs to be careful that we prefer capacity information learned from file format metadata (qcow2), and fall back to capacity from file system data; my refactoring accidentally swapped things so that file system capacity took precedence over metadata capacity. This still applies without conflict after Peter's 18-patch series (I was a bit surprised on that point, but a nice surprise); hopefully that means that it doesn't matter whether you commit before or after Peter. The fact that we don't have any testsuite coverage of this directly in 'make check' is a bit disconcerting; but at least the external regression testing is covering it. However, I'm still a bit worried that we are just attacking the symptoms, instead of addressing things correctly. It seems like we should be smarter in storage_backend_* to not override a capacity that was already set when probing the file metadata. There's a pretty telling comment in virStorageFileGetMetadataInternal: /* XXX we should consider moving virStorageBackendUpdateVolInfo * code into this method, for non-magic files */ where I think that being a bit smarter about tracing which pieces of information are gathered in which order, and then prioritizing them so that metadata information takes priority over stat() information, could avoid the need to pass an updateCapacity flag through quite so many layers of function calls. We may still go with your patch, but let's wait a day or two to see if I can come up with something more elegant... -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org