Re: [libvirt] [PATCH] don't test res == NULL after we've already dereferenced it
On Wed, Jan 06, 2010 at 09:55:23PM +0100, Jim Meyering wrote: Daniel Veillard wrote: On Wed, Jan 06, 2010 at 12:46:11PM +0100, Jim Meyering wrote: As the log says, once we've dereferenced it, there's no point in comparing to NULL. From 463eaf1027a154e71839a67eca85b3ada8b817ff Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Wed, 6 Jan 2010 12:45:07 +0100 Subject: [PATCH] don't test res == NULL after we've already dereferenced it * src/xen/proxy_internal.c (xenProxyCommand): It is known to be non-NULL at that point, so remove the ret == NULL guard, and instead add an assert(ret != NULL), in case future code changes cause the condition to becomes false. Include assert.h. --- src/xen/proxy_internal.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/xen/proxy_internal.c b/src/xen/proxy_internal.c index ec4522b..42b6e91 100644 --- a/src/xen/proxy_internal.c +++ b/src/xen/proxy_internal.c @@ -1,7 +1,7 @@ /* * proxy_client.c: client side of the communication with the libvirt proxy. * - * Copyright (C) 2006, 2008, 2009 Red Hat, Inc. + * Copyright (C) 2006, 2008, 2009, 2010 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -11,6 +11,7 @@ #include config.h #include stdio.h +#include assert.h #include stdlib.h #include unistd.h #include errno.h @@ -444,7 +445,8 @@ retry: /* * do more checks on the incoming packet. */ -if ((res == NULL) || (res-version != PROXY_PROTO_VERSION) || +assert (res != NULL); +if ((res-version != PROXY_PROTO_VERSION) || (res-len sizeof(virProxyPacket))) { virProxyError(conn, VIR_ERR_INTERNAL_ERROR, %s, _(Communication error with proxy: malformed packet\n)); I'm not too fond of adding an assert, res should not be null there or we should already have crashed. ACK to the change without the new assert line. Considering that this is in the daemon and that bad things have been known to happen via NULL derefs, some would argue that an assertion failure is preferable. No, this code is the client side of the Xen proxy implementation, that is never used within daemon context. 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] don't test res == NULL after we've already dereferenced it
Daniel P. Berrange wrote: On Wed, Jan 06, 2010 at 09:55:23PM +0100, Jim Meyering wrote: Daniel Veillard wrote: On Wed, Jan 06, 2010 at 12:46:11PM +0100, Jim Meyering wrote: As the log says, once we've dereferenced it, there's no point in comparing to NULL. From 463eaf1027a154e71839a67eca85b3ada8b817ff Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Wed, 6 Jan 2010 12:45:07 +0100 Subject: [PATCH] don't test res == NULL after we've already dereferenced it * src/xen/proxy_internal.c (xenProxyCommand): It is known to be non-NULL at that point, so remove the ret == NULL guard, and instead add an assert(ret != NULL), in case future code changes cause the condition to becomes false. Include assert.h. --- src/xen/proxy_internal.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/xen/proxy_internal.c b/src/xen/proxy_internal.c index ec4522b..42b6e91 100644 --- a/src/xen/proxy_internal.c +++ b/src/xen/proxy_internal.c @@ -1,7 +1,7 @@ /* * proxy_client.c: client side of the communication with the libvirt proxy. * - * Copyright (C) 2006, 2008, 2009 Red Hat, Inc. + * Copyright (C) 2006, 2008, 2009, 2010 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -11,6 +11,7 @@ #include config.h #include stdio.h +#include assert.h #include stdlib.h #include unistd.h #include errno.h @@ -444,7 +445,8 @@ retry: /* * do more checks on the incoming packet. */ -if ((res == NULL) || (res-version != PROXY_PROTO_VERSION) || +assert (res != NULL); +if ((res-version != PROXY_PROTO_VERSION) || (res-len sizeof(virProxyPacket))) { virProxyError(conn, VIR_ERR_INTERNAL_ERROR, %s, _(Communication error with proxy: malformed packet\n)); I'm not too fond of adding an assert, res should not be null there or we should already have crashed. ACK to the change without the new assert line. Considering that this is in the daemon and that bad things have been known to happen via NULL derefs, some would argue that an assertion failure is preferable. No, this code is the client side of the Xen proxy implementation, that is never used within daemon context. Oh, right. Thanks. However, the point is still valid, so I'll wait for confirmation. This is still about defensive coding, i.e., ensuring that maintenance doesn't violate invariants in harder-to-diagnose ways. If you get a bug report, which would you rather hear? libvirt sometimes segfaults or I get an assertion failure on file.c:999 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] don't test res == NULL after we've already dereferenced it
On Thu, Jan 07, 2010 at 09:37:57AM +0100, Jim Meyering wrote: Daniel P. Berrange wrote: Considering that this is in the daemon and that bad things have been known to happen via NULL derefs, some would argue that an assertion failure is preferable. No, this code is the client side of the Xen proxy implementation, that is never used within daemon context. Oh, right. Thanks. However, the point is still valid, so I'll wait for confirmation. This is still about defensive coding, i.e., ensuring that maintenance doesn't violate invariants in harder-to-diagnose ways. If you get a bug report, which would you rather hear? libvirt sometimes segfaults or I get an assertion failure on file.c:999 I guess it's mostly a matter of coding style, I'm not sure it makes such a difference in practice, libvirt output will likely be burried in a log file, unless virsh or similar CLI tool is used. We have only 4 asserts in the code currently, I think it shows that so far we are not relying on it. On one hand this mean calling exit() and I prefer a good old core dump for debugging than a one line message, on the other hand if you managed to catch that message, well this can help pinpoint if the person reporting has no debugging skills. I think there is pros and cons and that the current status quo is that we don't use asserts but more defensing coding by erroring out when this happen. The best way is not to assert() when we may dereference a NULL pointer but check for NULL at the point where we get that pointer, that's closer to the current code practice of libvirt (or well I expect so :-) and allow useful reporting (we failed to do a given operation) and a graceful error handling without exit'ing. So in general if we think we need an assert somewhere that mean that we failed to do the check at the right place, and I would rather not start to get into the practice of just asserting in a zillion place instead of checking at the correct place. So in my opinion, I'm still not fond of assert(), and I would prefer to catch up problem earlier, like parameter checking on function entry points or checking return value for functions producing pointers. In that case, res being NULL means that both answer and request parameters are null after the retry: label, since the two pointers are not modified in the function this should be tested when entering the function, if ((answer == NULL) (request == NULL)) { virProxyError (NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } you get the error location, as in the assert but propagate the error back in the stack cleanly instead of exit'ing Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt 0.7.5 vs qemu 0.12.1 imcompatability?
On 06/01/10 18:56, Nikola Ciprich wrote: I've had similar trouble with old bios images. Once I replaced them with those built with qemu-kvm, things started working... I've using the packages from virt-preview so I've got whatever BIOS images are being packaged. I've experimented some more now and even a simple: qemu-kvm -enable-kvm just gives me nothing whereas the older version brings up the BIOS screen. Actually looking at it, the BIOS is a separate package which doesn't exist in the virt-preview repository so I'm still using the normal Fedora one. Maybe that's the problem? What's the best channel for reporting issues with the virt-preview packages? Tom -- Tom Hughes (t...@compton.nu) http://www.compton.nu/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] don't test res == NULL after we've already dereferenced it
On Thu, Jan 07, 2010 at 11:03:23AM +0100, Daniel Veillard wrote: On Thu, Jan 07, 2010 at 09:37:57AM +0100, Jim Meyering wrote: However, the point is still valid, so I'll wait for confirmation. This is still about defensive coding, i.e., ensuring that maintenance doesn't violate invariants in harder-to-diagnose ways. If you get a bug report, which would you rather hear? libvirt sometimes segfaults or I get an assertion failure on file.c:999 I guess it's mostly a matter of coding style, I'm not sure it makes such a difference in practice, libvirt output will likely be burried in a log file, unless virsh or similar CLI tool is used. We have only 4 asserts in the code currently, I think it shows that so far we are not relying on it. On one hand this mean calling exit() and I prefer a good old core dump for debugging than a one line message, on the other hand if you managed to catch that message, well this can help pinpoint if the person reporting has no debugging skills. I think there is pros and cons and that the current status quo is that we don't use asserts but more defensing coding by erroring out when this happen. The best way is not to assert() when we may dereference a NULL pointer but check for NULL at the point where we get that pointer, that's closer to the current code practice of libvirt (or well I expect so :-) and allow useful reporting (we failed to do a given operation) and a graceful error handling without exit'ing. So in general if we think we need an assert somewhere that mean that we failed to do the check at the right place, and I would rather not start to get into the practice of just asserting in a zillion place instead of checking at the correct place. So in my opinion, I'm still not fond of assert(), and I would prefer to catch up problem earlier, like parameter checking on function entry points or checking return value for functions producing pointers. The other reason for adding assert(), is if the code leading upto a particular point is too complex to reliably understand whether a variable is NULL. I think that applies in this case. I don't think adding an assert() is the right way to deal with that complexity though. I think it is better to make the code clearer/easier to follow understand 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt 0.7.5 vs qemu 0.12.1 imcompatability?
On Thu, Jan 07, 2010 at 10:08:33AM +, Tom Hughes wrote: On 06/01/10 18:56, Nikola Ciprich wrote: I've had similar trouble with old bios images. Once I replaced them with those built with qemu-kvm, things started working... I've using the packages from virt-preview so I've got whatever BIOS images are being packaged. I've experimented some more now and even a simple: qemu-kvm -enable-kvm just gives me nothing whereas the older version brings up the BIOS screen. Actually looking at it, the BIOS is a separate package which doesn't exist in the virt-preview repository so I'm still using the normal Fedora one. Maybe that's the problem? Yes, the KVM from rawhide is not compatible with the BIOS from F12, so this sounds like a bug in virt-preview What's the best channel for reporting issues with the virt-preview packages? The fedora-virt mailing list 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] About VirtualBox and Libvirt
2010/1/7 Prashant Wakchaure prashant.wakcha...@gmail.com: Is anybody know how to connect libvirt to VirtualBox remotely -- Thanks Regards, Prashant Wakchaure See http://www.libvirt.org/drvvbox.html for some example remote URIs for VirtualBox and see http://www.libvirt.org/remote.html for some general information about remote URIs. Also make sure you have started libvirtd on the remote host and configured it properly for the chosen transport. Matthias -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network/bridge_driver.c: avoid potential NULL-dereference
2010/1/5 Jim Meyering j...@meyering.net: obvious typo: From 6f0810192dc6d9b223f2f973812fd787f398576c Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Tue, 5 Jan 2010 15:48:42 +0100 Subject: [PATCH] network/bridge_driver.c: avoid potential NULL-dereference * src/network/bridge_driver.c (networkBuildDnsmasqArgv): Correct test for NULL *argv. --- src/network/bridge_driver.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index abee78c..69809b3 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1,7 +1,7 @@ /* * driver.c: core driver methods for managing qemu guests * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -513,7 +513,7 @@ networkBuildDnsmasqArgv(virConnectPtr conn, return 0; no_memory: - if (argv) { + if (*argv) { for (i = 0; (*argv)[i]; i++) VIR_FREE((*argv)[i]); VIR_FREE(*argv); -- 1.6.6.384.g14e6a ACK. Matthias -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network/bridge_driver.c: avoid potential NULL-dereference
Matthias Bolte wrote: diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c ... - if (argv) { + if (*argv) { ... ACK. Thanks. Pushed. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Qemu: ask for memory preallocation with large pages
The -mem-prealloc flag should be used when using large pages This ensures qemu tries to allocate all required memory immediately, rather than when first used. The latter mode will crash qemu if hugepages aren't available when accessed, while the former should gracefully fallback to non-hugepages. * src/qemu/qemu_conf.c: add -mem-prealloc flag to qemu command line when using large pages Daniel diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 824055f..d3da776 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2108,6 +2108,7 @@ int qemudBuildCommandLine(virConnectPtr conn, def-emulator); goto error; } +ADD_ARG_LIT(-mem-prealloc); ADD_ARG_LIT(-mem-path); ADD_ARG_LIT(driver-hugepage_path); } -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [virt-tools-list] Questions about virt-manager running on Arch of Itanium 64
On 01/07/2010 01:03 AM, Dustin Xiong wrote: I sloved the problem. I modify the libvirt /src/qemu_conf.c. Add the arch ia64 into the static const struct qemu_arch_info const arch_info_hvm[] = {} as berrange said. So the libvirt passing -M ia64. But the kvm binary only experts '-M itanium'. So i replaced the ia64 by itanium in the file /src/qemu_conf.c. Then the virt-manager finally could work on the arch of Itanium. Thank you for your help. Thank you, everyone here. -Dustin Can you provide a diff of your changes so we can apply it to upstream libvirt code? Thanks, Cole -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Qemu: ask for memory preallocation with large pages
On Thu, Jan 07, 2010 at 02:53:20PM +0100, Daniel Veillard wrote: The -mem-prealloc flag should be used when using large pages This ensures qemu tries to allocate all required memory immediately, rather than when first used. The latter mode will crash qemu if hugepages aren't available when accessed, while the former should gracefully fallback to non-hugepages. * src/qemu/qemu_conf.c: add -mem-prealloc flag to qemu command line when using large pages ACK, definitely don't want VMs crashing when attempting to page in huge pages which don't exist anymore 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] fix build failure with --disable-shared
Laine Stump noticed a failure to build with --disable-shared and found that removing the unconditional -shared option in python/Makefile.am would fix it. The change below reverts 8838ee39ab1c2bb7fffe93bfda220692664e8be6, so Diego, if your goal (with the reverted change) was more than to avoid seemingly-unnecessary work, please tell us what it was. From 1de9c4a9a4175c9f1eb9302ad7fa9867e8a242a4 Mon Sep 17 00:00:00 2001 From: Laine Stump la...@laine.org Date: Thu, 7 Jan 2010 21:07:42 +0100 Subject: [PATCH] let configure --disable-shared work once again Without this change, ./autogen.sh --disable-shared make would evoke a can not build a shared library failure for libvirtmod.la. * python/Makefile.am (libvirtmod_la_LDFLAGS): Do not use -shared. --- python/Makefile.am |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/python/Makefile.am b/python/Makefile.am index 58c6729..04342b7 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -39,7 +39,7 @@ libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c libvirt.c libvirt.h # need extra flags here libvirtmod_la_CFLAGS = @WARN_PYTHON_CFLAGS@ -libvirtmod_la_LDFLAGS = -module -avoid-version -shared -L$(top_builddir)/src/.libs \ +libvirtmod_la_LDFLAGS = -module -avoid-version -L$(top_builddir)/src/.libs \ @CYGWIN_EXTRA_LDFLAGS@ libvirtmod_la_LIBADD = $(mylibs) \ @CYGWIN_EXTRA_LIBADD@ @CYGWIN_EXTRA_PYTHON_LIBADD@ -- 1.6.6.425.g4dc2d -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] fix build failure with --disable-shared
Il giorno gio, 07/01/2010 alle 21.14 +0100, Jim Meyering ha scritto: The change below reverts 8838ee39ab1c2bb7fffe93bfda220692664e8be6, so Diego, if your goal (with the reverted change) was more than to avoid seemingly-unnecessary work, please tell us what it was. Well, to put it simply: you *cannot* both have the Python extension *and* disable shared libraries. --disable-shared tells libtool not to build any kind of shared object for the project; Python extensions are shared objects _only_. While you could avoid building libvirt.so (and just have libvirt.a) you cannot get Python extensions by just building libvirtmod.a. So basically --disable-shared --with-python would just produce an unusable output without my change, and produce a proper error condition with. -- Diego Elio Pettenò — “Flameeyes” http://blog.flameeyes.eu/ If you found a .asc file in this mail and know not what it is, it's a GnuPG digital signature: http://www.gnupg.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] [RFC] xen domctl version 6
Jim Fehlig wrote: Daniel Veillard wrote: On Wed, Dec 23, 2009 at 10:59:09AM -0700, Jim Fehlig wrote: xen-unstable c/s 20685 changed the domctl interface, adding a field to xen_domctl_getdomaininfo structure. This additional field causes stack corruption in libvirt. xen-unstable c/s 20711 rightly bumped the domctl interface version so it is at least possible to handle the new field. The attached patch accounts for shr_pages field added to xen_domctl_getdomaininfo structure. I'm not thrilled about the changes to all the macros - suggestions for improvement welcomed. Tested with domctl version 5 and 6. Yeah, we don't really have much choice there I think, but I would make the macros a bit more forward optimistic, i.e replacing (dom_interface_version == 6 ? with (dom_interface_version = 6 ? if there is a new bump, the call should not fail because we didn't updated all the macros. Yes, good idea. Revised patch attached. Committed now. Thanks! Regards, Jim -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] fix build failure with --disable-shared
Diego Elio “Flameeyes” Pettenò wrote: Il giorno gio, 07/01/2010 alle 21.14 +0100, Jim Meyering ha scritto: The change below reverts 8838ee39ab1c2bb7fffe93bfda220692664e8be6, so Diego, if your goal (with the reverted change) was more than to avoid seemingly-unnecessary work, please tell us what it was. Well, to put it simply: you *cannot* both have the Python extension *and* disable shared libraries. --disable-shared tells libtool not to build any kind of shared object for the project; Python extensions are shared objects _only_. While you could avoid building libvirt.so (and just have libvirt.a) you cannot get Python extensions by just building libvirtmod.a. So basically --disable-shared --with-python would just produce an unusable output without my change, and produce a proper error condition with. But --disable-shared is useful, and your change broke it. As such, I can't see how you would be using --disable-shared. Does the proposed patch cause you any difficulty? If so, please tell us what/how, or propose an alternate patch. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] fix build failure with --disable-shared
On Thu, Jan 07, 2010 at 09:19:17PM +0100, Diego Elio ???Flameeyes??? Petten? wrote: Il giorno gio, 07/01/2010 alle 21.14 +0100, Jim Meyering ha scritto: The change below reverts 8838ee39ab1c2bb7fffe93bfda220692664e8be6, so Diego, if your goal (with the reverted change) was more than to avoid seemingly-unnecessary work, please tell us what it was. Well, to put it simply: you *cannot* both have the Python extension *and* disable shared libraries. --disable-shared tells libtool not to build any kind of shared object for the project; Python extensions are shared objects _only_. While you could avoid building libvirt.so (and just have libvirt.a) you cannot get Python extensions by just building libvirtmod.a. So basically --disable-shared --with-python would just produce an unusable output without my change, and produce a proper error condition with. Agreed, if we want to support --disable-shared, then we can't pretend that building python works. Either expect the error we already have which is accurate, or make configure forcably disable the whole python build. 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] fix build failure with --disable-shared
Il giorno gio, 07/01/2010 alle 21.29 +0100, Jim Meyering ha scritto: But --disable-shared is useful, and your change broke it. On the usefulness of that I have generally a lot to say, but that's another topic. As such, I can't see how you would be using --disable-shared. Very simple: ./configure --disable-shared --without-python Does the proposed patch cause you any difficulty? If so, please tell us what/how, or propose an alternate patch. Without my patch you're building (and installing) an useless file with the default configuration (--enable-shared). And _pretending_ to build Python bindings (that will never work even the slightest) with the configuration you suggested (--disable-shared --with-python). Your really can't get it both ways. As Daniel (Berrange) said, the other option is to check whether shared libraries are enabled by libtool and either disable Python bindings or bail out earlier (configure rather than make) with the impossible combination of no-shared-libraries but-python-bindings. -- Diego Elio Pettenò — “Flameeyes” http://blog.flameeyes.eu/ If you found a .asc file in this mail and know not what it is, it's a GnuPG digital signature: http://www.gnupg.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] fix build failure with --disable-shared
On Thu, Jan 07, 2010 at 08:36:25PM +, Daniel P. Berrange wrote: On Thu, Jan 07, 2010 at 09:19:17PM +0100, Diego Elio ???Flameeyes??? Petten? wrote: Il giorno gio, 07/01/2010 alle 21.14 +0100, Jim Meyering ha scritto: The change below reverts 8838ee39ab1c2bb7fffe93bfda220692664e8be6, so Diego, if your goal (with the reverted change) was more than to avoid seemingly-unnecessary work, please tell us what it was. Well, to put it simply: you *cannot* both have the Python extension *and* disable shared libraries. --disable-shared tells libtool not to build any kind of shared object for the project; Python extensions are shared objects _only_. While you could avoid building libvirt.so (and just have libvirt.a) you cannot get Python extensions by just building libvirtmod.a. So basically --disable-shared --with-python would just produce an unusable output without my change, and produce a proper error condition with. Agreed, if we want to support --disable-shared, then we can't pretend that building python works. Either expect the error we already have which is accurate, or make configure forcably disable the whole python build. I assume Jim want to keep --disable-shared as a convenient way to force libtool to generated binaries statically linked which is easier when dealing with gdb. If that's the case what would be nice is to be able to instruct auto*/libtool to build both libraries (where possible) but force linking with static libraries. I have no idea how to do this but that would be an useful option for autogen or configure. Or just disable python if --disable-shared is passed that would be fine too I think, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] fix build failure with --disable-shared
Il giorno gio, 07/01/2010 alle 22.23 +0100, Daniel Veillard ha scritto: I assume Jim want to keep --disable-shared as a convenient way to force libtool to generated binaries statically linked which is easier when dealing with gdb. If that's the case what would be nice is to be able to instruct auto*/libtool to build both libraries (where possible) but force linking with static libraries. I have no idea how to do this but that would be an useful option for autogen or configure. Either of two: - pass -static option to LDFLAGS (it should _only_ work on the binaries but I'm not sure about it); - add an option that passes -all-static (libtool flag, _not_ linker flag) to only the LDFLAGS of the binaries; -- Diego Elio Pettenò — “Flameeyes” http://blog.flameeyes.eu/ If you found a .asc file in this mail and know not what it is, it's a GnuPG digital signature: http://www.gnupg.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] fix build failure with --disable-shared
Diego Elio “Flameeyes” Pettenò wrote: Il giorno gio, 07/01/2010 alle 21.29 +0100, Jim Meyering ha scritto: But --disable-shared is useful, and your change broke it. On the usefulness of that I have generally a lot to say, but that's another topic. As such, I can't see how you would be using --disable-shared. Very simple: ./configure --disable-shared --without-python So your change forced all other users of --disable-shared to also configure with --without-python, but did not inform them of the new constraint. BTW, it also rendered build instructions in the FAQ invalid. Does the proposed patch cause you any difficulty? If so, please tell us what/how, or propose an alternate patch. Without my patch you're building (and installing) an useless file with I use --disable-shared only when debugging (and then only sometimes), and certainly never install the result. the default configuration (--enable-shared). And _pretending_ to build Python bindings (that will never work even the slightest) with the configuration you suggested (--disable-shared --with-python). Your really can't get it both ways. You're not considering my use case. So far, whenever I've used --disable-shared, whether python support is usable has been irrelevant to me. As Daniel (Berrange) said, the other option is to check whether shared libraries are enabled by libtool and either disable Python bindings or bail out earlier (configure rather than make) with the impossible combination of no-shared-libraries but-python-bindings. I'll make --disable-shared imply --without-python and silently override --with-python. Here's what I'll squash into the posted patch tomorrow: diff --git a/configure.in b/configure.in index 3f2f8ff..25df7a4 100644 --- a/configure.in +++ b/configure.in @@ -1813,6 +1813,10 @@ AC_ARG_WITH([qemu-group], AC_DEFINE_UNQUOTED([QEMU_USER], [$QEMU_USER], [QEMU user account]) AC_DEFINE_UNQUOTED([QEMU_GROUP], [$QEMU_GROUP], [QEMU group account]) +if test $enable_shared:$with_python = no:yes; then + AC_MSG_WARN([you've selected --disable-shared; so disabling python support]) + with_python=no +fi # Only COPYING.LIB is under version control, yet COPYING # is included as part of the distribution tarball. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] fix build failure with --disable-shared
Il giorno gio, 07/01/2010 alle 23.03 +0100, Jim Meyering ha scritto: So your change forced all other users of --disable-shared to also configure with --without-python, but did not inform them of the new constraint. BTW, it also rendered build instructions in the FAQ invalid. I don't want to sound defensive, but I *do* feel attacked a bit here, and I don't think I made any mistake in my patch and suggestion. If I did “force” users to configure properly, it was because the build system was working on a wrong assumption before. Sorry if I made it known that there is a bug there. As for the build instructions, I had to look them up now because I definitely hadn't looked at them; if I did, I would have et you know earlier about the fact that it's a *bad* advice to give. Why? Because I guess quite a few people interested in using libvirt would also be using virt-manager (especially since Daniel said that the XML files are not designed for user to edit, but for software to mangle) and that only works with proper Python bindings — and they are not generated with the suggested syntax in the FAQ. It actually can create quite a bad thing if the user tries to be smart and install the CVS (CVS? I thought it was developed in GIT) versions with make install after the configure, as the bindings and the virsh/libvirtd executables will be using very different libraries. Do I ask too much if next time, rather than noticing by chance that somebody wants to revert a change I proposed, you would reply to my own posting (which I watch more closely), letting me know it broke something? I'm pretty sure I can fix up things from there myself. -- Diego Elio Pettenò — “Flameeyes” http://blog.flameeyes.eu/ If you found a .asc file in this mail and know not what it is, it's a GnuPG digital signature: http://www.gnupg.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Replace old CVS references with GIT
--- HACKING |6 +++--- docs/FAQ.html.in|2 +- docs/bugs.html.in |6 +++--- docs/contact.html.in|4 ++-- docs/deployment.html.in |4 ++-- docs/hacking.html.in|6 +++--- src/Makefile.am |2 +- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/HACKING b/HACKING index 3fb1113..0c65dad 100644 --- a/HACKING +++ b/HACKING @@ -15,14 +15,14 @@ should work: or: - cvs diff -up libvirt-myfeature.patch + git diff libvirt-myfeature.patch (3) Split large changes into a series of smaller patches, self-contained if possible, with an explanation of each patch and an explanation of how the sequence of patches fits together. -(4) Make sure your patches apply against libvirt CVS. Developers -only follow CVS and don't care much about released versions. +(4) Make sure your patches apply against libvirt GIT. Developers +only follow GIT and don't care much about released versions. (5) Run the automated tests on your code before submitting any changes. In particular, configure with compile warnings set to -Werror: diff --git a/docs/FAQ.html.in b/docs/FAQ.html.in index a436e78..50f798d 100644 --- a/docs/FAQ.html.in +++ b/docs/FAQ.html.in @@ -119,7 +119,7 @@ packages as well as the public headers to compile against libxenstore./p /li li -emI use the CVS version and there is no configure script/em +emI use the GIT version and there is no configure script/em pThe configure script (and other Makefiles) are generated. Use the autogen.sh script to regenerate the configure script and Makefiles, like:/p diff --git a/docs/bugs.html.in b/docs/bugs.html.in index 380bcff..fa557a8 100644 --- a/docs/bugs.html.in +++ b/docs/bugs.html.in @@ -16,7 +16,7 @@ p If you are using official libvirt binaries from a Linux distribution check below for distribution specific bug reporting policies first. - For general libvirt bug reports, from self-built releases, CVS snapshots + For general libvirt bug reports, from self-built releases, GIT snapshots and any other non-distribution supported builds, enter tickets under the codeVirtualization Tools/code product and the codelibvirt/code component. @@ -64,8 +64,8 @@ /p ul - liThe version number of the libvirt build, or date of the CVS -checkout/li + liThe version number of the libvirt build, or SHA1 of the GIT +commit/li liThe hardware architecture being used/li liThe name of the hypervisor (Xen, QEMU, KVM)/li liThe XML config of the guest domain if relevant/li diff --git a/docs/contact.html.in b/docs/contact.html.in index 4b9f532..5d055b1 100644 --- a/docs/contact.html.in +++ b/docs/contact.html.in @@ -12,8 +12,8 @@ a href=https://www.redhat.com/mailman/listinfo/libvir-list;associated Web/a page and follow the instructions. Patches with explanations and provided as attachments are really appreciated and will be discussed on the mailing list. - If possible generate the patches by using codecvs diff -up/code in a CVS - checkout. + If possible generate the patches by using codegit diff/code in a GIT + clone. /p h2IRC discussion/h2 diff --git a/docs/deployment.html.in b/docs/deployment.html.in index 47c4892..94c480f 100644 --- a/docs/deployment.html.in +++ b/docs/deployment.html.in @@ -25,10 +25,10 @@ # make install /pre -h2Built from CVS / GIT/h2 +h2Built from GIT/h2 p - When building from CVS it is necessary to generate the autotools + When building from GIT it is necessary to generate the autotools support files. This requires having codeautoconf/code, codeautomake/code, codelibtool/code and codeintltool/code installed. The process can be automated with the codeautogen.sh/code diff --git a/docs/hacking.html.in b/docs/hacking.html.in index af63411..43a79f7 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -19,13 +19,13 @@ or: /p pre - cvs diff -up libvirt-myfeature.patch + git diff libvirt-myfeature.patch /pre/li liSplit large changes into a series of smaller patches, self-contained if possible, with an explanation of each patch and an explanation of how the sequence of patches fits together./li - liMake sure your patches apply against libvirt CVS. Developers -only follow CVS and don't care much about released versions./li + liMake sure your patches apply against libvirt GIT. Developers +only follow GIT and don't care much about released versions./li lipRun the automated tests on your code before submitting any changes. In particular, configure with compile warnings set to -Werror:/p pre diff --git a/src/Makefile.am b/src/Makefile.am index 8ef0e81..dbf708b 100644 --- a/src/Makefile.am +++
Re: [libvirt] [virt-tools-list] Questions about virt-manager running on Arch of Itanium 64
Date: Thu, 7 Jan 2010 09:15:32 -0500 From: crobi...@redhat.com To: x_k_...@hotmail.com CC: berra...@redhat.com; libvirt-l...@redhat.com Subject: Re: [libvirt] [virt-tools-list] Questions about virt-manager running on Arch of Itanium 64 On 01/07/2010 01:03 AM, Dustin Xiong wrote: I sloved the problem. I modify the libvirt /src/qemu_conf.c. Add the arch ia64 into the static const struct qemu_arch_info const arch_info_hvm[] = {} as berrange said. So the libvirt passing -M ia64. But the kvm binary only experts '-M itanium'. So i replaced the ia64 by itanium in the file /src/qemu_conf.c. Then the virt-manager finally could work on the arch of Itanium. Thank you for your help. Thank you, everyone here. -Dustin Can you provide a diff of your changes so we can apply it to upstream libvirt code? Thanks, Cole OK. But there is a problem. I modify the libvirt-0.6.3-20.el5.ia64.src.rpm(from the rhel-server-5.4-ia64-source.iso). The libvirt could work.But when i download the libvirt-0.6.3-20.tar.gz, then ./configure, make, make install. The libvirt couldn't work. The error as below: [r...@kvm bin]# ./virsh error: unable to connect to '/usr/local/libvirt/var/run/libvirt/libvirt-sock': No such file or directory error: failed to connect to the hypervisor Then I download the lastest libvirt-0.7.5, make it. The error still remains. So if you need I provide a diff of my changes about the libvirt-0.6.3-20.el5.ia64.src.rpm, I am pleasure to do this. -Dustin _ Windows Live: Keep your friends up to date with what you do online. http://www.microsoft.com/middleeast/windows/windowslive/see-it-in-action/social-network-basics.aspx?ocid=PID23461::T:WLMTAGL:ON:WL:en-xm:SI_SB_1:092010-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Add virRunWithHook.
Similar to virExecWithHook, but waits for child to exit. Useful for doing things like setuid after the fork but before the exec. --- src/util/util.c | 25 ++--- src/util/util.h |3 +++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 44a4b2f..1d493de 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -792,9 +792,11 @@ error: * only if the command could not be run. */ int -virRun(virConnectPtr conn, - const char *const*argv, - int *status) { +virRunWithHook(virConnectPtr conn, + const char *const*argv, + virExecHook hook, + void *data, + int *status) { pid_t childpid; int exitstatus, execret, waitret; int ret = -1; @@ -811,7 +813,7 @@ virRun(virConnectPtr conn, if ((execret = __virExec(conn, argv, NULL, NULL, childpid, -1, outfd, errfd, - VIR_EXEC_NONE, NULL, NULL, NULL)) 0) { + VIR_EXEC_NONE, hook, data, NULL)) 0) { ret = execret; goto error; } @@ -867,9 +869,11 @@ virRun(virConnectPtr conn, #else /* __MINGW32__ */ int -virRun(virConnectPtr conn, - const char *const *argv ATTRIBUTE_UNUSED, - int *status) +virRunWithHook(virConnectPtr conn, + const char *const *argv ATTRIBUTE_UNUSED, + virExecHook hook ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED, + int *status) { if (status) *status = ENOTSUP; @@ -895,6 +899,13 @@ virExec(virConnectPtr conn, #endif /* __MINGW32__ */ +int +virRun(virConnectPtr conn, + const char *const*argv, + int *status) { +return virRunWithHook(conn, argv, NULL, NULL, status); +} + /* Like gnulib's fread_file, but read no more than the specified maximum number of bytes. If the length of the input is = max_len, and upon error while reading that data, it works just like fread_file. */ diff --git a/src/util/util.h b/src/util/util.h index d556daa..5e70038 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -81,6 +81,9 @@ int virExec(virConnectPtr conn, int *errfd, int flags) ATTRIBUTE_RETURN_CHECK; int virRun(virConnectPtr conn, const char *const*argv, int *status) ATTRIBUTE_RETURN_CHECK; +int virRunWithHook(virConnectPtr conn, const char *const*argv, + virExecHook hook, void *data, + int *status) ATTRIBUTE_RETURN_CHECK; int virFileReadLimFD(int fd, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK; -- 1.6.6 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Some new utility functions
I'm planning to use these to fix the problems with creating storage volumes on root-squashing NFS servers, but since they may be of general utility I thought I'd throw them out for comments beforehand - it something different would make them more generally useful, better to change it now than after I've already finished code to use them. My plan is to use virRunWithHook() to call setuid (or possibly some sort of capng calisthenics) prior to execing qemu-img and qcow-create. virFileCreate and virDirCreate will be used when creating raw volumes. I have written test code to verify that virFileCreate works in several different scenarios. Haven't done so with virRun yet, but it's a fairly simple change, just exposing __virExec arguments that were already there. (I'd previously written test code (yes, it worked ;-)) that used a virRun with uid/gid args added on, but have decided against that approach, since this seems more flexible) -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] New utility functions virFileCreate and virDirCreate
These functions create a new file or directory with the given uid/gid. If the flag VIR_FILE_CREATE_AS_UID is given, they do this by forking a new process, calling setuid/setgid in the new process, and then creating the file. This is better than simply calling open then fchown, because in the latter case, a root-squashing nfs server would create the new file as user nobody, then refuse to allow fchown. If VIR_FILE_CREATE_AS_UID is not specified, the simpler tactic of creating the file/dir, then chowning is is used. This gives better results in cases where the parent directory isn't on a root-squashing NFS server, but doesn't give permission for the specified uid/gid to create files. (Note that if the fork/setuid method fails to create the file due to access privileges, the parent process will make a second attempt using this simpler method.) Return from both of these functions is 0 on success, or the value of errno if there was a failure. --- src/util/util.c | 247 +++ src/util/util.h |9 ++ 2 files changed, 256 insertions(+), 0 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 1d493de..d9a8e6e 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1126,6 +1126,253 @@ int virFileExists(const char *path) return(0); } + +static int virFileCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t gid) { +int fd = -1; +int ret = 0; + +if ((fd = open(path, O_RDWR | O_CREAT | O_EXCL, mode)) 0) { +ret = errno; +virReportSystemError(NULL, errno, _(failed to create file %s), + path); +goto error; +} +if ((getuid() == 0) ((uid != 0) || (gid != 0))) { +if (fchown(fd, uid, gid) 0) { +ret = errno; +virReportSystemError(NULL, errno, _(cannot chown %s to (%u, %u)), + path, uid, gid); +close(fd); +goto error; +} +} +if (close(fd) 0) { +ret = errno; +virReportSystemError(NULL, errno, _(failed to close new file %s), + path); +goto error; +} +fd = -1; +error: +return ret; +} + +static int virDirCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t gid) { +int ret = 0; + +if (mkdir(path, mode) 0) { +ret = errno; +virReportSystemError(NULL, errno, _(failed to create directory %s), + path); +goto error; +} + +if ((getuid() == 0) ((uid != 0) || (gid != getgid( { +if (chown(path, uid, gid) 0) { +ret = errno; +virReportSystemError(NULL, errno, _(cannot chown %s to (%u, %u)), + path, uid, gid); +goto error; +} +} +error: +return ret; +} + +#ifndef WIN32 +int virFileCreate(const char *path, mode_t mode, + uid_t uid, gid_t gid, unsigned int flags) { +pid_t pid; +int waitret, status, ret = 0; +int fd = -1; + +if ((!(flags VIR_FILE_CREATE_AS_UID)) +|| (getuid() != 0) +|| ((uid == 0) (gid == 0))) { +return virFileCreateSimple(path, mode, uid, gid); +} + +/* parent is running as root, but caller requested that the + * file be created as some other user and/or group). The + * following dance avoids problems caused by root-squashing + * NFS servers. */ + +if ((pid = fork()) 0) { +ret = errno; +virReportSystemError(NULL, errno, + _(cannot fork o create file '%s'), path); +return ret; +} + +if (pid) { /* parent */ +/* wait for child to complete, and retrieve its exit code */ +while ((waitret = waitpid(pid, status, 0) == -1) +(errno == EINTR)); +if (waitret == -1) { +ret = errno; +virReportSystemError(NULL, errno, + _(failed to wait for child creating '%s'), + path); +return ret; +} +ret = WEXITSTATUS(status); +if (!WIFEXITED(status) || (ret == EACCES)) { +/* fall back to the simpler method, which works better in + * some cases */ +/* Should we log a warning here? */ +return virFileCreateSimple(path, mode, uid, gid); +} +if ((ret == 0) (gid != 0)) { +/* check if group was set properly by creating after + * setgid. If not, try doing it with chown */ +struct stat st; +if (stat(path, st) == -1) { +ret = errno; +virReportSystemError(NULL, errno, + _(stat of '%s' failed), + path); +return ret; +} +if ((st.st_gid != gid) (chown(path, -1, gid) 0)) { +ret = errno; +
Re: [libvirt] [Patch v0.5] iSCSI Multi-IQN (Libvirt Support)
Please review. -Original Message- From: Iyer, Shyam Sent: Monday, December 28, 2009 2:53 PM To: libvir-list@redhat.com Cc: Bellad, Sudhir; Domsch, Matt; KM, Paniraja; dal...@redhat.com Subject: [Patch v0.5] iSCSI Multi-IQN (Libvirt Support) This patch set realizes the multi-IQN concept discussed in an earlier thread http://www.mail-archive.com/libvir-list@redhat.com/msg16706.html And here .. http://www.mail-archive.com/libvir- l...@redhat.com/msg17499.html The patch realizes an XML schema like the one below and allows libvirt to read through it to create storage pools. These XMLs when created using a virtualization manager realize unique VM to storage LUN mappings through a single console and thus opening up possibilities for the following scenarios - * possibility of multiple IQNs for a single Guest * option for hypervisor's initiator to use these IQNs on behalf of the guest Change Log from v0.4: 1) Set default tab space to 4(Hopefully this is corrected this time ;) ) 2) Review comments from Dave Allan a) Use output of iscsiadm -m iface to search for existing iqn names in the iface files. 3) Create new unique iface file names for user provided iqn names if the iqns are not present in the existing iface files. Change Log from v0.3: 1) Set default tab space to 4 2) Use Case Description for Commit Log 3) Review comments from Dave Allan a) No initiator iqn in the xml would mean use of the default initiator iqn name b) Initiator iqn provided would mean a unique session to be created using the provided iqn name. c) Single iSCSI session per pool 4) Added syntax in doc/schemas/storagepool.rng There are no new errors introduced by this patch with make check and make syntax-check tests. Signed-off-by: Sudhir Bellad sudhir_bel...@dell.com Signed-off-by: Shyam Iyer shyam_i...@dell.com -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list