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

2008-10-21 Thread Jim Meyering
Daniel P. Berrange [EMAIL PROTECTED] wrote:
...
 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 ?

You're right that it's not reasonable to call srand from library code.
One alternative is to use something like coreutils' gnulib-style randint
module.  I'm looking into whether we can relax it's GPL license to LGPLv2+.
Using it or any other library-safe random_r-like interface would mean
maintaining and reusing an internal-to-libvirt random state buffer.

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


[libvirt] FYI; warning while compiling lxc_driver.c

2008-10-21 Thread Chris Lalancette
Just wanted to give a heads up that the LXC driver spits a warning when
compiling on i686; the exact warning is:

 gcc -DHAVE_CONFIG_H -I. -I.. -I../gnulib/lib -I../gnulib/lib -I../include
-I../include -I../qemud -I/usr/include/libxml2 -DBINDIR=\/usr/libexec\
-DSBINDIR=\/usr/sbin\ -DSYSCONF_DIR=\/etc\
-DLOCALEBASEDIR=\/usr/share/locale\ -DLOCAL_STATE_DIR=\/var\
-DGETTEXT_PACKAGE=\libvirt\ -Wall -Wformat -Wformat-security
-Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wextra -Wshadow
-Wcast-align -Wwrite-strings -Waggregate-return -Wstrict-prototypes -Winline
-Wredundant-decls -Wno-sign-compare -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fasynchronous-unwind-tables -DWITH_OPENVZ -DWITH_LXC -DWITH_QEMU -DWITH_TEST
-DWITH_REMOTE -DWITH_LIBVIRTD -DWITH_XEN -DIN_LIBVIRT -g -O2 -MT
libvirt_la-lxc_driver.lo -MD -MP -MF .deps/libvirt_la-lxc_driver.Tpo -c
lxc_driver.c  -fPIC -DPIC -o .libs/libvirt_la-lxc_driver.o
lxc_driver.c: In function 'lxcGetSchedulerParameters':
lxc_driver.c:1211: warning: dereferencing type-punned pointer will break
strict-aliasing rules

Generally I would go and fix such things, but I didn't spend any time to look at
the code, and I don't have an LXC environment handy to test on, so I didn't want
to break anything.  I just thought I would point it out.

-- 
Chris Lalancette

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


Re: [libvirt] [PATCH/RFC] qemu/kvm: allow to hot remove scsi/virtio disks

2008-10-21 Thread Daniel Veillard
On Fri, Oct 17, 2008 at 04:24:52PM +0200, Guido Günther wrote:
 On Fri, Oct 17, 2008 at 02:37:17PM +0200, Daniel Veillard wrote:
 [..snip..] 
Looks fine, i just removed a couple of extra spaces at end of line
  before commiting :-)
 Thanks a lot for applying this so quickly! Attached is a patch that
 allows for unplugging of disks. To do that I added a token to
 virDomainDiskDef that holds the PCI bus/slot number [1] so we can
 indentify the device on unplug. It's a union since other
 busses/hypervisors might have other requirements. The token is meant as
 an internal handle and will never show up in an XML and should probably
 move up into virDomainDeviceDef. Comments are welcome.

  In principle this looks just fine to me except a couple of stylistic
issues:

[...]
 +++ b/src/domain_conf.h
 @@ -84,6 +84,12 @@ struct _virDomainDiskDef {
  int type;
  int device;
  int bus;
 +union {
 +struct {
 +int bus;
 +int slot;
 +} pci;
 +} token;

 I'm not sure what the point of the token union is. Other adressing
mechanism may be used still I think having the structure not unioned
at this point makes things a bit clearer.

 +static int qemudDomainDetchPciDiskDevice(virDomainPtr dom, 
 virDomainDeviceDefPtr dev)

  Well I don't really see an improvement in using Detch instead of
  Detach, it's not like the identifier is much smaller, makes it less
  readable IMHO, ditto for qemudDomainDetchDevice()

[...]
 +if (detach-token.pci.slot  1 || detach-token.pci.bus != 0) {
 +qemudReportError(dom-conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
 + _(disk %s cannot be detached), detach-dst);
 +return -1;
 +}

  shouldn't the error be non PCI disk %s cannot be detached instead ?

 +ret = asprintf(cmd, pci_del 0 %d, detach-token.pci.slot);
 +if (ret == -1) {
 +qemudReportError(dom-conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
 +return ret;
 +}
 +
 +if (qemudMonitorCommand(driver, vm, cmd, reply)  0) {
 +qemudReportError(dom-conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
 + _(cannot detach disk %s), detach-dst);

  Live attach/detach operations are among the ones the harder to debug from
an user perspective I would suggest to try to be as explicit as possible
   failed to execute detach disk %s command

 +VIR_FREE(cmd);
 +return -1;
 +}
 +
 +DEBUG (pci_del reply: %s, reply);
 +/* If the command fails due to a wrong slot qemu prints: invalid slot, 
 + * nothing is printed on success */
 +if (strstr(reply, invalid slot)) {
 +qemudReportError (dom-conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
 +  _(cannot detach disk %s), detach-dst);


same thing if we have a hint about why this failed ...
cannot detach disk %s : invalid slot

[...]
 +static int qemudDomainDetchDevice(virDomainPtr dom,
 +   const char *xml) {

  Detach :-)


 +if (dev-type == VIR_DOMAIN_DEVICE_DISK 
 +dev-data.disk-device == VIR_DOMAIN_DISK_DEVICE_DISK 
 +(dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_SCSI ||
 + dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO))
 +ret = qemudDomainDetchPciDiskDevice(dom, dev);
 +else {
 +qemudReportError(dom-conn, dom, NULL, VIR_ERR_NO_SUPPORT,
 + %s, _(this device type cannot be attached));

 or even better turn this positively as
   only SCSI or virtio disk device can be attached dynamically


  Those are just stylistic issues, I can apply the patch with those
  changed if you wish if you don't have time for a new patch,

thanks,

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] image locking?

2008-10-21 Thread Perry Myers

Itamar Heim wrote:

Hi,

 


I am interested to find out how libvirt envisions image locking.

i.e., how do we make sure multiple nodes are not trying to access the 
same storage volume, probably causing image corruption.


 

I know this can be solved by means of a cluster, but it seems excessive 
(and not possible in all scenarios).


Central administration (ovirt-like) is also problematic, since unless 
fencing is assumed, it cannot verify the image is no longer being used.


In an oVirt network, this shouldn't be a problem.  Storage can only be 
assigned to one VM at a time presently.  (In the future we may relax this 
for clustered filesystems, but shared storage will be marked as such).


Regardless of whether or not a VM is active/inactive, once an iSCSI LUN, 
disk image or otherwise is assigned to a VM it can't be used by other VMs. 
 The storage is not released to the available list until the VM using it 
is both destroyed and undefined.  We don't allow undefine until the VM has 
been destroyed and we won't confirm that a VM has been destroyed if we 
can't contact the host that it is running on to confirm.


Now... if you start creating VMs on an oVirt network outside of the oVirt 
Server or decide to share your storage pools between an oVirt network and 
a non-oVirt network that is problematic.  Our solution for that for the 
time being is 'just don't do that'. :)


Perry

--
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-21 Thread Ben Guthro
This is very similar to what I had in the original patch,
where on the server side, we just increment/decrement a callback_registered 
counter,
then keep the list of callbacks/opaque data on the client.

The server would then send one rpc to each connected client, whose job it would 
be
to multiplex this out to all registered callbacks.

There would still be a one-for-one for register/deregister, but this scheme 
has the following advantages for efficency:
a.) Fewer RPC calls (less data on the wire - one-to-many for events firing)
b.) Less RPC data passed on register/deregister (no need for cb/user_data)
c.) Code is simpler - no need to invent yet another data structure, and list 
for tracking tokens

Daniel - what are your thoughts on this? 
This is similar to what I had originally implemented, but I'm not sure if you 
objected to this part of it, or not...

Ben


David Lively wrote on 10/20/2008 04:05 PM:
 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


Re: [libvirt] [PATCH/RFC] qemu/kvm: allow to hot remove scsi/virtio disks

2008-10-21 Thread Daniel P. Berrange
On Fri, Oct 17, 2008 at 04:24:52PM +0200, Guido G?nther wrote:
 On Fri, Oct 17, 2008 at 02:37:17PM +0200, Daniel Veillard wrote:
 [..snip..] 
Looks fine, i just removed a couple of extra spaces at end of line
  before commiting :-)
 Thanks a lot for applying this so quickly! Attached is a patch that
 allows for unplugging of disks. To do that I added a token to
 virDomainDiskDef that holds the PCI bus/slot number [1] so we can
 indentify the device on unplug. It's a union since other
 busses/hypervisors might have other requirements. The token is meant as
 an internal handle and will never show up in an XML and should probably
 move up into virDomainDeviceDef. Comments are welcome.

The thing I'm not happy about with this scheme, is that it only works
if you previously hot-attached the disk during the lifetime of this
particular execution of the VM.   If you had a disk specified on the
command line with -driver, then it is not able to unplug it. Having
a detach command that may work or may fail depending on how the disk
was previously configured is not very nice.

Is there not some 'info pci' or 'info disk' command we cna use to find
out the PCI slot number at the time we want to detach the device. This
would make it work for all disks, and avoid the need to track the 
state in that virDomainDiskDef union

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-21 Thread Daniel P. Berrange
On Tue, Oct 21, 2008 at 08:57:51AM -0400, Ben Guthro wrote:
 This is very similar to what I had in the original patch,
 where on the server side, we just increment/decrement a callback_registered 
 counter,
 then keep the list of callbacks/opaque data on the client.
 
 The server would then send one rpc to each connected client, whose job it 
 would be
 to multiplex this out to all registered callbacks.
 
 There would still be a one-for-one for register/deregister, but 
 this scheme has the following advantages for efficency:
 a.) Fewer RPC calls (less data on the wire - one-to-many for events firing)
 b.) Less RPC data passed on register/deregister (no need for cb/user_data)
 c.) Code is simpler - no need to invent yet another data structure, and list 
 for tracking tokens
 
 Daniel - what are your thoughts on this? 
 This is similar to what I had originally implemented, but I'm not
 sure if you objected to this part of it, or not...

The bit I didn't like about the original was the extra logic on the server
side of the impl. If we can keep the explicit register  unregister RPC
calls, and require the client todo all ref-counting  multiplexing, then
I wouldn't mind this approach.

eg, on the client side, on the first 'register' API call, send
the 'register' RPC call. Then just count subsequent 'register'
API calls, and don't bother with the 'register' RPC call. Once
the 'unregister' has been called the matching number of times
it can send the 'unregister' the RPC call. 

So the 'remote_internal.c' would have to take care of broadcasting
the single incoming event, to all registered listeners of that 
particular client.

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] Re: [RFC] sVirt v0.10 - initial prototype

2008-10-21 Thread Daniel J Walsh
James Morris wrote:
 This is a request for comments on the initial prototype release of sVirt, 
 a project to add security labeling support to Linux-based virtualization.
 
 Project page: 
   http://www.selinuxproject.org/page/SVirt
 
 Previous libvirt discussions:
 
   High-level requirements:
 https://www.redhat.com/archives/libvir-list/2008-August/msg00255.html
 
   XML security labels:
 https://www.redhat.com/archives/libvir-list/2008-August/msg00740.html
 
 A patch for libvirt is attached; and also included in a release tarball at 
 http://namei.org/svirt/.  See 'readme.txt' there for more details on 
 building and running the code.
 
 The purpose of this release is to establish a proof of concept of applying 
 security labels to VMs, and for discussion of the underlying technical 
 approach.
 
 With this release, it is possible to define a security label for a 
 kvm/qemu domain in its XML configuration ('virsh edit'), launch the domain 
 and have it transition to the specified security label ('virsh start'), 
 then query the security label of the running domain ('virsh dominfo').
 
 The following changes were made to libvirt:
 
 1. Implementing a pluggable security label driver framework;
 
 2. Implementing an SELinux security label driver for (1);
 
 3. Wiring the security label framework into the Qemu driver;
 
 4. Implementing basic libvirt API calls for initializing the driver, 
and getting/setting domain security labels;
 
 5. Extending the domain XML configuration to include security labels;
 
 6. Adding domain security label display/edit/dump support to virsh.
 
 One of the design principles I've followed with this is to reduce or 
 eliminate configuration wherever possible.  If a variety of security 
 labeling drivers are present, libvirtd automatically detects which one to 
 enable and enables it.  e.g. if SELinux is enabled on the system, the 
 SELinux labeling driver is enabled automatically when livbirtd starts.
 
 Another is to treat security labels as opaque unless they're actually 
 being used for security purposes (e.g. to launch the domain).  So, virsh 
 and the domain configuration code currently do not need to semantically 
 interpet security labels, just understand their format.  This should suit 
 the initial simple goal of isolated domains, which only requires security 
 labels to be distinct.
 
 The domain security label configuration format is as follows:
 
 # virsh dumpxml sys1
 domain
 
   seclabel model='selinux'
 labelsystem_u:system_r:virtd_t:s0/label
 policytypetargeted/policytype
   /seclabel
 /domain
 
 It's possible to query the security label of a running domain via virsh:
 
 # virsh dominfo sys1
 Id: 1
 Name:   sys1
 UUID:   fa3c8e06-0877-2a08-06fd-f2479b7bacb4
 OS Type:hvm
 State:  running
 CPU(s): 1
 CPU time:   11.4s
 Max memory: 524288 kB
 Used memory:524288 kB
 Autostart:  disable
 Security label: system_u:system_r:virtd_t:s0 (selinux/targeted/enforcing)
 
 The security label is deterimed via the new virDomainGetSecLabel() API 
 method, which is transported over RPC to the backend driver (qemu in this 
 case), and is entirely independent of the local security model, if any.  
 e.g. this command could be run remotely from an entirely different 
 platform: you just see what's happening on the remote system, as with 
 other attributes of the domain.
 
 Feedback on the design thus far is sought before proceeding to more 
 comprehensive integration.
 
 In particular, I'd be interested in any thoughts on the placement of the 
 security labeling driver within libvirt, as this seems to be the most 
 critical architectural issue (I've already refactored this aspect once).  
 
 Currently, the idea is to attach the security labeling driver to the virt 
 driver, rather than implement it independently as a top-level component as 
 in the case of other types of drivers (e.g. storage).  This is because 
 process-based security labeling is highly dependent on the kind of 
 virtualization in use, and may not make sense at all in some cases (e.g. 
 when using a non-Linux hypervisor, or containers).
 
 In the case of qemu, a security labeling driver is added to qemud:
 
 @@ -63,6 +64,7 @@ struct qemud_driver {
  char *vncListen;
  
  virCapsPtr caps;
 +virSecLabelDriverPtr secLabelDriver;
  };
 
 and then initialized during qemud startup from qemudSecLabelInit().  
 
 During initialization, any available security labeling drivers are probed, 
 and the first one which thinks it should be used is installed. Top-level 
 libvirt API calls are then dispatched to the active security labeling 
 driver via the backend virt driver, as necessary.
 
 Note that the security labeling framework in this release is always 
 built-in -- it can be made a compile-time option later if desired.
 
 Requirements not yet addressed include:
 - Labeling of resources and generally comprehensive 

[libvirt] [PATCH 0/5]: Logical backend fixes and cleanups

2008-10-21 Thread Chris Lalancette
All,
This series of 5 patches fixes a couple of issues with the logical backend
implementation, and also extends the xml returned by
find-storage-pool-sources-as logical to also return the device's that a
logical backend is made of.  Patches 1 - 4 do the cleanup, while patch 5
implements the device addition to the find-storage-pool-sources.

-- 
Chris Lalancette

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


[libvirt] [PATCH 1/5]: Shore up virGetLastError handling

2008-10-21 Thread Chris Lalancette
While doing testing on this patch series, I mistakenly added a bogus piece of
storage XML to libvirtd, which was saved in /etc/libvirt/storage.  On subsequent
stop/start of libvirtd, because of another bug (fixed in a later patch), an
error wasn't being set properly in an error path, so libvirtd was SEGV'ing in
storage_conf.c:virStoragePoolObjLoad when trying to dereference the NULL err
returned from virGetLastError().  Make this more robust against errors by always
doing err ? err-message : NULL in the printf.  I looked around the tree and
found a couple of other places that weren't guarded, so this patch fixes them as
well.

Signed-off-by: Chris Lalancette [EMAIL PROTECTED]
diff -up ./src/qemu_driver.c.orig ./src/qemu_driver.c
--- ./src/qemu_driver.c.orig	2008-10-21 14:42:35.0 +0200
+++ ./src/qemu_driver.c	2008-10-21 14:43:05.0 +0200
@@ -136,7 +136,8 @@ qemudAutostartConfigs(struct qemud_drive
 qemudStartVMDaemon(NULL, driver, driver-domains.objs[i], NULL)  0) {
 virErrorPtr err = virGetLastError();
 qemudLog(QEMUD_ERR, _(Failed to autostart VM '%s': %s\n),
- driver-domains.objs[i]-def-name, err-message);
+ driver-domains.objs[i]-def-name,
+ err ? err-message : NULL);
 }
 }
 }
diff -up ./src/storage_backend_logical.c.orig ./src/storage_backend_logical.c
diff -up ./src/storage_conf.c.orig ./src/storage_conf.c
--- ./src/storage_conf.c.orig	2008-10-21 14:43:32.0 +0200
+++ ./src/storage_conf.c	2008-10-21 14:43:47.0 +0200
@@ -1043,7 +1043,7 @@ virStoragePoolObjLoad(virConnectPtr conn
 if (!(def = virStoragePoolDefParse(NULL, xml, file))) {
 virErrorPtr err = virGetLastError();
 virStorageLog(Error parsing storage pool config '%s' : %s,
-  path, err-message);
+  path, err ? err-message : NULL);
 return NULL;
 }
 
diff -up ./src/network_driver.c.orig ./src/network_driver.c
--- ./src/network_driver.c.orig	2008-10-21 14:41:43.0 +0200
+++ ./src/network_driver.c	2008-10-21 14:42:18.0 +0200
@@ -98,7 +98,8 @@ networkAutostartConfigs(struct network_d
 networkStartNetworkDaemon(NULL, driver, driver-networks.objs[i])  0) {
 virErrorPtr err = virGetLastError();
 networkLog(NETWORK_ERR, _(Failed to autostart network '%s': %s\n),
-   driver-networks.objs[i]-def-name, err-message);
+   driver-networks.objs[i]-def-name,
+   err ? err-message : NULL);
 }
 }
 }
diff -up ./src/storage_driver.c.orig ./src/storage_driver.c
--- ./src/storage_driver.c.orig	2008-10-21 14:44:38.0 +0200
+++ ./src/storage_driver.c	2008-10-21 14:44:54.0 +0200
@@ -232,7 +232,7 @@ storageDriverShutdown(void) {
 backend-stopPool(NULL, pool)  0) {
 virErrorPtr err = virGetLastError();
 storageLog(Failed to stop storage pool '%s': %s,
-   pool-def-name, err-message);
+   pool-def-name, err ? err-message : NULL);
 }
 virStoragePoolObjClearVols(pool);
 }
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 2/5]: Properly set errors on unknown format types

2008-10-21 Thread Chris Lalancette
Because of my patch last week that converted the various virStorage*FromString
and virStorage*ToString implementations to the generic VIR_ENUM_IMPL, there were
a couple of places that didn't properly set errors when they failed.  This patch
fixes these places up.  It also adds an additional check in virEnumFromString(),
so that if a NULL type is passed in, it fails instead of SEGV'ing.

Signed-off-by: Chris Lalancette [EMAIL PROTECTED]
diff -up ./src/storage_conf.c.p1 ./src/storage_conf.c
--- ./src/storage_conf.c.p1	2008-10-21 14:48:10.0 +0200
+++ ./src/storage_conf.c	2008-10-21 14:56:59.0 +0200
@@ -276,6 +276,8 @@ virStoragePoolDefParseDoc(virConnectPtr 
 if (options-formatFromString) {
 char *format = virXPathString(conn, string(/pool/source/format/@type), ctxt);
 if ((ret-source.format = (options-formatFromString)(format))  0) {
+virStorageReportError(conn, VIR_ERR_XML_ERROR,
+  _(unknown pool format type %s), format);
 VIR_FREE(format);
 goto cleanup;
 }
@@ -521,8 +526,12 @@ virStoragePoolDefFormat(virConnectPtr co
 
 if (options-formatToString) {
 const char *format = (options-formatToString)(def-source.format);
-if (!format)
+if (!format) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+  _(unknown pool format number %d),
+  def-source.format);
 goto cleanup;
+}
 virBufferVSprintf(buf,format type='%s'/\n, format);
 }
 
@@ -751,6 +760,8 @@ virStorageVolDefParseDoc(virConnectPtr c
 if (options-formatFromString) {
 char *format = virXPathString(conn, string(/volume/target/format/@type), ctxt);
 if ((ret-target.format = (options-formatFromString)(format))  0) {
+virStorageReportError(conn, VIR_ERR_XML_ERROR,
+  _(unknown volume format type %s), format);
 VIR_FREE(format);
 goto cleanup;
 }
@@ -885,8 +896,12 @@ virStorageVolDefFormat(virConnectPtr con
 
 if (options-formatToString) {
 const char *format = (options-formatToString)(def-target.format);
-if (!format)
+if (!format) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+  _(unknown volume format number %d),
+  def-target.format);
 goto cleanup;
+}
 virBufferVSprintf(buf,format type='%s'/\n, format);
 }
 
diff -up ./src/util.c.p1 ./src/util.c
--- ./src/util.c.p1	2008-10-21 14:59:37.0 +0200
+++ ./src/util.c	2008-10-21 15:00:07.0 +0200
@@ -1018,6 +1018,10 @@ int virEnumFromString(const char *const*
   const char *type)
 {
 unsigned int i;
+
+if (type == NULL)
+return -1;
+
 for (i = 0 ; i  ntypes ; i++)
 if (STREQ(types[i], type))
 return i;
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 3/5]: Set format type to LVM2 on NULL format

2008-10-21 Thread Chris Lalancette
Again because of my generic VIR_ENUM_IMPL patch last week, I unwittingly caused
a regression in the storage_backend_logical driver.  Previously, if you
submitted logical pool XML that had no sourceformat type='lvm2'/ string, it
would just default to VIR_STORAGE_POOL_LOGICAL_LVM2.  This would succeed just
fine and go on with life.  This is no longer the case, and now XML without the
format tag fails to define.  To maintain backwards compatibility with already
existing XML that expects this, add a compatibility wrapper to return
VIR_STORAGE_POOL_LOGICAL_LVM2 on an empty format tag.

Signed-off-by: Chris Lalancette [EMAIL PROTECTED]
diff -up ./src/storage_backend_logical.c.p2 ./src/storage_backend_logical.c
--- ./src/storage_backend_logical.c.p2	2008-10-21 14:53:16.0 +0200
+++ ./src/storage_backend_logical.c	2008-10-21 14:53:59.0 +0200
@@ -49,6 +49,19 @@ VIR_ENUM_IMPL(virStorageBackendLogicalPo
   VIR_STORAGE_POOL_LOGICAL_LAST,
   unknown, lvm2);
 
+static int virStorageBackendLogicalFormatFromStringWrap(const char *format)
+{
+/* 
+ * Sigh.  For compatibility purposes, we need to return
+ * VIR_STORAGE_POOL_LOGICAL_LVM2 when we get a NULL format
+ */
+
+if (format == NULL)
+return VIR_STORAGE_POOL_LOGICAL_LVM2;
+
+return virStorageBackendLogicalPoolTypeFromString(format);
+}
+
 static int
 virStorageBackendLogicalSetActive(virConnectPtr conn,
   virStoragePoolObjPtr pool,
@@ -616,7 +629,7 @@ virStorageBackend virStorageBackendLogic
 .poolOptions = {
 .flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_NAME |
   VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE),
-.formatFromString = virStorageBackendLogicalPoolTypeFromString,
+.formatFromString = virStorageBackendLogicalFormatFromStringWrap,
 .formatToString = virStorageBackendLogicalPoolTypeToString,
 },
 
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 4/5]: Make dumpXML more idempotent with defineXML

2008-10-21 Thread Chris Lalancette
Currently, you can define a logical storage pool with something like the 
following:

pool type='logical'
source
nameMyVG/name
device name='/dev/sdb'/
...

However, dumping out the XML for this same storage pool (with, say, virsh
pool-dumpxml), gives:

pool type='logical'
source
nameMyVG/name
device name='/dev/sdb'
/device


To make this more idempotent, do the device name='/dev/sdb'/ form by default,
and only do the device.../device form if .nfreeExtent is defined for the
storage pool.

Signed-off-by: Chris Lalancette [EMAIL PROTECTED]
diff -up ./src/storage_conf.c.p3 ./src/storage_conf.c
--- ./src/storage_conf.c.p3	2008-10-21 14:55:14.0 +0200
+++ ./src/storage_conf.c	2008-10-21 14:56:59.0 +0200
@@ -466,7 +466,7 @@ virStoragePoolDefFormat(virConnectPtr co
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 const char *type;
 char uuid[VIR_UUID_STRING_BUFLEN];
-int i;
+int i, j;
 
 options = virStorageBackendPoolOptionsForType(def-type);
 if (options == NULL)
@@ -499,16 +499,19 @@ virStoragePoolDefFormat(virConnectPtr co
 if ((options-flags  VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE) 
 def-source.ndevice) {
 for (i = 0 ; i  def-source.ndevice ; i++) {
-virBufferVSprintf(buf,device path='%s'\n, def-source.devices[i].path);
 if (def-source.devices[i].nfreeExtent) {
-int j;
+virBufferVSprintf(buf,device path='%s'\n,
+  def-source.devices[i].path);
 for (j = 0 ; j  def-source.devices[i].nfreeExtent ; j++) {
 virBufferVSprintf(buf, freeExtent start='%llu' end='%llu'/\n,
   def-source.devices[i].freeExtents[j].start,
   def-source.devices[i].freeExtents[j].end);
 }
+virBufferAddLit(buf,/device\n);
 }
-virBufferAddLit(buf,/device\n);
+else
+virBufferVSprintf(buf, device path='%s'/\n,
+  def-source.devices[i].path);
 }
 }
 if ((options-flags  VIR_STORAGE_BACKEND_POOL_SOURCE_DIR) 
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 5/5]: Export device when doing logical pool discovery

2008-10-21 Thread Chris Lalancette
When doing logical pool discovery, it currently returns XML like:

sourcessourcenamemyvg/name/source/sources

However, for oVirt, we need to know the device name that corresponds to this
logical volume group as well.  Therefore, extend the
virStorageBackendLogicalFindPoolSources function to return this information as
well.  This required pretty much a complete rewrite of the discovery function,
since I needed to be able to add device sections to already existing XML
nodes.  For that reason, this code uses direct libxml2 tree API's to handle the
parsing and the adding of nodes where necessary.

Signed-off-by: Chris Lalancette [EMAIL PROTECTED]
diff -up ./src/storage_backend_logical.c.p4 ./src/storage_backend_logical.c
--- ./src/storage_backend_logical.c.p4	2008-10-21 15:01:50.0 +0200
+++ ./src/storage_backend_logical.c	2008-10-21 15:06:55.0 +0200
@@ -23,6 +23,8 @@
 
 #include config.h
 
+#include libxml/xmlsave.h
+
 #include sys/wait.h
 #include sys/stat.h
 #include stdio.h
@@ -255,31 +257,77 @@ virStorageBackendLogicalRefreshPoolFunc(
 
 
 static int
-virStorageBackendLogicalFindPoolSourcesFunc(virConnectPtr conn ATTRIBUTE_UNUSED,
+virStorageBackendLogicalFindPoolSourcesFunc(virConnectPtr conn,
 virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
 char **const groups,
 void *data)
 {
-virStringList **rest = data;
-virStringList *newItem;
-const char *name = groups[0];
+const char *const pvname = groups[0];
+const char *const vgname = groups[1];
+char *content;
+xmlDoc *xml = data;
+xmlNode *root;
+xmlNode *device;
+xmlNode *curr;
+xmlNode *child;
+xmlNode *topNode;
+int vgmatch;
+
+root = xmlDocGetRootElement(xml);
+if (root == NULL) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+  _(Unexpected NULL root element));
+return -1;
+}
 
-/* Append new XML desc to list */
+/*
+ * The idea here is to try to find this vgname in the current XML.  If it
+ * already exists, then we just add a new device section to it.  If it
+ * doesn't yet exist, then we create new sourcenamefoo/name/source,
+ * then add the device section to it.
+ */
+topNode = NULL;
+curr = root-children;
+while (curr != NULL) {
+child = curr-children;
+while (child != NULL) {
+content = (char *)xmlNodeGetContent(child);
+vgmatch = STREQ(vgname, content);
+free(content);
+if (STREQ((char *)child-name, name)  vgmatch) {
+topNode = curr;
+break;
+}
+child = child-next;
+}
+curr = curr-next;
+}
 
-if (VIR_ALLOC(newItem) != 0) {
-virStorageReportError(conn, VIR_ERR_NO_MEMORY, %s, _(new xml desc));
-return -1;
+if (topNode == NULL) {
+topNode = xmlNewChild(root, NULL, BAD_CAST source, NULL);
+if (topNode == NULL) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+  _(Failed to create new XML source child));
+return -1;
+}
+if (xmlNewChild(topNode, NULL, BAD_CAST name, BAD_CAST vgname) == NULL) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+  _(Failed to create new XML name child));
+return -1;
+}
 }
 
-if (asprintf(newItem-val, sourcename%s/name/source, name) = 0) {
-virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, %s, _(asprintf failed));
-VIR_FREE(newItem);
+device = xmlNewChild(topNode, NULL, BAD_CAST device, NULL);
+if (device == NULL) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+  _(Failed to create new XML device child));
+return -1;
+}
+if (xmlSetNsProp(device, NULL, BAD_CAST path, BAD_CAST pvname) == NULL) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+  _(Failed to add device XML path property));
 return -1;
 }
-
-newItem-len = strlen(newItem-val);
-newItem-next = *rest;
-*rest = newItem;
 
 return 0;
 }
@@ -290,36 +338,76 @@ virStorageBackendLogicalFindPoolSources(
 unsigned int flags ATTRIBUTE_UNUSED)
 {
 /*
- * # vgs --noheadings -o vg_name
- *   VolGroup00
- *   VolGroup01
+ * # pvs --noheadings -o pv_name,vg_name
+ *   /dev/sdb
+ *   /dev/sdc VolGroup00
  */
 const char *regexes[] = {
-^\\s*(\\S+)\\s*$
+^\\s*(\\S+)\\s+(\\S+)\\s*$
 };
 int vars[] = {
-1
+2
 };
-virStringList *descs = NULL;
-const char *prog[] = { VGS, --noheadings, -o, vg_name, NULL };
+const char *const prog[] = { PVS, --noheadings, -o, 

Re: [libvirt] [PATCH 1/5]: Shore up virGetLastError handling

2008-10-21 Thread Daniel P. Berrange
On Tue, Oct 21, 2008 at 03:56:48PM +0200, Chris Lalancette wrote:
 While doing testing on this patch series, I mistakenly added a bogus piece of
 storage XML to libvirtd, which was saved in /etc/libvirt/storage.  On 
 subsequent
 stop/start of libvirtd, because of another bug (fixed in a later patch), an
 error wasn't being set properly in an error path, so libvirtd was SEGV'ing in
 storage_conf.c:virStoragePoolObjLoad when trying to dereference the NULL err
 returned from virGetLastError().  Make this more robust against errors by 
 always
 doing err ? err-message : NULL in the printf.  I looked around the tree and
 found a couple of other places that weren't guarded, so this patch fixes them 
 as
 well.

ACK.

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/5]: Properly set errors on unknown format types

2008-10-21 Thread Daniel P. Berrange
On Tue, Oct 21, 2008 at 03:57:00PM +0200, Chris Lalancette wrote:
 Because of my patch last week that converted the various virStorage*FromString
 and virStorage*ToString implementations to the generic VIR_ENUM_IMPL, there 
 were
 a couple of places that didn't properly set errors when they failed.  This 
 patch
 fixes these places up.  It also adds an additional check in 
 virEnumFromString(),
 so that if a NULL type is passed in, it fails instead of SEGV'ing.

ACK to the setting of errors, but not

 diff -up ./src/storage_conf.c.p1 ./src/storage_conf.c
 --- ./src/storage_conf.c.p1   2008-10-21 14:48:10.0 +0200
 +++ ./src/storage_conf.c  2008-10-21 14:56:59.0 +0200
 @@ -276,6 +276,8 @@ virStoragePoolDefParseDoc(virConnectPtr 
  if (options-formatFromString) {
  char *format = virXPathString(conn, 
 string(/pool/source/format/@type), ctxt);
  if ((ret-source.format = (options-formatFromString)(format))  0) {
 +virStorageReportError(conn, VIR_ERR_XML_ERROR,
 +  _(unknown pool format type %s), format);
  VIR_FREE(format);
  goto cleanup;
  }
 @@ -521,8 +526,12 @@ virStoragePoolDefFormat(virConnectPtr co
  
  if (options-formatToString) {
  const char *format = (options-formatToString)(def-source.format);
 -if (!format)
 +if (!format) {
 +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
 +  _(unknown pool format number %d),
 +  def-source.format);
  goto cleanup;
 +}
  virBufferVSprintf(buf,format type='%s'/\n, format);
  }
  
 @@ -751,6 +760,8 @@ virStorageVolDefParseDoc(virConnectPtr c
  if (options-formatFromString) {
  char *format = virXPathString(conn, 
 string(/volume/target/format/@type), ctxt);
  if ((ret-target.format = (options-formatFromString)(format))  0) {
 +virStorageReportError(conn, VIR_ERR_XML_ERROR,
 +  _(unknown volume format type %s), 
 format);
  VIR_FREE(format);
  goto cleanup;
  }
 @@ -885,8 +896,12 @@ virStorageVolDefFormat(virConnectPtr con
  
  if (options-formatToString) {
  const char *format = (options-formatToString)(def-target.format);
 -if (!format)
 +if (!format) {
 +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
 +  _(unknown volume format number %d),
 +  def-target.format);
  goto cleanup;
 +}
  virBufferVSprintf(buf,format type='%s'/\n, format);
  }
  
 diff -up ./src/util.c.p1 ./src/util.c
 --- ./src/util.c.p1   2008-10-21 14:59:37.0 +0200
 +++ ./src/util.c  2008-10-21 15:00:07.0 +0200
 @@ -1018,6 +1018,10 @@ int virEnumFromString(const char *const*
const char *type)
  {
  unsigned int i;
 +
 +if (type == NULL)
 +return -1;
 +
  for (i = 0 ; i  ntypes ; i++)
  if (STREQ(types[i], type))
  return i;


But not to this hunk - its better handled by having an explicit
default format for parsing, because we need to have a concept of
defaults, to be able to distinguish it from truely unknown settings.

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/5]: Make dumpXML more idempotent with defineXML

2008-10-21 Thread Daniel P. Berrange
On Tue, Oct 21, 2008 at 03:57:13PM +0200, Chris Lalancette wrote:
 Currently, you can define a logical storage pool with something like the 
 following:
 
 pool type='logical'
   source
   nameMyVG/name
   device name='/dev/sdb'/
 ...
 
 However, dumping out the XML for this same storage pool (with, say, virsh
 pool-dumpxml), gives:
 
 pool type='logical'
   source
   nameMyVG/name
   device name='/dev/sdb'
   /device
 
 
 To make this more idempotent, do the device name='/dev/sdb'/ form by 
 default,
 and only do the device.../device form if .nfreeExtent is defined for the
 storage pool.

ACK


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 5/5]: Export device when doing logical pool discovery

2008-10-21 Thread Daniel P. Berrange
On Tue, Oct 21, 2008 at 03:57:24PM +0200, Chris Lalancette wrote:
 When doing logical pool discovery, it currently returns XML like:
 
 sourcessourcenamemyvg/name/source/sources
 
 However, for oVirt, we need to know the device name that corresponds to this
 logical volume group as well.  Therefore, extend the
 virStorageBackendLogicalFindPoolSources function to return this information as
 well.  This required pretty much a complete rewrite of the discovery function,
 since I needed to be able to add device sections to already existing XML
 nodes.  For that reason, this code uses direct libxml2 tree API's to handle 
 the
 parsing and the adding of nodes where necessary.

This really makes my eyes hurt.

This is a great example of why we centralized all the XML parsing  
formatting  introduced proper struct's everywhere. We need to take 
advantage of the stuff already provided in storage_conf.c/h. We have
a struct to represent a single storage pool source, so we just need 
another to represent a list of them. eg

  typedef struct _virStoragePoolSourceList virStoragePoolSourceList;
  typedef virStoragePoolSourceList *virStoragePoolSourceListPtr;
  struct _virStoragePoolSourceList {
 unsigned int nsources;
 virStoragePoolSourcePtr sources;
  };

And a corresponding API

 char *virStoragePoolSourceListFormat(virConnectPtr conn,
  virStoragePoolSourceListPtr def);


So, the virStorageBackendLogicalFindPoolSourcesFunc() would then just
accept a virStoragePoolSourceListPtr() as the opaque param, and be
able to nicely populate another virStoragePoolSourcePtr object in it.
All the XML handling would be kept out of the driver implementation
code.

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 3/5]: Set format type to LVM2 on NULL format

2008-10-21 Thread Daniel P. Berrange
On Tue, Oct 21, 2008 at 03:57:07PM +0200, Chris Lalancette wrote:
 Again because of my generic VIR_ENUM_IMPL patch last week, I unwittingly 
 caused
 a regression in the storage_backend_logical driver.  Previously, if you
 submitted logical pool XML that had no sourceformat type='lvm2'/ string, 
 it
 would just default to VIR_STORAGE_POOL_LOGICAL_LVM2.  This would succeed just
 fine and go on with life.  This is no longer the case, and now XML without the
 format tag fails to define.  To maintain backwards compatibility with already
 existing XML that expects this, add a compatibility wrapper to return
 VIR_STORAGE_POOL_LOGICAL_LVM2 on an empty format tag.

I think I'd rather that we add a field to the .poolOptions struct
called  defaultFormat, and just do

  .defaultFormat = VIR_STORAGE_POOL_LOGICAL_LVM2

And then in the storage_conf.c method where we're parsing formats,
if we get a NULL format, explicitly set it to the default. We'd
want todo this for all drivers that support formats, not just LVM.

Then we wouldn't need to wrap the FromString methods.

 @@ -616,7 +629,7 @@ virStorageBackend virStorageBackendLogic
  .poolOptions = {
  .flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_NAME |
VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE),
 -.formatFromString = virStorageBackendLogicalPoolTypeFromString,
 +.formatFromString = virStorageBackendLogicalFormatFromStringWrap,
  .formatToString = virStorageBackendLogicalPoolTypeToString,
  },
  

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 0 of 2] Cgroup and LXC fixes

2008-10-21 Thread Dan Smith
DS This set moves the cgroup creation before that of the container
DS process, thus ensuring that it is put in the cgroup as well.  As a
DS result, I noticed that we need to allow device access to
DS /dev/pts/*, and thus added a cgroup mechanism to allow a whole
DS major device type.  The LXC driver is made to allow major type 136
DS as a result.

Were there any comments on this set?  I think it would be nice to get
it into the tree sooner than later so we at least have resource
controls on containers :)

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: [EMAIL PROTECTED]

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


Re: [libvirt] Re: [PATCH 01/12] Domain Events - Public API

2008-10-21 Thread Daniel Veillard
On Fri, Oct 17, 2008 at 11:53:53AM -0400, Ben Guthro wrote:
  This patch does the following:
-implements the Event register/deregister code
-Adds some callback lists, and queue functions used by drivers
-Move EventImpl definitions into the public

  Basically looks fine with however a few things in-line

 +
 +typedef int (*virConnectDomainEventCallback)(virConnectPtr conn,
 + virDomainPtr dom,
 + int event,
 + void *opaque);
 +
 +int virConnectDomainEventRegister(virConnectPtr conn,
 +  virConnectDomainEventCallback cb,
 +  void *opaque);
 +
 +int virConnectDomainEventDeregister(virConnectPtr conn,
 +virConnectDomainEventCallback cb);

  Please put comments for all function type definitions
this is needed for generating the docs, bindings, etc...
  The comment for the functions themselves should be set in the C file.
Basically the comment for a construct should be at the place where the
construct is defined.

 +/**
 + * virEventHandleCallback: callback for receiving file handle events
 + *
 + * @fd: file handle on which the event occurred
 + * @events: bitset of events from POLLnnn constants
 + * @opaque: user data registered with handle
 + */
 +typedef void (*virEventHandleCallback)(int fd, int events, void *opaque);
 +
 +typedef int (*virEventAddHandleFunc)(int, int, virEventHandleCallback, void 
 *);
 +typedef void (*virEventUpdateHandleFunc)(int, int);
 +typedef int (*virEventRemoveHandleFunc)(int);

  Also give name to all parameters, even if C allows passing only the
  type, we really need names and comments for all of them

 +
 +typedef int (*virEventAddTimeoutFunc)(int, virEventTimeoutCallback, void *);
 +typedef void (*virEventUpdateTimeoutFunc)(int, int);
 +typedef int (*virEventRemoveTimeoutFunc)(int);

  Same thing

  And similar for libvirt.h.in

 diff --git a/src/libvirt.c b/src/libvirt.c
[...]
 +/**
 + * virConnectDomainEventDeregister:
 + * @conn: pointer to the connection
 + * @cb: callback to the function handling domain events
 + *
 + * Removes a Domain Event Callback
 + *
 + * Returns 0 on success, -1 on failure
 + */
 +int virConnectDomainEventDeregister(virConnectPtr conn, 
 virConnectDomainEventCallback cb)
 +{

  the following comment is part of the function description, move it
in the function comment block, and please keep lines less than 80
columns.

  Also stylistic we use

int
virFunction(...)
{

for indenting and this kind of things the HACKING file has an indent
description which can be used to automate most of this :-)

 +/* De-registering for a domain callback will disable delivery of this 
 event type*/

  For any part of the public API, all parameters must be checked as
thorougthly as possible, there is macros to test active connections
reuse them. Maybe cb should be checked for NULL, if it is allowed
then the function description should indicate the semantic. of passing
NULL.

 +if (conn-driver  conn-driver-domainEventDeregister)
 +return conn-driver-domainEventDeregister (conn, cb);
 +
 +return -1;
 +}
 +
 +/**
 + * virDispatchDomainEvent:
 + * @dom: the domain
 + * @event: the domain event code
 + *
 + * Internal function by which drivers to dispatch domain events.
 + */
 +void virDispatchDomainEvent(virDomainPtr dom,
 +int event)
 +{

  parameter check

 +if (dom-conn  dom-conn-driver 
 +dom-conn-driver-domainEventDispatch)
 +dom-conn-driver-domainEventDispatch(dom, event);
 +}
 +
 +/**
 + * virDomainEventCallbackListRemove:

  missing @conn ... not important as it's private but for completion

 + * @cbList: the list
 + * @callback: the callback to remove
 + *
[...]
 +/**
 + * virDomainEventCallbackListAdd:

  same thing

 + * @cbList: the list
 + * @callback: the callback to add
 + * @opaque: opaque data tio pass to callback
 + *
 + * Internal function to add a callback from a virDomainEventCallbackListPtr
 + */
 +int __virDomainEventCallbackListAdd(virConnectPtr conn,
 +virDomainEventCallbackListPtr cbList,
 +virConnectDomainEventCallback callback,
 +void *opaque)
 +{
 +virDomainEventCallbackPtr event;
 +int n;
 +
 +/* Check incoming */
 +if ( !cbList ) {
 +return -1;
 +}
 +
 +/* check if we already have this callback on our list */
 +for( n=0; n  cbList-count; n++ )
 +{
 +if(cbList-callbacks[n]-cb == callback 
 +   conn == cbList-callbacks[n]-conn) {
 +DEBUG0(WARNING: Callback already tracked);
 +return -1;
 +}
 +}

  again, purely stylistic but libvirt code uses

   for (n=0; n  cbList-count; n++) {
if ((cbList-callbacks[n]-cb == callback) 
 

Re: [libvirt] [PATCH 1 of 2] Add virCgroupAllowMajor()

2008-10-21 Thread Daniel P. Berrange
On Thu, Oct 16, 2008 at 02:07:56PM -0700, Dan Smith wrote:
 This interface provides a way to allow an entire major device type

ACK.

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 of 2] [LXC] Create and enter the cgroup before starting container process

2008-10-21 Thread Daniel P. Berrange
On Thu, Oct 16, 2008 at 02:07:57PM -0700, Dan Smith wrote:
 Without this, our container child doesn't actually end up in the cgroup,
 and thus runs unrestricted.  Note that this does not address the container's
 ability to mount cgroup and move itself into the parent namespace.
 
 While making this change, it became clear that we need to allow access to
 the entire range of pty devices for the container console to work.  This
 patch adds that logic as well.

Yep, we also need the kernel guys to finish PTS namespace virtualization
so we can actually make /dev/pts private to the container :-)

ACK to this patch.

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/RFC] qemu/kvm: allow to hot remove scsi/virtio disks

2008-10-21 Thread Guido Günther
On Tue, Oct 21, 2008 at 02:42:40PM +0100, Daniel P. Berrange wrote:
[..snip..] 
 Is there not some 'info pci' or 'info disk' command we cna use to find
 out the PCI slot number at the time we want to detach the device. This
 would make it work for all disks, and avoid the need to track the 
 state in that virDomainDiskDef union
I don't like restricting this to hot added disks either but for one this
will probably be a very common suitation and more important qemu just
doesn't give enough information to do this reliably. 
The only really reliable way would (at least I think) be for the user
(in this case libvirt) to pass a token to qemu (with -drive and with
pci_add). We should then be able to detach using this token. Something
else will always be dependent on what hardware qemu currently emulates.
This would need to be implemented in qemu for usb and pci.
 -- Guido

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


Re: [libvirt] [PATCH/RFC] qemu/kvm: allow to hot remove scsi/virtio disks

2008-10-21 Thread Guido Günther
Hi Daniel,
On Tue, Oct 21, 2008 at 03:25:25PM +0200, Daniel Veillard wrote:
   Those are just stylistic issues, I can apply the patch with those
   changed if you wish if you don't have time for a new patch,
I'll come up with a corrected version, no problem.
 -- Guido

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


Re: [libvirt] [PATCH 1 of 2] Add virCgroupAllowMajor()

2008-10-21 Thread Daniel Veillard
On Thu, Oct 16, 2008 at 02:07:56PM -0700, Dan Smith wrote:
 This interface provides a way to allow an entire major device type

  Looks uncontroversial, +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] [PATCH 2 of 2] [LXC] Create and enter the cgroup before starting container process

2008-10-21 Thread Daniel Veillard
On Thu, Oct 16, 2008 at 02:07:57PM -0700, Dan Smith wrote:
 Without this, our container child doesn't actually end up in the cgroup,
 and thus runs unrestricted.  Note that this does not address the container's
 ability to mount cgroup and move itself into the parent namespace.

  Okay this moves the initialization  earlier, 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] [PATCH 0 of 2] Cgroup and LXC fixes

2008-10-21 Thread Daniel Veillard
On Tue, Oct 21, 2008 at 07:58:29AM -0700, Dan Smith wrote:
 DS This set moves the cgroup creation before that of the container
 DS process, thus ensuring that it is put in the cgroup as well.  As a
 DS result, I noticed that we need to allow device access to
 DS /dev/pts/*, and thus added a cgroup mechanism to allow a whole
 DS major device type.  The LXC driver is made to allow major type 136
 DS as a result.
 
 Were there any comments on this set?  I think it would be nice to get
 it into the tree sooner than later so we at least have resource
 controls on containers :)

  sorry for the delay, please push  !

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


[libvirt] Re: [RFC] sVirt v0.10 - initial prototype

2008-10-21 Thread Daniel P. Berrange
On Tue, Oct 21, 2008 at 09:57:15AM -0400, Daniel J Walsh wrote:
 James Morris wrote:

[snip]

  The domain security label configuration format is as follows:
  
  # virsh dumpxml sys1
  domain
  
seclabel model='selinux'
  labelsystem_u:system_r:virtd_t:s0/label
  policytypetargeted/policytype
/seclabel
  /domain
  
  It's possible to query the security label of a running domain via virsh:
  
  # virsh dominfo sys1
  Id: 1
  Name:   sys1
  UUID:   fa3c8e06-0877-2a08-06fd-f2479b7bacb4
  OS Type:hvm
  State:  running
  CPU(s): 1
  CPU time:   11.4s
  Max memory: 524288 kB
  Used memory:524288 kB
  Autostart:  disable
  Security label: system_u:system_r:virtd_t:s0 (selinux/targeted/enforcing)

[snip]

 Why do we care about the policy type?  Policy type is a fairly
 meaningless object.  If you are trying to figure out if the host machine
 is valid to run a virtual machine you should just check whether the type
 is valid on the machine,  That way if I define minimum policy with virt
 support on one host and targeted policy with virt support on another
 machine, both would work.  Finally I think it might be ok for the
 administrator to request that this virtual machine would only run on a
 machine with SELinux in enforcing mode. For example if you had a fairly
 untrusted virtual machine you would want to ensure that the machine was
 enforcing SELinux before it got started.

Who/what should be making such a policy decision though ? Seems like the
management app using libvirt would want to do that - perhaps even making
that decison before defining the existance of the VM on the target machine,
let alone starting it.

When migrating a VM from one host to another an application may also 
want to verify that the same policy is available on both the source
and target hosts. A simple 'targeted' vs 'enforcing'  string is likely
not sufficient in this context. This also feels like host level info,
rather per-VM.

I think the 'policytype' bit of the label may thus better live in the
host capabilities XML document, so you can query it independantly of
any virtual machine

eg perhaps something like

# virsh capabilities 
capabilities

  host
cpu
  archi686/arch
/cpu
secpolicy model='selinux'
   typetargetted/type
   stateenforcing/state
/secpolicy
  /host

   snip rest of XML...

Is there any meaningful / useful policy version information that can
be included here ? Or policy feature bits

The VM config would thus only need

   domain
 
 seclabel model='selinux'
   labelsystem_u:system_r:virtd_t:s0/label
 /seclabel
 ...
   /domain


I should note that the domain XML format is representative of the config
for a particular deployment of a virtual machine onto a host. 

It is not a generic interchange format for 'appliances'. If you were
distributing an appliance, then the virt-image XML format would be used,
and this encodes information on pre-requisites for host capabilities. 

When an appliance is deployed as a virtual domain, the virt-image tool,
validate the virt-image XML pre-requisites, against the host capabilites
XML to determine if the host is suitable.

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] minor usb masstorage hotadd cleanup

2008-10-21 Thread Guido Günther
On Tue, Oct 21, 2008 at 03:09:48PM +0200, Daniel Veillard wrote:
 On Fri, Oct 17, 2008 at 12:44:13PM +0200, Guido Günther wrote:
  Hi,
  this just brings qemudDomainAttachUsbMassstorageDevice in line with the
  rest of the code:
  
  * don't allow to add the same target device name more than once
  * escape paths for qemu's monitor command
 
   Good idea ! Applied and commited,
Would it make sense to push the check for duplicate device names further
up in the long term?
 -- Guido

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


Re: [libvirt] [PATCH 1/5]: Shore up virGetLastError handling

2008-10-21 Thread Chris Lalancette
Daniel P. Berrange wrote:
 On Tue, Oct 21, 2008 at 03:56:48PM +0200, Chris Lalancette wrote:
 While doing testing on this patch series, I mistakenly added a bogus piece of
 storage XML to libvirtd, which was saved in /etc/libvirt/storage.  On 
 subsequent
 stop/start of libvirtd, because of another bug (fixed in a later patch), an
 error wasn't being set properly in an error path, so libvirtd was SEGV'ing in
 storage_conf.c:virStoragePoolObjLoad when trying to dereference the NULL err
 returned from virGetLastError().  Make this more robust against errors by 
 always
 doing err ? err-message : NULL in the printf.  I looked around the tree 
 and
 found a couple of other places that weren't guarded, so this patch fixes 
 them as
 well.
 
 ACK.

Committed this.

-- 
Chris Lalancette

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


Re: [libvirt] [PATCH 2/5]: Properly set errors on unknown format types

2008-10-21 Thread Chris Lalancette
Daniel P. Berrange wrote:
 On Tue, Oct 21, 2008 at 03:57:00PM +0200, Chris Lalancette wrote:
 Because of my patch last week that converted the various 
 virStorage*FromString
 and virStorage*ToString implementations to the generic VIR_ENUM_IMPL, there 
 were
 a couple of places that didn't properly set errors when they failed.  This 
 patch
 fixes these places up.  It also adds an additional check in 
 virEnumFromString(),
 so that if a NULL type is passed in, it fails instead of SEGV'ing.
 
 ACK to the setting of errors, but not
 
 diff -up ./src/storage_conf.c.p1 ./src/storage_conf.c
 --- ./src/storage_conf.c.p1  2008-10-21 14:48:10.0 +0200
 +++ ./src/storage_conf.c 2008-10-21 14:56:59.0 +0200
 @@ -276,6 +276,8 @@ virStoragePoolDefParseDoc(virConnectPtr 
  if (options-formatFromString) {
  char *format = virXPathString(conn, 
 string(/pool/source/format/@type), ctxt);
  if ((ret-source.format = (options-formatFromString)(format))  0) 
 {
 +virStorageReportError(conn, VIR_ERR_XML_ERROR,
 +  _(unknown pool format type %s), format);
  VIR_FREE(format);
  goto cleanup;
  }
 @@ -521,8 +526,12 @@ virStoragePoolDefFormat(virConnectPtr co
  
  if (options-formatToString) {
  const char *format = (options-formatToString)(def-source.format);
 -if (!format)
 +if (!format) {
 +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
 +  _(unknown pool format number %d),
 +  def-source.format);
  goto cleanup;
 +}
  virBufferVSprintf(buf,format type='%s'/\n, format);
  }
  
 @@ -751,6 +760,8 @@ virStorageVolDefParseDoc(virConnectPtr c
  if (options-formatFromString) {
  char *format = virXPathString(conn, 
 string(/volume/target/format/@type), ctxt);
  if ((ret-target.format = (options-formatFromString)(format))  0) 
 {
 +virStorageReportError(conn, VIR_ERR_XML_ERROR,
 +  _(unknown volume format type %s), 
 format);
  VIR_FREE(format);
  goto cleanup;
  }
 @@ -885,8 +896,12 @@ virStorageVolDefFormat(virConnectPtr con
  
  if (options-formatToString) {
  const char *format = (options-formatToString)(def-target.format);
 -if (!format)
 +if (!format) {
 +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
 +  _(unknown volume format number %d),
 +  def-target.format);
  goto cleanup;
 +}
  virBufferVSprintf(buf,format type='%s'/\n, format);
  }
  
 diff -up ./src/util.c.p1 ./src/util.c
 --- ./src/util.c.p1  2008-10-21 14:59:37.0 +0200
 +++ ./src/util.c 2008-10-21 15:00:07.0 +0200
 @@ -1018,6 +1018,10 @@ int virEnumFromString(const char *const*
const char *type)
  {
  unsigned int i;
 +
 +if (type == NULL)
 +return -1;
 +
  for (i = 0 ; i  ntypes ; i++)
  if (STREQ(types[i], type))
  return i;
 
 
 But not to this hunk - its better handled by having an explicit
 default format for parsing, because we need to have a concept of
 defaults, to be able to distinguish it from truely unknown settings.

Committed this patch, minus the above hunk.  I'll rework it to have the .default
as Dan suggested in 3/5.

Chris Lalancette

-- 
Chris Lalancette

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


Re: [libvirt] [PATCH 4/5]: Make dumpXML more idempotent with defineXML

2008-10-21 Thread Chris Lalancette
Daniel P. Berrange wrote:
 On Tue, Oct 21, 2008 at 03:57:13PM +0200, Chris Lalancette wrote:
 Currently, you can define a logical storage pool with something like the 
 following:

 pool type='logical'
  source
  nameMyVG/name
  device name='/dev/sdb'/
 ...

 However, dumping out the XML for this same storage pool (with, say, virsh
 pool-dumpxml), gives:

 pool type='logical'
  source
  nameMyVG/name
  device name='/dev/sdb'
  /device


 To make this more idempotent, do the device name='/dev/sdb'/ form by 
 default,
 and only do the device.../device form if .nfreeExtent is defined for the
 storage pool.
 
 ACK

Committed this.

-- 
Chris Lalancette

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


Re: [libvirt] [PATCH 3/5]: Set format type to LVM2 on NULL format

2008-10-21 Thread Chris Lalancette
Daniel P. Berrange wrote:
 On Tue, Oct 21, 2008 at 03:57:07PM +0200, Chris Lalancette wrote:
 Again because of my generic VIR_ENUM_IMPL patch last week, I unwittingly 
 caused
 a regression in the storage_backend_logical driver.  Previously, if you
 submitted logical pool XML that had no sourceformat type='lvm2'/ string, 
 it
 would just default to VIR_STORAGE_POOL_LOGICAL_LVM2.  This would succeed just
 fine and go on with life.  This is no longer the case, and now XML without 
 the
 format tag fails to define.  To maintain backwards compatibility with already
 existing XML that expects this, add a compatibility wrapper to return
 VIR_STORAGE_POOL_LOGICAL_LVM2 on an empty format tag.
 
 I think I'd rather that we add a field to the .poolOptions struct
 called  defaultFormat, and just do
 
   .defaultFormat = VIR_STORAGE_POOL_LOGICAL_LVM2
 
 And then in the storage_conf.c method where we're parsing formats,
 if we get a NULL format, explicitly set it to the default. We'd
 want todo this for all drivers that support formats, not just LVM.
 
 Then we wouldn't need to wrap the FromString methods.

Yeah, that would work fine, and is more generic.  I'll implement it that way.

Chris Lalancette

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


[libvirt] Re: [RFC] sVirt v0.10 - initial prototype

2008-10-21 Thread Daniel P. Berrange
On Tue, Oct 21, 2008 at 01:06:17PM +1100, James Morris wrote:
 With this release, it is possible to define a security label for a 
 kvm/qemu domain in its XML configuration ('virsh edit'), launch the domain 
 and have it transition to the specified security label ('virsh start'), 
 then query the security label of the running domain ('virsh dominfo').
 
 The following changes were made to libvirt:
 
 1. Implementing a pluggable security label driver framework;
 
 2. Implementing an SELinux security label driver for (1);
 
 3. Wiring the security label framework into the Qemu driver;
 
 4. Implementing basic libvirt API calls for initializing the driver, 
and getting/setting domain security labels;
 
 5. Extending the domain XML configuration to include security labels;
 
 6. Adding domain security label display/edit/dump support to virsh.
 
 One of the design principles I've followed with this is to reduce or 
 eliminate configuration wherever possible.  If a variety of security 
 labeling drivers are present, libvirtd automatically detects which one to 
 enable and enables it.  e.g. if SELinux is enabled on the system, the 
 SELinux labeling driver is enabled automatically when livbirtd starts.
 
 Another is to treat security labels as opaque unless they're actually 
 being used for security purposes (e.g. to launch the domain).  So, virsh 
 and the domain configuration code currently do not need to semantically 
 interpet security labels, just understand their format.  This should suit 
 the initial simple goal of isolated domains, which only requires security 
 labels to be distinct.
 
 The domain security label configuration format is as follows:
 
 # virsh dumpxml sys1
 domain
 
   seclabel model='selinux'
 labelsystem_u:system_r:virtd_t:s0/label
 policytypetargeted/policytype
   /seclabel
 /domain 

As I mentioned in my reply to Dan Walsh's comments, I thing that the
policy type  its state (disabled, permissive, enforcing) is really a
property of the host, rather than the VM, and so should live in the
host capabilities XML document.

 Currently, the idea is to attach the security labeling driver to the virt 
 driver, rather than implement it independently as a top-level component as 
 in the case of other types of drivers (e.g. storage).  This is because 
 process-based security labeling is highly dependent on the kind of 
 virtualization in use, and may not make sense at all in some cases (e.g. 
 when using a non-Linux hypervisor, or containers).

Makes sense - the choice of hypervisor driver in libvirt determines
what security model has to be applied to the storage/network sub-drivers
in libvirt. So, eg if we activate Xen backend, we need to have an XSM
based implementation for the security model of both the Xen driver
and the storage backend.

 
 In the case of qemu, a security labeling driver is added to qemud:
 
 @@ -63,6 +64,7 @@ struct qemud_driver {
  char *vncListen;
  
  virCapsPtr caps;
 +virSecLabelDriverPtr secLabelDriver;
  };
 
 and then initialized during qemud startup from qemudSecLabelInit().  
 
 During initialization, any available security labeling drivers are probed, 
 and the first one which thinks it should be used is installed. Top-level 
 libvirt API calls are then dispatched to the active security labeling 
 driver via the backend virt driver, as necessary.

That all makes sense to me - you'll also likely need to expose the
hypervisor driver's active security driver, to the storage  network
drivers. For that I reckon extending the 'virDriverPtr' struct to
add a internal only method

   virSecLabelDriverPtr (*getSecLabelDriver)(void);

would be a suitable approach. This would avoid the storage/network
drivers needing to know about the internal state of the HV driver.

 Note that the security labeling framework in this release is always 
 built-in -- it can be made a compile-time option later if desired.

As long as we can turn off the specific security model backends
that is sufficient - no need to be able to turn off the entire
security framework within libvirt - assuming of course everything
handles a 'NULL' secLabelDriver.

 Requirements not yet addressed include:
 - Labeling of resources and generally comprehensive labeling management
 - Automatic labeling (e.g. for the simple isolation use-case)
 - Integration of labeling support into higher-level management tools such 
   as virt-manager
 - Integration with the audit subsystem to help with administration and 
   debugging
 - Domain of interpretation (DOI) checking/translation
 - Python bindings
 
 As mentioned, the goal at this stage is to get feedback on the underlying 
 design: comments welcome!

Looking at it from a libvirt architecture POV, i think its all basically
a sane approach. It appears, so far, to be generic enough that we could 
plug into an alternate XSM based impl for the Xen world, or delegate
to whatever APIs something like VMWare / Hyper-V might provide.

 diff --git 

[libvirt] Re: [PATCH 1/6] host (node) device enumeration - public API additions

2008-10-21 Thread David Lively
This patch contains the public API additions.

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 3624367..c888943 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -993,6 +993,76 @@ char *  virStorageVolGetPath(virStorageVolPtr vol);
 virDomainPtrvirDomainCreateLinux(virConnectPtr conn,
  const char *xmlDesc,
  unsigned int flags);
+
+/*
+ * Host device enumeration
+ */
+
+/**
+ * virNodeDevice:
+ *
+ * A virNodeDevice contains a node (host) device details.
+ */
+
+typedef struct _virNodeDevice virNodeDevice;
+
+/**
+ * virNodeDevicePtr:
+ *
+ * A virNodeDevicePtr is a pointer to a virNodeDevice structure.  Get
+ * one via virNodeDeviceLookupByKey, virNodeDeviceLookupByName, or
+ * virNodeDeviceCreate.  Be sure to Call virNodeDeviceFree when done
+ * using a virNodeDevicePtr obtained from any of the above functions to
+ * avoid leaking memory.
+ */
+
+typedef virNodeDevice *virNodeDevicePtr;
+
+
+int virNodeNumOfDevices (virConnectPtr conn,
+ unsigned int flags);
+
+int virNodeListDevices  (virConnectPtr conn,
+ char **const names,
+ int maxnames,
+ unsigned int flags);
+
+int virNodeNumOfDevicesByCap (virConnectPtr conn,
+  const char *cap,
+  unsigned int flags);
+
+int virNodeListDevicesByCap (virConnectPtr conn,
+ const char *cap,
+ char **const names,
+ int maxnames,
+ unsigned int flags);
+
+virNodeDevicePtrvirNodeDeviceLookupByName (virConnectPtr conn,
+   const char *name);
+
+const char *virNodeDeviceGetName (virNodeDevicePtr dev);
+
+const char *virNodeDeviceGetParent   (virNodeDevicePtr dev);
+
+int virNodeDeviceNumOfCaps   (virNodeDevicePtr dev);
+
+int virNodeDeviceListCaps(virNodeDevicePtr dev,
+  char **const names,
+  int maxnames);
+
+char *  virNodeDeviceGetXMLDesc (virNodeDevicePtr dev,
+ unsigned int flags);
+
+virNodeDevicePtrvirNodeDeviceCreate (virConnectPtr conn,
+ const char *xml,
+ unsigned int flags);
+
+int virNodeDeviceDestroy(virNodeDevicePtr dev,
+ unsigned int flags);
+
+int virNodeDeviceFree   (virNodeDevicePtr dev);
+
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 8e24708..020b8dc 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -58,6 +58,7 @@ typedef enum {
 VIR_FROM_STORAGE,   /* Error from storage driver */
 VIR_FROM_NETWORK,   /* Error from network config */
 VIR_FROM_DOMAIN,/* Error from domain config */
+VIR_FROM_NODE,  /* Error from node driver */
 } virErrorDomain;
 
 
@@ -148,6 +149,9 @@ typedef enum {
 VIR_WAR_NO_STORAGE, /* failed to start storage */
 VIR_ERR_NO_STORAGE_POOL, /* storage pool not found */
 VIR_ERR_NO_STORAGE_VOL, /* storage pool not found */
+VIR_WAR_NO_NODE, /* failed to start node driver */
+VIR_ERR_INVALID_NODE_DEVICE,/* invalid node device object */
+VIR_ERR_NO_NODE_DEVICE,/* node device not found */
 } virErrorNumber;
 
 /**
diff --git a/src/libvirt.c b/src/libvirt.c
index ca2675a..5cef752 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -73,6 +73,8 @@ static virNetworkDriverPtr virNetworkDriverTab[MAX_DRIVERS];
 static int virNetworkDriverTabCount = 0;
 static virStorageDriverPtr virStorageDriverTab[MAX_DRIVERS];
 static int virStorageDriverTabCount = 0;
+static virNodeDriverPtr virNodeDriverTab[MAX_DRIVERS];
+static int virNodeDriverTabCount = 0;
 #ifdef WITH_LIBVIRTD
 static virStateDriverPtr virStateDriverTab[MAX_DRIVERS];
 static int virStateDriverTabCount = 0;
@@ -299,6 +301,12 @@ virInitialize(void)
 #ifdef WITH_NETWORK
 if (networkRegister() == -1) return -1;
 #endif
+#ifdef HAVE_HAL
+if (halNodeRegister() == -1) return -1;
+#endif
+#ifdef HAVE_DEVKIT
+if (devkitNodeRegister() == -1) return -1;
+#endif
 if (storageRegister() == -1) return -1;
 

[libvirt] Re: [PATCH 2/6] host (node) device enumeration - internal API additions

2008-10-21 Thread David Lively
This patch contains the internal API additions.

diff --git a/src/hash.c b/src/hash.c
index 0a5bdcd..424c4a7 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -687,6 +687,9 @@ virGetConnect(void) {
 ret-storageVols = virHashCreate(20);
 if (ret-storageVols == NULL)
 goto failed;
+ret-nodeDevices = virHashCreate(256);
+if (ret-nodeDevices == NULL)
+goto failed;
 
 pthread_mutex_init(ret-lock, NULL);
 
@@ -703,6 +706,8 @@ failed:
 virHashFree(ret-storagePools, (virHashDeallocator) virStoragePoolFreeName);
 if (ret-storageVols != NULL)
 virHashFree(ret-storageVols, (virHashDeallocator) virStorageVolFreeName);
+if (ret-nodeDevices != NULL)
+virHashFree(ret-nodeDevices, (virHashDeallocator) virNodeDeviceFree);
 
 pthread_mutex_destroy(ret-lock);
 VIR_FREE(ret);
@@ -730,6 +735,8 @@ virReleaseConnect(virConnectPtr conn) {
 virHashFree(conn-storagePools, (virHashDeallocator) virStoragePoolFreeName);
 if (conn-storageVols != NULL)
 virHashFree(conn-storageVols, (virHashDeallocator) virStorageVolFreeName);
+if (conn-nodeDevices != NULL)
+virHashFree(conn-nodeDevices, (virHashDeallocator) virNodeDeviceFree);
 
 virResetError(conn-err);
 if (__lastErr.conn == conn)
@@ -1318,3 +1325,126 @@ virUnrefStorageVol(virStorageVolPtr vol) {
 pthread_mutex_unlock(vol-conn-lock);
 return (refs);
 }
+
+
+/**
+ * virGetNodeDevice:
+ * @conn: the hypervisor connection
+ * @name: device name (unique on node)
+ *
+ * Lookup if the device is already registered for that connection,
+ * if yes return a new pointer to it, if no allocate a new structure,
+ * and register it in the table. In any case a corresponding call to
+ * virFreeNodeDevice() is needed to not leak data.
+ *
+ * Returns a pointer to the node device, or NULL in case of failure
+ */
+virNodeDevicePtr
+__virGetNodeDevice(virConnectPtr conn, const char *name)
+{
+virNodeDevicePtr ret = NULL;
+
+if ((!VIR_IS_CONNECT(conn)) || (name == NULL)) {
+virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+return(NULL);
+}
+pthread_mutex_lock(conn-lock);
+
+ret = (virNodeDevicePtr) virHashLookup(conn-nodeDevices, name);
+if (ret == NULL) {
+   if (VIR_ALLOC(ret)  0) {
+virHashError(conn, VIR_ERR_NO_MEMORY, _(allocating node dev));
+goto error;
+}
+ret-magic = VIR_NODE_DEVICE_MAGIC;
+ret-conn = conn;
+ret-name = strdup(name);
+if (ret-name == NULL) {
+virHashError(conn, VIR_ERR_NO_MEMORY, _(copying node dev name));
+goto error;
+}
+
+if (virHashAddEntry(conn-nodeDevices, name, ret)  0) {
+virHashError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(failed to add node dev to conn hash table));
+goto error;
+}
+conn-refs++;
+}
+ret-refs++;
+pthread_mutex_unlock(conn-lock);
+return(ret);
+
+error:
+pthread_mutex_unlock(conn-lock);
+if (ret != NULL) {
+VIR_FREE(ret-name);
+VIR_FREE(ret);
+}
+return(NULL);
+}
+
+
+/**
+ * virReleaseNodeDevice:
+ * @dev: the dev to release
+ *
+ * Unconditionally release all memory associated with a dev.
+ * The conn.lock mutex must be held prior to calling this, and will
+ * be released prior to this returning. The dev obj must not
+ * be used once this method returns.
+ *
+ * It will also unreference the associated connection object,
+ * which may also be released if its ref count hits zero.
+ */
+static void
+virReleaseNodeDevice(virNodeDevicePtr dev) {
+virConnectPtr conn = dev-conn;
+DEBUG(release dev %p %s, dev, dev-name);
+
+if (virHashRemoveEntry(conn-nodeDevices, dev-name, NULL)  0)
+virHashError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(dev missing from connection hash table));
+
+dev-magic = -1;
+VIR_FREE(dev-name);
+VIR_FREE(dev);
+
+DEBUG(unref connection %p %s %d, conn, conn-name, conn-refs);
+conn-refs--;
+if (conn-refs == 0) {
+virReleaseConnect(conn);
+/* Already unlocked mutex */
+return;
+}
+
+pthread_mutex_unlock(conn-lock);
+}
+
+
+/**
+ * virUnrefNodeDevice:
+ * @dev: the dev to unreference
+ *
+ * Unreference the dev. If the use count drops to zero, the structure is
+ * actually freed.
+ *
+ * Returns the reference count or -1 in case of failure.
+ */
+int
+virUnrefNodeDevice(virNodeDevicePtr dev) {
+int refs;
+
+pthread_mutex_lock(dev-conn-lock);
+DEBUG(unref dev %p %s %d, dev, dev-name, dev-refs);
+dev-refs--;
+refs = dev-refs;
+if (refs == 0) {
+virReleaseNodeDevice(dev);
+/* Already unlocked mutex */
+return (0);
+}
+
+pthread_mutex_unlock(dev-conn-lock);
+return (refs);
+}
diff --git a/src/internal.h b/src/internal.h
index 2ae764d..76b57ad 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -37,6 

Re: [libvirt] [PATCH 0 of 2] Cgroup and LXC fixes

2008-10-21 Thread Dan Smith
DV   sorry for the delay, please push  !

Done.  Thanks!

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: [EMAIL PROTECTED]

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


[libvirt] [PATCH 0/6] host (node) device enumeration, take two

2008-10-21 Thread David Lively
Ok, here's my substantially-reworked node device enumeration patch, this
time done with a proper understanding of the public-obj/Obj/Def
model :-) as last discussed here:
https://www.redhat.com/archives/libvir-list/2008-September/msg00398.html

I've broken it into the following pieces:
  1-public-api  additions to the public API
  2-internal-apiadditions to the internal API
  3-local-node-drivers  the HAL  DeviceKit implementations
  4-remote-node-driver  the remote driver
  5-virsh-support   virsh support
  6-python-bindings python bindings

Big differences from the last submission:
  * public-obj/Obj/Def object model finally understood :-)
  * capabilities structure now struct-ified, handled like
other Def bits
  * using newfangled array-based lists for NodeDeviceObj's
  * added flags args to various public APIs (as suggested by Dan V)
  * bus folded into capabilities (as discussed w/Dan B)
  * device key no longer exists - name is unique on node

Some pieces are still incomplete, but I thought it would be better to
post what I have since it's useful as is.  Here's what I know of
that's left to do:
  * finish Python bindings (will get to Real Soon Now)
  * submit Java bindings (I have them, and have been using them)
  * implement virNodeDeviceCreate/Destroy (I have no plans for these)
  * subscribe to HAL events  update dev state from them (next for me?)
  * implement pci_bus/usb_bus capabilities (for PCI/USB controllers)
  * DeviceKit implementation is barely a proof of concept
* need to resolve naming  other issues (see
https://www.redhat.com/archives/libvir-list/2008-September/msg00430.html
* ... and then fill in impl (no hurry; devkit immature now)

Dave




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


[libvirt] Re: [PATCH 0/6] host (node) device enumeration - local node drivers

2008-10-21 Thread David Lively
This patch contains the actual HAL- and DeviceKit-based local node
drivers.

diff --git a/configure.in b/configure.in
index 66d271a..3441742 100644
--- a/configure.in
+++ b/configure.in
@@ -1048,6 +1048,93 @@ test $enable_shared = no  lt_cv_objdir=.
 LV_LIBTOOL_OBJDIR=${lt_cv_objdir-.}
 AC_SUBST([LV_LIBTOOL_OBJDIR])
 
+dnl HAL or DeviceKit library for host device enumeration
+HAL_REQUIRED=0.0
+HAL_CFLAGS=
+HAL_LIBS=
+AC_ARG_WITH([hal],
+  [  --with-hal use HAL for host device enumeration],
+  [],
+  [with_hal=check])
+
+if test x$with_hal = xyes -o x$with_hal = xcheck; then
+  PKG_CHECK_MODULES(HAL, hal = $HAL_REQUIRED,
+[with_hal=yes], [
+if test x$with_hal = xcheck ; then
+   with_hal=no
+else
+   AC_MSG_ERROR(
+ [You must install hal-devel = $HAL_REQUIRED to compile libvirt])
+fi
+  ])
+  if test x$with_hal = xyes ; then
+AC_DEFINE_UNQUOTED([HAVE_HAL], 1,
+  [use HAL for host device enumeration])
+
+old_CFLAGS=$CFLAGS
+old_LDFLAGS=$LDFLAGS
+CFLAGS=$CFLAGS $HAL_CFLAGS
+LDFLAGS=$LDFLAGS $HAL_LIBS
+AC_CHECK_FUNCS([libhal_get_all_devices],,[with_hal=no])
+CFLAGS=$old_CFLAGS
+LDFLAGS=$old_LDFLAGS
+  fi
+fi
+AM_CONDITIONAL([HAVE_HAL], [test x$with_hal = xyes])
+AC_SUBST([HAL_CFLAGS])
+AC_SUBST([HAL_LIBS])
+
+DEVKIT_REQUIRED=0.0
+DEVKIT_CFLAGS=
+DEVKIT_LIBS=
+AC_ARG_WITH([devkit],
+  [  --with-devkit  use DeviceKit for host device enumeration],
+  [],
+  [with_devkit=check])
+
+dnl Extra check needed while devkit pkg-config info missing glib2 dependency
+PKG_CHECK_MODULES(GLIB2, glib-2.0 = 0.0,,[
+  if test x$with_devkit = xcheck; then
+with_devkit=no
+  elif test x$with_devkit = xyes; then
+AC_MSG_ERROR([required package DeviceKit requires package glib-2.0])
+  fi])
+
+if test x$with_devkit = xyes -o x$with_devkit = xcheck; then
+  PKG_CHECK_MODULES(DEVKIT, devkit-gobject = $DEVKIT_REQUIRED,
+[with_devkit=yes], [
+if test x$with_devkit = xcheck ; then
+   with_devkit=no
+else
+   AC_MSG_ERROR(
+ [You must install DeviceKit-devel = $DEVKIT_REQUIRED to compile libvirt])
+fi
+  ])
+  if test x$with_devkit = xyes ; then
+AC_DEFINE_UNQUOTED([HAVE_DEVKIT], 1,
+  [use DeviceKit for host device enumeration])
+
+dnl Add glib2 flags explicitly while devkit pkg-config info missing glib2 dependency
+DEVKIT_CFLAGS=$GLIB2_CFLAGS $DEVKIT_CFLAGS
+DEVKIT_LIBS=$GLIB2_LIBS $DEVKIT_LIBS
+
+dnl Add more flags apparently required for devkit to work properly
+DEVKIT_CFLAGS=$DEVKIT_CFLAGS -D_POSIX_PTHREAD_SEMANTICS -D_REENTRANT
+
+old_CFLAGS=$CFLAGS
+old_LDFLAGS=$LDFLAGS
+CFLAGS=$CFLAGS $DEVKIT_CFLAGS
+LDFLAGS=$LDFLAGS $DEVKIT_LIBS
+AC_CHECK_FUNCS([devkit_client_connect],,[with_devkit=no])
+CFLAGS=$old_CFLAGS
+LDFLAGS=$old_LDFLAGS
+  fi
+fi
+AM_CONDITIONAL([HAVE_DEVKIT], [test x$with_devkit = xyes])
+AC_SUBST([DEVKIT_CFLAGS])
+AC_SUBST([DEVKIT_LIBS])
+
+
 # very annoying
 rm -f COPYING
 cp COPYING.LIB COPYING
@@ -1123,6 +1210,16 @@ AC_MSG_NOTICE([  numactl: $NUMACTL_CFLAGS $NUMACTL_LIBS])
 else
 AC_MSG_NOTICE([  numactl: no])
 fi
+if test $with_hal = yes ; then
+AC_MSG_NOTICE([  hal: $HAL_CFLAGS $HAL_LIBS])
+else
+AC_MSG_NOTICE([  hal: no])
+fi
+if test $with_devkit = yes ; then
+AC_MSG_NOTICE([  devkit: $DEVKIT_CFLAGS $DEVKIT_LIBS])
+else
+AC_MSG_NOTICE([  devkit: no])
+fi
 AC_MSG_NOTICE([])
 AC_MSG_NOTICE([Test suite])
 AC_MSG_NOTICE([])
diff --git a/po/POTFILES.in b/po/POTFILES.in
index f671155..684d300 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -12,6 +12,7 @@ src/lxc_controller.c
 src/lxc_driver.c
 src/network_conf.c
 src/network_driver.c
+src/node_device.c
 src/openvz_conf.c
 src/openvz_driver.c
 src/proxy_internal.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 5a769f8..963a307 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,5 +1,22 @@
 ## Process this file with automake to produce Makefile.in
 
+if WITH_LIBVIRTD
+NODE_DEVICE_SOURCES = node_device.c node_device.h \
+		node_device_conf.c node_device_conf.h
+NODE_DEVICE_CFLAGS =
+NODE_DEVICE_LIBS =
+if HAVE_HAL
+NODE_DEVICE_CFLAGS += $(HAL_CFLAGS)
+NODE_DEVICE_LIBS += $(HAL_LIBS)
+NODE_DEVICE_SOURCES += node_device_hal.c
+endif
+if HAVE_DEVKIT
+NODE_DEVICE_CFLAGS += $(DEVKIT_CFLAGS)
+NODE_DEVICE_LIBS += $(DEVKIT_LIBS)
+NODE_DEVICE_SOURCES += node_device_devkit.c
+endif
+endif
+
 INCLUDES = \
 	   -I$(top_srcdir)/gnulib/lib -I../gnulib/lib \
 	   -I../include \
@@ -10,6 +27,7 @@ INCLUDES = \
 	   $(SASL_CFLAGS) \
 	   $(SELINUX_CFLAGS) \
 	   $(NUMACTL_CFLAGS) \
+	   $(NODE_DEVICE_CFLAGS) \
 	   -DBINDIR=\$(libexecdir)\ \
 	   -DSBINDIR=\$(sbindir)\ \
 	   -DSYSCONF_DIR=\$(sysconfdir)\ \
@@ -146,6 +164,7 @@ libvirt_la_SOURCES =		\
 		libvirt.c	\
 		$(GENERIC_LIB_SOURCES)\
 		$(DOMAIN_CONF_SOURCES)\
+		$(NODE_DEVICE_SOURCES)\
 		$(NETWORK_CONF_SOURCES)\
 		$(STORAGE_CONF_SOURCES)
 
@@ -212,7 +231,7 @@ EXTRA_DIST +=			\
 
 
 libvirt_la_LIBADD = 

RE: [libvirt] image locking?

2008-10-21 Thread Itamar Heim
Hi Perry,

The problem is with unreachable hosts which are locking the image forever.

When fencing can't be used, there is no way for the management to release the 
image, since it can't verify the host stopped using the image.
A leased lock mechanism, while not providing 100% prevention, does allow a 
collaborative effort to allow releasing the image after the lock expired, by 
having the nodes check that they still own the lease and stop writing to the 
images.

It would have been much better if image access could have been enforced at 
storage level, but that is much more complex (and not relevant for images under 
LVM for example)

Itamar

-Original Message-
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Perry Myers
Sent: Tuesday, October 21, 2008 15:37 PM
To: Itamar Heim
Cc: libvir-list@redhat.com
Subject: Re: [libvirt] image locking?


In an oVirt network, this shouldn't be a problem.  Storage can only be 
assigned to one VM at a time presently.  (In the future we may relax this 
for clustered filesystems, but shared storage will be marked as such).

Regardless of whether or not a VM is active/inactive, once an iSCSI LUN, 
disk image or otherwise is assigned to a VM it can't be used by other VMs. 
  The storage is not released to the available list until the VM using it 
is both destroyed and undefined.  We don't allow undefine until the VM has 
been destroyed and we won't confirm that a VM has been destroyed if we 
can't contact the host that it is running on to confirm.

Now... if you start creating VMs on an oVirt network outside of the oVirt 
Server or decide to share your storage pools between an oVirt network and 
a non-oVirt network that is problematic.  Our solution for that for the 
time being is 'just don't do that'. :)

Perry

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

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


Re: [libvirt] image locking?

2008-10-21 Thread Perry Myers

Itamar Heim wrote:

Hi Perry,

The problem is with unreachable hosts which are locking the image
forever.

When fencing can't be used, there is no way for the management to
release the image, since it can't verify the host stopped using the
image. A leased lock mechanism, while not providing 100% prevention,
does allow a collaborative effort to allow releasing the image after
the lock expired, by having the nodes check that they still own the
lease and stop writing to the images.


If you have an unreachable host that is locking the image forever, you
walk into the datacenter and pull the plug.  Once that is done, you can
use the oVirt Server interface to undefine the vm and release the storage
volume.  So it can be done without hw fencing, it just involves manual
administrator action.  Not ideal, but it works :)


It would have been much better if image access could have been enforced
at storage level, but that is much more complex (and not relevant for
images under LVM for example)


Agreed.  We're using the above procedure (pull the plug or hw fencing) 
until a better mechanism is created.


Perry

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


RE: [libvirt] image locking?

2008-10-21 Thread Itamar Heim
While this might work for SBC (although most enterprises have the datacenter on 
remote sites as well, so not always that easy).
I don't think the solution is viable for CBC though (I am not sure CBC would 
use iSCSI, probably NFS is a more relevant option, but the leased locking is 
required there as well, just as a collaborative effort to notate to the 
non-responding node to stop writing to the image).

-Original Message-
From: Perry Myers [mailto:[EMAIL PROTECTED] 
Sent: Tuesday, October 21, 2008 21:09 PM
To: Itamar Heim
Cc: libvir-list@redhat.com
Subject: Re: [libvirt] image locking?

Itamar Heim wrote:
 Hi Perry,
 
 The problem is with unreachable hosts which are locking the image
 forever.
 
 When fencing can't be used, there is no way for the management to
 release the image, since it can't verify the host stopped using the
 image. A leased lock mechanism, while not providing 100% prevention,
 does allow a collaborative effort to allow releasing the image after
 the lock expired, by having the nodes check that they still own the
 lease and stop writing to the images.

If you have an unreachable host that is locking the image forever, you
walk into the datacenter and pull the plug.  Once that is done, you can
use the oVirt Server interface to undefine the vm and release the storage
volume.  So it can be done without hw fencing, it just involves manual
administrator action.  Not ideal, but it works :)

 It would have been much better if image access could have been enforced
 at storage level, but that is much more complex (and not relevant for
 images under LVM for example)

Agreed.  We're using the above procedure (pull the plug or hw fencing) 
until a better mechanism is created.

Perry

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


[libvirt] Re: [PATCH 03/12] Domain Events - daemon changes

2008-10-21 Thread Ben Guthro
[PATCH 03/12] Domain Events - daemon changes
This code changes the daemaon to:
  use the pulic def of virEventRegisterImpl
  Add functionality to dispatch events to connected remote drivers

 event.c  |   21 +
 event.h  |5 +-
 mdns.c   |   15 --
 qemud.c  |   72 ---
 qemud.h  |   11 
 remote.c |  143 +++
 6 files changed, 227 insertions(+), 40 deletions(-)
diff --git a/qemud/event.c b/qemud/event.c
index bb1f381..f391cd1 100644
--- a/qemud/event.c
+++ b/qemud/event.c
@@ -38,7 +38,7 @@
 /* State for a single file handle being monitored */
 struct virEventHandle {
 int fd;
-int events;
+virEventHandleType events;
 virEventHandleCallback cb;
 void *opaque;
 int deleted;
@@ -74,13 +74,13 @@ static struct virEventLoop eventLoop;
 /* Unique ID for the next timer to be registered */
 static int nextTimer = 0;
 
-
 /*
  * Register a callback for monitoring file handle events.
  * NB, it *must* be safe to call this from within a callback
  * For this reason we only ever append to existing list.
  */
-int virEventAddHandleImpl(int fd, int events, virEventHandleCallback cb, void *opaque) {
+int virEventAddHandleImpl(int fd, virEventHandleType events,
+  virEventHandleCallback cb, void *opaque) {
 EVENT_DEBUG(Add handle %d %d %p %p, fd, events, cb, opaque);
 if (eventLoop.handlesCount == eventLoop.handlesAlloc) {
 EVENT_DEBUG(Used %d handle slots, adding %d more,
@@ -92,7 +92,8 @@ int virEventAddHandleImpl(int fd, int events, virEventHandleCallback cb, void *o
 }
 
 eventLoop.handles[eventLoop.handlesCount].fd = fd;
-eventLoop.handles[eventLoop.handlesCount].events = events;
+eventLoop.handles[eventLoop.handlesCount].events =
+ virEventHandleTypeToPollEvent(events);
 eventLoop.handles[eventLoop.handlesCount].cb = cb;
 eventLoop.handles[eventLoop.handlesCount].opaque = opaque;
 eventLoop.handles[eventLoop.handlesCount].deleted = 0;
@@ -102,11 +103,12 @@ int virEventAddHandleImpl(int fd, int events, virEventHandleCallback cb, void *o
 return 0;
 }
 
-void virEventUpdateHandleImpl(int fd, int events) {
+void virEventUpdateHandleImpl(int fd, virEventHandleType events) {
 int i;
 for (i = 0 ; i  eventLoop.handlesCount ; i++) {
 if (eventLoop.handles[i].fd == fd) {
-eventLoop.handles[i].events = events;
+eventLoop.handles[i].events =
+virEventHandleTypeToPollEvent(events);
 break;
 }
 }
@@ -342,6 +344,7 @@ static int virEventDispatchTimeouts(void) {
  */
 static int virEventDispatchHandles(struct pollfd *fds) {
 int i;
+virEventHandleType hEvents;
 /* Save this now - it may be changed during dispatch */
 int nhandles = eventLoop.handlesCount;
 
@@ -352,8 +355,10 @@ static int virEventDispatchHandles(struct pollfd *fds) {
 }
 
 if (fds[i].revents) {
-EVENT_DEBUG(Dispatch %d %d %p, fds[i].fd, fds[i].revents, eventLoop.handles[i].opaque);
-(eventLoop.handles[i].cb)(fds[i].fd, fds[i].revents,
+hEvents = virPollEventToEventHandleType(fds[i].revents);
+EVENT_DEBUG(Dispatch %d %d %p, fds[i].fd, fds[i].revents,
+eventLoop.handles[i].opaque);
+(eventLoop.handles[i].cb)(fds[i].fd, hEvents,
   eventLoop.handles[i].opaque);
 }
 }
diff --git a/qemud/event.h b/qemud/event.h
index adf7b6d..3fff617 100644
--- a/qemud/event.h
+++ b/qemud/event.h
@@ -36,7 +36,8 @@
  *
  * returns -1 if the file handle cannot be registered, 0 upon success
  */
-int virEventAddHandleImpl(int fd, int events, virEventHandleCallback cb, void *opaque);
+int virEventAddHandleImpl(int fd, virEventHandleType events,
+  virEventHandleCallback cb, void *opaque);
 
 /**
  * virEventUpdateHandleImpl: change event set for a monitored file handle
@@ -46,7 +47,7 @@ int virEventAddHandleImpl(int fd, int events, virEventHandleCallback cb, void *o
  *
  * Will not fail if fd exists
  */
-void virEventUpdateHandleImpl(int fd, int events);
+void virEventUpdateHandleImpl(int fd, virEventHandleType events);
 
 /**
  * virEventRemoveHandleImpl: unregister a callback from a file handle
diff --git a/qemud/mdns.c b/qemud/mdns.c
index 9147946..df1b6c9 100644
--- a/qemud/mdns.c
+++ b/qemud/mdns.c
@@ -228,17 +228,20 @@ static void libvirtd_mdns_client_callback(AvahiClient *c, AvahiClientState state
 }
 
 
-static void libvirtd_mdns_watch_dispatch(int fd, int events, void *opaque)
+static void libvirtd_mdns_watch_dispatch(int fd, virEventHandleType events, 
+ void *opaque)
 {
 AvahiWatch *w = (AvahiWatch*)opaque;
-AVAHI_DEBUG(Dispatch watch FD %d Event %d, fd, events);
-w-revents = events;
-w-callback(w, fd, events, 

[libvirt] Re: [PATCH 04/12] Domain Events - rpc changes

2008-10-21 Thread Ben Guthro
[PATCH 04/12] Domain Events - rpc changes
Changes to the RPC protocol

 remote_dispatch_localvars.h   |3 +++
 remote_dispatch_proc_switch.h |   18 ++
 remote_dispatch_prototypes.h  |3 +++
 remote_protocol.c |   29 +
 remote_protocol.h |   25 +
 remote_protocol.x |   25 -
 6 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/qemud/remote_dispatch_localvars.h b/qemud/remote_dispatch_localvars.h
index f46b493..505fb30 100644
--- a/qemud/remote_dispatch_localvars.h
+++ b/qemud/remote_dispatch_localvars.h
@@ -6,6 +6,7 @@ remote_domain_lookup_by_uuid_args lv_remote_domain_lookup_by_uuid_args;
 remote_domain_lookup_by_uuid_ret lv_remote_domain_lookup_by_uuid_ret;
 remote_storage_pool_list_volumes_args lv_remote_storage_pool_list_volumes_args;
 remote_storage_pool_list_volumes_ret lv_remote_storage_pool_list_volumes_ret;
+remote_domain_events_deregister_ret lv_remote_domain_events_deregister_ret;
 remote_domain_shutdown_args lv_remote_domain_shutdown_args;
 remote_list_defined_domains_args lv_remote_list_defined_domains_args;
 remote_list_defined_domains_ret lv_remote_list_defined_domains_ret;
@@ -20,6 +21,7 @@ remote_domain_get_autostart_args lv_remote_domain_get_autostart_args;
 remote_domain_get_autostart_ret lv_remote_domain_get_autostart_ret;
 remote_domain_set_vcpus_args lv_remote_domain_set_vcpus_args;
 remote_get_hostname_ret lv_remote_get_hostname_ret;
+remote_domain_events_register_ret lv_remote_domain_events_register_ret;
 remote_network_undefine_args lv_remote_network_undefine_args;
 remote_domain_create_args lv_remote_domain_create_args;
 remote_network_destroy_args lv_remote_network_destroy_args;
@@ -121,6 +123,7 @@ remote_num_of_defined_storage_pools_ret lv_remote_num_of_defined_storage_pools_r
 remote_domain_core_dump_args lv_remote_domain_core_dump_args;
 remote_list_defined_storage_pools_args lv_remote_list_defined_storage_pools_args;
 remote_list_defined_storage_pools_ret lv_remote_list_defined_storage_pools_ret;
+remote_domain_event_ret lv_remote_domain_event_ret;
 remote_domain_get_max_memory_args lv_remote_domain_get_max_memory_args;
 remote_domain_get_max_memory_ret lv_remote_domain_get_max_memory_ret;
 remote_num_of_domains_ret lv_remote_num_of_domains_ret;
diff --git a/qemud/remote_dispatch_proc_switch.h b/qemud/remote_dispatch_proc_switch.h
index 89296d7..3cd5d78 100644
--- a/qemud/remote_dispatch_proc_switch.h
+++ b/qemud/remote_dispatch_proc_switch.h
@@ -116,6 +116,24 @@ case REMOTE_PROC_DOMAIN_DUMP_XML:
 ret = (char *) lv_remote_domain_dump_xml_ret;
 memset (lv_remote_domain_dump_xml_ret, 0, sizeof lv_remote_domain_dump_xml_ret);
 break;
+case REMOTE_PROC_DOMAIN_EVENT:
+fn = (dispatch_fn) remoteDispatchDomainEvent;
+ret_filter = (xdrproc_t) xdr_remote_domain_event_ret;
+ret = (char *) lv_remote_domain_event_ret;
+memset (lv_remote_domain_event_ret, 0, sizeof lv_remote_domain_event_ret);
+break;
+case REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER:
+fn = (dispatch_fn) remoteDispatchDomainEventsDeregister;
+ret_filter = (xdrproc_t) xdr_remote_domain_events_deregister_ret;
+ret = (char *) lv_remote_domain_events_deregister_ret;
+memset (lv_remote_domain_events_deregister_ret, 0, sizeof lv_remote_domain_events_deregister_ret);
+break;
+case REMOTE_PROC_DOMAIN_EVENTS_REGISTER:
+fn = (dispatch_fn) remoteDispatchDomainEventsRegister;
+ret_filter = (xdrproc_t) xdr_remote_domain_events_register_ret;
+ret = (char *) lv_remote_domain_events_register_ret;
+memset (lv_remote_domain_events_register_ret, 0, sizeof lv_remote_domain_events_register_ret);
+break;
 case REMOTE_PROC_DOMAIN_GET_AUTOSTART:
 fn = (dispatch_fn) remoteDispatchDomainGetAutostart;
 args_filter = (xdrproc_t) xdr_remote_domain_get_autostart_args;
diff --git a/qemud/remote_dispatch_prototypes.h b/qemud/remote_dispatch_prototypes.h
index 3f4eb9f..4b27a9f 100644
--- a/qemud/remote_dispatch_prototypes.h
+++ b/qemud/remote_dispatch_prototypes.h
@@ -18,6 +18,9 @@ static int remoteDispatchDomainDefineXml (struct qemud_server *server, struct qe
 static int remoteDispatchDomainDestroy (struct qemud_server *server, struct qemud_client *client, remote_message_header *req, remote_domain_destroy_args *args, void *ret);
 static int remoteDispatchDomainDetachDevice (struct qemud_server *server, struct qemud_client *client, remote_message_header *req, remote_domain_detach_device_args *args, void *ret);
 static int remoteDispatchDomainDumpXml (struct qemud_server *server, struct qemud_client *client, remote_message_header *req, remote_domain_dump_xml_args *args, remote_domain_dump_xml_ret *ret);
+static int remoteDispatchDomainEvent (struct qemud_server *server, struct qemud_client *client, remote_message_header *req, void *args, remote_domain_event_ret 

[libvirt] Re: [PATCH 05/12] Domain Events - Driver API

2008-10-21 Thread Ben Guthro
[PATCH 05/12] Domain Events - Driver API
Additions to the driver API

 driver.h |   13 +
 1 file changed, 13 insertions(+)
diff --git a/src/driver.h b/src/driver.h
index 0540f80..d1b541f 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -280,6 +280,17 @@ typedef unsigned long long
 (*virDrvNodeGetFreeMemory)
 (virConnectPtr conn);
 
+typedef int
+(*virDrvDomainEventRegister)
+(virConnectPtr conn,
+ void *callback,
+ void *opaque);
+
+typedef int
+(*virDrvDomainEventDeregister)
+(virConnectPtr conn,
+ void *callback);
+
 /**
  * _virDriver:
  *
@@ -352,6 +363,8 @@ struct _virDriver {
 virDrvDomainMemoryPeek  domainMemoryPeek;
 virDrvNodeGetCellsFreeMemory	nodeGetCellsFreeMemory;
 virDrvNodeGetFreeMemory		getFreeMemory;
+virDrvDomainEventRegister domainEventRegister;
+virDrvDomainEventDeregister   domainEventDeregister;
 };
 
 typedef int
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Re: [PATCH 06/12] Domain Events - qemu driver

2008-10-21 Thread Ben Guthro
[PATCH 06/12] Domain Events - qemu driver
Register for, and dispatch domain event callbacks

 qemu_conf.h   |3 +
 qemu_driver.c |  101 +-
 2 files changed, 97 insertions(+), 7 deletions(-)
diff --git a/src/qemu_conf.h b/src/qemu_conf.h
index cfd7d35..d06c4d7 100644
--- a/src/qemu_conf.h
+++ b/src/qemu_conf.h
@@ -63,6 +63,9 @@ struct qemud_driver {
 char *vncListen;
 
 virCapsPtr caps;
+
+/* An array of callbacks */
+virDomainEventCallbackListPtr domainEventCallbacks;
 };
 
 
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 59d7166..a9a9340 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -106,7 +106,12 @@ static int qemudSetNonBlock(int fd) {
 }
 
 
-static void qemudDispatchVMEvent(int fd, int events, void *opaque);
+static void qemudDomainEventDispatch (struct qemud_driver *driver,
+  virDomainObjPtr vm,
+  virDomainEventType evt);
+
+static void qemudDispatchVMEvent(int fd, virEventHandleType events,
+ void *opaque);
 static int qemudStartVMDaemon(virConnectPtr conn,
   struct qemud_driver *driver,
   virDomainObjPtr vm,
@@ -159,6 +164,10 @@ qemudStartup(void) {
 /* Don't have a dom0 so start from 1 */
 qemu_driver-nextvmid = 1;
 
+/* Init callback list */
+if(VIR_ALLOC(qemu_driver-domainEventCallbacks)  0)
+return -1;
+
 if (!uid) {
 if (asprintf(qemu_driver-logDir,
  %s/log/libvirt/qemu, LOCAL_STATE_DIR) == -1)
@@ -301,6 +310,9 @@ qemudShutdown(void) {
 VIR_FREE(qemu_driver-autostartDir);
 VIR_FREE(qemu_driver-vncTLSx509certdir);
 
+/* Free domain callback list */
+virDomainEventCallbackListFree(qemu_driver-domainEventCallbacks);
+
 if (qemu_driver-brctl)
 brShutdown(qemu_driver-brctl);
 
@@ -742,6 +754,9 @@ static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) {
 return -1;
 }
 
+static virDomainPtr qemudDomainLookupByName(virConnectPtr conn,
+const char *name);
+
 static int qemudStartVMDaemon(virConnectPtr conn,
   struct qemud_driver *driver,
   virDomainObjPtr vm,
@@ -756,6 +771,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
 unsigned int qemuCmdFlags;
 fd_set keepfd;
 const char *emulator;
+virDomainPtr dom;
 
 FD_ZERO(keepfd);
 
@@ -905,11 +921,15 @@ static int qemudStartVMDaemon(virConnectPtr conn,
 
 if (ret == 0) {
 if ((virEventAddHandle(vm-stdout_fd,
-   POLLIN | POLLERR | POLLHUP,
+   VIR_EVENT_HANDLE_READABLE |
+   VIR_EVENT_HANDLE_ERROR |
+   VIR_EVENT_HANDLE_HANGUP,
qemudDispatchVMEvent,
driver)  0) ||
 (virEventAddHandle(vm-stderr_fd,
-   POLLIN | POLLERR | POLLHUP,
+   VIR_EVENT_HANDLE_READABLE |
+   VIR_EVENT_HANDLE_ERROR |
+   VIR_EVENT_HANDLE_HANGUP,
qemudDispatchVMEvent,
driver)  0) ||
 (qemudWaitForMonitor(conn, driver, vm)  0) ||
@@ -918,6 +938,11 @@ static int qemudStartVMDaemon(virConnectPtr conn,
 qemudShutdownVMDaemon(conn, driver, vm);
 return -1;
 }
+dom = qemudDomainLookupByName(conn,vm-def-name);
+if(dom)
+qemudDomainEventDispatch(driver, vm, VIR_DOMAIN_EVENT_STARTED);
+else
+DEBUG0(Warning - dom is NULL at domain start);
 }
 
 return ret;
@@ -1012,6 +1037,7 @@ static int qemudDispatchVMLog(struct qemud_driver *driver, virDomainObjPtr vm, i
 static int qemudDispatchVMFailure(struct qemud_driver *driver, virDomainObjPtr vm,
   int fd ATTRIBUTE_UNUSED) {
 qemudShutdownVMDaemon(NULL, driver, vm);
+qemudDomainEventDispatch(driver, vm, VIR_DOMAIN_EVENT_STOPPED);
 if (!vm-persistent)
 virDomainRemoveInactive(driver-domains,
 vm);
@@ -1019,7 +1045,8 @@ static int qemudDispatchVMFailure(struct qemud_driver *driver, virDomainObjPtr v
 }
 
 
-static void qemudDispatchVMEvent(int fd, int events, void *opaque) {
+static void
+qemudDispatchVMEvent(int fd, virEventHandleType events, void *opaque) {
 struct qemud_driver *driver = (struct qemud_driver *)opaque;
 virDomainObjPtr vm = NULL;
 unsigned int i;
@@ -1036,7 +1063,7 @@ static void qemudDispatchVMEvent(int fd, int events, void *opaque) {
 if (!vm)
 return;
 
-if (events == POLLIN)
+if (events == VIR_EVENT_HANDLE_READABLE)
 

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

2008-10-21 Thread Ben Guthro
[PATCH 07/12] Domain Events - remote driver
Deliver local callbacks in response to remote events

 remote_internal.c |  276 +-
 1 file changed, 271 insertions(+), 5 deletions(-)
diff --git a/src/remote_internal.c b/src/remote_internal.c
index 35b7b4b..8875674 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -34,6 +34,7 @@
 #include signal.h
 #include sys/types.h
 #include sys/stat.h
+#include sys/poll.h
 #include fcntl.h
 
 #ifdef HAVE_SYS_WAIT_H
@@ -73,6 +74,7 @@
 #include remote_protocol.h
 #include memory.h
 #include util.h
+#include event.h
 
 /* Per-connection private data. */
 #define MAGIC 999   /* private_data-magic if OK */
@@ -97,6 +99,13 @@ struct private_data {
 unsigned int saslDecodedLength;
 unsigned int saslDecodedOffset;
 #endif
+/* The list of domain event callbacks */
+virDomainEventCallbackListPtr callbackList;
+/* The queue of domain events generated
+   during a call / response rpc  */
+virDomainEventQueuePtr domainEvents;
+/* Timer for flushing domainEvents queue */
+int eventFlushTimer;
 };
 
 #define GET_PRIVATE(conn,retcode)   \
@@ -156,7 +165,10 @@ static void make_nonnull_domain (remote_nonnull_domain *dom_dst, virDomainPtr do
 static void make_nonnull_network (remote_nonnull_network *net_dst, virNetworkPtr net_src);
 static void make_nonnull_storage_pool (remote_nonnull_storage_pool *pool_dst, virStoragePoolPtr vol_src);
 static void make_nonnull_storage_vol (remote_nonnull_storage_vol *vol_dst, virStorageVolPtr vol_src);
-
+void remoteDomainEventFired(int fd, virEventHandleType event, void *data);
+static void remoteDomainProcessEvent(virConnectPtr conn, XDR *xdr);
+static void remoteDomainQueueEvent(virConnectPtr conn, XDR *xdr);
+void remoteDomainEventQueueFlush(int timer, void *opaque);
 /*--*/
 
 /* Helper functions for remoteOpen. */
@@ -680,6 +692,36 @@ doRemoteOpen (virConnectPtr conn,
   (xdrproc_t) xdr_void, (char *) NULL) == -1)
 goto failed;
 
+if(VIR_ALLOC(priv-callbackList)0) {
+error(conn, VIR_ERR_INVALID_ARG, _(Error allocating callbacks list));
+goto failed;
+}
+
+if(VIR_ALLOC(priv-domainEvents)0) {
+error(conn, VIR_ERR_INVALID_ARG, _(Error allocating domainEvents));
+goto failed;
+}
+
+DEBUG0(Adding Handler for remote events);
+/* Set up a callback to listen on the socket data */
+if (virEventAddHandle(priv-sock,
+  VIR_EVENT_HANDLE_READABLE |
+  VIR_EVENT_HANDLE_ERROR |
+  VIR_EVENT_HANDLE_HANGUP,
+  remoteDomainEventFired,
+  conn)  0) {
+DEBUG0(virEventAddHandle failed: No addHandleImpl defined.
+continuing without events.);
+} else {
+
+DEBUG0(Adding Timeout for remote event queue flushing);
+if ( (priv-eventFlushTimer = virEventAddTimeout(-1,
+   remoteDomainEventQueueFlush,
+   conn))  0) {
+DEBUG0(virEventAddTimeout failed: No addTimeoutImpl defined. 
+continuing without events.);
+}
+}
 /* Successful. */
 retcode = VIR_DRV_OPEN_SUCCESS;
 
@@ -1101,6 +1143,11 @@ doRemoteClose (virConnectPtr conn, struct private_data *priv)
   (xdrproc_t) xdr_void, (char *) NULL) == -1)
 return -1;
 
+/* Remove handle for remote events */
+virEventRemoveHandle(priv-sock);
+/* Remove timout */
+virEventRemoveTimeout(priv-eventFlushTimer);
+
 /* Close socket. */
 if (priv-uses_tls  priv-session) {
 gnutls_bye (priv-session, GNUTLS_SHUT_RDWR);
@@ -1132,6 +1179,12 @@ doRemoteClose (virConnectPtr conn, struct private_data *priv)
 /* Free private data. */
 priv-magic = DEAD;
 
+/* Free callback list */
+virDomainEventCallbackListFree(priv-callbackList);
+
+/* Free queued events */
+virDomainEventQueueFree(priv-domainEvents);
+
 return 0;
 }
 
@@ -4288,6 +4341,52 @@ remoteAuthPolkit (virConnectPtr conn, struct private_data *priv, int in_open,
 return 0;
 }
 #endif /* HAVE_POLKIT */
+/*--*/
+
+static int remoteDomainEventRegister (virConnectPtr conn,
+   void *callback ATTRIBUTE_UNUSED,
+   void *opaque ATTRIBUTE_UNUSED)
+{
+struct private_data *priv = conn-privateData;
+
+if (virDomainEventCallbackListAdd(conn, priv-callbackList,
+  callback, opaque)  0) {
+ error (conn, VIR_ERR_RPC, _(adding cb to list));
+ return -1;
+}
+
+if ( priv-callbackList-count == 1 ) {
+/* Tell the server when we are 

[libvirt] Re: [PATCH 08/12] Domain Events - lxc driver

2008-10-21 Thread Ben Guthro
[PATCH 08/12] Domain Events - lxc driver
Minor changes to LXC driver structure to prevent compile errors

 lxc_driver.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/lxc_driver.c b/src/lxc_driver.c
index c598d1d..d1e4418 100644
--- a/src/lxc_driver.c
+++ b/src/lxc_driver.c
@@ -583,7 +583,7 @@ static int lxcVmTerminate(virConnectPtr conn,
 }
 
 static void lxcMonitorEvent(int fd,
-int events ATTRIBUTE_UNUSED,
+virEventHandleType events ATTRIBUTE_UNUSED,
 void *data)
 {
 lxc_driver_t *driver = data;
@@ -811,7 +811,7 @@ static int lxcVmStart(virConnectPtr conn,
 vm-state = VIR_DOMAIN_RUNNING;
 
 if (virEventAddHandle(vm-monitor,
-  POLLERR | POLLHUP,
+  VIR_EVENT_HANDLE_ERROR | VIR_EVENT_HANDLE_HANGUP,
   lxcMonitorEvent,
   driver)  0) {
 lxcVmTerminate(conn, driver, vm, 0);
@@ -1278,6 +1278,8 @@ static virDriver lxcDriver = {
 NULL, /* domainMemoryPeek */
 NULL, /* nodeGetCellsFreeMemory */
 NULL, /* getFreeMemory */
+NULL, /* domainEventRegister */
+NULL, /* domainEventDeregister */
 };
 
 static virStateDriver lxcStateDriver = {
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Re: [PATCH 09/12] Domain Events - openvz driver

2008-10-21 Thread Ben Guthro
[PATCH 09/12] Domain Events - openvz driver
Minor changes to openvz driver structure to prevent compile errors

 openvz_driver.c |2 ++
 1 file changed, 2 insertions(+)
diff --git a/src/openvz_driver.c b/src/openvz_driver.c
index b82d0df..2d0ddaa 100644
--- a/src/openvz_driver.c
+++ b/src/openvz_driver.c
@@ -1012,6 +1012,8 @@ static virDriver openvzDriver = {
 NULL, /* domainMemoryPeek */
 NULL, /* nodeGetCellsFreeMemory */
 NULL, /* nodeGetFreeMemory */
+NULL, /* domainEventRegister */
+NULL, /* domainEventDeregister */
 };
 
 int openvzRegister(void) {
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Re: [PATCH 10/12] Domain Events - test driver

2008-10-21 Thread Ben Guthro
[PATCH 10/12] Domain Events - test driver
Minor changes to test driver structure to prevent compile errors

 test.c |2 ++
 1 file changed, 2 insertions(+)
diff --git a/src/test.c b/src/test.c
index aab74e4..34d35db 100644
--- a/src/test.c
+++ b/src/test.c
@@ -1552,6 +1552,8 @@ static virDriver testDriver = {
 NULL, /* domainMemoryPeek */
 testNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */
 NULL, /* getFreeMemory */
+NULL, /* domainEventRegister */
+NULL, /* domainEventDeregister */
 };
 
 static virNetworkDriver testNetworkDriver = {
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Re: [PATCH 11/12] Domain Events - test harness

2008-10-21 Thread Ben Guthro
[PATCH 11/12] Domain Events - test harness
Test app, and infrastructure changes for an example dir

 Makefile.am  |2
 configure.in |3
 examples/domain-events/events-c/Makefile.am  |5
 examples/domain-events/events-c/event-test.c |  261 +++
 libvirt.spec.in  |3
 5 files changed, 272 insertions(+), 2 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 073063d..d40a151 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -4,7 +4,7 @@ LCOV = lcov
 GENHTML = genhtml
 
 SUBDIRS = gnulib/lib include src qemud proxy docs gnulib/tests \
-  python tests po
+  python tests po examples/domain-events/events-c
 
 ACLOCAL_AMFLAGS = -I m4 -I gnulib/m4
 
diff --git a/configure.in b/configure.in
index 66d271a..b7890d1 100644
--- a/configure.in
+++ b/configure.in
@@ -1067,7 +1067,8 @@ AC_OUTPUT(Makefile src/Makefile include/Makefile docs/Makefile \
   tests/sexpr2xmldata/Makefile \
   tests/xmconfigdata/Makefile \
   tests/xencapsdata/Makefile \
-  tests/virshdata/Makefile tests/confdata/Makefile)
+  tests/virshdata/Makefile tests/confdata/Makefile \
+  examples/domain-events/events-c/Makefile)
 
 AC_MSG_NOTICE([])
 AC_MSG_NOTICE([Configuration summary])
diff --git a/examples/domain-events/events-c/Makefile.am b/examples/domain-events/events-c/Makefile.am
new file mode 100644
index 000..3c63ca3
--- /dev/null
+++ b/examples/domain-events/events-c/Makefile.am
@@ -0,0 +1,5 @@
+INCLUDES = [EMAIL PROTECTED]@/include
+bin_PROGRAMS = event-test
+event_test_CFLAGS = $(WARN_CFLAGS)
+event_test_SOURCES = event-test.c
+event_test_LDADD = @top_builddir@/src/libvirt.la
diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c
new file mode 100644
index 000..55ed2f7
--- /dev/null
+++ b/examples/domain-events/events-c/event-test.c
@@ -0,0 +1,261 @@
+#include config.h
+#include stdio.h
+#include string.h
+#include sys/types.h
+#include sys/poll.h
+#include libvirt/libvirt.h
+
+#define DEBUG0(fmt) printf(%s:%d ::  fmt \n, \
+__FUNCTION__, __LINE__)
+#define DEBUG(fmt, ...) printf(%s:%d:  fmt \n, \
+__FUNCTION__, __LINE__, __VA_ARGS__)
+#define STREQ(a,b) (strcmp((a),(b)) == 0)
+
+#ifndef ATTRIBUTE_UNUSED
+#define ATTRIBUTE_UNUSED __attribute__((__unused__))
+#endif
+
+/* handle globals */
+int h_fd = 0;
+virEventHandleType h_event = 0;
+virEventHandleCallback h_cb = NULL;
+void *h_opaque = NULL;
+
+/* timeout globals */
+#define TIMEOUT_MS 1000
+int t_active = 0;
+int t_timeout = -1;
+virEventTimeoutCallback t_cb = NULL;
+void *t_opaque = NULL;
+
+
+/* Prototypes */
+const char *eventToString(int event);
+int myDomainEventCallback1 (virConnectPtr conn, virDomainPtr dom,
+int event, void *opaque);
+int myDomainEventCallback2 (virConnectPtr conn, virDomainPtr dom,
+int event, void *opaque);
+int myEventAddHandleFunc  (int fd, virEventHandleType event,
+   virEventHandleCallback cb, void *opaque);
+void myEventUpdateHandleFunc(int fd, virEventHandleType event);
+int  myEventRemoveHandleFunc(int fd);
+
+int myEventAddTimeoutFunc(int timeout, virEventTimeoutCallback cb,
+  void *opaque);
+void myEventUpdateTimeoutFunc(int timer, int timout);
+int myEventRemoveTimeoutFunc(int timer);
+
+int myEventHandleTypeToPollEvent(virEventHandleType events);
+virEventHandleType myPollEventToEventHandleType(int events);
+
+void usage(const char *pname);
+
+/* Callback functions */
+
+const char *eventToString(int event) {
+const char *ret = NULL;
+switch(event) {
+case VIR_DOMAIN_EVENT_ADDED:
+ret =Added;
+break;
+case VIR_DOMAIN_EVENT_REMOVED:
+ret =Removed;
+break;
+case VIR_DOMAIN_EVENT_STARTED:
+ret =Started;
+break;
+case VIR_DOMAIN_EVENT_SUSPENDED:
+ret =Suspended;
+break;
+case VIR_DOMAIN_EVENT_RESUMED:
+ret =Resumed;
+break;
+case VIR_DOMAIN_EVENT_STOPPED:
+ret =Stopped;
+break;
+case VIR_DOMAIN_EVENT_SAVED:
+ret =Saved;
+break;
+case VIR_DOMAIN_EVENT_RESTORED:
+ret =Restored;
+break;
+default:
+ret =Unknown Event;
+}
+return ret;
+}
+
+int myDomainEventCallback1 (virConnectPtr conn ATTRIBUTE_UNUSED,
+virDomainPtr dom,
+int event,
+void *opaque ATTRIBUTE_UNUSED)
+{
+printf(%s EVENT: Domain %s(%d) %s\n, __FUNCTION__, virDomainGetName(dom),
+   virDomainGetID(dom), eventToString(event));
+return 0;
+}
+
+int myDomainEventCallback2 (virConnectPtr conn ATTRIBUTE_UNUSED,
+virDomainPtr dom,
+   

[libvirt] [PATCH 00/12] Domain Events - Round 3

2008-10-21 Thread Ben Guthro
The following patch series implements the Events API discussed here previously 
in this thread:
http://www.redhat.com/archives/libvir-list/2008-September/msg00321.html 
http://www.redhat.com/archives/libvir-list/2008-October/msg00245.html and
https://www.redhat.com/archives/libvir-list/2008-October/msg00432.html

Once again, I have broken these into the following patches:

events-01-public-api  Changes to libvirt.h, and libvirt.c
events-02-internal-apiChanges to internal.h and event code
events-03-qemud   Changes to the daemon 
events-04-qemud-rpc-protocol  Changes to the qemud rpc protocol
events-05-driver.patchChanges to the driver API
events-06-driver-qemu Changes to the qemu driver
events-07-driver-remote   Changes to the remote driver
events-08-driver-lxc  Minor changes to LXC driver structure to prevent 
compile errors
events-09-driver-openvz   Minor changes to openvz driver structure to 
prevent compile errors
events-10-driver-test Minor changes to test driver structure to prevent 
compile errors
events-11-example-testapp Test app, and infrastructure changes for an 
example dir
events-12-python-ignore.patch Add functions to be ignored, for now

Differences from last submission - 
Formatting issues brought up by Daniel V
Some protocol simplifications
General fixes suggested from last time around.
Removal of POLLxxx stuff (this caused a bit of churn)

I think I got them all, but please let me know if I missed something.

Again, domain events are only emitted from qemu/kvm guests.


Ben Guthro



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


[libvirt] [PATCH 00/12] Domain Events - Round 3

2008-10-21 Thread Ben Guthro
The following patch series implements the Events API discussed here previously 
in this thread:
http://www.redhat.com/archives/libvir-list/2008-September/msg00321.html 
http://www.redhat.com/archives/libvir-list/2008-October/msg00245.html and
https://www.redhat.com/archives/libvir-list/2008-October/msg00432.html

Once again, I have broken these into the following patches:

events-01-public-api  Changes to libvirt.h, and libvirt.c
events-02-internal-apiChanges to internal.h and event code
events-03-qemud   Changes to the daemon 
events-04-qemud-rpc-protocol  Changes to the qemud rpc protocol
events-05-driver.patchChanges to the driver API
events-06-driver-qemu Changes to the qemu driver
events-07-driver-remote   Changes to the remote driver
events-08-driver-lxc  Minor changes to LXC driver structure to prevent 
compile errors
events-09-driver-openvz   Minor changes to openvz driver structure to 
prevent compile errors
events-10-driver-test Minor changes to test driver structure to prevent 
compile errors
events-11-example-testapp Test app, and infrastructure changes for an 
example dir
events-12-python-ignore.patch Add functions to be ignored, for now

Differences from last submission - 
Formatting issues brought up by Daniel V
Some protocol simplifications
General fixes suggested from last time around.
Removal of POLLxxx stuff (this caused a bit of churn)

I think I got them all, but please let me know if I missed something.

Again, domain events are only emitted from qemu/kvm guests.


Ben Guthro


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


[libvirt] Re: [PATCH 02/12] Domain Events - Internal API

2008-10-21 Thread Ben Guthro
[PATCH 02/12] Domain Events - Internal API
This patch
   -Removes EventImpl from being a private function
   -Introduces the virDomainEventCallbackListPtr, and virDomainEventQueuePtr 
objects
diff --git a/proxy/Makefile.am b/proxy/Makefile.am
index 5902cab..75ba6b4 100644
--- a/proxy/Makefile.am
+++ b/proxy/Makefile.am
@@ -17,6 +17,7 @@ libvirt_proxy_SOURCES = libvirt_proxy.c @top_srcdir@/src/xend_internal.c \
 @top_srcdir@/src/memory.c \
 @top_srcdir@/src/domain_conf.c \
 @top_srcdir@/src/util.c \
+	@top_srcdir@/src/event.c \
 	@top_srcdir@/src/uuid.c
 libvirt_proxy_LDFLAGS = $(WARN_CFLAGS)
 libvirt_proxy_DEPENDENCIES =
diff --git a/src/event.c b/src/event.c
index 49a9e61..1e2b234 100644
--- a/src/event.c
+++ b/src/event.c
@@ -34,14 +34,15 @@ static virEventAddTimeoutFunc addTimeoutImpl = NULL;
 static virEventUpdateTimeoutFunc updateTimeoutImpl = NULL;
 static virEventRemoveTimeoutFunc removeTimeoutImpl = NULL;
 
-int virEventAddHandle(int fd, int events, virEventHandleCallback cb, void *opaque) {
+int virEventAddHandle(int fd, virEventHandleType events,
+  virEventHandleCallback cb, void *opaque) {
 if (!addHandleImpl)
 return -1;
 
 return addHandleImpl(fd, events, cb, opaque);
 }
 
-void virEventUpdateHandle(int fd, int events) {
+void virEventUpdateHandle(int fd, virEventHandleType events) {
 updateHandleImpl(fd, events);
 }
 
@@ -70,12 +71,22 @@ int virEventRemoveTimeout(int timer) {
 return removeTimeoutImpl(timer);
 }
 
-void __virEventRegisterImpl(virEventAddHandleFunc addHandle,
-virEventUpdateHandleFunc updateHandle,
-virEventRemoveHandleFunc removeHandle,
-virEventAddTimeoutFunc addTimeout,
-virEventUpdateTimeoutFunc updateTimeout,
-virEventRemoveTimeoutFunc removeTimeout) {
+/**
+ * virEventRegisterImpl:
+ * Register an EventImpl
+ * @addHandle: the callback to add fd handles
+ * @updateHandle: the callback to update fd handles
+ * @removeHandle: the callback to remove fd handles
+ * @addTimeout: the callback to add a timeout
+ * @updateTimeout: the callback to update a timeout
+ * @removeTimeout: the callback to remove a timeout
+ */
+void virEventRegisterImpl(virEventAddHandleFunc addHandle,
+  virEventUpdateHandleFunc updateHandle,
+  virEventRemoveHandleFunc removeHandle,
+  virEventAddTimeoutFunc addTimeout,
+  virEventUpdateTimeoutFunc updateTimeout,
+  virEventRemoveTimeoutFunc removeTimeout) {
 addHandleImpl = addHandle;
 updateHandleImpl = updateHandle;
 removeHandleImpl = removeHandle;
diff --git a/src/event.h b/src/event.h
index 758573c..5cd6310 100644
--- a/src/event.h
+++ b/src/event.h
@@ -23,38 +23,29 @@
 
 #ifndef __VIR_EVENT_H__
 #define __VIR_EVENT_H__
-
-
-/**
- * virEventHandleCallback: callback for receiving file handle events
- *
- * @fd: file handle on which the event occurred
- * @events: bitset of events from POLLnnn constants
- * @opaque: user data registered with handle
- */
-typedef void (*virEventHandleCallback)(int fd, int events, void *opaque);
-
+#include internal.h
 /**
  * virEventAddHandle: register a callback for monitoring file handle events
  *
  * @fd: file handle to monitor for events
- * @events: bitset of events to watch from POLLnnn constants
+ * @events: bitset of events to watch from virEventHandleType constants
  * @cb: callback to invoke when an event occurs
  * @opaque: user data to pass to callback
  *
  * returns -1 if the file handle cannot be registered, 0 upon success
  */
-int virEventAddHandle(int fd, int events, virEventHandleCallback cb, void *opaque);
+int virEventAddHandle(int fd, virEventHandleType events,
+  virEventHandleCallback cb, void *opaque);
 
 /**
  * virEventUpdateHandle: change event set for a monitored file handle
  *
  * @fd: file handle to monitor for events
- * @events: bitset of events to watch from POLLnnn constants
+ * @events: bitset of events to watch from virEventHandleType constants
  *
  * Will not fail if fd exists
  */
-void virEventUpdateHandle(int fd, int events);
+void virEventUpdateHandle(int fd, virEventHandleType events);
 
 /**
  * virEventRemoveHandle: unregister a callback from a file handle
@@ -66,14 +57,6 @@ void virEventUpdateHandle(int fd, int events);
 int virEventRemoveHandle(int fd);
 
 /**
- * virEventTimeoutCallback: callback for receiving timer events
- *
- * @timer: timer id emitting the event
- * @opaque: user data registered with handle
- */
-typedef void (*virEventTimeoutCallback)(int timer, void *opaque);
-
-/**
  * virEventAddTimeout: register a callback for a timer event
  *
  * @frequency: time between events in milliseconds
@@ -110,21 +93,4 @@ void virEventUpdateTimeout(int timer, int frequency);
  */
 int 

[libvirt] Re: [PATCH 01/12] Domain Events - Public API

2008-10-21 Thread Ben Guthro
[PATCH 01/12] Domain Events - Public API

This patch does the following:
   -implements the Event register/deregister code
   -Adds some callback lists, and queue functions used by drivers
   -Move EventImpl definitions into the public
diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
index 35b80d0..7f784c6 100644
--- a/include/libvirt/libvirt.h
+++ b/include/libvirt/libvirt.h
@@ -993,6 +993,156 @@ char *  virStorageVolGetPath(virStorageVolPtr vol);
 virDomainPtrvirDomainCreateLinux(virConnectPtr conn,
  const char *xmlDesc,
  unsigned int flags);
+
+/*
+ * Domain Event Notification
+ */
+
+/**
+ * virDomainEventType:
+ *
+ * a virDomainEventType is emitted during domain lifecycle events
+ */
+typedef enum {
+  VIR_DOMAIN_EVENT_ADDED,
+  VIR_DOMAIN_EVENT_REMOVED,
+  VIR_DOMAIN_EVENT_STARTED,
+  VIR_DOMAIN_EVENT_SUSPENDED,
+  VIR_DOMAIN_EVENT_RESUMED,
+  VIR_DOMAIN_EVENT_STOPPED,
+  VIR_DOMAIN_EVENT_SAVED,
+  VIR_DOMAIN_EVENT_RESTORED,
+} virDomainEventType;
+
+/**
+ * virConnectDomainEventCallback:
+ * @conn: virConnect connection
+ * @dom: The domain on which the event occured
+ * @event: The specfic virDomainEventType which occured
+ * @opaque: opaque user data
+ *
+ * A callback function to be registered, and called when a domain event occurs 
+ */
+typedef int (*virConnectDomainEventCallback)(virConnectPtr conn,
+ virDomainPtr dom,
+ int event,
+ void *opaque);
+
+int virConnectDomainEventRegister(virConnectPtr conn,
+  virConnectDomainEventCallback cb,
+  void *opaque);
+
+int virConnectDomainEventDeregister(virConnectPtr conn,
+virConnectDomainEventCallback cb);
+
+/*
+ * Events Implementation
+ */
+
+/**
+ * virEventHandleType:
+ *
+ * a virEventHandleType is used similar to POLLxxx FD events, but is specific
+ * to libvirt. A client app must translate to, and from POLL events when using
+ * this construct.
+ */
+typedef enum {
+VIR_EVENT_HANDLE_READABLE  = (1  0),
+VIR_EVENT_HANDLE_WRITABLE  = (1  1),
+VIR_EVENT_HANDLE_ERROR = (1  2),
+VIR_EVENT_HANDLE_HANGUP= (1  3),
+} virEventHandleType;
+
+/**
+ * virEventHandleCallback: callback for receiving file handle events
+ *
+ * @fd: file handle on which the event occurred
+ * @events: bitset of events from virEventHandleType constants
+ * @opaque: user data registered with handle
+ */
+typedef void (*virEventHandleCallback)(int fd, virEventHandleType events,
+   void *opaque);
+
+/**
+ * virEventAddHandleFunc:
+ * @fd: file descriptor to listen on
+ * @event: events on which to fire the callback
+ * @cb: the callback to be called
+ * @opaque: user data to pass to the callback
+ *
+ * Part of the EventImpl, this callback Adds a file handle callback to
+ *  listen for specific events
+ */
+typedef int (*virEventAddHandleFunc)(int fd, virEventHandleType event,
+ virEventHandleCallback cb, void *opaque);
+
+/**
+ * virEventUpdateHandleFunc:
+ * @fd: file descriptor to modify
+ * @event: new events to listen on
+ *
+ * Part of the EventImpl, this user-provided callback is notified when
+ * events to listen on change
+ */
+typedef void (*virEventUpdateHandleFunc)(int fd, virEventHandleType event);
+
+/**
+ * virEventRemoveHandleFunc:
+ * @fd: file descriptor to stop listening on
+ *
+ * Part of the EventImpl, this user-provided callback is notified when
+ * an fd is no longer being listened on
+ */
+typedef int (*virEventRemoveHandleFunc)(int fd);
+
+/**
+ * virEventTimeoutCallback: callback for receiving timer events
+ *
+ * @timer: timer id emitting the event
+ * @opaque: user data registered with handle
+ */
+typedef void (*virEventTimeoutCallback)(int timer, void *opaque);
+
+/**
+ * virEventAddTimeoutFunc:
+ * @timeout: The timeout to monitor
+ * @cb: the callback to call when timeout has expired
+ * @opaque: user data to pass to the callback
+ *
+ * Part of the EventImpl, this user-defined callback handles adding an
+ * event timeout.
+ *
+ * Returns a timer value
+ */
+typedef int (*virEventAddTimeoutFunc)(int timeout, virEventTimeoutCallback cb,
+  void *opaque);
+
+/**
+ * virEventUpdateTimeoutFunc:
+ * @timer: the timer to modify
+ * @timeout: the new timeout value
+ *
+ * Part of the EventImpl, this user-defined callback updates an
+ * event timeout.
+ */
+typedef void (*virEventUpdateTimeoutFunc)(int timer, int timeout);
+
+/**
+ * virEventRemoveTimeoutFunc:
+ * @timer: the timer to remove
+ *
+ * Part of the EventImpl, this user-defined callback removes a timer
+ *
+ * Returns 0 on success, -1 on 

[libvirt] Re: [PATCH 01/12] Domain Events - Public API

2008-10-21 Thread Ben Guthro
[PATCH 01/12] Domain Events - Public API

This patch does the following:
   -implements the Event register/deregister code
   -Adds some callback lists, and queue functions used by drivers
   -Move EventImpl definitions into the public

diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
index 35b80d0..7f784c6 100644
--- a/include/libvirt/libvirt.h
+++ b/include/libvirt/libvirt.h
@@ -993,6 +993,156 @@ char *  virStorageVolGetPath(virStorageVolPtr vol);
 virDomainPtrvirDomainCreateLinux(virConnectPtr conn,
  const char *xmlDesc,
  unsigned int flags);
+
+/*
+ * Domain Event Notification
+ */
+
+/**
+ * virDomainEventType:
+ *
+ * a virDomainEventType is emitted during domain lifecycle events
+ */
+typedef enum {
+  VIR_DOMAIN_EVENT_ADDED,
+  VIR_DOMAIN_EVENT_REMOVED,
+  VIR_DOMAIN_EVENT_STARTED,
+  VIR_DOMAIN_EVENT_SUSPENDED,
+  VIR_DOMAIN_EVENT_RESUMED,
+  VIR_DOMAIN_EVENT_STOPPED,
+  VIR_DOMAIN_EVENT_SAVED,
+  VIR_DOMAIN_EVENT_RESTORED,
+} virDomainEventType;
+
+/**
+ * virConnectDomainEventCallback:
+ * @conn: virConnect connection
+ * @dom: The domain on which the event occured
+ * @event: The specfic virDomainEventType which occured
+ * @opaque: opaque user data
+ *
+ * A callback function to be registered, and called when a domain event occurs 
+ */
+typedef int (*virConnectDomainEventCallback)(virConnectPtr conn,
+ virDomainPtr dom,
+ int event,
+ void *opaque);
+
+int virConnectDomainEventRegister(virConnectPtr conn,
+  virConnectDomainEventCallback cb,
+  void *opaque);
+
+int virConnectDomainEventDeregister(virConnectPtr conn,
+virConnectDomainEventCallback cb);
+
+/*
+ * Events Implementation
+ */
+
+/**
+ * virEventHandleType:
+ *
+ * a virEventHandleType is used similar to POLLxxx FD events, but is specific
+ * to libvirt. A client app must translate to, and from POLL events when using
+ * this construct.
+ */
+typedef enum {
+VIR_EVENT_HANDLE_READABLE  = (1  0),
+VIR_EVENT_HANDLE_WRITABLE  = (1  1),
+VIR_EVENT_HANDLE_ERROR = (1  2),
+VIR_EVENT_HANDLE_HANGUP= (1  3),
+} virEventHandleType;
+
+/**
+ * virEventHandleCallback: callback for receiving file handle events
+ *
+ * @fd: file handle on which the event occurred
+ * @events: bitset of events from virEventHandleType constants
+ * @opaque: user data registered with handle
+ */
+typedef void (*virEventHandleCallback)(int fd, virEventHandleType events,
+   void *opaque);
+
+/**
+ * virEventAddHandleFunc:
+ * @fd: file descriptor to listen on
+ * @event: events on which to fire the callback
+ * @cb: the callback to be called
+ * @opaque: user data to pass to the callback
+ *
+ * Part of the EventImpl, this callback Adds a file handle callback to
+ *  listen for specific events
+ */
+typedef int (*virEventAddHandleFunc)(int fd, virEventHandleType event,
+ virEventHandleCallback cb, void *opaque);
+
+/**
+ * virEventUpdateHandleFunc:
+ * @fd: file descriptor to modify
+ * @event: new events to listen on
+ *
+ * Part of the EventImpl, this user-provided callback is notified when
+ * events to listen on change
+ */
+typedef void (*virEventUpdateHandleFunc)(int fd, virEventHandleType event);
+
+/**
+ * virEventRemoveHandleFunc:
+ * @fd: file descriptor to stop listening on
+ *
+ * Part of the EventImpl, this user-provided callback is notified when
+ * an fd is no longer being listened on
+ */
+typedef int (*virEventRemoveHandleFunc)(int fd);
+
+/**
+ * virEventTimeoutCallback: callback for receiving timer events
+ *
+ * @timer: timer id emitting the event
+ * @opaque: user data registered with handle
+ */
+typedef void (*virEventTimeoutCallback)(int timer, void *opaque);
+
+/**
+ * virEventAddTimeoutFunc:
+ * @timeout: The timeout to monitor
+ * @cb: the callback to call when timeout has expired
+ * @opaque: user data to pass to the callback
+ *
+ * Part of the EventImpl, this user-defined callback handles adding an
+ * event timeout.
+ *
+ * Returns a timer value
+ */
+typedef int (*virEventAddTimeoutFunc)(int timeout, virEventTimeoutCallback cb,
+  void *opaque);
+
+/**
+ * virEventUpdateTimeoutFunc:
+ * @timer: the timer to modify
+ * @timeout: the new timeout value
+ *
+ * Part of the EventImpl, this user-defined callback updates an
+ * event timeout.
+ */
+typedef void (*virEventUpdateTimeoutFunc)(int timer, int timeout);
+
+/**
+ * virEventRemoveTimeoutFunc:
+ * @timer: the timer to remove
+ *
+ * Part of the EventImpl, this user-defined callback removes a timer
+ *
+ * Returns 0 on success, -1 on 

[libvirt] Re: [PATCH 6/6] host (node) device enumeration - python bindings

2008-10-21 Thread David Lively
This patch implements the python bindings, rather lamely.  I'll submit a
new version with a proper NodeDevice object soon ...

diff --git a/python/generator.py b/python/generator.py
index c706b19..a8fd969 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -258,6 +258,11 @@ py_types = {
 'const virConnectPtr':  ('O', virConnect, virConnectPtr, virConnectPtr),
 'virConnect *':  ('O', virConnect, virConnectPtr, virConnectPtr),
 'const virConnect *':  ('O', virConnect, virConnectPtr, virConnectPtr),
+
+'virNodeDevicePtr':  ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
+'const virNodeDevicePtr':  ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
+'virNodeDevice *':  ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
+'const virNodeDevice *':  ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
 }
 
 py_return_types = {
@@ -315,6 +320,8 @@ skip_impl = (
 'virStoragePoolListVolumes',
 'virDomainBlockPeek',
 'virDomainMemoryPeek',
+'virNodeListDevicesByCap',
+'virNodeListDevices',
 )
 
 
@@ -332,6 +339,10 @@ skip_function = (
 'virCopyLastError', # Python API is called virGetLastError instead
 'virConnectOpenAuth', # Python C code is manually written
 'virDefaultErrorFunc', # Python virErrorFuncHandler impl calls this from C
+'virNodeDeviceGetName',
+'virNodeDeviceGetParent',
+'virNodeDeviceNumOfCaps',
+'virNodeDeviceListCaps',
 )
 
 
@@ -613,6 +624,7 @@ classes_destructors = {
 virStoragePool: virStoragePoolFree,
 virStorageVol: virStorageVolFree,
 virConnect: virConnectClose,
+virNodeDevice : virNodeDeviceFree
 }
 
 functions_noexcept = {
diff --git a/python/libvir.c b/python/libvir.c
index 9cc0c81..e03101d 100644
--- a/python/libvir.c
+++ b/python/libvir.c
@@ -1466,7 +1466,90 @@ libvirt_virStoragePoolLookupByUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *ar
 return(py_retval);
 }
 
+static PyObject *
+libvirt_virNodeListDevices(PyObject *self ATTRIBUTE_UNUSED,
+   PyObject *args) {
+PyObject *py_retval;
+char **names = NULL;
+int c_retval, i;
+virConnectPtr conn;
+PyObject *pyobj_conn;
+unsigned int flags;
+
+
+if (!PyArg_ParseTuple(args, (char *)Oi:virNodeListDevices, pyobj_conn, flags))
+return(NULL);
+conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
+
+c_retval = virNodeNumOfDevices(conn, flags);
+if (c_retval  0)
+return VIR_PY_NONE;
+
+if (c_retval) {
+names = malloc(sizeof(*names) * c_retval);
+if (!names)
+return VIR_PY_NONE;
+c_retval = virNodeListDevices(conn, names, c_retval, flags);
+if (c_retval  0) {
+free(names);
+return VIR_PY_NONE;
+}
+}
+py_retval = PyList_New(c_retval);
+
+if (names) {
+for (i = 0;i  c_retval;i++) {
+PyList_SetItem(py_retval, i, libvirt_constcharPtrWrap(names[i]));
+free(names[i]);
+}
+free(names);
+}
+
+return(py_retval);
+}
 
+static PyObject *
+libvirt_virNodeListDevicesByCap(PyObject *self ATTRIBUTE_UNUSED,
+PyObject *args) {
+PyObject *py_retval;
+char **names = NULL;
+int c_retval, i;
+virConnectPtr conn;
+PyObject *pyobj_conn;
+char *cap;
+unsigned int flags;
+
+if (!PyArg_ParseTuple(args, (char *)Ozi:virNodeListDevicesByCap,
+  pyobj_conn, cap, flags))
+return(NULL);
+conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
+
+c_retval = virNodeNumOfDevicesByCap(conn, cap, flags);
+if (c_retval  0)
+return VIR_PY_NONE;
+
+if (c_retval) {
+names = malloc(sizeof(*names) * c_retval);
+if (!names)
+return VIR_PY_NONE;
+c_retval = virNodeListDevicesByCap(conn, cap, names, c_retval, flags);
+if (c_retval  0) {
+free(names);
+return VIR_PY_NONE;
+}
+}
+py_retval = PyList_New(c_retval);
+
+if (names) {
+for (i = 0;i  c_retval;i++) {
+PyList_SetItem(py_retval, i, libvirt_constcharPtrWrap(names[i]));
+free(names[i]);
+}
+free(names);
+}
+
+return(py_retval);
+}
 
 /
  *	*
@@ -1511,6 +1594,8 @@ static PyMethodDef libvirtMethods[] = {
 {(char *) virStoragePoolGetUUID, libvirt_virStoragePoolGetUUID, METH_VARARGS, NULL},
 {(char *) virStoragePoolGetUUIDString, libvirt_virStoragePoolGetUUIDString, METH_VARARGS, NULL},
 {(char *) virStoragePoolLookupByUUID, libvirt_virStoragePoolLookupByUUID, METH_VARARGS, NULL},
+{(char *) virNodeListDevices, libvirt_virNodeListDevices, METH_VARARGS, NULL},
+{(char *) virNodeListDevicesByCap, libvirt_virNodeListDevicesByCap, METH_VARARGS, NULL},
 {NULL, NULL, 0, NULL}
 };
 
diff --git a/python/libvirt-python-api.xml 

[libvirt] Re: [PATCH 5/6] host (node) device enumeration - virsh support

2008-10-21 Thread David Lively
This patch implements virsh support.  It's pretty sparse right now, and
I'm not wild about the command names I chose.  I welcome suggestions
(i.e., command names  / args) for completing (or changing) this.


diff --git a/src/virsh.c b/src/virsh.c
index 89aa4fa..67963f0 100644
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -4415,6 +4415,83 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
 }
 
 /*
+ * node-list-devices command
+ */
+static const vshCmdInfo info_node_list_devices[] = {
+{syntax, node-list-devices},
+{help, gettext_noop(enumerate devices on this host)},
+{NULL, NULL}
+};
+
+static int
+cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
+{
+char **devices;
+int num_devices, i;
+
+if (!vshConnectionUsability(ctl, ctl-conn, TRUE))
+return FALSE;
+
+num_devices = virNodeNumOfDevices(ctl-conn, 0);
+if (num_devices  0) {
+vshError(ctl, FALSE, %s, _(Failed to count node devices));
+return FALSE;
+} else if (num_devices == 0) {
+return TRUE;
+}
+
+devices = vshMalloc(ctl, sizeof(char *) * num_devices);
+num_devices = virNodeListDevices(ctl-conn, devices, num_devices, 0);
+if (num_devices  0) {
+vshError(ctl, FALSE, %s, _(Failed to list node devices));
+free(devices);
+return FALSE;
+}
+for (i = 0; i  num_devices; i++) {
+vshPrint(ctl, %s\n, devices[i]);
+free(devices[i]);
+}
+free(devices);
+return TRUE;
+}
+
+/*
+ * node-device-dumpxml command
+ */
+static const vshCmdInfo info_node_device_dumpxml[] = {
+{syntax, node-device-dumpxml device},
+{help, gettext_noop(node device details in XML)},
+{desc, gettext_noop(Output the node device details as an XML dump to stdout.)},
+{NULL, NULL}
+};
+
+
+static const vshCmdOptDef opts_node_device_dumpxml[] = {
+{device, VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop(device key)},
+{NULL, 0, 0, NULL}
+};
+
+static int
+cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd)
+{
+const char *name;
+virNodeDevicePtr device;
+
+if (!vshConnectionUsability(ctl, ctl-conn, TRUE))
+return FALSE;
+if (!(name = vshCommandOptString(cmd, device, NULL)))
+return FALSE;
+if (!(device = virNodeDeviceLookupByName(ctl-conn, name))) {
+vshError(ctl, FALSE, %s '%s', _(Could not find matching device), name);
+return FALSE;
+}
+
+vshPrint(ctl, %s\n, virNodeDeviceGetXMLDesc(device, 0));
+virNodeDeviceFree(device);
+return TRUE;
+}
+
+/*
  * hostkey command
  */
 static const vshCmdInfo info_hostname[] = {
@@ -5566,6 +5643,9 @@ static const vshCmdDef commands[] = {
 {net-uuid, cmdNetworkUuid, opts_network_uuid, info_network_uuid},
 {nodeinfo, cmdNodeinfo, NULL, info_nodeinfo},
 
+{node-list-devices, cmdNodeListDevices, NULL, info_node_list_devices},
+{node-device-dumpxml, cmdNodeDeviceDumpXML, opts_node_device_dumpxml, info_node_device_dumpxml},
+
 {pool-autostart, cmdPoolAutostart, opts_pool_autostart, info_pool_autostart},
 {pool-build, cmdPoolBuild, opts_pool_build, info_pool_build},
 {pool-create, cmdPoolCreate, opts_pool_create, info_pool_create},
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 00/12] Domain Events - Round 3

2008-10-21 Thread Ben Guthro
Apologies about the duplicate 00, 01, and 02 emails.

My email seems to be acting up, and I didn't see those patches come 
through...so I resent those 3 
Then they arrived in pairs.

Please ignore the duplicates.


Ben Guthro wrote on 10/21/2008 03:10 PM:
 The following patch series implements the Events API discussed here 
 previously in this thread:
 http://www.redhat.com/archives/libvir-list/2008-September/msg00321.html 
 http://www.redhat.com/archives/libvir-list/2008-October/msg00245.html and
 https://www.redhat.com/archives/libvir-list/2008-October/msg00432.html
 
 Once again, I have broken these into the following patches:
 
 events-01-public-api  Changes to libvirt.h, and libvirt.c
 events-02-internal-apiChanges to internal.h and event code
 events-03-qemud   Changes to the daemon 
 events-04-qemud-rpc-protocol  Changes to the qemud rpc protocol
 events-05-driver.patchChanges to the driver API
 events-06-driver-qemu Changes to the qemu driver
 events-07-driver-remote   Changes to the remote driver
 events-08-driver-lxc  Minor changes to LXC driver structure to 
 prevent compile errors
 events-09-driver-openvz   Minor changes to openvz driver structure to 
 prevent compile errors
 events-10-driver-test Minor changes to test driver structure to 
 prevent compile errors
 events-11-example-testapp Test app, and infrastructure changes for an 
 example dir
 events-12-python-ignore.patch Add functions to be ignored, for now
 
 Differences from last submission - 
 Formatting issues brought up by Daniel V
 Some protocol simplifications
 General fixes suggested from last time around.
 Removal of POLLxxx stuff (this caused a bit of churn)
 
 I think I got them all, but please let me know if I missed something.
 
 Again, domain events are only emitted from qemu/kvm guests.
 
 
 Ben Guthro
 
 
 --
 Libvir-list mailing list
 Libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

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


[libvirt] errors building rpm

2008-10-21 Thread Ben Guthro
I keep getting the following error when trying to build via a
'sudo make rpm' on the tip of the tree

+ /usr/lib/rpm/find-lang.sh /var/tmp/libvirt-0.4.6-1.fc9-root libvirt
No translations found for libvirt in /var/tmp/libvirt-0.4.6-1.fc9-root
error: Bad exit status from /var/tmp/rpm-tmp.82848 (%install)


RPM build errors:
Bad exit status from /var/tmp/rpm-tmp.82848 (%install)
make: *** [rpm] Error 1


This looks like it came in with one of Jim Meyering's latest checkins.

I'm wondering if I'm doing something wrong - or if I'm missing a build
dependency that I did not need previously?

Ben Guthro

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