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

li guang lig.fnst at cn.fujitsu.com
Thu Nov 15 00:43:57 UTC 2012


在 2012-11-14三的 07:07 -0700,Eric Blake写道:
> 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.

OK, will add more commit messages.

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

will change further.

-- 
li guang  lig.fnst at cn.fujitsu.com
linux kernel team at FNST, china





More information about the libvir-list mailing list