Re: [libvirt] [PATCH] ALL_LINGUAS: remove no, now that it's superseded by np.po

2008-10-20 Thread Daniel Veillard
On Sun, Oct 19, 2008 at 11:56:49AM +0200, Jim Meyering wrote:
 Without the first change below, make distcheck would fail due to the
 absence of po/no.po.  But that file was deliberately removed, because
 np.po supersedes it.

  Oops, right, I forgot to update the list

 However, there is no need to maintain that hard-coded list of .po files.
 Here's a proposed patch to do away with it:
 
 diff --git a/configure.in b/configure.in
 index 54f1fe1..32fffb2 100644
 --- a/configure.in
 +++ b/configure.in
 @@ -1011,12 +1011,7 @@ AM_CONDITIONAL([WITH_LIBVIRTD],[test x$with_libvirtd 
 = xyes])
  dnl Check for gettext
  AM_GNU_GETTEXT_VERSION([0.14.1])
  AM_GNU_GETTEXT([external])
 -if test -d po
 -then
 -ALL_LINGUAS=`(cd po  /dev/null  ls *.po) | sed 's+\.po$++'`
 -else
 -ALL_LINGUAS=af am ar as be bg bn_IN bn ca cs cy da de el en_GB es et 
 eu_ES fa fi fr gl gu he hi hr hu hy id is it ja ka kn ko ku lo lt lv mk ml mr 
 ms my nb nl nn nso or pa pl pt_BR pt ro ru si sk sl sq [EMAIL PROTECTED] sr 
 sv ta te th tr uk ur vi zh_CN zh_TW zu
 -fi
 +ALL_LINGUAS=`(cd $srcdir/po  /dev/null  ls *.po) | sed 's+\.po$++'`
 
  dnl Extra link-time flags for Cygwin.
  dnl Copied from libxml2 configure.in, but I removed mingw changes

  yes, that makes sense, +1

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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


Re: [libvirt] xen+ssh

2008-10-20 Thread Daniel P. Berrange
On Sun, Oct 19, 2008 at 08:14:30PM -0500, deface wrote:
 Trying to connect to remote host via ssh. keys work fine but get this
 
 --
 *Trace - *
 Unable to open connection to hypervisor URI 
 'xen+ssh://[EMAIL PROTECTED]/':
 class 'libvirt.libvirtError' virConnectOpenAuth() failed socket closed 
 unexpectedly
 Traceback (most recent call last):
  File /usr/share/virt-manager/virtManager/connection.py, line 430, in 
 _open_thread
None], flags)
  File /usr/lib/python2.5/site-packages/libvirt.py, line 98, in openAuth
if ret is None:raise libvirtError('virConnectOpenAuth() failed')
 libvirtError: virConnectOpenAuth() failed socket closed unexpectedly
 
 *Remote Log - *
 Oct 19 20:12:09 zeus sshd[1332]: Accepted publickey for root from 
 x.x.x.x port 52555 ssh2
 Oct 19 20:12:10 zeus sshd[1336]: pam_unix(sshd:session): session opened 
 for user root by (uid=0)
 
 not sure where to go - any way to enable verbose/debug logging?

This usually means that you don't have 'nc'  (aka netcat) installed
on the remote host, or that the 'libvirtd' daemon is not running.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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: [libvirt] PATCH 2/4: implement getVersion method for openvz

2008-10-20 Thread Daniel Veillard
On Tue, Oct 14, 2008 at 04:19:06PM +0100, Daniel P. Berrange wrote:
 This patch implements the getVersion driver method for openvz to report
 the version number of vzctl. This is needed in the next patch to determine
 if we have builtin support for bridges

  Looks fine except:

 +static int
 +openvzExtractVersionInfo(const char *cmd, int *retversion)
 +{
[...]
 +enum { MAX_HELP_OUTPUT_SIZE = 8192 };
 +int len = virFileReadLimFD(newstdout, MAX_HELP_OUTPUT_SIZE, help);

  Except that part which puzzles me. Why defining an enum for that. Also
enum is one of the worse part of C, it has no clear storage size
definition,  and virFileReadLimFD takes an input int anyway...

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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


Re: [libvirt] PATCH 4/4: support FS template config

2008-10-20 Thread Daniel P. Berrange
On Thu, Oct 16, 2008 at 11:28:40AM +0200, Jim Meyering wrote:
 Daniel P. Berrange [EMAIL PROTECTED] wrote:
  The root filesystem for an openvz guest is defined from a template name.
  We support this when creating a new guest, but never include this info
  when dumping the XML. Thsi patch addresses this problem by reading the
  OSTEMPLATE config parameter
 
  diff -r e0c166ce24bd src/openvz_conf.c
  --- a/src/openvz_conf.c Tue Oct 14 15:46:24 2008 +0100
  +++ b/src/openvz_conf.c Tue Oct 14 15:49:41 2008 +0100
  @@ -308,6 +308,47 @@ error:
   }
 
 
  +static int
  +openvzReadFSConf(virConnectPtr conn,
  + virDomainDefPtr def,
  + int veid) {
  +int ret;
  +virDomainFSDefPtr fs;
 
 This needs to be initialized here or in the 'if' block.
 Otherwise, it will be freed uninitialized upon read failure:
 
virDomainFSDefPtr fs = NULL;

Thanks, will fix that and then commit this.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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: [libvirt] PATCH 1/4: More generic MAC address handling

2008-10-20 Thread Daniel P. Berrange
On Thu, Oct 16, 2008 at 12:37:36PM +0200, Jim Meyering wrote:
 Daniel P. Berrange [EMAIL PROTECTED] wrote:
  This patch improves the MAC address handling.
 
  Currently our XML parser auto-generates a MAC addres using the KVM vendor
  prefix. This isn't much use for other drivers. This patch addresses this:
 
   - Stores each driver's vendor prefix in the capability object
   - Changes domain parser to use the per-driver vendor prefix for
 generating mac addresses
   - Adds more utility methods to util.c for parsing/generating/formatting
 MAC addresses
   - Updates each driver to record its vendor prefix for MAC address.
 
 This all looks fine, but I have a question about the moved code
 that makes up the new virGenerateMacAddr function:
 
  +void virGenerateMacAddr(const unsigned char *prefix,
  +unsigned char *addr)
  +{
  +addr[0] = prefix[0];
  +addr[1] = prefix[1];
  +addr[2] = prefix[2];
  +addr[3] = 1 + (int)(256*(rand()/(RAND_MAX+1.0)));
  +addr[4] = 1 + (int)(256*(rand()/(RAND_MAX+1.0)));
  +addr[5] = 1 + (int)(256*(rand()/(RAND_MAX+1.0)));
  +}
 
 I presume the intent is to generated numbers in 0..255,
 but that code generates them in 1..256 and relies on the
 truncate-to-8-bit-unsigned-char to map back to 0..255.
 
 Why are those 1 + there?
 It doesn't seem to hurt, but doesn't make sense (to me), either.
 Removing them would not change the results.

This was just moving code from elsewhere, not sure why it was like that
in the first place. Seems sensible to just remove the '1 +' bit.

 Also, unless there's a guarantee that the random number state is
 initialized elsewhere, it should be initialized here, like it was in
 the now-removed xenXMAutoAssignMac function.
 
srand((unsigned)time(NULL));
 
 Or maybe just initialize it once at start-up?

Could just add a call to srand() in virInitialize() i guess, although is
that a reasonable thing for a shared library to be doing, rather than 
leaving it upto the application ? 

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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: [libvirt] PATCH 3/4: SUpport bridge config for openvz

2008-10-20 Thread Daniel P. Berrange
On Thu, Oct 16, 2008 at 07:52:42PM +0400, Evgeniy Sokolov wrote:
 This implements support for bridge configs in openvz following the rules
 set out in
 
 http://wiki.openvz.org/Virtual_Ethernet_device#Making_a_bridged_veth-device_persistent
 
 This simply requires that the admin has created /etc/vz/vznetctl.conf
 containing
 
   #!/bin/bash
   EXTERNAL_SCRIPT=/usr/sbin/vznetaddbr
 
 
 For openvz = 3.0.22, we have to manually re-write the NETIF line to
 add the bridge config parameter. 
 It is alternative, but more flexible way. It require simple modification 
 of vznetaddbr.
 Common scenario is to add VZHOSTBR=bridge if to config.
 For newer openvz we can simply pass
 the bridge name on the commnand line to --netif_add.
 
 Older openvz also requires that the admin install /usr/sbin/vznetaddbr
 since it is not available out of the box
 
 Daniel
 
 
 
 +
 +while(1) {
 +if (openvz_readline(fd, line, sizeof(line)) = 0)
 +break;
 +
 +if (!STRPREFIX(line, param)) {
 need to check for '='.
 Currently, if you will search for 'NETIF', you can find any string with 
 such prefix. example 'NETIFOTHERPARAM'

Ahh, good point, will fix that.

 +if (safewrite(temp_fd, line, strlen(line)) !=
 +strlen(line))
 +goto error;
 +}
 +}
 +
  /*
 
 +
 +if (!(opt = virBufferContentAndReset(buf)))
 +goto no_memory;
 +
  ADD_ARG_LIT(opt) ;
 Need to free opt

Yes.

 -}else if (net-type == VIR_DOMAIN_NET_TYPE_ETHERNET 
 +} else if (net-type == VIR_DOMAIN_NET_TYPE_ETHERNET 
net-data.ethernet.ipaddr != NULL) {
 
 +static int
 +openvzDomainSetNetworkConfig(virConnectPtr conn,
 + virDomainDefPtr def)
 +{
 +unsigned int i;
 +virBuffer buf = VIR_BUFFER_INITIALIZER;
 +char *param;
 +struct openvz_driver *driver = (struct openvz_driver *) 
 conn-privateData;
 +
 +for (i = 0 ; i  def-nnets ; i++) {
 +if (driver-version  VZCTL_BRIDGE_MIN_VERSION  i  0)
 +virBufferAddLit(buf, ;);
 Need to check that network is bridge.
 In other case we will have NETIF='some params'

Opps, completely forgot that check

 +
 +if (openvzDomainSetNetwork(conn, def-name, def-nets[i], buf)  
 0) {
 +openvzError(conn, VIR_ERR_INTERNAL_ERROR,
 +%s, _(Could not configure network));
 +goto exit;
 +}
 +}
 +

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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: [libvirt] PATCH 4/4: support FS template config

2008-10-20 Thread Daniel Veillard
On Tue, Oct 14, 2008 at 04:22:35PM +0100, Daniel P. Berrange wrote:
 The root filesystem for an openvz guest is defined from a template name.
 We support this when creating a new guest, but never include this info
 when dumping the XML. Thsi patch addresses this problem by reading the
 OSTEMPLATE config parameter

  Except the fs initialization to NULL pointed by Jim looks fine,

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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


Re: [libvirt] PATCH 3/4: SUpport bridge config for openvz

2008-10-20 Thread Daniel P. Berrange
On Fri, Oct 17, 2008 at 03:58:05PM +0200, Jim Meyering wrote:
 Daniel P. Berrange [EMAIL PROTECTED] wrote:
 ...
  +int
  +openvzWriteConfigParam(int vpsid, const char *param, const char *value)
  +{
  +char conf_file[PATH_MAX];
  +char temp_file[PATH_MAX];
  +char line[PATH_MAX] ;
  +int fd, temp_fd;
  +
  +if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, conf)0)
  +return -1;
  +if (openvzLocateConfFile(vpsid, temp_file, PATH_MAX, tmp)0)
  +return -1;
  +
  +fd = open(conf_file, O_RDONLY);
  +if (fd == -1)
  +return -1;
  +temp_fd = open(temp_file, O_WRONLY | O_CREAT | O_TRUNC, 0644);
  +if (temp_fd == -1) {
  +close(fd);
  +return -1;
  +}
  +
  +while(1) {
  +if (openvz_readline(fd, line, sizeof(line)) = 0)
  +break;
  +
  +if (!STRPREFIX(line, param)) {
  +if (safewrite(temp_fd, line, strlen(line)) !=
  +strlen(line))
  +goto error;
  +}
  +}
  +
  +if (safewrite(temp_fd, param, strlen(param)) !=
  +strlen(param))
  +goto error;
  +if (safewrite(temp_fd, =\, 2) != 2)
  +goto error;
  +if (safewrite(temp_fd, value, strlen(value)) !=
  +strlen(value))
  +goto error;
  +if (safewrite(temp_fd, \\n, 2) != 2)
  +goto error;
 
 How about this instead, so the reader doesn't have to
 manually count string lengths and verify that the VAR in strlen(VAR)
 on the RHS matches the one on the LHS:
 
if (safewrite(temp_fd, param, strlen(param))  0 ||
safewrite(temp_fd, =\, 2)  0 ||
safewrite(temp_fd, value, strlen(value))  0 ||
safewrite(temp_fd, \\n, 2)  0)
goto error;

Yeah, that's good.

  +close(fd);
  +close(temp_fd);
 
 Officially, you should always check for failure when
 you've opened the file descriptor for writing.

Ok, will fix.

 
  +fd = temp_fd = -1;
  +
  +if (rename(temp_file, conf_file)  0)
  +goto error;
  +
  +return 0;
  +
  +error:
  +fprintf(stderr, damn %s\n, strerror(errno));
 
 How about mentioning the file name, too:
 
fprintf(stderr, failed to write %s: %s\n, conf_file,
strerror(errno));

That is debugging code I should have removed - we should raise a 
proper libvirt error if applicable.

  +
  +static int
  +openvzDomainSetNetworkConfig(virConnectPtr conn,
  + virDomainDefPtr def)
  +{
  +unsigned int i;
  +virBuffer buf = VIR_BUFFER_INITIALIZER;
  +char *param;
  +struct openvz_driver *driver = (struct openvz_driver *) 
  conn-privateData;
  +
  +for (i = 0 ; i  def-nnets ; i++) {
  +if (driver-version  VZCTL_BRIDGE_MIN_VERSION  i  0)
  +virBufferAddLit(buf, ;);
  +
  +if (openvzDomainSetNetwork(conn, def-name, def-nets[i], buf)  
  0) {
  +openvzError(conn, VIR_ERR_INTERNAL_ERROR,
  +%s, _(Could not configure network));
  +goto exit;
  +}
  +}
  +
  +param = virBufferContentAndReset(buf);
  +if (driver-version  VZCTL_BRIDGE_MIN_VERSION  def-nnets) {
  +if (openvzWriteConfigParam(strtoI(def-name), NETIF, param)  0) 
  {
  +VIR_FREE(param);
  +openvzError(conn, VIR_ERR_INTERNAL_ERROR,
  +%s, _(cannot replace NETIF config));
  +return -1;
  +}
  +}
  +
  +VIR_FREE(param);
  +return 0;
 
 param can be NULL and then dereferenced via openvzWriteConfigParam.
 How about this instead:
 
if (driver-version  VZCTL_BRIDGE_MIN_VERSION  def-nnets) {
char *param = virBufferContentAndReset(buf);
if (openvzWriteConfigParam(strtoI(def-name), NETIF, param)  0) 
 {
VIR_FREE(param);
openvzError(conn, VIR_ERR_INTERNAL_ERROR,
%s, _(cannot replace NETIF config));
return -1;
}
VIR_FREE(param);
}
 
return 0;
 
  +exit:
  +param = virBufferContentAndReset(buf);
  +VIR_FREE(param);
 
 Then free the above directly.

Yep, good idea.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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


[libvirt] Bridge network for VM using libvirt library

2008-10-20 Thread Shanmuga Rajan
Hi

  I trying to create an application to manage virtual
machines using libvirt library.
Currently i am focusing with KVM hypervisor and bridge networking.


I face two issues which i cant find a solution

First issue:

When i tried to reboot a virtual machine (using virDomain's reboot fn).
I got exception message that there was no support in the hypervisor
for reboot. is it true that we cant reboot a KVM based VM?


Second issue:


When i tried to setup a bridge network for KVM based VM.

I got error message  like this ..

QEMU quit during console startup
bind() failed

I have listed below my steps which i tried setup bridge network.

On Dom0
===



brctl addbr br0
ifconfig eth0 0.0.0.0
brctl addif br0 eth0
ifconfig br0 192.168.1.82 netmask 255.255.255.0 up
route add -net 192.168.1.0 netmask 255.255.255.0 br0
route add default gw 192.168.1.1 br0


** dom0 IP 192.168.1.82

after that i checked the network connection of the Dom0.. it was fine.
ifconfig  was fine.


then i created tun/tap device like given in the following link
http://wiki.centos.org/HowTos/KVM#head-c02a0b33e7949b0bc3b151ac6e0bdfb91b6bbd1c



sudo tunctl -b -u john
sudo ifconfig tap1 up
sudo brctl addif br0 tap1
export SDL_VIDEO_X11_DGAMOUSE=0
sudo iptables -I INPUT -i br0 -j ACCEPT


then i started to define vm like this...

domain type=kvm
nametest82/name
uuide5520db4-ad8a-38c3-b9d9-4e3c7a1052dc/uuid
memory131072/memory
vcpu1/vcpu
on_poweroffdestroy/on_poweroff
ostype arch=i686hvm/type
boot dev=hd/
/os
clock sync=localtime/
features
pae/
acpi/
apic/
/features
devices
emulator/usr/bin/qemu-kvm/emulator
disk type=file device=disk
source file=/dev/FluidVM_grp/Windows_NT_2003_121560222596/
target dev=hda/
/disk


interface type=bridge
source bridge=br0/
/interface

interface type=bridge
source bridge=br0/
target dev=tap1/
mac address=00:16:3e:8f:ca:4b /
/interface


graphics type=vnc port=5900 listen=0.0.0.0/
/devices
/domain



is anything i am missing in setting up the bridge network?


Thanks and Regards,
-- Shan

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


Re: [libvirt] PATCH: Pass 'remote_error' object to RPC handlers

2008-10-20 Thread Daniel Veillard
On Fri, Oct 17, 2008 at 12:27:52PM +0100, Daniel P. Berrange wrote:
 In the libvirtd daemon, remote.c file the current RPC handlers have 
 a return value contract saying
 
0 - success
   -1 - failure in libvirt call, no error returned
   -2 - failure in dispatch process, error already serialized  sent
 
 While this isn't a problem for the code as it stands today, for the thread
 support I want to be able to avoid the dispatch handlers having to touch
 the 'struct qemud_client' object in normal usage.  Allowing the RPC 
 handlers to directly serialize  send the error makes this impossible
 because it requires access to the client object.
 
 So this patch changes the way the RPC handlers deal with errors. The
 RPC handler API changes from
 
   typedef int (*dispatch_fn) (struct qemud_server *server,
   struct qemud_client *client,
   dispatch_args *args,
   dispatch_ret *ret);
 
 To
 
   typedef int (*dispatch_fn) (struct qemud_server *server,
   struct qemud_client *client,
   remote_error *err,
   dispatch_args *args,
   dispatch_ret *ret);
 
 Note, the addition of a 'remote_error *err' argument. Whenever an error
 occurs during the dispatch process, the RPC handler must populate this
 'remote_error *err' object with the error details. This rule applies
 whether its a libvirt error, or a dispatch process error, so there is
 no longer any need to have separate -1 or -2 return values, a simple
 -1 or 0 return value suffices.

  Sounds fine,

 @@ -1381,7 +1380,14 @@ static void qemudDispatchClientRead(stru
  if (client-bufferOffset  client-bufferLength)
  return; /* Not read enough */
  
 -remoteDispatchClientRequest (server, client);
 +if ((len = remoteDispatchClientRequest (server, client)) == 0)
 +qemudDispatchClientFailure(server, client);

  Shouldn't you return here instead of continuing ?

 +
 +/* Set up the output buffer. */
 +client-mode = QEMUD_MODE_TX_PACKET;
 +client-bufferLength = len;
 +client-bufferOffset = 0;
 +
  if (qemudRegisterClientEvent(server, client, 1)  0)
  qemudDispatchClientFailure(server, client);
  
[...]
 +static void
 +remoteDispatchFormatError (remote_error *rerr,
 +   const char *fmt, ...)
 +{
 +va_list args;
 +char msgbuf[1024];
 +char *msg = msgbuf;
 +
 +va_start (args, fmt);
 +vsnprintf (msgbuf, sizeof msgbuf, fmt, args);
 +va_end (args);
 +
 +remoteDispatchStringError (rerr, VIR_ERR_RPC, msg);
 +}

  Really no way we would get more than 1024 bytes ? The problem is that
if this happens it's usually a stack being printed and the useful info
can be at the end.

[...]
 -remoteDispatchError (client, req,
 - _(program mismatch (actual %x, expected %x)),
 - req.prog, REMOTE_PROGRAM);
 -xdr_destroy (xdr);
 -return;
 +remoteDispatchFormatError (rerr,
 +   _(program mismatch (actual %x, expected 
 %x)),
 +   req.prog, REMOTE_PROGRAM);

  stylistic issue, indenting to the opening ( is nice but can exceed the
  80 columns rule, I usually prefer dropping the ( alignment to fit in.


 @@ -1395,6 +1368,7 @@ remoteDispatchDomainMigratePrepare (stru
 args-flags, dname, args-resource);
  if (r == -1) {

  maybe if (r  0)  { as a mor generic error catching instruction

  VIR_FREE(uri_out);
 +remoteDispatchConnError(rerr, client-conn);
  return -1;
  }
  
 @@ -1439,7 +1413,10 @@ remoteDispatchDomainMigratePerform (stru
 args-uri,
 args-flags, dname, args-resource);
  virDomainFree (dom);
 -if (r == -1) return -1;
 +if (r == -1) {

  Same thing, we didn't defined virDrvDomainMigratePerform error code
so maybe it's better to be generic

[...]
 @@ -482,8 +482,12 @@ void virDomainRemoveInactive(virDomainOb
  memmove(doms-objs + i, doms-objs + i + 1,
  sizeof(*(doms-objs)) * (doms-count - (i + 1)));
  
 -if (VIR_REALLOC_N(doms-objs, doms-count - 1)  0) {
 -; /* Failure to reduce memory allocation isn't fatal */
 +if (doms-count  1) {
 +if (VIR_REALLOC_N(doms-objs, doms-count - 1)  0) {
 +; /* Failure to reduce memory allocation isn't fatal */
 +}
 +} else {
 +VIR_FREE(doms-objs);
  }
  doms-count--;

  Like this there is a couple case where new error handling blocks have
been added, but after having reviewed the full patch I really could not
find anything suspicious. 

Re: [libvirt] Question about supporting other hypervisor

2008-10-20 Thread Daniel Veillard
On Mon, Oct 20, 2008 at 06:16:14PM +0900, Atsushi SAKAI wrote:
 Hi, Daniel and all
 
 Thank you for various comments.
 These comments are very helpful for me.
 
 Daniel Veillard [EMAIL PROTECTED] wrote:
 
Atsushi do you know if by default VMWare ESX and Hyper-V allow CIM
  XML access, or if some 'additional' software need to be installed to
  get this to work ? Also if you have feedback about the quality of the
  client CIM APIs implemented (completeness, interoperability ...) that's
  good to know too,
 
 For VMware, there has CIM tool.
 http://www.vmware.com/download/sdk/
 
 For Hyper-V, there is a WMI.
 http://msdn.microsoft.com/en-us/library/aa490398.aspx
 
 By the way,
 I am not sure the quality of these CIM I/F implementation.
 But If there are have a possiblity, I will consider to check it.

  Well as Dan Smith pointed out this may get really tedious, especially
the asynchronous part of CIM processing.

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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


Re: [libvirt] Re: [PATCH 07/12] Domain Events - remote driver

2008-10-20 Thread Ben Guthro

 Just discovered one tiny problem here - need to check 'event' to see
 if the POLLHUP or POLLERR flags are set, and unregister the callback.
 Otherwise if you kill the server, the client will just spin on POLLHUP
 or ERR  forever. Something like this ought todo the trick
 
 if (event  (POLLERR | POLLHUP)) {
  virEventRemoveHandle(fd);
  return;
 }
 

I've been looking over the rest of your changes.
Generally, I agree all these suggestions are good ones...except for the code 
above

With this code in, I run the following test
1. start libvirtd
2. begin to monitor events with event-test
3. virsh create foo.xml

At this point, the event-test app encounters a HUP, or ERR, and stops 
monitoring for events - it will only ever get the Started event

I handle this in the event-test poll loop via

if ( pfd.revents  POLLHUP ) {
DEBUG0(Reset by peer);
return -1;
}


Is it not a reasonable restriction to require the client app to handle a Hangup?


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


Re: [libvirt] PATCH 1/4: More generic MAC address handling

2008-10-20 Thread Daniel Veillard
On Tue, Oct 14, 2008 at 04:17:57PM +0100, Daniel P. Berrange wrote:
 This patch improves the MAC address handling.
 
 Currently our XML parser auto-generates a MAC addres using the KVM vendor
 prefix. This isn't much use for other drivers. This patch addresses this:
 
  - Stores each driver's vendor prefix in the capability object
  - Changes domain parser to use the per-driver vendor prefix for
generating mac addresses
  - Adds more utility methods to util.c for parsing/generating/formatting
MAC addresses
  - Updates each driver to record its vendor prefix for MAC address.
 
 
 NB, for LXC driver we're 'borrowing' KVM's vendor prefix. 

  Looks fine to me. Jim's raised an interesting point though,

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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


Re: [libvirt] Re: [PATCH 07/12] Domain Events - remote driver

2008-10-20 Thread Daniel P. Berrange
On Fri, Oct 17, 2008 at 12:02:13PM -0400, Ben Guthro wrote:
 Deliver local callbacks in response to remote events
 
  remote_internal.c |  255 
 --
  1 file changed, 248 insertions(+), 7 deletions(-)

 diff --git a/src/remote_internal.c b/src/remote_internal.c
 index 35b7b4b..13537f7 100644
 --- a/src/remote_internal.c
 +++ b/src/remote_internal.c
 @@ -34,6 +34,7 @@
 +/** remoteDomainEventFired:
 + *
 + * The callback for monitoring the remote socket
 + * for event data
 + */
 +void remoteDomainEventFired(int fd ATTRIBUTE_UNUSED,
 + int event ATTRIBUTE_UNUSED,
 + void *opaque)
 +{
 +char buffer[REMOTE_MESSAGE_MAX];
 +char buffer2[4];
 +struct remote_message_header hdr;
 +XDR xdr;
 +int len;
 +
 +virConnectPtrconn = opaque;
 +struct private_data *priv = conn-privateData;
 +
 +DEBUG(%s : Event fired, __FUNCTION__);
 +
 +/* Read and deserialise length word. */
 +if (really_read (conn, priv, 0, buffer2, sizeof buffer2) == -1)
 +return;


Just discovered one tiny problem here - need to check 'event' to see
if the POLLHUP or POLLERR flags are set, and unregister the callback.
Otherwise if you kill the server, the client will just spin on POLLHUP
or ERR  forever. Something like this ought todo the trick

if (event  (POLLERR | POLLHUP)) {
 virEventRemoveHandle(fd);
 return;
}

before we try to read any data.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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: [libvirt] Re: [PATCH 07/12] Domain Events - remote driver

2008-10-20 Thread Daniel P. Berrange
On Mon, Oct 20, 2008 at 10:48:38AM -0400, Ben Guthro wrote:
 
  Just discovered one tiny problem here - need to check 'event' to see
  if the POLLHUP or POLLERR flags are set, and unregister the callback.
  Otherwise if you kill the server, the client will just spin on POLLHUP
  or ERR  forever. Something like this ought todo the trick
  
  if (event  (POLLERR | POLLHUP)) {
   virEventRemoveHandle(fd);
   return;
  }
  
 
 I've been looking over the rest of your changes.
 Generally, I agree all these suggestions are good ones...except for the code 
 above
 
 With this code in, I run the following test
 1. start libvirtd
 2. begin to monitor events with event-test
 3. virsh create foo.xml
 
 At this point, the event-test app encounters a HUP, or ERR, and stops
 monitoring for events - it will only ever get the Started event

That's a little odd - I'm not sure why 'event-test' would be getting
a HUP/ERR when 'virsh' starts a domain.

'event-test' should only get a HUP/ERR if it looses its socket connection
to the libvirtd server. Once you've lost the connection like this, the 
entire virConnectPtr is non-operative, and the client app needs to 
create a new virConnectPtr to re-connect. So removing the FD from the
event loop shouldn't result in us loosing any events - we're already
doomed there.to 

 I handle this in the event-test poll loop via
 
 if ( pfd.revents  POLLHUP ) {
 DEBUG0(Reset by peer);
 return -1;
 }

The integration into the event loop though is only something the app
will have general control off - eg, in the example libvirt-glib code
I posted its totally opaque to the app.

 Is it not a reasonable restriction to require the client app to handle a 
 Hangup?

Once the socket is dead, all subsequent libvirt call made on that 
virConnectPtr object will throw errors, which the app should see. 
Though if they're only ever waiting for events, and not making any
other calls, they'd not see it. Perhaps we could do with an explicit
callback for connection disconnect.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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: [libvirt] Re: [PATCH 07/12] Domain Events - remote driver

2008-10-20 Thread Ben Guthro
OK - it looks like my EventImpl was passing along the wrong bits.

I'll look into the token scheme suggested in an earlier email, and get this 
ready for re-submission.

Would you prefer a new patch series, as before - or another patch that modifies 
the prior series?




Daniel P. Berrange wrote on 10/20/2008 10:59 AM:
 On Mon, Oct 20, 2008 at 10:48:38AM -0400, Ben Guthro wrote:
 Just discovered one tiny problem here - need to check 'event' to see
 if the POLLHUP or POLLERR flags are set, and unregister the callback.
 Otherwise if you kill the server, the client will just spin on POLLHUP
 or ERR  forever. Something like this ought todo the trick

 if (event  (POLLERR | POLLHUP)) {
  virEventRemoveHandle(fd);
  return;
 }

 I've been looking over the rest of your changes.
 Generally, I agree all these suggestions are good ones...except for the code 
 above

 With this code in, I run the following test
 1. start libvirtd
 2. begin to monitor events with event-test
 3. virsh create foo.xml

 At this point, the event-test app encounters a HUP, or ERR, and stops
 monitoring for events - it will only ever get the Started event
 
 That's a little odd - I'm not sure why 'event-test' would be getting
 a HUP/ERR when 'virsh' starts a domain.
 
 'event-test' should only get a HUP/ERR if it looses its socket connection
 to the libvirtd server. Once you've lost the connection like this, the 
 entire virConnectPtr is non-operative, and the client app needs to 
 create a new virConnectPtr to re-connect. So removing the FD from the
 event loop shouldn't result in us loosing any events - we're already
 doomed there.to 
 
 I handle this in the event-test poll loop via

 if ( pfd.revents  POLLHUP ) {
 DEBUG0(Reset by peer);
 return -1;
 }
 
 The integration into the event loop though is only something the app
 will have general control off - eg, in the example libvirt-glib code
 I posted its totally opaque to the app.
 
 Is it not a reasonable restriction to require the client app to handle a 
 Hangup?
 
 Once the socket is dead, all subsequent libvirt call made on that 
 virConnectPtr object will throw errors, which the app should see. 
 Though if they're only ever waiting for events, and not making any
 other calls, they'd not see it. Perhaps we could do with an explicit
 callback for connection disconnect.
 
 
 Daniel

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


Re: [libvirt] Re: [PATCH 07/12] Domain Events - remote driver

2008-10-20 Thread Daniel P. Berrange
On Mon, Oct 20, 2008 at 11:43:24AM -0400, Ben Guthro wrote:
 OK - it looks like my EventImpl was passing along the wrong bits.

 I'll look into the token scheme suggested in an earlier email, and 
 get this ready for re-submission.
 
 Would you prefer a new patch series, as before - or another patch that
 modifies the prior series?

Best to just re-post the whole series with changes incorporated, rather 
than trying to add more patches ontop of patches.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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: [libvirt] Re: [PATCH 04/12] Domain Events - rpc changes

2008-10-20 Thread David Lively
On Sun, 2008-10-19 at 20:22 +0100, Daniel P. Berrange wrote:
 On Fri, Oct 17, 2008 at 11:58:15AM -0400, Ben Guthro wrote:
  Changes to the RPC protocol
  
  +struct remote_domain_event_ret {
  +remote_nonnull_domain dom;
  +int event;
  +unsigned long int callback;
  +unsigned long int user_data;
  +};
 
 Using a 'unsigned long int' field to transmit the raw pointer feels a little
 wrong to me. Could we have the client side pass a simple integer 'token' when
 registering / unregistering, and have that 'token' passed back by the server
 in the actual event. The client could use this token to lookup the callback
 and user_data. 

Hold on.  We can (and IMO should) quite easily avoid both this lookup
and the passing of the callback pointer to the server:

Suppose we have the same client registered for two different domain
event callbacks.  In the current patch, the server will send two RPCs
per event, one for each callback (which the client then unmarshals,
casts, and calls).

But what if we sent just one RPC per event ( per client) and had the
client walk its list of callbacks (which we'll need to track on the
client side anyway, if we're not sending the data over the wire)?  We
*always* make *all* the callbacks on the list, so there's no point in
making individual RPCs to fire off each callback individually.  This
gets rid of the need to send callback/user_data over the wire, and also
doesn't require tokenization (which is all just extra overhead in this
case).

remote_domain_events_register/deregister_args structs will go away.
remoteDispatchhDomainEventsRegister/Deregister will simply inc/dec a
value, sending events only when the value is 0.

While I'm not sure I've described this very well, I feel pretty strongly
that it's the right way to go.  If my explanation isn't clear, please
get back to me ...

Dave

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