[libvirt] [PATCH 1/2] init qemu_driver's qemuImgBinary field

Eric Blake eblake at redhat.com
Wed Nov 14 14:07:23 UTC 2012


On 11/14/2012 01:57 AM, li guang wrote:
> 在 2012-11-14三的 02:28 -0600,Doug Goldstein写道:
>> On Wed, Nov 14, 2012 at 1:56 AM, li guang <lig.fnst at cn.fujitsu.com> wrote:
>>> 在 2012-11-14三的 01:38 -0600,Doug Goldstein写道:
>>>> On Tue, Nov 13, 2012 at 9:03 PM, liguang <lig.fnst at cn.fujitsu.com> wrote:
>>>>> Signed-off-by: liguang <lig.fnst at cn.fujitsu.com>
>>>>> ---

Your commit message needs justification why you are moving from a lazy
cache to a guaranteed initialization.

>>
>> I would argue that its bad form to do that for at least 2 reasons.
>> 1. The fact that driver->qemuImgBinary is "hidden" behind a function
>> to access it means you should effectively treat it as opaque and not
>> directly access it.
> 
> well, quite conceptually,
> but, qemu_driver is global variable,
> every member can't be hidden at all.
> e.g. driver->snapshotDir was used directly.

I don't mind accessing the member variable directly, but only on
condition that we completely delete qemuFindQemuImgBinary(), and instead
inline its initialization code into the one-time initialization.  As
long as the function call remains, then code should use the function
call.  If you are trying to avoid the caching function call, then you
should avoid it everywhere, and this patch didn't go far enough.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 617 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20121114/4a9696c4/attachment-0001.sig>


More information about the libvir-list mailing list