Re: [Libvir] [PATCH] (for discussion) DHCP host mappings using 3 arrays API

2008-02-27 Thread Hugh O. Brock
On Mon, Feb 25, 2008 at 05:28:05PM +, Richard W.M. Jones wrote:
 On Mon, Feb 25, 2008 at 04:49:00PM +, 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

--
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

2008-02-27 Thread Richard W.M. Jones
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
Index: include/libvirt/libvirt.h.in
===
RCS file: /data/cvs/libvirt/include/libvirt/libvirt.h.in,v
retrieving revision 1.42
diff -u -r1.42 libvirt.h.in
--- include/libvirt/libvirt.h.in20 Feb 2008 14:57:39 -  1.42
+++ include/libvirt/libvirt.h.in27 Feb 2008 16:08:26 -
@@ -760,6 +760,27 @@
 intvirNetworkSetAutostart  (virNetworkPtr network,
 int autostart);
 
+/*
+ * DHCP host mappings
+ */
+int virNetworkNumOfDHCPHostMappings
+(virNetworkPtr network);
+int virNetworkListDHCPHostMappings
+(virNetworkPtr network,
+char **const hwaddrs,
+char **const ipaddrs,
+char **const hostnames,
+int maxmappings);
+
+int virNetworkAddDHCPHostMapping
+(virNetworkPtr network,
+const char *hwaddr,
+const char *ipaddr,
+const char *hostname,
+unsigned int flags);
+int virNetworkDeleteDHCPHostMapping
+(virNetworkPtr network,
+const char *hwaddr);
 
 /**
  * virStoragePool:
Index: qemud/remote.c
===
RCS file: /data/cvs/libvirt/qemud/remote.c,v
retrieving revision 1.24
diff -u -r1.24 remote.c
--- qemud/remote.c  22 Feb 2008 16:26:13 -  1.24
+++ qemud/remote.c  27 Feb 2008 16:08:28 -
@@ -2058,6 +2058,173 @@
 
 
 static int
+remoteDispatchNetworkNumOfDhcpHostMappings (struct qemud_server *server 
ATTRIBUTE_UNUSED,
+struct qemud_client *client,
+remote_message_header *req,
+
remote_network_num_of_dhcp_host_mappings_args *args,
+
remote_network_num_of_dhcp_host_mappings_ret *ret)
+{
+virNetworkPtr net;
+CHECK_CONN(client);
+
+net = get_nonnull_network (client-conn, args-net);
+if (net == NULL) {
+remoteDispatchError (client, req, network not found);
+return -2;
+}
+
+ret-num = virNetworkNumOfDHCPHostMappings (net);
+if (ret-num == -1) {
+virNetworkFree (net);
+return -1;
+}
+
+virNetworkFree (net);
+return 0;
+}
+
+
+static int
+remoteDispatchNetworkListDhcpHostMappings (struct qemud_server *server 
ATTRIBUTE_UNUSED,
+   struct qemud_client *client,
+   remote_message_header *req,
+   
remote_network_list_dhcp_host_mappings_args *args,
+   
remote_network_list_dhcp_host_mappings_ret *ret)
+{
+virNetworkPtr net;
+char **hwaddrs, **ipaddrs, **hostnames;
+int nr_mappings, i;
+CHECK_CONN(client);
+
+net = get_nonnull_network (client-conn, args-net);
+if (net == NULL) {
+remoteDispatchError (client, req, network not found);
+return -2;
+}
+
+if (args-maxmappings  REMOTE_NETWORK_DHCP_HOST_MAPPINGS_LIST_MAX) {
+remoteDispatchError (client, req,
+ maxmappings  
REMOTE_NETWORK_DHCP_HOST_MAPPINGS_LIST_MAX);
+return -2;
+}
+
+hwaddrs = malloc (args-maxmappings * sizeof (char *));
+if (!hwaddrs) {
+remoteDispatchSendError (client, req, VIR_ERR_NO_MEMORY, hwaddrs);
+return -2;
+}
+ipaddrs = malloc (args-maxmappings * sizeof (char *));
+if (!ipaddrs) {
+remoteDispatchSendError (client, req, VIR_ERR_NO_MEMORY, ipaddrs);
+free (hwaddrs);
+return -2;
+}
+hostnames = malloc (args-maxmappings * sizeof (char *));
+if (!hostnames) {
+remoteDispatchSendError (client, req, VIR_ERR_NO_MEMORY, hostnames);
+free (hwaddrs);
+free (ipaddrs);
+return -2;
+}
+
+nr_mappings = virNetworkListDHCPHostMappings (net,
+  hwaddrs, 

Re: [Libvir] [PATCH] (for discussion) DHCP host mappings using 3 arrays API

2008-02-27 Thread Jim Meyering
Richard W.M. Jones [EMAIL PROTECTED] 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 -  1.57
 +++ src/qemu_driver.c 27 Feb 2008 16:08:34 -
 @@ -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.

--
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

2008-02-27 Thread Richard W.M. Jones
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

--
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

2008-02-26 Thread Jim Meyering
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] (for discussion) DHCP host mappings using 3 arrays API

2008-02-26 Thread Richard W.M. Jones
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] (for discussion) DHCP host mappings using 3 arrays API

2008-02-26 Thread Daniel Veillard
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] (for discussion) DHCP host mappings using 3 arrays API

2008-02-25 Thread Richard W.M. Jones
On Mon, Feb 25, 2008 at 04:49:00PM +, 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

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list