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

[Eric Blake]

On 10/31/2012 06:40 PM, liguang wrote:
> Signed-off-by: liguang <lig.fnst at 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 17ae3b9..ac16772 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1654,7 +1654,7 @@ qemuDomainSnapshotForEachQcow2Raw(struct qemud_driver *driver,
>      int i;
>      bool skipped = false;
>  
> -    qemuimgarg[0] = qemuFindQemuImgBinary(driver);
> +    qemuimgarg[0] = driver->qemuImgBinary;

Personally, I like going through accessor methods rather than direct
field access, as it leaves us more freedom for changing the
implementation in the future.  I'm not sure this patch buys us anything,
other than it would fail to start the libvirtd qemu driver at startup
rather than on the first command that needed to use qemu-img. Since
failing early on a bad setup is generally useful, I guess I could live
with this patch, except that you need to rebase it on top of Peter's
recent patches that added another caller of qemuFindQemuImgBinary.  For
that matter, if you are going to gaurantee that driver->qemuImgBinary is
always set, it might be better to remove the qemuFindQemuImgBinary()
function altogether, and instead inline its body...

> +++ b/src/qemu/qemu_driver.c
> @@ -624,6 +624,9 @@ qemudStartup(int privileged) {
>      if (!qemu_driver->domainEventState)
>          goto error;
>  
> +    /*find kvm-img or qemu-img*/
> +    qemuFindQemuImgBinary(qemu_driver);
> +

into this one remaining call site.

Also, I would rebase the series to put this patch first, not last, since
your patch 1/2 was using driver->qemuImgBinary instead of
qemuFindQemuImgBinary().

-- 
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/20121108/c7ab7fb4/attachment-0001.sig>