[libvirt] [PATCH] [v2] Support kvm-img or qemu-img dynamically
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 Signed-off-by: Doug Goldstein <cardoe@gentoo.org> --- configure.in | 15 --------- src/libvirt_private.syms | 1 + src/storage_backend_fs.c | 76 ++++++++++++++++++++++++++++++++------------- src/util.c | 28 +++++++++++++++++ src/util.h | 2 + 5 files changed, 85 insertions(+), 37 deletions(-)
On Wed, Jun 10, 2009 at 10:28:00AM -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
Signed-off-by: Doug Goldstein <cardoe@gentoo.org>
ACK, this looks good 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 :|
On Wed, Jun 10, 2009 at 10:28:00AM -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
That version looks perfect, applied and commited :-) Thanks a lot ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
2009/6/10 Doug Goldstein <cardoe@gentoo.org>:
This patch adds a new function virFindFileInPath() and uses it to find where a binary lives in the PATH environment variable. [...]
+/* + * Finds a requested file in the PATH env. e.g.: + * "kvm-img" will return "/usr/bin/kvm-img" + * + * You must free the result + */ +char *virFindFileInPath(const char *file) +{ + char pathenv[PATH_MAX]; + char *penv = &pathenv; /* this is for glibc 2.10 strsep chnages */ + char *pathseg; + char fullpath[PATH_MAX]; + + /* copy PATH env so we can tweak it */ + strncpy(pathenv, getenv("PATH"), PATH_MAX); + pathenv[PATH_MAX - 1] = '\0'; + + /* for each path segment, append the file to search for and test for + * it. return it if found. + */ + while ((pathseg = strsep(&penv, ":")) != NULL) { + snprintf(fullpath, PATH_MAX, "%s/%s", pathseg, file); + if (virFileExists(fullpath)) + return strdup(fullpath); + } + + return NULL; +} GCC (version 4.3.3 here) warns about "initialization from incompatible pointer type" at this line: char *penv = &pathenv; /* this is for glibc 2.10 strsep chnages */ Changing it to char *penv = pathenv; /* this is for glibc 2.10 strsep chnages */ fixes the warning. At first I was surprised that the first version with & works and gives correct results, then I checked that actual values that are assigned to char *penv and it's the same in both cases. But the point is: the additional & triggers a warning (breaks compilation with --enable-compile-warnings=error) and removing it doesn't change the behavior. PS: What's this comment about changes in glibc 2.10 to strsep() referring to? I looked at the changelog but couldn't find any recent changes to strsep(). Regards, Matthias
Anno domini 2009 Matthias Bolte scripsit:
2009/6/10 Doug Goldstein <cardoe@gentoo.org>:
This patch adds a new function virFindFileInPath() and uses it to find where a binary lives in the PATH environment variable. [...]
+char *virFindFileInPath(const char *file) +{ [...] +}
GCC (version 4.3.3 here) warns about "initialization from incompatible pointer type" at this line:
char *penv = &pathenv; /* this is for glibc 2.10 strsep chnages */
Changing it to
char *penv = pathenv; /* this is for glibc 2.10 strsep chnages */
fixes the warning.
At first I was surprised that the first version with & works and gives correct results, then I checked that actual values that are assigned to char *penv and it's the same in both cases. But the point is: the additional & triggers a warning (breaks compilation with --enable-compile-warnings=error) and removing it doesn't change the behavior.
As we found this subtle "bug"(?) - at least it's questionable why the '&' was added, which does not change the semantics for the stack var - by the mails our libVirt build daemon sends to the two of us, I wanted to raise the question, if this service might be of public interest? It would be no problem for to forward the mails (max. one per hour, IFF a new commit could be pulled from the public GIT repository) to any of the libVirt lists or maybe I'm even able to setup an own one. I would prefer the first option, though. Anybody interested? Ciao Max -- Follow the white penguin.
On Fri, Jun 12, 2009 at 10:53:01PM +0200, Maximilian Wilhelm wrote:
Anno domini 2009 Matthias Bolte scripsit: [...]
additional & triggers a warning (breaks compilation with --enable-compile-warnings=error) and removing it doesn't change the behavior.
As we found this subtle "bug"(?) - at least it's questionable why the '&' was added, which does not change the semantics for the stack var - by the mails our libVirt build daemon sends to the two of us, I wanted to raise the question, if this service might be of public interest?
It would be no problem for to forward the mails (max. one per hour, IFF a new commit could be pulled from the public GIT repository) to any of the libVirt lists or maybe I'm even able to setup an own one. I would prefer the first option, though.
Anybody interested?
Hum, not for the main list of course. Somehow I don't find mail to be the best way to carry the informations, something like a set of web pages for the runs in the last couple of days would certainly be more convenient. One way could be to send the output by email and aggregate it somewhere, or export the last result on a web page and I could do the aggregation. But I know some people compile with -Werror on a regular basis so in any case this kind of problems should be caught quickly, maybe it's not worth developping a big setup... Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Fri, Jun 12, 2009 at 10:53:01PM +0200, Maximilian Wilhelm wrote:
Anno domini 2009 Matthias Bolte scripsit:
As we found this subtle "bug"(?) - at least it's questionable why the '&' was added, which does not change the semantics for the stack var - by the mails our libVirt build daemon sends to the two of us, I wanted to raise the question, if this service might be of public interest?
It would be no problem for to forward the mails (max. one per hour, IFF a new commit could be pulled from the public GIT repository) to any of the libVirt lists or maybe I'm even able to setup an own one. I would prefer the first option, though.
We already have an automated build system which alerts us to problems in libvirt & related tools http://builder.virt-manager.org/ Regards, 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 :|
participants (5)
-
Daniel P. Berrange -
Daniel Veillard -
Doug Goldstein -
Matthias Bolte -
Maximilian Wilhelm