On Mon, Jun 08, 2009 at 02:33:04PM -0500, Doug Goldstein wrote:

This patch adds a new function virFindFileInPath() and uses it to find where a binary lives in the PATH environment variable. Using this, we can dynamically find where utility functions exist (and if they even exists). So such we remove the build-time check for qemu-img and make it dynamic for kvm-img and qemu-img. Several distros uses kvm-img over qemu-img when installing KVM. kvm-img also includes several patches which Red Hat is trying to upstream with QEMU so this patch supports those features which are commented out in libvirt when using kvm-img

@@ -1277,17 +1273,45 @@ static int createQemuImg(virConnectPtr conn, } }

+ if ((create_tool = virFindFileInPath("kvm-img")) != NULL) + use_kvmimg = 1; + else if ((create_tool = virFindFileInPath("qemu-img")) != NULL) + use_kvmimg = 0; + else + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unable to find kvm-img or qemu-img"));

I think you missed a 'return -1' here, otherwise it'll go on to use a NULL 'create_tool' later.

+ + if (inputvol) { + convargv[0] = create_tool; + imgargv = convargv; + } else if (vol->backingStore.path) { + imgargvbacking[0] = create_tool; + if (use_kvmimg) { + imgargvbacking[6] = "-F"; + imgargvbacking[7] = backingType; + imgargvbacking[8] = vol->target.path; + imgargvbacking[9] = size; + } + imgargv = imgargvbacking; + } else { + imgargvnormal[0] = create_tool; + imgargv = imgargvnormal; + } + + /* Size in KB */ snprintf(size, sizeof(size), "%llu", vol->capacity/1024);

if (virRun(conn, imgargv, NULL) < 0) { + VIR_FREE(imgargv[0]); return -1; }

+ VIR_FREE(imgargv[0]); + return 0; }

Aside from that one possible bug, I think this is OK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|