date:20120131

On 30.01.2012 21:30, Laine Stump wrote:
 On 01/30/2012 06:02 AM, Daniel P. Berrange wrote:
 On Fri, Jan 27, 2012 at 01:35:35PM -0500, Laine Stump wrote:
 When libvirt is shutting down the qemu process, it first sends
 SIGTERM, then waits for 1.6 seconds and, if it sees the process still
 there, sends a SIGKILL.

 There have been reports that this behavior can lead to data loss
 because the guest running in qemu doesn't have time to flush it's disk
 cache buffers before it's unceremoniously whacked.

 One suggestion on how to solve that problem was to remove SIGKILL from
 the normal virDomainDestroyFlags, but still provide the ability to
 kill qemu with SIGKILL by using a new flag to virDomainDestroyFlags.
 This patch is a quick attempt at that in order to start a
 conversation on the topic.

 So what are your opinions? Is this the right way to solve the problem?
 No, we can't change the default semantics of virDomainDestroy in
 this case. Applications expect that we do absolutely everything
 possible to kill of the guest. This is particularly important for
 cluster fencing usage. If we only use SIGTERM, then we're introducing
 unacceptable risk to apps relying on this.

 We could do the opposite though - have a flag to do a gracefully
 destroy, leaving the default as un-graceful.
 
 virDomainShutdown ends up calling qemuProcessKill() too. So, I guess we
 need to add a flag there too.
 
 In the meantime, shouldn't we at least wait longer before resorting to
 SIGKILL? (especially since it appears the current timeout is quite often
 too short). (If we don't at least do that, what we're saying is the
 behavior of virDomainShutdown / virDomainDestroy is to lose your data
 unless you're lucky. If you don't want this behavior, you need to use
 virDomainXXXFlags, and specify the VIR_DOMAIN_DONT_TRASH_MY_DATA flag
 :-P).

I should probably hop into this as I've tried to solve this issue
earlier but got sidetracked and then forgot about it.

Increasing the delay could be temporary workaround, but we should keep
in mind that if we change the delay to X (units of time), I bet in some
cases it will take qemu X+1 units to flush caches.

Therefore I lean to the flag DONT_SENT_SIGKILL and leave the default to
what it is now. However, as a qemu developer pointed out (Luiz?), even
with -no-shutdown qemu will terminate itself after receiving SIGINT and
flushing own caches. So this might be the right way to solve this.

Michal

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

Re: [libvirt] [PATCH] resize: slightly alter signature

On 30.01.2012 20:04, Eric Blake wrote:
 Our existing virDomainBlockResize takes an unsigned long long
 argument; if that command is later taught a DELTA and SHRINK flag,
 we cannot change its type without breaking API (but at least such
 a change would be ABI compatible).  Meanwhile, the only time a
 negative size makes sense is if both DELTA and SHRINK are used
 together, but if we keep the argument unsigned, applications can
 pass the positive delta amount by which they would like to shrink
 the system, and have the flags imply the negative value.  So,
 since this API has not yet been released, and in the interest of
 consistency with existing API, we swap virStorageVolResize to
 always pass an unsigned value.
 
 * include/libvirt/libvirt.h.in (virStorageVolResize): Use unsigned
 argument.
 * src/libvirt.c (virStorageVolResize): Likewise.
 * src/driver.h (virDrvStorageVolUpload): Adjust clients.
 * src/remote/remote_protocol.x (remote_storage_vol_resize_args):
 Likewise.
 * src/remote_protocol-structs: Regenerate.
 Suggested by Daniel P. Berrange.
 ---
  include/libvirt/libvirt.h.in |2 +-
  src/driver.h |2 +-
  src/libvirt.c|   17 +
  src/remote/remote_protocol.x |2 +-
  src/remote_protocol-structs  |2 +-
  5 files changed, 13 insertions(+), 12 deletions(-)
 

ACK

Michal

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

[libvirt] [test-API][PATCH 2/2] Add and update functions in streamAPI

2012-01-31 Thread Wayne Sun

  * only accpet stream object as parameter in __init__

  * remove newStream() from each function
stream object should be pass in as parameter, not create new at
each function.
function also need flags parameter.

  * Add 5 new functions
screenshot(self, domain, screen, flags = 0)
download(self, vol, offset, length, flags = 0)
upload(self, vol, offset, length, flags = 0)
recvAll(self, handler, opaque)
sendAll(self, handler, opaque)

for recvAll and sendAll, handler is a user defined function which
write/read data to/from file.
---
 lib/streamAPI.py |   84 +
 1 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/lib/streamAPI.py b/lib/streamAPI.py
index bc7d217..a1b2d0d 100644
--- a/lib/streamAPI.py
+++ b/lib/streamAPI.py
@@ -38,76 +38,108 @@ append_path(result.group(0))
 import exception
 
 class StreamAPI(object):
-def __init__(self, connection):
-self.conn = connection
+def __init__(self, stream):
+self.stream_obj = stream
 
-def abort(self, flag = 0):
+def abort(self):
 try:
-stream_obj = newStream(flag)
-return stream_obj.abort()
+return self.stream_obj.abort()
 except libvirt.libvirtError, e:
 message = e.get_error_message()
 code = e.get_error_code()
 raise exception.LibvirtAPI(message, code)
 
-def connect(self, flag = 0):
+def connect(self):
 try:
-stream_obj = newStream(flag)
-return stream_obj.connect()
+return self.stream_obj.connect()
 except libvirt.libvirtError, e:
 message = e.get_error_message()
 code = e.get_error_code()
 raise exception.LibvirtAPI(message, code)
 
-def finish(self, flag = 0):
+def finish(self):
 try:
-stream_obj = newStream(flag)
-return stream_obj.finish()
+return self.stream_obj.finish()
 except libvirt.libvirtError, e:
 message = e.get_error_message()
 code = e.get_error_code()
 raise exception.LibvirtAPI(message, code)
 
-def recv(self, flag = 0, data, nbytes):
+def recv(self, nbytes):
 try:
-stream_obj = newStream(flag)
-return stream_obj.recv(data, nbytes)
+return self.stream_obj.recv(nbytes)
 except libvirt.libvirtError, e:
 message = e.get_error_message()
 code = e.get_error_code()
 raise exception.LibvirtAPI(message, code)
 
-def send(self, flag = 0, data, nbytes):
+def send(self, data):
 try:
-stream_obj = newStream(flag)
-return stream_obj.send(data, nbytes)
+return self.stream_obj.send(data)
 except libvirt.libvirtError, e:
 message = e.get_error_message()
 code = e.get_error_code()
 raise exception.LibvirtAPI(message, code)
 
-def eventAddCallback(self, flag = 0, cb, opaque):
+def eventAddCallback(self, cb, opaque):
 try:
-stream_obj = newStream(flag)
-return stream_obj.eventAddCallback(cb, opaque)
+return self.stream_obj.eventAddCallback(cb, opaque)
 except libvirt.libvirtError, e:
 message = e.get_error_message()
 code = e.get_error_code()
 raise exception.LibvirtAPI(message, code)
 
-def eventRemoveCallback(self, flag = 0):
+def eventRemoveCallback(self):
 try:
-stream_obj = newStream(flag)
-return stream_obj.eventRemoveCallback()
+return self.stream_obj.eventRemoveCallback()
 except libvirt.libvirtError, e:
 message = e.get_error_message()
 code = e.get_error_code()
 raise exception.LibvirtAPI(message, code)
 
-def eventUpdateCallback(self, flag = 0, events)
+def eventUpdateCallback(self, events):
 try:
-stream_obj = newStream(flag)
-return stream_obj.eventUpdateCallback(events)
+return self.stream_obj.eventUpdateCallback(events)
+except libvirt.libvirtError, e:
+message = e.get_error_message()
+code = e.get_error_code()
+raise exception.LibvirtAPI(message, code)
+
+def screenshot(self, domain, screen, flags = 0):
+try:
+return self.stream_obj.screenshot(domain, screen, flags)
+except libvirt.libvirtError, e:
+message = e.get_error_message()
+code = e.get_error_code()
+raise exception.LibvirtAPI(message, code)
+
+def download(self, vol, offset, length, flags = 0):
+try:
+return self.stream_obj.download(vol, offset, length, flags)
+except libvirt.libvirtError, e:
+message = e.get_error_message()
+code = e.get_error_code()
+raise

[libvirt] [test-API][PATCH 1/2] Add 2 new functions in storageAPI

2012-01-31 Thread Wayne Sun

  * download(self, poolname, volname, stream, offset, length, flags = 0)
  * upload(self, poolname, volname, stream, offset, length, flags = 0)
---
 lib/storageAPI.py |   17 +
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/lib/storageAPI.py b/lib/storageAPI.py
index 6c9d286..b0733f8 100644
--- a/lib/storageAPI.py
+++ b/lib/storageAPI.py
@@ -466,3 +466,20 @@ class StorageAPI(object):
 code = e.get_error_code()
 raise exception.LibvirtAPI(message, code)
 
+def download(self, poolname, volname, stream, offset, length, flags = 0):
+try:
+volobj = self.get_volume_obj(poolname, volname)
+return volobj.download(stream, offset, length, flags)
+except libvirt.libvirtError, e:
+message = e.get_error_message()
+code = e.get_error_code()
+raise exception.LibvirtAPI(message, code)
+
+def upload(self, poolname, volname, stream, offset, length, flags = 0):
+try:
+volobj = self.get_volume_obj(poolname, volname)
+return volobj.upload(stream, offset, length, flags)
+except libvirt.libvirtError, e:
+message = e.get_error_message()
+code = e.get_error_code()
+raise exception.LibvirtAPI(message, code)
-- 
1.7.1

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

Re: [libvirt] RFC: setting mac address on network devices being assigned to a guest via PCI passthrough (hostdev)

2012-01-31 Thread Laine Stump


On 01/30/2012 08:16 PM, Roopa Prabhu wrote:

Laine, I haven't gone through your whole email yet. Was just curious about
one quick thing,

For sriov VF's, are we expecting that a net device (eth interface) be
present on the host if its being used as a hostdev ?.



Either should be possible. If the VF is bound to a net dev, it can be 
specified either with dev='ethxx' or using its pci address, and will be 
unbound before assigning to the guest. If it's not bound to a net dev, 
then a pci address must be used to describe it in the config.




  If yes, then libvirt
will need to do an unbind of the driver on the VF before assigning it to the
VM. Which today it does not do (correct me if I am wrong).



That may have been the case in the past, but with libvirt-0.9.9 (the 
first version that I've tested with sriov and PCI passthrough - I'm a 
newbie to both), the net driver is unbound prior to assigning to the 
guest, and when the the device is detached from the guest, the net 
driver is once again bound to the device.




Which is still
ok. Just wanted to call that out.
Plus ideally it would be nice to not have an expectation that a vf netdevice
be present on the host. Because for sriov vf's, it would mean that the vf
driver has to be loaded on the host. Which is really not required for vfs
because mac and port profile can be set via the pf with the vf index as
argument.


Right. For sriov VFs, I intend that the MAC address will always be set 
via the PF. I hadn't previously thought through the details for 
non-sriov devices, but your event sequence list below made me realize 
that, at least for non-sriov, the MAC address and virtual port setup 
will need to be done *before* unbinding the driver (my intent, for sriov 
at least, was that this would happen *after* unbinding the driver). I 
guess that will take some experimentation.


Anyway, at the moment I'm slogging around in the data structures.



Basically,
For non-sriov network devices on host,
 - find netdevice
 - set mac on netdevice
 - If required set port profile on the netdevice
 - unbind netdevice driver
 - assign net pci device to guest


For sriov vf network devices on host,
 - find pf netdevice
 - set mac for vf via the pf netdevice
 - If required set port profile on the vf via the pf netdevice
 - unbind vf driver if its loaded on the vf /* not mandatory */
 - assign vf pci device to guest

Thanks!
-Roopa




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

Re: [libvirt] [PATCH] qemu: never send SIGKILL to qemu process unless specifically requested

2012-01-31 Thread Laine Stump


On 01/31/2012 03:30 AM, Michal Privoznik wrote:

On 30.01.2012 21:30, Laine Stump wrote:

On 01/30/2012 06:02 AM, Daniel P. Berrange wrote:

On Fri, Jan 27, 2012 at 01:35:35PM -0500, Laine Stump wrote:

When libvirt is shutting down the qemu process, it first sends
SIGTERM, then waits for 1.6 seconds and, if it sees the process still
there, sends a SIGKILL.

There have been reports that this behavior can lead to data loss
because the guest running in qemu doesn't have time to flush it's disk
cache buffers before it's unceremoniously whacked.

One suggestion on how to solve that problem was to remove SIGKILL from
the normal virDomainDestroyFlags, but still provide the ability to
kill qemu with SIGKILL by using a new flag to virDomainDestroyFlags.
This patch is a quick attempt at that in order to start a
conversation on the topic.

So what are your opinions? Is this the right way to solve the problem?

No, we can't change the default semantics of virDomainDestroy in
this case. Applications expect that we do absolutely everything
possible to kill of the guest.


But not that we get it all done within 1.6 seconds, right? :-)


This is particularly important for
cluster fencing usage. If we only use SIGTERM, then we're introducing
unacceptable risk to apps relying on this.

We could do the opposite though - have a flag to do a gracefully
destroy, leaving the default as un-graceful.

virDomainShutdown ends up calling qemuProcessKill() too. So, I guess we
need to add a flag there too.

In the meantime, shouldn't we at least wait longer before resorting to
SIGKILL? (especially since it appears the current timeout is quite often
too short). (If we don't at least do that, what we're saying is the
behavior of virDomainShutdown / virDomainDestroy is to lose your data
unless you're lucky. If you don't want this behavior, you need to use
virDomainXXXFlags, and specify the VIR_DOMAIN_DONT_TRASH_MY_DATA flag
:-P).

I should probably hop into this as I've tried to solve this issue
earlier but got sidetracked and then forgot about it.

Increasing the delay could be temporary workaround, but we should keep
in mind that if we change the delay to X (units of time), I bet in some
cases it will take qemu X+1 units to flush caches.

Therefore I lean to the flag DONT_SENT_SIGKILL and leave the default to
what it is now.


Sure, even if you increase the timeout, there will still be cases where 
it fails. But we can't magically cause every piece of management 
software to switch to using the DONT_SEND_SIGKILL flag overnight, and in 
the meantime increasing the timeout will lead to fewer failures, and 
won't cause a longer delay than currently *unless it was going to cause 
a failure anyway* (since the loop exits as soon as it detects the 
process has exited).


So, I don't see a downside to increasing the timeout (in addition to 
adding the flag, *not* instead of it). The current timeout is arbitrary, 
so why not make it a bit larger arbitrary amount?




However, as a qemu developer pointed out (Luiz?), even
with -no-shutdown qemu will terminate itself after receiving SIGINT and
flushing own caches. So this might be the right way to solve this.


Please define this more specifically :-) (maybe in patch form?)

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

Re: [libvirt] [PATCH] qemu: never send SIGKILL to qemu process unless specifically requested

On Mon, Jan 30, 2012 at 03:30:18PM -0500, Laine Stump wrote:
 On 01/30/2012 06:02 AM, Daniel P. Berrange wrote:
 On Fri, Jan 27, 2012 at 01:35:35PM -0500, Laine Stump wrote:
 When libvirt is shutting down the qemu process, it first sends
 SIGTERM, then waits for 1.6 seconds and, if it sees the process still
 there, sends a SIGKILL.
 
 There have been reports that this behavior can lead to data loss
 because the guest running in qemu doesn't have time to flush it's disk
 cache buffers before it's unceremoniously whacked.
 
 One suggestion on how to solve that problem was to remove SIGKILL from
 the normal virDomainDestroyFlags, but still provide the ability to
 kill qemu with SIGKILL by using a new flag to virDomainDestroyFlags.
 This patch is a quick attempt at that in order to start a
 conversation on the topic.
 
 So what are your opinions? Is this the right way to solve the problem?
 No, we can't change the default semantics of virDomainDestroy in
 this case. Applications expect that we do absolutely everything
 possible to kill of the guest. This is particularly important for
 cluster fencing usage. If we only use SIGTERM, then we're introducing
 unacceptable risk to apps relying on this.
 
 We could do the opposite though - have a flag to do a gracefully
 destroy, leaving the default as un-graceful.
 
 virDomainShutdown ends up calling qemuProcessKill() too. So, I guess
 we need to add a flag there too.

 In the meantime, shouldn't we at least wait longer before resorting
 to SIGKILL? (especially since it appears the current timeout is
 quite often too short). (If we don't at least do that, what we're
 saying is the behavior of virDomainShutdown / virDomainDestroy is
 to lose your data unless you're lucky. If you don't want this
 behavior, you need to use virDomainXXXFlags, and specify the
 VIR_DOMAIN_DONT_TRASH_MY_DATA flag :-P).

If you add a flag to trigger a graceful kill(SIGINT) only, then
we don't need to change the timeout. The application now has the
ability to wait as long as they like, before issuing another
virDomainDestroy()


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [test-API][PATCH 2/2] Add and update functions in streamAPI

2012-01-31 Thread Alex Jia


On 01/31/2012 04:46 PM, Wayne Sun wrote:

   * only accpet stream object as parameter in __init__

   * remove newStream() from each function
 stream object should be pass in as parameter, not create new at
 each function.
 function also need flags parameter.

   * Add 5 new functions
 screenshot(self, domain, screen, flags = 0)
 download(self, vol, offset, length, flags = 0)
 upload(self, vol, offset, length, flags = 0)
 recvAll(self, handler, opaque)
 sendAll(self, handler, opaque)

 for recvAll and sendAll, handler is a user defined function which
 write/read data to/from file.
---
  lib/streamAPI.py |   84 +
  1 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/lib/streamAPI.py b/lib/streamAPI.py
index bc7d217..a1b2d0d 100644
--- a/lib/streamAPI.py
+++ b/lib/streamAPI.py
@@ -38,76 +38,108 @@ append_path(result.group(0))
  import exception

  class StreamAPI(object):
-def __init__(self, connection):
-self.conn = connection
+def __init__(self, stream):
+self.stream_obj = stream

Need a 'stream' instance to initialize class StreamAPI? how to do this?
users must to do 'stream = con.newStream(0)' then StreamAPI(stream),
right? maybe, we may do this like this:

class StreamAPI(object):
def __init__(self, connection, flag = 0):
self.conn = connection
self.stream_obj = newStream(flag)


Alex


-def abort(self, flag = 0):
+def abort(self):
  try:
-stream_obj = newStream(flag)
-return stream_obj.abort()
+return self.stream_obj.abort()
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def connect(self, flag = 0):
+def connect(self):
  try:
-stream_obj = newStream(flag)
-return stream_obj.connect()
+return self.stream_obj.connect()
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def finish(self, flag = 0):
+def finish(self):
  try:
-stream_obj = newStream(flag)
-return stream_obj.finish()
+return self.stream_obj.finish()
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def recv(self, flag = 0, data, nbytes):
+def recv(self, nbytes):
  try:
-stream_obj = newStream(flag)
-return stream_obj.recv(data, nbytes)
+return self.stream_obj.recv(nbytes)
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def send(self, flag = 0, data, nbytes):
+def send(self, data):
  try:
-stream_obj = newStream(flag)
-return stream_obj.send(data, nbytes)
+return self.stream_obj.send(data)
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def eventAddCallback(self, flag = 0, cb, opaque):
+def eventAddCallback(self, cb, opaque):
  try:
-stream_obj = newStream(flag)
-return stream_obj.eventAddCallback(cb, opaque)
+return self.stream_obj.eventAddCallback(cb, opaque)
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def eventRemoveCallback(self, flag = 0):
+def eventRemoveCallback(self):
  try:
-stream_obj = newStream(flag)
-return stream_obj.eventRemoveCallback()
+return self.stream_obj.eventRemoveCallback()
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def eventUpdateCallback(self, flag = 0, events)
+def eventUpdateCallback(self, events):
  try:
-stream_obj = newStream(flag)
-return stream_obj.eventUpdateCallback(events)
+return self.stream_obj.eventUpdateCallback(events)
+except libvirt.libvirtError, e:
+message = e.get_error_message()
+code = e.get_error_code()
+raise exception.LibvirtAPI(message, code)
+
+def screenshot(self, domain, screen, flags = 0):
+try:
+return self.stream_obj.screenshot(domain, screen, flags)
+except libvirt.libvirtError, e:
+message =

Re: [libvirt] [PATCH V4 3/5] add nodeGetCPUmap.

2012-01-31 Thread Hu Tao

On Sat, Jan 28, 2012 at 03:24:04PM +0900, KAMEZAWA Hiroyuki wrote:
 add nodeGetCPUmap() for getting available CPU IDs in a bitmap.
 add virBitmapParseCommaSeparetedFormat() for parsing bitmap in
 comma separeted ascii format. This format of bitmap is used in Linux
 sysfs and cpuset.
 
 * cpuacct's percpu usage information is provided based on
   /sys/devices/system/cpu/present cpu map.
   So, this kind of call is required. This routine itself may be
   useful for other purpose.
 
  libvirt_private.syms |2 +
  nodeinfo.c   |   51 
  nodeinfo.h   |4 +++
  util/bitmap.c|   65 
 +++
  util/bitmap.h|6 +++-
  5 files changed, 127 insertions(+), 1 deletion(-)
 ---
  src/libvirt_private.syms |2 +
  src/nodeinfo.c   |   51 
  src/nodeinfo.h   |4 +++
  src/util/bitmap.c|   65 
 ++
  src/util/bitmap.h|6 +++-
  5 files changed, 127 insertions(+), 1 deletions(-)
 
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 915a43f..fd44322 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -17,6 +17,7 @@ virBitmapFree;
  virBitmapGetBit;
  virBitmapSetBit;
  virBitmapString;
 +virBitmapParseCommaSeparatedFormat;
  
  
  # buf.h
 @@ -800,6 +801,7 @@ nodeGetCellsFreeMemory;
  nodeGetFreeMemory;
  nodeGetInfo;
  nodeGetMemoryStats;
 +nodeGetCPUmap;
  
  
  # nwfilter_conf.h
 diff --git a/src/nodeinfo.c b/src/nodeinfo.c
 index e0b66f7..92bada7 100644
 --- a/src/nodeinfo.c
 +++ b/src/nodeinfo.c
 @@ -47,6 +47,7 @@
  #include count-one-bits.h
  #include intprops.h
  #include virfile.h
 +#include bitmap.h
  
  
  #define VIR_FROM_THIS VIR_FROM_NONE
 @@ -569,6 +570,33 @@ int linuxNodeGetMemoryStats(FILE *meminfo,
  cleanup:
  return ret;
  }
 +
 +/*
 + * Linux maintains cpu bit map. For example, if cpuid=5's flag is not set
 + * and max cpu is 7. The map file shows 0-4,6-7. This functin parse
 + * it and returns bitmap.
 + */
 +static virBitmapPtr linuxParseCPUmap(int *max_cpuid, const char *path)
 +{
 +char str[1024];
 +FILE *fp;
 +virBitmapPtr map = NULL;
 +
 +fp = fopen(path, r);
 +if (!fp) {
 +virReportSystemError(errno,  _(cannot open %s), path);
 +goto cleanup;
 +}
 +if (fgets(str, sizeof(str), fp) == NULL) {
 +virReportSystemError(errno, _(cannot read from %s), path);
 +goto cleanup;
 +}
 +map = virBitmapParseCommaSeparatedFormat(str, max_cpuid);
 +
 +cleanup:
 +VIR_FORCE_FCLOSE(fp);
 +return map;
 +}
  #endif
  
  int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr 
 nodeinfo) {
 @@ -712,6 +740,29 @@ int nodeGetMemoryStats(virConnectPtr conn 
 ATTRIBUTE_UNUSED,
  #endif
  }
  
 +virBitmapPtr nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED,
 +   int *max_id,
 +   const char *mapname)
 +{
 +#ifdef __linux__
 +char *path;
 +virBitmapPtr map;
 +
 +if (virAsprintf(path, CPU_SYS_PATH /%s, mapname)  0) {
 +virReportOOMError();
 +return NULL;
 +}
 +
 +map = linuxParseCPUmap(max_id, path);
 +VIR_FREE(path);
 +return map;
 +#else
 + nodeReportError(VIR_ERR_NO_SUPPORT, %s,
 + _(node cpumap not implemented on this platform));
 + return -1;
 +#endif
 +}
 +
  #if HAVE_NUMACTL
  # if LIBNUMA_API_VERSION = 1
  #  define NUMA_MAX_N_CPUS 4096
 diff --git a/src/nodeinfo.h b/src/nodeinfo.h
 index 4766152..7f26b77 100644
 --- a/src/nodeinfo.h
 +++ b/src/nodeinfo.h
 @@ -26,6 +26,7 @@
  
  # include libvirt/libvirt.h
  # include capabilities.h
 +# include bitmap.h
  
  int nodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo);
  int nodeCapsInitNUMA(virCapsPtr caps);
 @@ -46,4 +47,7 @@ int nodeGetCellsFreeMemory(virConnectPtr conn,
 int maxCells);
  unsigned long long nodeGetFreeMemory(virConnectPtr conn);
  
 +virBitmapPtr nodeGetCPUmap(virConnectPtr conn,
 +   int *max_id,
 +   const char *mapname);
  #endif /* __VIR_NODEINFO_H__*/
 diff --git a/src/util/bitmap.c b/src/util/bitmap.c
 index 8c326c4..c7e1a81 100644
 --- a/src/util/bitmap.c
 +++ b/src/util/bitmap.c
 @@ -33,6 +33,11 @@
  #include bitmap.h
  #include memory.h
  #include buf.h
 +#include c-ctype.h
 +#include util.h
 +#include virterror_internal.h
 +
 +#define VIR_FROM_THIS VIR_FROM_NONE
  
  
  struct _virBitmap {
 @@ -180,3 +185,63 @@ char *virBitmapString(virBitmapPtr bitmap)
  
  return virBufferContentAndReset(buf);
  }
 +
 +/**
 + * virBitmapParseCommaSeparatedFormat:
 + *
 + * When bitmap is printed in ascii format, expecially in Linux,

s/expecially/especially/

 + * comma-separated format is sometimes used. For example, a bitmap 1011 
 is
 + * represetned as 0-4,6-7. This function

Re: [libvirt] [test-API][PATCH 2/2] Add and update functions in streamAPI

2012-01-31 Thread Alex Jia


On 01/31/2012 06:07 PM, Alex Jia wrote:

On 01/31/2012 04:46 PM, Wayne Sun wrote:

   * only accpet stream object as parameter in __init__

   * remove newStream() from each function
 stream object should be pass in as parameter, not create new at
 each function.
 function also need flags parameter.

   * Add 5 new functions
 screenshot(self, domain, screen, flags = 0)
 download(self, vol, offset, length, flags = 0)
 upload(self, vol, offset, length, flags = 0)
 recvAll(self, handler, opaque)
 sendAll(self, handler, opaque)

 for recvAll and sendAll, handler is a user defined function which
 write/read data to/from file.
---
  lib/streamAPI.py |   84 
+

  1 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/lib/streamAPI.py b/lib/streamAPI.py
index bc7d217..a1b2d0d 100644
--- a/lib/streamAPI.py
+++ b/lib/streamAPI.py
@@ -38,76 +38,108 @@ append_path(result.group(0))
  import exception

  class StreamAPI(object):
-def __init__(self, connection):
-self.conn = connection
+def __init__(self, stream):
+self.stream_obj = stream

Need a 'stream' instance to initialize class StreamAPI? how to do this?
users must to do 'stream = con.newStream(0)' then StreamAPI(stream),
right? maybe, we may do this like this:

class StreamAPI(object):
def __init__(self, connection, flag = 0):
self.conn = connection
self.stream_obj = newStream(flag)


It should be 'self.stream_obj = self.conn.newStream(flag)'


Alex


-def abort(self, flag = 0):
+def abort(self):
  try:
-stream_obj = newStream(flag)
-return stream_obj.abort()
+return self.stream_obj.abort()
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def connect(self, flag = 0):
+def connect(self):
  try:
-stream_obj = newStream(flag)
-return stream_obj.connect()
+return self.stream_obj.connect()
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def finish(self, flag = 0):
+def finish(self):
  try:
-stream_obj = newStream(flag)
-return stream_obj.finish()
+return self.stream_obj.finish()
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def recv(self, flag = 0, data, nbytes):
+def recv(self, nbytes):
  try:
-stream_obj = newStream(flag)
-return stream_obj.recv(data, nbytes)
+return self.stream_obj.recv(nbytes)
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def send(self, flag = 0, data, nbytes):
+def send(self, data):
  try:
-stream_obj = newStream(flag)
-return stream_obj.send(data, nbytes)
+return self.stream_obj.send(data)
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def eventAddCallback(self, flag = 0, cb, opaque):
+def eventAddCallback(self, cb, opaque):
  try:
-stream_obj = newStream(flag)
-return stream_obj.eventAddCallback(cb, opaque)
+return self.stream_obj.eventAddCallback(cb, opaque)
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def eventRemoveCallback(self, flag = 0):
+def eventRemoveCallback(self):
  try:
-stream_obj = newStream(flag)
-return stream_obj.eventRemoveCallback()
+return self.stream_obj.eventRemoveCallback()
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)

-def eventUpdateCallback(self, flag = 0, events)
+def eventUpdateCallback(self, events):
  try:
-stream_obj = newStream(flag)
-return stream_obj.eventUpdateCallback(events)
+return self.stream_obj.eventUpdateCallback(events)
+except libvirt.libvirtError, e:
+message = e.get_error_message()
+code = e.get_error_code()
+raise exception.LibvirtAPI(message, code)
+
+def screenshot(self, domain, screen, flags = 0):
+try:
+return

[libvirt] [[libvirt-glib PATCHv2]] API to get/set custom metadata from/to domain config

Based on a patch from Zeeshan Ali (Khattak) zeesha...@gnome.org
---
 libvirt-gconfig/libvirt-gconfig-domain.c  |   60 +
 libvirt-gconfig/libvirt-gconfig-domain.h  |7 +++
 libvirt-gconfig/libvirt-gconfig-helpers-private.h |1 +
 libvirt-gconfig/libvirt-gconfig-helpers.c |   23 -
 libvirt-gconfig/libvirt-gconfig-object-private.h  |3 +
 libvirt-gconfig/libvirt-gconfig-object.c  |   20 +++
 libvirt-gconfig/libvirt-gconfig.sym   |2 +
 7 files changed, 115 insertions(+), 1 deletions(-)

diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c 
b/libvirt-gconfig/libvirt-gconfig-domain.c
index 61af625..606f5a4 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain.c
@@ -449,3 +449,63 @@ GList *gvir_config_domain_get_devices(GVirConfigDomain 
*domain)
 
 return data.devices;
 }
+
+gboolean gvir_config_domain_set_custom_xml(GVirConfigDomain *domain,
+   const gchar *xml,
+   const gchar *ns,
+   const gchar *ns_uri,
+   GError **error)
+{
+GVirConfigObject *metadata;
+GVirConfigObject *custom_xml;
+
+g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), FALSE);
+g_return_val_if_fail(xml != NULL, FALSE);
+g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
+
+metadata = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(domain), 
metadata);
+
+custom_xml = gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_OBJECT, 
NULL, NULL, xml, error);
+if (error != NULL  *error != NULL)
+return FALSE;
+
+gvir_config_object_set_namespace(custom_xml, ns, ns_uri);
+
+gvir_config_object_attach_replace(metadata, custom_xml);
+
+return TRUE;
+}
+
+struct LookupNamespacedNodeData {
+const char *ns_uri;
+xmlNodePtr node;
+};
+
+static gboolean lookup_namespaced_node(xmlNodePtr node, gpointer opaque)
+{
+struct LookupNamespacedNodeData* data = opaque;
+
+if (node-ns == NULL)
+return TRUE;
+
+if (g_strcmp0((char *)node-ns-href, data-ns_uri) == 0) {
+data-node = node;
+return FALSE;
+}
+
+return TRUE;
+}
+
+gchar *gvir_config_domain_get_custom_xml(GVirConfigDomain *domain,
+ const gchar *ns_uri)
+{
+struct LookupNamespacedNodeData data = { NULL, NULL };
+
+g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), NULL);
+g_return_val_if_fail(ns_uri != NULL, NULL);
+
+data.ns_uri = ns_uri;
+gvir_config_object_foreach_child(GVIR_CONFIG_OBJECT(domain), metadata,
+ lookup_namespaced_node, data);
+return gvir_config_xml_node_to_string(data.node);
+}
diff --git a/libvirt-gconfig/libvirt-gconfig-domain.h 
b/libvirt-gconfig/libvirt-gconfig-domain.h
index 6cdaec9..769d2f0 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain.h
+++ b/libvirt-gconfig/libvirt-gconfig-domain.h
@@ -126,6 +126,13 @@ GList *gvir_config_domain_get_devices(GVirConfigDomain 
*domain);
 void gvir_config_domain_set_lifecycle(GVirConfigDomain *domain,
   GVirConfigDomainLifecycleEvent event,
   GVirConfigDomainLifecycleAction action);
+gboolean gvir_config_domain_set_custom_xml(GVirConfigDomain *domain,
+   const gchar *xml,
+   const gchar *ns,
+   const gchar *ns_uri,
+   GError **error);
+gchar *gvir_config_domain_get_custom_xml(GVirConfigDomain *domain,
+ const gchar *ns_uri);
 
 G_END_DECLS
 
diff --git a/libvirt-gconfig/libvirt-gconfig-helpers-private.h 
b/libvirt-gconfig/libvirt-gconfig-helpers-private.h
index 96ec02e..514aeb0 100644
--- a/libvirt-gconfig/libvirt-gconfig-helpers-private.h
+++ b/libvirt-gconfig/libvirt-gconfig-helpers-private.h
@@ -56,6 +56,7 @@ char *gvir_config_xml_get_child_element_content_glib (xmlNode 
   *node,
   const char *child_name);
 xmlChar *gvir_config_xml_get_attribute_content(xmlNodePtr node,
const char *attr_name);
+char *gvir_config_xml_node_to_string(xmlNodePtr node);
 char *gvir_config_xml_get_attribute_content_glib(xmlNodePtr node,
  const char *attr_name);
 const char *gvir_config_genum_get_nick (GType enum_type, gint value);
diff --git a/libvirt-gconfig/libvirt-gconfig-helpers.c 
b/libvirt-gconfig/libvirt-gconfig-helpers.c
index fecf3eb..a324d0e 100644
--- a/libvirt-gconfig/libvirt-gconfig-helpers.c
+++ b/libvirt-gconfig/libvirt-gconfig-helpers.c
@@ -148,7 +148,8 @@ gvir_config_xml_parse(const char *xml, const char 
*root_node, GError

[libvirt] [[libvirt-glib PATCHv2] 2/4] Don't use g_error in xxx_get_name

g_error will trigger an abort of the running process so it's not
desirable to call it in a library. This commit adds a GError **
parameter to all _get_name methods which were calling g_error.
---
 libvirt-gobject/libvirt-gobject-connection.c  |   10 --
 libvirt-gobject/libvirt-gobject-domain-snapshot.c |5 -
 libvirt-gobject/libvirt-gobject-domain.c  |7 +--
 libvirt-gobject/libvirt-gobject-domain.h  |2 +-
 libvirt-gobject/libvirt-gobject-interface.c   |7 +--
 libvirt-gobject/libvirt-gobject-interface.h   |2 +-
 libvirt-gobject/libvirt-gobject-network-filter.c  |8 ++--
 libvirt-gobject/libvirt-gobject-network-filter.h  |2 +-
 libvirt-gobject/libvirt-gobject-network.c |7 +--
 libvirt-gobject/libvirt-gobject-network.h |2 +-
 libvirt-gobject/libvirt-gobject-node-device.c |7 +--
 libvirt-gobject/libvirt-gobject-node-device.h |2 +-
 libvirt-gobject/libvirt-gobject-storage-pool.c|7 +--
 libvirt-gobject/libvirt-gobject-storage-pool.h|2 +-
 14 files changed, 49 insertions(+), 21 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
b/libvirt-gobject/libvirt-gobject-connection.c
index a90581a..c19e411 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -1112,7 +1112,10 @@ GVirDomain 
*gvir_connection_find_domain_by_name(GVirConnection *conn,
 
 while (g_hash_table_iter_next(iter, key, value)) {
 GVirDomain *dom = value;
-const gchar *thisname = gvir_domain_get_name(dom);
+const gchar *thisname = gvir_domain_get_name(dom, NULL);
+
+if (thisname == NULL)
+continue;
 
 if (strcmp(thisname, name) == 0) {
 g_object_ref(dom);
@@ -1143,7 +1146,10 @@ GVirStoragePool 
*gvir_connection_find_storage_pool_by_name(GVirConnection *conn,
 
 while (g_hash_table_iter_next(iter, key, value)) {
 GVirStoragePool *pool = value;
-const gchar *thisname = gvir_storage_pool_get_name(pool);
+const gchar *thisname = gvir_storage_pool_get_name(pool, NULL);
+
+if (thisname == NULL)
+continue;
 
 if (strcmp(thisname, name) == 0) {
 g_object_ref(pool);
diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c 
b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
index 950555a..0560595 100644
--- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c
+++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
@@ -166,7 +166,10 @@ const gchar 
*gvir_domain_snapshot_get_name(GVirDomainSnapshot *snapshot)
 const char *name;
 
 if (!(name = virDomainSnapshotGetName(priv-handle))) {
-g_error(Failed to get domain_snapshot name on %p, priv-handle);
+gvir_set_error(err, GVIR_DOMAIN_SNAPSHOT_ERROR, 0,
+   Failed to get domain_snapshot name on %p,
+   priv-handle);
+return NULL;
 }
 
 return name;
diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index c1a67a5..bfbb80f 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -264,13 +264,16 @@ G_DEFINE_BOXED_TYPE(GVirDomainInfo, gvir_domain_info,
 gvir_domain_info_copy, gvir_domain_info_free)
 
 
-const gchar *gvir_domain_get_name(GVirDomain *dom)
+const gchar *gvir_domain_get_name(GVirDomain *dom, GError **err)
 {
 GVirDomainPrivate *priv = dom-priv;
 const char *name;
 
 if (!(name = virDomainGetName(priv-handle))) {
-g_error(Failed to get domain name on %p, priv-handle);
+gvir_set_error(err, GVIR_DOMAIN_ERROR, 0,
+   Failed to get domain name on %p,
+   priv-handle);
+return NULL;
 }
 
 return name;
diff --git a/libvirt-gobject/libvirt-gobject-domain.h 
b/libvirt-gobject/libvirt-gobject-domain.h
index 20388f2..d912b21 100644
--- a/libvirt-gobject/libvirt-gobject-domain.h
+++ b/libvirt-gobject/libvirt-gobject-domain.h
@@ -102,7 +102,7 @@ GType gvir_domain_get_type(void);
 GType gvir_domain_info_get_type(void);
 GType gvir_domain_handle_get_type(void);
 
-const gchar *gvir_domain_get_name(GVirDomain *dom);
+const gchar *gvir_domain_get_name(GVirDomain *dom, GError **error);
 const gchar *gvir_domain_get_uuid(GVirDomain *dom);
 gint gvir_domain_get_id(GVirDomain *dom,
 GError **err);
diff --git a/libvirt-gobject/libvirt-gobject-interface.c 
b/libvirt-gobject/libvirt-gobject-interface.c
index f7bdde8..095e22a 100644
--- a/libvirt-gobject/libvirt-gobject-interface.c
+++ b/libvirt-gobject/libvirt-gobject-interface.c
@@ -157,13 +157,16 @@ gvir_interface_handle_free(GVirInterfaceHandle *src)
 G_DEFINE_BOXED_TYPE(GVirInterfaceHandle, gvir_interface_handle,
 gvir_interface_handle_copy, gvir_interface_handle_free)
 
-const gchar

[libvirt] [[libvirt-glib PATCHv2] 3/4] Output runtime warning when *_get_name fails

*_get_name failures used to trigger a crash of the application
because of the use of g_error. Now that it's removed, such failures
may go unnoticed and unchecked. Add a runtime warning when a failure
occurs.
---
 libvirt-gobject/libvirt-gobject-domain-snapshot.c |1 +
 libvirt-gobject/libvirt-gobject-domain.c  |1 +
 libvirt-gobject/libvirt-gobject-interface.c   |1 +
 libvirt-gobject/libvirt-gobject-network-filter.c  |1 +
 libvirt-gobject/libvirt-gobject-network.c |1 +
 libvirt-gobject/libvirt-gobject-node-device.c |1 +
 libvirt-gobject/libvirt-gobject-storage-pool.c|1 +
 libvirt-gobject/libvirt-gobject-storage-vol.c |2 ++
 8 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c 
b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
index 0560595..3784b60 100644
--- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c
+++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
@@ -169,6 +169,7 @@ const gchar 
*gvir_domain_snapshot_get_name(GVirDomainSnapshot *snapshot)
 gvir_set_error(err, GVIR_DOMAIN_SNAPSHOT_ERROR, 0,
Failed to get domain_snapshot name on %p,
priv-handle);
+g_warn_if_reached();
 return NULL;
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index bfbb80f..0f64833 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -273,6 +273,7 @@ const gchar *gvir_domain_get_name(GVirDomain *dom, GError 
**err)
 gvir_set_error(err, GVIR_DOMAIN_ERROR, 0,
Failed to get domain name on %p,
priv-handle);
+g_warn_if_reached();
 return NULL;
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-interface.c 
b/libvirt-gobject/libvirt-gobject-interface.c
index 095e22a..83d4136 100644
--- a/libvirt-gobject/libvirt-gobject-interface.c
+++ b/libvirt-gobject/libvirt-gobject-interface.c
@@ -166,6 +166,7 @@ const gchar *gvir_interface_get_name(GVirInterface *iface, 
GError **err)
 gvir_set_error(err, GVIR_INTERFACE_ERROR, 0,
Failed to get interface name on %p,
priv-handle);
+g_warn_if_reached();
 return NULL;
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-network-filter.c 
b/libvirt-gobject/libvirt-gobject-network-filter.c
index 473ef5d..efefdf4 100644
--- a/libvirt-gobject/libvirt-gobject-network-filter.c
+++ b/libvirt-gobject/libvirt-gobject-network-filter.c
@@ -183,6 +183,7 @@ const gchar *gvir_network_filter_get_name(GVirNetworkFilter 
*filter,
 gvir_set_error(err, GVIR_NETWORK_FILTER_ERROR, 0,
Failed to get network_filter name on %p,
priv-handle);
+g_warn_if_reached();
 return NULL;
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-network.c 
b/libvirt-gobject/libvirt-gobject-network.c
index 143e80b..43c1a52 100644
--- a/libvirt-gobject/libvirt-gobject-network.c
+++ b/libvirt-gobject/libvirt-gobject-network.c
@@ -180,6 +180,7 @@ const gchar *gvir_network_get_name(GVirNetwork *network, 
GError **err)
 gvir_set_error(err, GVIR_NETWORK_ERROR, 0,
Failed to get network name on %p,
priv-handle);
+g_warn_if_reached();
 return NULL;
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-node-device.c 
b/libvirt-gobject/libvirt-gobject-node-device.c
index 55f7098..e34d37b 100644
--- a/libvirt-gobject/libvirt-gobject-node-device.c
+++ b/libvirt-gobject/libvirt-gobject-node-device.c
@@ -166,6 +166,7 @@ const gchar *gvir_node_device_get_name(GVirNodeDevice 
*device, GError **err)
 gvir_set_error(err, GVIR_NODE_DEVICE_ERROR, 0,
Failed to get node_device name on %p,
priv-handle);
+g_warn_if_reached();
 return NULL;
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c 
b/libvirt-gobject/libvirt-gobject-storage-pool.c
index d2ee6d6..ef8617f 100644
--- a/libvirt-gobject/libvirt-gobject-storage-pool.c
+++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
@@ -212,6 +212,7 @@ const gchar *gvir_storage_pool_get_name(GVirStoragePool 
*pool, GError **err)
 gvir_set_error(err, GVIR_STORAGE_POOL_ERROR, 0,
Failed to get storage_pool name on %p,
priv-handle);
+g_warn_if_reached();
 return NULL;
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c 
b/libvirt-gobject/libvirt-gobject-storage-vol.c
index c427e42..c27cfb7 100644
--- a/libvirt-gobject/libvirt-gobject-storage-vol.c
+++ b/libvirt-gobject/libvirt-gobject-storage-vol.c
@@ -181,6 +181,7 @@ const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol, 
GError **error)
 gvir_set_error(error, GVIR_STORAGE_VOL_ERROR, 0,

[libvirt] [[libvirt-glib PATCHv2] 1/4] Add proper error reporting to GVirStorageVol getters

gvir_storage_vol_get_name and gvir_storage_vol_get_path currently
call g_error when an error occurs. Since g_error trigger a coredump,
calling it in a library is harmful. Replace this with proper GError
error reporting.
---
 libvirt-gobject/libvirt-gobject-storage-pool.c |   10 +++---
 libvirt-gobject/libvirt-gobject-storage-vol.c  |   14 ++
 libvirt-gobject/libvirt-gobject-storage-vol.h  |4 ++--
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c 
b/libvirt-gobject/libvirt-gobject-storage-pool.c
index bf25641..a88699e 100644
--- a/libvirt-gobject/libvirt-gobject-storage-pool.c
+++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
@@ -528,6 +528,7 @@ GVirStorageVol *gvir_storage_pool_create_volume
 const gchar *xml;
 virStorageVolPtr handle;
 GVirStoragePoolPrivate *priv = pool-priv;
+const char *name;
 
 xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf));
 
@@ -545,11 +546,14 @@ GVirStorageVol *gvir_storage_pool_create_volume
 volume = GVIR_STORAGE_VOL(g_object_new(GVIR_TYPE_STORAGE_VOL,
handle, handle,
NULL));
+name = gvir_storage_vol_get_name(volume, err);
+if (name == NULL) {
+g_object_unref(G_OBJECT(volume));
+return NULL;
+}
 
 g_mutex_lock(priv-lock);
-g_hash_table_insert(priv-volumes,
-g_strdup(gvir_storage_vol_get_name(volume)),
-volume);
+g_hash_table_insert(priv-volumes, g_strdup(name), volume);
 g_mutex_unlock(priv-lock);
 
 return g_object_ref(volume);
diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c 
b/libvirt-gobject/libvirt-gobject-storage-vol.c
index 5b18877..c427e42 100644
--- a/libvirt-gobject/libvirt-gobject-storage-vol.c
+++ b/libvirt-gobject/libvirt-gobject-storage-vol.c
@@ -172,25 +172,31 @@ gvir_storage_vol_info_free(GVirStorageVolInfo *info)
 G_DEFINE_BOXED_TYPE(GVirStorageVolInfo, gvir_storage_vol_info,
 gvir_storage_vol_info_copy, gvir_storage_vol_info_free)
 
-const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol)
+const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol, GError **error)
 {
 GVirStorageVolPrivate *priv = vol-priv;
 const char *name;
 
 if (!(name = virStorageVolGetName(priv-handle))) {
-g_error(Failed to get storage_vol name on %p, priv-handle);
+gvir_set_error(error, GVIR_STORAGE_VOL_ERROR, 0,
+   Failed to get storage_vol name on %p,
+   priv-handle);
+return NULL;
 }
 
 return name;
 }
 
-const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol)
+const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol, GError **error)
 {
 GVirStorageVolPrivate *priv = vol-priv;
 const char *path;
 
 if (!(path = virStorageVolGetPath(priv-handle))) {
-g_error(Failed to get storage_vol path on %p, priv-handle);
+gvir_set_error(error, GVIR_STORAGE_VOL_ERROR, 0,
+   Failed to get storage_vol path on %p,
+   priv-handle);
+return NULL;
 }
 
 return path;
diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.h 
b/libvirt-gobject/libvirt-gobject-storage-vol.h
index db63865..fd2be80 100644
--- a/libvirt-gobject/libvirt-gobject-storage-vol.h
+++ b/libvirt-gobject/libvirt-gobject-storage-vol.h
@@ -77,8 +77,8 @@ GType gvir_storage_vol_get_type(void);
 GType gvir_storage_vol_info_get_type(void);
 GType gvir_storage_vol_handle_get_type(void);
 
-const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol);
-const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol);
+const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol, GError **error);
+const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol, GError **error);
 
 gboolean gvir_storage_vol_delete(GVirStorageVol *vol,
  guint flags,
-- 
1.7.7.6

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

[libvirt] [[libvirt-glib PATCHv2] 4/4] Replace g_error with g_warning in constructors

g_error generates a fatal error message, meaning it will abort
the currently running process. It's nicer to use g_warning here.
---
 libvirt-gobject/libvirt-gobject-domain.c |4 +++-
 libvirt-gobject/libvirt-gobject-network-filter.c |4 +++-
 libvirt-gobject/libvirt-gobject-network.c|4 +++-
 libvirt-gobject/libvirt-gobject-secret.c |4 +++-
 libvirt-gobject/libvirt-gobject-storage-pool.c   |4 +++-
 5 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index 0f64833..e4d480a 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -134,7 +134,9 @@ static void gvir_domain_constructed(GObject *object)
 
 /* xxx we may want to turn this into an initable */
 if (virDomainGetUUIDString(priv-handle, priv-uuid)  0) {
-g_error(Failed to get domain UUID on %p, priv-handle);
+virErrorPtr verr = virGetLastError();
+g_warning(Failed to get domain UUID on %p: %s,
+  priv-handle, verr-message);
 }
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-network-filter.c 
b/libvirt-gobject/libvirt-gobject-network-filter.c
index efefdf4..eb9cc82 100644
--- a/libvirt-gobject/libvirt-gobject-network-filter.c
+++ b/libvirt-gobject/libvirt-gobject-network-filter.c
@@ -119,7 +119,9 @@ static void gvir_network_filter_constructed(GObject *object)
 
 /* xxx we may want to turn this into an initable */
 if (virNWFilterGetUUIDString(priv-handle, priv-uuid)  0) {
-g_error(Failed to get network filter UUID on %p, priv-handle);
+virErrorPtr verr = virGetLastError();
+g_warning(Failed to get network filter UUID on %p: %s,
+  priv-handle, verr-message);
 }
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-network.c 
b/libvirt-gobject/libvirt-gobject-network.c
index 43c1a52..dcd2300 100644
--- a/libvirt-gobject/libvirt-gobject-network.c
+++ b/libvirt-gobject/libvirt-gobject-network.c
@@ -118,7 +118,9 @@ static void gvir_network_constructed(GObject *object)
 
 /* xxx we may want to turn this into an initable */
 if (virNetworkGetUUIDString(priv-handle, priv-uuid)  0) {
-g_error(Failed to get network UUID on %p, priv-handle);
+virErrorPtr verr = virGetLastError();
+g_warning(Failed to get network UUID on %p: %s,
+  priv-handle, verr-message);
 }
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-secret.c 
b/libvirt-gobject/libvirt-gobject-secret.c
index bc5ee3b..cd819a5 100644
--- a/libvirt-gobject/libvirt-gobject-secret.c
+++ b/libvirt-gobject/libvirt-gobject-secret.c
@@ -119,7 +119,9 @@ static void gvir_secret_constructed(GObject *object)
 
 /* xxx we may want to turn this into an initable */
 if (virSecretGetUUIDString(priv-handle, priv-uuid)  0) {
-g_error(Failed to get secret UUID on %p, priv-handle);
+virErrorPtr verr = virGetLastError();
+g_warning(Failed to get secret UUID on %p: %s,
+  priv-handle, verr-message);
 }
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c 
b/libvirt-gobject/libvirt-gobject-storage-pool.c
index ef8617f..ffb8b26 100644
--- a/libvirt-gobject/libvirt-gobject-storage-pool.c
+++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
@@ -129,7 +129,9 @@ static void gvir_storage_pool_constructed(GObject *object)
 
 /* xxx we may want to turn this into an initable */
 if (virStoragePoolGetUUIDString(priv-handle, priv-uuid)  0) {
-g_error(Failed to get storage pool UUID on %p, priv-handle);
+virErrorPtr verr = virGetLastError();
+g_warning(Failed to get storage pool UUID on %p: %s,
+  priv-handle, verr-message);
 }
 }
 
-- 
1.7.7.6

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

[libvirt] Qemu text monitor

2012-01-31 Thread Shradha Shah

Hello All,

I am currently working with libvirt on RHEL6.2. There are a few points I have 
noticed regarding the QEMU monitor.

When I hotplug a device into the guest, the Qemu text monitor receives a 
device_add command and adds the device to its current devices list (I found 
this list via the qemu-monitor-command info pci). When I hot-unplug the 
device from the guest, the Qemu text monitor receives the device_del command 
but does not remove the device from its current devices list 
(qemu-monitor-command info pci still shows the device).

The result of this is that the next hotplug of the device fails giving an error 
Duplicate id.

I was wondering if any of you have hit this issue before or know about any 
qemu-monitor bugs I may not be aware of?

Many Thanks in advance,

Regards,
Shradha Shah

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

Re: [libvirt] g_error considered harmful

On Mon, Jan 30, 2012 at 07:22:54PM +0100, Marc-André Lureau wrote:
 Hi
 
 In general I agree with the patch series dropping g_error() in favour
 of normal GError reporting, so programs can cope with errors.
 
 However, it removes the forced logging and it's too easy for the
 caller to ignore them, making it hard to track down when something
 goes wrong.

I added a patch to the series to add a g_warn_if_reached when this is
triggered

 
 I think this is even more relevant, because libvirt-glib is logging
 *tons* of normal/useless runtime messages

To be honest I didn't really notice that... What kind of useless messages
are you getting ?

Christophe


pgpk3E5TsjUSz.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Build issue with libvirt 0.9.8

On Fri, Dec 09, 2011 at 02:18:02PM +0100, Christophe Fergeau wrote:
 Hi,
 
 On Fri, Dec 09, 2011 at 11:18:48AM +, Daniel P. Berrange wrote:
  
  There is something odd about the way it is being built:
 
  The combination of these 3 sets of messages, makes me think you have
  confused libtool into linking to the old binary
  
  What arguments are you passing to 'configure' and what if any variables
  are you passing to 'make install' ?
 
 Yes, jhbuild is being a bit convoluted when building things, it uses
 ./configure --prefix $prefix ..., but then it does a
 make install DESTDIR=$prefix/_jhbuild (or something like this), and
 then it moves the files to the right place in $prefix after cleaning up the
 old version of the library that might be there. I just tried without
 using jhbuild to build libvirt, but just the tarballs, and I can reproduce
 when using make install DESTDIR=... and then cleaning $prefix from libvirt
 libs, and then copying what's in DESTDIR to $prefix. Not sure it's worth
 spending a lot of time on this convoluted scenario :)


I looked some more into it, and I think what happens is that:
- libvirt.so gets installed to $DESTDIR/$libdir
- libvirt-qemu.so gets relinked by libtool during installation, but uses
  libvirt from $libdir for this relink
- $libdir/libvirt.so gets replaced with $DESDIR/$libdir/libvirt.so
- libvirt-qemu.so now has some references to a library that has now
  disappeared

http://www.gnu.org/software/libtool/manual/html_node/Install-mode.html
seems to indicate this scenario cannot be supported correctly :(
For systems where fast installation can not be turned on, relinking may be
needed. In this case, a ‘DESTDIR’ install will fail.

Currently it is not generally possible to install into a temporary staging
area that contains needed third-party libraries which are not yet visible
at their final location. 

(I think linux doesn't have fast installation)

Christophe


pgpuJ0Mx4fx2b.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Qemu text monitor

On 31.01.2012 12:12, Shradha Shah wrote:
 Hello All,
 
 I am currently working with libvirt on RHEL6.2. There are a few points I have 
 noticed regarding the QEMU monitor.
 
 When I hotplug a device into the guest, the Qemu text monitor receives a 
 device_add command and adds the device to its current devices list (I found 
 this list via the qemu-monitor-command info pci). When I hot-unplug the 
 device from the guest, the Qemu text monitor receives the device_del command 
 but does not remove the device from its current devices list 
 (qemu-monitor-command info pci still shows the device).
 
 The result of this is that the next hotplug of the device fails giving an 
 error Duplicate id.
 
 I was wondering if any of you have hit this issue before or know about any 
 qemu-monitor bugs I may not be aware of?
 

Yeah, it's a known issue

https://bugzilla.redhat.com/show_bug.cgi?id=758658

but I don't know if/when it will be fixed.

Michal

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

Re: [libvirt] [RFC Incomplete Patch] Libvirt + Openvswitch

On Fri, Jan 27, 2012 at 02:58:58AM -0800, Dan Wendlandt wrote:
Hello all,

I know of many people who want to spin up VMs using libvirt + kvm/qemu and
attach those VMs to an openvswitch bridge (see: http://www.openvswitch.org).
However, the only way I know of to get this working is a kludge that uses
to tap devices along with interface type=ethernet while running
ovs-vsctl outside of libvirt. Even worse, doing this on RHEL/Fedora seems
to require privilege tweaks (e.g., running qemu as root, not dropping
capabilities), which may not be acceptable for production deployments
(see:
http://fedoraproject.org/wiki/How_to_debug_Virtualization_problems#Errors_using_.3Cinterface_type.3D.27ethernet.27.2F.3E).

So I would like to start taking steps toward better libvirt/openvswitch
integration. My initial step has the fairly limit goal of enabling
kvm/qemu VM NICs to attach to an openvswitch bridge in much the same way VM
NIC can already attached to the linux bridge. For example, specifying:

interface type=openvswitch
source bridge=br0/
mac address=ca:fe:de;ad:be:ef/
/interface

IMHO we should not be introducing a new type for OpenVSwitch. Contrary
to common understanding, type='bridge' is not referring explicitly to
Linux software bridging. Rather it refers to the concept of bridging the
guest to the LAN at the network level, of which Linux software briding
is one possible impl. OpenVSwitch is another possible impl. Other hypervisors
have different impls too of course.

If OpenVSwitch is available in the kernel, is there really any reason
to *not* use it ? ie, could we just have

interface type=bridge
source bridge=br0/
mac address=ca:fe:de;ad:be:ef/
/interface

and if we see that 'br0' is using OpenVSwitch, then libvirt can know
to just do the right thing. That way every application that uses
libvirt today will automatically be able to take advantage of the
benefits OpenVSwitch brings without further work

I also wanted to add a new child element of interface that can be used to
specify some OVS specific configuration. To start, I just want to expose a
single 'interfaceid' value (this parameter is specific to openvswitch).
Extending the previous example:

interface type=openvswitch
source bridge=br0/
mac address=ca:fe:de;ad:be:ef/
openvswitchport interfaceid=interface-xyz/
/interface

We already have a virtualport element under interface that
we use for setting 802.1Qb[gh] parameters. It seems to me that we
can use this existing syntax to cope with the openvswitch port
interfaceid perhaps ?

Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [RFC Incomplete Patch] Libvirt + Openvswitch

On Mon, Jan 30, 2012 at 02:49:03PM -0600, kmestery wrote:
 On Jan 30, 2012, at 2:41 PM, Dan Wendlandt wrote:
  Hi Kyle!  Funny how we keep bumping into each other...  I hope you're
  keeping warm in Minnesota :)
  
  On Fri, Jan 27, 2012 at 11:22 AM, kmestery kmest...@cisco.com wrote:
  Hi Dan:
  
  We at Cisco have been looking at this as well, and in fact were going to 
  propose some things in this area as well. Our proposal (which frankly just 
  builds on top of what you have):
  
  I agree, I think the two proposals are complementary.  Our first goal
  was to enable the basic mode of plugging an interface into an OVS
  bridge that was created outside of libvirt.  This would require
  changes to the interface XML only, and would mirror how libvirt
  already let's one plug into an existing bridge using interface
  type=bridge.
  
 This makes sense.
 
  The second step would be to also allow libvirt to actually create +
  configure the OVS bridges as well.  This I believe would map very
  closely to the XML you and Roopa have suggested.  We would need to put
  some thought into what of the many configuration options on an OVS
  bridge need to be exposed as part of the OVS network XML (e.g., how
  to configure an OpenFlow controller, etc.).  These are definitely
  discussions worth having, but I was hoping to start out with a fairly
  clean initial patch to achieve our first goal.
  
 OK, this makes sense too.
 
  I do like the idea of using the virtual port construct even in the
  initial interface only case.  For example:
  
  interface type='bridge'
   bridge name='br0'
   virtualport type=openvswitch
 parameters interfaceid='xyzzy'/
   /virtualport
  /interface
  
  This would seem to create a nice parallel with the model you proposed
  where virtualport is used with interface type=network.   I'm
  still not sure whether the type=openvswitch should be an attribute
  of the interface, bridge, or virtualport element.  Either way, I
  think the underlying code will be treating OVS + Linux Bridge as two
  different cases, so I believe this is really just a matter of
  consistently of presentation in XML.

Yes, I prefer this design to the initial proposal.

  
 I think fundamentally an Open vSwitch bridge is different from a
 standard Linux, thus the type=openvswitch needs to be a part of
 the bridge for sure. I like adding it to the virtualport element
 above.

NB,  type='bridge' technically refers to the *concept* of bridging
an interface to a LAN, not the implemntation of Linux software
bridging. Thus it shouldn't change for OpenVSwitch which is also
providing bridging to the LAN here.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH v2 3/4] util: extend virExecWithHook()

On Tue, Jan 31, 2012 at 01:51:22PM +0900, Taku Izumi wrote:
 
 This patch extends virExecWithHook() to receive
 capability information.
 
 
 Signed-off-by: Taku Izumi izumi.t...@jp.fujitsu.com
 Signed-off-by: Shota Hirae m11g1...@hibikino.ne.jp
 ---
  src/util/command.c |   16 ++--
  1 file changed, 10 insertions(+), 6 deletions(-)
 
 Index: libvirt/src/util/command.c
 ===
 --- libvirt.orig/src/util/command.c
 +++ libvirt/src/util/command.c
 @@ -393,6 +393,7 @@ prepareStdFd(int fd, int std)
   * @hook optional virExecHook function to call prior to exec
   * @data data to pass to the hook function
   * @pidfile path to use as pidfile for daemonized process (needs DAEMON flag)
 + * @capabilities capabilities to keep
   */
  static int
  virExecWithHook(const char *const*argv,
 @@ -404,7 +405,8 @@ virExecWithHook(const char *const*argv,
  unsigned int flags,
  virExecHook hook,
  void *data,
 -char *pidfile)
 +char *pidfile,
 +unsigned long long capabilities)
  {
  pid_t pid;
  int null = -1, i, openmax;
 @@ -633,9 +635,9 @@ virExecWithHook(const char *const*argv,
  
  /* The steps above may need todo something privileged, so
   * we delay clearing capabilities until the last minute */
 -if ((flags  VIR_EXEC_CLEAR_CAPS) 
 -virClearCapabilities()  0)
 -goto fork_error;
 +if (capabilities || (flags  VIR_EXEC_CLEAR_CAPS))
 +if (virSetCapabilities(capabilities)  0)
 +goto fork_error;
  
  /* Close logging again to ensure no FDs leak to child */
  virLogReset();
 @@ -723,7 +725,8 @@ virExecWithHook(const char *const*argv A
  int flags_unused ATTRIBUTE_UNUSED,
  virExecHook hook ATTRIBUTE_UNUSED,
  void *data ATTRIBUTE_UNUSED,
 -char *pidfile ATTRIBUTE_UNUSED)
 +char *pidfile ATTRIBUTE_UNUSED,
 +unsigned long long capabilities ATTRIBUTE_UNUSED)
  {
  /* XXX: Some day we can implement pieces of virCommand/virExec on
   * top of _spawn() or CreateProcess(), but we can't implement
 @@ -2171,7 +2174,8 @@ virCommandRunAsync(virCommandPtr cmd, pi
cmd-flags,
virCommandHook,
cmd,
 -  cmd-pidfile);
 +  cmd-pidfile,
 +  cmd-capabilities);
  
  VIR_DEBUG(Command result %d, with PID %d,
ret, (int)cmd-pid);

ACK

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH v2 1/4] conf: add rawio attribute to disk element of domain XML

On Tue, Jan 31, 2012 at 01:49:11PM +0900, Taku Izumi wrote:
 @@ -3156,6 +3159,26 @@ virDomainDiskDefParseXML(virCapsPtr caps
  def-snapshot = VIR_DOMAIN_DISK_SNAPSHOT_NO;
  }
  
 +def-rawio = -1; /* unspecified */
 +if (rawio) {
 +if (def-device == VIR_DOMAIN_DISK_DEVICE_LUN) {
 +if (STREQ(rawio, yes)) {
 +def-rawio = 1;
 +} else if (STREQ(rawio, no)) {
 +def-rawio = 0;
 +} else {
 +virDomainReportError(VIR_ERR_INTERNAL_ERROR,
 + _(unknown disk rawio setting '%s'),
 + rawio);
 +goto error;
 +}
 +} else {
 +virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +_(rawio can be used only with 
 device='lun'));

s/INTERNAL_ERROR/XML_ERROR/


ACK with that change

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH v2 2/4] util: add functions to keep capabilities

On Tue, Jan 31, 2012 at 01:50:42PM +0900, Taku Izumi wrote:
 
 This patch introduces virSetCapabilities() function and implements
 virCommandAllowCap() function.
 
 Existing virClearCapabilities() is function to clear all capabilities.
 Instead virSetCapabilities() is function to set arbitrary capabilities.
 
 
 Signed-off-by: Taku Izumi izumi.t...@jp.fujitsu.com
 Signed-off-by: Shota Hirae m11g1...@hibikino.ne.jp
 ---
  src/util/command.c |   43 +--
  src/util/command.h |2 --
  2 files changed, 37 insertions(+), 8 deletions(-)
 
 Index: libvirt/src/util/command.c
 ===
 --- libvirt.orig/src/util/command.c
 +++ libvirt/src/util/command.c
 @@ -103,6 +103,8 @@ struct _virCommand {
  pid_t pid;
  char *pidfile;
  bool reap;
 +
 +unsigned long long capabilities;
  };
  
  /*
 @@ -182,6 +184,33 @@ static int virClearCapabilities(void)
  
  return 0;
  }
 +
 +/**
 + * virSetCapabilities:
 + *  @capabilities - capability flag to set.
 + *  In case of 0, this function is identical to
 + *  virClearCapabilities()
 + *
 + */
 +static int virSetCapabilities(unsigned long long capabilities)
 +{
 +int ret, i;
 +
 +capng_clear(CAPNG_SELECT_BOTH);
 +
 +for (i = 0; i = CAP_LAST_CAP; i++) {
 +if (capabilities  (1ULL  i))
 +capng_update(CAPNG_ADD, CAPNG_BOUNDING_SET, i);
 +}
 +
 +if ((ret = capng_apply(CAPNG_SELECT_BOTH))  0) {
 +virCommandError(VIR_ERR_INTERNAL_ERROR,
 +_(cannot apply process capabilities %d), ret);
 +return -1;
 +}
 +
 +return 0;
 +}
  # else
  static int virClearCapabilities(void)
  {
 @@ -189,6 +218,11 @@ static int virClearCapabilities(void)
  // capabilities);
  return 0;
  }
 +
 +static int virSetCapabilities(unsigned long long capabilities)
 +{
 +return 0;
 +}
  # endif
  
  /**
 @@ -883,26 +917,23 @@ virCommandClearCaps(virCommandPtr cmd)
  cmd-flags |= VIR_EXEC_CLEAR_CAPS;
  }
  
 -#if 0 /* XXX Enable if we have a need for capability management.  */
 -
  /**
   * virCommandAllowCap:
   * @cmd: the command to modify
   * @capability: what to allow
   *
 - * Re-allow a specific capability
 + * Allow specific capabilities
   */
  void
  virCommandAllowCap(virCommandPtr cmd,
 -   int capability ATTRIBUTE_UNUSED)
 +   int capability)
  {
  if (!cmd || cmd-has_error)
  return;
  
 -/* XXX ? */
 +cmd-capabilities |= (1ULL  capability);
  }
  
 -#endif /* 0 */
  
  
  /**
 Index: libvirt/src/util/command.h
 ===
 --- libvirt.orig/src/util/command.h
 +++ libvirt/src/util/command.h
 @@ -60,10 +60,8 @@ void virCommandSetPidFile(virCommandPtr 
  
  void virCommandClearCaps(virCommandPtr cmd);
  
 -# if 0
  void virCommandAllowCap(virCommandPtr cmd,
  int capability);
 -# endif
  
  void virCommandDaemonize(virCommandPtr cmd);

ACK


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH v2 4/4] qemu: make qemu processes to retain rawio capability

On Tue, Jan 31, 2012 at 01:52:27PM +0900, Taku Izumi wrote:
 
 This patch revises qemuProcessStart() function for qemu 
 processes to retain CAP_SYS_RAWIO if needed.
 And in case of that, add taint flag to domain.
 
 Signed-off-by: Taku Izumi izumi.t...@jp.fujitsu.com
 Signed-off-by: Shota Hirae m11g1...@hibikino.ne.jp
 ---
  src/qemu/qemu_domain.c  |3 +++
  src/qemu/qemu_process.c |8 
  2 files changed, 11 insertions(+)
 
 Index: libvirt/src/qemu/qemu_process.c
 ===
 --- libvirt.orig/src/qemu/qemu_process.c
 +++ libvirt/src/qemu/qemu_process.c
 @@ -27,6 +27,7 @@
  #include sys/stat.h
  #include sys/time.h
  #include sys/resource.h
 +#include linux/capability.h
  
  #include qemu_process.h
  #include qemu_domain.h
 @@ -3083,6 +3084,7 @@ int qemuProcessStart(virConnectPtr conn,
  virCommandPtr cmd = NULL;
  struct qemuProcessHookData hookData;
  unsigned long cur_balloon;
 +int i;
  
  hookData.conn = conn;
  hookData.vm = vm;
 @@ -3335,6 +3337,12 @@ int qemuProcessStart(virConnectPtr conn,
  if (driver-clearEmulatorCapabilities)
  virCommandClearCaps(cmd);
  
 +/* in case a certain disk is desirous of CAP_SYS_RAWIO, add this */
 +for (i = 0; i  vm-def-ndisks; i++) {
 +if (vm-def-disks[i]-rawio == 1)
 +virCommandAllowCap(cmd, CAP_SYS_RAWIO);
 +}
 +
  virCommandSetPreExecHook(cmd, qemuProcessHook, hookData);
  
  virCommandSetOutputFD(cmd, logfile);
 Index: libvirt/src/qemu/qemu_domain.c
 ===
 --- libvirt.orig/src/qemu/qemu_domain.c
 +++ libvirt/src/qemu/qemu_domain.c
 @@ -1259,6 +1259,9 @@ void qemuDomainObjCheckDiskTaint(struct 
  if (!disk-driverType 
  driver-allowDiskFormatProbing)
  qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_DISK_PROBING, 
 logFD);
 +
 +if (disk-rawio)
 +qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, 
 logFD);
  }

ACK


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [[libvirt-glib PATCHv2] 1/4] Add proper error reporting to GVirStorageVol getters

On Tue, Jan 31, 2012 at 12:00:20PM +0100, Christophe Fergeau wrote:
 gvir_storage_vol_get_name and gvir_storage_vol_get_path currently
 call g_error when an error occurs. Since g_error trigger a coredump,
 calling it in a library is harmful. Replace this with proper GError
 error reporting.
 ---
  libvirt-gobject/libvirt-gobject-storage-pool.c |   10 +++---
  libvirt-gobject/libvirt-gobject-storage-vol.c  |   14 ++
  libvirt-gobject/libvirt-gobject-storage-vol.h  |4 ++--
  3 files changed, 19 insertions(+), 9 deletions(-)
 
 diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c 
 b/libvirt-gobject/libvirt-gobject-storage-pool.c
 index bf25641..a88699e 100644
 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c
 +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
 @@ -528,6 +528,7 @@ GVirStorageVol *gvir_storage_pool_create_volume
  const gchar *xml;
  virStorageVolPtr handle;
  GVirStoragePoolPrivate *priv = pool-priv;
 +const char *name;
  
  xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf));
  
 @@ -545,11 +546,14 @@ GVirStorageVol *gvir_storage_pool_create_volume
  volume = GVIR_STORAGE_VOL(g_object_new(GVIR_TYPE_STORAGE_VOL,
 handle, handle,
 NULL));
 +name = gvir_storage_vol_get_name(volume, err);
 +if (name == NULL) {
 +g_object_unref(G_OBJECT(volume));
 +return NULL;
 +}
  
  g_mutex_lock(priv-lock);
 -g_hash_table_insert(priv-volumes,
 -g_strdup(gvir_storage_vol_get_name(volume)),
 -volume);
 +g_hash_table_insert(priv-volumes, g_strdup(name), volume);
  g_mutex_unlock(priv-lock);
  
  return g_object_ref(volume);
 diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c 
 b/libvirt-gobject/libvirt-gobject-storage-vol.c
 index 5b18877..c427e42 100644
 --- a/libvirt-gobject/libvirt-gobject-storage-vol.c
 +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c
 @@ -172,25 +172,31 @@ gvir_storage_vol_info_free(GVirStorageVolInfo *info)
  G_DEFINE_BOXED_TYPE(GVirStorageVolInfo, gvir_storage_vol_info,
  gvir_storage_vol_info_copy, gvir_storage_vol_info_free)
  
 -const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol)
 +const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol, GError **error)
  {
  GVirStorageVolPrivate *priv = vol-priv;
  const char *name;
  
  if (!(name = virStorageVolGetName(priv-handle))) {
 -g_error(Failed to get storage_vol name on %p, priv-handle);
 +gvir_set_error(error, GVIR_STORAGE_VOL_ERROR, 0,
 +   Failed to get storage_vol name on %p,
 +   priv-handle);
 +return NULL;
  }
  
  return name;
  }
  
 -const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol)
 +const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol, GError **error)
  {
  GVirStorageVolPrivate *priv = vol-priv;
  const char *path;
  
  if (!(path = virStorageVolGetPath(priv-handle))) {
 -g_error(Failed to get storage_vol path on %p, priv-handle);
 +gvir_set_error(error, GVIR_STORAGE_VOL_ERROR, 0,
 +   Failed to get storage_vol path on %p,
 +   priv-handle);
 +return NULL;
  }
  
  return path;
 diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.h 
 b/libvirt-gobject/libvirt-gobject-storage-vol.h
 index db63865..fd2be80 100644
 --- a/libvirt-gobject/libvirt-gobject-storage-vol.h
 +++ b/libvirt-gobject/libvirt-gobject-storage-vol.h
 @@ -77,8 +77,8 @@ GType gvir_storage_vol_get_type(void);
  GType gvir_storage_vol_info_get_type(void);
  GType gvir_storage_vol_handle_get_type(void);
  
 -const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol);
 -const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol);
 +const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol, GError **error);
 +const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol, GError **error);
  
  gboolean gvir_storage_vol_delete(GVirStorageVol *vol,
   guint flags,

ACK for the change to get_path, but we don't need todo this for get_name.

The virStorageVolGetName() method will always succeed, unless passed
an invalid pointer, since it is not involving any RPC. It merely
returns the static string in the virStorageVolPtr struct.


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [[libvirt-glib PATCHv2] 2/4] Don't use g_error in xxx_get_name

On Tue, Jan 31, 2012 at 12:00:21PM +0100, Christophe Fergeau wrote:
 g_error will trigger an abort of the running process so it's not
 desirable to call it in a library. This commit adds a GError **
 parameter to all _get_name methods which were calling g_error.
 ---
  libvirt-gobject/libvirt-gobject-connection.c  |   10 --
  libvirt-gobject/libvirt-gobject-domain-snapshot.c |5 -
  libvirt-gobject/libvirt-gobject-domain.c  |7 +--
  libvirt-gobject/libvirt-gobject-domain.h  |2 +-
  libvirt-gobject/libvirt-gobject-interface.c   |7 +--
  libvirt-gobject/libvirt-gobject-interface.h   |2 +-
  libvirt-gobject/libvirt-gobject-network-filter.c  |8 ++--
  libvirt-gobject/libvirt-gobject-network-filter.h  |2 +-
  libvirt-gobject/libvirt-gobject-network.c |7 +--
  libvirt-gobject/libvirt-gobject-network.h |2 +-
  libvirt-gobject/libvirt-gobject-node-device.c |7 +--
  libvirt-gobject/libvirt-gobject-node-device.h |2 +-
  libvirt-gobject/libvirt-gobject-storage-pool.c|7 +--
  libvirt-gobject/libvirt-gobject-storage-pool.h|2 +-
  14 files changed, 49 insertions(+), 21 deletions(-)

I think that you can just replace g_error with g_warning. The
vir*GetName() methods will never fail unless passed an invalid
pointer, since they just return a const string from the struct,
avoiding any RPC

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [[libvirt-glib PATCHv2] 4/4] Replace g_error with g_warning in constructors

On Tue, Jan 31, 2012 at 12:00:23PM +0100, Christophe Fergeau wrote:
 g_error generates a fatal error message, meaning it will abort
 the currently running process. It's nicer to use g_warning here.
 ---
  libvirt-gobject/libvirt-gobject-domain.c |4 +++-
  libvirt-gobject/libvirt-gobject-network-filter.c |4 +++-
  libvirt-gobject/libvirt-gobject-network.c|4 +++-
  libvirt-gobject/libvirt-gobject-secret.c |4 +++-
  libvirt-gobject/libvirt-gobject-storage-pool.c   |4 +++-
  5 files changed, 15 insertions(+), 5 deletions(-)
 
 diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
 b/libvirt-gobject/libvirt-gobject-domain.c
 index 0f64833..e4d480a 100644
 --- a/libvirt-gobject/libvirt-gobject-domain.c
 +++ b/libvirt-gobject/libvirt-gobject-domain.c
 @@ -134,7 +134,9 @@ static void gvir_domain_constructed(GObject *object)
  
  /* xxx we may want to turn this into an initable */
  if (virDomainGetUUIDString(priv-handle, priv-uuid)  0) {
 -g_error(Failed to get domain UUID on %p, priv-handle);
 +virErrorPtr verr = virGetLastError();
 +g_warning(Failed to get domain UUID on %p: %s,
 +  priv-handle, verr-message);
  }
  }

This is what I think we should be doing for  GetName() too, since
it has the same error scenarios as GetUUID*().

NB, for extra paranoia you shouldn't assume verr is non-NULL.

eg,verr ? verr-message : NULL

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [[libvirt-glib PATCHv2]] API to get/set custom metadata from/to domain config

On Tue, Jan 31, 2012 at 12:02:05PM +0100, Christophe Fergeau wrote:
 Based on a patch from Zeeshan Ali (Khattak) zeesha...@gnome.org
 ---
  libvirt-gconfig/libvirt-gconfig-domain.c  |   60 
 +
  libvirt-gconfig/libvirt-gconfig-domain.h  |7 +++
  libvirt-gconfig/libvirt-gconfig-helpers-private.h |1 +
  libvirt-gconfig/libvirt-gconfig-helpers.c |   23 -
  libvirt-gconfig/libvirt-gconfig-object-private.h  |3 +
  libvirt-gconfig/libvirt-gconfig-object.c  |   20 +++
  libvirt-gconfig/libvirt-gconfig.sym   |2 +
  7 files changed, 115 insertions(+), 1 deletions(-)

ACK, if one question is answered.

 diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c 
 b/libvirt-gconfig/libvirt-gconfig-domain.c
 index 61af625..606f5a4 100644
 --- a/libvirt-gconfig/libvirt-gconfig-domain.c
 +++ b/libvirt-gconfig/libvirt-gconfig-domain.c
 @@ -449,3 +449,63 @@ GList *gvir_config_domain_get_devices(GVirConfigDomain 
 *domain)
  
  return data.devices;
  }
 +
 +gboolean gvir_config_domain_set_custom_xml(GVirConfigDomain *domain,
 +   const gchar *xml,
 +   const gchar *ns,
 +   const gchar *ns_uri,
 +   GError **error)
 +{
 +GVirConfigObject *metadata;
 +GVirConfigObject *custom_xml;
 +
 +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), FALSE);
 +g_return_val_if_fail(xml != NULL, FALSE);

Shouldn't we allow NULL 'xml' here, as a means to remove the existing
element under that namespace ?  Alternatively we can add an explicit
API to delete custom XML nodes.

 +g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
 +
 +metadata = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(domain), 
 metadata);
 +
 +custom_xml = gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_OBJECT, 
 NULL, NULL, xml, error);
 +if (error != NULL  *error != NULL)
 +return FALSE;
 +
 +gvir_config_object_set_namespace(custom_xml, ns, ns_uri);
 +
 +gvir_config_object_attach_replace(metadata, custom_xml);
 +
 +return TRUE;
 +}


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH] tests: virnettlscontexttest needs gnutls-2.6.0

On Mon, Jan 30, 2012 at 12:10:57PM -0700, Eric Blake wrote:
 On 01/30/2012 10:44 AM, Philipp Hahn wrote:
  virnettlscontexttest uses gnutls_x509_crt_set_subject_alt_name() and
  GNUTLS_FSAN_APPEND, which - according to
  http://www.gnu.org/software/gnutls/manual/gnutls.html - are only
  available since 2.6.0.
  
  Since libvirt still works fine with gnutls-1.0.25 from RHEL5, only
  enable the test when the version of GNUTLS is at least 2.6.0.
  
  Signed-off-by: Philipp Hahn h...@univention.de
  ---
   tests/virnettlscontexttest.c |2 +-
   1 files changed, 1 insertions(+), 1 deletions(-)
  
  diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c
  index 51f75b4..6dd42d4 100644
  --- a/tests/virnettlscontexttest.c
  +++ b/tests/virnettlscontexttest.c
  @@ -36,7 +36,7 @@
   #include virsocketaddr.h
   #include gnutls_1_0_compat.h
   
  -#if !defined WIN32  HAVE_LIBTASN1_H  !defined GNUTLS_1_0_COMPAT
  +#if !defined WIN32  HAVE_LIBTASN1_H  !defined GNUTLS_1_0_COMPAT  
  LIBGNUTLS_VERSION_NUMBER  0x020600
 
 Isn't that what GNUTLS_1_0_COMPAT is already doing?  That is,
 GNUTLS_1_0_COMPAT should only be defined if we are already dealing with
 a newer gnutls, and should not be present when using RHEL5 1.0.25.  What
 version of gnutls are you using, where this patch made a difference?

The GNUTLS_1_0_COMPAT code is only dealing with versions = 2.0.0.

This check is making it more strict to also skip any version
in between 2.0.0 and 2.6.0

Perhaps, this allows us to now remove the GNUTLS_1_0_COMPAT, but I
can't remember if the 1.0.x GNUTLS had LIBGNUTLS_VERSION_NUMBER
defined or not.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] Implement virStorageVolResize() for FS backend

On Mon, Jan 30, 2012 at 09:40:11AM +0200, Zeeshan Ali (Khattak) wrote:
 From: Zeeshan Ali (Khattak) zeesha...@gnome.org
 
 Currently only VIR_STORAGE_VOL_RESIZE_DELTA flag is supported.
 ---
  src/storage/storage_backend.h|6 +++
  src/storage/storage_backend_fs.c |   53 +++
  src/storage/storage_driver.c |   88 
 ++
  src/util/storage_file.c  |   16 +++
  src/util/storage_file.h  |2 +
  5 files changed, 165 insertions(+), 0 deletions(-)

ACK, looks ok to me.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [[libvirt-glib PATCHv2]] API to get/set custom metadata from/to domain config

On Tue, Jan 31, 2012 at 12:11:31PM +, Daniel P. Berrange wrote:
 On Tue, Jan 31, 2012 at 12:02:05PM +0100, Christophe Fergeau wrote:
  Based on a patch from Zeeshan Ali (Khattak) zeesha...@gnome.org
  ---
   libvirt-gconfig/libvirt-gconfig-domain.c  |   60 
  +
   libvirt-gconfig/libvirt-gconfig-domain.h  |7 +++
   libvirt-gconfig/libvirt-gconfig-helpers-private.h |1 +
   libvirt-gconfig/libvirt-gconfig-helpers.c |   23 -
   libvirt-gconfig/libvirt-gconfig-object-private.h  |3 +
   libvirt-gconfig/libvirt-gconfig-object.c  |   20 +++
   libvirt-gconfig/libvirt-gconfig.sym   |2 +
   7 files changed, 115 insertions(+), 1 deletions(-)
 
 ACK, if one question is answered.
 
  diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c 
  b/libvirt-gconfig/libvirt-gconfig-domain.c
  index 61af625..606f5a4 100644
  --- a/libvirt-gconfig/libvirt-gconfig-domain.c
  +++ b/libvirt-gconfig/libvirt-gconfig-domain.c
  @@ -449,3 +449,63 @@ GList *gvir_config_domain_get_devices(GVirConfigDomain 
  *domain)
   
   return data.devices;
   }
  +
  +gboolean gvir_config_domain_set_custom_xml(GVirConfigDomain *domain,
  +   const gchar *xml,
  +   const gchar *ns,
  +   const gchar *ns_uri,
  +   GError **error)
  +{
  +GVirConfigObject *metadata;
  +GVirConfigObject *custom_xml;
  +
  +g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), FALSE);
  +g_return_val_if_fail(xml != NULL, FALSE);
 
 Shouldn't we allow NULL 'xml' here, as a means to remove the existing
 element under that namespace ?  Alternatively we can add an explicit
 API to delete custom XML nodes.

Allowing NULL makes sense, I'll look at changing that.

Christophe


pgpGY4dZTCdnj.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] qemu: Clenup qemuDomainSetInterfaceParameters

which contained some useless lines, copied code, NULL
dereference.
---
 src/libvirt.c  |4 ++--
 src/qemu/qemu_driver.c |   44 
 2 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index e702a34..433ae0a 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -7226,8 +7226,8 @@ error:
  *
  * Change a subset or all parameters of interface; currently this
  * includes bandwidth parameters.  The value of @flags should be
- * either VIR_DOMAIN_AFFECT_CURRENT, or a bitwise-or of values from
- * VIR_DOMAIN_AFFECT_LIVE and VIR_DOMAIN_AFFECT_CURRENT, although
+ * either VIR_DOMAIN_AFFECT_CURRENT, or a bitwise-or of values
+ * VIR_DOMAIN_AFFECT_LIVE and VIR_DOMAIN_AFFECT_CONFIG, although
  * hypervisors vary in which flags are supported.
  *
  * This function may require privileged access to the hypervisor.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1b147a9..b1d0b7f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7779,20 +7779,12 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 }
 }
 
-if (VIR_ALLOC(bandwidth)  0) {
+if ((VIR_ALLOC(bandwidth)  0) ||
+(VIR_ALLOC(bandwidth-in)  0) ||
+(VIR_ALLOC(bandwidth-out)  0)) {
 virReportOOMError();
 goto cleanup;
 }
-if (VIR_ALLOC(bandwidth-in)  0) {
-virReportOOMError();
-goto cleanup;
-}
-memset(bandwidth-in, 0, sizeof(*bandwidth-in));
-if (VIR_ALLOC(bandwidth-out)  0) {
-virReportOOMError();
-goto cleanup;
-}
-memset(bandwidth-out, 0, sizeof(*bandwidth-out));
 
 for (i = 0; i  nparams; i++) {
 virTypedParameterPtr param = params[i];
@@ -7817,11 +7809,9 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
  * inbound/outbound to not be set. */
 if (!bandwidth-in-average) {
 VIR_FREE(bandwidth-in);
-bandwidth-in = NULL;
 }
 if (!bandwidth-out-average) {
 VIR_FREE(bandwidth-out);
-bandwidth-out = NULL;
 }
 
 if (flags  VIR_DOMAIN_AFFECT_LIVE) {
@@ -7833,29 +7823,27 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 /* virNetDevBandwidthSet() will clear any previous value of
  * bandwidth parameters, so merge with old bandwidth parameters
  * here to prevent them from being lost. */
-if (bandwidth-in || net-bandwidth-in) {
+if (bandwidth-in ||
+(net-bandwidth  net-bandwidth-in)) {
 if (VIR_ALLOC(newBandwidth-in)  0) {
 virReportOOMError();
 goto cleanup;
 }
-if (bandwidth-in)
-memcpy(newBandwidth-in, bandwidth-in,
-   sizeof(*newBandwidth-in));
-else if (net-bandwidth-in)
-memcpy(newBandwidth-in, net-bandwidth-in,
-   sizeof(*newBandwidth-in));
-}
-if (bandwidth-out || net-bandwidth-out) {
+
+memcpy(newBandwidth-in,
+   bandwidth-in ? bandwidth-in : net-bandwidth-in,
+   sizeof(*newBandwidth-in));
+}
+if (bandwidth-out ||
+(net-bandwidth  net-bandwidth-out)) {
 if (VIR_ALLOC(newBandwidth-out)  0) {
 virReportOOMError();
 goto cleanup;
 }
-if (bandwidth-out)
-memcpy(newBandwidth-out, bandwidth-out,
-   sizeof(*newBandwidth-out));
-else if (net-bandwidth-out)
-memcpy(newBandwidth-out, net-bandwidth-out,
-   sizeof(*newBandwidth-out));
+
+memcpy(newBandwidth-out,
+   bandwidth-out ? bandwidth-out : net-bandwidth-out,
+   sizeof(*newBandwidth-out));
 }
 
 if (virNetDevBandwidthSet(net-ifname, newBandwidth)  0) {
-- 
1.7.3.4

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

Re: [libvirt] [PATCH] tests: virnettlscontexttest needs gnutls-2.6.0

2012-01-31 Thread Philipp Hahn

Hello Eric,

Am Montag 30 Januar 2012 20:10:57 schrieb Eric Blake:
  +#if !defined WIN32  HAVE_LIBTASN1_H  !defined GNUTLS_1_0_COMPAT 
  LIBGNUTLS_VERSION_NUMBER  0x020600

 Isn't that what GNUTLS_1_0_COMPAT is already doing?  That is,
 GNUTLS_1_0_COMPAT should only be defined if we are already dealing with
 a newer gnutls, and should not be present when using RHEL5 1.0.25.  What
 version of gnutls are you using, where this patch made a difference?

I'm trying to build libvirt-git on an older Debian-Lenny box. There running the 
check target fails:

# make virnettlscontexttest
gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -I../gnulib/lib -I../gnulib/lib 
-I../include -I../include -I../src -I../src -I../src/util -I../src/conf 
-Dabs_builddir=\/root/libvirt/tests\ -I/usr/include/libxml2 -Wall 
-W -Wformat-y2k -Wformat-security -Winit-self -Wmissing-include-dirs -Wunused 
-Wunknown-pragmas -Wstrict-aliasing -Wshadow -Wpointer-arith 
-Wbad-function-cast -Wcast-align -Wwrite-strings -Wlogical-op 
-Waggregate-return -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn 
-Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Winline 
-Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization -Wattributes 
-Wcoverage-mismatch -Wmultichar -Wdeprecated-declarations -Wdiv-by-zero 
-Wendif-labels -Wextra -Wformat-contains-nul -Wformat-extra-args 
-Wformat-zero-length -Wformat=2 -Wmultichar -Wnormalized=nfc -Woverflow 
-Wpointer-to-int-cast -Wpragmas -Wno-missing-field-initializers 
-Wno-sign-compare -Wno-format-nonliteral -fstack-protector-all 
--param=ssp-buffer-size=4 -fexceptions -fasynchronous-unwind-tables 
-fdiagnostics-show-option -funit-at-a-time -fipa-pure-const 
-Wno-suggest-attribute=pure -Wno-suggest-attribute=const -g -O2 -MT 
virnettlscontexttest-virnettlscontexttest.o -MD -MP -MF 
.deps/virnettlscontexttest-virnettlscontexttest.Tpo -c -o 
virnettlscontexttest-virnettlscontexttest.o 
`test -f 'virnettlscontexttest.c' || echo './'`virnettlscontexttest.c
virnettlscontexttest.c: In function ‘testTLSGenerateCert’:
virnettlscontexttest.c:207: warning: implicit declaration of function 
‘gnutls_x509_crt_set_subject_alt_name’
virnettlscontexttest.c:207: warning: nested extern declaration of 
‘gnutls_x509_crt_set_subject_alt_name’ [-Wnested-externs]
virnettlscontexttest.c:210: error: ‘GNUTLS_FSAN_APPEND’ undeclared (first use 
in this function)
virnettlscontexttest.c:210: error: (Each undeclared identifier is reported only 
once
virnettlscontexttest.c:210: error: for each function it appears in.)

# egrep '(LIB)?GNUTLS_VERSION_NUMBER|GNUTLS_1_0_COMPAT|GNUTLS_FSAN_APPEND' 
/usr/include/gnutls/*
/usr/include/gnutls/gnutls.h:#define LIBGNUTLS_VERSION_NUMBER 0x020402

# dpkg -S /usr/include/gnutls/gnutls.h
libgnutls-dev: /usr/include/gnutls/gnutls.h

# dpkg -l libgnutls-dev
ii  libgnutls-dev   2.4.2-6.8.200910141301  
the GNU TLS library - development files

If you look at 
http://www.gnu.org/software/gnutls/manual/gnutls.html#gnutls_005fx509_005fcrt_005fset_005fsubject_005falt_005fname
 that function is described 
as available only since 2.6.0.

From a newer Debian-Squeeze box for comparison:
# egrep '(LIB)?GNUTLS_VERSION_NUMBER|GNUTLS_1_0_COMPAT|GNUTLS_FSAN_APPEND' 
/usr/include/gnutls/*
/usr/include/gnutls/compat.h:#define LIBGNUTLS_VERSION_NUMBER 
GNUTLS_VERSION_NUMBER
/usr/include/gnutls/gnutls.h:#define GNUTLS_VERSION_NUMBER 0x020806
/usr/include/gnutls/x509.h:#define GNUTLS_FSAN_APPEND 1

BYtE
Philipp
-- 
Philipp Hahn   Open Source Software Engineer  h...@univention.de
Univention GmbHLinux for Your Businessfon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen fax: +49 421 22 232-99
   http://www.univention.de/


signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [[libvirt-glib PATCHv3] 1/3] Add proper error reporting to gvir_storage_vol_get_path

gvir_storage_vol_get_name currently call g_error when an error occurs.
Since g_error trigger a coredump, calling it in a library is harmful.
Replace this with proper GError error reporting.
---
 libvirt-gobject/libvirt-gobject-storage-vol.c |7 +--
 libvirt-gobject/libvirt-gobject-storage-vol.h |2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c 
b/libvirt-gobject/libvirt-gobject-storage-vol.c
index 5b18877..5e83e1b 100644
--- a/libvirt-gobject/libvirt-gobject-storage-vol.c
+++ b/libvirt-gobject/libvirt-gobject-storage-vol.c
@@ -184,13 +184,16 @@ const gchar *gvir_storage_vol_get_name(GVirStorageVol 
*vol)
 return name;
 }
 
-const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol)
+const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol, GError **error)
 {
 GVirStorageVolPrivate *priv = vol-priv;
 const char *path;
 
 if (!(path = virStorageVolGetPath(priv-handle))) {
-g_error(Failed to get storage_vol path on %p, priv-handle);
+gvir_set_error(error, GVIR_STORAGE_VOL_ERROR, 0,
+   Failed to get storage_vol path on %p,
+   priv-handle);
+return NULL;
 }
 
 return path;
diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.h 
b/libvirt-gobject/libvirt-gobject-storage-vol.h
index db63865..25f683a 100644
--- a/libvirt-gobject/libvirt-gobject-storage-vol.h
+++ b/libvirt-gobject/libvirt-gobject-storage-vol.h
@@ -78,7 +78,7 @@ GType gvir_storage_vol_info_get_type(void);
 GType gvir_storage_vol_handle_get_type(void);
 
 const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol);
-const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol);
+const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol, GError **error);
 
 gboolean gvir_storage_vol_delete(GVirStorageVol *vol,
  guint flags,
-- 
1.7.7.6

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

[libvirt] [[libvirt-glib PATCHv3] 3/3] Replace g_error with g_warning in constructors

g_error generates a fatal error message, meaning it will abort
the currently running process. It's nicer to use g_warning here.
---
 libvirt-gobject/libvirt-gobject-domain.c |9 -
 libvirt-gobject/libvirt-gobject-network-filter.c |9 -
 libvirt-gobject/libvirt-gobject-network.c|9 -
 libvirt-gobject/libvirt-gobject-secret.c |8 +++-
 libvirt-gobject/libvirt-gobject-storage-pool.c   |8 +++-
 5 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index 82da0c3..d9e4c00 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -134,7 +134,14 @@ static void gvir_domain_constructed(GObject *object)
 
 /* xxx we may want to turn this into an initable */
 if (virDomainGetUUIDString(priv-handle, priv-uuid)  0) {
-g_error(Failed to get domain UUID on %p, priv-handle);
+virErrorPtr verr = virGetLastError();
+if (verr) {
+g_warning(Failed to get domain UUID on %p: %s,
+  priv-handle, verr-message);
+} else {
+g_warning(Failed to get domain UUID on %p,
+  priv-handle);
+}
 }
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-network-filter.c 
b/libvirt-gobject/libvirt-gobject-network-filter.c
index b1543a3..5956a3d 100644
--- a/libvirt-gobject/libvirt-gobject-network-filter.c
+++ b/libvirt-gobject/libvirt-gobject-network-filter.c
@@ -119,7 +119,14 @@ static void gvir_network_filter_constructed(GObject 
*object)
 
 /* xxx we may want to turn this into an initable */
 if (virNWFilterGetUUIDString(priv-handle, priv-uuid)  0) {
-g_error(Failed to get network filter UUID on %p, priv-handle);
+virErrorPtr verr = virGetLastError();
+if (verr) {
+g_warning(Failed to get network filter UUID on %p: %s,
+  priv-handle, verr-message);
+} else {
+g_warning(Failed to get network filter UUID on %p,
+  priv-handle);
+}
 }
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-network.c 
b/libvirt-gobject/libvirt-gobject-network.c
index 8cbea07..16cd2f1 100644
--- a/libvirt-gobject/libvirt-gobject-network.c
+++ b/libvirt-gobject/libvirt-gobject-network.c
@@ -118,7 +118,14 @@ static void gvir_network_constructed(GObject *object)
 
 /* xxx we may want to turn this into an initable */
 if (virNetworkGetUUIDString(priv-handle, priv-uuid)  0) {
-g_error(Failed to get network UUID on %p, priv-handle);
+virErrorPtr verr = virGetLastError();
+if (verr) {
+g_warning(Failed to get network UUID on %p: %s,
+  priv-handle, verr-message);
+} else {
+g_warning(Failed to get network UUID on %p,
+  priv-handle);
+}
 }
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-secret.c 
b/libvirt-gobject/libvirt-gobject-secret.c
index bc5ee3b..3c9da86 100644
--- a/libvirt-gobject/libvirt-gobject-secret.c
+++ b/libvirt-gobject/libvirt-gobject-secret.c
@@ -119,7 +119,13 @@ static void gvir_secret_constructed(GObject *object)
 
 /* xxx we may want to turn this into an initable */
 if (virSecretGetUUIDString(priv-handle, priv-uuid)  0) {
-g_error(Failed to get secret UUID on %p, priv-handle);
+virErrorPtr verr = virGetLastError();
+if (verr) {
+g_warning(Failed to get secret UUID on %p: %s,
+  priv-handle, verr-message);
+} else {
+g_warning(Failed to get secret UUID on %p, priv-handle);
+}
 }
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c 
b/libvirt-gobject/libvirt-gobject-storage-pool.c
index aa71029..db496f3 100644
--- a/libvirt-gobject/libvirt-gobject-storage-pool.c
+++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
@@ -129,7 +129,13 @@ static void gvir_storage_pool_constructed(GObject *object)
 
 /* xxx we may want to turn this into an initable */
 if (virStoragePoolGetUUIDString(priv-handle, priv-uuid)  0) {
-g_error(Failed to get storage pool UUID on %p, priv-handle);
+virErrorPtr verr = virGetLastError();
+if (verr) {
+g_warning(Failed to get storage pool UUID on %p: %s,
+  priv-handle, verr-message);
+} else {
+g_warning(Failed to get storage pool UUID on %p, priv-handle);
+}
 }
 }
 
-- 
1.7.7.6

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

[libvirt] [[libvirt-glib PATCHv3] 2/3] Don't use g_error in xxx_get_name

g_error will trigger an abort of the running process so it's not
desirable to call it in a library. This commit uses g_warning when
this occurs and returns NULL. libvirt will only return NULL for these
strings if some kind of memory corruption is going on, or if we are
using an invalid pointer.
---
 libvirt-gobject/libvirt-gobject-connection.c  |6 ++
 libvirt-gobject/libvirt-gobject-domain-snapshot.c |3 ++-
 libvirt-gobject/libvirt-gobject-domain.c  |3 ++-
 libvirt-gobject/libvirt-gobject-interface.c   |3 ++-
 libvirt-gobject/libvirt-gobject-network-filter.c  |3 ++-
 libvirt-gobject/libvirt-gobject-network.c |3 ++-
 libvirt-gobject/libvirt-gobject-node-device.c |3 ++-
 libvirt-gobject/libvirt-gobject-storage-pool.c|   13 +
 libvirt-gobject/libvirt-gobject-storage-vol.c |3 ++-
 9 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
b/libvirt-gobject/libvirt-gobject-connection.c
index a90581a..cb19e9d 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -1114,6 +1114,9 @@ GVirDomain 
*gvir_connection_find_domain_by_name(GVirConnection *conn,
 GVirDomain *dom = value;
 const gchar *thisname = gvir_domain_get_name(dom);
 
+if (thisname == NULL)
+continue;
+
 if (strcmp(thisname, name) == 0) {
 g_object_ref(dom);
 g_mutex_unlock(priv-lock);
@@ -1145,6 +1148,9 @@ GVirStoragePool 
*gvir_connection_find_storage_pool_by_name(GVirConnection *conn,
 GVirStoragePool *pool = value;
 const gchar *thisname = gvir_storage_pool_get_name(pool);
 
+if (thisname == NULL)
+continue;
+
 if (strcmp(thisname, name) == 0) {
 g_object_ref(pool);
 g_mutex_unlock(priv-lock);
diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c 
b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
index 950555a..c386491 100644
--- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c
+++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
@@ -166,7 +166,8 @@ const gchar 
*gvir_domain_snapshot_get_name(GVirDomainSnapshot *snapshot)
 const char *name;
 
 if (!(name = virDomainSnapshotGetName(priv-handle))) {
-g_error(Failed to get domain_snapshot name on %p, priv-handle);
+g_warning(Failed to get domain_snapshot name on %p, priv-handle);
+return NULL;
 }
 
 return name;
diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index c1a67a5..82da0c3 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -270,7 +270,8 @@ const gchar *gvir_domain_get_name(GVirDomain *dom)
 const char *name;
 
 if (!(name = virDomainGetName(priv-handle))) {
-g_error(Failed to get domain name on %p, priv-handle);
+g_warning(Failed to get domain name on %p, priv-handle);
+return NULL;
 }
 
 return name;
diff --git a/libvirt-gobject/libvirt-gobject-interface.c 
b/libvirt-gobject/libvirt-gobject-interface.c
index f7bdde8..892db8b 100644
--- a/libvirt-gobject/libvirt-gobject-interface.c
+++ b/libvirt-gobject/libvirt-gobject-interface.c
@@ -163,7 +163,8 @@ const gchar *gvir_interface_get_name(GVirInterface *iface)
 const char *name;
 
 if (!(name = virInterfaceGetName(priv-handle))) {
-g_error(Failed to get interface name on %p, priv-handle);
+g_warning(Failed to get interface name on %p, priv-handle);
+return NULL;
 }
 
 return name;
diff --git a/libvirt-gobject/libvirt-gobject-network-filter.c 
b/libvirt-gobject/libvirt-gobject-network-filter.c
index fe1a042..b1543a3 100644
--- a/libvirt-gobject/libvirt-gobject-network-filter.c
+++ b/libvirt-gobject/libvirt-gobject-network-filter.c
@@ -179,7 +179,8 @@ const gchar *gvir_network_filter_get_name(GVirNetworkFilter 
*filter)
 const char *name;
 
 if (!(name = virNWFilterGetName(priv-handle))) {
-g_error(Failed to get network_filter name on %p, priv-handle);
+g_warning(Failed to get network_filter name on %p, priv-handle);
+return NULL;
 }
 
 return name;
diff --git a/libvirt-gobject/libvirt-gobject-network.c 
b/libvirt-gobject/libvirt-gobject-network.c
index 75e010d..8cbea07 100644
--- a/libvirt-gobject/libvirt-gobject-network.c
+++ b/libvirt-gobject/libvirt-gobject-network.c
@@ -177,7 +177,8 @@ const gchar *gvir_network_get_name(GVirNetwork *network)
 const char *name;
 
 if (!(name = virNetworkGetName(priv-handle))) {
-g_error(Failed to get network name on %p, priv-handle);
+g_warning(Failed to get network name on %p, priv-handle);
+return NULL;
 }
 
 return name;
diff --git a/libvirt-gobject/libvirt-gobject-node-device.c 
b/libvirt-gobject/libvirt-gobject-node-device.c
index f8d536e..b565052 100644
---

Re: [libvirt] RFC: setting mac address on network devices being assigned to a guest via PCI passthrough (hostdev)

2012-01-31 Thread Roopa Prabhu




On 1/31/12 1:16 AM, Laine Stump la...@laine.org wrote:

 On 01/30/2012 08:16 PM, Roopa Prabhu wrote:
 Laine, I haven't gone through your whole email yet. Was just curious about
 one quick thing,
 
 For sriov VF's, are we expecting that a net device (eth interface) be
 present on the host if its being used as a hostdev ?.
 
 
 Either should be possible. If the VF is bound to a net dev, it can be
 specified either with dev='ethxx' or using its pci address, and will be
 unbound before assigning to the guest. If it's not bound to a net dev,
 then a pci address must be used to describe it in the config.
 
 
   If yes, then libvirt
 will need to do an unbind of the driver on the VF before assigning it to the
 VM. Which today it does not do (correct me if I am wrong).
 
 
 That may have been the case in the past, but with libvirt-0.9.9 (the
 first version that I've tested with sriov and PCI passthrough - I'm a
 newbie to both), the net driver is unbound prior to assigning to the
 guest, and when the the device is detached from the guest, the net
 driver is once again bound to the device.
 
 
 Which is still
 ok. Just wanted to call that out.
 Plus ideally it would be nice to not have an expectation that a vf netdevice
 be present on the host. Because for sriov vf's, it would mean that the vf
 driver has to be loaded on the host. Which is really not required for vfs
 because mac and port profile can be set via the pf with the vf index as
 argument.
 
 Right. For sriov VFs, I intend that the MAC address will always be set
 via the PF. I hadn't previously thought through the details for
 non-sriov devices, but your event sequence list below made me realize
 that, at least for non-sriov, the MAC address and virtual port setup
 will need to be done *before* unbinding the driver (my intent, for sriov
 at least, was that this would happen *after* unbinding the driver). I
 guess that will take some experimentation.
 
 Anyway, at the moment I'm slogging around in the data structures.
 

ok. Thanks Laine.

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

Re: [libvirt] [[libvirt-glib PATCHv3] 1/3] Add proper error reporting to gvir_storage_vol_get_path

On Tue, Jan 31, 2012 at 03:34:09PM +0100, Christophe Fergeau wrote:
 gvir_storage_vol_get_name currently call g_error when an error occurs.
 Since g_error trigger a coredump, calling it in a library is harmful.
 Replace this with proper GError error reporting.
 ---
  libvirt-gobject/libvirt-gobject-storage-vol.c |7 +--
  libvirt-gobject/libvirt-gobject-storage-vol.h |2 +-
  2 files changed, 6 insertions(+), 3 deletions(-)

ACK


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [[libvirt-glib PATCHv3] 2/3] Don't use g_error in xxx_get_name

On Tue, Jan 31, 2012 at 03:34:10PM +0100, Christophe Fergeau wrote:
 g_error will trigger an abort of the running process so it's not
 desirable to call it in a library. This commit uses g_warning when
 this occurs and returns NULL. libvirt will only return NULL for these
 strings if some kind of memory corruption is going on, or if we are
 using an invalid pointer.
 ---
  libvirt-gobject/libvirt-gobject-connection.c  |6 ++
  libvirt-gobject/libvirt-gobject-domain-snapshot.c |3 ++-
  libvirt-gobject/libvirt-gobject-domain.c  |3 ++-
  libvirt-gobject/libvirt-gobject-interface.c   |3 ++-
  libvirt-gobject/libvirt-gobject-network-filter.c  |3 ++-
  libvirt-gobject/libvirt-gobject-network.c |3 ++-
  libvirt-gobject/libvirt-gobject-node-device.c |3 ++-
  libvirt-gobject/libvirt-gobject-storage-pool.c|   13 +
  libvirt-gobject/libvirt-gobject-storage-vol.c |3 ++-
  9 files changed, 29 insertions(+), 11 deletions(-)

ACK


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH] tests: dynamically replace dnsmasq path

2012-01-31 Thread Philipp Hahn

Hello Eric,

Am Montag 30 Januar 2012 20:18:15 schrieb Eric Blake:
  +if (diff  0) {
  +tmp = realloc((void *)*buf, new_len);

 We should not be using realloc in this file, but should be using
 VIR_RESIZE_N or similar.

Okay, makes the code even more readable.

  +memmove(token_start, replacement, replacement_len);

 But this would be more efficient as memcpy, since replacement (better
 not) overlap with token_start.

I know of some projects which forbid memcpy() because of the missing overlap 
handling. But if this is okay with libvirt, I'll use it.
I hope performance will never be a problem with this simple test scenario, 
because then doing one realloc() instead of one for each found would be 
better.

  +if (diff  0) {
  +tmp = realloc((void *)*buf, new_len);

 No need to downsize the allocation - it's okay to leave junk in the tail
 end of the buffer, as long as we have a trailing NUL to stop us in time.

Fine with me for this simple test case, but if that would be generalized and 
be used in other places, this might become a problem. I'll add a comment.

Sincerely
Philipp
-- 
Philipp Hahn   Open Source Software Engineer  h...@univention.de
Univention GmbHLinux for Your Businessfon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen fax: +49 421 22 232-99
   http://www.univention.de/


signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] tests: virnettlscontexttest needs gnutls-2.6.0

2012-01-31 Thread Philipp Hahn

Hello,

On Monday 30 January 2012 18:44:13 Philipp Hahn wrote:
 +#if !defined WIN32  HAVE_LIBTASN1_H  !defined GNUTLS_1_0_COMPAT 
 LIBGNUTLS_VERSION_NUMBER  0x020600

That should probably be = instead of , but I haven't checked the gnutls 
source code history.

Sincerely
Philipp
-- 
Philipp Hahn   Open Source Software Engineer  h...@univention.de
Univention GmbHLinux for Your Businessfon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen fax: +49 421 22 232-99
   http://www.univention.de/


signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] virDomainNetGetActualBridgeName doesn't return the actual bridge

2012-01-31 Thread Cole Robinson

On 01/30/2012 08:15 AM, Hendrik Schwartke wrote:
 Hi,
 
 calling virDomainNetGetActualBridgeName on a bridge with type
 VIR_DOMAIN_NET_TYPE_NETWORK seems to return NULL in any case, because
 iface-data.network.actual is NULL. Is that intented?
 
 What is the best way to determine the bridge the interface is connected to?
 

For the virtual network case, you could do it in a round about way by getting
the network name from the domain interface block, then parsing the bridge
name from the equivalent of virsh net-dumpxml netname

But if we are providing an API like that it should probably work for virtual
networks (but then again I don't know the specific use case that API was meant
to address so it may be conceptually wrong).

- Cole

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

[libvirt] [PATCH] qemu: Don't jump to endjob if no job was even started