Re: [PATCH] Restore skipping of setting capacity

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/23/2014 11:58 AM, Eric Blake wrote:
> On 04/23/2014 07:28 AM, John Ferlan wrote:
>> Commit id 'ac9a0963' refactored out the 'withCapacity' for the
>> virStorageBackendUpdateVolInfo() API.  See:
> 
> Fortunately, we haven't released this regression of mine :)
> 
>>
>> http://www.redhat.com/archives/libvir-list/2014-April/msg00043.html
>>

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

I have run out of time to come up with anything more elegant that
doesn't feel like it is violating release candidate freeze, so ACK to
your patch for 1.2.4.  We can then tackle the problem more fully
(possibly reverting your patch for a nicer more complex solution) in 1.2.5.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]