2:37 a.m.
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:
3:44 a.m.
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
5:13 a.m.
"Richard W.M. Jones" <rjones(a)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);
6:54 a.m.
On Tue, Feb 26, 2008 at 12:13:43PM +0100, Jim Meyering wrote:
"Richard W.M. Jones" <rjones(a)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 -=|
7:11 a.m.
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(a)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
3:36 a.m.
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
10:13 a.m.
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
5:59 a.m.
New subject: [Libvir] ignore leading zeros, too [Change MAC address to case insensitive
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Tue, Feb 26, 2008 at 12:13:43PM +0100, Jim Meyering wrote:
> "Richard W.M. Jones" <rjones(a)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(a)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
6:27 a.m.
New subject: [Libvir] Re: ignore leading zeros, too [Change MAC address to case insensitive
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
6:33 a.m.
New subject: [Libvir] Re: ignore leading zeros, too [Change MAC address to case insensitive
"Richard W.M. Jones" <rjones(a)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?
6:44 a.m.
New subject: [Libvir] Re: ignore leading zeros, too [Change MAC address to case insensitive
On Fri, Feb 29, 2008 at 01:33:42PM +0100, Jim Meyering wrote:
"Richard W.M. Jones" <rjones(a)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
6110
days inactive
6116
days old
12 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Jim Meyering -
Richard W.M. Jones -
S.Sakamoto