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

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



在 2012-11-14三的 01:38 -0600,Doug Goldstein写道:
> On Tue, Nov 13, 2012 at 9:03 PM, liguang <lig fnst cn fujitsu com> wrote:
> > Signed-off-by: liguang <lig fnst cn fujitsu com>
> > ---
> >  src/qemu/qemu_domain.c |    2 +-
> >  src/qemu/qemu_driver.c |    3 +++
> >  2 files changed, 4 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index a5592b9..eb5a3d1 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -1647,7 +1647,7 @@ qemuDomainSnapshotForEachQcow2Raw(struct qemud_driver *driver,
> >      int i;
> >      bool skipped = false;
> >
> > -    qemuimgarg[0] = qemuFindQemuImgBinary(driver);
> > +    qemuimgarg[0] = driver->qemuImgBinary;
> >      if (qemuimgarg[0] == NULL) {
> >          /* qemuFindQemuImgBinary set the error */
> >          return -1;
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 5f91688..a25fb53 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -626,6 +626,9 @@ qemudStartup(int privileged) {
> >      if (!qemu_driver->domainEventState)
> >          goto error;
> >
> > +    /* find kvm-img or qemu-img */
> > +    qemuFindQemuImgBinary(qemu_driver);
> > +
> >      /* read the host sysinfo */
> >      if (privileged)
> >          qemu_driver->hostsysinfo = virSysinfoRead();
> > --
> > 1.7.1
> >
> > --
> > libvir-list mailing list
> > libvir-list redhat com
> > https://www.redhat.com/mailman/listinfo/libvir-list
> 
> Honestly this entire patch can be dropped because its not correct.
> qemuFindQemuImgBinary() only looks up the path to the binary the first
> time and then caches it from there on in. So you should always be
> using qemuFindQemuImgBinary(). As far as the addition to the startup
> of looking it up, it seems like that's really a pre-mature
> optimization for your second patch.
> 
> So I'd have to say NACK on this change.
> 

your reason sounds strange, you call qemuFindQemuImgBinary()
to get qemu-img handler every time you want to use it,
I use the handler freely for I've I've initialized before
why can't I use driver->qemuImgBinary instead of
qemuFindQemuImgBinary()?



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



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