[Libvir] [PATCH]check NULL of mac
Hi,
attach-device become the segmentation fault,
when it uses following xml. (There is no address='xx')
++
|interface type='bridge' |
| source bridge='xenbr0'/ |
| mac/|
| script path='/etc/xen/scripts/vif-bridge'/ |
|/interface|
++
Libxml gives back NULL when there is not an attribute.
xenXMAttachInterface compares it in strcmp as NULL.
So, I make the patch which solved this problem.
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 - 1.64
+++ src/xm_internal.c 26 Feb 2008 06:39:05 -
@@ -2769,12 +2769,13 @@ xenXMAttachInterface(virDomainPtr domain
}
source = xmlGetProp(node, BAD_CAST type);
-if (!(node = virXPathNode(/interface/mac, ctxt))) {
+if ((node = virXPathNode(/interface/mac, ctxt)))
+mac = xmlGetProp(node, BAD_CAST address);
+if (!node || !mac) {
if (!(mac = (xmlChar *)xenXMAutoAssignMac()))
goto cleanup;
autoassign = 1;
-} else
-mac = xmlGetProp(node, BAD_CAST address);
+}
list_item = virConfGetValue(entry-conf, vif);
if (list_item list_item-type == VIR_CONF_LIST) {
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] default hypervisor selection
On Mon, Feb 25, 2008 at 09:32:48AM -0500, Daniel Veillard wrote:
On Sun, Feb 24, 2008 at 10:12:05PM +, Daniel P. Berrange wrote:
On Fri, Feb 22, 2008 at 10:59:43AM -0500, Daniel Veillard wrote:
Okay, first patch enclosed, it seems to work for me:
+ /*
+ * if running a xen kernel, give it priority over
+ * QEmu emultation
+ */
+ if (STREQ(latest, xen:///))
+ use = latest;
If we edit virInitialize() to make 'xenUnifiedRegister' run before the
call to 'qemudRegister', then we won't need this check, since the Xen
driver would get probed ahead of the QEMU driver.
Honnestly I prefer to keep the test in than base the behaviour purely
on the order of the drivers, it exposes the intent clearly, and it's not
like it's a timely critical operation :-)
+ else if ((use == NULL) (!STREQ(latest, test:///)))
+ use = latest;
IMHO, remove the !STREQ(latest, test:///) and get rid of the 'probe' impl
for the test driver. The test driver will always succeed, but should
only ever be used for test suites, never real world deployment, so
we shouldn't probe it.
Okay, i wondered if i should keep the test probe or not, and though it would
still be useful if we were to expose the list of drivers from a library API
as you suggested too. But yeah with the current code, let's get rid of it,
[...]
+/**
+ * qemudProbe:
+ *
+ * Probe for the availability of the qemu driver, assume the
+ * presence of QEmu emulation if the binaries are installed
+ */
+static const char *qemudProbe(void)
+{
+if ((virFileExists(/usr/bin/qemu)) ||
+(virFileExists(/usr/bin/qemu-kvm))) {
+if (getuid() == 0) {
+ return(qemu:///system);
+ } else {
+ return(qemu:///session);
+ }
+}
+return(NULL);
+}
Should add a check for '/usr/bin/xenner' too, which is Gerd's
Xen compatability layer for the QEMU driver :-)
Okidoc :-)
[...]
+static const char *
+xenUnifiedProbe (void)
+{
+#ifdef __linux__
+if (virFileExists(/proc/xen))
+return(xen:///);
+#endif
+#ifdef __sun__
+FILE *fh;
+
+if (fh = fopen(path, r)) {
+ fclose(fh);
+return(xen:///);
+}
+#endif
AFAICT this won't compile on Solaris since 'path' isn't defined.
Oops it's fopen(/dev/xen/domcaps, r) as John suggested,
I will commit with those fixes later today unless someone has an objection,
Okay, commited, seems to work well for me as root and as normal
user on RHEL-5.1 , but on Fedora-8 as non-root this doesn't work
the debug shows
DEBUG: libvirt.c: do_open (Probed qemu:///session)
DEBUG: libvirt.c: do_open (Using qemu:///session as default URI, 1 hypervisor
found)
DEBUG: libvirt.c: do_open (name qemu:///session to URI components:
[...]
DEBUG: libvirt.c: do_open (trying driver 1 (QEMU) ...)
DEBUG: libvirt.c: do_open (driver 1 QEMU returned DECLINED)
The problem seems to be that in qemudOpen at that point qemu_driver is
NULL, and we return VIR_DRV_OPEN_DECLINED immediately as a result.
When we end up in the remote driver I see
DEBUG: remote_internal.c: doRemoteOpen (proceeding with name = qemu:///session?)
DEBUG: remote_internal.c: remoteAuthPolkit (Client initialize PolicyKit
authentication)
Attempting to gain the privilege for org.libvirt.unix.monitor.
polkit-grant-helper: given auth type (8 - yes) is bogus
Failed to gain the privilege for org.libvirt.unix.monitor.
libvir: Remote error : authentication failed
but I didn't got any option to authenticate at that point.
virsh -c qemu:///session --readonly list
also fails in a similar way, but
virsh -c qemu:///system --readonly list
succeeds.
There is something strange in the processing of qemu:///session
in my Fedora 8 (libvirt-0.4.0-4.fc8 , daemon running)
I will try to understand the problem, it would be good to have this
fixed before pushing the next release,
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
[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 - 1.64
+++ src/xm_internal.c 26 Feb 2008 06:40:57 -
@@ -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:
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH]Change MAC address to case insensitive
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 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH 1/2] Add internal API for managing capabilities data
Yes, this all looks good. Another advantage of doing this is that we can be sure that the capabilities XML generated by each driver will have the same schema. 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 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH 2/2] Refactor drivers to use capabilities API
On Tue, Feb 26, 2008 at 05:13:51AM +, Daniel P. Berrange wrote: This patch refactors the Xen, QEMU and Test drivers to use the new API for dealing with capabilities data. Yes, +1 to this patch as well. 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 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH]check NULL of mac
+1. I'll apply this today if no one else objects. 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 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] (for discussion) DHCP host mappings using 3 arrays API
Richard W.M. Jones [EMAIL PROTECTED] wrote: This patch is an evolution of the previous patch for implementing DHCP ... +if (maxmappings == 0) return 0; ... +if (col == 0) hwaddr = token; ... +if (!(*hwaddrs)[lines]) goto mem_error; Hi Rich, Is there a libvirt style convention that says whether a one-line if-block is written all on one line? I find it slightly easier to spot the conditional at a glance when the statement is on a separate line, but can certainly adapt to a different style. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH]Change MAC address to case insensitive
Richard W.M. Jones [EMAIL PROTECTED] 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); -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[Libvir] How to stop 'make install' clobbering /etc/libvirt/libvirtd.conf
Currently 'make install' clobbers configuration files, in particular /etc/libvirt/libvirtd.conf. It shouldn't. Does anyone know how to persuade automake to stop it from doing this? As far as I can see the only way would be to write a custom install rule. 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 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[Libvir] [PATCH] Implement virDomainBlockStats for QEMU/KVM
This little patch just implements the virDomainBlockStats call for
qemu kvm. It does this by using the new 'info blockstats' monitor
command which I added to qemu KVM upstream some months back, and
(hopefully) it does the right thing if this command is not available.
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
Index: src/qemu_driver.c
===
RCS file: /data/cvs/libvirt/src/qemu_driver.c,v
retrieving revision 1.55
diff -u -r1.55 qemu_driver.c
--- src/qemu_driver.c 26 Feb 2008 07:05:18 - 1.55
+++ src/qemu_driver.c 26 Feb 2008 11:31:53 -
@@ -59,6 +59,9 @@
static int qemudShutdown(void);
+/* qemudDebug statements should be changed to use this macro instead. */
+#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__)
+#define DEBUG0(msg) VIR_DEBUG(__FILE__, %s, msg)
#define qemudLog(level, msg...) fprintf(stderr, msg)
@@ -2527,6 +2530,144 @@
return 0;
}
+/* This uses the 'info blockstats' monitor command which was
+ * integrated into both qemu kvm in late 2007. If the command is
+ * not supported we detect this and return the appropriate error.
+ */
+static int
+qemudDomainBlockStats (virDomainPtr dom,
+ const char *path,
+ struct _virDomainBlockStats *stats)
+{
+struct qemud_driver *driver =
+(struct qemud_driver *)dom-conn-privateData;
+char *info, *p, *dummy, *eol;
+char qemu_dev_name[32];
+int len;
+struct qemud_vm *vm = qemudFindVMByID(driver, dom-id);
+
+if (!vm) {
+qemudReportError (dom-conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
+ _(no domain with matching id %d), dom-id);
+return -1;
+}
+if (!qemudIsActiveVM (vm)) {
+qemudReportError (dom-conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _(domain is not running));
+return -1;
+}
+
+/*
+ * QEMU internal block device names are different from the device
+ * names we use in libvirt, so we need to map between them:
+ *
+ * hd[a-] to ide0-hd[0-]
+ * cdromto ide1-cd0
+ * fd[a-] to floppy[0-]
+ */
+if (STREQLEN (path, hd, 2) path[2] = 'a' path[2] = 'z')
+snprintf (qemu_dev_name, sizeof (qemu_dev_name),
+ ide0-hd%d, path[2] - 'a');
+else if (STREQ (path, cdrom))
+strcpy (qemu_dev_name, ide1-cd0);
+else if (STREQLEN (path, fd, 2) path[2] = 'a' path[2] = 'z')
+snprintf (qemu_dev_name, sizeof (qemu_dev_name),
+ floppy%d, path[2] - 'a');
+else {
+qemudReportError (dom-conn, dom, NULL, VIR_ERR_INVALID_ARG,
+ _(invalid path: %s), path);
+return -1;
+}
+
+len = strlen (qemu_dev_name);
+
+if (qemudMonitorCommand (driver, vm, info blockstats, info) 0) {
+qemudReportError (dom-conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _('info blockstats' command failed));
+return -1;
+}
+
+DEBUG (info blockstats reply: %s, info);
+
+/* If the command isn't supported then qemu prints the supported
+ * info commands, so the output starts info . Since this is
+ * unlikely to be the name of a block device, we can use this
+ * to detect if qemu supports the command.
+ */
+if (STREQLEN (info, info , 5)) {
+free (info);
+qemudReportError (dom-conn, dom, NULL, VIR_ERR_NO_SUPPORT,
+ _('info blockstats' not supported by this qemu));
+return -1;
+}
+
+stats-rd_req = -1;
+stats-rd_bytes = -1;
+stats-wr_req = -1;
+stats-wr_bytes = -1;
+stats-errs = -1;
+
+/* The output format for both qemu KVM is:
+ * blockdevice: rd_bytes=% wr_bytes=% rd_operations=% wr_operations=%
+ * (repeated for each block device)
+ * where '%' is a 64 bit number.
+ */
+p = info;
+
+while (*p) {
+if (STREQLEN (p, qemu_dev_name, len)
+ p[len] == ':' p[len+1] == ' ') {
+
+eol = strchr (p, '\n'); if (!eol) eol = p + strlen (p);
+
+p += len+2; /* Skip to first label. */
+
+while (*p) {
+if (STREQLEN (p, rd_bytes=, 9)) {
+p += 9;
+if (virStrToLong_ll (p, dummy, 10, stats-rd_bytes) ==
-1)
+DEBUG (error reading rd_bytes: %s, p);
+} else if (STREQLEN (p, wr_bytes=, 9)) {
+p += 9;
+if (virStrToLong_ll (p, dummy, 10, stats-wr_bytes) ==
-1)
+DEBUG (error reading wr_bytes: %s, p);
+} else if (STREQLEN (p, rd_operations=, 14)) {
+p += 14;
+
Re: [Libvir] [PATCH] (for discussion) DHCP host mappings using 3 arrays API
On Tue, Feb 26, 2008 at 11:43:59AM +0100, Jim Meyering wrote: Richard W.M. Jones [EMAIL PROTECTED] wrote: This patch is an evolution of the previous patch for implementing DHCP ... +if (maxmappings == 0) return 0; ... +if (col == 0) hwaddr = token; ... +if (!(*hwaddrs)[lines]) goto mem_error; Hi Rich, Is there a libvirt style convention that says whether a one-line if-block is written all on one line? I find it slightly easier to spot the conditional at a glance when the statement is on a separate line, but can certainly adapt to a different style. I don't know ... Is there a libvirt manual of style? 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 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] default hypervisor selection
On Tue, Feb 26, 2008 at 03:34:56AM -0500, Daniel Veillard wrote: Okay, commited, seems to work well for me as root and as normal user on RHEL-5.1 , but on Fedora-8 as non-root this doesn't work the debug shows DEBUG: libvirt.c: do_open (Probed qemu:///session) DEBUG: libvirt.c: do_open (Using qemu:///session as default URI, 1 hypervisor found) DEBUG: libvirt.c: do_open (name qemu:///session to URI components: [...] DEBUG: libvirt.c: do_open (trying driver 1 (QEMU) ...) DEBUG: libvirt.c: do_open (driver 1 QEMU returned DECLINED) The problem seems to be that in qemudOpen at that point qemu_driver is NULL, and we return VIR_DRV_OPEN_DECLINED immediately as a result. This is intentional - the QEMU driver should only ever be invoked from within the context of the daemon. Hence qemu_driver will always be NULL when accessed from the standalone library - it'll be initialized when the daemon invokes the private __virSTateInitialize method. When we end up in the remote driver I see DEBUG: remote_internal.c: doRemoteOpen (proceeding with name = qemu:///session?) DEBUG: remote_internal.c: remoteAuthPolkit (Client initialize PolicyKit authentication) Attempting to gain the privilege for org.libvirt.unix.monitor. polkit-grant-helper: given auth type (8 - yes) is bogus Failed to gain the privilege for org.libvirt.unix.monitor. libvir: Remote error : authentication failed but I didn't got any option to authenticate at that point. This looks like a bug - I think the session daemon is mistakenly asking for auth, when none is neccessary because it runs as same UID. 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 -=| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH]Change MAC address to case insensitive
On Tue, Feb 26, 2008 at 12:13:43PM +0100, Jim Meyering wrote: Richard W.M. Jones [EMAIL PROTECTED] 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 -=| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH 1/2] Add internal API for managing capabilities data
On Tue, Feb 26, 2008 at 09:39:49AM +, Richard W.M. Jones wrote: Yes, this all looks good. Another advantage of doing this is that we can be sure that the capabilities XML generated by each driver will have the same schema. Yes indeed - this is a very good benefit, particularly as we extend the capabilities with more data like NUMA / migration / etc. At some point I think it'd be interesting to do a similar refactoring for domains. So we could have an internal virDomainDefPtr to store the definitions and a generic XML formatter / XML parser shared by all drivers. 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 -=| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH]Change MAC address to case insensitive
On Tue, Feb 26, 2008 at 12:54:15PM +, Daniel P. Berrange wrote:
On Tue, Feb 26, 2008 at 12:13:43PM +0100, Jim Meyering wrote:
Richard W.M. Jones [EMAIL PROTECTED] 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
Index: src/internal.h
===
RCS file: /data/cvs/libvirt/src/internal.h,v
retrieving revision 1.63
diff -u -r1.63 internal.h
--- src/internal.h 20 Feb 2008 15:06:53 - 1.63
+++ src/internal.h 26 Feb 2008 13:10:46 -
@@ -54,7 +54,6 @@
#define STRNEQLEN(a,b,n) (strncmp((a),(b),(n)) != 0)
#define STRCASENEQLEN(a,b,n) (strncasecmp((a),(b),(n)) != 0)
-
/* If configured with --enable-debug=yes then library calls
* are printed to stderr for debugging.
*/
Index: src/libvirt_sym.version
===
RCS file: /data/cvs/libvirt/src/libvirt_sym.version,v
retrieving revision 1.36
diff -u -r1.36 libvirt_sym.version
--- src/libvirt_sym.version 21 Feb 2008 03:53:03 - 1.36
+++ src/libvirt_sym.version 26 Feb 2008 13:10:46 -
@@ -185,5 +185,7 @@
__virBufferAdd;
__virBufferAddChar;
+ __virMacAddrCompare;
+
local: *;
};
Index: src/util.c
===
RCS file: /data/cvs/libvirt/src/util.c,v
retrieving revision 1.22
diff -u -r1.22 util.c
--- src/util.c 26 Feb 2008 07:05:18 - 1.22
+++ src/util.c 26 Feb 2008 13:10:46 -
@@ -602,6 +602,16 @@
return 0;
}
+/* 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.
+ */
+int
+__virMacAddrCompare (const char *mac1, const char *mac2)
+{
+return strcasecmp (mac1, mac2);
+}
+
/*
* Local variables:
* indent-tabs-mode: nil
Index: src/util.h
===
RCS file: /data/cvs/libvirt/src/util.h,v
retrieving revision 1.11
diff -u -r1.11 util.h
--- src/util.h 26 Feb 2008 07:05:18 - 1.11
+++ src/util.h 26 Feb 2008 13:10:46 -
@@ -79,4 +79,7 @@
unsigned long long *result);
#define virStrToLong_ull(s,e,b,r) __virStrToLong_ull((s),(e),(b),(r))
+int __virMacAddrCompare (const char *mac1, const char *mac2);
+#define virMacAddrCompare(mac1,mac2) __virMacAddrCompare((mac1),(mac2))
+
#endif /* __VIR_UTIL_H__ */
Index: src/virsh.c
===
RCS file: /data/cvs/libvirt/src/virsh.c,v
retrieving revision 1.133
diff -u -r1.133 virsh.c
--- src/virsh.c 22 Feb 2008 15:55:04 - 1.133
+++ src/virsh.c 26 Feb 2008 13:10:50 -
@@ -4742,7 +4742,7 @@
while (cur != NULL) {
if (cur-type == XML_ELEMENT_NODE xmlStrEqual(cur-name,
BAD_CAST mac)) {
tmp_mac = xmlGetProp(cur, BAD_CAST address);
-diff_mac = xmlStrcasecmp(tmp_mac, BAD_CAST mac);
+diff_mac = virMacAddrCompare ((char *) tmp_mac, mac);
xmlFree(tmp_mac);
if (!diff_mac) {
goto hit;
Index: src/xm_internal.c
===
RCS file: /data/cvs/libvirt/src/xm_internal.c,v
retrieving revision 1.64
diff -u -r1.64 xm_internal.c
--- src/xm_internal.c 20 Feb 2008 15:29:13 - 1.64
+++ src/xm_internal.c 26 Feb 2008 13:10:52 -
@@ -53,6 +53,7 @@
#include xml.h
#include buf.h
#include uuid.h
+#include util.h
static int xenXMConfigSetString(virConfPtr conf, const char *setting,
const char *str);
@@ -2812,7 +2813,7 @@
key = nextkey;
}
-if (!(strcmp(dommac, (const char *) mac))) {
+if (virMacAddrCompare (dommac, (const char *) mac) == 0) {
if (autoassign) {
free(mac);
mac = NULL;
@@ -3087,7 +3088,7 @@
mac = nextmac;
}
-if
Re: [Libvir] [PATCH] (for discussion) DHCP host mappings using 3 arrays API
On Tue, Feb 26, 2008 at 12:01:04PM +, Richard W.M. Jones wrote: On Tue, Feb 26, 2008 at 11:43:59AM +0100, Jim Meyering wrote: Richard W.M. Jones [EMAIL PROTECTED] wrote: This patch is an evolution of the previous patch for implementing DHCP ... +if (maxmappings == 0) return 0; ... +if (col == 0) hwaddr = token; ... +if (!(*hwaddrs)[lines]) goto mem_error; Hi Rich, Is there a libvirt style convention that says whether a one-line if-block is written all on one line? I find it slightly easier to spot the conditional at a glance when the statement is on a separate line, but can certainly adapt to a different style. I don't know ... Is there a libvirt manual of style? I usually go to a separate line myself. I use the following: paphio:~ - cat bin/cb #!/bin/sh indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 -nut -sbi4 -psl -saf -sai -saw -sbi4 -ss -sc -cdw -cli4 -npcs -nbc but I'm not sure there is an option for that (well I could not find it) 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: [Libvir] [PATCH 1/2] Add internal API for managing capabilities data
On Tue, Feb 26, 2008 at 12:52:32PM +, Daniel P. Berrange wrote: On Tue, Feb 26, 2008 at 09:39:49AM +, Richard W.M. Jones wrote: Yes, this all looks good. Another advantage of doing this is that we can be sure that the capabilities XML generated by each driver will have the same schema. Yes indeed - this is a very good benefit, particularly as we extend the capabilities with more data like NUMA / migration / etc. At some point I think it'd be interesting to do a similar refactoring for domains. So we could have an internal virDomainDefPtr to store the definitions and a generic XML formatter / XML parser shared by all drivers. Yes I think that has been on the back of everybody's mind since the beginning, but we were not confident enough in the set of properties doing it internally would be good, then at some point we may even expose it in the API, but there is no hurry. On the other hand cleaning up the domain code mess (incredible how simple things become complex as the code is extended) with new internal structures and APIs in the same way would be a really great thing. But that should probably wait a little bit I feel we are too close to the next release for this, 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: [Libvir] [PATCH 0/2] Re-organize generation / handling of capabilities data
On Tue, Feb 26, 2008 at 04:57:56AM +, Daniel P. Berrange wrote: Earlier today I started the 'simple' task of extending the QEMU driver to support Xenner for running Xen paravirt guests under KVM. This was at first glance easy because it basically looks like QEMU - in fact all we needed was to extend the capabilities XML for the QEMU driver to include data about Xen guests if Xenner is available. Unfortunately this was easier said than done... - The code generating the XML in qemu_driver.c was a mix of string formatting for the XML, and functional logic for determining runtime capabilities (kvm, kqemu, etc). - More runtime logic was in the qemu_conf.c file and stuff that should be runtime logic was static. I attempted to fix up the QEMU code for capabilities but it quickly turned into a horrible mess of spaghetti. Looking at the Xen driver, the situation was pretty much the same, but with the capabilities XML generation spread out over 3 files (xml.c, xen_internal.c and xend_internal.c). So I decided that we needed to have a internal API structure to allow drivers to formally register query their capabilities , and also add a central method for serializing it to XML. The following 2 patches implement this idea, and porting the Xen, QEMU and Test drivers to use it. This removes a lot of code duplication in XML formatting and makes the resulting code easier to follow, and should make it much quicker to adding capabilities info to new drivers too. Okay I looked at both patches (too bad diff can't spot blocks moved from one file to another) they look fine to me. This really is a good cleanup, there are some changes included which seems a bit unrelated but fine too, +1 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: [Libvir] [PATCH]check NULL of mac
On Tue, Feb 26, 2008 at 09:44:33AM +, Richard W.M. Jones wrote: +1. I'll apply this today if no one else objects. looks fine to me too +1 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
[Libvir] Re: RFC: Supporting serial parallel ports for QEMU (and improving Xen)
Daniel P. Berrange wrote: Lots of difficult bits, but I'm starting to get onto coding it. Was this ever completed and merged? I don't see it documented at http://libvirt.org/format.html -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] How to stop 'make install' clobbering /etc/libvirt/libvirtd.conf
Richard W.M. Jones [EMAIL PROTECTED] wrote: Currently 'make install' clobbers configuration files, in particular /etc/libvirt/libvirtd.conf. It shouldn't. Does anyone know how to persuade automake to stop it from doing this? As far as I can see the only way would be to write a custom install rule. IMHO, the default make install behavior (clobber any target) is the right approach. Doing anything else starts to implement policy. Policy belongs more in the domain of the distribution, packaging, and higher-level tools like yum, etc. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] Implement virDomainBlockStats for QEMU/KVM
On Tue, Feb 26, 2008 at 11:32:20AM +, Richard W.M. Jones wrote:
This little patch just implements the virDomainBlockStats call for
qemu kvm. It does this by using the new 'info blockstats' monitor
command which I added to qemu KVM upstream some months back, and
(hopefully) it does the right thing if this command is not available.
sounds good, just a few comments
+/* This uses the 'info blockstats' monitor command which was
+ * integrated into both qemu kvm in late 2007. If the command is
+ * not supported we detect this and return the appropriate error.
+ */
+static int
+qemudDomainBlockStats (virDomainPtr dom,
[...]
+if (STREQLEN (path, hd, 2) path[2] = 'a' path[2] = 'z')
Please fully parenthesize binary expressions relying just on
the priority of operators is more risky and IMHO harder to check/understand
+snprintf (qemu_dev_name, sizeof (qemu_dev_name),
+ ide0-hd%d, path[2] - 'a');
+else if (STREQ (path, cdrom))
+strcpy (qemu_dev_name, ide1-cd0);
safe but strcpy turns my paranoid mode on :-)
[...]
+while (*p) {
+if (STREQLEN (p, qemu_dev_name, len)
+ p[len] == ':' p[len+1] == ' ') {
(STREQLEN (p, qemu_dev_name, len)
(p[len] == ':') (p[len+1] == ' '))
looks so much more readable to me
+eol = strchr (p, '\n'); if (!eol) eol = p + strlen (p);
one statement per line, please, plus going to the line, did you changed
editors recently :-) ?
+1
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: [Libvir] [PATCH 1/2] Add internal API for managing capabilities data
On Tue, Feb 26, 2008 at 08:24:53AM -0500, Daniel Veillard wrote: On Tue, Feb 26, 2008 at 12:52:32PM +, Daniel P. Berrange wrote: On Tue, Feb 26, 2008 at 09:39:49AM +, Richard W.M. Jones wrote: Yes, this all looks good. Another advantage of doing this is that we can be sure that the capabilities XML generated by each driver will have the same schema. Yes indeed - this is a very good benefit, particularly as we extend the capabilities with more data like NUMA / migration / etc. At some point I think it'd be interesting to do a similar refactoring for domains. So we could have an internal virDomainDefPtr to store the definitions and a generic XML formatter / XML parser shared by all drivers. Yes I think that has been on the back of everybody's mind since the beginning, but we were not confident enough in the set of properties doing it internally would be good, then at some point we may even expose it in the API, but there is no hurry. Yes, if we keep the structs internal we can still change it at will. At worst we'd get compile errors for the drivers which could be trivially fixed. On the other hand cleaning up the domain code mess (incredible how simple things become complex as the code is extended) with new internal structures and APIs in the same way would be a really great thing. But that should probably wait a little bit I feel we are too close to the next release for this, Absolutely - i wasn't suggesting that we do it right now - there's plenty of other things to worry about first :-) The existing qemud_conf.h file contains a reasonable starting point for internal VM API, but would need a fair bit more work to be generic enough for the container based drivers. 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 -=| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] Re: RFC: Supporting serial parallel ports for QEMU (and improving Xen)
On Tue, Feb 26, 2008 at 08:01:11AM -0600, Charles Duffy wrote: Daniel P. Berrange wrote: Lots of difficult bits, but I'm starting to get onto coding it. Was this ever completed and merged? I don't see it documented at http://libvirt.org/format.html No, I never got a nice solution to the hard part of parsing QEMU's stdout to extract the name of the /dev/pts/XXX files allocated. Once that's done the rest is trivial. 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 -=| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] Implement virDomainBlockStats for QEMU/KVM
Daniel P. Berrange [EMAIL PROTECTED] wrote:
On Tue, Feb 26, 2008 at 03:38:56PM +0100, Jim Meyering wrote:
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index f97ef18..d9a7aca 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1315,8 +1315,9 @@ static void qemudDispatchVMEvent(int fd, int events,
void *opaque) {
qemudDispatchVMFailure(driver, vm, fd);
}
-static int qemudMonitorCommand(struct qemud_driver *driver ATTRIBUTE_UNUSED,
- struct qemud_vm *vm,
+static int qemudMonitorCommand(const struct qemud_driver *driver
+ ATTRIBUTE_UNUSED,
+ const struct qemud_vm *vm,
I don't like having annotations for a param on a separate line, because
it doesn't read naturally. If we really need to wrap this, then lets
follow the style of putting the return type on a line before , eg
static int
qemudMonitorCommand(const struct qemud_driver *driver ATTRIBUTE_UNUSED,
const struct qemud_vm *vm,
Good idea.
I prefer that anyhow, because it's what I'm used to and since
some naive start-of-function-finding tools work better that way.
If it is still over 80 characters with this convention, then so be it.
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH]check NULL of mac
Committed. 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 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] Implement virDomainBlockStats for QEMU/KVM
Thanks everyone. I've committed that with the suggested changes. 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 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH 0/2] Re-organize generation / handling of capabilities data
On Tue, Feb 26, 2008 at 08:29:54AM -0500, Daniel Veillard wrote: On Tue, Feb 26, 2008 at 04:57:56AM +, Daniel P. Berrange wrote: Earlier today I started the 'simple' task of extending the QEMU driver to support Xenner for running Xen paravirt guests under KVM. This was at first glance easy because it basically looks like QEMU - in fact all we needed was to extend the capabilities XML for the QEMU driver to include data about Xen guests if Xenner is available. Unfortunately this was easier said than done... - The code generating the XML in qemu_driver.c was a mix of string formatting for the XML, and functional logic for determining runtime capabilities (kvm, kqemu, etc). - More runtime logic was in the qemu_conf.c file and stuff that should be runtime logic was static. I attempted to fix up the QEMU code for capabilities but it quickly turned into a horrible mess of spaghetti. Looking at the Xen driver, the situation was pretty much the same, but with the capabilities XML generation spread out over 3 files (xml.c, xen_internal.c and xend_internal.c). So I decided that we needed to have a internal API structure to allow drivers to formally register query their capabilities , and also add a central method for serializing it to XML. The following 2 patches implement this idea, and porting the Xen, QEMU and Test drivers to use it. This removes a lot of code duplication in XML formatting and makes the resulting code easier to follow, and should make it much quicker to adding capabilities info to new drivers too. Okay I looked at both patches (too bad diff can't spot blocks moved from one file to another) they look fine to me. This really is a good cleanup, there are some changes included which seems a bit unrelated but fine too, +1 This is all committed now. Regards, 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 -=| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH]Change MAC address to case insensitive
OK, try this patch. I tried this patch. It works fine to me. Shigeki Sakamoto. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
