[Libvir] [PATCH]Change MAC address to case insensitive
Hi, xenXMDomainAttachDevice and xenXMDomainDetachDevice treats "case sensitve" for MAC address of stopping domain. This patch changes from case sensitive to case insensitive. Thanks, Shigeki Sakamoto. Index: src/xm_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xm_internal.c,v retrieving revision 1.64 diff -u -p -r1.64 xm_internal.c --- src/xm_internal.c 20 Feb 2008 15:29:13 -0000 1.64 +++ src/xm_internal.c 26 Feb 2008 06:40:57 -0000 @@ -2812,7 +2812,7 @@ xenXMAttachInterface(virDomainPtr domain key = nextkey; } - if (!(strcmp(dommac, (const char *) mac))) { + if (!(strcasecmp(dommac, (const char *) mac))) { if (autoassign) { free(mac); mac = NULL; @@ -3087,7 +3087,7 @@ xenXMDomainDetachDevice(virDomainPtr dom mac = nextmac; } - if (!(strcmp(dommac, (const char *) key))) + if (!(strcasecmp(dommac, (const char *) key))) break; } skip:
I'll apply this today (using our STRCASEEQ macros) if no one else objects. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
"Richard W.M. Jones" <rjones@redhat.com> wrote:
I'll apply this today (using our STRCASEEQ macros) if no one else objects.
Hi, What do you think about using a MAC-address-specific comparison function? I.e., one that is not only case-independent, but that also knows leading zeros are unnecessary? i.e., it would admit that these two match: 00:0A:FF:3A:00:09 0:a:ff:3a:0:9 That would be a little more user friendly. In any case, even if we stick with simply case-ignoring, I suggest we give it a name, like macCompare and use it consistently. There's at least one other MAC comparison: virsh.c currently uses xmlStrcasecmp in cmdDetachInterface: diff_mac = xmlStrcasecmp(tmp_mac, BAD_CAST mac);
On Tue, Feb 26, 2008 at 12:13:43PM +0100, Jim Meyering wrote:
"Richard W.M. Jones" <rjones@redhat.com> wrote:
I'll apply this today (using our STRCASEEQ macros) if no one else objects.
Hi,
What do you think about using a MAC-address-specific comparison function? I.e., one that is not only case-independent, but that also knows leading zeros are unnecessary?
i.e., it would admit that these two match:
00:0A:FF:3A:00:09 0:a:ff:3a:0:9
That would be a little more user friendly.
Yes, we should add a helper function for comparing mac addresses for equality. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Tue, Feb 26, 2008 at 12:54:15PM +0000, Daniel P. Berrange wrote:
On Tue, Feb 26, 2008 at 12:13:43PM +0100, Jim Meyering wrote:
"Richard W.M. Jones" <rjones@redhat.com> wrote:
I'll apply this today (using our STRCASEEQ macros) if no one else objects.
Hi,
What do you think about using a MAC-address-specific comparison function? I.e., one that is not only case-independent, but that also knows leading zeros are unnecessary?
i.e., it would admit that these two match:
00:0A:FF:3A:00:09 0:a:ff:3a:0:9
That would be a little more user friendly.
Yes, we should add a helper function for comparing mac addresses for equality.
OK, try this patch. Jim, not sure I understood to full complexities of making static linking work without duplicate symbols, so this patch may be wrong in that regard. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
On Wed, Feb 27, 2008 at 02:27:30PM +0900, S.Sakamoto wrote:
OK, try this patch.
I tried this patch. It works fine to me.
Yes, the patch works for me too. I'll commit this end of today unless anyone objects. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
This patch has been committed now. Thanks for your contribution to libvirt. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Tue, Feb 26, 2008 at 12:13:43PM +0100, Jim Meyering wrote:
"Richard W.M. Jones" <rjones@redhat.com> wrote:
I'll apply this today (using our STRCASEEQ macros) if no one else objects.
What do you think about using a MAC-address-specific comparison function? I.e., one that is not only case-independent, but that also knows leading zeros are unnecessary?
i.e., it would admit that these two match:
00:0A:FF:3A:00:09 0:a:ff:3a:0:9
Yes, we should add a helper function for comparing mac addresses for equality.
With this patch, they now match: Also ignore leading zeros when comparing MAC addresses. * src/util.c: Include <ctype.h>. (TOLOWER): Define. (__virMacAddrCompare): Rewrite to also ignore leading zeros. Signed-off-by: Jim Meyering <meyering@redhat.com> --- src/util.c | 34 +++++++++++++++++++++++++++++----- 1 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/util.c b/src/util.c index 4d61540..edaa5aa 100644 --- a/src/util.c +++ b/src/util.c @@ -35,6 +35,7 @@ #include <sys/stat.h> #include <sys/wait.h> #include <string.h> +#include <ctype.h> #ifdef HAVE_PATHS_H #include <paths.h> @@ -50,6 +51,8 @@ #define MAX_ERROR_LEN 1024 +#define TOLOWER(Ch) (isupper (Ch) ? tolower (Ch) : (Ch)) + #define virLog(msg...) fprintf(stderr, msg) #ifndef PROXY @@ -654,14 +657,35 @@ virParseNumber(const char **str) return (ret); } -/* Use this function when comparing two MAC addresses. It deals with - * string case compare and will eventually be extended to understand - * that 01:02:03:04:05:06 is the same as 1:2:3:4:5:6. +/* Compare two MAC addresses, ignoring differences in case, + * as well as leading zeros. */ int -__virMacAddrCompare (const char *mac1, const char *mac2) +__virMacAddrCompare (const char *p, const char *q) { - return strcasecmp (mac1, mac2); + unsigned char c, d; + do { + while (*p == '0' && isxdigit (p[1])) + ++p; + while (*q == '0' && isxdigit (q[1])) + ++q; + c = TOLOWER (*p); + d = TOLOWER (*q); + + if (c == 0 || d == 0) + break; + + ++p; + ++q; + } while (c == d); + + if (UCHAR_MAX <= INT_MAX) + return c - d; + + /* On machines where 'char' and 'int' are types of the same size, the + difference of two 'unsigned char' values - including the sign bit - + doesn't fit in an 'int'. */ + return (c > d ? 1 : c < d ? -1 : 0); } /* -- 1.5.4.3.326.g7655e3
On Fri, Feb 29, 2008 at 12:59:39PM +0100, Jim Meyering wrote:
diff --git a/src/util.c b/src/util.c index 4d61540..edaa5aa 100644 --- a/src/util.c +++ b/src/util.c @@ -35,6 +35,7 @@ #include <sys/stat.h> #include <sys/wait.h> #include <string.h> +#include <ctype.h>
I thought we're not supposed to be using ctype? Anyway, +1 the patch looks fine. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
"Richard W.M. Jones" <rjones@redhat.com> wrote:
On Fri, Feb 29, 2008 at 12:59:39PM +0100, Jim Meyering wrote:
diff --git a/src/util.c b/src/util.c index 4d61540..edaa5aa 100644 --- a/src/util.c +++ b/src/util.c @@ -35,6 +35,7 @@ #include <sys/stat.h> #include <sys/wait.h> #include <string.h> +#include <ctype.h>
Anyway, +1 the patch looks fine.
Hi Rich, Thanks for the review.
I thought we're not supposed to be using ctype?
As far as i know, #include <ctype.h> is still the only way to use the things it specifies (tolower, isupper, isxdigit, etc.) Maybe you're thinking of strings.h?
On Fri, Feb 29, 2008 at 01:33:42PM +0100, Jim Meyering wrote:
"Richard W.M. Jones" <rjones@redhat.com> wrote:
I thought we're not supposed to be using ctype?
As far as i know, #include <ctype.h> is still the only way to use the things it specifies (tolower, isupper, isxdigit, etc.)
Maybe you're thinking of strings.h?
I was thinking that the ctype functions like isxdigit were deprecated. Don't worry, you know lots more about this than me. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
participants (4)
-
Daniel P. Berrange -
Jim Meyering -
Richard W.M. Jones -
S.Sakamoto