Re: [libvirt] [PATCH] add const to file-scoped statics
Daniel P. Berrange [EMAIL PROTECTED] wrote:
On Thu, May 08, 2008 at 04:40:11PM +0200, Jim Meyering wrote:
From fed1a1abfd2b9ece7bd8e44aae13de91eaad4f5d Mon Sep 17 00:00:00 2001
From: Jim Meyering [EMAIL PROTECTED]
Date: Wed, 7 May 2008 23:12:13 +0200
Subject: [PATCH] add const to file-scoped statics
These were relatively new additions to the list from here:
nm src/*.o|grep ' D '.
ACK
Thanks for the quick feedback.
I've pushed that and one more small change corresponding
to this ChangeLog entry:
It adds one more const and gives the struct type a name.
* src/storage_backend_fs.c (fileTypeInfo): Add static and two
const attributes.
(FileTypeInfo): Name the previously anonymous struct,
so the declaration of fileTypeInfo looks more conventional.
* src/xend_internal.c (sound_models): Mark as static, and
as an array of const entries.
These were relatively new additions to the list from here:
nm src/*.o|grep ' D '
From 550f475b6a33aa06e103329696be4f55c1b3c81d Mon Sep 17 00:00:00 2001
From: Jim Meyering [EMAIL PROTECTED]
Date: Fri, 9 May 2008 10:05:58 +0200
Subject: [PATCH] complete the previous change
---
src/storage_backend_fs.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
index b41b952..36f1fb8 100644
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -79,7 +79,7 @@ enum {
};
/* Either 'magic' or 'extension' *must* be provided */
-static const struct {
+struct FileTypeInfo {
int type; /* One of the constants above */
const char *magic; /* Optional string of file magic
* to check at head of file */
@@ -94,7 +94,8 @@ static const struct {
* -1 to use st_size as capacity */
int sizeBytes;/* Number of bytes for size field */
int sizeMultiplier; /* A scaling factor if size is not in bytes */
-} fileTypeInfo[] = {
+};
+const struct FileTypeInfo const fileTypeInfo[] = {
/* Bochs */
/* XXX Untested
{ VIR_STORAGE_VOL_BOCHS, Bochs Virtual HD Image, NULL,
--
1.5.5.1.179.gdc42
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Implement autostart commands for xen
On Thu, May 08, 2008 at 01:49:15PM -0400, Cole Robinson wrote: Cole Robinson wrote: The attached patch implements the domain autostart commands for xen. The xen sexpr (since at least 3.0.4 = 1.5 years) has a on_xend_start field which can be used to autostart a domain. Updated patch with fixes Dan recommended. Okidoc, applied and commited, thanks a lot ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] sytnax-check: add a check for risky ctype macro use
On Thu, May 08, 2008 at 04:41:21PM +0200, Jim Meyering wrote: A follow-up to the patch that introduced to_uchar. This rule will ensure no new offenders sneak back in. Fine by me, even if my own feeling would be to rather check we don't use is* and to* at all since their behaviour is locale dependant and being macros they don't do type checking. But I'm certainly biased on this topic by my XML background :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Implement autostart commands for xen
The attached patch implements the domain autostart commands for xen. The xen sexpr (since at least 3.0.4 = 1.5 years) has a on_xend_start field which can be used to autostart a domain. It is a good patch for me. It moved without a problem when I tested it. By the way, I have a question. What is getAutostart used by? getAutostart seems to be used from no virsh command. Well virt-manager will call it and display the autostart status. I don't know what virsh doesn't use it. It should really show the autostart status when display a 'virsh domiinfo' Thanks! I understand that getAutostart was made to confirm an autostart parameter. thanks, Shigeki Sakamoto. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] avoid one more ctype vs. sign-extension problem
On Thu, May 08, 2008 at 04:44:38PM +0200, Jim Meyering wrote: This change demonstrates that the new syntax-check rule's regexp can be improved. It missed the unsafe tolower use, since there was already a to_uchar use on that line. Since this shows up again, let me argue a bit more why I dislike using is* and to* in libvirt. pedantic topic=string encodings We use to* only for comparing A-F/a-f in ethernet addresses comparisons, so those use are safe. The use of XML as the front end for all text parsed in libvirt also safeguards against what I consider a nasty problems with the use of strings in system code i.e. their dependance to locale. I think it's not acceptable to have a risk of different behaviour if the user locale has changed, this can lead to untractable bugs, and also mean we allowed at a given point the problem of string whose encoding is undefined to occur in the library (in the libxml2 docs I point to [1] and [2] as introductions to the problem). As long as we use XML for input, we are safe IMHO, because any string input will automatically come converted as UTF-8 by the way of libxml2, that means if someone want to use japanese characters for the name of their domain or files, at the libvirt level they can and we should not misparse or break those (it's likely to break down the line in xend, but i would hope that lxc and QEmu would be better at handling this though I don't know how we could enforce the exec'ec programs args to be interpreted as UTF-8 in the forked process, as a library we just can't touch the locale value, but maybe we can do that after fork() and before exec()). But each time we extract a string from libxml2 we should know and keep in mind it is an UTF-8 string. In practice the ethernet MAC addresses strings used with to_lower and to_upper in util.c are UTF-8, not strings in the user locale. We are safe here because most locales match with ASCII on the first 128 values, UTF-8 included, and the characters for the MAC address range cannot conflict for UTF-8 encoding, but from a pure correctness POV using those convenience macros on those UTF-8 strings is a mistake. And I would rather see automated checks on the code look for those mistakes, rather than doing a lookup for a wrong cast to then use a wrong macro with a false sense of security. /pedantic okay now I feel better :-) Daniel [1] http://www.joelonsoftware.com/articles/Unicode.html [2] http://www.tbray.org/ongoing/When/200x/2003/04/06/Unicode -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [Libvir] [PATCH] sound support for qemu and xen
Hi, Cole and Dan I have a question about the reason why it does not support in xen_proxy. Would you explain why it does not support in xen_proxy? This policy comes from security issue or other? If it does already commented on this please show me a pointer. P.S. I raise this question when I see the patch of follows. But I cannot find the reason. Disable sound functions when in proxy http://git.et.redhat.com/?p=libvirt.git;a=commit;h=0f7619f25e7dd03296c95241c1de06953ac110c3 Thanks Atsushi SAKAI Cole Robinson [EMAIL PROTECTED] wrote: The patch below adds xml support for the soundhw option to qemu and xen. The new xml element takes the form: sound driver='drivername'/ Where driver name can be pcspk, sb16, es1370, or all. Everything seems to be in working order but I have a few implementation questions: 1) Should multiple drivers be able to be specified? qemu accommodates this, allowing '-soundhw sb16,pcspk' for example. If this should be allowed, what should the xml format be? 2) Should acceptable driver options be hardcoded? The other option is to just pass the input straight to qemu. This patch has the options hardcoded. Also I realize this will probably need to be rediff'd around Dan's serial + parallel device patch, but I figured I would just get this out there. Thanks, Cole -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] avoid one more ctype vs. sign-extension problem
Daniel Veillard [EMAIL PROTECTED] wrote:
On Thu, May 08, 2008 at 04:44:38PM +0200, Jim Meyering wrote:
This change demonstrates that the new syntax-check rule's
regexp can be improved. It missed the unsafe tolower use,
since there was already a to_uchar use on that line.
Since this shows up again, let me argue a bit more why I dislike
using is* and to* in libvirt.
I agree completely, especially since I've just fixed three
similar bugs in coreutils:
http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/13512
http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=209850fd7
I'm in the process of removing all of them from libvirt, since there
aren't many. Actually, I've just realized that the 'right' way to do
this is to change each use of isspace to c_isspace, isdigit to c_isdigit,
etc., relying on the gnulib c-ctype module:
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/c-ctype.h
Looking into that, I see more room for improvement, i.e., where things
like isdigit and isalnum have been open-coded. With these c_is* functions,
we can have portability, readability, *and* efficiency:
For example, compare these:
while ((*cur = '0') (*cur = '9')) {
while (c_isdigit(*cur)) {
and these:
char_ok =
(model[i] = '0' model[i] = '9') ||
(model[i] = 'a' model[i] = 'z') ||
(model[i] = 'A' model[i] = 'Z') || model[i] == '_';
char_ok = c_isalnum (model[i]) || model[i] == '_';
So I'm making changes like these, too.
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [patch][doc] libvirtd description
Hi, This patch changes libvirtd description, since libvirtd can control not only Qemu but also Xen and LXC now. Thanks Atsushi SAKAI change_libvirtd_description.patch Description: Binary data -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [Libvir] [PATCH] sound support for qemu and xen
On Fri, May 09, 2008 at 06:33:19PM +0900, Atsushi SAKAI wrote: Hi, Cole and Dan I have a question about the reason why it does not support in xen_proxy. Would you explain why it does not support in xen_proxy? That changeset was just disabling a couple of functions there were not needed in the proxy. The proxy is read-only, so doesn't need ability to create VMs thus those sound functions were disabled. Dan. -- |: Red Hat, Engineering, Boston -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] [patch][doc and comment] typos fix
Hi, I post a typo patch since it reaches 20. docs/apps.html |2 +- docs/apps.html.in|2 +- docs/deployment.html |2 +- docs/deployment.html.in |2 +- docs/index.py|6 +++--- docs/intro.html |2 +- docs/intro.html.in |2 +- docs/library.xen |4 ++-- docs/virsh.pod |2 +- include/libvirt/libvirt.h|2 +- include/libvirt/libvirt.h.in |2 +- proxy/libvirt_proxy.c|2 +- qemud/event.c|2 +- qemud/qemud.c|2 +- qemud/remote.c |2 +- src/event.c |2 +- src/proxy_internal.c |2 +- 17 files changed, 20 insertions(+), 20 deletions(-) Thanks Atsushi SAKAI fix_typos_20080509.patch Description: Binary data -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Use gnulib's c-ctype.h; prohibit use of ctype.h
Following up on this thread:
http://thread.gmane.org/gmane.comp.emulators.libvirt/6338/focus=6349
This change removes all uses of ctype macros and ensures
that no new ones will be added.
The other changes I mentioned will come later.
From 3966a3ebaaacc14b4f330677fd2ed8f8546c6f4a Mon Sep 17 00:00:00 2001
From: Jim Meyering [EMAIL PROTECTED]
Date: Fri, 9 May 2008 12:55:29 +0200
Subject: [PATCH] Use gnulib's c-ctype.h, not ctype.h.
re=$(man isspace|grep is.,.is|sed 's/ -.*//' \
|tr -s ', \n' \||sed 's/^|//;s/|$//')
git grep -l -E $re|grep -Ev 'Chan|gnulib' \
|xargs perl -pi -e 's/\b('$re')\b/c_$1/g'
git grep -l to_uchar|xargs perl -pi -e 's/to_uchar\((.*?)\)/$1/g'
* src/util.h (to_uchar): Remove definition.
(TOLOWER): Remove definition.
(__virMacAddrCompare): Use c_tolower, not TOLOWER.
Globally:
Where needed, change ctype.h to c-ctype.h.
Remove unnecessary inclusion of ctype.h.
Ensure the global changes are never needed again:
* Makefile.maint (sc_avoid_ctype_macros): Prohibit use of ctype
macros. Recommend c-ctype.h instead.
(sc_prohibit_c_ctype_without_use): New rule.
(sc_prohibit_ctype_h): New rule. Disallow use of ctype.h.
---
Makefile.maint | 16 +---
qemud/remote.c |1 -
src/buf.c |3 +--
src/nodeinfo.c | 12 ++--
src/openvz_driver.c |1 -
src/qemu_driver.c |4 ++--
src/sexpr.c |4 ++--
src/stats_linux.c |8
src/util.c | 15 ++-
src/util.h |5 -
src/virsh.c |8
src/xend_internal.c |1 -
12 files changed, 38 insertions(+), 40 deletions(-)
diff --git a/Makefile.maint b/Makefile.maint
index bab8e1d..0f79ed7 100644
--- a/Makefile.maint
+++ b/Makefile.maint
@@ -157,6 +157,16 @@ sc_prohibit_quotearg_without_use:
sc_prohibit_quote_without_use:
@h='quote.h' re='\quote(_n)? *\(' $(_header_without_use)
+# Prohibit the inclusion of c-ctype.h without an actual use.
+sc_prohibit_c_ctype_without_use:
+ @h='[]c-ctype.h[]' re='\c_($(ctype_re)) *\(' $(_header_without_use)
+
+# Prohibit the inclusion of ctype.h.
+sc_prohibit_ctype_h:
+ @grep -E '^# *include *ctype\.h' $$($(VC_LIST_EXCEPT)) \
+ { echo $(ME): don't use ctype.h; instead, use c-ctype.h \
+ 12; exit 1; } || :
+
sc_obsolete_symbols:
@grep -nE '\(HAVE''_FCNTL_H|O''_NDELAY)\' \
$$($(VC_LIST_EXCEPT))\
@@ -305,10 +315,10 @@ sc_TAB_in_indentation:
ctype_re = isalnum|isalpha|isascii|isblank|iscntrl|isdigit|isgraph|islower\
|isprint|ispunct|isspace|isupper|isxdigit|tolower|toupper
-sc_risky_ctype_macros:
+sc_avoid_ctype_macros:
@grep -E '\b($(ctype_re)) *\(' /dev/null\
-$$($(VC_LIST_EXCEPT)) | grep -v to_uchar \
- { echo '$(ME): found ctype macro use without to_uchar'\
+$$($(VC_LIST_EXCEPT))\
+ { echo $(ME): don't use ctype macros (use c-ctype.h)\
12; exit 1; } || :
# Match lines like the following, but where there is only one space
diff --git a/qemud/remote.c b/qemud/remote.c
index e249529..22dd86e 100644
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -41,7 +41,6 @@
#include syslog.h
#include string.h
#include errno.h
-#include ctype.h
#include fnmatch.h
#ifdef HAVE_POLKIT
diff --git a/src/buf.c b/src/buf.c
index 2e17fbb..b56a9c1 100644
--- a/src/buf.c
+++ b/src/buf.c
@@ -1,7 +1,7 @@
/*
* buf.c: buffers for libvirt
*
- * Copyright (C) 2005-2007 Red Hat, Inc.
+ * Copyright (C) 2005-2008 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -16,7 +16,6 @@
#include stdlib.h
#include string.h
#include stdarg.h
-#include ctype.h
#define __VIR_BUFFER_C__
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index c1e8013..e227c69 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -27,7 +27,7 @@
#include string.h
#include stdlib.h
#include errno.h
-#include ctype.h
+#include c-ctype.h
#ifdef HAVE_SYS_UTSNAME_H
#include sys/utsname.h
@@ -59,7 +59,7 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE
*cpuinfo, virNodeInfoPtr n
char *buf = line;
if (STREQLEN(buf, processor, 9)) { /* aka a single logical CPU */
buf += 9;
-while (*buf isspace(to_uchar(*buf)))
+while (*buf c_isspace(*buf))
buf++;
if (*buf != ':') {
__virRaiseError(conn, NULL, NULL, 0, VIR_ERR_INTERNAL_ERROR,
@@ -72,7 +72,7 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE
*cpuinfo, virNodeInfoPtr n
char *p;
unsigned int ui;
buf += 9;
-while (*buf isspace(to_uchar(*buf)))
+while (*buf c_isspace(*buf))
buf++;
if (*buf != ':' || !buf[1]) {
__virRaiseError(conn, NULL, NULL, 0,
Re: [libvirt] [PATCH] avoid one more ctype vs. sign-extension problem
On Fri, May 09, 2008 at 12:13:50PM +0200, Jim Meyering wrote:
Daniel Veillard [EMAIL PROTECTED] wrote:
On Thu, May 08, 2008 at 04:44:38PM +0200, Jim Meyering wrote:
This change demonstrates that the new syntax-check rule's
regexp can be improved. It missed the unsafe tolower use,
since there was already a to_uchar use on that line.
Since this shows up again, let me argue a bit more why I dislike
using is* and to* in libvirt.
I agree completely, especially since I've just fixed three
similar bugs in coreutils:
Ah, good, it's not just the Frenchman being overly stubborn then :-)
http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/13512
http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=209850fd7
I'm in the process of removing all of them from libvirt, since there
aren't many. Actually, I've just realized that the 'right' way to do
this is to change each use of isspace to c_isspace, isdigit to c_isdigit,
etc., relying on the gnulib c-ctype module:
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/c-ctype.h
That looks fine, yes.
Looking into that, I see more room for improvement, i.e., where things
like isdigit and isalnum have been open-coded. With these c_is* functions,
we can have portability, readability, *and* efficiency:
For example, compare these:
while ((*cur = '0') (*cur = '9')) {
while (c_isdigit(*cur)) {
and these:
char_ok =
(model[i] = '0' model[i] = '9') ||
(model[i] = 'a' model[i] = 'z') ||
(model[i] = 'A' model[i] = 'Z') || model[i] == '_';
char_ok = c_isalnum (model[i]) || model[i] == '_';
So I'm making changes like these, too.
great, thanks !
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
[EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [patch][doc] libvirtd description
On Fri, May 09, 2008 at 08:44:14PM +0900, Atsushi SAKAI wrote: Hi, This patch changes libvirtd description, since libvirtd can control not only Qemu but also Xen and LXC now. Looks fine, +1, go for it :-) thanks, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [patch][doc and comment] typos fix
On Fri, May 09, 2008 at 08:52:00PM +0900, Atsushi SAKAI wrote: Hi, I post a typo patch since it reaches 20. okay, thanks except a coupld of things: docs/index.py|6 +++--- -# Create a database user 'veillard' and give him passord access +# Create a database user 'veillard' and give him passed access i really meant password, not passed, wrong pick ;-) proxy/libvirt_proxy.c|2 +- * very fist thing, use the socket as an exclusive lock, this then * allow to do timed exits, avoiding constant CPU usage in case of * failure. - */ +*/ if (proxyListenUnixSocket(PROXY_SOCKET_PATH) 0) exit(0); if (proxyInitXen() == 0) doesn't look like a typo, and i prefer to keep the * aligned vertically But except those, it looks fine to me, please apply :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Use gnulib's c-ctype.h; prohibit use of ctype.h
On Fri, May 09, 2008 at 02:39:06PM +0200, Jim Meyering wrote: Jim Meyering [EMAIL PROTECTED] wrote: Following up on this thread: http://thread.gmane.org/gmane.comp.emulators.libvirt/6338/focus=6349 This change removes all uses of ctype macros and ensures that no new ones will be added. The other changes I mentioned will come later. [...] I should mention that there is a prerequisite patch to add the c-ctype module and pull in gnulib-related updates: From 105399f605ff7c4c7f5148f18ea08cb63c8c0411 Mon Sep 17 00:00:00 2001 From: Jim Meyering [EMAIL PROTECTED] Date: Fri, 9 May 2008 13:59:48 +0200 Subject: [PATCH] Prepare to use gnulib's c-type module. Oops problem ! Assuming c-ctype licence is LGPL i'm all for it but I find http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/c-ctype.h ---line 10 -- This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 2 of the License, or (at your option) any later version. - I would postpone the c-ctype migration until this is fixed to LGPL, assuming it's possible Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Use gnulib's c-ctype.h; prohibit use of ctype.h
Daniel Veillard [EMAIL PROTECTED] wrote: On Fri, May 09, 2008 at 02:39:06PM +0200, Jim Meyering wrote: Jim Meyering [EMAIL PROTECTED] wrote: Following up on this thread: http://thread.gmane.org/gmane.comp.emulators.libvirt/6338/focus=6349 This change removes all uses of ctype macros and ensures that no new ones will be added. The other changes I mentioned will come later. [...] I should mention that there is a prerequisite patch to add the c-ctype module and pull in gnulib-related updates: From 105399f605ff7c4c7f5148f18ea08cb63c8c0411 Mon Sep 17 00:00:00 2001 From: Jim Meyering [EMAIL PROTECTED] Date: Fri, 9 May 2008 13:59:48 +0200 Subject: [PATCH] Prepare to use gnulib's c-type module. Oops problem ! Assuming c-ctype licence is LGPL i'm all for it but I find That's not a problem (and this is documented -- but to find it, you have to dig). The authoritative source for the license is specified in the module-definition file, gnulib/modules/c-ctype. It is LGPLv2+. Besides, our invocation of gnulib-tool (in bootstrap) requires that any module be compatible with LGPLv2+ via its --lgpl=2 option, so this is checked automatically. You may rest assured that any module I propose for addition has the right copyright. In addition, when gnulib-tool copies the files into gnulib, it rewrites the license to be what we require: This program 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 program 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. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Use gnulib's c-ctype.h; prohibit use of ctype.h
On Fri, May 09, 2008 at 03:09:17PM +0200, Jim Meyering wrote: Daniel Veillard [EMAIL PROTECTED] wrote: On Fri, May 09, 2008 at 02:39:06PM +0200, Jim Meyering wrote: Jim Meyering [EMAIL PROTECTED] wrote: Following up on this thread: http://thread.gmane.org/gmane.comp.emulators.libvirt/6338/focus=6349 This change removes all uses of ctype macros and ensures that no new ones will be added. The other changes I mentioned will come later. [...] I should mention that there is a prerequisite patch to add the c-ctype module and pull in gnulib-related updates: From 105399f605ff7c4c7f5148f18ea08cb63c8c0411 Mon Sep 17 00:00:00 2001 From: Jim Meyering [EMAIL PROTECTED] Date: Fri, 9 May 2008 13:59:48 +0200 Subject: [PATCH] Prepare to use gnulib's c-type module. Oops problem ! Assuming c-ctype licence is LGPL i'm all for it but I find That's not a problem (and this is documented -- but to find it, you have to dig). The authoritative source for the license is specified in the module-definition file, gnulib/modules/c-ctype. It is LGPLv2+. Looks quite confusing. The .h file says this module is under Licence X and somewhere a text file says it's under a different Licence Y. My instinctive reaction (and I guess i'm not the only one) is to assume the licencing information in the source would be the one binding from a legal POV, but IANAL, so all i can say is that it looks weird. Besides, our invocation of gnulib-tool (in bootstrap) requires that any module be compatible with LGPLv2+ via its --lgpl=2 option, so this is checked automatically. You may rest assured that any module I propose for addition has the right copyright. In addition, when gnulib-tool copies the files into gnulib, it rewrites the license to be what we require: Okay, i assume there is no problem, changing licences when copying exceeds my limited understanding, but there is certainly a good and legally okay reason for that, but I'm fine to stay ignorant as long as you tell me it's okay :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Use gnulib's c-ctype.h; prohibit use of ctype.h
Daniel Veillard [EMAIL PROTECTED] wrote: ... The authoritative source for the license is specified in the module-definition file, gnulib/modules/c-ctype. It is LGPLv2+. Looks quite confusing. The .h file says this module is under Licence X and somewhere a text file says it's under a different Licence Y. My instinctive reaction (and I guess i'm not the only one) is to assume the licencing information in the source would be the one binding from a legal POV, but IANAL, so all i can say is that it looks weird. Besides, our invocation of gnulib-tool (in bootstrap) requires that any module be compatible with LGPLv2+ via its --lgpl=2 option, so this is checked automatically. You may rest assured that any module I propose for addition has the right copyright. In addition, when gnulib-tool copies the files into gnulib, it rewrites the license to be what we require: Okay, i assume there is no problem, changing licences when copying exceeds my limited understanding, but there is certainly a good and legally okay reason for that, but I'm fine to stay ignorant as long as you tell me it's okay :-) Yes, it's confusing, and we'd like it to be fixed. It will be, eventually. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] Network interface XML for containers
We never really settled on the XML format for container network interfaces. I know a little more about what these look like now and have been working on the code so would like to get this sorted out. With network namespaces enabled, processes within the container will not be able to see any network devices outside of the container. A veth device pair will be used to transport traffic into and out off the container. One end of the veth pair will be attached to a bridge in the parent namespace. The other end of will be moved into the container namespace. We need to be able to represent the following in the XML: Network or bridge name Name for parent side veth device Name for container side veth device inet address for container side veth device So this should end up looking quite a bit like the formats for Virtual network and Bridge to LAN with a couple new items. The formats I've been kicking around are: Virtual network devices interface type='network' source network='default' dev='veth0'/ target dev='veth1' address='192.168.0.150'/ /interface /devices Bridge to LAN devices interface type='bridge' source bridge='virbr0' dev='veth4'/ target dev='veth5' address='192.168.0.155'/ /interface /devices All comments welcome. Thanks! -- Best Regards, Dave Leskovec IBM Linux Technology Center Open Virtualization -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Network interface XML for containers
On Fri, 9 May 2008, Dave Leskovec wrote: We never really settled on the XML format for container network interfaces. I know a little more about what these look like now and have been working on the code so would like to get this sorted out. With network namespaces enabled, processes within the container will not be able to see any network devices outside of the container. 1) How does it solve 'mac address spoofing' between virtual machines? 2) How does it solve 'dupplicate mac addresses' in a cluster of processing units? 3) How does it solve 'migration arp issues'? Stefan -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Network interface XML for containers
On Fri, May 09, 2008 at 10:16:38AM -0700, Dave Leskovec wrote: We never really settled on the XML format for container network interfaces. I know a little more about what these look like now and have been working on the code so would like to get this sorted out. With network namespaces enabled, processes within the container will not be able to see any network devices outside of the container. A veth device pair will be used to transport traffic into and out off the container. One end of the veth pair will be attached to a bridge in the parent namespace. The other end of will be moved into the container namespace. We need to be able to represent the following in the XML: Network or bridge name Name for parent side veth device Name for container side veth device Do you really need to be able to specify the guest side of the NIC name in the XML ? I'd rather we just left it out unless there was a clear need it can't be automatically determined by the driver. With other virt drivers, the guest side of the NIC just gets name sequentially assigned eth0, eth1,... Linux lets you rename the NICs anyway so IMHO the only important thing is the MAC address since that's guarenteeed persistent unique property for NICs, which a name is not. inet address for container side veth device Again, why are you specifying an IP address manually ? THe guest IP is typically determined by the guest admin, either statically or via DHCP. It shouldn't need to be configured in the host side if using the libvirt virtual networking or bridging. So this should end up looking quite a bit like the formats for Virtual network and Bridge to LAN with a couple new items. The formats I've been kicking around are: Virtual network devices interface type='network' source network='default' dev='veth0'/ target dev='veth1' address='192.168.0.150'/ /interface /devices Due to a historical accident the target element in the interface XML is actually refering to the host side of the NIC. We don't have any existing element in our XML format describing the guest name of the NIC - its just implied by ordering. THe target element is also treated as read-only by existing drivers, being automagically assigned unique name by the virt driver. The containers should follow the existing XML format 100% interface type='network' source network='default' target dev='veth0'/ /interface Bridge to LAN devices interface type='bridge' source bridge='virbr0' dev='veth4'/ target dev='veth5' address='192.168.0.155'/ /interface /devices Again I think including the guest NIC name is redundant, likewise for ip address we can follow the existing XML format 100% interface type='bridge' source network='br0' target dev='veth4'/ /interface NB, We do have an IP address element in the interface XML format, however, this is not for configuring the guest IP address directly. Rather it is to setup a filter on the host side to reject any IP traffic other than than coming from the designate IP address. This basically sets a firewall rule on the host side of the NIC matching on IP. This is only implemented in Xen though and even then I don't think anyone really uses it in practice. Regards, Daniel. -- |: Red Hat, Engineering, Boston -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] [RFC] Network interface XML for containers
On Fri, May 09, 2008 at 07:26:07PM +0200, Stefan de Konink wrote: On Fri, 9 May 2008, Dave Leskovec wrote: We never really settled on the XML format for container network interfaces. I know a little more about what these look like now and have been working on the code so would like to get this sorted out. With network namespaces enabled, processes within the container will not be able to see any network devices outside of the container. 1) How does it solve 'mac address spoofing' between virtual machines? That's an irrelevant question for the libvirt XML format. We're simply defining what MAC address a guest has. Its upto the underlying virtualization technology to do the neccessary mac address spoofing protection based on the define MAC libvirt assignes to a guest 2) How does it solve 'dupplicate mac addresses' in a cluster of processing units? It doesn't. Libvirt does not provide policy, only mechanism. Its upto tools using libvirt to ensure uniqueness. 3) How does it solve 'migration arp issues'? Again, not libvirt's problem. Its the underlying virt technology to deal with. Dan -- |: Red Hat, Engineering, Boston -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] [RFC] Network interface XML for containers
DL So the users init would be responsible for configuring the DL container veth device then? At that point they can assign a DL static ip if desired? This would mean that for a container that DL wanted to just run an application, init would have to point to a DL script that configured the network and then ran the application. DL Not sure that's a problem, just stating the consequence. I think that specifying the IP in the XML is a nice shortcut, but I wonder about two things: First, if you're just starting a single application in a container, what are the chances you want that single application to have an interface and IP address of its own? Second, the IP address that shows up in the libvirt config would imply to viewers that they can access the guest in that way. However, the guest could certainly have changed the address of its interface, thus invalidating the IP information that libvirt has. This problem exists with MAC addresses as well, although I think it's less severe because people don't usually try to connect to a guest based on its MAC. A grand total of 30 seconds of pondering on the above makes me lean in the direction of not specifying the IP address in the XML. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: [EMAIL PROTECTED] pgpaYc3gTt3XL.pgp Description: PGP signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Network interface XML for containers
On Fri, May 09, 2008 at 12:29:39PM -0700, Dan Smith wrote: DL So the users init would be responsible for configuring the DL container veth device then? At that point they can assign a DL static ip if desired? This would mean that for a container that DL wanted to just run an application, init would have to point to a DL script that configured the network and then ran the application. DL Not sure that's a problem, just stating the consequence. I think that specifying the IP in the XML is a nice shortcut, but I wonder about two things: First, if you're just starting a single application in a container, what are the chances you want that single application to have an interface and IP address of its own? And you could trivially write a short shell script around the app that sets the IP that it requirs. Second, the IP address that shows up in the libvirt config would imply to viewers that they can access the guest in that way. However, the guest could certainly have changed the address of its interface, thus invalidating the IP information that libvirt has. Yes, this is a compelling reason not to expose it to me - its simply not data that can be reliably determined from the host OS side where libvirt is living. Dan. -- |: Red Hat, Engineering, Boston -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] PATCH: Remove use of strcmp, etc
This patch removes all use of strcmp, strncmp, strcasecmp and strncasecmp
in favour of the equality macros we have defined in internal.h, eg STREQ,
STRNEQ, STRNEQLEN, STREQLEN, etc, etc
The only strcasecmp left is in virsh where it is used in a sort function
so does genuinely need the -1, 0, +1 tristate return value rather than a
simple boolean for equality.
This brings all code into compliance with string comparison guidelines in
the HACKING file.
proxy/libvirt_proxy.c |4 -
src/conf.c|4 -
src/hash.c| 12 ++---
src/iptables.c|6 +-
src/libvirt.c |4 -
src/openvz_conf.c | 10 ++--
src/qemu_conf.c | 62 ++---
src/remote_internal.c | 10 ++--
src/sexpr.c |4 -
src/test.c| 14 +++---
src/util.c|8 +--
src/virsh.c | 30 +++---
src/xen_unified.c |6 +-
src/xend_internal.c | 40 +--
src/xm_internal.c | 104 +-
src/xml.c | 22 +-
src/xmlrpc.c |2
tests/virshtest.c |6 +-
tests/xml2sexprtest.c |2
tests/xmlrpctest.c|2
20 files changed, 177 insertions(+), 175 deletions(-)
Dan.
Index: proxy/libvirt_proxy.c
===
RCS file: /data/cvs/libvirt/proxy/libvirt_proxy.c,v
retrieving revision 1.33
diff -u -p -u -p -r1.33 libvirt_proxy.c
--- proxy/libvirt_proxy.c 24 Apr 2008 09:17:29 - 1.33
+++ proxy/libvirt_proxy.c 9 May 2008 21:08:07 -
@@ -802,9 +802,9 @@ int main(int argc, char **argv) {
}
for (i = 1; i argc; i++) {
- if (!strcmp(argv[i], -v)) {
+ if (STREQ(argv[i], -v)) {
debug++;
- } else if (!strcmp(argv[i], -no-timeout)) {
+ } else if (STREQ(argv[i], -no-timeout)) {
persist = 1;
} else {
usage(argv[0]);
Index: src/conf.c
===
RCS file: /data/cvs/libvirt/src/conf.c,v
retrieving revision 1.26
diff -u -p -u -p -r1.26 conf.c
--- src/conf.c 28 Apr 2008 15:14:59 - 1.26
+++ src/conf.c 9 May 2008 21:08:08 -
@@ -800,7 +800,7 @@ __virConfGetValue(virConfPtr conf, const
cur = conf-entries;
while (cur != NULL) {
-if ((cur-name != NULL) (!strcmp(cur-name, setting)))
+if ((cur-name != NULL) (STREQ(cur-name, setting)))
return(cur-value);
cur = cur-next;
}
@@ -829,7 +829,7 @@ __virConfSetValue (virConfPtr conf,
cur = conf-entries;
while (cur != NULL) {
-if ((cur-name != NULL) (!strcmp(cur-name, setting))) {
+if ((cur-name != NULL) (STREQ(cur-name, setting))) {
break;
}
prev = cur;
Index: src/hash.c
===
RCS file: /data/cvs/libvirt/src/hash.c,v
retrieving revision 1.39
diff -u -p -u -p -r1.39 hash.c
--- src/hash.c 29 Apr 2008 13:48:41 - 1.39
+++ src/hash.c 9 May 2008 21:08:08 -
@@ -270,11 +270,11 @@ virHashAddEntry(virHashTablePtr table, c
} else {
for (insert = (table-table[key]); insert-next != NULL;
insert = insert-next) {
-if (!strcmp(insert-name, name))
+if (STREQ(insert-name, name))
return (-1);
len++;
}
-if (!strcmp(insert-name, name))
+if (STREQ(insert-name, name))
return (-1);
}
@@ -335,14 +335,14 @@ virHashUpdateEntry(virHashTablePtr table
} else {
for (insert = (table-table[key]); insert-next != NULL;
insert = insert-next) {
-if (!strcmp(insert-name, name)) {
+if (STREQ(insert-name, name)) {
if (f)
f(insert-payload, insert-name);
insert-payload = userdata;
return (0);
}
}
-if (!strcmp(insert-name, name)) {
+if (STREQ(insert-name, name)) {
if (f)
f(insert-payload, insert-name);
insert-payload = userdata;
@@ -393,7 +393,7 @@ virHashLookup(virHashTablePtr table, con
if (table-table[key].valid == 0)
return (NULL);
for (entry = (table-table[key]); entry != NULL; entry = entry-next) {
-if (!strcmp(entry-name, name))
+if (STREQ(entry-name, name))
return (entry-payload);
}
return (NULL);
@@ -445,7 +445,7 @@ virHashRemoveEntry(virHashTablePtr table
} else {
for (entry = (table-table[key]); entry != NULL;
entry = entry-next) {
-if (!strcmp(entry-name, name)) {
+if (STREQ(entry-name, name)) {
if ((f != NULL) (entry-payload != NULL))
f(entry-payload, entry-name);
entry-payload = NULL;
Index:
Re: [libvirt] PATCH: Remove use of strcmp, etc
On Fri, May 09, 2008 at 11:40:39PM +0200, Jim Meyering wrote: Daniel P. Berrange [EMAIL PROTECTED] wrote: This patch removes all use of strcmp, strncmp, strcasecmp and strncasecmp in favour of the equality macros we have defined in internal.h, eg STREQ, STRNEQ, STRNEQLEN, STREQLEN, etc, etc Nice. With that, you can remove sc_prohibit_strcmp from the list of disabled checks in Makefile.cfg. You might want to extend the corresponding regexp in Makefile.maint to prohibit the other completely excluded functions like strncmp. BTW, did you make this change automatically? or do a binary before/after comparison. In which case there's not much point in reviewing the details... No I did it manually checked with inspection. Could try a binary diff with without the patch if you want... Dan. -- |: Red Hat, Engineering, Boston -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: Remove use of strcmp, etc
On Fri, May 09, 2008 at 10:42:08PM +0100, Daniel P. Berrange wrote: On Fri, May 09, 2008 at 11:40:39PM +0200, Jim Meyering wrote: Daniel P. Berrange [EMAIL PROTECTED] wrote: This patch removes all use of strcmp, strncmp, strcasecmp and strncasecmp in favour of the equality macros we have defined in internal.h, eg STREQ, STRNEQ, STRNEQLEN, STREQLEN, etc, etc Nice. With that, you can remove sc_prohibit_strcmp from the list of disabled checks in Makefile.cfg. You might want to extend the corresponding regexp in Makefile.maint to prohibit the other completely excluded functions like strncmp. BTW, did you make this change automatically? or do a binary before/after comparison. In which case there's not much point in reviewing the details... No I did it manually checked with inspection. Could try a binary diff with without the patch if you want... Actually a binary diff won't work because of places where I changed from strncmp to STRPREFIX - the latter includes a calls to strlen instead of hardcoding the constant Dan -- |: Red Hat, Engineering, Boston -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: Remove use of strcmp, etc
Daniel P. Berrange [EMAIL PROTECTED] wrote:
On Fri, May 09, 2008 at 10:42:08PM +0100, Daniel P. Berrange wrote:
On Fri, May 09, 2008 at 11:40:39PM +0200, Jim Meyering wrote:
Daniel P. Berrange [EMAIL PROTECTED] wrote:
This patch removes all use of strcmp, strncmp, strcasecmp and strncasecmp
in favour of the equality macros we have defined in internal.h, eg STREQ,
STRNEQ, STRNEQLEN, STREQLEN, etc, etc
Nice. With that, you can remove sc_prohibit_strcmp
from the list of disabled checks in Makefile.cfg.
You might want to extend the corresponding regexp in Makefile.maint
to prohibit the other completely excluded functions like strncmp.
BTW, did you make this change automatically? or do a binary before/after
comparison. In which case there's not much point in reviewing the
details...
No I did it manually checked with inspection. Could try a binary diff
with without the patch if you want...
Actually a binary diff won't work because of places where I changed
from strncmp to STRPREFIX - the latter includes a calls to strlen
instead of hardcoding the constant
To check, so far I've converted all strcmp uses with these:
git grep -l '!strcmp *('|xargs perl -pi -e 's/!strcmp( *)\(/STREQ$1(/g'
git grep -l 'strcmp *([^=]*== *0'|xargs perl -pi -e 's/\bstrcmp( *\(.*?\)) *==
*0/STREQ$1/'
git grep -l 'strcmp *([^=]*!= *0'|xargs perl -pi -e 's/\bstrcmp( *\(.*?\)) *!=
*0/STRNEQ$1/g'
git grep -l '!(strcmp *('|xargs perl -pi -e 's/!\(strcmp( *\(.*?\))\)/STREQ$1/'
git grep -l 'strcmp *('|xargs perl -pi -e 's/\bstrcmp( *\()/STRNEQ$1/g'
Next (later ;-) is to convert strncmp uses and compare with yours.
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
