10:49 a.m.
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
11:28 a.m.
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
9:49 a.m.
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
10:09 a.m.
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
10:27 a.m.
"Richard W.M. Jones" <rjones(a)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.
11:02 a.m.
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
4:43 a.m.
"Richard W.M. Jones" <rjones(a)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.
6:01 a.m.
On Tue, Feb 26, 2008 at 11:43:59AM +0100, Jim Meyering wrote:
"Richard W.M. Jones" <rjones(a)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
7:20 a.m.
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(a)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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
6113
days inactive
6115
days old
8 comments
4 participants
participants (4)
-
Daniel Veillard -
Hugh O. Brock -
Jim Meyering -
Richard W.M. Jones