[libvirt] [PATCH 0/7] Make a minimal client only build work on RHEL4

This series makes a few changes to let a minimal client only build of libvirt work on RHEL4. Well aside from the last patch which is nothing todo with RHEL4, but happens to be in my queue

From: "Daniel P. Berrange" <berrange@redhat.com> On RHEL-4 vintage one of the header files is polluted causing a clash between the clone() syscall and the 'clone' parameter in a libvirt driver API Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/driver.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/driver.h b/src/driver.h index f60bf93..0931be2 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1443,7 +1443,7 @@ typedef char * typedef virStorageVolPtr (*virDrvStorageVolCreateXMLFrom) (virStoragePoolPtr pool, const char *xmldesc, - virStorageVolPtr clone, + virStorageVolPtr clonefrom, unsigned int flags); typedef int (*virDrvStorageVolDownload) (virStorageVolPtr vol, -- 1.8.1.4

On 03/07/13 17:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
On RHEL-4 vintage one of the header files is polluted causing a clash between the clone() syscall and the 'clone' parameter in a libvirt driver API
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/driver.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/driver.h b/src/driver.h index f60bf93..0931be2 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1443,7 +1443,7 @@ typedef char * typedef virStorageVolPtr (*virDrvStorageVolCreateXMLFrom) (virStoragePoolPtr pool, const char *xmldesc, - virStorageVolPtr clone, + virStorageVolPtr clonefrom,
libvirt.c uses "clonevol" here.
unsigned int flags); typedef int (*virDrvStorageVolDownload) (virStorageVolPtr vol,
ACK the name in the header isn't that important. Peter

On Thu, Mar 07, 2013 at 06:14:45PM +0100, Peter Krempa wrote:
On 03/07/13 17:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
On RHEL-4 vintage one of the header files is polluted causing a clash between the clone() syscall and the 'clone' parameter in a libvirt driver API
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/driver.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/driver.h b/src/driver.h index f60bf93..0931be2 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1443,7 +1443,7 @@ typedef char * typedef virStorageVolPtr (*virDrvStorageVolCreateXMLFrom) (virStoragePoolPtr pool, const char *xmldesc, - virStorageVolPtr clone, + virStorageVolPtr clonefrom,
libvirt.c uses "clonevol" here.
Good point, i'll make it consistent here 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 :|

From: "Daniel P. Berrange" <berrange@redhat.com> To avoid a clash with daemon() libc API, rename the 'daemon' param in the header file to 'binary'. The source file already uses the name 'binary'. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetclient.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index 99ea263..4204a93 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -37,7 +37,7 @@ virNetClientPtr virNetClientNewUNIX(const char *path, bool spawnDaemon, - const char *daemon); + const char *binary); virNetClientPtr virNetClientNewTCP(const char *nodename, const char *service); -- 1.8.1.4

On 03/07/13 17:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
To avoid a clash with daemon() libc API, rename the 'daemon' param in the header file to 'binary'. The source file already uses the name 'binary'.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetclient.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK Peter

From: "Daniel P. Berrange" <berrange@redhat.com> The loop.h on RHEL4 is broken and cannot be imported. We already detect this in configure as a side-effect of looking for whether LO_FLAGS_AUTOCLEAR is available. We protected the impl with HAVE_DECL_LO_FLAGS_AUTOCLEAR, but not the header import Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virfile.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index eec9bcc..9f79daf 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -32,7 +32,8 @@ #include <unistd.h> #include <dirent.h> -#ifdef __linux__ +#if defined(__linux__) && HAVE_DECL_LO_FLAGS_AUTOCLEAR +# include <sys/types.h> # include <linux/loop.h> # include <sys/ioctl.h> #endif -- 1.8.1.4

On 03/07/13 17:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The loop.h on RHEL4 is broken and cannot be imported. We already detect this in configure as a side-effect of looking for whether LO_FLAGS_AUTOCLEAR is available. We protected the impl with HAVE_DECL_LO_FLAGS_AUTOCLEAR, but not the header import
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virfile.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
ACK. Peter

On 03/07/2013 09:41 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The loop.h on RHEL4 is broken and cannot be imported. We already detect this in configure as a side-effect of looking for whether LO_FLAGS_AUTOCLEAR is available. We protected the impl with HAVE_DECL_LO_FLAGS_AUTOCLEAR, but not the header import
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virfile.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c index eec9bcc..9f79daf 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -32,7 +32,8 @@ #include <unistd.h> #include <dirent.h>
-#ifdef __linux__ +#if defined(__linux__) && HAVE_DECL_LO_FLAGS_AUTOCLEAR +# include <sys/types.h>
Drop the #include <sys/types.h> line, it adds nothing. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Thu, Mar 07, 2013 at 10:42:49AM -0700, Eric Blake wrote:
On 03/07/2013 09:41 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The loop.h on RHEL4 is broken and cannot be imported. We already detect this in configure as a side-effect of looking for whether LO_FLAGS_AUTOCLEAR is available. We protected the impl with HAVE_DECL_LO_FLAGS_AUTOCLEAR, but not the header import
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virfile.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c index eec9bcc..9f79daf 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -32,7 +32,8 @@ #include <unistd.h> #include <dirent.h>
-#ifdef __linux__ +#if defined(__linux__) && HAVE_DECL_LO_FLAGS_AUTOCLEAR +# include <sys/types.h>
Drop the #include <sys/types.h> line, it adds nothing.
Opps, yes that was left over from an attempt to fix the loop.h on rhel4 which failed 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 :|

From: "Daniel P. Berrange" <berrange@redhat.com> The RHEL4 vintage header files do not define GET_VLAN_VID_CMD. Conditionally define it in our source, since the kernel can raise a runtime error if it isn't supported Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virnetdev.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 296871c..4cdbb0c 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -673,6 +673,9 @@ int virNetDevGetIndex(const char *ifname ATTRIBUTE_UNUSED, #if defined(SIOCGIFVLAN) && defined(HAVE_STRUCT_IFREQ) +# ifndef GET_VLAN_VID_CMD +# define GET_VLAN_VID_CMD 9 +# endif int virNetDevGetVLanID(const char *ifname, int *vlanid) { struct vlan_ioctl_args vlanargs = { -- 1.8.1.4

On 03/07/13 17:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The RHEL4 vintage header files do not define GET_VLAN_VID_CMD. Conditionally define it in our source, since the kernel can raise a runtime error if it isn't supported
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virnetdev.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 296871c..4cdbb0c 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -673,6 +673,9 @@ int virNetDevGetIndex(const char *ifname ATTRIBUTE_UNUSED,
#if defined(SIOCGIFVLAN) && defined(HAVE_STRUCT_IFREQ) +# ifndef GET_VLAN_VID_CMD +# define GET_VLAN_VID_CMD 9 +# endif int virNetDevGetVLanID(const char *ifname, int *vlanid) { struct vlan_ioctl_args vlanargs = {
As you are attempting just fixing of the client it might be feasible to do: #if defined(SIOCGIFVLAN) && defined(HAVE_STRUCT_IFREQ) && defined(GET_VLAN_VID_CMD) and let the compiler compile the fallback function that probably won't be used anyways. Additionally if somebody might want to fix the qemu driver to work too, lack of vlan support won't be on the top of the priority list. Peter

On Thu, Mar 07, 2013 at 06:31:30PM +0100, Peter Krempa wrote:
On 03/07/13 17:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The RHEL4 vintage header files do not define GET_VLAN_VID_CMD. Conditionally define it in our source, since the kernel can raise a runtime error if it isn't supported
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virnetdev.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 296871c..4cdbb0c 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -673,6 +673,9 @@ int virNetDevGetIndex(const char *ifname ATTRIBUTE_UNUSED,
#if defined(SIOCGIFVLAN) && defined(HAVE_STRUCT_IFREQ) +# ifndef GET_VLAN_VID_CMD +# define GET_VLAN_VID_CMD 9 +# endif int virNetDevGetVLanID(const char *ifname, int *vlanid) { struct vlan_ioctl_args vlanargs = {
As you are attempting just fixing of the client it might be feasible to do: #if defined(SIOCGIFVLAN) && defined(HAVE_STRUCT_IFREQ) && defined(GET_VLAN_VID_CMD)
and let the compiler compile the fallback function that probably won't be used anyways.
hmm, yes good idea. 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 :|

From: "Daniel P. Berrange" <berrange@redhat.com> The virNetDevSetupControlFull function was protected by a Update the conditionals around all callers to do stricter checks to ensure we always build Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virnetdevbridge.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 3c00be9..9d46cc4 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -46,7 +46,7 @@ #define VIR_FROM_THIS VIR_FROM_NONE -#ifdef SIOCBRADDBR +#if defined(HAVE_STRUCT_IFREQ) && defined(__linux__) static int virNetDevSetupControlFull(const char *ifname, struct ifreq *ifr, int domain, @@ -89,7 +89,7 @@ static int virNetDevSetupControl(const char *ifname, } #endif -#ifdef __linux__ +#if defined(HAVE_STRUCT_IFREQ) && defined(__linux__) # define SYSFS_NET_DIR "/sys/class/net" /* * Bridge parameters can be set via sysfs on newish kernels, @@ -211,7 +211,7 @@ cleanup: * * Returns 0 in case of success or -1 on failure */ -#ifdef SIOCBRADDBR +#if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR) int virNetDevBridgeCreate(const char *brname) { int fd = -1; @@ -249,7 +249,7 @@ int virNetDevBridgeCreate(const char *brname) * * Returns 0 in case of success or an errno code in case of failure. */ -#ifdef SIOCBRDELBR +#if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRDELBR) int virNetDevBridgeDelete(const char *brname) { int fd = -1; @@ -288,7 +288,7 @@ int virNetDevBridgeDelete(const char *brname ATTRIBUTE_UNUSED) * * Returns 0 in case of success or an errno code in case of failure. */ -#ifdef SIOCBRADDIF +#if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDIF) int virNetDevBridgeAddPort(const char *brname, const char *ifname) { @@ -335,7 +335,7 @@ int virNetDevBridgeAddPort(const char *brname, * * Returns 0 in case of success or an errno code in case of failure. */ -#ifdef SIOCBRDELIF +#if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRDELIF) int virNetDevBridgeRemovePort(const char *brname, const char *ifname) { @@ -375,7 +375,7 @@ int virNetDevBridgeRemovePort(const char *brname, #endif -#ifdef __linux__ +#if defined(HAVE_STRUCT_IFREQ) && defined(__linux__) /** * virNetDevBridgeSetSTPDelay: * @brname: the bridge name -- 1.8.1.4

On 03/07/13 17:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virNetDevSetupControlFull function was protected by a
Part of the sentence missing?
Update the conditionals around all callers to do stricter checks to ensure we always build
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virnetdevbridge.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 3c00be9..9d46cc4 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -46,7 +46,7 @@ #define VIR_FROM_THIS VIR_FROM_NONE
SIOCBRADDBR -#ifdef SIOCBRADDBR +#if defined(HAVE_STRUCT_IFREQ) && defined(__linux__)
Doesn't follow the pattern established in the rest of the patch: s/__linux__/SIOCBRADDBR/
static int virNetDevSetupControlFull(const char *ifname, struct ifreq *ifr, int domain,
ACK with the change and commit message fixed. Peter

On Thu, Mar 07, 2013 at 06:34:04PM +0100, Peter Krempa wrote:
On 03/07/13 17:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virNetDevSetupControlFull function was protected by a
Part of the sentence missing?
Update the conditionals around all callers to do stricter checks to ensure we always build
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virnetdevbridge.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 3c00be9..9d46cc4 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -46,7 +46,7 @@ #define VIR_FROM_THIS VIR_FROM_NONE
SIOCBRADDBR -#ifdef SIOCBRADDBR +#if defined(HAVE_STRUCT_IFREQ) && defined(__linux__)
Doesn't follow the pattern established in the rest of the patch: s/__linux__/SIOCBRADDBR/
No, the use of SIOCBRADDBR was flawed - this function is required even in places where SIOCBRADDBR is not defined. HAVE_STRUCT_IFREQ is the commonality in all callers.
static int virNetDevSetupControlFull(const char *ifname, struct ifreq *ifr, int domain,
ACK with the change and commit message fixed.
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 :|

On 03/07/13 19:10, Daniel P. Berrange wrote:
On Thu, Mar 07, 2013 at 06:34:04PM +0100, Peter Krempa wrote:
On 03/07/13 17:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virNetDevSetupControlFull function was protected by a
Part of the sentence missing?
Update the conditionals around all callers to do stricter checks to ensure we always build
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virnetdevbridge.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 3c00be9..9d46cc4 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -46,7 +46,7 @@ #define VIR_FROM_THIS VIR_FROM_NONE
SIOCBRADDBR -#ifdef SIOCBRADDBR +#if defined(HAVE_STRUCT_IFREQ) && defined(__linux__)
Doesn't follow the pattern established in the rest of the patch: s/__linux__/SIOCBRADDBR/
No, the use of SIOCBRADDBR was flawed - this function is required even in places where SIOCBRADDBR is not defined. HAVE_STRUCT_IFREQ is the commonality in all callers.
Okay,
static int virNetDevSetupControlFull(const char *ifname, struct ifreq *ifr, int domain,
ACK with the change and commit message fixed.
just fix the commit message then.
Daniel

From: "Daniel P. Berrange" <berrange@redhat.com> RHEL4 vintage libxml2 header files are missing xmlSaveToBuffer despite the symbol existing in the binary Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tools/virsh-domain.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 96dd4fa..950fc86 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -56,6 +56,17 @@ #include "virtypedparam.h" #include "virxml.h" +/* libxml2 in RHEL4 has this symbol in the binary but it + * is commented out in the header, despite apparently + * working fine. This hacks around that header problem + */ +#ifndef xmlSaveToBuffer +XMLPUBFUN xmlSaveCtxtPtr XMLCALL +xmlSaveToBuffer (xmlBufferPtr buffer, + const char *encoding, + int options); +#endif + /* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO # define SA_SIGINFO 0 -- 1.8.1.4

On 03/07/13 17:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
RHEL4 vintage libxml2 header files are missing xmlSaveToBuffer despite the symbol existing in the binary
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tools/virsh-domain.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 96dd4fa..950fc86 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -56,6 +56,17 @@ #include "virtypedparam.h" #include "virxml.h"
+/* libxml2 in RHEL4 has this symbol in the binary but it + * is commented out in the header, despite apparently + * working fine. This hacks around that header problem + */ +#ifndef xmlSaveToBuffer +XMLPUBFUN xmlSaveCtxtPtr XMLCALL +xmlSaveToBuffer (xmlBufferPtr buffer, + const char *encoding, + int options);
The code where you copied this from uses tabs for indentation breaking the syntax check.
+#endif + /* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO # define SA_SIGINFO 0
I can't say I like this kind of hacks, but the explanation is fair enough. Additionally the only function using that call is unused so it might be better #if 0 it out instead of having the ATTRIBUTE_UNUSED and that hack in the code. Peter

On Thu, Mar 07, 2013 at 06:22:46PM +0100, Peter Krempa wrote:
On 03/07/13 17:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
RHEL4 vintage libxml2 header files are missing xmlSaveToBuffer despite the symbol existing in the binary
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tools/virsh-domain.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 96dd4fa..950fc86 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -56,6 +56,17 @@ #include "virtypedparam.h" #include "virxml.h"
+/* libxml2 in RHEL4 has this symbol in the binary but it + * is commented out in the header, despite apparently + * working fine. This hacks around that header problem + */ +#ifndef xmlSaveToBuffer +XMLPUBFUN xmlSaveCtxtPtr XMLCALL +xmlSaveToBuffer (xmlBufferPtr buffer, + const char *encoding, + int options);
The code where you copied this from uses tabs for indentation breaking the syntax check.
+#endif + /* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO # define SA_SIGINFO 0
I can't say I like this kind of hacks, but the explanation is fair enough. Additionally the only function using that call is unused so it might be better #if 0 it out instead of having the ATTRIBUTE_UNUSED and that hack in the code.
Hmm, why does vshCompleteXMLFromDomain exist at all, if nothgin is calling it. We should just delete it. 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 :|

On 03/07/2013 10:58 AM, Daniel P. Berrange wrote:
Hmm, why does vshCompleteXMLFromDomain exist at all, if nothgin is calling it. We should just delete it.
Ah, that's leftover code from earlier attempts to make virsh canonicalize XML, which we had to revert because it was only a half-baked approach at the time: https://www.redhat.com/archives/libvir-list/2012-February/msg00365.html Until we actually ever get code that works for turning a user's partial description into an unambiguous device XML, deleting the unused function probably won't hurt (after all, we have git history and can revive it). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> In the LXC container startup code when switching stdio streams, we call VIR_FORCE_CLOSE on all FDs. This triggers a huge number of warnings, but we don't see them because stdio is closed at this point. strace() however shows them which can confuse people debugging the code. Switch to VIR_MASS_CLOSE to avoid this Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_container.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 497539c..7ec67f4 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -245,7 +245,7 @@ static int lxcContainerSetStdio(int control, int ttyfd, int handshakefd) for (i = 0; i < open_max; i++) if (i != ttyfd && i != control && i != handshakefd) { int tmpfd = i; - VIR_FORCE_CLOSE(tmpfd); + VIR_MASS_CLOSE(tmpfd); } if (dup2(ttyfd, 0) < 0) { -- 1.8.1.4

On 03/07/13 17:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
In the LXC container startup code when switching stdio streams, we call VIR_FORCE_CLOSE on all FDs. This triggers a huge number of warnings, but we don't see them because stdio is closed at this point. strace() however shows them which can confuse people debugging the code. Switch to VIR_MASS_CLOSE to avoid this
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_container.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK. PEter
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Peter Krempa