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

Re: [libvirt] [PATCH] Support kvm-img or qemu-img dynamically



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


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