[libvirt] RFC: automatic setting of ip_forwarding (or not)
by Laine Stump
Currently libvirt will turn on net.ipv4.ip_forward by writing "1\n" to
/proc/sys/net/ipv4/ip_forward whenever a virtual network of with a
forward mode of "nat" or "route" is started. This is problematic for two
reasons: 1) /etc/sysctl.conf is not updated with this information, so
any other process reprocessing /etc/sysctl.conf (with "sysctl -a -p")
will potentially turn ip forward back to 0, leaving libvirt-created
virtual networks in a non-working state, and 2) it's possible the
administrator turned off ip forwarding on purpose for security reasons,
and our silently turning it on leaves them mistakenly believing it is
still off.
We've discussed a few ways of remedying this situation lately, and I
thought I should summarize all the mentioned ideas, and take a poll to
try and determine which way we should fix this.
1) Leave it as is. The simplest solution, but has the problems outlines
above.
2) Turn it on in the same place, but do it by writing
net.ipv4.ip_forward = 1
to /etc/sysctl.conf and calling "sysctl -a -p". This gives us the same
behavior as currently, but with the advantages that a) our change to the
config is documented in /etc/sysctl.conf and b) virtual networked guests
won't suddenly have their network fail when some other process runs
"sysconfig -a -p".
However, it seems rather drastic to be turning this on every time a
virtual network is started, especially without alerting the admin that
this has been done.
3) Whenever a virtual network that would require ip_forward = 1 to
operate properly is started (ie at libvirtd start time, and when a
network is newly defined), check if it's currently on, and if not log a
warning message, informing the admin that they should turn on ip_forward
in sysctl.conf and reload it in order to have properly working networking.
This would assure that the admin was informed of the necessity for
ip_forward, but eliminate the possibility of two processes fighting over
the setting of ip_forward, leaving it up to the admin to make the
decision and do the right thing. On the other hand, it would prevent
libvirt's networking from "just working" on a new install.
4) Turn ip_forward on during libvirt install.
This one doesn't make sense to me, because you don't know at the time of
libvirt install whether or not the installation if going to end up with
any virtual networks that need forwarding.
5) Make ip_forward a tunable in /etc/libvirt/libvirtd.conf, and set it
accordingly every time libvirtd is started.
I don't know if this makes sense either - if you have NATed or routed
virtual networks, you will need ip_forward=1 for them to work properly,
and if you don't have them, you don't care, so it's really redundant.
****
I think the important things to accomplish are:
1) Avoid having networking magically stop working when someone else
reloads sysctl.conf
2) Make sure that the admin realizes that ip_forward is being turned on
(or needs to be turned on).
3) If we're going to turn it on, at least don't do it if it's not needed.
4) Something else we need to consider is the ability to provision a host
for proper guest networking purely through the libvirt API, and if we
were to stop turning on ip_forward automatically when a network was
started, that wouldn't work anymore (unless ip_forward happened to be
turned on already).
So, what are your opinions?
(BTW, the firewall rules added for virtual networks suffer from a
similar problem - because they're loaded into the kernel directly with
the iptables command, there is no external record of them, and some
other process reloading the firewall will flush out all libvirt's rules,
leaving the guests with nonworking networking. But that discussion is a
bigger one, that probably needs to go outside just libvirt, so I'll
avoid that here...)
14 years, 6 months
[libvirt] Suggested schedule for next release
by Daniel Veillard
Last release was 5 weeks ago so this release cycle will be longuer than
usual. There was still a number of patches I wanted to get in for next
release, but now it might be time to get ready for next release.
I suggest to keep until the end of the week for feature patches but
enter the feeze during this week-end. That would lead to a 0.8.5 release
by Friday 29, getting back to end-of-month release cycles.
I'm wondering a bit if the amount of changes at the API level would
not be worth a 0.9.0 release instead,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
14 years, 6 months
[libvirt] Mac OS X: dyld: lazy symbol binding failed
by Mitchell Hashimoto
Hi,
This is a cross-post from libvirt-users since there didn't seem to be
anyone there familiar with what is going on and this is a dev issue as
well.
I'm using the Ruby/FFI libvirt library and getting this consistently
on Mac OS X:
ruby-1.9.2-p0 > FFI::Libvirt.virInitialize
dyld: lazy symbol binding failed: Symbol not found: _virThreadInitialize
Referenced from: /usr/local/lib/libvirt.dylib
Expected in: flat namespace
dyld: Symbol not found: _virThreadInitialize
Referenced from: /usr/local/lib/libvirt.dylib
Expected in: flat namespace
Trace/BPT trap
I exported DYLD_PRINT_LIBRARIES and it shows that libvirt.dylib is
loaded in the process space, but the above still occurs. When I
statically link into a C program it works fine, however. Anyone have
any idea what could be causing this?
See this archive if you'd like to catch up on what was said in
libvirt-users (its not a long thread):
https://www.redhat.com/archives/libvirt-users/2010-October/msg00050.html
Thanks,
Mitchell
14 years, 6 months
[libvirt] alignment of data fields when compiling with mingwin
by arnaud.champion＠devatom.fr
?Hi,
I'm currently working on libvirt csharp bindings, and I have some trouble with virConnectOpenAuth marshaling.
I need to know what is the alignment of data fields in structure when compiling with mingwin.
Anyone know that ?
cheers,
Arnaud Champion
14 years, 6 months
[libvirt] [PATCH] esx: Handle Windows-1252 encoded VMX files
by Matthias Bolte
ESX(i) uses UTF-8, but a Windows based GSX server writes
Windows-1252 encoded VMX files.
---
src/esx/esx_util.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
src/esx/esx_util.h | 2 ++
src/esx/esx_vmx.c | 33 +++++++++++++++++++++++++++++++++
tests/esxutilstest.c | 41 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 121 insertions(+), 3 deletions(-)
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 5fe72fc..b3c927a 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -612,9 +612,9 @@ esxUtil_ReformatUuid(const char *input, char *output)
unsigned char uuid[VIR_UUID_BUFLEN];
if (virUUIDParse(input, uuid) < 0) {
- ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR,
- _("Could not parse UUID from string '%s'"),
- input);
+ ESX_ERROR(VIR_ERR_INTERNAL_ERROR,
+ _("Could not parse UUID from string '%s'"),
+ input);
return -1;
}
@@ -819,3 +819,45 @@ esxUtil_EscapeDatastoreItem(const char *string)
return escaped2;
}
+
+
+
+char *
+esxUtil_ConvertWindows1252ToUTF8(const char *string)
+{
+ char *result = NULL;
+ xmlCharEncodingHandlerPtr handler;
+ xmlBufferPtr windows1252;
+ xmlBufferPtr utf8;
+
+ handler = xmlFindCharEncodingHandler("Windows-1252");
+
+ if (handler == NULL) {
+ ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("libxml2 doesn't handle Windows-1252 encoding"));
+ return NULL;
+ }
+
+ windows1252 = xmlBufferCreateStatic((char *)string, strlen(string));
+ utf8 = xmlBufferCreate();
+
+ if (xmlCharEncInFunc(handler, utf8, windows1252) < 0) {
+ ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Could not convert from Windows-1252 to UTF-8 encoding"));
+ goto cleanup;
+ }
+
+ result = strdup((const char *)xmlBufferContent(utf8));
+
+ if (result == NULL) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ cleanup:
+ xmlCharEncCloseFunc(handler);
+ xmlBufferFree(windows1252);
+ xmlBufferFree(utf8);
+
+ return result;
+}
diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h
index 669a4f2..9906459 100644
--- a/src/esx/esx_util.h
+++ b/src/esx/esx_util.h
@@ -89,4 +89,6 @@ void esxUtil_ReplaceSpecialWindowsPathChars(char *string);
char *esxUtil_EscapeDatastoreItem(const char *string);
+char *esxUtil_ConvertWindows1252ToUTF8(const char *string);
+
#endif /* __ESX_UTIL_H__ */
diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c
index 7dc8e60..016cb75 100644
--- a/src/esx/esx_vmx.c
+++ b/src/esx/esx_vmx.c
@@ -868,6 +868,8 @@ esxVMX_ParseConfig(esxVMX_Context *ctx, virCapsPtr caps, const char *vmx,
{
bool success = false;
virConfPtr conf = NULL;
+ char *encoding = NULL;
+ char *utf8;
virDomainDefPtr def = NULL;
long long config_version = 0;
long long virtualHW_version = 0;
@@ -895,6 +897,36 @@ esxVMX_ParseConfig(esxVMX_Context *ctx, virCapsPtr caps, const char *vmx,
return NULL;
}
+ /* vmx:.encoding */
+ if (esxUtil_GetConfigString(conf, ".encoding", &encoding, true) < 0) {
+ goto cleanup;
+ }
+
+ if (encoding == NULL || STRCASEEQ(encoding, "UTF-8")) {
+ /* nothing */
+ } else if (STRCASEEQ(encoding, "Windows-1252")) {
+ virConfFree(conf);
+
+ utf8 = esxUtil_ConvertWindows1252ToUTF8(vmx);
+
+ if (utf8 == NULL) {
+ goto cleanup;
+ }
+
+ conf = virConfReadMem(utf8, strlen(utf8), VIR_CONF_FLAG_VMX_FORMAT);
+
+ VIR_FREE(utf8);
+
+ if (conf == NULL) {
+ goto cleanup;
+ }
+ } else {
+ ESX_ERROR(VIR_ERR_INTERNAL_ERROR,
+ _("VMX file has unsupported encoding '%s'"), encoding);
+ goto cleanup;
+ }
+
+ /* Allocate domain def */
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
return NULL;
@@ -1359,6 +1391,7 @@ esxVMX_ParseConfig(esxVMX_Context *ctx, virCapsPtr caps, const char *vmx,
}
virConfFree(conf);
+ VIR_FREE(encoding);
VIR_FREE(sched_cpu_affinity);
VIR_FREE(guestOS);
diff --git a/tests/esxutilstest.c b/tests/esxutilstest.c
index d4042c2..5f3b6f2 100644
--- a/tests/esxutilstest.c
+++ b/tests/esxutilstest.c
@@ -280,6 +280,46 @@ testEscapeDatastoreItem(const void *data ATTRIBUTE_UNUSED)
+struct testWindows1252ToUTF8 {
+ const char *windows1252;
+ const char *utf8;
+};
+
+static struct testWindows1252ToUTF8 windows1252ToUTF8[] = {
+ { "normal", "normal" },
+ { /* "A€Z" */ "A\200Z", "A\342\202\254Z" },
+ { /* "Aä1ö2ü3ß4#5~6!7§8/9%Z" */ "A\3441\3662\3743\3374#5~6!7\2478/9%Z",
+ "A\303\2441\303\2662\303\2743\303\2374#5~6!7\302\2478/9%Z" },
+ { /* "hÀÁÂÃÄÅH" */ "h\300\301\302\303\304\305H",
+ "h\303\200\303\201\303\202\303\203\303\204\303\205H" },
+};
+
+static int
+testConvertWindows1252ToUTF8(const void *data ATTRIBUTE_UNUSED)
+{
+ int i;
+ char *utf8 = NULL;
+
+ for (i = 0; i < ARRAY_CARDINALITY(windows1252ToUTF8); ++i) {
+ VIR_FREE(utf8);
+
+ utf8 = esxUtil_ConvertWindows1252ToUTF8(windows1252ToUTF8[i].windows1252);
+
+ if (utf8 == NULL) {
+ return -1;
+ }
+
+ if (STRNEQ(windows1252ToUTF8[i].utf8, utf8)) {
+ VIR_FREE(utf8);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+
+
static int
mymain(int argc, char **argv)
{
@@ -312,6 +352,7 @@ mymain(int argc, char **argv)
DO_TEST(ParseDatastorePath);
DO_TEST(ConvertDateTimeToCalendarTime);
DO_TEST(EscapeDatastoreItem);
+ DO_TEST(ConvertWindows1252ToUTF8);
return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
1.7.0.4
14 years, 6 months
[libvirt] KVM Windows guest: add hard drive
by Mihamina Rakotomandimby
Manao ahoana, Hello, Bonjour,
I installed a Windows (XP and 2003) guest using one partition as hard
drive.
I used:
sudo virt-install --connect qemu:///system \
--name win2003-01 \
--ram 1024 \
--keymap=fr \
--disk path=/dev/lvm0/win2003-01 \
--cdrom=/home/mihamina/Windows_2003_Server.iso \
--os-type windows \
--os-variant=win2k3 \
--noapic \
--noacpi \
--network=bridge:br0 \
--accelerate \
--vnc \
--force
And this works fine. Really fine.
The disk section in the definition:
<disk type='block' device='disk'>
<source dev='/dev/lvm0/win2003-01'/>
<target dev='hda' bus='ide'/>
</disk>
After installation, people asked me to add another hard drive to this
installation, and then, I added another "disk" in the definition:
<disk type='block' device='disk'>
<source dev='/dev/lvm0/win2003-01'/>
<target dev='hda' bus='ide'/>
</disk>
<disk type='block' device='disk'>
<source dev='/dev/lvm0/win2003-01-a'/>
<target dev='hdb' bus='ide'/>
</disk>
I destroyed + undefined + defined + started the VM and the definition
is OK (dumpxml), but the Windows guest doesnt see any new hard drive.
I already tried with replacing "ide" with "virtio", no new hard drive
yet.
This is my configuration:
mihamina@kvm-lxc-02:~$ dpkg -l | grep "virt"
ii kvm 72+dfsg-5~lenny5 Full virtualization on x86 hardware
ii libvirt-bin 0.4.6-10 the programs for the libvirt library
ii libvirt0 0.4.6-10 library for interfacing with different virtualization systems
ii python-libvirt 0.4.6-10 libvirt Python bindings
ii virt-manager 0.6.0-6 desktop application for managing virtual machines
ii virt-viewer 0.0.3-2 Displaying the graphical console of a virtual machine
ii virtinst 0.400.0-7 Programs to create and clone virtual machines
How to make, with this, new hard drive recognized by the Windows guest?
Misaotra, Thanks, Merci.
--
Architecte Informatique chez Blueline/Gulfsat:
Administration Systeme, Recherche & Developpement
+261 34 56 000 19
14 years, 6 months
[libvirt] [PATCH] esx: Fix check in esxDomainGetInfo's perf metric handling
by Matthias Bolte
---
src/esx/esx_driver.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 8bc3be2..1b4ee29 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2329,15 +2329,17 @@ esxDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
perfEntityMetric =
esxVI_PerfEntityMetric_DynamicCast(perfEntityMetricBase);
- if (perfMetricIntSeries == NULL) {
- VIR_ERROR0(_("QueryPerf returned object with unexpected type"));
+ if (perfEntityMetric == NULL) {
+ VIR_ERROR(_("QueryPerf returned object with unexpected type '%s'"),
+ esxVI_Type_ToString(perfEntityMetricBase->_type));
}
perfMetricIntSeries =
esxVI_PerfMetricIntSeries_DynamicCast(perfEntityMetric->value);
if (perfMetricIntSeries == NULL) {
- VIR_ERROR0(_("QueryPerf returned object with unexpected type"));
+ VIR_ERROR(_("QueryPerf returned object with unexpected type '%s'"),
+ esxVI_Type_ToString(perfEntityMetric->value->_type));
}
for (; perfMetricIntSeries != NULL;
--
1.7.0.4
14 years, 7 months
[libvirt] [PATCH v2] introduce VIR_CLOSE to be used rather than close()
by Stefan Berger
V2:
- following Eric's suggestions and picking up his code suggestions
Since bugs due to double-closed file descriptors are difficult to track
down in a multi-threaded system, I am introducing the VIR_CLOSE(fd)
macro to help avoid mistakes here.
There are lots of places where close() is being used. In this patch I am
only cleaning up usage of close() in src/conf where the problems were.
I also dare to declare close() as being deprecated in libvirt code base
(HACKING).
Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>
---
HACKING | 17 +++++++++++++
docs/hacking.html.in | 22 +++++++++++++++++
src/Makefile.am | 3 +-
src/conf/domain_conf.c | 15 +++++++----
src/conf/network_conf.c | 7 +++--
src/conf/nwfilter_conf.c | 13 ++++------
src/conf/storage_conf.c | 8 +++---
src/conf/storage_encryption_conf.c | 6 +++-
src/util/files.c | 47
+++++++++++++++++++++++++++++++++++++
src/util/files.h | 45
+++++++++++++++++++++++++++++++++++
10 files changed, 161 insertions(+), 22 deletions(-)
Index: libvirt-acl/src/util/files.c
===================================================================
--- /dev/null
+++ libvirt-acl/src/util/files.c
@@ -0,0 +1,47 @@
+/*
+ * memory.c: safer file handling
+ *
+ * Copyright (C) 2010 IBM Corporation
+ * Copyright (C) 2010 Stefan Berger
+ * Copyright (C) 2010 RedHat, Inc.
+ * Copyright (C) 2010 Eric Blake
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include <config.h>
+
+#include <unistd.h>
+
+#include "files.h"
+
+int virClose(int *fdptr, bool preserve_errno)
+{
+ int saved_errno;
+ int rc;
+
+ if (*fdptr >= 0) {
+ if (preserve_errno)
+ saved_errno = errno;
+ rc = close(*fdptr);
+ *fdptr = -1;
+ if (preserve_errno)
+ errno = saved_errno;
+ } else
+ rc = 0;
+
+ return rc;
+}
Index: libvirt-acl/src/util/files.h
===================================================================
--- /dev/null
+++ libvirt-acl/src/util/files.h
@@ -0,0 +1,45 @@
+/*
+ * files.h: safer file handling
+ *
+ * Copyright (C) 2010 IBM Corporation
+ * Copyright (C) 2010 Stefan Berger
+ * Copyright (C) 2010 RedHat, Inc.
+ * Copyright (C) 2010 Eric Blake
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+
+#ifndef __VIR_FILES_H_
+# define __VIR_FILES_H_
+
+# include <stdbool.h>
+
+# include "internal.h"
+
+
+/* Don't call this directly - use the macros below */
+int virClose(int *fdptr, bool preserve_errno) ATTRIBUTE_RETURN_CHECK;
+
+/* For use on normal paths; caller must check return value,
+ and failure sets errno per close(). */
+# define VIR_CLOSE(FD) virClose(&(FD),false)
+
+/* For use on cleanup paths; errno is unaffected by close,
+ and no return value to worry about. */
+# define VIR_FORCE_CLOSE(FD) ignore_value (virClose(&(FD),true))
+
+#endif /* __VIR_FILES_H */
Index: libvirt-acl/src/Makefile.am
===================================================================
--- libvirt-acl.orig/src/Makefile.am
+++ libvirt-acl/src/Makefile.am
@@ -82,7 +82,8 @@ UTIL_SOURCES = \
util/uuid.c util/uuid.h \
util/util.c util/util.h \
util/xml.c util/xml.h \
- util/virterror.c util/virterror_internal.h
+ util/virterror.c util/virterror_internal.h \
+ util/files.c util/files.h
EXTRA_DIST += util/threads-pthread.c util/threads-win32.c
Index: libvirt-acl/src/conf/domain_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/domain_conf.c
+++ libvirt-acl/src/conf/domain_conf.c
@@ -46,6 +46,7 @@
#include "nwfilter_conf.h"
#include "ignore-value.h"
#include "storage_file.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -6798,7 +6799,7 @@ int virDomainSaveXML(const char *configD
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot save config file '%s'"),
configFile);
@@ -6807,8 +6808,8 @@ int virDomainSaveXML(const char *configD
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
+
VIR_FREE(configFile);
return ret;
}
@@ -7765,10 +7766,14 @@ int virDomainDiskDefForeachPath(virDomai
}
if (virStorageFileGetMetadataFromFD(path, fd, format, &meta) <
0) {
- close(fd);
+ VIR_FORCE_CLOSE(fd);
goto cleanup;
}
- close(fd);
+
+ if (VIR_CLOSE(fd) < 0)
+ virReportSystemError(errno,
+ _("could not close file %s"),
+ path);
if (virHashAddEntry(paths, path, (void*)0x1) < 0) {
virReportOOMError();
Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -45,6 +45,8 @@
#include "nwfilter_conf.h"
#include "domain_conf.h"
#include "c-ctype.h"
+#include "files.h"
+#include "ignore-value.h"
#define VIR_FROM_THIS VIR_FROM_NWFILTER
@@ -2193,7 +2195,7 @@ int virNWFilterSaveXML(const char *confi
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot save config file '%s'"),
configFile);
@@ -2203,9 +2205,7 @@ int virNWFilterSaveXML(const char *confi
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
-
+ VIR_FORCE_CLOSE(fd);
VIR_FREE(configFile);
return ret;
@@ -2604,7 +2604,7 @@ virNWFilterPoolObjSaveDef(virNWFilterDri
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot save config file %s"),
pool->configFile);
@@ -2614,8 +2614,7 @@ virNWFilterPoolObjSaveDef(virNWFilterDri
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
VIR_FREE(xml);
Index: libvirt-acl/src/conf/storage_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/storage_conf.c
+++ libvirt-acl/src/conf/storage_conf.c
@@ -43,6 +43,8 @@
#include "buf.h"
#include "util.h"
#include "memory.h"
+#include "files.h"
+#include "ignore-value.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -1560,7 +1562,7 @@ virStoragePoolObjSaveDef(virStorageDrive
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot save config file %s"),
pool->configFile);
@@ -1570,9 +1572,7 @@ virStoragePoolObjSaveDef(virStorageDrive
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
-
+ VIR_FORCE_CLOSE(fd);
VIR_FREE(xml);
return ret;
Index: libvirt-acl/src/conf/network_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/network_conf.c
+++ libvirt-acl/src/conf/network_conf.c
@@ -43,6 +43,8 @@
#include "util.h"
#include "buf.h"
#include "c-ctype.h"
+#include "files.h"
+#include "ignore-value.h"
#define MAX_BRIDGE_ID 256
#define VIR_FROM_THIS VIR_FROM_NETWORK
@@ -687,7 +689,7 @@ int virNetworkSaveXML(const char *config
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot save config file '%s'"),
configFile);
@@ -697,8 +699,7 @@ int virNetworkSaveXML(const char *config
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_FORCE_CLOSE(fd);
VIR_FREE(configFile);
Index: libvirt-acl/src/conf/storage_encryption_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/storage_encryption_conf.c
+++ libvirt-acl/src/conf/storage_encryption_conf.c
@@ -35,6 +35,8 @@
#include "xml.h"
#include "virterror_internal.h"
#include "uuid.h"
+#include "files.h"
+#include "ignore-value.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -286,12 +288,12 @@ virStorageGenerateQcowPassphrase(unsigne
if (r <= 0) {
virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot read from /dev/urandom"));
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return -1;
}
if (dest[i] >= 0x20 && dest[i] <= 0x7E)
i++; /* Got an acceptable character */
}
- close(fd);
+ VIR_FORCE_CLOSE(fd);
return 0;
}
Index: libvirt-acl/HACKING
===================================================================
--- libvirt-acl.orig/HACKING
+++ libvirt-acl/HACKING
@@ -318,6 +318,23 @@ routines, use the macros from memory.h
VIR_FREE(domain);
+File handling
+=============
+
+Use of the close() API is deprecated in libvirt code base to help avoiding
+double-closing of a filedescriptor. Instead of this API, use the macro from
+files.h
+
+ - eg close a file descriptor
+
+ if (VIR_CLOSE(fd) < 0) {
+ virReportSystemError(errno, _("failed to close file"));
+ }
+
+ - eg close a file descriptor in an error path; this function
+ preserves the error code
+
+ VIR_FORCE_CLOSE(fd)
String comparisons
==================
Index: libvirt-acl/docs/hacking.html.in
===================================================================
--- libvirt-acl.orig/docs/hacking.html.in
+++ libvirt-acl/docs/hacking.html.in
@@ -389,7 +389,29 @@
</pre></li>
</ul>
+ <h2><a name="file_handling">File handling</a></h2>
+ <p>
+ Use of the close() API is deprecated in libvirt code base to help
+ avoiding double-closing of a filedescriptor. Instead of this API,
+ use the macro from files.h
+ </p>
+
+ <ul>
+ <li><p>eg close a file descriptor</p>
+
+<pre>
+ if (VIR_CLOSE(fd) < 0) {
+ virReportSystemError(errno, _("failed to close file"));
+ }
+</pre></li>
+
+ <li><p>eg close a file descriptor in an error path; this function
preserves the error code</p>
+
+<pre>
+ VIR_FORCE_CLOSE(fd);
+</pre></li>
+ </ul>
<h2><a name="string_comparision">String comparisons</a></h2>
14 years, 7 months
[libvirt] [RFC PATCH] virsh: new echo command
by Eric Blake
* tools/virsh.c (cmdEcho): New command.
(commands): Add it.
(vshCmdOptType): Add VSH_OT_ARGV.
(vshCmddefGetData): Special case new opt flag.
(vshCommandOptArgv): New function.
---
Not complete yet, but this shows what I'm thinking of. Adding the
echo command has two benefits:
1. It will let me add unit tests for the recent virsh command line
improvements - echo back arbitrary strings to make sure quoting is as
desired. This part works with what I have here, before I ran out of
time to finish this today.
2. Make it easier for a user on the command line to conver an
arbitrary string into something safe for shell evalution and/or XML
usage, by munging the input in a way that it can be reused in the
desired context. Not yet implemented; hence the RFC.
It exploits the fact that "--" is consumed as the end-of-options,
hence, there is no way for "" to be recognized as a valid option
name, so the only way we can encounter VSH_OT_ARGV is via the
new argv handling, at which point we can handle all remaining
command line arguments.
tools/virsh.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 80 insertions(+), 8 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 89c2e1e..f361658 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -119,11 +119,12 @@ typedef enum {
* vshCmdOptType - command option type
*/
typedef enum {
- VSH_OT_NONE = 0, /* none */
- VSH_OT_BOOL, /* boolean option */
- VSH_OT_STRING, /* string option */
- VSH_OT_INT, /* int option */
- VSH_OT_DATA /* string data (as non-option) */
+ VSH_OT_NONE = 0, /* none */
+ VSH_OT_BOOL, /* boolean option */
+ VSH_OT_STRING, /* string option */
+ VSH_OT_INT, /* int option */
+ VSH_OT_DATA, /* string data (as non-option) */
+ VSH_OT_ARGV /* remaining arguments, opt->name should be "" */
} vshCmdOptType;
/*
@@ -230,6 +231,7 @@ static char *vshCommandOptString(const vshCmd *cmd, const char *name,
static long long vshCommandOptLongLong(const vshCmd *cmd, const char *name,
int *found);
static int vshCommandOptBool(const vshCmd *cmd, const char *name);
+static char *vshCommandOptArgv(const vshCmd *cmd, int count);
#define VSH_BYID (1 << 1)
#define VSH_BYUUID (1 << 2)
@@ -8917,6 +8919,54 @@ cmdPwd(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
#endif
/*
+ * "echo" command
+ */
+static const vshCmdInfo info_echo[] = {
+ {"help", N_("echo arguments")},
+ {"desc", N_("Echo back arguments, possibly with quoting.")},
+ {NULL, NULL}
+};
+
+static const vshCmdOptDef opts_echo[] = {
+ {"shell", VSH_OT_BOOL, 0, N_("escape for shell use")},
+ {"xml", VSH_OT_BOOL, 0, N_("escape for XML use")},
+ {"", VSH_OT_ARGV, 0, N_("arguments to echo")},
+ {NULL, 0, 0, NULL}
+};
+
+/* Exists mainly for debugging virsh, but also handy for adding back
+ * quotes for later evaluation.
+ */
+static int
+cmdEcho (vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd)
+{
+ bool shell = false;
+ bool xml = false;
+ int count = 0;
+ char *arg;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ if (vshCommandOptBool(cmd, "shell"))
+ shell = true;
+ if (vshCommandOptBool(cmd, "xml"))
+ xml = true;
+
+ while ((arg = vshCommandOptArgv(cmd, count)) != NULL) {
+ /* TODO - use buf */
+ if (xml) {
+ /* TODO - use virBufferEscapeString */
+ }
+ if (shell) {
+ /* TODO - add '' and escape embedded ' */
+ }
+ vshPrint(ctl, "%s%s", count ? " " : "", arg);
+ count++;
+ }
+
+ return TRUE;
+}
+
+/*
* "edit" command
*/
static const vshCmdInfo info_edit[] = {
@@ -9545,6 +9595,7 @@ static const vshCmdDef commands[] = {
{"domxml-from-native", cmdDomXMLFromNative, opts_domxmlfromnative, info_domxmlfromnative},
{"domxml-to-native", cmdDomXMLToNative, opts_domxmltonative, info_domxmltonative},
{"dumpxml", cmdDumpXML, opts_dumpxml, info_dumpxml},
+ {"echo", cmdEcho, opts_echo, info_echo},
{"edit", cmdEdit, opts_edit, info_edit},
{"find-storage-pool-sources", cmdPoolDiscoverSources,
opts_find_storage_pool_sources, info_find_storage_pool_sources},
@@ -9707,8 +9758,8 @@ vshCmddefGetData(const vshCmdDef * cmd, int data_ct)
const vshCmdOptDef *opt;
for (opt = cmd->opts; opt && opt->name; opt++) {
- if (opt->type == VSH_OT_DATA) {
- if (data_ct == 0)
+ if (opt->type >= VSH_OT_DATA) {
+ if (data_ct == 0 || opt->type == VSH_OT_ARGV)
return opt;
else
data_ct--;
@@ -9970,6 +10021,27 @@ vshCommandOptBool(const vshCmd *cmd, const char *name)
return vshCommandOpt(cmd, name) ? TRUE : FALSE;
}
+/*
+ * Returns the COUNT argv argument, or NULL after last argument.
+ *
+ * Requires that a VSH_OT_ARGV option with the name "" be last in the
+ * list of supported options in CMD->def->opts.
+ */
+static char *
+vshCommandOptArgv(const vshCmd *cmd, int count)
+{
+ vshCmdOpt *opt = cmd->opts;
+
+ while (opt) {
+ if (opt->def && opt->def->type == VSH_OT_ARGV) {
+ if (count-- == 0)
+ return opt->data;
+ }
+ opt = opt->next;
+ }
+ return NULL;
+}
+
/* Determine whether CMD->opts includes an option with name OPTNAME.
If not, give a diagnostic and return false.
If so, return true. */
--
1.7.2.3
14 years, 7 months
[libvirt] [PATCH] introduce VIR_CLOSE to be used rather than close()
by Stefan Berger
Since bugs due to double-closed filedescriptors are difficult to track
down in a multi-threaded system, I am introducing the VIR_CLOSE(fd)
macro to help avoid mistakes here.
There are lots of places where close() is being used. In this patch I am
only cleaning up usage of close() in src/conf where the problems were.
I also dare to declare close() as being deprecated in libvirt code base
(HACKING). :-)
Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>
---
HACKING | 11 +++++++++++
src/Makefile.am | 3 ++-
src/conf/domain_conf.c | 14 ++++++++------
src/conf/network_conf.c | 9 +++++----
src/conf/nwfilter_conf.c | 17 +++++++++--------
src/conf/storage_conf.c | 9 +++++----
src/conf/storage_encryption_conf.c | 5 +++--
src/util/files.c | 37
+++++++++++++++++++++++++++++++++++++
src/util/files.h | 37
+++++++++++++++++++++++++++++++++++++
9 files changed, 117 insertions(+), 25 deletions(-)
Index: libvirt-acl/src/util/files.c
===================================================================
--- /dev/null
+++ libvirt-acl/src/util/files.c
@@ -0,0 +1,37 @@
+/*
+ * memory.c: safer file handling
+ *
+ * Copyright (C) 2010 IBM Corporation
+ * Copyright (C) 2010 Stefan Berger
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include <unistd.h>
+
+#include "files.h"
+
+int virClose(int *fdptr)
+{
+ int rc;
+
+ if (*fdptr >= 0) {
+ rc = close(*fdptr);
+ *fdptr = -1;
+ } else
+ rc = 0;
+ return rc;
+}
Index: libvirt-acl/src/util/files.h
===================================================================
--- /dev/null
+++ libvirt-acl/src/util/files.h
@@ -0,0 +1,37 @@
+/*
+ * files.h: safer file handling
+ *
+ * Copyright (C) 2010 IBM Corporation
+ * Copyright (C) 2010 Stefan Berger
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+
+#ifndef __VIR_FILES_H_
+# define __VIR_FILES_H_
+
+# include "internal.h"
+
+/* Don't call these directly - use the macros below */
+int virClose(int *fdptr);
+
+# define VIR_CLOSE(FD) \
+ virClose(&(FD))
+
+
+#endif /* __VIR_FILES_H */
+
Index: libvirt-acl/src/Makefile.am
===================================================================
--- libvirt-acl.orig/src/Makefile.am
+++ libvirt-acl/src/Makefile.am
@@ -82,7 +82,8 @@ UTIL_SOURCES = \
util/uuid.c util/uuid.h \
util/util.c util/util.h \
util/xml.c util/xml.h \
- util/virterror.c util/virterror_internal.h
+ util/virterror.c util/virterror_internal.h \
+ util/files.c util/files.h
EXTRA_DIST += util/threads-pthread.c util/threads-win32.c
Index: libvirt-acl/src/conf/domain_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/domain_conf.c
+++ libvirt-acl/src/conf/domain_conf.c
@@ -46,6 +46,7 @@
#include "nwfilter_conf.h"
#include "ignore-value.h"
#include "storage_file.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -6798,17 +6799,18 @@ int virDomainSaveXML(const char *configD
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot save config file '%s'"),
configFile);
- goto cleanup;
+ goto cleanup_free;
}
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_CLOSE(fd);
+
+ cleanup_free:
VIR_FREE(configFile);
return ret;
}
@@ -7765,10 +7767,10 @@ int virDomainDiskDefForeachPath(virDomai
}
if (virStorageFileGetMetadataFromFD(path, fd, format, &meta) <
0) {
- close(fd);
+ VIR_CLOSE(fd);
goto cleanup;
}
- close(fd);
+ VIR_CLOSE(fd);
if (virHashAddEntry(paths, path, (void*)0x1) < 0) {
virReportOOMError();
Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -45,6 +45,7 @@
#include "nwfilter_conf.h"
#include "domain_conf.h"
#include "c-ctype.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_NWFILTER
@@ -2193,19 +2194,19 @@ int virNWFilterSaveXML(const char *confi
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot save config file '%s'"),
configFile);
- goto cleanup;
+ goto cleanup_free;
}
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_CLOSE(fd);
+ cleanup_free:
VIR_FREE(configFile);
return ret;
@@ -2604,19 +2605,19 @@ virNWFilterPoolObjSaveDef(virNWFilterDri
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot save config file %s"),
pool->configFile);
- goto cleanup;
+ goto cleanup_free;
}
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_CLOSE(fd);
+ cleanup_free:
VIR_FREE(xml);
return ret;
Index: libvirt-acl/src/conf/storage_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/storage_conf.c
+++ libvirt-acl/src/conf/storage_conf.c
@@ -43,6 +43,7 @@
#include "buf.h"
#include "util.h"
#include "memory.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -1560,19 +1561,19 @@ virStoragePoolObjSaveDef(virStorageDrive
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot save config file %s"),
pool->configFile);
- goto cleanup;
+ goto cleanup_free;
}
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_CLOSE(fd);
+ cleanup_free:
VIR_FREE(xml);
return ret;
Index: libvirt-acl/src/conf/network_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/network_conf.c
+++ libvirt-acl/src/conf/network_conf.c
@@ -43,6 +43,7 @@
#include "util.h"
#include "buf.h"
#include "c-ctype.h"
+#include "files.h"
#define MAX_BRIDGE_ID 256
#define VIR_FROM_THIS VIR_FROM_NETWORK
@@ -687,19 +688,19 @@ int virNetworkSaveXML(const char *config
goto cleanup;
}
- if (close(fd) < 0) {
+ if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot save config file '%s'"),
configFile);
- goto cleanup;
+ goto cleanup_free;
}
ret = 0;
cleanup:
- if (fd != -1)
- close(fd);
+ VIR_CLOSE(fd);
+ cleanup_free:
VIR_FREE(configFile);
return ret;
Index: libvirt-acl/src/conf/storage_encryption_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/storage_encryption_conf.c
+++ libvirt-acl/src/conf/storage_encryption_conf.c
@@ -35,6 +35,7 @@
#include "xml.h"
#include "virterror_internal.h"
#include "uuid.h"
+#include "files.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -286,12 +287,12 @@ virStorageGenerateQcowPassphrase(unsigne
if (r <= 0) {
virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot read from /dev/urandom"));
- close(fd);
+ VIR_CLOSE(fd);
return -1;
}
if (dest[i] >= 0x20 && dest[i] <= 0x7E)
i++; /* Got an acceptable character */
}
- close(fd);
+ VIR_CLOSE(fd);
return 0;
}
Index: libvirt-acl/HACKING
===================================================================
--- libvirt-acl.orig/HACKING
+++ libvirt-acl/HACKING
@@ -318,6 +318,17 @@ routines, use the macros from memory.h
VIR_FREE(domain);
+File handling
+=============
+
+Use of the close() API is deprecated in libvirt code base to help avoiding
+double-closing of a filedescriptor. Instead of this API, use the macro from
+files.h
+
+ - eg close a filedescriptor
+
+ VIR_CLOSE(fd);
+
String comparisons
==================
14 years, 7 months
