[Libvir] [PATCH] (for discussion) DHCP host mappings using 3 arrays API
This patch is an evolution of the previous patch for implementing DHCP host mappings. The virNetworkListDHCPHostMappings calls has been changed to take three arrays of strings: int virNetworkListDHCPHostMappings (virNetworkPtr network, char **const hwaddrs, char **const ipaddrs, char **const hostnames, int maxmappings); Also I've fixed a number of bugs throughout, particularly in the way that the hosts file was being parsed. This patch is for discussion only. There is a segfault on the client side of (perhaps ironically) the virNetworkListDHCPHostMappings call. Been looking for about an hour but I can't see it yet ... All the other calls appear to work OK. 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 Mon, Feb 25, 2008 at 04:49:00PM +0000, Richard W.M. Jones wrote: [...] Thanks to Jim for pointing me in the right direction here. If you try this patch you'll need to insert memset() calls at the two places indicated below. That fixes all the problems I can see and it appears to all work fine now. Rich.
+static int +remoteNetworkNumOfDHCPHostMappings (virNetworkPtr network) +{ + remote_network_num_of_dhcp_host_mappings_args args; + remote_network_num_of_dhcp_host_mappings_ret ret; + GET_NETWORK_PRIVATE (network->conn, -1); + + make_nonnull_network (&args.net, network); +
memset (&ret, 0, sizeof ret);
+ if (call (network->conn, priv, 0, REMOTE_PROC_NETWORK_NUM_OF_DHCP_HOST_MAPPINGS, + (xdrproc_t) xdr_remote_network_num_of_dhcp_host_mappings_args, (char *) &args, + (xdrproc_t) xdr_remote_network_num_of_dhcp_host_mappings_ret, (char *) &ret) == -1) + return -1; + + return ret.num; +} + + +static int +remoteNetworkListDHCPHostMappings (virNetworkPtr network, + char **const hwaddrs, + char **const ipaddrs, + char **const hostnames, + int maxmappings) +{ + remote_network_list_dhcp_host_mappings_args args; + remote_network_list_dhcp_host_mappings_ret ret; + int i; + GET_NETWORK_PRIVATE (network->conn, -1); + + make_nonnull_network (&args.net, network); + args.maxmappings = maxmappings; +
memset (&ret, 0, sizeof ret);
+ if (call (network->conn, priv, 0, REMOTE_PROC_NETWORK_LIST_DHCP_HOST_MAPPINGS, + (xdrproc_t) xdr_remote_network_list_dhcp_host_mappings_args, (char *) &args, + (xdrproc_t) xdr_remote_network_list_dhcp_host_mappings_ret, (char *) &ret) == -1) + return -1;
-- 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
On Mon, Feb 25, 2008 at 05:28:05PM +0000, Richard W.M. Jones wrote:
On Mon, Feb 25, 2008 at 04:49:00PM +0000, Richard W.M. Jones wrote: [...]
Thanks to Jim for pointing me in the right direction here. If you try this patch you'll need to insert memset() calls at the two places indicated below. That fixes all the problems I can see and it appears to all work fine now.
Rich.
+1 If no one has any objections, can we go ahead and commit this? I'd like to use it for oVirt... --Hugh
Here's the final version of the "3 arrays" version of the patch, with the memory corruptor fixed. 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:
Here's the final version of the "3 arrays" version of the patch, with the memory corruptor fixed. ... Index: src/qemu_driver.c =================================================================== RCS file: /data/cvs/libvirt/src/qemu_driver.c,v retrieving revision 1.57 diff -u -r1.57 qemu_driver.c --- src/qemu_driver.c 27 Feb 2008 04:37:07 -0000 1.57 +++ src/qemu_driver.c 27 Feb 2008 16:08:34 -0000 @@ -925,6 +930,7 @@ dhcpStartDhcpDaemon(virConnectPtr conn, struct qemud_network *network) { + char buf[PATH_MAX]; char **argv; int ret, i;
@@ -934,6 +940,15 @@ return -1; }
+ /* Touch the DHCP hosts file so it exists before dnsmasq starts up. */ + snprintf (buf, sizeof buf, DHCP_HOSTS_FORMAT, network->def->name); + ret = open (buf, O_CREAT|O_WRONLY, 0644); + if (ret == -1) { + qemudReportError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, + "%s: %s", buf, strerror (errno)); + return -1; + } + argv = NULL; if (qemudBuildDnsmasqArgv(conn, network, &argv) < 0) return -1;
Whoops. You'll want to close that file descriptor. E.g., put this right before "argv = NULL;": if (close (ret)) return -1; It'd be nice to declare a new variable, say fd, for that bit.
On Wed, Feb 27, 2008 at 05:27:33PM +0100, Jim Meyering wrote:
Whoops. You'll want to close that file descriptor. E.g., put this right before "argv = NULL;":
if (close (ret)) return -1;
It'd be nice to declare a new variable, say fd, for that bit.
Oh yes, that would be a good idea :-) 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:
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.
On Tue, Feb 26, 2008 at 11:43:59AM +0100, Jim Meyering wrote:
"Richard W.M. Jones" <rjones@redhat.com> 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
On Tue, Feb 26, 2008 at 12:01:04PM +0000, Richard W.M. Jones wrote:
On Tue, Feb 26, 2008 at 11:43:59AM +0100, Jim Meyering wrote:
"Richard W.M. Jones" <rjones@redhat.com> 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/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (4)
-
Daniel Veillard -
Hugh O. Brock -
Jim Meyering -
Richard W.M. Jones