Re: [libvirt] [PATCH V3] Add proxy FS support to libvirt
On Wed, Aug 08, 2012 at 03:03:51PM +0100, Daniel P. Berrange wrote:
On Sun, Aug 05, 2012 at 10:04:35AM +0530, M. Mohan Kumar wrote:
> @@ -2575,9 +2577,43 @@ error:
> return NULL;
> }
>
> +/*
> + * Invokes the Proxy Helper with one of the socketpair as its parameter
> + *
> + */
> +static int qemuInvokeProxyHelper(const char *emulator, int sock, const char *path)
> +{
> +#define HELPER "virtfs-proxy-helper"
> + int ret_val, status;
> + virCommandPtr cmd;
> + char *helper, *dname;
> +
> + dname = dirname(strdup(emulator));
Missing check for NULL from strdup()
> + if (virAsprintf(&helper, "%s/%s", dname, HELPER) < 0) {
> + VIR_FREE(dname);
> + virReportOOMError();
> + return -1;
You must VIR_FORCE_CLOSE(sock) here, since other exit
paths will close it
> + }
I think this is slightly bogus - and it'd be better to
do virFindFileInPath(HELPER), and if that doesn't work
then also look in /usr/libexec/$HELPER.
Actually I take this back. You are doing what I originally
suggested you do, which is correct. Looking in $PATH would
be wrong since it might find a proxy that's from a different
version of QEMU. We do need to take account of fact that
QEMU might be in $prefix/libexec, instead of $prefix/bin.
Also Unfortunately dirname is not thread-safe since it can
return a statically allocated string - which must not
be free()d, so we can't use that.
So I think I'd build the path slightly differently:
char *tmp, *prefix;
if (!(prefix = strdup(emulator))) {
virReportOOMError();
return -1;
}
if (tmp = strstr(prefix, "/bin/") ||
tmp = strstr(prefix, "/libexec/") {
*tmp = '\0';
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to determine prefix from %s"),
emulator);
VIR_FREE(prefix);
}
if (virAsprintf(&path, "%s/bin/" HELPER, prefix) < 0) {
VIR_FREE(prefix);
return -1;
}
VIR_FREE(prefix);
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|