[libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Chen Hanxiao
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

When adding a large number of virtio storage devices to
virtual machine by using 'virsh edit' command, there is
a problem:
When we added more than 190 virtio disks by 'virsh edit'
command, we got a feedback as 'error: Unable to encode
message payload'. In virt-mananger, the same defect occurs
with about 180 virtio disks added.

PCI devices are limited by the virtualized system
architecture. Out of the 32 available PCI devices for a
guest, 4 are not removable. This means there are up to
28 free PCI slots available for additional devices per
guest. Each PCI device in a guest can have up to 8 functions.
One slot kept for network interface, we could add at most
216 (27*8) virtio disks.
The length of xml description with multifuction for one virtio
disk is about 290 characters; In virt-manger, an extra tag
'alias name' will come to the xml description, which would add
about 40 more characters.
So for one xml description, 330 characters would be enough.
A brand new virtual machine with one IDE bus disk needs about
1900 characters for xml description.

In src/remote/remote_protocol.x, there is a limitation for XDR
enoding length as REMOTE_STRING_MAX.
According to the analysis above, at least 73180(1900+216*330)
characters are needed for 216 virtio disks. In the view of
variable length in tag 'source file', so I think it would be
better to extend this limitation from 65536 to 8.
---
 src/remote/remote_protocol.x |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 2d57247..2c8dcbb 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -65,7 +65,7 @@
  * This is an arbitrary limit designed to stop the decoder from trying
  * to allocate unbounded amounts of memory when fed with a bad message.
  */
-const REMOTE_STRING_MAX = 65536;
+const REMOTE_STRING_MAX = 8;
 
 /* A long string, which may NOT be NULL. */
 typedef string remote_nonnull_stringREMOTE_STRING_MAX;
-- 
1.7.1

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


Re: [libvirt] [PATCHv5 08/23] blockjob: add qemu capabilities related to block jobs

2012-04-19 Thread Paolo Bonzini
Il 18/04/2012 23:40, Eric Blake ha scritto:
 It's
 not the nicest to commit to an API without an implementation, but we've
 done it before, and I'm quite confident that no matter what qemu finally
 settles on, at least our API decision is robust enough to support that
 qemu implementation.
 
 Paolo, any corrections?

I think so.

Paolo

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


[libvirt] [PATCH] virsh: avoid uninitialized memory usage

2012-04-19 Thread Alex Jia
Detected by valgrind.

* tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.
 
* How to reproduce?
$ qemu-img create /var/lib/libvirt/images/test 1M
$ cat  /tmp/test.xml EOF
domain type='qemu'
  nametest/name
  memory219200/memory
  vcpu1/vcpu
  os
type arch='x86_64'hvm/type
boot dev='hd'/
  /os
  devices
disk type='file' device='disk'
  driver name='qemu' type='raw'/
  source file='/var/lib/libvirt/images/test'/
  target dev='vda' bus='virtio'/
/disk
input type='mouse' bus='ps2'/
graphics type='spice' autoport='yes' listen='0.0.0.0'/
  /devices
/domain
EOF
$ virsh define /tmp/test.xml
$ valgrind -v virsh blockpull test /var/lib/libvirt/images/test --wait

actual result:

==10906== 1 errors in context 1 of 1:
==10906== Syscall param rt_sigaction(act-sa_flags) points to uninitialised 
byte(s)
==10906==at 0x39CF80F5BE: __libc_sigaction (sigaction.c:67)
==10906==by 0x43016C: cmdBlockPull (virsh.c:7638)
==10906==by 0x4150D4: vshCommandRun (virsh.c:18574)
==10906==by 0x425E73: main (virsh.c:20178)
==10906==  Address 0x7fefffae8 is on thread 1's stack


Signed-off-by: Alex Jia a...@redhat.com
---
 tools/virsh.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 95ed7bc..4e4ca57 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
 
 intCaught = 0;
 sig_action.sa_sigaction = vshCatchInt;
+sigemptyset((sigset_t *)sig_action.sa_flags);
 sigemptyset(sig_action.sa_mask);
 sigaction(SIGINT, sig_action, old_sig_action);
 
-- 
1.7.1

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


Re: [libvirt] [PATCH] virsh: avoid uninitialized memory usage

2012-04-19 Thread Wen Congyang
At 04/19/2012 04:09 PM, Alex Jia Wrote:
 Detected by valgrind.
 
 * tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.
  
 * How to reproduce?
 $ qemu-img create /var/lib/libvirt/images/test 1M
 $ cat  /tmp/test.xml EOF
 domain type='qemu'
   nametest/name
   memory219200/memory
   vcpu1/vcpu
   os
 type arch='x86_64'hvm/type
 boot dev='hd'/
   /os
   devices
 disk type='file' device='disk'
   driver name='qemu' type='raw'/
   source file='/var/lib/libvirt/images/test'/
   target dev='vda' bus='virtio'/
 /disk
 input type='mouse' bus='ps2'/
 graphics type='spice' autoport='yes' listen='0.0.0.0'/
   /devices
 /domain
 EOF
 $ virsh define /tmp/test.xml
 $ valgrind -v virsh blockpull test /var/lib/libvirt/images/test --wait
 
 actual result:
 
 ==10906== 1 errors in context 1 of 1:
 ==10906== Syscall param rt_sigaction(act-sa_flags) points to uninitialised 
 byte(s)
 ==10906==at 0x39CF80F5BE: __libc_sigaction (sigaction.c:67)
 ==10906==by 0x43016C: cmdBlockPull (virsh.c:7638)
 ==10906==by 0x4150D4: vshCommandRun (virsh.c:18574)
 ==10906==by 0x425E73: main (virsh.c:20178)
 ==10906==  Address 0x7fefffae8 is on thread 1's stack
 
 
 Signed-off-by: Alex Jia a...@redhat.com
 ---
  tools/virsh.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/tools/virsh.c b/tools/virsh.c
 index 95ed7bc..4e4ca57 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
  
  intCaught = 0;
  sig_action.sa_sigaction = vshCatchInt;
 +sigemptyset((sigset_t *)sig_action.sa_flags);

Why using sigemptyset here? You should use 'sig_action.sa_flags = 0'.

Thanks
Wen Congyang

  sigemptyset(sig_action.sa_mask);
  sigaction(SIGINT, sig_action, old_sig_action);
  

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


Re: [libvirt] [PATCH] virsh: avoid uninitialized memory usage

2012-04-19 Thread Alex Jia

On 04/19/2012 04:19 PM, Wen Congyang wrote:

At 04/19/2012 04:09 PM, Alex Jia Wrote:

Detected by valgrind.

* tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.

* How to reproduce?
$ qemu-img create /var/lib/libvirt/images/test 1M
$ cat  /tmp/test.xmlEOF
domain type='qemu'
   nametest/name
   memory219200/memory
   vcpu1/vcpu
   os
 type arch='x86_64'hvm/type
 boot dev='hd'/
   /os
   devices
 disk type='file' device='disk'
   driver name='qemu' type='raw'/
   source file='/var/lib/libvirt/images/test'/
   target dev='vda' bus='virtio'/
 /disk
 input type='mouse' bus='ps2'/
 graphics type='spice' autoport='yes' listen='0.0.0.0'/
   /devices
/domain
EOF
$ virsh define /tmp/test.xml
$ valgrind -v virsh blockpull test /var/lib/libvirt/images/test --wait

actual result:

==10906== 1 errors in context 1 of 1:
==10906== Syscall param rt_sigaction(act-sa_flags) points to uninitialised 
byte(s)
==10906==at 0x39CF80F5BE: __libc_sigaction (sigaction.c:67)
==10906==by 0x43016C: cmdBlockPull (virsh.c:7638)
==10906==by 0x4150D4: vshCommandRun (virsh.c:18574)
==10906==by 0x425E73: main (virsh.c:20178)
==10906==  Address 0x7fefffae8 is on thread 1's stack


Signed-off-by: Alex Jiaa...@redhat.com
---
  tools/virsh.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 95ed7bc..4e4ca57 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)

  intCaught = 0;
  sig_action.sa_sigaction = vshCatchInt;
+sigemptyset((sigset_t *)sig_action.sa_flags);

Why using sigemptyset here? You should use 'sig_action.sa_flags = 0'.
Yeah, I think 'sig_action.sa_flags = 0' is right, but I don't know what 
the difference are,

could you explain more?

Thanks,
Alex

Thanks
Wen Congyang


  sigemptyset(sig_action.sa_mask);
  sigaction(SIGINT,sig_action,old_sig_action);



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


Re: [libvirt] [PATCH V11 1/7] Implement virHashRemoveAll function

2012-04-19 Thread Daniel Veillard
On Tue, Apr 17, 2012 at 10:44:02AM -0400, Stefan Berger wrote:
 Implement function to remove all entries of a hash table.
 
 ---
  src/libvirt_private.syms |1 +
  src/util/virhash.c   |   25 +
  src/util/virhash.h   |5 +
  3 files changed, 31 insertions(+)
 
 Index: libvirt-acl/src/libvirt_private.syms
 ===
 --- libvirt-acl.orig/src/libvirt_private.syms
 +++ libvirt-acl/src/libvirt_private.syms
 @@ -578,6 +578,7 @@ virHashForEach;
  virHashFree;
  virHashGetItems;
  virHashLookup;
 +virHashRemoveAll;
  virHashRemoveEntry;
  virHashRemoveSet;
  virHashSearch;
 Index: libvirt-acl/src/util/virhash.c
 ===
 --- libvirt-acl.orig/src/util/virhash.c
 +++ libvirt-acl/src/util/virhash.c
 @@ -575,6 +575,31 @@ virHashRemoveSet(virHashTablePtr table,
  return count;
  }
  
 +static int
 +_virHashRemoveAllIter(const void *payload ATTRIBUTE_UNUSED,
 +  const void *name ATTRIBUTE_UNUSED,
 +  const void *data ATTRIBUTE_UNUSED)
 +{
 +return 1;
 +}
 +
 +/**
 + * virHashRemoveAll
 + * @table: the hash table to clear
 + *
 + * Free the hash @table's contents. The userdata is
 + * deallocated with the function provided at creation time.
 + *
 + * Returns the number of items removed on success, -1 on failure
 + */
 +ssize_t
 +virHashRemoveAll(virHashTablePtr table)
 +{
 +return virHashRemoveSet(table,
 +_virHashRemoveAllIter,
 +NULL);
 +}
 +
  /**
   * virHashSearch:
   * @table: the hash table to search
 Index: libvirt-acl/src/util/virhash.h
 ===
 --- libvirt-acl.orig/src/util/virhash.h
 +++ libvirt-acl/src/util/virhash.h
 @@ -127,6 +127,11 @@ int virHashRemoveEntry(virHashTablePtr t
 const void *name);
  
  /*
 + * Remove all entries from the hash table.
 + */
 +ssize_t virHashRemoveAll(virHashTablePtr table);
 +
 +/*
   * Retrieve the userdata.
   */
  void *virHashLookup(virHashTablePtr table, const void *name);
 

  There is certainly a way to implement this in a more efficient fashion
but that's fine :-)

  ACK,

Daniel

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

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


Re: [libvirt] [PATCH 0/2 v3 use qemu's dump-guest-meory when vm uses host device

2012-04-19 Thread Wen Congyang
At 04/19/2012 04:38 PM, Daniel P. Berrange Wrote:
 On Thu, Apr 19, 2012 at 09:03:16AM +0800, Wen Congyang wrote:
 Currently, we use migrate to dump guest's memory. There is one
 restriction in migrate command: the device's status should be
 stored in qemu because the device's status should be passed to
 target machine.

 If we passthrough a host device to guest, the device's status
 is stored in the real device. So migrate command will fail.

 We usually use dump when guest is panicked. So there is no need
 to store device's status in the vmcore.

 qemu will have a new monitor command dump-guest-memory to dump
 guest memory, but it doesn't support async now(it will support
 later when the common async API is implemented).

 So I use dump-guest-memory only when the guest uses host device
 in this patchset.
 
 Hmm, doesn't the new command generate dump files in a totally
 different format to the existing command ?  If so I don't
 think it is nice behaviour to silently switch between 2 different
 dump formats depending on the XML config.  I think we'd want some
 kind of flag to specify what format is desired.

Agree with it. But the new command is not a async command, and it
will block the other operation. So I only use it when the vm uses
host device(migrate command will fail in this case).

The new command will be converted to async command later(after
async API is implemented). I think we can allow the user specify
the format when the new command is async command.

Thanks
Wen Congyang

 
 Daniel

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


Re: [libvirt] [PATCH] virsh: avoid uninitialized memory usage

2012-04-19 Thread Alex Jia

On 04/19/2012 04:40 PM, Alex Jia wrote:

On 04/19/2012 04:19 PM, Wen Congyang wrote:

At 04/19/2012 04:09 PM, Alex Jia Wrote:

Detected by valgrind.

* tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.

* How to reproduce?
$ qemu-img create /var/lib/libvirt/images/test 1M
$ cat  /tmp/test.xmlEOF
domain type='qemu'
nametest/name
memory219200/memory
vcpu1/vcpu
os
type arch='x86_64'hvm/type
boot dev='hd'/
/os
devices
disk type='file' device='disk'
driver name='qemu' type='raw'/
source file='/var/lib/libvirt/images/test'/
target dev='vda' bus='virtio'/
/disk
input type='mouse' bus='ps2'/
graphics type='spice' autoport='yes' listen='0.0.0.0'/
/devices
/domain
EOF
$ virsh define /tmp/test.xml
$ valgrind -v virsh blockpull test /var/lib/libvirt/images/test --wait

actual result:

==10906== 1 errors in context 1 of 1:
==10906== Syscall param rt_sigaction(act-sa_flags) points to 
uninitialised byte(s)

==10906==at 0x39CF80F5BE: __libc_sigaction (sigaction.c:67)
==10906==by 0x43016C: cmdBlockPull (virsh.c:7638)
==10906==by 0x4150D4: vshCommandRun (virsh.c:18574)
==10906==by 0x425E73: main (virsh.c:20178)
==10906==  Address 0x7fefffae8 is on thread 1's stack


Signed-off-by: Alex Jiaa...@redhat.com
---
  tools/virsh.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 95ed7bc..4e4ca57 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)

  intCaught = 0;
  sig_action.sa_sigaction = vshCatchInt;
+sigemptyset((sigset_t *)sig_action.sa_flags);

Why using sigemptyset here? You should use 'sig_action.sa_flags = 0'.
Yeah, I think 'sig_action.sa_flags = 0' is right, but I don't know 
what the difference are,

could you explain more?
The sigemptyset() function manipulates sets of signals and initialize 
signal set to be empty.

'sig_action.sa_flags = 0' is right and enough in here.

Thanks,
Alex


Thanks,
Alex

Thanks
Wen Congyang


  sigemptyset(sig_action.sa_mask);
  sigaction(SIGINT,sig_action,old_sig_action);



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


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


Re: [libvirt] [PATCH 0/2 v3 use qemu's dump-guest-meory when vm uses host device

2012-04-19 Thread Daniel P. Berrange
On Thu, Apr 19, 2012 at 09:03:16AM +0800, Wen Congyang wrote:
 Currently, we use migrate to dump guest's memory. There is one
 restriction in migrate command: the device's status should be
 stored in qemu because the device's status should be passed to
 target machine.
 
 If we passthrough a host device to guest, the device's status
 is stored in the real device. So migrate command will fail.
 
 We usually use dump when guest is panicked. So there is no need
 to store device's status in the vmcore.
 
 qemu will have a new monitor command dump-guest-memory to dump
 guest memory, but it doesn't support async now(it will support
 later when the common async API is implemented).
 
 So I use dump-guest-memory only when the guest uses host device
 in this patchset.

Hmm, doesn't the new command generate dump files in a totally
different format to the existing command ?  If so I don't
think it is nice behaviour to silently switch between 2 different
dump formats depending on the XML config.  I think we'd want some
kind of flag to specify what format is desired.

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


[libvirt] [PATCHv2] virsh: avoid uninitialized memory usage

2012-04-19 Thread Alex Jia
Detected by valgrind.

* tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.
 
* How to reproduce?
$ qemu-img create /var/lib/libvirt/images/test 1M
$ cat  /tmp/test.xml EOF
domain type='qemu'
  nametest/name
  memory219200/memory
  vcpu1/vcpu
  os
type arch='x86_64'hvm/type
boot dev='hd'/
  /os
  devices
disk type='file' device='disk'
  driver name='qemu' type='raw'/
  source file='/var/lib/libvirt/images/test'/
  target dev='vda' bus='virtio'/
/disk
input type='mouse' bus='ps2'/
graphics type='spice' autoport='yes' listen='0.0.0.0'/
  /devices
/domain
EOF
$ virsh define /tmp/test.xml
$ valgrind -v virsh blockpull test /var/lib/libvirt/images/test --wait

actual result:

==10906== 1 errors in context 1 of 1:
==10906== Syscall param rt_sigaction(act-sa_flags) points to uninitialised 
byte(s)
==10906==at 0x39CF80F5BE: __libc_sigaction (sigaction.c:67)
==10906==by 0x43016C: cmdBlockPull (virsh.c:7638)
==10906==by 0x4150D4: vshCommandRun (virsh.c:18574)
==10906==by 0x425E73: main (virsh.c:20178)
==10906==  Address 0x7fefffae8 is on thread 1's stack


Signed-off-by: Alex Jia a...@redhat.com
---
 tools/virsh.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 95ed7bc..4e4ca57 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
 
 intCaught = 0;
 sig_action.sa_sigaction = vshCatchInt;
+sig_action.sa_flags = 0;
 sigemptyset(sig_action.sa_mask);
 sigaction(SIGINT, sig_action, old_sig_action);
 
-- 
1.7.1

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


Re: [libvirt] [PATCH] virsh: avoid uninitialized memory usage

2012-04-19 Thread Wen Congyang
At 04/19/2012 04:40 PM, Alex Jia Wrote:
 On 04/19/2012 04:19 PM, Wen Congyang wrote:
 At 04/19/2012 04:09 PM, Alex Jia Wrote:
 Detected by valgrind.

 * tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.

 * How to reproduce?
 $ qemu-img create /var/lib/libvirt/images/test 1M
 $ cat  /tmp/test.xmlEOF
 domain type='qemu'
nametest/name
memory219200/memory
vcpu1/vcpu
os
  type arch='x86_64'hvm/type
  boot dev='hd'/
/os
devices
  disk type='file' device='disk'
driver name='qemu' type='raw'/
source file='/var/lib/libvirt/images/test'/
target dev='vda' bus='virtio'/
  /disk
  input type='mouse' bus='ps2'/
  graphics type='spice' autoport='yes' listen='0.0.0.0'/
/devices
 /domain
 EOF
 $ virsh define /tmp/test.xml
 $ valgrind -v virsh blockpull test /var/lib/libvirt/images/test --wait

 actual result:

 ==10906== 1 errors in context 1 of 1:
 ==10906== Syscall param rt_sigaction(act-sa_flags) points to
 uninitialised byte(s)
 ==10906==at 0x39CF80F5BE: __libc_sigaction (sigaction.c:67)
 ==10906==by 0x43016C: cmdBlockPull (virsh.c:7638)
 ==10906==by 0x4150D4: vshCommandRun (virsh.c:18574)
 ==10906==by 0x425E73: main (virsh.c:20178)
 ==10906==  Address 0x7fefffae8 is on thread 1's stack


 Signed-off-by: Alex Jiaa...@redhat.com
 ---
   tools/virsh.c |1 +
   1 files changed, 1 insertions(+), 0 deletions(-)

 diff --git a/tools/virsh.c b/tools/virsh.c
 index 95ed7bc..4e4ca57 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)

   intCaught = 0;
   sig_action.sa_sigaction = vshCatchInt;
 +sigemptyset((sigset_t *)sig_action.sa_flags);
 Why using sigemptyset here? You should use 'sig_action.sa_flags = 0'.
 Yeah, I think 'sig_action.sa_flags = 0' is right, but I don't know what
 the difference are,
 could you explain more?

sigset_t is:
# define _SIGSET_NWORDS (1024 / (8 * sizeof (unsigned long int)))
typedef struct
  {
unsigned long int __val[_SIGSET_NWORDS];
  } __sigset_t;

The length of sigset is larger than sizeof(int)

If you use sigemptyset() to clear flags, it will affect the memory after flags.
It is very dangerous!!!

Thanks
Wen Congyang

 
 Thanks,
 Alex
 Thanks
 Wen Congyang

   sigemptyset(sig_action.sa_mask);
   sigaction(SIGINT,sig_action,old_sig_action);

 
 

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


Re: [libvirt] [PATCH V11 2/7] Support for atomic operations on integers

2012-04-19 Thread Daniel Veillard
On Tue, Apr 17, 2012 at 10:44:03AM -0400, Stefan Berger wrote:
 For threading support, add atomic add and sub operations working on
 integers. Base this on locking support provided by virMutex.
 
 ---
  src/util/viratomic.h |   91 
 +++
  1 file changed, 91 insertions(+)
 
 Index: libvirt-acl/src/util/viratomic.h
 ===
 --- /dev/null
 +++ libvirt-acl/src/util/viratomic.h
 @@ -0,0 +1,91 @@
 +/*
 + * viratomic.h: atomic integer operations
 + *
 + * Copyright (C) 2012 IBM Corporation
 + *
 + * Authors:
 + * Stefan Berger stef...@linux.vnet.ibm.com
 + *
 + * This library is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU Lesser General Public
 + * License as published by the Free Software Foundation; either
 + * version 2.1 of the License, or (at your option) any later version.
 + *
 + * This library is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * Lesser General Public License for more details.
 + *
 + * You should have received a copy of the GNU Lesser General Public
 + * License along with this library; if not, write to the Free Software
 + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
 + *
 + */
 +
 +#ifndef __VIR_ATOMIC_H__
 +# define __VIR_ATOMIC_H__
 +
 +# include threads.h
 +
 +typedef struct _virAtomicInt virAtomicInt;
 +typedef virAtomicInt *virAtomicIntPtr;
 +
 +struct _virAtomicInt {
 +virMutex lock;
 +int value;
 +};
 +
 +static inline int
 +virAtomicIntInit(virAtomicIntPtr vaip)
 +{
 +vaip-value = 0;
 +return virMutexInit(vaip-lock);
 +}
 +
 +static inline void
 +virAtomicIntSet(virAtomicIntPtr vaip, int value)
 +{
 + virMutexLock(vaip-lock);
 +
 + vaip-value = value;
 +
 + virMutexUnlock(vaip-lock);
 +}
 +
 +static inline int
 +virAtomicIntRead(virAtomicIntPtr vaip)
 +{
 + return vaip-value;
 +}
 +
 +static inline int
 +virAtomicIntAdd(virAtomicIntPtr vaip, int add)
 +{
 +int ret;
 +
 +virMutexLock(vaip-lock);
 +
 +vaip-value += add;
 +ret = vaip-value;
 +
 +virMutexUnlock(vaip-lock);
 +
 +return ret;
 +}
 +
 +static inline int
 +virAtomicIntSub(virAtomicIntPtr vaip, int sub)
 +{
 +int ret;
 +
 +virMutexLock(vaip-lock);
 +
 +vaip-value -= sub;
 +ret = vaip-value;
 +
 +virMutexUnlock(vaip-lock);
 +
 +return ret;
 +}
 +
 +#endif /* __VIR_ATOMIC_H */

  To be honnest, I'm not too fond of inline functions, but we already
rely on them (though for accessors mostly). In that case it's
reasonnable to use them,

  ACK, but it itches me a bit

Daniel

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

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


Re: [libvirt] [PATCHv2] virsh: avoid uninitialized memory usage

2012-04-19 Thread Wen Congyang
At 04/19/2012 04:51 PM, Alex Jia Wrote:
 Detected by valgrind.
 
 * tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.
  
 * How to reproduce?
 $ qemu-img create /var/lib/libvirt/images/test 1M
 $ cat  /tmp/test.xml EOF
 domain type='qemu'
   nametest/name
   memory219200/memory
   vcpu1/vcpu
   os
 type arch='x86_64'hvm/type
 boot dev='hd'/
   /os
   devices
 disk type='file' device='disk'
   driver name='qemu' type='raw'/
   source file='/var/lib/libvirt/images/test'/
   target dev='vda' bus='virtio'/
 /disk
 input type='mouse' bus='ps2'/
 graphics type='spice' autoport='yes' listen='0.0.0.0'/
   /devices
 /domain
 EOF
 $ virsh define /tmp/test.xml
 $ valgrind -v virsh blockpull test /var/lib/libvirt/images/test --wait
 
 actual result:
 
 ==10906== 1 errors in context 1 of 1:
 ==10906== Syscall param rt_sigaction(act-sa_flags) points to uninitialised 
 byte(s)
 ==10906==at 0x39CF80F5BE: __libc_sigaction (sigaction.c:67)
 ==10906==by 0x43016C: cmdBlockPull (virsh.c:7638)
 ==10906==by 0x4150D4: vshCommandRun (virsh.c:18574)
 ==10906==by 0x425E73: main (virsh.c:20178)
 ==10906==  Address 0x7fefffae8 is on thread 1's stack
 
 
 Signed-off-by: Alex Jia a...@redhat.com
 ---
  tools/virsh.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/tools/virsh.c b/tools/virsh.c
 index 95ed7bc..4e4ca57 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
  
  intCaught = 0;
  sig_action.sa_sigaction = vshCatchInt;
 +sig_action.sa_flags = 0;
  sigemptyset(sig_action.sa_mask);
  sigaction(SIGINT, sig_action, old_sig_action);
  

ACK

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


Re: [libvirt] [PATCH] virsh: avoid uninitialized memory usage

2012-04-19 Thread Alex Jia

On 04/19/2012 04:53 PM, Wen Congyang wrote:

At 04/19/2012 04:40 PM, Alex Jia Wrote:

On 04/19/2012 04:19 PM, Wen Congyang wrote:

At 04/19/2012 04:09 PM, Alex Jia Wrote:

Detected by valgrind.

* tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.

* How to reproduce?
$ qemu-img create /var/lib/libvirt/images/test 1M
$ cat   /tmp/test.xmlEOF
domain type='qemu'
nametest/name
memory219200/memory
vcpu1/vcpu
os
  type arch='x86_64'hvm/type
  boot dev='hd'/
/os
devices
  disk type='file' device='disk'
driver name='qemu' type='raw'/
source file='/var/lib/libvirt/images/test'/
target dev='vda' bus='virtio'/
  /disk
  input type='mouse' bus='ps2'/
  graphics type='spice' autoport='yes' listen='0.0.0.0'/
/devices
/domain
EOF
$ virsh define /tmp/test.xml
$ valgrind -v virsh blockpull test /var/lib/libvirt/images/test --wait

actual result:

==10906== 1 errors in context 1 of 1:
==10906== Syscall param rt_sigaction(act-sa_flags) points to
uninitialised byte(s)
==10906==at 0x39CF80F5BE: __libc_sigaction (sigaction.c:67)
==10906==by 0x43016C: cmdBlockPull (virsh.c:7638)
==10906==by 0x4150D4: vshCommandRun (virsh.c:18574)
==10906==by 0x425E73: main (virsh.c:20178)
==10906==  Address 0x7fefffae8 is on thread 1's stack


Signed-off-by: Alex Jiaa...@redhat.com
---
   tools/virsh.c |1 +
   1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 95ed7bc..4e4ca57 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)

   intCaught = 0;
   sig_action.sa_sigaction = vshCatchInt;
+sigemptyset((sigset_t *)sig_action.sa_flags);

Why using sigemptyset here? You should use 'sig_action.sa_flags = 0'.

Yeah, I think 'sig_action.sa_flags = 0' is right, but I don't know what
the difference are,
could you explain more?

sigset_t is:
# define _SIGSET_NWORDS (1024 / (8 * sizeof (unsigned long int)))
typedef struct
   {
 unsigned long int __val[_SIGSET_NWORDS];
   } __sigset_t;

The length of sigset is larger than sizeof(int)

If you use sigemptyset() to clear flags, it will affect the memory after flags.
It is very dangerous!!!

Yeah, thanks for your explanation again.

Thanks
Wen Congyang


Thanks,
Alex

Thanks
Wen Congyang


   sigemptyset(sig_action.sa_mask);
   sigaction(SIGINT,sig_action,old_sig_action);





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


Re: [libvirt] [libvirt-glib] Add async variant of gvir_domain_get_info()

2012-04-19 Thread Christophe Fergeau
On Thu, Apr 19, 2012 at 03:12:01AM +0300, Zeeshan Ali (Khattak) wrote:
 From: Zeeshan Ali (Khattak) zeesha...@gnome.org
 
 ---
  libvirt-gobject/libvirt-gobject-domain.c |   74 
 ++
  libvirt-gobject/libvirt-gobject-domain.h |7 +++
  libvirt-gobject/libvirt-gobject.sym  |2 +
  3 files changed, 83 insertions(+), 0 deletions(-)
 
 diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
 b/libvirt-gobject/libvirt-gobject-domain.c
 index 0bafa7e..896aae1 100644
 --- a/libvirt-gobject/libvirt-gobject-domain.c
 +++ b/libvirt-gobject/libvirt-gobject-domain.c
 @@ -572,6 +572,80 @@ GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom,
  return ret;
  }
  
 +static void
 +gvir_domain_get_info_helper(GSimpleAsyncResult *res,
 +GObject *object,
 +GCancellable *cancellable G_GNUC_UNUSED)
 +{
 +GVirDomain *dom = GVIR_DOMAIN(object);
 +GVirDomainInfo *info;
 +GError *err = NULL;
 +
 +info = gvir_domain_get_info(dom, err);
 +if (err)
 +g_simple_async_result_take_error(res, err);
 +else
 +g_simple_async_result_set_op_res_gpointer(res, info, NULL);

Shouldn't the last parameter be gvir_domain_info_free?

 +}
 +
 +/**
 + * gir_domain_get_info_async:
 + * @dom: the domain
 + * @cancellable: (allow-none)(transfer none): cancellation object
 + * @callback: (scope async): completion callback
 + * @user_data: (closure): opaque data for callback
 + *
 + * Asynchronous variant of #gvir_domain_get_info.
 + */
 +void gvir_domain_get_info_async(GVirDomain *dom,
 +GCancellable *cancellable,
 +GAsyncReadyCallback callback,
 +gpointer user_data)
 +{
 +GSimpleAsyncResult *res;
 +
 +g_return_if_fail(GVIR_IS_DOMAIN(dom));
 +
 +res = g_simple_async_result_new(G_OBJECT(dom),
 +callback,
 +user_data,
 +gvir_domain_get_info_async);
 +g_simple_async_result_run_in_thread(res,
 +gvir_domain_get_info_helper,
 +G_PRIORITY_DEFAULT,
 +cancellable);
 +g_object_unref(res);
 +}
 +
 +/**
 + * gir_domain_get_info_finish:
 + * @dom: the domain
 + * @result: (transfer none): async method result
 + * @err: Place-holder for possible errors
 + *
 + * Finishes the operation started by #gvir_domain_get_info_async.
 + *
 + * Returns: (transfer full): the info
 + */
 +GVirDomainInfo *gvir_domain_get_info_finish(GVirDomain *dom,
 +GAsyncResult *result,
 +GError **err)
 +{
 +GSimpleAsyncResult *res = G_SIMPLE_ASYNC_RESULT(result);
 +
 +g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);

FALSE - NULL

 +g_return_val_if_fail
 +(g_simple_async_result_is_valid(result,
 +G_OBJECT(dom),
 +gvir_domain_get_info_async),
 + FALSE);

Same here

 +
 +if (g_simple_async_result_propagate_error(res, err))
 +return NULL;
 +
 +return g_simple_async_result_get_op_res_gpointer(res);
 +}
 +
  /**
   * gvir_domain_screenshot:
   * @stream: stream to use as output
 diff --git a/libvirt-gobject/libvirt-gobject-domain.h 
 b/libvirt-gobject/libvirt-gobject-domain.h
 index 8a4836e..677fbe6 100644
 --- a/libvirt-gobject/libvirt-gobject-domain.h
 +++ b/libvirt-gobject/libvirt-gobject-domain.h
 @@ -141,6 +141,13 @@ gboolean gvir_domain_reboot(GVirDomain *dom,
  
  GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom,
   GError **err);
 +void gvir_domain_get_info_async(GVirDomain *dom,
 +GCancellable *cancellable,
 +GAsyncReadyCallback callback,
 +gpointer user_data);
 +GVirDomainInfo *gvir_domain_get_info_finish(GVirDomain *dom,
 +GAsyncResult *result,
 +GError **err);
  
  GVirConfigDomain *gvir_domain_get_config(GVirDomain *dom,
   guint flags,
 diff --git a/libvirt-gobject/libvirt-gobject.sym 
 b/libvirt-gobject/libvirt-gobject.sym
 index 64c91cc..00b32d4 100644
 --- a/libvirt-gobject/libvirt-gobject.sym
 +++ b/libvirt-gobject/libvirt-gobject.sym
 @@ -70,6 +70,8 @@ LIBVIRT_GOBJECT_0.0.7 {
   gvir_domain_set_config;
   gvir_domain_get_devices;
   gvir_domain_get_info;
 + gvir_domain_get_info_async;
 + gvir_domain_get_info_finish;
   gvir_domain_get_persistent;
   gvir_domain_get_saved;
   gvir_domain_screenshot;
 -- 
 1.7.7.6
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 

[libvirt] [PATCH] virsh: Fix and clarify the --title flag for the list command in man page

2012-04-19 Thread Peter Krempa
---
 tools/virsh.pod |9 +
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 140d8e8..25751b6 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -398,13 +398,14 @@ may be combined. Default behavior is as though both were 
specified. (Note that
 if any of these flags is specified, the check for the persistence state is done
 and may fail. If none of these flags is specified, the check is skipped.)

-If I--title is specified, then the domain note is printed. The output then
-the output looks as follows. This flag is usable only with the default
-I--table output.
+If I--title is specified, then the short domain description - title - is
+printed. This flag is usable only with the default I--table output.
+
+Example:

 Bvirsh list --title
  Id Name State  Title

+ ---
   0 Domain-0 runningMailserver 1
   2 fedora   paused

-- 
1.7.3.4

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


Re: [libvirt] [test-API 01/14] Change libvirt-test-api to be executable

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

And remove the redundant python in usage string.
---
  libvirt-test-api |   16 
  1 files changed, 8 insertions(+), 8 deletions(-)
  mode change 100644 =  100755 libvirt-test-api

diff --git a/libvirt-test-api b/libvirt-test-api
old mode 100644
new mode 100755
index 373e5c6..7eaaf21
--- a/libvirt-test-api
+++ b/libvirt-test-api
@@ -46,14 +46,14 @@ def usage():


  print example: \
-   \n python libvirt-test-api -l 0|1 -c TEST.CONF\
-   \n python libvirt-test-api -c TEST.CONF -f TEST.XML \
-   \n python libvirt-test-api -t repos/domain/start.py ... \
-   \n python libvirt-test-api -m TESTONE.XML TESTTWO.XML \
-   \n python libvirt-test-api -d TEST.XML TESTRUNID TESTID \
-   \n python libvirt-test-api -d TEST.XML TESTRUNID \
-   \n python libvirt-test-api -d TEST.XML all \
-   \n python libvirt-test-api -f TEST.XML \
+   \n libvirt-test-api -l 0|1 -c TEST.CONF\
+   \n libvirt-test-api -c TEST.CONF -f TEST.XML \
+   \n libvirt-test-api -t repos/domain/start.py ... \
+   \n libvirt-test-api -m TESTONE.XML TESTTWO.XML \
+   \n libvirt-test-api -d TEST.XML TESTRUNID TESTID \
+   \n libvirt-test-api -d TEST.XML TESTRUNID \
+   \n libvirt-test-api -d TEST.XML all \
+   \n libvirt-test-api -f TEST.XML \
  -r TESTRUNID TESTID ...

  def append_path():


   ACK

   Guannan Ren

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


Re: [libvirt] [test-API 02/14] Remove NEWS and TODO

2012-04-19 Thread Osier Yang

On 2012年04月19日 17:33, Guannan Ren wrote:

On 04/19/2012 10:25 AM, Osier Yang wrote:

We don't have a release cycle, and seems we will never update
TODO
---
TODO | 4 
1 files changed, 0 insertions(+), 4 deletions(-)
delete mode 100644 NEWS
delete mode 100644 TODO

diff --git a/NEWS b/NEWS
deleted file mode 100644
index e69de29..000
diff --git a/TODO b/TODO
deleted file mode 100644
index 6803754..000
--- a/TODO
+++ /dev/null
@@ -1,4 +0,0 @@
-
-- Refine the comments and coding style.
-
-- There are redundant codes, need to optimize, e.g. proxy.py




really? I have a plan!!
But I don't tell you.



Well, it's the words hundrends of years ago. :-)


ACK

Guannan Ren

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


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

Re: [libvirt] [test-API 02/14] Remove NEWS and TODO

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

We don't have a release cycle, and seems we will never update
TODO
---
  TODO |4 
  1 files changed, 0 insertions(+), 4 deletions(-)
  delete mode 100644 NEWS
  delete mode 100644 TODO

diff --git a/NEWS b/NEWS
deleted file mode 100644
index e69de29..000
diff --git a/TODO b/TODO
deleted file mode 100644
index 6803754..000
--- a/TODO
+++ /dev/null
@@ -1,4 +0,0 @@
-
-- Refine the comments and coding style.
-
-- There are redundant codes, need to optimize, e.g. proxy.py


really?  I have a plan!!
But I don't tell you.

ACK

Guannan Ren

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


Re: [libvirt] [test-API 03/14] Destroy utils/check.py and move the functions into utils/utils.py

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

Scripts which use 'check.py' are updated to use utils.py.
---
  repos/domain/balloon_memory.py  |2 +-
  repos/domain/dump.py|2 +-
  repos/domain/install_linux_check.py |   15 +-
  repos/snapshot/file_flag.py |3 +-
  repos/snapshot/flag_check.py|3 +-
  utils/check.py  |  392 ---
  utils/utils.py  |  362 
  7 files changed, 373 insertions(+), 406 deletions(-)
  delete mode 100644 utils/check.py




   I support the idea, but we really need a run to ensure no break
   ACK

   Guannan Ren

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


Re: [libvirt] [test-API 04/14] Move directory dist to src

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

env_inspect.py is part of the framework.
---
  {dist =  src/dist}/__init__.py   |0
  {dist =  src/dist}/redhat/__init__.py|0
  {dist =  src/dist}/redhat/env_inspect.py |0
  src/generator.py |4 ++--
  4 files changed, 2 insertions(+), 2 deletions(-)
  rename {dist =  src/dist}/__init__.py (100%)
  rename {dist =  src/dist}/redhat/__init__.py (100%)
  rename {dist =  src/dist}/redhat/env_inspect.py (100%)

diff --git a/dist/__init__.py b/src/dist/__init__.py
similarity index 100%
rename from dist/__init__.py
rename to src/dist/__init__.py
diff --git a/dist/redhat/__init__.py b/src/dist/redhat/__init__.py
similarity index 100%
rename from dist/redhat/__init__.py
rename to src/dist/redhat/__init__.py
diff --git a/dist/redhat/env_inspect.py b/src/dist/redhat/env_inspect.py
similarity index 100%
rename from dist/redhat/env_inspect.py
rename to src/dist/redhat/env_inspect.py
diff --git a/src/generator.py b/src/generator.py
index e3bc344..26e1553 100644
--- a/src/generator.py
+++ b/src/generator.py
@@ -32,9 +32,9 @@ from utils import env_parser
  # Import of distribution-specific code.  If this is needed somewhere
  # else in the future, please don't copy-paste this, but create some
  # sensible distribution-specific package
-for dist in os.listdir('dist'):
+for dist in os.listdir('src/dist'):
  if os.path.exists('/etc/%s-release' % dist):
-exec('from dist.%s import env_inspect' % dist)
+exec('from src.dist.%s import env_inspect' % dist)
  break

  class FuncGen(object):


maybe the dist folder should be removed.
I rewrote the env_inspect, it is more potable than checking the 
rpm package.

so, the dist/redhat is not necessary strongly

ACK
Guannan Ren

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


Re: [libvirt] [test-API 05/14] Move utils/env_parser.py into src

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

env_parser.py is to parse env.cfg, which is part of the framework.
---
  repos/domain/install_image.py |2 +-
  repos/domain/install_linux_cdrom.py   |2 +-
  repos/domain/install_linux_check.py   |2 +-
  repos/domain/install_linux_net.py |2 +-
  repos/domain/install_windows_cdrom.py |2 +-
  {utils =  src}/env_parser.py  |0
  src/generator.py  |2 +-
  src/parser.py |2 +-
  8 files changed, 7 insertions(+), 7 deletions(-)
  rename {utils =  src}/env_parser.py (100%)

diff --git a/repos/domain/install_image.py b/repos/domain/install_image.py
index 88b35c1..89b7e40 100644
--- a/repos/domain/install_image.py
+++ b/repos/domain/install_image.py
@@ -11,8 +11,8 @@ import libvirt
  from libvirt import libvirtError

  from src import sharedmod
+from src import env_parser
  from utils import utils
-from utils import env_parser
  from utils import xmlbuilder

  HOME_PATH = os.getcwd()
diff --git a/repos/domain/install_linux_cdrom.py 
b/repos/domain/install_linux_cdrom.py
index f5af6db..342ed84 100644
--- a/repos/domain/install_linux_cdrom.py
+++ b/repos/domain/install_linux_cdrom.py
@@ -14,8 +14,8 @@ import libvirt
  from libvirt import libvirtError

  from src import sharedmod
+from src import env_parser
  from utils import utils
-from utils import env_parser
  from utils import xmlbuilder

  required_params = ('guestname', 'guesttype', 'guestos', 'guestarch',)
diff --git a/repos/domain/install_linux_check.py 
b/repos/domain/install_linux_check.py
index 7a5a9ac..f4d04d9 100644
--- a/repos/domain/install_linux_check.py
+++ b/repos/domain/install_linux_check.py
@@ -12,8 +12,8 @@ import libvirt
  from libvirt import libvirtError

  from src import sharedmod
+from src import env_parser
  from utils import utils
-from utils import env_parser

  required_params = ('guestname', 'guesttype', 'hdmodel', 'nicmodel',)
  optional_params = ('disksize',
diff --git a/repos/domain/install_linux_net.py 
b/repos/domain/install_linux_net.py
index 69f3279..8c8931d 100644
--- a/repos/domain/install_linux_net.py
+++ b/repos/domain/install_linux_net.py
@@ -14,8 +14,8 @@ import libvirt
  from libvirt import libvirtError

  from src import sharedmod
+from src import env_parser
  from utils import utils
-from utils import env_parser
  from utils import xmlbuilder

  required_params = ('guestname', 'guesttype', 'guestos', 
'guestarch','netmethod',)
diff --git a/repos/domain/install_windows_cdrom.py 
b/repos/domain/install_windows_cdrom.py
index beeb7dd..652e721 100644
--- a/repos/domain/install_windows_cdrom.py
+++ b/repos/domain/install_windows_cdrom.py
@@ -13,8 +13,8 @@ import libvirt
  from libvirt import libvirtError

  from src import sharedmod
+from src import env_parser
  from utils import utils
-from utils import env_parser
  from utils import xmlbuilder

  VIRSH_QUIET_LIST = virsh --quiet list --all|awk '{print $2}'|grep \^%s$\
diff --git a/utils/env_parser.py b/src/env_parser.py
similarity index 100%
rename from utils/env_parser.py
rename to src/env_parser.py
diff --git a/src/generator.py b/src/generator.py
index 26e1553..1f19ea3 100644
--- a/src/generator.py
+++ b/src/generator.py
@@ -25,9 +25,9 @@ import os
  import traceback

  from src import mapper
+from src import env_parser
  from utils import log
  from utils import format
-from utils import env_parser

  # Import of distribution-specific code.  If this is needed somewhere
  # else in the future, please don't copy-paste this, but create some
diff --git a/src/parser.py b/src/parser.py
index f1c2a43..1c4abcb 100644
--- a/src/parser.py
+++ b/src/parser.py
@@ -23,8 +23,8 @@ import sys
  import copy
  import string

-from utils import env_parser
  from src import exception
+from src import env_parser

  class CaseFileParser(object):
   Parser the case configuration file to generate a data list.


 ACK

 Guannan Ren

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


Re: [libvirt] [test-API 08/14] Remove trainling blank lines for utils/*.sh

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

---
  utils/dev_num.sh  |3 ---
  utils/disk_num.sh |3 ---
  2 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/utils/dev_num.sh b/utils/dev_num.sh
index 9f50398..5e79313 100755
--- a/utils/dev_num.sh
+++ b/utils/dev_num.sh
@@ -23,6 +23,3 @@ else
 rm -f guestdump.xml
 exit 1
  fi
-
-
-
diff --git a/utils/disk_num.sh b/utils/disk_num.sh
index 97a6248..8c10e52 100755
--- a/utils/disk_num.sh
+++ b/utils/disk_num.sh
@@ -21,6 +21,3 @@ else
 rm -f guestdump.xml
 exit 1
  fi
-
-
-


ACK

Guannan Ren

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


Re: [libvirt] [PATCH V11 3/7] nwfilter: Fix support for trusted DHCP servers

2012-04-19 Thread Daniel Veillard
On Tue, Apr 17, 2012 at 10:44:04AM -0400, Stefan Berger wrote:
 Fix the support for trusted DHCP server in the ebtables code's
 hard-coded function applying DHCP only filtering rules:
 Rather than using a char * use the more flexible
 virNWFilterVarValuePtr that contains the trusted DHCP server(s)
 IP address. Process all entries.
 
 Since all callers so far provided NULL as parameter, no changes
 are necessary in any other code.
 
 ---
  src/conf/nwfilter_conf.h  |2 
  src/nwfilter/nwfilter_ebiptables_driver.c |   72 
 +-
  2 files changed, 44 insertions(+), 30 deletions(-)
 
 Index: libvirt-acl/src/conf/nwfilter_conf.h
 ===
 --- libvirt-acl.orig/src/conf/nwfilter_conf.h
 +++ libvirt-acl/src/conf/nwfilter_conf.h
 @@ -625,7 +625,7 @@ typedef int (*virNWFilterApplyBasicRules
  
  typedef int (*virNWFilterApplyDHCPOnlyRules)(const char *ifname,
   const unsigned char *macaddr,
 - const char *dhcpserver,
 + virNWFilterVarValuePtr dhcpsrvs,
   bool leaveTemporary);
  
  typedef int (*virNWFilterRemoveBasicRules)(const char *ifname);
 Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
 ===
 --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
 +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
 @@ -3195,7 +3195,7 @@ tear_down_tmpebchains:
   * @ifname: name of the backend-interface to which to apply the rules
   * @macaddr: MAC address the VM is using in packets sent through the
   *interface
 - * @dhcpserver: The DHCP server from which the VM may receive traffic
 + * @dhcpsrvrs: The DHCP server(s) from which the VM may receive traffic
   *from; may be NULL
   * @leaveTemporary: Whether to leave the table names with their temporary
   *names (true) or also perform the renaming to their final names as
 @@ -3209,14 +3209,15 @@ tear_down_tmpebchains:
  static int
  ebtablesApplyDHCPOnlyRules(const char *ifname,
 const unsigned char *macaddr,
 -   const char *dhcpserver,
 +   virNWFilterVarValuePtr dhcpsrvrs,
 bool leaveTemporary)
  {
  virBuffer buf = VIR_BUFFER_INITIALIZER;
  char chain_in [MAX_CHAINNAME_LENGTH],
   chain_out[MAX_CHAINNAME_LENGTH];
  char macaddr_str[VIR_MAC_STRING_BUFLEN];
 -char *srcIPParam = NULL;
 +unsigned int idx = 0;
 +unsigned int num_dhcpsrvrs;
  
  if (!ebtables_cmd_path) {
  virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, %s,
 @@ -3225,15 +3226,6 @@ ebtablesApplyDHCPOnlyRules(const char *i
  return -1;
  }
  
 -if (dhcpserver) {
 -virBufferAsprintf(buf,  --ip-src %s, dhcpserver);
 -if (virBufferError(buf)) {
 -virBufferFreeAndReset(buf);
 -return -1;
 -}
 -srcIPParam = virBufferContentAndReset(buf);
 -}
 -
  virMacAddrFormat(macaddr, macaddr_str);
  
  ebiptablesAllTeardown(ifname);
 @@ -3267,20 +3259,46 @@ ebtablesApplyDHCPOnlyRules(const char *i
chain_in,
CMD_STOPONERR(1));
  
 -virBufferAsprintf(buf,
 -  CMD_DEF($EBT -t nat -A %s
 -   -d %s
 -   -p ipv4 --ip-protocol udp
 -   %s
 -   --ip-sport 67 --ip-dport 68
 -   -j ACCEPT) CMD_SEPARATOR
 -  CMD_EXEC
 -  %s,
 +num_dhcpsrvrs = (dhcpsrvrs != NULL)
 +? virNWFilterVarValueGetCardinality(dhcpsrvrs)
 +: 0;
  
 -  chain_out,
 -  macaddr_str,
 -  srcIPParam != NULL ? srcIPParam : ,
 -  CMD_STOPONERR(1));
 +while (true) {
 +char *srcIPParam = NULL;
 +
 +if (idx  num_dhcpsrvrs) {
 +const char *dhcpserver;
 +
 +dhcpserver = virNWFilterVarValueGetNthValue(dhcpsrvrs, idx);
 +
 +if (virAsprintf(srcIPParam, --ip-src %s, dhcpserver)  0) {
 +virReportOOMError();
 +goto tear_down_tmpebchains;
 +}
 +}
 +
 +virBufferAsprintf(buf,
 +  CMD_DEF($EBT -t nat -A %s
 +   -d %s
 +   -p ipv4 --ip-protocol udp
 +   %s
 +   --ip-sport 67 --ip-dport 68
 +   -j ACCEPT) CMD_SEPARATOR
 +  CMD_EXEC
 +  %s,
 +
 +  

Re: [libvirt] [test-API 06/14] Improve the comments telling what the scripts does for utils/*.py

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

What GNU recommends:

quote
one line to give the program's name and a brief idea of what it does.
Copyright (C)yearname of author
/quote

---
v1 was ACKed, but I forgot to push it, this is a rebasing on the same
files (some of them are moved to different place).
---
  src/env_parser.py |6 ++
  utils/XMLParser.py|7 ++-
  utils/format.py   |6 ++
  utils/log.py  |6 ++
  utils/xmlbuilder.py   |6 ++
  utils/xmlgenerator.py |6 ++
  6 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/src/env_parser.py b/src/env_parser.py
index 58f05fc..0d2f2a2 100644
--- a/src/env_parser.py
+++ b/src/env_parser.py
@@ -1,5 +1,7 @@
  #!/usr/bin/env python
  #
+# env_parser.py: Parser for environment config (env.cfg).
+#
  # Copyright (C) 2010-2012 Red Hat, Inc.
  #
  # libvirt-test-API is free software; you can redistribute it and/or
@@ -14,10 +16,6 @@
  #
  # You should have received a copy of the GNU General Public License
  # along with this program. If not, seehttp://www.gnu.org/licenses/.
-#
-# Filename: env_parser.py
-# Summary: parse the env configuration file
-# Description: The module is a tool to parse the env configuration file

  import ConfigParser
  import os
diff --git a/utils/XMLParser.py b/utils/XMLParser.py
index b254023..20a376e 100644
--- a/utils/XMLParser.py
+++ b/utils/XMLParser.py
@@ -1,5 +1,7 @@
  #!/usr/bin/env python
  #
+# XMLParser.py: Parse XML document, the result is a python dict.
+#
  # Copyright (C) 2010-2012 Red Hat, Inc.
  #
  # libvirt-test-API is free software; you can redistribute it and/or
@@ -14,11 +16,6 @@
  #
  # You should have received a copy of the GNU General Public License
  # along with this program. If not, seehttp://www.gnu.org/licenses/.
-#
-# Filename: XMLParser.py
-# Summary: parse and xml document into a python dictionary
-# Description: The module is a tool to parses
-# and xml document into a python dictionary

  import os
  from xml.dom import minidom
diff --git a/utils/format.py b/utils/format.py
index cb2b8e0..5d69e5c 100644
--- a/utils/format.py
+++ b/utils/format.py
@@ -1,5 +1,7 @@
  #!/usr/bin/env python
  #
+# format.py: Print logging message in specific format.
+#
  # Copyright (C) 2010-2012 Red Hat, Inc.
  #
  # libvirt-test-API is free software; you can redistribute it and/or
@@ -14,10 +16,6 @@
  #
  # You should have received a copy of the GNU General Public License
  # along with this program. If not, seehttp://www.gnu.org/licenses/.
-#
-# Filename: format.py
-# Summary: generate specified kind of format string
-# Description: The module is a tool to generate specified kind of format string

  import os
  from string import Template
diff --git a/utils/log.py b/utils/log.py
index 3ca8729..4111fd9 100644
--- a/utils/log.py
+++ b/utils/log.py
@@ -1,5 +1,7 @@
  #!/usr/bin/env python
  #
+# log.py: Classes for logging.
+#
  # Copyright (C) 2010-2012 Red Hat, Inc.
  #
  # libvirt-test-API is free software; you can redistribute it and/or
@@ -14,10 +16,6 @@
  #
  # You should have received a copy of the GNU General Public License
  # along with this program. If not, seehttp://www.gnu.org/licenses/.
-#
-# Filename: log.py
-# Summary: log file operation
-# Description: The module is a tool to provide basic log file operation

  import time
  import os
diff --git a/utils/xmlbuilder.py b/utils/xmlbuilder.py
index a8a9871..d2ce6bb 100644
--- a/utils/xmlbuilder.py
+++ b/utils/xmlbuilder.py
@@ -1,5 +1,7 @@
  #!/usr/bin/env python
  #
+# xmlbuilder.py: Class for building XML for libvirt objects.
+#
  # Copyright (C) 2010-2012 Red Hat, Inc.
  #
  # libvirt-test-API is free software; you can redistribute it and/or
@@ -14,10 +16,6 @@
  #
  # You should have received a copy of the GNU General Public License
  # along with this program. If not, seehttp://www.gnu.org/licenses/.
-#
-# Filename: xmlbuilder.py
-# Summary: operation for building domain xml
-# Description: The module is to provide operation for building domain xml

  __DEBUG__ = False

diff --git a/utils/xmlgenerator.py b/utils/xmlgenerator.py
index 7f5839b..ee019b7 100644
--- a/utils/xmlgenerator.py
+++ b/utils/xmlgenerator.py
@@ -1,5 +1,7 @@
  #!/usr/bin/env python
  #
+# xmlgenerator.py: Generate XML for libvirt objects.
+#
  # Copyright (C) 2010-2012 Red Hat, Inc.
  #
  # libvirt-test-API is free software; you can redistribute it and/or
@@ -14,10 +16,6 @@
  #
  # You should have received a copy of the GNU General Public License
  # along with this program. If not, seehttp://www.gnu.org/licenses/.
-#
-# Filename: xmlgenerator.py
-# Summary: generate domain xml
-# Description: The module is a tool to generate domain xml

  import os
  import sys


   ACK

   Guannan Ren

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


Re: [libvirt] [test-API 04/14] Move directory dist to src

2012-04-19 Thread Martin Kletzander
On 04/19/2012 11:38 AM, Guannan Ren wrote:
 On 04/19/2012 10:25 AM, Osier Yang wrote:
 env_inspect.py is part of the framework.
 ---
   {dist =  src/dist}/__init__.py   |0
   {dist =  src/dist}/redhat/__init__.py|0
   {dist =  src/dist}/redhat/env_inspect.py |0
   src/generator.py |4 ++--
   4 files changed, 2 insertions(+), 2 deletions(-)
   rename {dist =  src/dist}/__init__.py (100%)
   rename {dist =  src/dist}/redhat/__init__.py (100%)
   rename {dist =  src/dist}/redhat/env_inspect.py (100%)

 diff --git a/dist/__init__.py b/src/dist/__init__.py
 similarity index 100%
 rename from dist/__init__.py
 rename to src/dist/__init__.py
 diff --git a/dist/redhat/__init__.py b/src/dist/redhat/__init__.py
 similarity index 100%
 rename from dist/redhat/__init__.py
 rename to src/dist/redhat/__init__.py
 diff --git a/dist/redhat/env_inspect.py b/src/dist/redhat/env_inspect.py
 similarity index 100%
 rename from dist/redhat/env_inspect.py
 rename to src/dist/redhat/env_inspect.py
 diff --git a/src/generator.py b/src/generator.py
 index e3bc344..26e1553 100644
 --- a/src/generator.py
 +++ b/src/generator.py
 @@ -32,9 +32,9 @@ from utils import env_parser
   # Import of distribution-specific code.  If this is needed somewhere
   # else in the future, please don't copy-paste this, but create some
   # sensible distribution-specific package
 -for dist in os.listdir('dist'):
 +for dist in os.listdir('src/dist'):
   if os.path.exists('/etc/%s-release' % dist):
 -exec('from dist.%s import env_inspect' % dist)
 +exec('from src.dist.%s import env_inspect' % dist)
   break

   class FuncGen(object):
 
 maybe the dist folder should be removed.
 I rewrote the env_inspect, it is more potable than checking the
 rpm package.
 so, the dist/redhat is not necessary strongly
 
 ACK
 Guannan Ren
 

The new env_inspect is written universally, so it can be moved back, yes.

Martin

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


Re: [libvirt] [test-API 09/14] Use bash as the interpreter explicitly

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

---
  utils/dev_num.sh  |2 +-
  utils/disk_num.sh |2 +-
  utils/ipget.sh|2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/utils/dev_num.sh b/utils/dev_num.sh
index 5e79313..552721b 100755
--- a/utils/dev_num.sh
+++ b/utils/dev_num.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
  # counting disk and interface numbers

  guestname=$1
diff --git a/utils/disk_num.sh b/utils/disk_num.sh
index 8c10e52..57f46aa 100755
--- a/utils/disk_num.sh
+++ b/utils/disk_num.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash

  guestname=$1
  if [[ -z $guestname ]]; then
diff --git a/utils/ipget.sh b/utils/ipget.sh
index 8d44b14..dd074be 100755
--- a/utils/ipget.sh
+++ b/utils/ipget.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash

  mac=$1
  if [[ -z $mac ]]; then


I don't think it's right.
The /bin/sh is symbolic link to bash in most GNU/Linux.
But it could be a link to another shell (ksh, dash)
we should make the script more portable to support POSIX shell.

NACK

Guannan Ren


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


Re: [libvirt] [test-API 07/14] Improve more comments to tell what the program does

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

---
  src/dist/redhat/env_inspect.py |5 ++---
  src/exception.py   |7 ++-
  src/logxmlparser.py|7 ++-
  src/process.py |7 ++-
  4 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/src/dist/redhat/env_inspect.py b/src/dist/redhat/env_inspect.py
index 06f6408..4942b33 100644
--- a/src/dist/redhat/env_inspect.py
+++ b/src/dist/redhat/env_inspect.py
@@ -1,5 +1,7 @@
  #!/usr/bin/env python
  #
+# env_inspect.py: Check the testing environment.
+
  # Copyright (C) 2010-2012 Red Hat, Inc.
  #
  # libvirt-test-API is free software; you can redistribute it and/or
@@ -14,9 +16,6 @@
  #
  # You should have received a copy of the GNU General Public License
  # along with this program. If not, seehttp://www.gnu.org/licenses/.
-#
-# Filename: envinspect.py
-# Description: check the testing environment and state of libvirt as well

  import commands
  import libvirt
diff --git a/src/exception.py b/src/exception.py
index 3625dab..7f67602 100644
--- a/src/exception.py
+++ b/src/exception.py
@@ -1,5 +1,7 @@
  #!/usr/bin/env python
  #
+# exception.py: Exceptions for the framework.
+#
  # Copyright (C) 2010-2012 Red Hat, Inc.
  #
  # libvirt-test-API is free software; you can redistribute it and/or
@@ -14,11 +16,6 @@
  #
  # You should have received a copy of the GNU General Public License
  # along with this program. If not, seehttp://www.gnu.org/licenses/.
-#
-# Filename: exception.py
-# Summary: the exception class
-# Description: The module defines the exceptions the framework could use
-#  when fatal error occurred.

  import libvirt

diff --git a/src/logxmlparser.py b/src/logxmlparser.py
index 6e0b531..3b377ce 100644
--- a/src/logxmlparser.py
+++ b/src/logxmlparser.py
@@ -1,5 +1,7 @@
  #!/usr/bin/env python
  #
+# logxmlparser.py: Generate output log file in XML format.
+
  # Copyright (C) 2010-2012 Red Hat, Inc.
  #
  # libvirt-test-API is free software; you can redistribute it and/or
@@ -14,11 +16,6 @@
  #
  # You should have received a copy of the GNU General Public License
  # along with this program. If not, seehttp://www.gnu.org/licenses/.
-#
-# Filename: xmlparser.py
-# Summary: generate xmlfile for log management
-# Description: The module gives methods to write xml block
-#  during a testrun running

  import os
  import copy
diff --git a/src/process.py b/src/process.py
index b34c9af..71d26ea 100644
--- a/src/process.py
+++ b/src/process.py
@@ -1,5 +1,7 @@
  #!/usr/bin/env python
  #
+# process.py: Multiple process module
+#
  # Copyright (C) 2010-2012 Red Hat, Inc.
  #
  # libvirt-test-API is free software; you can redistribute it and/or
@@ -14,11 +16,6 @@
  #
  # You should have received a copy of the GNU General Public License
  # along with this program. If not, seehttp://www.gnu.org/licenses/.
-#
-# Filename: process.py
-# Summary: multiprocessing module
-# Description: If the switch of multiprocessing is on,
-#  the module will be called to fork subprocess

  import os
  import sys



   ACK

   Guannan Ren

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


[libvirt] autogen.sh broken on RHEL5

2012-04-19 Thread Daniel P. Berrange


$ ./autogen.sh  --system
Running ./configure with --prefix=/usr --sysconfdir=/etc --localstatedir=/var 
--libdir=/usr/lib64 
running bootstrap...
./bootstrap: Bootstrapping from checked-out http://libvirt.org sources...
./bootstrap: consider installing git-merge-changelog from gnulib
./bootstrap: getting gnulib files...
./bootstrap: autopoint --force
Copying file build-aux/config.rpath
running: libtoolize --copy --install
libtoolize: unrecognized option `--install'
Try `libtoolize --help' for more information.


There is no such '--install' option - is that even needed - AFAIK that is
the default behaviour ?

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 V11 5/7] nwfilter: move code for IP address map into separate file

2012-04-19 Thread Daniel Veillard
On Tue, Apr 17, 2012 at 10:44:06AM -0400, Stefan Berger wrote:
 The goal of this patch is to prepare for support for multiple IP
 addresses per interface in the DHCP snooping code.
 
 Move the code for the IP address map that maps interface names to
 IP addresses into their own file. Rename the functions on the way
 but otherwise leave the code as-is. Initialize this new layer
 separately before dependent layers (iplearning, dhcpsnooping)
 and shut it down after them.
 
 ---
  src/Makefile.am|4 
  src/conf/nwfilter_ipaddrmap.c  |  167 
 +
  src/conf/nwfilter_ipaddrmap.h  |   37 +++
  src/libvirt_private.syms   |8 +
  src/nwfilter/nwfilter_driver.c |   11 +-
  src/nwfilter/nwfilter_gentech_driver.c |5 
  src/nwfilter/nwfilter_learnipaddr.c|  126 
  src/nwfilter/nwfilter_learnipaddr.h|3 
  8 files changed, 229 insertions(+), 132 deletions(-)
 
 Index: libvirt-acl/src/Makefile.am
 ===
 --- libvirt-acl.orig/src/Makefile.am
 +++ libvirt-acl/src/Makefile.am
 @@ -159,9 +159,11 @@ NETWORK_CONF_SOURCES =   
 \
  # Network filter driver generic impl APIs
  NWFILTER_PARAM_CONF_SOURCES =\
   conf/nwfilter_params.c conf/nwfilter_params.h   \
 + conf/nwfilter_ipaddrmap.c   \
 + conf/nwfilter_ipaddrmap.h   \
   conf/nwfilter_conf.h
  
 -NWFILTER_CONF_SOURCES =  \
 +NWFILTER_CONF_SOURCES =  \
   $(NWFILTER_PARAM_CONF_SOURCES)  \
   conf/nwfilter_conf.c conf/nwfilter_conf.h
  
 Index: libvirt-acl/src/nwfilter/nwfilter_driver.c
 ===
 --- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c
 +++ libvirt-acl/src/nwfilter/nwfilter_driver.c
 @@ -39,6 +39,7 @@
  #include nwfilter_gentech_driver.h
  #include configmake.h
  
 +#include nwfilter_ipaddrmap.h
  #include nwfilter_dhcpsnoop.h
  #include nwfilter_learnipaddr.h
  
 @@ -67,10 +68,12 @@ static int
  nwfilterDriverStartup(int privileged) {
  char *base = NULL;
  
 -if (virNWFilterDHCPSnoopInit()  0)
 +if (virNWFilterIPAddrMapInit()  0)
  return -1;
  if (virNWFilterLearnInit()  0)
 -return -1;
 +goto err_exit_ipaddrmapshutdown;
 +if (virNWFilterDHCPSnoopInit()  0)
 +goto err_exit_learnshutdown;
  
  virNWFilterTechDriversInit(privileged);
  
 @@ -131,7 +134,10 @@ alloc_err_exit:
  conf_init_err:
  virNWFilterTechDriversShutdown();
  virNWFilterDHCPSnoopShutdown();
 +err_exit_learnshutdown:
  virNWFilterLearnShutdown();
 +err_exit_ipaddrmapshutdown:
 +virNWFilterIPAddrMapShutdown();
  
  return -1;
  }
 @@ -210,6 +216,7 @@ nwfilterDriverShutdown(void) {
  virNWFilterTechDriversShutdown();
  virNWFilterDHCPSnoopShutdown();
  virNWFilterLearnShutdown();
 +virNWFilterIPAddrMapShutdown();
  
  nwfilterDriverLock(driverState);
  
 Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c
 ===
 --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.c
 +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c
 @@ -33,6 +33,7 @@
  #include nwfilter_gentech_driver.h
  #include nwfilter_ebiptables_driver.h
  #include nwfilter_dhcpsnoop.h
 +#include nwfilter_ipaddrmap.h
  #include nwfilter_learnipaddr.h
  #include virnetdev.h
  #include datatypes.h
 @@ -870,7 +871,7 @@ __virNWFilterInstantiateFilter(const uns
  goto err_exit;
  }
  
 -ipaddr = virNWFilterGetIpAddrForIfname(ifname);
 +ipaddr = virNWFilterIPAddrMapGetIPAddr(ifname);
  
  vars1 = virNWFilterCreateVarHashmap(str_macaddr, ipaddr);
  if (!vars1) {
 @@ -1132,7 +1133,7 @@ _virNWFilterTeardownFilter(const char *i
  
  techdriver-allTeardown(ifname);
  
 -virNWFilterDelIpAddrForIfname(ifname, NULL);
 +virNWFilterIPAddrMapDelIPAddr(ifname, NULL);
  
  virNWFilterUnlockIface(ifname);
  
 Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c
 ===
 --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c
 +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c
 @@ -52,6 +52,7 @@
  #include conf/domain_conf.h
  #include nwfilter_gentech_driver.h
  #include nwfilter_ebiptables_driver.h
 +#include nwfilter_ipaddrmap.h
  #include nwfilter_learnipaddr.h
  
  #define VIR_FROM_THIS VIR_FROM_NWFILTER
 @@ -118,9 +119,6 @@ struct ether_vlan_header
  static virMutex pendingLearnReqLock;
  static virHashTablePtr pendingLearnReq;
  
 -static virMutex ipAddressMapLock;
 -static virNWFilterHashTablePtr ipAddressMap;
 -
  static virMutex 

Re: [libvirt] [test-API 10/14] Substitute 'guesttype' with 'virt_type'

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

Also 'guest type' to 'virt type'. The principle is to use the same
TERM as libvirt, and the projects around libvirt, e.g. python-virtinst.
---
  cases/consumption_cpu_topology.conf|2 +-
  cases/consumption_domain_nfs_start.conf|2 +-
  cases/consumption_eventhandler.conf|2 +-
  cases/consumption_libvirtd.conf|2 +-
  cases/consumption_ownership_test.conf  |2 +-
  cases/domain_linux_net_inst.conf   |4 +-
  cases/linux_domain.conf|   10 ++--
  cases/migration/ssh_persistent_paused_no_dst.conf  |   24 
  .../migration/ssh_persistent_paused_with_dst.conf  |   48 +++---
  cases/migration/ssh_persistent_running_no_dst.conf |   24 
  .../migration/ssh_persistent_running_with_dst.conf |   48 +++---
  cases/migration/ssh_transient_paused_no_dst.conf   |   22 
  cases/migration/ssh_transient_paused_with_dst.conf |   48 +++---
  cases/migration/ssh_transient_running_no_dst.conf  |   24 
  .../migration/ssh_transient_running_with_dst.conf  |   48 +++---
  cases/migration/tcp_persistent_paused_no_dst.conf  |   24 
  .../migration/tcp_persistent_paused_with_dst.conf  |   48 +++---
  cases/migration/tcp_persistent_running_no_dst.conf |   24 
  .../migration/tcp_persistent_running_with_dst.conf |   48 +++---
  .../tcp_sasl_persistent_paused_no_dst.conf |8 +-
  .../tcp_sasl_persistent_paused_with_dst.conf   |   16 +++---
  .../tcp_sasl_persistent_running_no_dst.conf|8 +-
  .../tcp_sasl_persistent_running_with_dst.conf  |   16 +++---
  .../tcp_sasl_transient_paused_no_dst.conf  |8 +-
  .../tcp_sasl_transient_paused_with_dst.conf|   16 +++---
  .../tcp_sasl_transient_running_no_dst.conf |8 +-
  .../tcp_sasl_transient_running_with_dst.conf   |   16 +++---
  cases/migration/tcp_transient_paused_no_dst.conf   |   22 
  cases/migration/tcp_transient_paused_with_dst.conf |   48 +++---
  cases/migration/tcp_transient_running_no_dst.conf  |   24 
  .../migration/tcp_transient_running_with_dst.conf  |   48 +++---
  cases/migration/tls_persistent_paused_no_dst.conf  |   24 
  .../migration/tls_persistent_paused_with_dst.conf  |   48 +++---
  cases/migration/tls_persistent_running_no_dst.conf |   24 
  .../migration/tls_persistent_running_with_dst.conf |   48 +++---
  .../tls_sasl_persistent_paused_no_dst.conf |8 +-
  .../tls_sasl_persistent_paused_with_dst.conf   |   16 +++---
  .../tls_sasl_persistent_running_no_dst.conf|8 +-
  .../tls_sasl_persistent_running_with_dst.conf  |   16 +++---
  .../tls_sasl_transient_paused_no_dst.conf  |8 +-
  .../tls_sasl_transient_paused_with_dst.conf|   16 +++---
  .../tls_sasl_transient_running_no_dst.conf |8 +-
  .../tls_sasl_transient_running_with_dst.conf   |   16 +++---
  cases/migration/tls_transient_paused_no_dst.conf   |   22 
  cases/migration/tls_transient_paused_with_dst.conf |   48 +++---
  cases/migration/tls_transient_running_no_dst.conf  |   24 
  .../migration/tls_transient_running_with_dst.conf  |   48 +++---
  cases/snapshot.conf|2 +-
  cases/windows_domain.conf  |8 +-
  .../en-US/Writing_a_test_case.xml  |2 +-
  repos/domain/attach_disk.py|2 +-
  repos/domain/create.py |2 +-
  repos/domain/define.py |   14 ++--
  repos/domain/detach_disk.py|2 +-
  repos/domain/install_image.py  |6 +-
  repos/domain/install_linux_cdrom.py|   18 +++---
  repos/domain/install_linux_check.py|8 +-
  repos/domain/install_linux_net.py  |   20 +++---
  repos/domain/install_windows_cdrom.py  |8 +-
  repos/domain/update_devflag.py |2 +-
  .../multiple_thread_block_on_domain_create.py  |6 +-
  utils/xmlbuilder.py|   14 ++--
  utils/xmlgenerator.py  |   64 ++--
  63 files changed, 626 insertions(+), 626 deletions(-)




  ACK

  Guannan Ren

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


Re: [libvirt] [PATCH] virsh: Fix and clarify the --title flag for the list command in man page

2012-04-19 Thread Osier Yang

On 2012年04月19日 17:03, Peter Krempa wrote:

---
  tools/virsh.pod |9 +
  1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 140d8e8..25751b6 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -398,13 +398,14 @@ may be combined. Default behavior is as though both were 
specified. (Note that
  if any of these flags is specified, the check for the persistence state is 
done
  and may fail. If none of these flags is specified, the check is skipped.)

-If I--title  is specified, then the domain note is printed. The output then
-the output looks as follows. This flag is usable only with the default
-I--table  output.
+If I--title  is specified, then the short domain description - title - is
+printed. This flag is usable only with the default I--table  output.
+
+Example:

  Bvirsh  list --title
   Id Name State  Title

+ ---
0 Domain-0 runningMailserver 1
2 fedora   paused



ACK. But how about short domain description (title)?

Osier

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

Re: [libvirt] [PATCH] virsh: Fix and clarify the --title flag for the list command in man page

2012-04-19 Thread Peter Krempa
On 04/19/2012 11:03 AM, Peter Krempa wrote:
 ---
   tools/virsh.pod |9 +
   1 files changed, 5 insertions(+), 4 deletions(-)
Effect of this patch on the manpage:

Before:
   If --title is specified, then the domain note is printed. The output then 
the output looks as follows. This flag
   is usable only with the default --table output.

virsh list --title
Id Name State  Title 
---
 0 Domain-0 runningMailserver 1
 2 fedora   paused

After:
   If --title is specified, then the short domain description - title - is 
printed. This flag is usable only with the
   default --table output.

Example:

virsh list --title
Id Name State  Title
---
0 Domain-0 runningMailserver 1
2 fedora   paused

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


Re: [libvirt] [test-API 13/14] Rename src/logxmlparser.py as src/log_generator.py

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

 From the codes, it's to generator the output log in XML format,
but not to parse. The class name is also changed from Logxml_parser
to LogGenerator.
---
  libvirt-test-api  |8 
  src/{logxmlparser.py =  log_generator.py} |4 ++--
  2 files changed, 6 insertions(+), 6 deletions(-)
  rename src/{logxmlparser.py =  log_generator.py} (99%)

diff --git a/libvirt-test-api b/libvirt-test-api
index 62e18f6..a409556 100755
--- a/libvirt-test-api
+++ b/libvirt-test-api
@@ -28,7 +28,7 @@ from src import generator
  from src import env_clear
  from src import process
  from utils import log
-from src.logxmlparser import Logxml_parser
+from src.log_generator import LogGenerator
  from src.activityfilter import Filter
  from src.casecfgcheck import CaseCfgCheck

@@ -93,7 +93,7 @@ class Main(object):
  testrunid = time.strftime(%Y%m%d%H%M%S)
  os.makedirs('log/%s' %testrunid)

-log_xml_parser = Logxml_parser(self.logxml)
+log_xml_parser = LogGenerator(self.logxml)

  # If the specified log xmlfile exists, then append the testrun
  # item of this time to the file, if not, create a new log xmlfile
@@ -256,7 +256,7 @@ class Main(object):

  def remove_log(self, testrunid, testid = None):
to remove log item in the log xmlfile 
-log_xml_parser = Logxml_parser(self.logxml)
+log_xml_parser = LogGenerator(self.logxml)

  # remove a test in a testrun
  if testrunid and testid:
@@ -289,7 +289,7 @@ class Main(object):

  def merge_logxmls(self, logxml_two):
   to merge two log xml files of log into one
-log_xml_parser = Logxml_parser(self.logxml)
+log_xml_parser = LogGenerator(self.logxml)
  log_xml_parser.merge_xmlfiles(logxml_two)
  print Merge the second log xml file %s to %s successfully  % \
(logxml_two, self.logxml)
diff --git a/src/logxmlparser.py b/src/log_generator.py
similarity index 99%
rename from src/logxmlparser.py
rename to src/log_generator.py
index 36d743e..a001ef2 100644
--- a/src/logxmlparser.py
+++ b/src/log_generator.py
@@ -1,6 +1,6 @@
  #!/usr/bin/env python
  #
-# logxmlparser.py: Generate output log file in XML format.
+# log_generator.py: Generate output log file in XML format.

  # Copyright (C) 2010-2012 Red Hat, Inc.
  #
@@ -24,7 +24,7 @@ from xml.dom.minidom import Document

  from src import exception

-class Logxml_parser(object):
+class LogGenerator(object):
   Generate and parser log xml file
  
  def __init__(self, logxml):


 ACK

 Guannan Ren

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


Re: [libvirt] [test-API 12/14] Rename utils/*.py with consistent naming style

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

We use foo_bar across the project (except the main program), though
it might be not good to use foo_bar as the module name in Python,
but let's unify it first.
---
  .../en-US/Understanding_libvirt-test-API.xml   |4 +-
  .../en-US/Writing_a_test_case.xml  |8 ++--
  libvirt-test-api   |8 ++--
  repos/domain/attach_disk.py|4 +-
  repos/domain/attach_interface.py   |4 +-
  repos/domain/create.py |4 +-
  repos/domain/define.py |4 +-
  repos/domain/detach_disk.py|4 +-
  repos/domain/detach_interface.py   |4 +-
  repos/domain/install_image.py  |4 +-
  repos/domain/install_linux_cdrom.py|6 +-
  repos/domain/install_linux_net.py  |6 +-
  repos/domain/install_windows_cdrom.py  |6 +-
  repos/domain/migrate.py|2 +-
  repos/domain/update_devflag.py |6 +-
  repos/interface/create.py  |2 +-
  repos/interface/define.py  |4 +-
  repos/interface/destroy.py |2 +-
  repos/interface/undefine.py|2 +-
  repos/network/create.py|4 +-
  repos/network/define.py|4 +-
  repos/npiv/create_virtual_hba.py   |4 +-
  .../multiple_thread_block_on_domain_create.py  |4 +-
  repos/snapshot/internal_create.py  |4 +-
  repos/storage/activate_pool.py |2 +-
  repos/storage/create_dir_pool.py   |4 +-
  repos/storage/create_dir_volume.py |4 +-
  repos/storage/create_fs_pool.py|8 ++--
  repos/storage/create_iscsi_pool.py |4 +-
  repos/storage/create_logical_volume.py |4 +-
  repos/storage/create_netfs_pool.py |8 ++--
  repos/storage/create_netfs_volume.py   |4 +-
  repos/storage/create_partition_volume.py   |4 +-
  repos/storage/define_dir_pool.py   |4 +-
  repos/storage/define_disk_pool.py  |4 +-
  repos/storage/define_iscsi_pool.py |4 +-
  repos/storage/define_logical_pool.py   |4 +-
  repos/storage/define_mpath_pool.py |4 +-
  repos/storage/define_netfs_pool.py |4 +-
  repos/storage/define_scsi_pool.py  |4 +-
  repos/storage/delete_partition_volume.py   |2 +-
  repos/storage/destroy_pool.py  |2 +-
  src/logxmlparser.py|2 +-
  utils/{xmlbuilder.py =  xml_builder.py}|   44 
++--
  utils/{xmlgenerator.py =  xml_generator.py}|2 +-
  utils/{XMLParser.py =  xml_parser.py}  |8 ++--
  46 files changed, 117 insertions(+), 117 deletions(-)
  rename utils/{xmlbuilder.py =  xml_builder.py} (91%)
  rename utils/{xmlgenerator.py =  xml_generator.py} (99%)
  rename utils/{XMLParser.py =  xml_parser.py} (96%)

diff --git 
a/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml
 
b/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml
index 7fe1e97..88c1b76 100644
--- 
a/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml
+++ 
b/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml
@@ -197,10 +197,10 @@ repos
parafilename/utils/filename  is a directory which contains various 
scripts to assist in creating and verifying test cases./para
!--itemizedlist
  listitem
-parafilenamexmlgenerator.py/filenameXML generator./para
+parafilenamexml_generator.py/filenameXML generator./para
  /listitem
  listitem
-parafilenamexmlbuilder.py/filename  builds a virtual machine from an XML 
file./para
+parafilenamexml_builder.py/filename  builds a virtual machine from an XML 
file./para
  /listitem
  listitem
paraRandom MAC address generator. Useful when installing a guest 
machine./para
diff --git 
a/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml 
b/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml
index 21c4ac6..2a74518 100644
--- a/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml
+++ b/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml
@@ -142,7 +142,7 @@ import exception
  from lib import connectAPI
  from lib import storageAPI
  from utils import env_parser
-from utils import xmlbuilder
+from utils import xml_builder

  envfile = 'env.ini'

@@ -154,7 +154,7 @@ def 

Re: [libvirt] [PATCH V11 3/7] nwfilter: Fix support for trusted DHCP servers

2012-04-19 Thread Stefan Berger

On 04/19/2012 05:27 AM, Daniel Veillard wrote:

On Tue, Apr 17, 2012 at 10:44:04AM -0400, Stefan Berger wrote:

Fix the support for trusted DHCP server in the ebtables code's
hard-coded function applying DHCP only filtering rules:
Rather than using a char * use the more flexible
virNWFilterVarValuePtr that contains the trusted DHCP server(s)
IP address. Process all entries.

Since all callers so far provided NULL as parameter, no changes
are necessary in any other code.


[...]

+while (true) {
+char *srcIPParam = NULL;
+
+if (idx  num_dhcpsrvrs) {
+const char *dhcpserver;
+
+dhcpserver = virNWFilterVarValueGetNthValue(dhcpsrvrs, idx);
+
+if (virAsprintf(srcIPParam, --ip-src %s, dhcpserver)  0) {
+virReportOOMError();
+goto tear_down_tmpebchains;
+}
+}
+
+virBufferAsprintf(buf,
+  CMD_DEF($EBT -t nat -A %s
+   -d %s
+   -p ipv4 --ip-protocol udp
+   %s
+   --ip-sport 67 --ip-dport 68
+   -j ACCEPT) CMD_SEPARATOR
+  CMD_EXEC
+  %s,
+
+  chain_out,
+  macaddr_str,
+  srcIPParam != NULL ? srcIPParam : ,
+  CMD_STOPONERR(1));
+
+VIR_FREE(srcIPParam);
+
+if (idx == num_dhcpsrvrs)
+break;
+
+idx++;
+}

   There is something I don't understand in that loop, you repetedly
write to buf, but you don't seems to use buf in the loop. This looks
fishy to me, or are you using side effect execution in the Asprintf
argument evaluation. Too cryptic to my taste, I'm lost !


I am accumulating (shell) commands in the buffer and then test the 
buffer for error before finally executing the commands.


   Stefan

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


Re: [libvirt] [test-API 14/14] Rename 'env.cfg' as 'global.cfg'

2012-04-19 Thread Guannan Ren

On 04/19/2012 10:25 AM, Osier Yang wrote:

It's not only about environment actually, many default settings are
defined in it.
---
  env.cfg =  global.cfg  |0
  repos/domain/install_image.py  |2 +-
  repos/domain/install_linux_cdrom.py|2 +-
  repos/domain/install_linux_check.py|2 +-
  repos/domain/install_linux_net.py  |4 ++--
  repos/domain/install_windows_cdrom.py  |6 +++---
  .../multiple_thread_block_on_domain_create.py  |2 +-
  src/env_parser.py  |2 +-
  src/exception.py   |2 +-
  src/generator.py   |2 +-
  src/parser.py  |4 ++--
  11 files changed, 14 insertions(+), 14 deletions(-)
  rename env.cfg =  global.cfg (100%)

diff --git a/env.cfg b/global.cfg
similarity index 100%
rename from env.cfg
rename to global.cfg
diff --git a/repos/domain/install_image.py b/repos/domain/install_image.py
index be36b03..e94ebc7 100644
--- a/repos/domain/install_image.py
+++ b/repos/domain/install_image.py
@@ -60,7 +60,7 @@ def install_image(params):
  logger.info(the path of directory of disk images located on is %s %
   imgfullpath)

-envfile = os.path.join(HOME_PATH, 'env.cfg')
+envfile = os.path.join(HOME_PATH, 'global.cfg')
  logger.info(the environment file is %s % envfile)

  envparser = env_parser.Envparser(envfile)
diff --git a/repos/domain/install_linux_cdrom.py 
b/repos/domain/install_linux_cdrom.py
index ee3c397..1d5fb28 100644
--- a/repos/domain/install_linux_cdrom.py
+++ b/repos/domain/install_linux_cdrom.py
@@ -223,7 +223,7 @@ def install_linux_cdrom(params):
  logger.info(creating disk images file is successful.)

  logger.info(get system environment information)
-envfile = os.path.join(HOME_PATH, 'env.cfg')
+envfile = os.path.join(HOME_PATH, 'global.cfg')
  logger.info(the environment file is %s % envfile)

  envparser = env_parser.Envparser(envfile)
diff --git a/repos/domain/install_linux_check.py 
b/repos/domain/install_linux_check.py
index 303ab7f..74b8819 100644
--- a/repos/domain/install_linux_check.py
+++ b/repos/domain/install_linux_check.py
@@ -148,7 +148,7 @@ def install_linux_check(params):
  # Check app works fine in guest, such as: wget
  logger.info(check point5: check app works fine in guest, such as: wget)
  logger.info(get system environment information)
-envfile = os.path.join(HOME_PATH, 'env.cfg')
+envfile = os.path.join(HOME_PATH, 'global.cfg')
  logger.info(the environment file is %s % envfile)

  envparser = env_parser.Envparser(envfile)
diff --git a/repos/domain/install_linux_net.py 
b/repos/domain/install_linux_net.py
index 105c0ea..a47bcea 100644
--- a/repos/domain/install_linux_net.py
+++ b/repos/domain/install_linux_net.py
@@ -188,13 +188,13 @@ def install_linux_net(params):


  logger.info(get system environment information)
-envfile = os.path.join(HOME_PATH, 'env.cfg')
+envfile = os.path.join(HOME_PATH, 'global.cfg')
  logger.info(the environment file is %s % envfile)

  envparser = env_parser.Envparser(envfile)

  # Get http, ftp or nfs url based on guest os, arch
-# and installation method from env.cfg
+# and installation method from global.cfg

  if installmethod == 'http':
  ks = envparser.get_value(guest, guestos + _ + guestarch +
diff --git a/repos/domain/install_windows_cdrom.py 
b/repos/domain/install_windows_cdrom.py
index 825aa29..402fa25 100644
--- a/repos/domain/install_windows_cdrom.py
+++ b/repos/domain/install_windows_cdrom.py
@@ -256,10 +256,10 @@ def install_windows_cdrom(params):
  logger.info(creating disk images file is successful.)

  logger.info(get system environment information)
-envfile = os.path.join(HOME_PATH, 'env.cfg')
+envfile = os.path.join(HOME_PATH, 'global.cfg')
  logger.info(the environment file is %s % envfile)

-# Get iso file based on guest os and arch from env.cfg
+# Get iso file based on guest os and arch from global.cfg
  envparser = env_parser.Envparser(envfile)
  iso_file = envparser.get_value(guest, guestos + '_' + guestarch)
  cdkey = envparser.get_value(guest, %s_%s_key % (guestos, guestarch))
@@ -427,7 +427,7 @@ def install_windows_cdrom_clean(params):
  guestos = params.get('guestos')
  guestarch = params.get('guestarch')

-envfile = os.path.join(HOME_PATH, 'env.cfg')
+envfile = os.path.join(HOME_PATH, 'global.cfg')
  envparser = env_parser.Envparser(envfile)
  iso_file = envparser.get_value(guest, guestos + '_' + guestarch)

diff --git a/repos/regression/multiple_thread_block_on_domain_create.py 
b/repos/regression/multiple_thread_block_on_domain_create.py
index 1c85706..180736a 100644
--- 

[libvirt] Making the Thread-safety issues with vbox driver ?

2012-04-19 Thread Jean-Baptiste Rouault
On Wednesday 04 April 2012 17:56:12 Jean-Baptiste Rouault wrote:
 On Monday 16 January 2012 11:34:53 Matthias Bolte wrote:
  Okay, without looking deeper into this here are some ideas:
  
  The XPCOM API libvirt uses might not be threadsafe, or needs to be
  initialized for every thread that wants to use it. Currently its only
  initialized for the thread that opens the driver. I know that this is
  the case on Windows were VirtualBox uses MSCOM for its API and you
  need to call CoInitialize on every thread. This is currently not done
  for the MSCOM glue in libvirt, so I know that on Windows the
  VirtualBox driver is not threadsafe currently. Also I didn't look into
  a solution for this yet. Maybe we need a thread local variable that
  holds whether (MS/XP)COM was already initialized for this thread and
  add a check to every driver function to initialize it when needed.
  
  Did you try to open a connection for each thread instead of trying to
  share one? If that works reliable it might indicate that there is an
  VirtualBox API initialization problem.
 
 I tried today with one connection for each thread and it works.
 
 I changed the vbox driver so that the pfnComInitialize function is called
 only when the first connection is opened : it breaks the test, even with
 one connection per thread.
 
 I guess we'll have to use a thread local variable as you suggested, unless
 someone has a better idea to handle this problem.

Hi,

I looked deeper into these thread-safety issues, once a new connection is 
opened for each thread, everything works well.
However, opening and closing connections isn't thread-safe at all for two 
reasons :

- VirtualBox C bindings initialization and uninitialization functions aren't 
thread-safe. I talked about it with upstream on IRC and they are probably not 
going to fix it, but would accept a patch fixing the issue. I'm going to 
contact 
upstream again to get some advices so I can write a patch.

- In the libvirt vbox driver, for each new connection, modification of the 
global variable g_pVBoxGlobalData isn't protected (see line 1040 of 
vbox_tmpl.c). 

First of all, is it really necessary to replace it on each new connection, or 
would it be ok to initialize it only when the first connection is opened ?

A global mutex is needed in the vbox driver to protect access to 
g_pVBoxGlobalData, the vboxRegister() function seems to be the best place to 
initialize such a mutex unless there is another entry point to do this ?

-- 
Jean-Baptiste ROUAULT
Ingénieur RD - diateam : Architectes de l'information
Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 051

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

Re: [libvirt] [PATCH V11 4/7] nwfilter: add DHCP snooping

2012-04-19 Thread Stefan Berger

On 04/19/2012 05:54 AM, Daniel Veillard wrote:

On Tue, Apr 17, 2012 at 10:44:05AM -0400, Stefan Berger wrote:


Index: libvirt-acl/src/conf/nwfilter_params.h
===
--- libvirt-acl.orig/src/conf/nwfilter_params.h
+++ libvirt-acl/src/conf/nwfilter_params.h
@@ -91,6 +91,11 @@ int virNWFilterHashTablePutAll(virNWFilt
  # define VALID_VARVALUE \
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:

+# define NWFILTER_VARNAME_IP IP
+# define NWFILTER_VARNAME_MAC MAC
+# define NWFILTER_VARNAME_IP_LEARNING ip_learning
+# define NWFILTER_VARNAME_DHCPSERVER DHCPSERVER
+
  enum virNWFilterVarAccessType {
  VIR_NWFILTER_VAR_ACCESS_ELEMENT = 0,
  VIR_NWFILTER_VAR_ACCESS_ITERATOR = 1,

   A fairly big patch ! I have tried to really read everything, but
finding locking issues, especially when there is multiple layers
of locks is nearly impossible by review.
   I think the best is to get this running and tested as much as possible
before the next release,

   ACK



Thanks for the ACK.

However, there are some things that I would like to adapt before 
checking in:


- rename the 'ip_learning' variable to CTRL_IP_LEARNING to open up a 
'namespace' for future extensions providing control to users over the 
behavior of the algorithm; someone may be interested in having IP 
addresses detected by the DHCP snooper but have no filters instantiated 
and for that there could be a variable CTRL_NO_INSTANTIATE (=true)


- some small optimization to not have packets submitted to the worker 
that are shorter than the minimal size a valid DHCP packet must have


- forgot about the existence of virSocketAddr and so I needed to replace 
the usage of uint32_t's with virSocketAddr


So I would check in patches 1-3 soon and post the rest of the series 
again later.


   Stefan


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


Re: [libvirt] [PATCH V11 5/7] nwfilter: move code for IP address map into separate file

2012-04-19 Thread Stefan Berger

On 04/19/2012 05:58 AM, Daniel Veillard wrote:

On Tue, Apr 17, 2012 at 10:44:06AM -0400, Stefan Berger wrote:

[...]


Index: libvirt-acl/src/libvirt_private.syms
===
--- libvirt-acl.orig/src/libvirt_private.syms
+++ libvirt-acl/src/libvirt_private.syms
@@ -853,6 +853,14 @@ virNWFilterTestUnassignDef;
  virNWFilterUnlockFilterUpdates;


+# nwfilter_ipaddrmap
+virNWFilterIPAddrMapAddIPAddr;
+virNWFilterIPAddrMapDelIPAddr;
+virNWFilterIPAddrMapGetIPAddr;
+virNWFilterIPAddrMapInit;
+virNWFilterIPAddrMapShutdown;
+
+
  # nwfilter_params.h
  virNWFilterHashTableCreate;
  virNWFilterHashTableFree;

   Weird I would have expected some of the renamed/moved functions
to be removed from the libvirt_private.syms, I'm surprized to see
only additions :)


[...]

   ACK, but double check the syms, thanks,



There were none there before.

Regards,

Stefan

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


[libvirt] PXE boot issue with macvtap

2012-04-19 Thread Shradha Shah
Hello All,

I currently have an issue with using macvtap interfaces on KVM. 
I was wondering if this is a known issue?
Also may I request for any ideas on how I should go forward with this
issue?

Issue: 
When you try to PXE boot one KVM guest of another KVM guest over a macvtap 
interface
the client does not receive the tftp packets sent by the server and the
connection times out.

Setup:
Both the PXE client and the PXE server are VM's running on the same host,
using macvtap on top of a LOM interface for connectivity

Reproducible always.

Findings of the testing:
1) The issue is seen between guests on the same on the same host and not 
between 
guests on different hosts.

2) PXE booting works fine if we're using a linux bridge for connectivity rather
than macvtap

3) The initial DHCP stage works fine and the client acquires an IP address,
but then the TFTP attempt stalls.

4) The client sends an RRQ to which the server replies with option 
acknowledgement, which is exactly what should happen.

5) However the client appears not to receive the acknowledgement.

6) Both client and server keep retransmitting their packets for a while, until
the client times out and gives up.

7) Observing the ifconfig stats for the host macvtap interface for each guest,
the packet counters seemed to show that the packets were being received - so 
they
are dropped somewhere between macvtap on the host and the virtio device in the 
guest.

8) Tcpdump does not seem to work with macvtap so could not inspect further.  

-- 
Many Thanks,
Regards,
Shradha Shah

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


Re: [libvirt] [PATCH V11 3/7] nwfilter: Fix support for trusted DHCP servers

2012-04-19 Thread Daniel Veillard
On Thu, Apr 19, 2012 at 06:30:59AM -0400, Stefan Berger wrote:
 On 04/19/2012 05:27 AM, Daniel Veillard wrote:
 On Tue, Apr 17, 2012 at 10:44:04AM -0400, Stefan Berger wrote:
 Fix the support for trusted DHCP server in the ebtables code's
 hard-coded function applying DHCP only filtering rules:
 Rather than using a char * use the more flexible
 virNWFilterVarValuePtr that contains the trusted DHCP server(s)
 IP address. Process all entries.
 
 Since all callers so far provided NULL as parameter, no changes
 are necessary in any other code.
 
 [...]
 +while (true) {
 +char *srcIPParam = NULL;
 +
 +if (idx  num_dhcpsrvrs) {
 +const char *dhcpserver;
 +
 +dhcpserver = virNWFilterVarValueGetNthValue(dhcpsrvrs, idx);
 +
 +if (virAsprintf(srcIPParam, --ip-src %s, dhcpserver)  0) {
 +virReportOOMError();
 +goto tear_down_tmpebchains;
 +}
 +}
 +
 +virBufferAsprintf(buf,
 +  CMD_DEF($EBT -t nat -A %s
 +   -d %s
 +   -p ipv4 --ip-protocol udp
 +   %s
 +   --ip-sport 67 --ip-dport 68
 +   -j ACCEPT) CMD_SEPARATOR
 +  CMD_EXEC
 +  %s,
 +
 +  chain_out,
 +  macaddr_str,
 +  srcIPParam != NULL ? srcIPParam : ,
 +  CMD_STOPONERR(1));
 +
 +VIR_FREE(srcIPParam);
 +
 +if (idx == num_dhcpsrvrs)
 +break;
 +
 +idx++;
 +}
There is something I don't understand in that loop, you repetedly
 write to buf, but you don't seems to use buf in the loop. This looks
 fishy to me, or are you using side effect execution in the Asprintf
 argument evaluation. Too cryptic to my taste, I'm lost !
 
 I am accumulating (shell) commands in the buffer and then test the
 buffer for error before finally executing the commands.

  Okay, virBufferAsprintf contrary to virAsprintf accumulates the
  writes, makes sense now ...

Daniel

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

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


Re: [libvirt] [PATCH V11 5/7] nwfilter: move code for IP address map into separate file

2012-04-19 Thread Daniel Veillard
On Thu, Apr 19, 2012 at 06:51:46AM -0400, Stefan Berger wrote:
 On 04/19/2012 05:58 AM, Daniel Veillard wrote:
 On Tue, Apr 17, 2012 at 10:44:06AM -0400, Stefan Berger wrote:
 [...]
 
 Index: libvirt-acl/src/libvirt_private.syms
 ===
 --- libvirt-acl.orig/src/libvirt_private.syms
 +++ libvirt-acl/src/libvirt_private.syms
 @@ -853,6 +853,14 @@ virNWFilterTestUnassignDef;
   virNWFilterUnlockFilterUpdates;
 
 
 +# nwfilter_ipaddrmap
 +virNWFilterIPAddrMapAddIPAddr;
 +virNWFilterIPAddrMapDelIPAddr;
 +virNWFilterIPAddrMapGetIPAddr;
 +virNWFilterIPAddrMapInit;
 +virNWFilterIPAddrMapShutdown;
 +
 +
   # nwfilter_params.h
   virNWFilterHashTableCreate;
   virNWFilterHashTableFree;
Weird I would have expected some of the renamed/moved functions
 to be removed from the libvirt_private.syms, I'm surprized to see
 only additions :)
 
 [...]
ACK, but double check the syms, thanks,
 
 
 There were none there before.

  okay the functions were not exported then, thanks !

Daniel

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

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


[libvirt] [PATCH] storage: Allow multiple hosts for a storage pool

2012-04-19 Thread Wido den Hollander
The current storage pools for NFS and iSCSI only require one host to
connect to. Future storage pools like RBD and Sheepdog will require
multiple hosts.

This patch allows multiple source hosts and rewrites the current
storage drivers.

Signed-off-by: Wido den Hollander w...@widodh.nl
---
 src/conf/storage_conf.c |   59 --
 src/conf/storage_conf.h |5 ++-
 src/esx/esx_storage_driver.c|6 +++-
 src/storage/storage_backend_fs.c|   12 +++---
 src/storage/storage_backend_iscsi.c |   12 +++---
 src/test/test_driver.c  |4 +-
 6 files changed, 63 insertions(+), 35 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 2330fa1..aa23ff3 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -276,7 +276,11 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source)
 if (!source)
 return;
 
-VIR_FREE(source-host.name);
+for (i = 0 ; i  source-nhost ; i++) {
+VIR_FREE(source-hosts[i].name);
+}
+VIR_FREE(source-hosts);
+
 for (i = 0 ; i  source-ndevice ; i++) {
 VIR_FREE(source-devices[i].freeExtents);
 VIR_FREE(source-devices[i].path);
@@ -404,6 +408,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
 char *authType = NULL;
 int nsource, i;
 virStoragePoolOptionsPtr options;
+char *name = NULL;
 char *port = NULL;
 
 relnode = ctxt-node;
@@ -431,17 +436,34 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
 VIR_FREE(format);
 }
 
-source-host.name = virXPathString(string(./host/@name), ctxt);
-port = virXPathString(string(./host/@port), ctxt);
-if (port) {
-if (virStrToLong_i(port, NULL, 10, source-host.port)  0) {
-virStorageReportError(VIR_ERR_XML_ERROR,
-  _(Invalid port number: %s),
-  port);
+source-nhost = virXPathNodeSet(./host, ctxt, nodeset);
+
+if (source-nhost) {
+if (VIR_ALLOC_N(source-hosts, source-nhost)  0) {
+virReportOOMError();
 goto cleanup;
 }
-}
 
+for (i = 0 ; i  source-nhost ; i++) {
+name = virXMLPropString(nodeset[i], name);
+if (name == NULL) {
+virStorageReportError(VIR_ERR_XML_ERROR,
+%s, _(missing storage pool host name));
+goto cleanup;
+}
+source-hosts[i].name = name;
+
+port = virXMLPropString(nodeset[i], port);
+if (port) {
+if (virStrToLong_i(port, NULL, 10, source-hosts[i].port)  
0) {
+virStorageReportError(VIR_ERR_XML_ERROR,
+  _(Invalid port number: %s),
+  port);
+goto cleanup;
+}
+}
+}
+}
 
 source-initiator.iqn = virXPathString(string(./initiator/iqn/@name), 
ctxt);
 
@@ -674,7 +696,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) {
 }
 
 if (options-flags  VIR_STORAGE_POOL_SOURCE_HOST) {
-if (!ret-source.host.name) {
+if (!ret-source.nhost) {
 virStorageReportError(VIR_ERR_XML_ERROR,
   %s,
   _(missing storage pool source host name));
@@ -800,12 +822,13 @@ virStoragePoolSourceFormat(virBufferPtr buf,
 int i, j;
 
 virBufferAddLit(buf,  source\n);
-if ((options-flags  VIR_STORAGE_POOL_SOURCE_HOST) 
-src-host.name) {
-virBufferAsprintf(buf, host name='%s', src-host.name);
-if (src-host.port)
-virBufferAsprintf(buf,  port='%d', src-host.port);
-virBufferAddLit(buf, /\n);
+if ((options-flags  VIR_STORAGE_POOL_SOURCE_HOST)  src-nhost) {
+for (i = 0; i  src-nhost; i++) {
+virBufferAsprintf(buf,  host name='%s', src-hosts[i].name);
+if (src-hosts[i].port)
+virBufferAsprintf(buf,  port='%d', src-hosts[i].port);
+virBufferAddLit(buf, /\n);
+}
 }
 
 if ((options-flags  VIR_STORAGE_POOL_SOURCE_DEVICE) 
@@ -1677,7 +1700,7 @@ int 
virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
 break;
 case VIR_STORAGE_POOL_NETFS:
 if ((STREQ(pool-def-source.dir, def-source.dir)) \
- (STREQ(pool-def-source.host.name, def-source.host.name)))
+ (STREQ(pool-def-source.hosts[0].name, 
def-source.hosts[0].name)))
 matchpool = pool;
 break;
 case VIR_STORAGE_POOL_SCSI:
@@ -1688,7 +1711,7 @@ int 
virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
 {
 matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def);
 if (matchpool) {
-if (STREQ(matchpool-def-source.host.name, 
def-source.host.name)) {

Re: [libvirt] [PATCH] virsh: Fix and clarify the --title flag for the list command in man page

2012-04-19 Thread Peter Krempa

On 04/19/2012 12:01 PM, Osier Yang wrote:

On 2012年04月19日 17:03, Peter Krempa wrote:

---
tools/virsh.pod | 9 +
1 files changed, 5 insertions(+), 4 deletions(-)







ACK. But how about short domain description (title)?


That looks better! I changed the hyphens to brackets, added a short note 
that the title is printed in a new column of the output and pushed


Thanks.

Peter



Osier



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

Re: [libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Michal Privoznik
On 19.04.2012 08:25, Chen Hanxiao wrote:
 From: Chen Hanxiao chenhanx...@cn.fujitsu.com
 
 When adding a large number of virtio storage devices to
 virtual machine by using 'virsh edit' command, there is
 a problem:
 When we added more than 190 virtio disks by 'virsh edit'
 command, we got a feedback as 'error: Unable to encode
 message payload'. In virt-mananger, the same defect occurs
 with about 180 virtio disks added.
 
 PCI devices are limited by the virtualized system
 architecture. Out of the 32 available PCI devices for a
 guest, 4 are not removable. This means there are up to
 28 free PCI slots available for additional devices per
 guest. Each PCI device in a guest can have up to 8 functions.
 One slot kept for network interface, we could add at most
 216 (27*8) virtio disks.
 The length of xml description with multifuction for one virtio
 disk is about 290 characters; In virt-manger, an extra tag
 'alias name' will come to the xml description, which would add
 about 40 more characters.
 So for one xml description, 330 characters would be enough.
 A brand new virtual machine with one IDE bus disk needs about
 1900 characters for xml description.
 
 In src/remote/remote_protocol.x, there is a limitation for XDR
 enoding length as REMOTE_STRING_MAX.
 According to the analysis above, at least 73180(1900+216*330)
 characters are needed for 216 virtio disks. In the view of
 variable length in tag 'source file', so I think it would be
 better to extend this limitation from 65536 to 8.
 ---
  src/remote/remote_protocol.x |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
 index 2d57247..2c8dcbb 100644
 --- a/src/remote/remote_protocol.x
 +++ b/src/remote/remote_protocol.x
 @@ -65,7 +65,7 @@
   * This is an arbitrary limit designed to stop the decoder from trying
   * to allocate unbounded amounts of memory when fed with a bad message.
   */
 -const REMOTE_STRING_MAX = 65536;
 +const REMOTE_STRING_MAX = 8;
  
  /* A long string, which may NOT be NULL. */
  typedef string remote_nonnull_stringREMOTE_STRING_MAX;

While this limit is pretty harmless, the much bigger issue is - a per
message buffer of VIR_NET_MESSAGE_MAX bytes which is hold during whole
API. I am writing a code to allocate this buffer dynamically based on
message length.

btw: don't you want size up VIR_NET_MESSAGE_STRING_MAX in
virnetprotocol.x as well?

Michal

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


Re: [libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Richard W.M. Jones
On Thu, Apr 19, 2012 at 02:25:16PM +0800, Chen Hanxiao wrote:
   * This is an arbitrary limit designed to stop the decoder from trying
   * to allocate unbounded amounts of memory when fed with a bad message.
   */
 -const REMOTE_STRING_MAX = 65536;
 +const REMOTE_STRING_MAX = 8;

Can this limit be changed?  I thought it would break existing clients
or servers.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

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


Re: [libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Michal Privoznik
On 19.04.2012 14:45, Richard W.M. Jones wrote:
 On Thu, Apr 19, 2012 at 02:25:16PM +0800, Chen Hanxiao wrote:
   * This is an arbitrary limit designed to stop the decoder from trying
   * to allocate unbounded amounts of memory when fed with a bad message.
   */
 -const REMOTE_STRING_MAX = 65536;
 +const REMOTE_STRING_MAX = 8;
 
 Can this limit be changed?  I thought it would break existing clients
 or servers.
 
 Rich.
 

Yes  no. There are 4 possibilities:

- old both server  client: long string get dropped at server side and
unable to encode string/payload error is thrown.

- old server  new client: if server wants to send long string it's the
previous case; client will successfully retransmit message to the daemon
where it gets later dropped because of limit violation.

- new server  old client: see previous case

- new server  new client: nothing gets broken, everything works.

The only problem is with these gimme-list-of-* APIs. Because here you'll
get only partial result (first N entries - based on RPC limit).

Thus, changing this limit is okay.

Michal

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


Re: [libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Richard W.M. Jones
On Thu, Apr 19, 2012 at 03:04:09PM +0200, Michal Privoznik wrote:
 On 19.04.2012 14:45, Richard W.M. Jones wrote:
  On Thu, Apr 19, 2012 at 02:25:16PM +0800, Chen Hanxiao wrote:
* This is an arbitrary limit designed to stop the decoder from trying
* to allocate unbounded amounts of memory when fed with a bad message.
*/
  -const REMOTE_STRING_MAX = 65536;
  +const REMOTE_STRING_MAX = 8;
  
  Can this limit be changed?  I thought it would break existing clients
  or servers.
  
  Rich.
  
 
 Yes  no. There are 4 possibilities:
 
 - old both server  client: long string get dropped at server side and
 unable to encode string/payload error is thrown.
 
 - old server  new client: if server wants to send long string it's the
 previous case; client will successfully retransmit message to the daemon
 where it gets later dropped because of limit violation.
 
 - new server  old client: see previous case
 
 - new server  new client: nothing gets broken, everything works.
 
 The only problem is with these gimme-list-of-* APIs. Because here you'll
 get only partial result (first N entries - based on RPC limit).
 
 Thus, changing this limit is okay.

If we're going to change it, I say change it to something much bigger.

The original limit was very conservative, and was based on some
assumptions about wanting to keep buffers on the stack which is no
longer true in modern libvirt code.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/

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


Re: [libvirt] [libvirt-glib] Add async variant of gvir_domain_get_info()

2012-04-19 Thread Zeeshan Ali (Khattak)
On Thu, Apr 19, 2012 at 11:52 AM, Christophe Fergeau
cferg...@redhat.com wrote:
 On Thu, Apr 19, 2012 at 03:12:01AM +0300, Zeeshan Ali (Khattak) wrote:
 From: Zeeshan Ali (Khattak) zeesha...@gnome.org
 +static void
 +gvir_domain_get_info_helper(GSimpleAsyncResult *res,
 +                            GObject *object,
 +                            GCancellable *cancellable G_GNUC_UNUSED)
 +{
 +    GVirDomain *dom = GVIR_DOMAIN(object);
 +    GVirDomainInfo *info;
 +    GError *err = NULL;
 +
 +    info = gvir_domain_get_info(dom, err);
 +    if (err)
 +        g_simple_async_result_take_error(res, err);
 +    else
 +        g_simple_async_result_set_op_res_gpointer(res, info, NULL);

 Shouldn't the last parameter be gvir_domain_info_free?

No, we give this info to user in the finalize call:

 +
 +    if (g_simple_async_result_propagate_error(res, err))
 +        return NULL;
 +
 +    return g_simple_async_result_get_op_res_gpointer(res);
 +}

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

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

Re: [libvirt] Making the Thread-safety issues with vbox driver ?

2012-04-19 Thread Daniel P. Berrange
On Thu, Apr 19, 2012 at 12:51:10PM +0200, Jean-Baptiste Rouault wrote:
 On Wednesday 04 April 2012 17:56:12 Jean-Baptiste Rouault wrote:
  On Monday 16 January 2012 11:34:53 Matthias Bolte wrote:
   Okay, without looking deeper into this here are some ideas:
   
   The XPCOM API libvirt uses might not be threadsafe, or needs to be
   initialized for every thread that wants to use it. Currently its only
   initialized for the thread that opens the driver. I know that this is
   the case on Windows were VirtualBox uses MSCOM for its API and you
   need to call CoInitialize on every thread. This is currently not done
   for the MSCOM glue in libvirt, so I know that on Windows the
   VirtualBox driver is not threadsafe currently. Also I didn't look into
   a solution for this yet. Maybe we need a thread local variable that
   holds whether (MS/XP)COM was already initialized for this thread and
   add a check to every driver function to initialize it when needed.
   
   Did you try to open a connection for each thread instead of trying to
   share one? If that works reliable it might indicate that there is an
   VirtualBox API initialization problem.
  
  I tried today with one connection for each thread and it works.
  
  I changed the vbox driver so that the pfnComInitialize function is called
  only when the first connection is opened : it breaks the test, even with
  one connection per thread.
  
  I guess we'll have to use a thread local variable as you suggested, unless
  someone has a better idea to handle this problem.
 
 Hi,
 
 I looked deeper into these thread-safety issues, once a new connection is 
 opened for each thread, everything works well.
 However, opening and closing connections isn't thread-safe at all for two 
 reasons :
 
 - VirtualBox C bindings initialization and uninitialization functions aren't 
 thread-safe. I talked about it with upstream on IRC and they are probably not 
 going to fix it, but would accept a patch fixing the issue. I'm going to 
 contact 
 upstream again to get some advices so I can write a patch.
 
 - In the libvirt vbox driver, for each new connection, modification of the 
 global variable g_pVBoxGlobalData isn't protected (see line 1040 of 
 vbox_tmpl.c). 
 
 First of all, is it really necessary to replace it on each new connection, or 
 would it be ok to initialize it only when the first connection is opened ?
 
 A global mutex is needed in the vbox driver to protect access to 
 g_pVBoxGlobalData, the vboxRegister() function seems to be the best place to 
 initialize such a mutex unless there is another entry point to do this ?

I can't comment on where the best place to put hte lock is, but I agree
that we should make sure we do all the locking in libvirt. Not only because
upstream isnt likely to make it threadsafe soon, but also because it will
ensure we have compatibilty  safety with all existing/previous releases
or VirtualBox

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] storage: Allow multiple hosts for a storage pool

2012-04-19 Thread Daniel P. Berrange
On Thu, Apr 19, 2012 at 02:33:27PM +0200, Wido den Hollander wrote:
 The current storage pools for NFS and iSCSI only require one host to
 connect to. Future storage pools like RBD and Sheepdog will require
 multiple hosts.
 
 This patch allows multiple source hosts and rewrites the current
 storage drivers.
 
 Signed-off-by: Wido den Hollander w...@widodh.nl
 ---
  src/conf/storage_conf.c |   59 --
  src/conf/storage_conf.h |5 ++-
  src/esx/esx_storage_driver.c|6 +++-
  src/storage/storage_backend_fs.c|   12 +++---
  src/storage/storage_backend_iscsi.c |   12 +++---
  src/test/test_driver.c  |4 +-
  6 files changed, 63 insertions(+), 35 deletions(-)
 

 @@ -1677,7 +1700,7 @@ int 
 virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
  break;
  case VIR_STORAGE_POOL_NETFS:
  if ((STREQ(pool-def-source.dir, def-source.dir)) \
 - (STREQ(pool-def-source.host.name, 
 def-source.host.name)))
 + (STREQ(pool-def-source.hosts[0].name, 
 def-source.hosts[0].name)))
  matchpool = pool;
  break;
  case VIR_STORAGE_POOL_SCSI:

Here we need to add to the conditional check nhosts == 1  before 
dereferencing hosts[0].


 @@ -1688,7 +1711,7 @@ int 
 virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
  {
  matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def);
  if (matchpool) {
 -if (STREQ(matchpool-def-source.host.name, 
 def-source.host.name)) {
 +if (STREQ(matchpool-def-source.hosts[0].name, 
 def-source.hosts[0].name)) {
  if ((matchpool-def-source.initiator.iqn)  
 (def-source.initiator.iqn)) {
  if (STREQ(matchpool-def-source.initiator.iqn, 
 def-source.initiator.iqn))
  break;

Same here.

 diff --git a/src/storage/storage_backend_fs.c 
 b/src/storage/storage_backend_fs.c
 index 1af12e6..79eefd3 100644
 --- a/src/storage/storage_backend_fs.c
 +++ b/src/storage/storage_backend_fs.c
 @@ -206,7 +206,7 @@ 
 virStorageBackendFileSystemNetFindPoolSourcesFunc(virStoragePoolObjPtr pool 
 ATTR
  if (!(src = virStoragePoolSourceListNewSource(state-list)))
  goto cleanup;

At the start of this function we need to have

   if (src-nhosts != 1) {
  virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s
Expected exactly 1 host for the filesystem pool));
  goto cleanup;
   }

And similarly at the start of other functions which are referencing
src-hosts[0]


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


[libvirt] Call for Proposals: Linux Plumbers Conference (Aug 2012)

2012-04-19 Thread Amit Shah
Hello,

The Call for Proposals for the Linux Plumbers Conf 2012 is out.  We're
looking for speakers to talk at the Virtualization microconference as
well as the main conference.  The deadline for proposal submissions is
1st May.  This year's edition of LPC is co-located with LinuxCon NA
and will be held at San Diego from the 29th to the 31st of August.

More details are at:

http://www.linuxplumbersconf.org/2012/2012-lpc-call-for-proposals/

Please note: to submit a proposal for the virt microconf, use the
'lpc2012-virt-' prefix in the Name field.

LPC is oriented towards solving problems, and good proposal topics are
those which are unresolved problems or proposals that need interaction
with multiple subsystems.

Please see the CFP page linked above for more details.

Thanks,

Amit

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


[libvirt] [libvirt-glib] Add async variant of gvir_domain_get_info()

2012-04-19 Thread Zeeshan Ali (Khattak)
From: Zeeshan Ali (Khattak) zeesha...@gnome.org

---
 libvirt-gobject/libvirt-gobject-domain.c |   74 ++
 libvirt-gobject/libvirt-gobject-domain.h |7 +++
 libvirt-gobject/libvirt-gobject.sym  |4 +-
 3 files changed, 84 insertions(+), 1 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index 0bafa7e..c1a2086 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -572,6 +572,80 @@ GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom,
 return ret;
 }
 
+static void
+gvir_domain_get_info_helper(GSimpleAsyncResult *res,
+GObject *object,
+GCancellable *cancellable G_GNUC_UNUSED)
+{
+GVirDomain *dom = GVIR_DOMAIN(object);
+GVirDomainInfo *info;
+GError *err = NULL;
+
+info = gvir_domain_get_info(dom, err);
+if (err)
+g_simple_async_result_take_error(res, err);
+else
+g_simple_async_result_set_op_res_gpointer(res, info, NULL);
+}
+
+/**
+ * gvir_domain_get_info_async:
+ * @dom: the domain
+ * @cancellable: (allow-none)(transfer none): cancellation object
+ * @callback: (scope async): completion callback
+ * @user_data: (closure): opaque data for callback
+ *
+ * Asynchronous variant of #gvir_domain_get_info.
+ */
+void gvir_domain_get_info_async(GVirDomain *dom,
+GCancellable *cancellable,
+GAsyncReadyCallback callback,
+gpointer user_data)
+{
+GSimpleAsyncResult *res;
+
+g_return_if_fail(GVIR_IS_DOMAIN(dom));
+
+res = g_simple_async_result_new(G_OBJECT(dom),
+callback,
+user_data,
+gvir_domain_get_info_async);
+g_simple_async_result_run_in_thread(res,
+gvir_domain_get_info_helper,
+G_PRIORITY_DEFAULT,
+cancellable);
+g_object_unref(res);
+}
+
+/**
+ * gvir_domain_get_info_finish:
+ * @dom: the domain
+ * @result: (transfer none): async method result
+ * @err: Place-holder for possible errors
+ *
+ * Finishes the operation started by #gvir_domain_get_info_async.
+ *
+ * Returns: (transfer full): the info
+ */
+GVirDomainInfo *gvir_domain_get_info_finish(GVirDomain *dom,
+GAsyncResult *result,
+GError **err)
+{
+GSimpleAsyncResult *res = G_SIMPLE_ASYNC_RESULT(result);
+
+g_return_val_if_fail(GVIR_IS_DOMAIN(dom), NULL);
+g_return_val_if_fail
+(g_simple_async_result_is_valid(result,
+G_OBJECT(dom),
+gvir_domain_get_info_async),
+ NULL);
+
+if (g_simple_async_result_propagate_error(res, err))
+return NULL;
+
+return g_simple_async_result_get_op_res_gpointer(res);
+}
+
 /**
  * gvir_domain_screenshot:
  * @stream: stream to use as output
diff --git a/libvirt-gobject/libvirt-gobject-domain.h 
b/libvirt-gobject/libvirt-gobject-domain.h
index 8a4836e..677fbe6 100644
--- a/libvirt-gobject/libvirt-gobject-domain.h
+++ b/libvirt-gobject/libvirt-gobject-domain.h
@@ -141,6 +141,13 @@ gboolean gvir_domain_reboot(GVirDomain *dom,
 
 GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom,
  GError **err);
+void gvir_domain_get_info_async(GVirDomain *dom,
+GCancellable *cancellable,
+GAsyncReadyCallback callback,
+gpointer user_data);
+GVirDomainInfo *gvir_domain_get_info_finish(GVirDomain *dom,
+GAsyncResult *result,
+GError **err);
 
 GVirConfigDomain *gvir_domain_get_config(GVirDomain *dom,
  guint flags,
diff --git a/libvirt-gobject/libvirt-gobject.sym 
b/libvirt-gobject/libvirt-gobject.sym
index 64c91cc..f43836f 100644
--- a/libvirt-gobject/libvirt-gobject.sym
+++ b/libvirt-gobject/libvirt-gobject.sym
@@ -1,4 +1,4 @@
-LIBVIRT_GOBJECT_0.0.7 {
+LIBVIRT_GOBJECT_0.0.8 {
   global:
 gvir_init_object;
 gvir_init_object_check;
@@ -70,6 +70,8 @@ LIBVIRT_GOBJECT_0.0.7 {
gvir_domain_set_config;
gvir_domain_get_devices;
gvir_domain_get_info;
+   gvir_domain_get_info_async;
+   gvir_domain_get_info_finish;
gvir_domain_get_persistent;
gvir_domain_get_saved;
gvir_domain_screenshot;
-- 
1.7.7.6

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


Re: [libvirt] [PATCH] cpu: Improve error reporting on incompatible CPUs

2012-04-19 Thread Jiri Denemark
On Wed, Apr 18, 2012 at 15:19:53 +0200, Peter Krempa wrote:
 This patch modifies the CPU comparrison function to report the
 incompatibilities in more detail to ease identification of problems.
 
 * src/cpu/cpu.h:
 cpuGuestData(): Add argument to return detailed error message.
 * src/cpu/cpu.c:
 cpuGuestData(): Add passthrough for error argument.
 * src/cpu/cpu_x86.c
 x86FeatureNames(): Add function to convert a CPU definition to flag
names.
 x86Compute(): - Add error message parameter
   - Add macro for reporting detailed error messages.
   - Improve error reporting.
   - Simplify calculation of forbidden flags.
 x86DataIteratorInit():
 x86cpuidMatchAny(): Remove functions that are no longer needed.
 * src/qemu/qemu_command.c:
 qemuBuildCpuArgStr(): - Modify for new function prototype
   - Add detailed error reports
   - Change error code on incompatible processors
 to VIR_ERR_CONFIG_UNSUPPORTED instead of
 internal error
 * tests/cputest.c:
 cpuTestGuestData(): Modify for new function prototype
 ---
 Sample of error message:
 $ virsh start Bare
 error: Failed to start domain Bare
 error: unsupported configuration: guest and host CPU are not compatible: Host 
 CPU does not provide required features: svm, avx
 
 
  src/cpu/cpu.c   |7 ++-
  src/cpu/cpu.h   |6 ++-
  src/cpu/cpu_x86.c   |  123 
 +++
  src/qemu/qemu_command.c |   12 -
  tests/cputest.c |2 +-
  5 files changed, 100 insertions(+), 50 deletions(-)
 
 diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
 index 01c31bb..b8ccd24 100644
 --- a/src/cpu/cpu.c
 +++ b/src/cpu/cpu.c
 @@ -248,11 +248,12 @@ cpuNodeData(const char *arch)
  virCPUCompareResult
  cpuGuestData(virCPUDefPtr host,
   virCPUDefPtr guest,
 - union cpuData **data)
 + union cpuData **data,
 + char **msg)
  {
  struct cpuArchDriver *driver;
 
 -VIR_DEBUG(host=%p, guest=%p, data=%p, host, guest, data);
 +VIR_DEBUG(host=%p, guest=%p, data=%p, msg=%p, host, guest, data, msg);
 
  if ((driver = cpuGetSubDriver(host-arch)) == NULL)
  return VIR_CPU_COMPARE_ERROR;
 @@ -264,7 +265,7 @@ cpuGuestData(virCPUDefPtr host,
  return VIR_CPU_COMPARE_ERROR;
  }
 
 -return driver-guestData(host, guest, data);
 +return driver-guestData(host, guest, data, msg);
  }
 
 
 diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
 index 9f01f17..d7bc54e 100644
 --- a/src/cpu/cpu.h
 +++ b/src/cpu/cpu.h
 @@ -70,7 +70,8 @@ typedef union cpuData *
  typedef virCPUCompareResult
  (*cpuArchGuestData) (virCPUDefPtr host,
   virCPUDefPtr guest,
 - union cpuData **data);
 + union cpuData **data,
 + char **message);
 
  typedef virCPUDefPtr
  (*cpuArchBaseline)  (virCPUDefPtr *cpus,
 @@ -138,7 +139,8 @@ cpuNodeData (const char *arch);
  extern virCPUCompareResult
  cpuGuestData(virCPUDefPtr host,
   virCPUDefPtr guest,
 - union cpuData **data);
 + union cpuData **data,
 + char **msg);
 
  extern char *
  cpuBaselineXML(const char **xmlCPUs,
 diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
 index 8d92649..e1500bf 100644
 --- a/src/cpu/cpu_x86.c
 +++ b/src/cpu/cpu_x86.c
 @@ -31,6 +31,7 @@
  #include cpu.h
  #include cpu_map.h
  #include cpu_x86.h
 +#include buf.h
 
 
  #define VIR_FROM_THIS VIR_FROM_CPU
 @@ -89,16 +90,6 @@ struct data_iterator {
  { data, -1, false }
 
 
 -static void
 -x86DataIteratorInit(struct data_iterator *iter,
 -union cpuData *data)
 -{
 -struct data_iterator init = DATA_ITERATOR_INIT(data);
 -
 -*iter = init;
 -}
 -
 -
  static int
  x86cpuidMatch(const struct cpuX86cpuid *cpuid1,
const struct cpuX86cpuid *cpuid2)
 @@ -121,17 +112,6 @@ x86cpuidMatchMasked(const struct cpuX86cpuid *cpuid,
  }
 
 
 -static int
 -x86cpuidMatchAny(const struct cpuX86cpuid *cpuid,
 - const struct cpuX86cpuid *mask)
 -{
 -return ((cpuid-eax  mask-eax) ||
 -(cpuid-ebx  mask-ebx) ||
 -(cpuid-ecx  mask-ecx) ||
 -(cpuid-edx  mask-edx));
 -}
 -
 -
  static void
  x86cpuidSetBits(struct cpuX86cpuid *cpuid,
  const struct cpuX86cpuid *mask)
 @@ -649,6 +629,34 @@ x86FeatureFind(const struct x86_map *map,
  }
 
 
 +static char *
 +x86FeatureNames(const struct x86_map *map,
 +const char *separator,
 +union cpuData *data)
 +{
 +virBuffer ret = VIR_BUFFER_INITIALIZER;
 +bool first = true;
 +
 +struct x86_feature *next_feature = map-features;
 +
 +virBufferAdd(ret, , 0);
 +
 +while (next_feature) {
 +if (x86DataIsSubset(data, next_feature-data)) {
 +if 

Re: [libvirt] [libvirt-glib] Correct some minor typos in docs

2012-04-19 Thread Daniel P. Berrange
On Thu, Apr 19, 2012 at 04:46:20PM +0300, Zeeshan Ali (Khattak) wrote:
 From: Zeeshan Ali (Khattak) zeesha...@gnome.org
 
 ---
  libvirt-gobject/libvirt-gobject-domain.c |6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
 b/libvirt-gobject/libvirt-gobject-domain.c
 index c1a2086..7fcd733 100644
 --- a/libvirt-gobject/libvirt-gobject-domain.c
 +++ b/libvirt-gobject/libvirt-gobject-domain.c
 @@ -775,7 +775,7 @@ cleanup:
  }
  
  /**
 - * gir_domain_suspend:
 + * gvir_domain_suspend:
   * @dom: the domain to suspend
   * @err: Place-holder for possible errors
   *
 @@ -857,7 +857,7 @@ gvir_domain_save_helper(GSimpleAsyncResult *res,
  }
  
  /**
 - * gir_domain_save_async:
 + * gvir_domain_save_async:
   * @dom: the domain to save
   * @flags: extra flags, currently unused
   * @cancellable: (allow-none)(transfer none): cancellation object
 @@ -893,7 +893,7 @@ void gvir_domain_save_async (GVirDomain *dom,
  }
  
  /**
 - * gir_domain_save_finish:
 + * gvir_domain_save_finish:
   * @dom: the domain to save
   * @result: (transfer none): async method result
   * @err: Place-holder for possible errors

ACK,

BTW, I consider that obvious docs typos like this can be pushed without
needing an explicit ACK under the so called trivial rule.


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] Add async variant of gvir_domain_get_info()

2012-04-19 Thread Christophe Fergeau
On Thu, Apr 19, 2012 at 04:24:35PM +0300, Zeeshan Ali (Khattak) wrote:
 On Thu, Apr 19, 2012 at 11:52 AM, Christophe Fergeau
 cferg...@redhat.com wrote:
  On Thu, Apr 19, 2012 at 03:12:01AM +0300, Zeeshan Ali (Khattak) wrote:
  From: Zeeshan Ali (Khattak) zeesha...@gnome.org
  +static void
  +gvir_domain_get_info_helper(GSimpleAsyncResult *res,
  +                            GObject *object,
  +                            GCancellable *cancellable G_GNUC_UNUSED)
  +{
  +    GVirDomain *dom = GVIR_DOMAIN(object);
  +    GVirDomainInfo *info;
  +    GError *err = NULL;
  +
  +    info = gvir_domain_get_info(dom, err);
  +    if (err)
  +        g_simple_async_result_take_error(res, err);
  +    else
  +        g_simple_async_result_set_op_res_gpointer(res, info, NULL);
 
  Shouldn't the last parameter be gvir_domain_info_free?
 
 No, we give this info to user in the finalize call:

Looking at GSimpleAsyncResult documentation and use, it seems customary to pass 
a
GDestroyNotify function here, and to ref the returned object in the _finish
function if needed. It seems it's perfectly valid to call
gvir_domain_get_info_async() with a NULL callback, or not to call _finish
if you are not interested in the result. If you don't pass a non-NULL
GDestroyNotify to set_op_res_gpointer, I don't see how you can avoid a
memory leak in these cases. Think also of an error (cancellation) occurring
between the call to _helper and the call to finish.

Christophe


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

Re: [libvirt] [libvirt-glib] Add async variant of gvir_domain_get_info()

2012-04-19 Thread Christophe Fergeau
On Thu, Apr 19, 2012 at 04:45:29PM +0300, Zeeshan Ali (Khattak) wrote:
 From: Zeeshan Ali (Khattak) zeesha...@gnome.org
 
 ---
  libvirt-gobject/libvirt-gobject-domain.c |   74 
 ++
  libvirt-gobject/libvirt-gobject-domain.h |7 +++
  libvirt-gobject/libvirt-gobject.sym  |4 +-
  3 files changed, 84 insertions(+), 1 deletions(-)
 
 diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
 b/libvirt-gobject/libvirt-gobject-domain.c
 index 0bafa7e..c1a2086 100644
 --- a/libvirt-gobject/libvirt-gobject-domain.c
 +++ b/libvirt-gobject/libvirt-gobject-domain.c
 @@ -572,6 +572,80 @@ GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom,
  return ret;
  }
  
 +static void
 +gvir_domain_get_info_helper(GSimpleAsyncResult *res,
 +GObject *object,
 +GCancellable *cancellable G_GNUC_UNUSED)
 +{
 +GVirDomain *dom = GVIR_DOMAIN(object);
 +GVirDomainInfo *info;
 +GError *err = NULL;
 +
 +info = gvir_domain_get_info(dom, err);
 +if (err)
 +g_simple_async_result_take_error(res, err);
 +else
 +g_simple_async_result_set_op_res_gpointer(res, info, NULL);

I still think this can leak memory in error cases/corner cases, see my answer to
your first patch.

Christophe


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

[libvirt] [libvirt-glib] Add async variant of gvir_domain_get_info()

2012-04-19 Thread Zeeshan Ali (Khattak)
From: Zeeshan Ali (Khattak) zeesha...@gnome.org

---
 libvirt-gobject/libvirt-gobject-domain.c |   80 ++
 libvirt-gobject/libvirt-gobject-domain.h |7 +++
 libvirt-gobject/libvirt-gobject.sym  |4 +-
 3 files changed, 90 insertions(+), 1 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index 350a5b0..67ca257 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -572,6 +572,86 @@ GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom,
 return ret;
 }
 
+static void
+gvir_domain_get_info_helper(GSimpleAsyncResult *res,
+GObject *object,
+GCancellable *cancellable G_GNUC_UNUSED)
+{
+GVirDomain *dom = GVIR_DOMAIN(object);
+GVirDomainInfo *info;
+GError *err = NULL;
+
+info = gvir_domain_get_info(dom, err);
+if (err)
+g_simple_async_result_take_error(res, err);
+else
+g_simple_async_result_set_op_res_gpointer
+(res,
+ info,
+ (GDestroyNotify) gvir_domain_info_free);
+}
+
+/**
+ * gvir_domain_get_info_async:
+ * @dom: the domain
+ * @cancellable: (allow-none)(transfer none): cancellation object
+ * @callback: (scope async): completion callback
+ * @user_data: (closure): opaque data for callback
+ *
+ * Asynchronous variant of #gvir_domain_get_info.
+ */
+void gvir_domain_get_info_async(GVirDomain *dom,
+GCancellable *cancellable,
+GAsyncReadyCallback callback,
+gpointer user_data)
+{
+GSimpleAsyncResult *res;
+
+g_return_if_fail(GVIR_IS_DOMAIN(dom));
+
+res = g_simple_async_result_new(G_OBJECT(dom),
+callback,
+user_data,
+gvir_domain_get_info_async);
+g_simple_async_result_run_in_thread(res,
+gvir_domain_get_info_helper,
+G_PRIORITY_DEFAULT,
+cancellable);
+g_object_unref(res);
+}
+
+/**
+ * gvir_domain_get_info_finish:
+ * @dom: the domain
+ * @result: (transfer none): async method result
+ * @err: Place-holder for possible errors
+ *
+ * Finishes the operation started by #gvir_domain_get_info_async.
+ *
+ * Returns: (transfer full): the info
+ */
+GVirDomainInfo *gvir_domain_get_info_finish(GVirDomain *dom,
+GAsyncResult *result,
+GError **err)
+{
+GSimpleAsyncResult *res = G_SIMPLE_ASYNC_RESULT(result);
+GVirDomainInfo *ret;
+
+g_return_val_if_fail(GVIR_IS_DOMAIN(dom), NULL);
+g_return_val_if_fail
+(g_simple_async_result_is_valid(result,
+G_OBJECT(dom),
+gvir_domain_get_info_async),
+ NULL);
+
+if (g_simple_async_result_propagate_error(res, err))
+return NULL;
+
+ret = g_simple_async_result_get_op_res_gpointer(res);
+
+return gvir_domain_info_copy (ret);
+}
+
 /**
  * gvir_domain_screenshot:
  * @stream: stream to use as output
diff --git a/libvirt-gobject/libvirt-gobject-domain.h 
b/libvirt-gobject/libvirt-gobject-domain.h
index 8a4836e..677fbe6 100644
--- a/libvirt-gobject/libvirt-gobject-domain.h
+++ b/libvirt-gobject/libvirt-gobject-domain.h
@@ -141,6 +141,13 @@ gboolean gvir_domain_reboot(GVirDomain *dom,
 
 GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom,
  GError **err);
+void gvir_domain_get_info_async(GVirDomain *dom,
+GCancellable *cancellable,
+GAsyncReadyCallback callback,
+gpointer user_data);
+GVirDomainInfo *gvir_domain_get_info_finish(GVirDomain *dom,
+GAsyncResult *result,
+GError **err);
 
 GVirConfigDomain *gvir_domain_get_config(GVirDomain *dom,
  guint flags,
diff --git a/libvirt-gobject/libvirt-gobject.sym 
b/libvirt-gobject/libvirt-gobject.sym
index 64c91cc..f43836f 100644
--- a/libvirt-gobject/libvirt-gobject.sym
+++ b/libvirt-gobject/libvirt-gobject.sym
@@ -1,4 +1,4 @@
-LIBVIRT_GOBJECT_0.0.7 {
+LIBVIRT_GOBJECT_0.0.8 {
   global:
 gvir_init_object;
 gvir_init_object_check;
@@ -70,6 +70,8 @@ LIBVIRT_GOBJECT_0.0.7 {
gvir_domain_set_config;
gvir_domain_get_devices;
gvir_domain_get_info;
+   gvir_domain_get_info_async;
+   gvir_domain_get_info_finish;
gvir_domain_get_persistent;
gvir_domain_get_saved;
gvir_domain_screenshot;
-- 

Re: [libvirt] [PATCH 0/5] -no-user-config option, move CPU models to /usr/share

2012-04-19 Thread Jiri Denemark
On Wed, Apr 18, 2012 at 17:02:35 -0300, Eduardo Habkost wrote:
 This is the first try of the new -no-user-config option.
 
 Patches 1 to 3 just move some code around, patch 4 just adds the new option
 without adding any new config file. Patch 5 finally creates a /usr/share/qemu
 /cpus-x86_64.conf file, with the CPU models we currently have on Qemu.
 
 Reference to previous discussion:
  - http://marc.info/?l=qemu-develm=133278877315665
 
 This series depends on the following:
   Subject: [PATCH v5 00/14] configure: --with-confsuffix option
   Message-Id: 1334778950-18660-1-git-send-email-ehabk...@redhat.com
 
 
 Eduardo Habkost (5):
   move code to read default config files to a separate function
   eliminate arch_config_name variable
   move list of default config files to an array
   implement -no-user-config command-line option
   move CPU definitions to /usr/share/qemu/cpus-x86_64.conf

This is a huge improvement from libvirt point of view. Thanks Eduadro. I'll
prepare corresponding libvirt patches.

Jirka

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


Re: [libvirt] Making the Thread-safety issues with vbox driver ?

2012-04-19 Thread Matthias Bolte
Am 19. April 2012 12:51 schrieb Jean-Baptiste Rouault
jean-baptiste.roua...@diateam.net:
 On Wednesday 04 April 2012 17:56:12 Jean-Baptiste Rouault wrote:
 On Monday 16 January 2012 11:34:53 Matthias Bolte wrote:
  Okay, without looking deeper into this here are some ideas:
 
  The XPCOM API libvirt uses might not be threadsafe, or needs to be
  initialized for every thread that wants to use it. Currently its only
  initialized for the thread that opens the driver. I know that this is
  the case on Windows were VirtualBox uses MSCOM for its API and you
  need to call CoInitialize on every thread. This is currently not done
  for the MSCOM glue in libvirt, so I know that on Windows the
  VirtualBox driver is not threadsafe currently. Also I didn't look into
  a solution for this yet. Maybe we need a thread local variable that
  holds whether (MS/XP)COM was already initialized for this thread and
  add a check to every driver function to initialize it when needed.
 
  Did you try to open a connection for each thread instead of trying to
  share one? If that works reliable it might indicate that there is an
  VirtualBox API initialization problem.

 I tried today with one connection for each thread and it works.

 I changed the vbox driver so that the pfnComInitialize function is called
 only when the first connection is opened : it breaks the test, even with
 one connection per thread.

 I guess we'll have to use a thread local variable as you suggested, unless
 someone has a better idea to handle this problem.

 Hi,

 I looked deeper into these thread-safety issues, once a new connection is
 opened for each thread, everything works well.
 However, opening and closing connections isn't thread-safe at all for two
 reasons :

 - VirtualBox C bindings initialization and uninitialization functions aren't
 thread-safe. I talked about it with upstream on IRC and they are probably not
 going to fix it, but would accept a patch fixing the issue. I'm going to 
 contact
 upstream again to get some advices so I can write a patch.

 - In the libvirt vbox driver, for each new connection, modification of the
 global variable g_pVBoxGlobalData isn't protected (see line 1040 of
 vbox_tmpl.c).

 First of all, is it really necessary to replace it on each new connection, or
 would it be ok to initialize it only when the first connection is opened ?

 A global mutex is needed in the vbox driver to protect access to
 g_pVBoxGlobalData, the vboxRegister() function seems to be the best place to
 initialize such a mutex unless there is another entry point to do this ?

Such a mutex doesn't help.

Looking at g_pVBoxGlobalData tells me that we need to get rid of it in
its current form for different reasons. The major reason is that it
contains connection specific data such as the conn and domainEvents
members. This means when you open a new connection you break all other
open connections because connection specific data is overwritten. We
need to redesign this.

A major reason for the existence of g_pVBoxGlobalData is given in line
209 of vbox_tmpl.c: g_pVBoxGlobalData needs to be global because event
callbacks won't work otherwise. Actually that's not true. Who ever did
the original implementation of this was not aware of how COM (MS and
XP variants) works.

There is a vboxAllocCallbackObj function that creates an
IVirtualBoxCallback COM object. The first member in a COM objects is a
pointer to its vtbl that contains pointers to all the methods that can
be called on it. After this vtbl the COM object contains its private
data members, this aspect is not used in the current implementation.

So we could just allocate a bigger object and put the data that
belongs to the IVirtualBoxCallback implementation into this additional
memory. This includes at least vboxCallBackRefCount and domainEvents
that are currently located in g_pVBoxGlobalData.

The only two members of g_pVBoxGlobalData that can stay global are
caps and pFuncs, all other members are specific to a connection and
cannot be global.

Are you referring to data-pFuncs-pfnComInitialize and
data-pFuncs-pfnComUninitialize when you say VirtualBox C bindings
initialization and uninitialization functions? As those are used to
create connection specific objects we cannot move them out of
vboxOpen/vboxClose. If they are not thread-safe than we need to put a
global mutex around these calls.

This is just what I noticed from a quick look at the code in the
context of the threading problem. This probably needs a more detailed
analysis.

-- 
Matthias Bolte
http://photron.blogspot.com

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


Re: [libvirt] Memory leak on list_domains

2012-04-19 Thread Carlos Rodrigues
Thank you for your answer.

The memory leak problem on my application its solved now, and it is not
related to list_domains method that i suspected before.

So thank you any way and keep up the good job.

Best regards,

-- 
Carlos Rodrigues c...@eurotux.com
 Investigação e Desenvolvimento
 Eurotux Informática, S.A. [http://eurotux.com]



On Thu, 2012-03-15 at 17:03 -0600, Eric Blake wrote:
 On 01/25/2012 03:20 AM, Carlos Rodrigues wrote:
  Hi,
  
  I have some problems on my application with memory leak when call
  list_domains method.
  
  I'm using libvirt 0.8.3 and Sys::Virt 0.2.4 Perl Module.
  
  Does anyone have any idea what's the problem?
 
 Sorry for not noticing this mail sooner.  Without seeing the actual
 code, it's tough to know whether the leak is due to a bug in your code,
 in the perl bindings, or even if it is a bug in older libvirt that has
 since been patched.  Can you try again with libvirt 0.9.10, or even
 libvirt.git?  Or can you provide more evidence of a leak, such as a
 valgrind backtrace of the memory leak?
 


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] Problem with USB pass-through

2012-04-19 Thread Carlos Rodrigues
Hello,

I have a problem with USB pass-through.

I can see the USB device in host machine, i see it in libvirt but when
attach to VM i get an error:

# lsusb 
...
Bus 002 Device 014: ID 0529:0001 Aladdin Knowledge Systems HASP v0.06
...

# virsh nodedev-list
...
usb_device_529_1_noserial
usb_device_529_1_noserial_if0
usb_device_529_1_noserial_usbraw
...

# virsh dumpxml vm
...

hostdev mode='subsystem' type='usb' managed='no'
  source
vendor id='0x0529'/
product id='0x0001'/
  /source
/hostdev
...

# cat /var/log/libvirt/qemu/vm.log
...
LC_ALL=C PATH=/sbin:/usr/sbin:/bin:/usr/bin HOME=/
QEMU_AUDIO_DRV=none /usr/bin/qemu-kvm -S -M pc -m 3800 -smp 4 -name vm
-uuid 056dad8b-22a4-4472-cb5e-8cf60215a41b -monitor
unix:/var/lib/libvirt/qemu/v,.monitor,server,nowait -boot c -drive
file=/dev/vg_sata2/vol_vm,if=virtio,boot=on,format=raw -net
nic,macaddr=00:00:00:0b:5b:ec,vlan=0,model=virtio,name=net0 -net
tap,fd=21,vlan=0,name=hostnet0 -serial none -parallel none -usb
-usbdevice tablet -vnc 0.0.0.0:4 -k pt -vga cirrus -usbdevice
host:002.014 -balloon virtio 
husb: open device 2.12
/proc/bus/usb/002/012: No such file or directory
Warning: could not add USB device host:002.014
...

I'm using libvirt 0.8.3 with kvm 88.

Anyone have see this problem?

Regards,

-- 
Carlos Rodrigues c...@eurotux.com
 Investigação e Desenvolvimento
 Eurotux Informática, S.A. [http://eurotux.com]





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] Correct some minor typos in docs

2012-04-19 Thread Zeeshan Ali (Khattak)
From: Zeeshan Ali (Khattak) zeesha...@gnome.org

---
 libvirt-gobject/libvirt-gobject-domain.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index c1a2086..7fcd733 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -775,7 +775,7 @@ cleanup:
 }
 
 /**
- * gir_domain_suspend:
+ * gvir_domain_suspend:
  * @dom: the domain to suspend
  * @err: Place-holder for possible errors
  *
@@ -857,7 +857,7 @@ gvir_domain_save_helper(GSimpleAsyncResult *res,
 }
 
 /**
- * gir_domain_save_async:
+ * gvir_domain_save_async:
  * @dom: the domain to save
  * @flags: extra flags, currently unused
  * @cancellable: (allow-none)(transfer none): cancellation object
@@ -893,7 +893,7 @@ void gvir_domain_save_async (GVirDomain *dom,
 }
 
 /**
- * gir_domain_save_finish:
+ * gvir_domain_save_finish:
  * @dom: the domain to save
  * @result: (transfer none): async method result
  * @err: Place-holder for possible errors
-- 
1.7.7.6

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


[libvirt] [PATCH V12 0/5] Add DHCP snooping support to nwfilter

2012-04-19 Thread Stefan Berger
This series of patches adds DHCP snooping support to libvirt's
nwfilter subsystem.

DHCP snooping detects DHCP leases obtained by a VM and automatically
adjusts the network traffic filters to reflect the IP addresses
with which a VM may send its traffic, thus for example preventing
IP address spoofing.
Once leases on IP addresses expire or if a VM gives up on a
lease on an IP address, the filters are also adjusted.
All leases are persisted and automatically applied upon a VM's restart.
Leases are associated with the tuple of VM-UUID and interface MAC
address.

The following interface XML activates and uses the DHCP snooping:

interface type='bridge'
  source bridge='virbr0'/
  filterref filter='clean-traffic'
parameter name='CTRL_IP_LEARNING' value='dhcp'/
  /filterref
/interface

Once an IP address has been detected on an interface, 'virsh dumpxml vm'
would show the IP address lease in the format IP address,lease timeout
in seconds:

interface type='bridge'
  source bridge='virbr0'/
  filterref filter='clean-traffic'
parameter name='CTRL_IP_LEARNING' value='dhcp'/
parameter name='IP_LEASE' value='192.168.122.210,180'/
  /filterref
/interface

Regards,
   David and Stefan

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


[libvirt] [PATCH V12 1/5] [PATCH] Add new functions to virSocketAddr

2012-04-19 Thread Stefan Berger
Add 2 new functions to the virSocketAddr 'class':

- virSocketAddrEqual: tests whether two IP addresses and their ports are equal
- virSocketaddSetIPv4Addr: set a virSocketAddr given a 32 bit int

---
 src/libvirt_private.syms |2 ++
 src/util/virsocketaddr.c |   45 +
 src/util/virsocketaddr.h |4 
 3 files changed, 51 insertions(+)

Index: libvirt-acl/src/libvirt_private.syms
===
--- libvirt-acl.orig/src/libvirt_private.syms
+++ libvirt-acl/src/libvirt_private.syms
@@ -1448,6 +1448,7 @@ virRandomInitialize;
 virSocketAddrBroadcast;
 virSocketAddrBroadcastByPrefix;
 virSocketAddrCheckNetmask;
+virSocketAddrEqual;
 virSocketAddrFormat;
 virSocketAddrFormatFull;
 virSocketAddrGetPort;
@@ -1459,6 +1460,7 @@ virSocketAddrParse;
 virSocketAddrParseIPv4;
 virSocketAddrParseIPv6;
 virSocketAddrPrefixToNetmask;
+virSocketAddrSetIPv4Addr;
 virSocketAddrSetPort;
 
 
Index: libvirt-acl/src/util/virsocketaddr.c
===
--- libvirt-acl.orig/src/util/virsocketaddr.c
+++ libvirt-acl/src/util/virsocketaddr.c
@@ -152,6 +152,51 @@ virSocketAddrParseIPv6(virSocketAddrPtr 
 }
 
 /*
+ * virSocketAddrSetIPv4Addr:
+ * @addr: the location to store the result
+ * @val: the 32bit integer in host byte order representing the IPv4 address
+ *
+ * Set the IPv4 address given an integer in host order. This function does not
+ * touch any previously set port.
+ */
+void
+virSocketAddrSetIPv4Addr(virSocketAddrPtr addr, uint32_t val)
+{
+addr-data.stor.ss_family = AF_INET;
+addr-data.inet4.sin_addr.s_addr = htonl(val);
+addr-len = sizeof(struct sockaddr_in);
+}
+
+/*
+ * virSocketAddrEqual:
+ * @s1: the location of the one IP address
+ * @s2: the location of the other IP address
+ *
+ * Compare two IP addresses for equality. Two addresses are equal
+ * if their IP addresses and ports are equal.
+ */
+bool
+virSocketAddrEqual(const virSocketAddrPtr s1, const virSocketAddrPtr s2)
+{
+if (s1-data.stor.ss_family != s2-data.stor.ss_family)
+return false;
+
+switch (s1-data.stor.ss_family) {
+case AF_INET:
+return (memcmp(s1-data.inet4.sin_addr.s_addr,
+   s2-data.inet4.sin_addr.s_addr,
+   4) == 0 
+s1-data.inet4.sin_port == s2-data.inet4.sin_port);
+case AF_INET6:
+return (memcmp(s1-data.inet6.sin6_addr.s6_addr,
+   s2-data.inet6.sin6_addr.s6_addr,
+   4) == 0 
+s1-data.inet6.sin6_port == s2-data.inet6.sin6_port);
+}
+return false;
+}
+
+/*
  * virSocketAddrFormat:
  * @addr: an initialized virSocketAddrPtr
  *
Index: libvirt-acl/src/util/virsocketaddr.h
===
--- libvirt-acl.orig/src/util/virsocketaddr.h
+++ libvirt-acl/src/util/virsocketaddr.h
@@ -66,6 +66,8 @@ int virSocketAddrParseIPv4(virSocketAddr
 int virSocketAddrParseIPv6(virSocketAddrPtr addr,
const char *val);
 
+void virSocketAddrSetIPv4Addr(const virSocketAddrPtr s, uint32_t addr);
+
 char * virSocketAddrFormat(virSocketAddrPtr addr);
 char * virSocketAddrFormatFull(virSocketAddrPtr addr,
bool withService,
@@ -100,5 +102,7 @@ int virSocketAddrGetNumNetmaskBits(const
 int virSocketAddrPrefixToNetmask(unsigned int prefix,
  virSocketAddrPtr netmask,
  int family);
+bool virSocketAddrEqual(const virSocketAddrPtr s1,
+const virSocketAddrPtr s2);
 
 #endif /* __VIR_SOCKETADDR_H__ */

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


[libvirt] [PATCH V12 4/5] nwfilter: Add multiple IP address support to DHCP snooping

2012-04-19 Thread Stefan Berger
With support for multiple IP addresses per interface in place, this patch
now adds support for multiple IP addresses per interface for the DHCP
snooping code.


Testing:

Since the infrastructure I tested this with does not provide multiple IP
addresses per MAC address (anymore), I either had to plug the VM's interface
from the virtual bride connected directly to the infrastructure to virbr0
to get a 2nd IP address from dnsmasq (kill and run dhclient inside the VM)
or changed the lease file  (/var/run/libvirt/network/nwfilter.leases) and
restart libvirtd to have a 2nd IP address on an existing interface.
Note that dnsmasq can take a lease timeout parameter as part of the --dhcp-range
command line parameter, so that timeouts can be tested that way
(--dhcp-range 192.168.122.2,192.168.122.254,120). So, terminating and restarting
dnsmasq with that parameter is another choice to watch an IP address disappear
after 120 seconds.

Regards,
   Stefan

---
 src/nwfilter/nwfilter_dhcpsnoop.c |  107 +++---
 1 file changed, 67 insertions(+), 40 deletions(-)

Index: libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_dhcpsnoop.c
+++ libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -59,6 +59,7 @@
 #include conf/domain_conf.h
 #include nwfilter_gentech_driver.h
 #include nwfilter_dhcpsnoop.h
+#include nwfilter_ipaddrmap.h
 #include virnetdev.h
 #include virfile.h
 #include viratomic.h
@@ -269,7 +270,8 @@ struct _virNWFilterSnoopRateLimitConf {
 /* local function prototypes */
 static int virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
virSocketAddrPtr ipaddr,
-   bool update_leasefile);
+   bool update_leasefile,
+   bool instantiate);
 
 static void virNWFilterSnoopReqLock(virNWFilterSnoopReqPtr req);
 static void virNWFilterSnoopReqUnlock(virNWFilterSnoopReqPtr req);
@@ -445,32 +447,22 @@ virNWFilterSnoopIPLeaseInstallRule(virNW
 {
 char *ipaddr;
 int rc = -1;
-virNWFilterVarValuePtr ipVar;
 virNWFilterSnoopReqPtr req;
 
 ipaddr = virSocketAddrFormat(ipl-ipAddress);
 if (!ipaddr)
 return -1;
 
-ipVar = virNWFilterVarValueCreateSimple(ipaddr);
-if (!ipVar)
-goto cleanup;
-
-ipaddr = NULL; /* belongs to ipVar now */
-
 req = ipl-snoopReq;
 
-/* protect req-ifname and req-vars */
+/* protect req-ifname */
 virNWFilterSnoopReqLock(req);
 
-if (virNWFilterHashTablePut(req-vars, NWFILTER_VARNAME_IP,
-ipVar, 1)  0) {
-virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
-   _(Could not add variable \
- NWFILTER_VARNAME_IP \ to hashmap));
-virNWFilterVarValueFree(ipVar);
+if (virNWFilterIPAddrMapAddIPAddr(req-ifname, ipaddr)  0)
 goto exit_snooprequnlock;
-}
+
+/* ipaddr now belongs to the map */
+ipaddr = NULL;
 
 if (!instantiate) {
 rc = 0;
@@ -493,7 +485,6 @@ virNWFilterSnoopIPLeaseInstallRule(virNW
 exit_snooprequnlock:
 virNWFilterSnoopReqUnlock(req);
 
-cleanup:
 VIR_FREE(ipaddr);
 
 return rc;
@@ -536,12 +527,18 @@ static unsigned int
 virNWFilterSnoopReqLeaseTimerRun(virNWFilterSnoopReqPtr req)
 {
 time_t now = time(0);
+bool is_last = false;
 
 /* protect req-start */
 virNWFilterSnoopReqLock(req);
 
-while (req-start  req-start-timeout = now)
-virNWFilterSnoopReqLeaseDel(req, req-start-ipAddress, true);
+while (req-start  req-start-timeout = now) {
+if (req-start-next == NULL ||
+req-start-next-timeout  now)
+is_last = true;
+virNWFilterSnoopReqLeaseDel(req, req-start-ipAddress, true,
+is_last);
+}
 
 virNWFilterSnoopReqUnlock(req);
 
@@ -612,7 +609,7 @@ virNWFilterSnoopReqFree(virNWFilterSnoop
 
 /* free all leases */
 for (ipl = req-start; ipl; ipl = req-start)
-virNWFilterSnoopReqLeaseDel(req, ipl-ipAddress, false);
+virNWFilterSnoopReqLeaseDel(req, ipl-ipAddress, false, false);
 
 /* free all req data */
 VIR_FREE(req-ifname);
@@ -745,15 +742,6 @@ virNWFilterSnoopReqLeaseAdd(virNWFilterS
 
 virNWFilterSnoopReqUnlock(req);
 
-/* support for multiple addresses requires the ability to add filters
- * to existing chains, or to instantiate address lists via
- * virNWFilterInstantiateFilterLate(). Until one of those capabilities
- * is added, don't allow a new address when one is already assigned to
- * this interface.
- */
-if (req-start)
- return 0;/* silently ignore multiple addresses */
-
 if (VIR_ALLOC(pl)  0) {
 virReportOOMError();
 return -1;
@@ -821,34 +809,62 @@ 

Re: [libvirt] [libvirt-glib] Add async variant of gvir_domain_get_info()

2012-04-19 Thread Christophe Fergeau
ACK

Christophe

On Thu, Apr 19, 2012 at 05:17:04PM +0300, Zeeshan Ali (Khattak) wrote:
 From: Zeeshan Ali (Khattak) zeesha...@gnome.org
 
 ---
  libvirt-gobject/libvirt-gobject-domain.c |   80 
 ++
  libvirt-gobject/libvirt-gobject-domain.h |7 +++
  libvirt-gobject/libvirt-gobject.sym  |4 +-
  3 files changed, 90 insertions(+), 1 deletions(-)
 
 diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
 b/libvirt-gobject/libvirt-gobject-domain.c
 index 350a5b0..67ca257 100644
 --- a/libvirt-gobject/libvirt-gobject-domain.c
 +++ b/libvirt-gobject/libvirt-gobject-domain.c
 @@ -572,6 +572,86 @@ GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom,
  return ret;
  }
  
 +static void
 +gvir_domain_get_info_helper(GSimpleAsyncResult *res,
 +GObject *object,
 +GCancellable *cancellable G_GNUC_UNUSED)
 +{
 +GVirDomain *dom = GVIR_DOMAIN(object);
 +GVirDomainInfo *info;
 +GError *err = NULL;
 +
 +info = gvir_domain_get_info(dom, err);
 +if (err)
 +g_simple_async_result_take_error(res, err);
 +else
 +g_simple_async_result_set_op_res_gpointer
 +(res,
 + info,
 + (GDestroyNotify) gvir_domain_info_free);
 +}
 +
 +/**
 + * gvir_domain_get_info_async:
 + * @dom: the domain
 + * @cancellable: (allow-none)(transfer none): cancellation object
 + * @callback: (scope async): completion callback
 + * @user_data: (closure): opaque data for callback
 + *
 + * Asynchronous variant of #gvir_domain_get_info.
 + */
 +void gvir_domain_get_info_async(GVirDomain *dom,
 +GCancellable *cancellable,
 +GAsyncReadyCallback callback,
 +gpointer user_data)
 +{
 +GSimpleAsyncResult *res;
 +
 +g_return_if_fail(GVIR_IS_DOMAIN(dom));
 +
 +res = g_simple_async_result_new(G_OBJECT(dom),
 +callback,
 +user_data,
 +gvir_domain_get_info_async);
 +g_simple_async_result_run_in_thread(res,
 +gvir_domain_get_info_helper,
 +G_PRIORITY_DEFAULT,
 +cancellable);
 +g_object_unref(res);
 +}
 +
 +/**
 + * gvir_domain_get_info_finish:
 + * @dom: the domain
 + * @result: (transfer none): async method result
 + * @err: Place-holder for possible errors
 + *
 + * Finishes the operation started by #gvir_domain_get_info_async.
 + *
 + * Returns: (transfer full): the info
 + */
 +GVirDomainInfo *gvir_domain_get_info_finish(GVirDomain *dom,
 +GAsyncResult *result,
 +GError **err)
 +{
 +GSimpleAsyncResult *res = G_SIMPLE_ASYNC_RESULT(result);
 +GVirDomainInfo *ret;
 +
 +g_return_val_if_fail(GVIR_IS_DOMAIN(dom), NULL);
 +g_return_val_if_fail
 +(g_simple_async_result_is_valid(result,
 +G_OBJECT(dom),
 +gvir_domain_get_info_async),
 + NULL);
 +
 +if (g_simple_async_result_propagate_error(res, err))
 +return NULL;
 +
 +ret = g_simple_async_result_get_op_res_gpointer(res);
 +
 +return gvir_domain_info_copy (ret);
 +}
 +
  /**
   * gvir_domain_screenshot:
   * @stream: stream to use as output
 diff --git a/libvirt-gobject/libvirt-gobject-domain.h 
 b/libvirt-gobject/libvirt-gobject-domain.h
 index 8a4836e..677fbe6 100644
 --- a/libvirt-gobject/libvirt-gobject-domain.h
 +++ b/libvirt-gobject/libvirt-gobject-domain.h
 @@ -141,6 +141,13 @@ gboolean gvir_domain_reboot(GVirDomain *dom,
  
  GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom,
   GError **err);
 +void gvir_domain_get_info_async(GVirDomain *dom,
 +GCancellable *cancellable,
 +GAsyncReadyCallback callback,
 +gpointer user_data);
 +GVirDomainInfo *gvir_domain_get_info_finish(GVirDomain *dom,
 +GAsyncResult *result,
 +GError **err);
  
  GVirConfigDomain *gvir_domain_get_config(GVirDomain *dom,
   guint flags,
 diff --git a/libvirt-gobject/libvirt-gobject.sym 
 b/libvirt-gobject/libvirt-gobject.sym
 index 64c91cc..f43836f 100644
 --- a/libvirt-gobject/libvirt-gobject.sym
 +++ b/libvirt-gobject/libvirt-gobject.sym
 @@ -1,4 +1,4 @@
 -LIBVIRT_GOBJECT_0.0.7 {
 +LIBVIRT_GOBJECT_0.0.8 {
global:
  gvir_init_object;
  gvir_init_object_check;
 @@ -70,6 +70,8 @@ LIBVIRT_GOBJECT_0.0.7 {
   gvir_domain_set_config;
   

[libvirt] [PATCH V12 5/5] nwfilter: Display detected IP address in domain XML

2012-04-19 Thread Stefan Berger
Display detected IP addresses in the domain XML using the
IP_LEASE variable name. This variable name now becomes
a reserved variable name that can be read only but not set
by the user.

The format of the value is: ip addresss,lease timeout in seconds

An example of a displayed XML may then be:

interface type='bridge'
  mac address='52:54:00:68:e3:90'/
  source bridge='virbr0'/
  target dev='vnet1'/
  model type='virtio'/
  filterref filter='clean-traffic'
parameter name='CTRL_IP_LEARNING' value='dhcp'/
parameter name='IP_LEASE' value='192.168.122.210,100'/
  /filterref
  alias name='net0'/
  address type='pci' domain='0x' bus='0x00' slot='0x04' 
function='0x0'/
/interface


---
 docs/formatnwfilter.html.in|   27 
 src/conf/domain_conf.c |   16 +++--
 src/conf/domain_nwfilter.c |   11 ++
 src/conf/domain_nwfilter.h |   12 +++
 src/conf/nwfilter_conf.c   |3 +
 src/conf/nwfilter_ipaddrmap.c  |   29 ++
 src/conf/nwfilter_ipaddrmap.h  |5 +++
 src/conf/nwfilter_params.c |   45 ++--
 src/conf/nwfilter_params.h |9 -
 src/libvirt_private.syms   |3 +
 src/nwfilter/nwfilter_dhcpsnoop.c  |   53 +
 src/nwfilter/nwfilter_dhcpsnoop.h  |4 ++
 src/nwfilter/nwfilter_driver.c |   10 ++
 src/nwfilter/nwfilter_gentech_driver.c |   24 ++
 src/nwfilter/nwfilter_gentech_driver.h |6 +++
 src/nwfilter/nwfilter_learnipaddr.c|   14 
 src/nwfilter/nwfilter_learnipaddr.h|2 +
 17 files changed, 264 insertions(+), 9 deletions(-)

Index: libvirt-acl/src/conf/nwfilter_params.h
===
--- libvirt-acl.orig/src/conf/nwfilter_params.h
+++ libvirt-acl/src/conf/nwfilter_params.h
@@ -72,7 +72,10 @@ struct _virNWFilterHashTable {
 virNWFilterHashTablePtr virNWFilterParseParamAttributes(xmlNodePtr cur);
 int virNWFilterFormatParamAttributes(virBufferPtr buf,
  virNWFilterHashTablePtr table,
- const char *filterref);
+ const char *filterref,
+ const unsigned char *vmuuid,
+ const unsigned char *mac,
+ const char *ifname);
 
 virNWFilterHashTablePtr virNWFilterHashTableCreate(int n);
 void virNWFilterHashTableFree(virNWFilterHashTablePtr table);
@@ -89,12 +92,14 @@ int virNWFilterHashTablePutAll(virNWFilt
   abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_
 
 # define VALID_VARVALUE \
-  abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:
+  abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:,
 
 # define NWFILTER_VARNAME_IP IP
 # define NWFILTER_VARNAME_MAC MAC
 # define NWFILTER_VARNAME_CTRL_IP_LEARNING CTRL_IP_LEARNING
 # define NWFILTER_VARNAME_DHCPSERVER DHCPSERVER
+# define NWFILTER_VARNAME_IP_LEASE IP_LEASE
+# define NWFILTER_VARNAME_IPV6_LEASE IPV6_LEASE /* future */
 
 enum virNWFilterVarAccessType {
 VIR_NWFILTER_VAR_ACCESS_ELEMENT = 0,
Index: libvirt-acl/src/conf/domain_conf.c
===
--- libvirt-acl.orig/src/conf/domain_conf.c
+++ libvirt-acl/src/conf/domain_conf.c
@@ -43,6 +43,7 @@
 #include buf.h
 #include c-ctype.h
 #include logging.h
+#include nwfilter_params.h
 #include nwfilter_conf.h
 #include ignore-value.h
 #include storage_file.h
@@ -11208,7 +11209,8 @@ error:
 static int
 virDomainNetDefFormat(virBufferPtr buf,
   virDomainNetDefPtr def,
-  unsigned int flags)
+  unsigned int flags,
+  const unsigned char *vmuuid)
 {
 const char *type = virDomainNetTypeToString(def-type);
 
@@ -11349,9 +11351,15 @@ virDomainNetDefFormat(virBufferPtr buf,
 }
 }
 if (def-filter) {
+const char *ifname = NULL;
+
+if (!(flags  VIR_DOMAIN_XML_INACTIVE))
+ifname = def-ifname;
+
 virBufferAdjustIndent(buf, 6);
 if (virNWFilterFormatParamAttributes(buf, def-filterparams,
- def-filter)  0)
+ def-filter, vmuuid, def-mac,
+ ifname)  0)
 return -1;
 virBufferAdjustIndent(buf, -6);
 }
@@ -12635,7 +12643,7 @@ virDomainDefFormatInternal(virDomainDefP
 
 
 for (n = 0 ; n  def-nnets ; n++)
-if (virDomainNetDefFormat(buf, def-nets[n], flags)  0)
+if (virDomainNetDefFormat(buf, def-nets[n], flags, def-uuid)  0)
 goto cleanup;
 
 for (n = 0 ; n  def-nsmartcards ; n++)
@@ -14888,7 +14896,7 @@ 

[libvirt] [PATCH V12 3/5] nwfilter: move code for IP address map into separate file

2012-04-19 Thread Stefan Berger
The goal of this patch is to prepare for support for multiple IP
addresses per interface in the DHCP snooping code.

Move the code for the IP address map that maps interface names to
IP addresses into their own file. Rename the functions on the way
but otherwise leave the code as-is. Initialize this new layer
separately before dependent layers (iplearning, dhcpsnooping)
and shut it down after them.

---
 src/Makefile.am|4 
 src/conf/nwfilter_ipaddrmap.c  |  167 +
 src/conf/nwfilter_ipaddrmap.h  |   37 +++
 src/libvirt_private.syms   |8 +
 src/nwfilter/nwfilter_driver.c |   11 +-
 src/nwfilter/nwfilter_gentech_driver.c |5 
 src/nwfilter/nwfilter_learnipaddr.c|  126 
 src/nwfilter/nwfilter_learnipaddr.h|3 
 8 files changed, 229 insertions(+), 132 deletions(-)

Index: libvirt-acl/src/Makefile.am
===
--- libvirt-acl.orig/src/Makefile.am
+++ libvirt-acl/src/Makefile.am
@@ -159,9 +159,11 @@ NETWORK_CONF_SOURCES = 
\
 # Network filter driver generic impl APIs
 NWFILTER_PARAM_CONF_SOURCES =  \
conf/nwfilter_params.c conf/nwfilter_params.h   \
+   conf/nwfilter_ipaddrmap.c   \
+   conf/nwfilter_ipaddrmap.h   \
conf/nwfilter_conf.h
 
-NWFILTER_CONF_SOURCES =\
+NWFILTER_CONF_SOURCES =\
$(NWFILTER_PARAM_CONF_SOURCES)  \
conf/nwfilter_conf.c conf/nwfilter_conf.h
 
Index: libvirt-acl/src/nwfilter/nwfilter_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_driver.c
@@ -39,6 +39,7 @@
 #include nwfilter_gentech_driver.h
 #include configmake.h
 
+#include nwfilter_ipaddrmap.h
 #include nwfilter_dhcpsnoop.h
 #include nwfilter_learnipaddr.h
 
@@ -67,10 +68,12 @@ static int
 nwfilterDriverStartup(int privileged) {
 char *base = NULL;
 
-if (virNWFilterDHCPSnoopInit()  0)
+if (virNWFilterIPAddrMapInit()  0)
 return -1;
 if (virNWFilterLearnInit()  0)
-return -1;
+goto err_exit_ipaddrmapshutdown;
+if (virNWFilterDHCPSnoopInit()  0)
+goto err_exit_learnshutdown;
 
 virNWFilterTechDriversInit(privileged);
 
@@ -131,7 +134,10 @@ alloc_err_exit:
 conf_init_err:
 virNWFilterTechDriversShutdown();
 virNWFilterDHCPSnoopShutdown();
+err_exit_learnshutdown:
 virNWFilterLearnShutdown();
+err_exit_ipaddrmapshutdown:
+virNWFilterIPAddrMapShutdown();
 
 return -1;
 }
@@ -210,6 +216,7 @@ nwfilterDriverShutdown(void) {
 virNWFilterTechDriversShutdown();
 virNWFilterDHCPSnoopShutdown();
 virNWFilterLearnShutdown();
+virNWFilterIPAddrMapShutdown();
 
 nwfilterDriverLock(driverState);
 
Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c
@@ -33,6 +33,7 @@
 #include nwfilter_gentech_driver.h
 #include nwfilter_ebiptables_driver.h
 #include nwfilter_dhcpsnoop.h
+#include nwfilter_ipaddrmap.h
 #include nwfilter_learnipaddr.h
 #include virnetdev.h
 #include datatypes.h
@@ -870,7 +871,7 @@ __virNWFilterInstantiateFilter(const uns
 goto err_exit;
 }
 
-ipaddr = virNWFilterGetIpAddrForIfname(ifname);
+ipaddr = virNWFilterIPAddrMapGetIPAddr(ifname);
 
 vars1 = virNWFilterCreateVarHashmap(str_macaddr, ipaddr);
 if (!vars1) {
@@ -1132,7 +1133,7 @@ _virNWFilterTeardownFilter(const char *i
 
 techdriver-allTeardown(ifname);
 
-virNWFilterDelIpAddrForIfname(ifname, NULL);
+virNWFilterIPAddrMapDelIPAddr(ifname, NULL);
 
 virNWFilterUnlockIface(ifname);
 
Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c
+++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c
@@ -52,6 +52,7 @@
 #include conf/domain_conf.h
 #include nwfilter_gentech_driver.h
 #include nwfilter_ebiptables_driver.h
+#include nwfilter_ipaddrmap.h
 #include nwfilter_learnipaddr.h
 
 #define VIR_FROM_THIS VIR_FROM_NWFILTER
@@ -118,9 +119,6 @@ struct ether_vlan_header
 static virMutex pendingLearnReqLock;
 static virHashTablePtr pendingLearnReq;
 
-static virMutex ipAddressMapLock;
-static virNWFilterHashTablePtr ipAddressMap;
-
 static virMutex ifaceMapLock;
 static virHashTablePtr ifaceLockMap;
 
@@ -310,113 +308,8 @@ virNWFilterDeregisterLearnReq(int ifinde
 return res;
 }
 
-/* Add an IP address to the list of IP addresses an 

Re: [libvirt] [RH-BZ #595428] 'virsh list' should output more information 'xm list'

2012-04-19 Thread Maxim Sditanov
2012/4/2 Maxim Sditanov feni...@rambler.ru:
 Is this new feature request still
 https://bugs.gentoo.org/show_bug.cgi?id=366561 actual?

 Because virsh still show not enough information:

 $./virsh -c qemu:///system list --all

  IdName   State
 
  1 running
  - winxp  shut off

 And it will be good idea to show more detailed information or add
 additional flag, something like --detail.
 ./virsh -c qemu:///system list --all --detailed


 Id Name   StateMemory   VCPU UptimeCPU   IOPS
 --
 -
  1  running768M  2 3hours 5%12.1
  -  winxp  shut off512M  1


I implemented first part of this task - VCPU and CPU load.
I ported from virt-manager CPU load algorithm
(thanks Cole Robinson for help
https://www.redhat.com/archives/virt-tools-list/2012-April/msg00051.html)

virt manager update graphic in such way:
It create thread, than it take host CPU ticks (how much cpu time take host)
then every 1 sec it update value and calculate percentage.
But this algorithm is bad in virsh, because virsh give information about
domains imidiatly, without delays and thats why i can't caclulate cpu
(also io and
network usage).

I think it will be good idea if libvirtd will hold information about
resource usage and implement interface for this data via libvirt
And virsh and virt-manager will use
the same functions to get cpu, disk and network usage, host uptime

How do you think?

--
With best wishes, Feniks Gordon Freeman

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


Re: [libvirt] Problem with USB pass-through

2012-04-19 Thread Laine Stump
On 04/19/2012 10:44 AM, Carlos Rodrigues wrote:
 Hello,

 I have a problem with USB pass-through.

 I can see the USB device in host machine, i see it in libvirt but when
 attach to VM i get an error:

 # lsusb 
 ...
 Bus 002 Device 014: ID 0529:0001 Aladdin Knowledge Systems HASP v0.06
 ...

 # virsh nodedev-list
 ...
 usb_device_529_1_noserial
 usb_device_529_1_noserial_if0
 usb_device_529_1_noserial_usbraw
 ...

 # virsh dumpxml vm
 ...

 hostdev mode='subsystem' type='usb' managed='no'
   source
 vendor id='0x0529'/
 product id='0x0001'/
   /source
 /hostdev
 ...

 # cat /var/log/libvirt/qemu/vm.log
 ...
 LC_ALL=C PATH=/sbin:/usr/sbin:/bin:/usr/bin HOME=/
 QEMU_AUDIO_DRV=none /usr/bin/qemu-kvm -S -M pc -m 3800 -smp 4 -name vm
 -uuid 056dad8b-22a4-4472-cb5e-8cf60215a41b -monitor
 unix:/var/lib/libvirt/qemu/v,.monitor,server,nowait -boot c -drive
 file=/dev/vg_sata2/vol_vm,if=virtio,boot=on,format=raw -net
 nic,macaddr=00:00:00:0b:5b:ec,vlan=0,model=virtio,name=net0 -net
 tap,fd=21,vlan=0,name=hostnet0 -serial none -parallel none -usb
 -usbdevice tablet -vnc 0.0.0.0:4 -k pt -vga cirrus -usbdevice
 host:002.014 -balloon virtio 
 husb: open device 2.12
 /proc/bus/usb/002/012: No such file or directory
 Warning: could not add USB device host:002.014
 ...

 I'm using libvirt 0.8.3 with kvm 88.

 Anyone have see this problem?

That sounds like this problem:

  commit 823a684f8d0495bd5e7b413e1a81fd5a600abef7
  Author: Daniel P. Berrange berra...@redhat.com
  Date:   Thu Feb 11 14:39:13 2010 +

  Fix USB device path formatting mixup
   
  * src/util/hostusb.c: The device path for a USB device wants the
bus/device IDs in decimal not octal

but the fix was included in libvirt 0.7.7. What is the output of find
/proc/bus/usb?  I'm guessing that there *is* a 002/014, but no 002/012.
Since kvm-88's error message lists both, my next guess is that kvm-88
really *is* expecting the USB device to be given as octal. Are you able
to upgrade to newer kvm or libvirt? (note that there may be a newer kvm
available under the package name qemu-kvm or even qemu, depending on
your distro).

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


[libvirt] [PATCH v3 7/9] pvs: implement virDomainDefineXML operation for existing domains

2012-04-19 Thread Dmitry Guryanov
Add pvsDomainDefineXML functions, it works only for existing
domains for the present.

It's too hard to convert libvirt's XML domain configuration into
PVS's one, so I've decided to compare virDomainDef structures:
current domain definition and the one created from XML, given to
the function. And change only different parameters.

Only description change implemetented, changing other parameters
will be implemented later.

Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
 src/pvs/pvs_driver.c |   89 ++
 1 files changed, 89 insertions(+), 0 deletions(-)

diff --git a/src/pvs/pvs_driver.c b/src/pvs/pvs_driver.c
index 45c777e..a7d27f5 100644
--- a/src/pvs/pvs_driver.c
+++ b/src/pvs/pvs_driver.c
@@ -1070,6 +1070,94 @@ pvsShutdownDomain(virDomainPtr domain)
 VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
 }
 
+static int
+pvsSetDescription(virDomainObjPtr dom, const char *description)
+{
+pvsDomObjPtr pvsdom;
+
+pvsdom = dom-privateData;
+if (pvsCmdRun(PRLCTL, set, pvsdom-uuid,
+  --description, description, NULL))
+return -1;
+
+return 0;
+}
+
+static int
+pvsApplyChanges(virDomainObjPtr dom, virDomainDefPtr newdef)
+{
+virDomainDefPtr olddef = dom-def;
+
+if (newdef-description  !STREQ(olddef-description, 
newdef-description)) {
+if (pvsSetDescription(dom, newdef-description))
+return -1;
+}
+
+/* TODO: compare all other parameters */
+
+return 0;
+}
+
+static virDomainPtr
+pvsDomainDefineXML(virConnectPtr conn, const char *xml)
+{
+pvsConnPtr privconn = conn-privateData;
+virDomainPtr ret = NULL;
+virDomainDefPtr def;
+virDomainObjPtr dom = NULL, olddom = NULL;
+virDomainEventPtr event = NULL;
+int dupVM;
+
+pvsDriverLock(privconn);
+if ((def = virDomainDefParseString(privconn-caps, xml,
+   1  VIR_DOMAIN_VIRT_PVS,
+   VIR_DOMAIN_XML_INACTIVE)) == NULL) {
+pvsError(VIR_ERR_INVALID_ARG, _(Can't parse XML desc));
+goto cleanup;
+}
+
+if ((dupVM = virDomainObjIsDuplicate(privconn-domains, def, 0))  0) {
+pvsError(VIR_ERR_INVALID_ARG, _(Already exists));
+goto cleanup;
+}
+
+if (dupVM == 1) {
+olddom = virDomainFindByUUID(privconn-domains, def-uuid);
+pvsApplyChanges(olddom, def);
+virDomainObjUnlock(olddom);
+
+if (!(dom = virDomainAssignDef(privconn-caps,
+   privconn-domains, def, false))) {
+pvsError(VIR_ERR_INTERNAL_ERROR, _(Can't allocate domobj));
+goto cleanup;
+}
+
+def = NULL;
+} else {
+pvsError(VIR_ERR_NO_SUPPORT, _(Not implemented yet));
+goto cleanup;
+}
+
+event = virDomainEventNewFromObj(dom,
+ VIR_DOMAIN_EVENT_DEFINED,
+ !dupVM ?
+ VIR_DOMAIN_EVENT_DEFINED_ADDED :
+ VIR_DOMAIN_EVENT_DEFINED_UPDATED);
+
+ret = virGetDomain(conn, dom-def-name, dom-def-uuid);
+if (ret)
+ret-id = dom-def-id;
+
+  cleanup:
+virDomainDefFree(def);
+if (dom)
+virDomainObjUnlock(dom);
+if (event)
+pvsDomainEventQueue(privconn, event);
+pvsDriverUnlock(privconn);
+return ret;
+}
+
 static virDriver pvsDriver = {
 .no = VIR_DRV_PVS,
 .name = PVS,
@@ -1097,6 +1185,7 @@ static virDriver pvsDriver = {
 .domainDestroy = pvsDestroyDomain,  /* 0.9.12 */
 .domainShutdown = pvsShutdownDomain, /* 0.9.12 */
 .domainCreate = pvsDomainCreate,/* 0.9.12 */
+.domainDefineXML = pvsDomainDefineXML,  /* 0.9.12 */
 };
 
 /**
-- 
1.7.1

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


[libvirt] [PATCH v3 3/9] pvs: add functions to list domains and get info

2012-04-19 Thread Dmitry Guryanov
PVS driver is 'stateless', like vmware or openvz drivers.
It collects information about domains during startup using
command-line utility prlctl. VMs in PVS identified by UUIDs
or unique names, which can be used as respective fields in
virDomainDef structure. Currently only basic info, like
description, virtual cpus number and memory amount implemented.
Quering devices information will be added in the next patches.

PVS does't support non-persistent domains - you can't run
a domain having only disk image, it must always be registered
in system.

Functions for quering domain info have been just copied from
test driver with some changes - they extract needed data from
previouly created list of virDomainObj objects.

Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
 src/Makefile.am  |3 +-
 src/pvs/pvs_driver.c |  516 +-
 src/pvs/pvs_driver.h |   14 ++
 src/pvs/pvs_utils.c  |  101 ++
 4 files changed, 632 insertions(+), 2 deletions(-)
 create mode 100644 src/pvs/pvs_utils.c

diff --git a/src/Makefile.am b/src/Makefile.am
index 3cbd385..bc9efcf 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -467,7 +467,8 @@ HYPERV_DRIVER_EXTRA_DIST =  
\
 
 PVS_DRIVER_SOURCES =   
\
pvs/pvs_driver.h
\
-   pvs/pvs_driver.c
+   pvs/pvs_driver.c
\
+   pvs/pvs_utils.c
 
 NETWORK_DRIVER_SOURCES =   \
network/bridge_driver.h network/bridge_driver.c
diff --git a/src/pvs/pvs_driver.c b/src/pvs/pvs_driver.c
index 289bb7f..7a8f99e 100644
--- a/src/pvs/pvs_driver.c
+++ b/src/pvs/pvs_driver.c
@@ -51,12 +51,13 @@
 #include configmake.h
 #include storage_file.h
 #include nodeinfo.h
-#include json.h
+#include domain_conf.h
 
 #include pvs_driver.h
 
 #define VIR_FROM_THIS VIR_FROM_PVS
 
+void pvsFreeDomObj(void *p);
 static virCapsPtr pvsBuildCapabilities(void);
 static int pvsClose(virConnectPtr conn);
 
@@ -78,6 +79,14 @@ pvsDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED)
 return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
 }
 
+void
+pvsFreeDomObj(void *p)
+{
+pvsDomObjPtr pdom = (pvsDomObjPtr) p;
+
+VIR_FREE(pdom);
+};
+
 static virCapsPtr
 pvsBuildCapabilities(void)
 {
@@ -126,6 +135,218 @@ pvsGetCapabilities(virConnectPtr conn)
 return xml;
 }
 
+/*
+ * Must be called with privconn-lock held
+ */
+static virDomainObjPtr
+pvsLoadDomain(pvsConnPtr privconn, virJSONValuePtr jobj)
+{
+virDomainObjPtr dom = NULL;
+virDomainDefPtr def = NULL;
+pvsDomObjPtr pdom = NULL;
+virJSONValuePtr jobj2, jobj3;
+const char *tmp;
+char *endptr;
+unsigned long mem;
+unsigned int x;
+
+if (VIR_ALLOC(def)  0)
+goto no_memory;
+
+def-virtType = VIR_DOMAIN_VIRT_PVS;
+def-id = -1;
+
+tmp = virJSONValueObjectGetString(jobj, Name);
+if (!tmp) {
+pvsParseError();
+goto cleanup;
+}
+if (!(def-name = strdup(tmp)))
+goto no_memory;
+
+tmp = virJSONValueObjectGetString(jobj, ID);
+if (!tmp) {
+pvsParseError();
+goto cleanup;
+}
+
+if (virUUIDParse(tmp, def-uuid)  0) {
+pvsError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(UUID in config file malformed));
+goto cleanup;
+}
+
+tmp = virJSONValueObjectGetString(jobj, Description);
+if (!tmp) {
+pvsParseError();
+goto cleanup;
+}
+if (!(def-description = strdup(tmp)))
+goto no_memory;
+
+jobj2 = virJSONValueObjectGet(jobj, Hardware);
+if (!jobj2) {
+pvsParseError();
+goto cleanup;
+}
+
+jobj3 = virJSONValueObjectGet(jobj2, cpu);
+if (!jobj3) {
+pvsParseError();
+goto cleanup;
+}
+
+if (virJSONValueObjectGetNumberUint(jobj3, cpus, x)  0) {
+pvsParseError();
+goto cleanup;
+}
+def-vcpus = x;
+def-maxvcpus = x;
+
+jobj3 = virJSONValueObjectGet(jobj2, memory);
+if (!jobj3) {
+pvsParseError();
+goto cleanup;
+}
+
+tmp = virJSONValueObjectGetString(jobj3, size);
+
+if (virStrToLong_ul(tmp, endptr, 10, mem)  0) {
+pvsParseError();
+goto cleanup;
+}
+
+if (!STREQ(endptr, Mb)) {
+pvsParseError();
+goto cleanup;
+}
+
+def-mem.max_balloon = mem;
+def-mem.max_balloon = 10;
+def-mem.cur_balloon = def-mem.max_balloon;
+
+if (!(def-os.type = strdup(hvm)))
+goto no_memory;
+
+if (!(def-os.init = strdup(/sbin/init)))
+goto no_memory;
+
+if (!(dom = virDomainAssignDef(privconn-caps,
+   privconn-domains, def, false)))
+goto cleanup;
+/* dom is locked here */
+
+if (VIR_ALLOC(pdom)  0)
+goto 

[libvirt] [PATCH v3 0/9] Add basic driver for Parallels Virtuozzo Server

2012-04-19 Thread Dmitry Guryanov
Parallels Virtuozzo Server is a cloud-ready virtualization
solution that allows users to simultaneously run multiple virtual
machines and containers on the same physical server.

Current name of this product is Parallels Server Bare Metal and
more information about it can be found here -
http://www.parallels.com/products/server/baremetal/sp/.

This driver will work with PVS version 6.0 , beta version
scheduled at 2012 Q2.

Dmitry Guryanov (9):
  pvs: add driver skeleton
  util: add functions for interating over json object
  pvs: add functions to list domains and get info
  pvs: implement functions for domain life cycle management
  pvs: get info about serial ports
  pvs: add support of VNC remote display
  pvs: implement virDomainDefineXML operation for existing domains
  pvs: add storage driver
  pvs: implement VM creation

 cfg.mk  |1 +
 configure.ac|   23 +
 docs/drvpvs.html.in |   28 +
 include/libvirt/virterror.h |1 +
 libvirt.spec.in |7 +
 mingw32-libvirt.spec.in |6 +
 po/POTFILES.in  |1 +
 src/Makefile.am |   23 +
 src/conf/domain_conf.c  |3 +-
 src/conf/domain_conf.h  |1 +
 src/driver.h|1 +
 src/libvirt.c   |   12 +
 src/pvs/pvs_driver.c| 1281 +
 src/pvs/pvs_driver.h|   75 +++
 src/pvs/pvs_storage.c   | 1458 +++
 src/pvs/pvs_utils.c |  143 +
 src/util/json.c |   30 +
 src/util/json.h |4 +
 src/util/virterror.c|3 +
 19 files changed, 3100 insertions(+), 1 deletions(-)
 create mode 100644 docs/drvpvs.html.in
 create mode 100644 src/pvs/pvs_driver.c
 create mode 100644 src/pvs/pvs_driver.h
 create mode 100644 src/pvs/pvs_storage.c
 create mode 100644 src/pvs/pvs_utils.c

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


[libvirt] [PATCH v3 4/9] pvs: implement functions for domain life cycle management

2012-04-19 Thread Dmitry Guryanov
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
 src/pvs/pvs_driver.c |  148 ++
 src/pvs/pvs_driver.h |1 +
 src/pvs/pvs_utils.c  |   18 ++
 3 files changed, 167 insertions(+), 0 deletions(-)

diff --git a/src/pvs/pvs_driver.c b/src/pvs/pvs_driver.c
index 7a8f99e..d2e0762 100644
--- a/src/pvs/pvs_driver.c
+++ b/src/pvs/pvs_driver.c
@@ -60,6 +60,11 @@
 void pvsFreeDomObj(void *p);
 static virCapsPtr pvsBuildCapabilities(void);
 static int pvsClose(virConnectPtr conn);
+int pvsPause(virDomainObjPtr privdom);
+int pvsResume(virDomainObjPtr privdom);
+int pvsStart(virDomainObjPtr privdom);
+int pvsKill(virDomainObjPtr privdom);
+int pvsStop(virDomainObjPtr privdom);
 
 static void
 pvsDriverLock(pvsConnPtr driver)
@@ -87,6 +92,12 @@ pvsFreeDomObj(void *p)
 VIR_FREE(pdom);
 };
 
+static void
+pvsDomainEventQueue(pvsConnPtr driver, virDomainEventPtr event)
+{
+virDomainEventStateQueue(driver-domainEventState, event);
+}
+
 static virCapsPtr
 pvsBuildCapabilities(void)
 {
@@ -747,6 +758,138 @@ pvsDomainGetAutostart(virDomainPtr domain, int *autostart)
 return ret;
 }
 
+typedef int (*pvsChangeState)(virDomainObjPtr privdom);
+#define PVS_UUID(x) (((pvsDomObjPtr)(x-privateData))-uuid)
+
+static int
+pvsDomainChangeState(virDomainPtr domain,
+ virDomainState req_state, const char * req_state_name,
+ pvsChangeState chstate,
+ virDomainState new_state, int reason,
+ int event_type, int event_detail)
+{
+pvsConnPtr privconn = domain-conn-privateData;
+virDomainObjPtr privdom;
+virDomainEventPtr event = NULL;
+int state;
+int ret = -1;
+
+pvsDriverLock(privconn);
+privdom = virDomainFindByName(privconn-domains, domain-name);
+pvsDriverUnlock(privconn);
+
+if (privdom == NULL) {
+pvsError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto cleanup;
+}
+
+state = virDomainObjGetState(privdom, NULL);
+if (state != req_state) {
+pvsError(VIR_ERR_INTERNAL_ERROR, _(domain '%s' not %s),
+ privdom-def-name, req_state_name);
+goto cleanup;
+}
+
+if (chstate(privdom))
+goto cleanup;
+
+virDomainObjSetState(privdom, new_state, reason);
+
+event = virDomainEventNewFromObj(privdom, event_type, event_detail);
+ret = 0;
+
+  cleanup:
+if (privdom)
+virDomainObjUnlock(privdom);
+
+if (event) {
+pvsDriverLock(privconn);
+pvsDomainEventQueue(privconn, event);
+pvsDriverUnlock(privconn);
+}
+return ret;
+}
+
+int pvsPause(virDomainObjPtr privdom)
+{
+return pvsCmdRun(PRLCTL, pause, PVS_UUID(privdom), NULL);
+}
+
+static int
+pvsPauseDomain(virDomainPtr domain)
+{
+return pvsDomainChangeState(domain,
+VIR_DOMAIN_RUNNING, running,
+pvsPause,
+VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER,
+VIR_DOMAIN_EVENT_SUSPENDED,
+VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
+}
+
+int pvsResume(virDomainObjPtr privdom)
+{
+return pvsCmdRun(PRLCTL, resume, PVS_UUID(privdom), NULL);
+}
+
+static int
+pvsResumeDomain(virDomainPtr domain)
+{
+return pvsDomainChangeState(domain,
+VIR_DOMAIN_PAUSED, paused,
+pvsResume,
+VIR_DOMAIN_RUNNING, 
VIR_DOMAIN_RUNNING_UNPAUSED,
+VIR_DOMAIN_EVENT_RESUMED,
+VIR_DOMAIN_EVENT_RESUMED_UNPAUSED);
+}
+
+int pvsStart(virDomainObjPtr privdom)
+{
+return pvsCmdRun(PRLCTL, start, PVS_UUID(privdom), NULL);
+}
+
+static int
+pvsDomainCreate(virDomainPtr domain)
+{
+return pvsDomainChangeState(domain,
+VIR_DOMAIN_SHUTOFF, stopped,
+pvsStart,
+VIR_DOMAIN_RUNNING, 
VIR_DOMAIN_EVENT_STARTED_BOOTED,
+VIR_DOMAIN_EVENT_STARTED,
+VIR_DOMAIN_EVENT_STARTED_BOOTED);
+}
+
+int pvsKill(virDomainObjPtr privdom)
+{
+return pvsCmdRun(PRLCTL, stop, PVS_UUID(privdom), --kill, NULL);
+}
+
+static int
+pvsDestroyDomain(virDomainPtr domain)
+{
+return pvsDomainChangeState(domain,
+VIR_DOMAIN_RUNNING, running,
+pvsKill,
+VIR_DOMAIN_SHUTOFF, 
VIR_DOMAIN_SHUTOFF_DESTROYED,
+VIR_DOMAIN_EVENT_STOPPED,
+VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
+}
+
+int pvsStop(virDomainObjPtr privdom)
+{
+return pvsCmdRun(PRLCTL, stop, PVS_UUID(privdom), NULL);
+}
+
+static int
+pvsShutdownDomain(virDomainPtr domain)
+{
+return pvsDomainChangeState(domain,
+

[libvirt] [PATCH v3 2/9] util: add functions for interating over json object

2012-04-19 Thread Dmitry Guryanov
Add function virJSONValueObjectKeysNumber, virJSONValueObjectGetKey
and virJSONValueObjectGetValue, which allow you to iterate over all
fields of json object: you can get number of fields and then get
name and value, stored in field with that name by index.

Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
 src/util/json.c |   30 ++
 src/util/json.h |4 
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/src/util/json.c b/src/util/json.c
index 3258c3f..e7dc272 100644
--- a/src/util/json.c
+++ b/src/util/json.c
@@ -431,6 +431,36 @@ virJSONValuePtr virJSONValueObjectGet(virJSONValuePtr 
object, const char *key)
 return NULL;
 }
 
+int virJSONValueObjectKeysNumber(virJSONValuePtr object)
+{
+if (object-type != VIR_JSON_TYPE_OBJECT)
+return -1;
+
+return object-data.object.npairs;
+}
+
+const char *virJSONValueObjectGetKey(virJSONValuePtr object, unsigned int n)
+{
+if (object-type != VIR_JSON_TYPE_OBJECT)
+return NULL;
+
+if (n = object-data.object.npairs)
+return NULL;
+
+return object-data.object.pairs[n].key;
+}
+
+virJSONValuePtr virJSONValueObjectGetValue(virJSONValuePtr object, unsigned 
int n)
+{
+if (object-type != VIR_JSON_TYPE_OBJECT)
+return NULL;
+
+if (n = object-data.object.npairs)
+return NULL;
+
+return object-data.object.pairs[n].value;
+}
+
 int virJSONValueArraySize(virJSONValuePtr array)
 {
 if (array-type != VIR_JSON_TYPE_ARRAY)
diff --git a/src/util/json.h b/src/util/json.h
index 686a8fb..436405f 100644
--- a/src/util/json.h
+++ b/src/util/json.h
@@ -100,6 +100,10 @@ virJSONValuePtr virJSONValueObjectGet(virJSONValuePtr 
object, const char *key);
 int virJSONValueArraySize(virJSONValuePtr object);
 virJSONValuePtr virJSONValueArrayGet(virJSONValuePtr object, unsigned int 
element);
 
+int virJSONValueObjectKeysNumber(virJSONValuePtr object);
+const char *virJSONValueObjectGetKey(virJSONValuePtr object, unsigned int n);
+virJSONValuePtr virJSONValueObjectGetValue(virJSONValuePtr object, unsigned 
int n);
+
 const char *virJSONValueGetString(virJSONValuePtr object);
 int virJSONValueGetNumberInt(virJSONValuePtr object, int *value);
 int virJSONValueGetNumberUint(virJSONValuePtr object, unsigned int *value);
-- 
1.7.1

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


[libvirt] [PATCH v3 5/9] pvs: get info about serial ports

2012-04-19 Thread Dmitry Guryanov
Add support of collecting information about serial
ports. This change is needed mostly as an example,
support of other devices will be added later.

Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
 src/pvs/pvs_driver.c |  115 ++
 1 files changed, 115 insertions(+), 0 deletions(-)

diff --git a/src/pvs/pvs_driver.c b/src/pvs/pvs_driver.c
index d2e0762..723432a 100644
--- a/src/pvs/pvs_driver.c
+++ b/src/pvs/pvs_driver.c
@@ -146,6 +146,118 @@ pvsGetCapabilities(virConnectPtr conn)
 return xml;
 }
 
+static int
+pvsGetSerialInfo(virDomainChrDefPtr chr,
+ const char *name, virJSONValuePtr value)
+{
+const char *tmp;
+
+chr-deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL;
+chr-targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
+chr-target.port = atoi(name + strlen(serial));
+
+if (virJSONValueObjectHasKey(value, output)) {
+chr-source.type = VIR_DOMAIN_CHR_TYPE_FILE;
+
+tmp = virJSONValueObjectGetString(value, output);
+if (!tmp) {
+pvsParseError();
+return -1;
+}
+
+if (!(chr-source.data.file.path = strdup(tmp)))
+goto no_memory;
+} else if (virJSONValueObjectHasKey(value, socket)) {
+chr-source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+
+tmp = virJSONValueObjectGetString(value, socket);
+if (!tmp) {
+pvsParseError();
+return -1;
+}
+
+if (!(chr-source.data.nix.path = strdup(tmp)))
+goto no_memory;
+chr-source.data.nix.listen = false;
+} else if (virJSONValueObjectHasKey(value, real)) {
+chr-source.type = VIR_DOMAIN_CHR_TYPE_DEV;
+
+tmp = virJSONValueObjectGetString(value, real);
+if (!tmp) {
+pvsParseError();
+return -1;
+}
+
+if (!(chr-source.data.file.path = strdup(tmp)))
+goto no_memory;
+} else {
+pvsParseError();
+return -1;
+}
+
+return 0;
+
+  no_memory:
+virReportOOMError();
+return -1;
+}
+
+static int
+pvsAddSerialInfo(virDomainObjPtr dom,
+ const char *key, virJSONValuePtr value)
+{
+virDomainDefPtr def = dom-def;
+virDomainChrDefPtr chr = NULL;
+
+if (!(chr = virDomainChrDefNew()))
+goto no_memory;
+
+if (pvsGetSerialInfo(chr, key, value))
+goto cleanup;
+
+if (VIR_REALLOC_N(def-serials, def-nserials + 1)  0) {
+virDomainChrDefFree(chr);
+goto no_memory;
+}
+
+def-serials[def-nserials++] = chr;
+
+return 0;
+
+  no_memory:
+virReportOOMError();
+  cleanup:
+virDomainChrDefFree(chr);
+return -1;
+}
+
+static int
+pvsAddDomainHardware(virDomainObjPtr dom, virJSONValuePtr jobj)
+{
+int n, i;
+virJSONValuePtr value;
+const char *key;
+
+n = virJSONValueObjectKeysNumber(jobj);
+if (n  1)
+goto cleanup;
+
+for (i = 0; i  n; i++) {
+key = virJSONValueObjectGetKey(jobj, i);
+value = virJSONValueObjectGetValue(jobj, i);
+
+if (STRPREFIX(key, serial)) {
+if (pvsAddSerialInfo(dom, key, value))
+goto cleanup;
+}
+}
+
+return 0;
+
+  cleanup:
+return -1;
+}
+
 /*
  * Must be called with privconn-lock held
  */
@@ -294,6 +406,9 @@ pvsLoadDomain(pvsConnPtr privconn, virJSONValuePtr jobj)
 else
 dom-autostart = 0;
 
+if (pvsAddDomainHardware(dom, jobj2)  0)
+goto cleanup_unlock;
+
 virDomainObjUnlock(dom);
 
 return dom;
-- 
1.7.1

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


[libvirt] [PATCH v3 9/9] pvs: implement VM creation

2012-04-19 Thread Dmitry Guryanov
To create a new VM in PVS we should issue prlctl create command,
and give path to the directory, where VM should be created. VM's
storage will be in that directory later. So in this first version
find out location of first VM's hard disk and create VM there.

Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
 src/pvs/pvs_driver.c  |   78 -
 src/pvs/pvs_driver.h  |4 ++
 src/pvs/pvs_storage.c |6 +---
 3 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/src/pvs/pvs_driver.c b/src/pvs/pvs_driver.c
index 68dca1a..ff438f0 100644
--- a/src/pvs/pvs_driver.c
+++ b/src/pvs/pvs_driver.c
@@ -1098,6 +1098,74 @@ pvsApplyChanges(virDomainObjPtr dom, virDomainDefPtr 
newdef)
 return 0;
 }
 
+static int
+pvsCreateVm(virConnectPtr conn, virDomainDefPtr def)
+{
+pvsConnPtr privconn = conn-privateData;
+int i;
+virStorageVolDefPtr privvol = NULL;
+virStoragePoolObjPtr pool = NULL;
+virStorageVolPtr vol = NULL;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+for (i = 0; i  def-ndisks; i++) {
+if (def-disks[i]-device != VIR_DOMAIN_DISK_DEVICE_DISK)
+continue;
+
+vol = pvsStorageVolumeLookupByPathLocked(conn, def-disks[i]-src);
+if (!vol) {
+pvsError(VIR_ERR_INVALID_ARG,
+ _(Can't find volume with path '%s'),
+ def-disks[i]-src);
+return -1;
+}
+break;
+}
+
+if (!vol) {
+pvsError(VIR_ERR_INVALID_ARG,
+ _(Can't create VM without hard disks));
+return -1;
+}
+
+pool = virStoragePoolObjFindByName(privconn-pools, vol-pool);
+if (!pool) {
+pvsError(VIR_ERR_INVALID_ARG,
+ _(Can't find storage pool with name '%s'),
+ vol-pool);
+goto error;
+}
+
+privvol = virStorageVolDefFindByPath(pool, def-disks[i]-src);
+if (!privvol) {
+pvsError(VIR_ERR_INVALID_ARG,
+ _(Can't find storage volume definition for path '%s'),
+ def-disks[i]-src);
+goto error2;
+}
+
+virUUIDFormat(def-uuid, uuidstr);
+
+if (pvsCmdRun(PRLCTL, create, def-name, --dst,
+  pool-def-target.path, --no-hdd,
+  --uuid, uuidstr, NULL)  0)
+goto error2;
+
+if (pvsCmdRun(PRLCTL, set, def-name, --vnc-mode, auto, NULL)  0)
+goto error2;
+
+virStoragePoolObjUnlock(pool);
+virUnrefStorageVol(vol);
+
+return 0;
+
+  error2:
+virStoragePoolObjUnlock(pool);
+  error:
+virUnrefStorageVol(vol);
+return -1;
+}
+
 static virDomainPtr
 pvsDomainDefineXML(virConnectPtr conn, const char *xml)
 {
@@ -1134,8 +1202,16 @@ pvsDomainDefineXML(virConnectPtr conn, const char *xml)
 
 def = NULL;
 } else {
-pvsError(VIR_ERR_NO_SUPPORT, _(Not implemented yet));
+if (pvsCreateVm(conn, def))
 goto cleanup;
+if (pvsLoadDomains(privconn, def-name))
+goto cleanup;
+dom = virDomainFindByName(privconn-domains, def-name);
+if (!dom) {
+pvsError(VIR_ERR_INTERNAL_ERROR,
+ _(Domain is not defined after creation));
+goto cleanup;
+}
 }
 
 event = virDomainEventNewFromObj(dom,
diff --git a/src/pvs/pvs_driver.h b/src/pvs/pvs_driver.h
index 40ab546..63f3ad1 100644
--- a/src/pvs/pvs_driver.h
+++ b/src/pvs/pvs_driver.h
@@ -67,5 +67,9 @@ int pvsCmdRun(const char *binary, ...);
 char * pvsAddFileExt(const char *path, const char *ext);
 void pvsDriverLock(pvsConnPtr driver);
 void pvsDriverUnlock(pvsConnPtr driver);
+virStorageVolPtr pvsStorageVolumeLookupByPathLocked(virConnectPtr
+   conn,
+   const char
+   *path);
 
 #endif
diff --git a/src/pvs/pvs_storage.c b/src/pvs/pvs_storage.c
index fc1e2c8..75928b4 100644
--- a/src/pvs/pvs_storage.c
+++ b/src/pvs/pvs_storage.c
@@ -41,10 +41,6 @@ static virStorageVolDefPtr 
pvsStorageVolumeDefine(virStoragePoolObjPtr pool,
   const char *xmldesc,
   const char *xmlfile,
   bool is_new);
-static virStorageVolPtr pvsStorageVolumeLookupByPathLocked(virConnectPtr
-   conn,
-   const char
-   *path);
 static virStorageVolPtr pvsStorageVolumeLookupByPath(virConnectPtr conn,
  const char *path);
 static int pvsStoragePoolGetAlloc(virStoragePoolDefPtr def);
@@ -941,7 +937,7 @@ pvsStorageVolumeLookupByKey(virConnectPtr conn, const char 
*key)
 return ret;
 }
 
-static 

[libvirt] [PATCH v3 8/9] pvs: add storage driver

2012-04-19 Thread Dmitry Guryanov
PVS has one serious discrepancy with libvirt: libvirt stores
domain configuration files always in one place, and storage files
in other places (with API of storage pools and storage volumes).
PVS store all domain data in a single directory, for example, you
may have domain with name fedora-15, which will be located in
'/var/parallels/fedora-15.pvm', and it's hard disk image will be
in '/var/parallels/fedora-15.pvm/harddisk1.hdd'.

I've decided to create storage driver, which produces pseudo-volumes
(xml files with volume description), and they will be 'converted' to
real disk images after attaching to a VM.

So if someone creates VM with one hard disk using virt-manager,
at first virt-manager creates a new volume, and then defines a
domain. We can lookup a volume by path in XML domain definition
and find out location of new domain and size of its hard disk.

This code mostly duplicates code in libvirt's default storage
driver, but I haven't found, how functions from that driver can
be reused. So if it possible I'll be very grateful for the advice,
how to do it.

Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
 src/Makefile.am   |3 +-
 src/pvs/pvs_driver.c  |6 +-
 src/pvs/pvs_driver.h  |5 +
 src/pvs/pvs_storage.c | 1462 +
 src/pvs/pvs_utils.c   |   24 +
 5 files changed, 1497 insertions(+), 3 deletions(-)
 create mode 100644 src/pvs/pvs_storage.c

diff --git a/src/Makefile.am b/src/Makefile.am
index bc9efcf..0765aaf 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -468,7 +468,8 @@ HYPERV_DRIVER_EXTRA_DIST =  
\
 PVS_DRIVER_SOURCES =   
\
pvs/pvs_driver.h
\
pvs/pvs_driver.c
\
-   pvs/pvs_utils.c
+   pvs/pvs_utils.c 
\
+   pvs/pvs_storage.c
 
 NETWORK_DRIVER_SOURCES =   \
network/bridge_driver.h network/bridge_driver.c
diff --git a/src/pvs/pvs_driver.c b/src/pvs/pvs_driver.c
index a7d27f5..68dca1a 100644
--- a/src/pvs/pvs_driver.c
+++ b/src/pvs/pvs_driver.c
@@ -66,13 +66,13 @@ int pvsStart(virDomainObjPtr privdom);
 int pvsKill(virDomainObjPtr privdom);
 int pvsStop(virDomainObjPtr privdom);
 
-static void
+void
 pvsDriverLock(pvsConnPtr driver)
 {
 virMutexLock(driver-lock);
 }
 
-static void
+void
 pvsDriverUnlock(pvsConnPtr driver)
 {
 virMutexUnlock(driver-lock);
@@ -1198,6 +1198,8 @@ pvsRegister(void)
 {
 if (virRegisterDriver(pvsDriver)  0)
 return -1;
+if (pvsStorageRegister())
+return -1;
 
 return 0;
 }
diff --git a/src/pvs/pvs_driver.h b/src/pvs/pvs_driver.h
index 8c2d02b..40ab546 100644
--- a/src/pvs/pvs_driver.h
+++ b/src/pvs/pvs_driver.h
@@ -26,6 +26,7 @@
 
 #include domain_conf.h
 #include storage_conf.h
+#include driver.h
 #include domain_event.h
 
 #include json.h
@@ -59,8 +60,12 @@ typedef struct _pvsConn pvsConn;
 typedef struct _pvsConn *pvsConnPtr;
 
 int pvsRegister(void);
+int pvsStorageRegister(void);
 
 virJSONValuePtr pvsParseOutput(const char *binary, ...);
 int pvsCmdRun(const char *binary, ...);
+char * pvsAddFileExt(const char *path, const char *ext);
+void pvsDriverLock(pvsConnPtr driver);
+void pvsDriverUnlock(pvsConnPtr driver);
 
 #endif
diff --git a/src/pvs/pvs_storage.c b/src/pvs/pvs_storage.c
new file mode 100644
index 000..fc1e2c8
--- /dev/null
+++ b/src/pvs/pvs_storage.c
@@ -0,0 +1,1462 @@
+/*
+ * pvs_storage.c: core driver functions for managing
+ * Parallels Virtuozzo Server hosts
+ *
+ * Copyright (C) 2012 Parallels, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ */
+
+#include config.h
+
+#include stdlib.h
+#include dirent.h
+#include sys/statvfs.h
+
+#include datatypes.h
+#include memory.h
+#include configmake.h
+#include storage_file.h
+#include virterror_internal.h
+
+#include pvs_driver.h
+
+#define VIR_FROM_THIS VIR_FROM_PVS
+
+static int pvsStorageClose(virConnectPtr conn);
+static virStorageVolDefPtr pvsStorageVolumeDefine(virStoragePoolObjPtr pool,
+  

Re: [libvirt] [PATCH v3 0/9] Add basic driver for Parallels Virtuozzo Server

2012-04-19 Thread Stefan Berger

On 04/19/2012 12:05 PM, Dmitry Guryanov wrote:

Parallels Virtuozzo Server is a cloud-ready virtualization
solution that allows users to simultaneously run multiple virtual
machines and containers on the same physical server.


I should have told you yesterday but forgot ... please run a 'make 
syntax-check' on it. Some '#include' will have to be changed to '# 
include' for example.


Regards,
Stefan

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


Re: [libvirt] Making the Thread-safety issues with vbox driver ?

2012-04-19 Thread Jean-Baptiste Rouault
On Thursday 19 April 2012 16:23:19 Matthias Bolte wrote:
 Am 19. April 2012 12:51 schrieb Jean-Baptiste Rouault
  
  A global mutex is needed in the vbox driver to protect access to
  g_pVBoxGlobalData, the vboxRegister() function seems to be the best place
  to initialize such a mutex unless there is another entry point to do
  this ?
 
 Such a mutex doesn't help.
 
 Looking at g_pVBoxGlobalData tells me that we need to get rid of it in
 its current form for different reasons. The major reason is that it
 contains connection specific data such as the conn and domainEvents
 members. This means when you open a new connection you break all other
 open connections because connection specific data is overwritten. We
 need to redesign this.
 
 A major reason for the existence of g_pVBoxGlobalData is given in line
 209 of vbox_tmpl.c: g_pVBoxGlobalData needs to be global because event
 callbacks won't work otherwise. Actually that's not true. Who ever did
 the original implementation of this was not aware of how COM (MS and
 XP variants) works.
 
 There is a vboxAllocCallbackObj function that creates an
 IVirtualBoxCallback COM object. The first member in a COM objects is a
 pointer to its vtbl that contains pointers to all the methods that can
 be called on it. After this vtbl the COM object contains its private
 data members, this aspect is not used in the current implementation.
 
 So we could just allocate a bigger object and put the data that
 belongs to the IVirtualBoxCallback implementation into this additional
 memory. This includes at least vboxCallBackRefCount and domainEvents
 that are currently located in g_pVBoxGlobalData.
 
 The only two members of g_pVBoxGlobalData that can stay global are
 caps and pFuncs, all other members are specific to a connection and
 cannot be global.

I'm not familiar with COM, but if all this connection-specific code can be 
moved out it's nice.

 Are you referring to data-pFuncs-pfnComInitialize and
 data-pFuncs-pfnComUninitialize when you say VirtualBox C bindings
 initialization and uninitialization functions? As those are used to
 create connection specific objects we cannot move them out of
 vboxOpen/vboxClose. If they are not thread-safe than we need to put a
 global mutex around these calls.

Yes these two functions aren't thread-safe, it is even worse than that.
They are located in src/VBox/Main/cbinding/VBoxXPCOMC.cpp in the VirtualBox 
source code. There are 4 static pointers which are replaced at each 
pfnComInitialize call, and released (via the NS_RELEASE macro) in 
pfnComUninitialize.

These accesses aren't protected at all, but what is worse is that when you 
call pfnComUninitialize, the pointers to the IVirtualBox, ISession and 
nsIEventQueue are released. So if you open multiple connections, then close 
one of them, these objects are deleted and your last opened connection is 
broken.

As you said, a global mutex is needed around these calls, but I think we 
should also use the VBOX_ADDREF() macro on the IVirtualBox, ISession and 
nsIEventQueue objects after pfnComInitialize, and VBOX_RELEASE() after 
pfnComUninitialize.

-- 
Jean-Baptiste ROUAULT
Ingénieur RD - diateam : Architectes de l'information
Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 051

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

Re: [libvirt] [RH-BZ #595428] 'virsh list' should output more information 'xm list'

2012-04-19 Thread dennis jenkins
On Thu, Apr 19, 2012 at 10:25 AM, Maxim Sditanov feni...@rambler.ru wrote:

 2012/4/2 Maxim Sditanov feni...@rambler.ru:
  Is this new feature request still
  https://bugs.gentoo.org/show_bug.cgi?id=366561 actual?
 
  Because virsh still show not enough information:
 
  $./virsh -c qemu:///system list --all
 
   IdName   State
  
   1 running
   - winxp  shut off
 
  And it will be good idea to show more detailed information or add
  additional flag, something like --detail.
  ./virsh -c qemu:///system list --all --detailed
 
 
  Id Name   StateMemory   VCPU UptimeCPU   IOPS
  --
  -
   1  running768M  2 3hours 5%12.1
   -  winxp  shut off512M  1
 

 I implemented first part of this task - VCPU and CPU load.
 I ported from virt-manager CPU load algorithm
 (thanks Cole Robinson for help
 https://www.redhat.com/archives/virt-tools-list/2012-April/msg00051.html)

 virt manager update graphic in such way:
 It create thread, than it take host CPU ticks (how much cpu time take host)
 then every 1 sec it update value and calculate percentage.
 But this algorithm is bad in virsh, because virsh give information about
 domains imidiatly, without delays and thats why i can't caclulate cpu
 (also io and
 network usage).

 I think it will be good idea if libvirtd will hold information about
 resource usage and implement interface for this data via libvirt
 And virsh and virt-manager will use
 the same functions to get cpu, disk and network usage, host uptime

 How do you think?


(disclaimer: I'm not a libvirt contributor, just a consumer.  Do I get to
vote? :) )

If statistics collection is implemented directly in libvirtd, how often
would libvirtd capture these statistics?  Once per second? 10s, 1m, etc...?

Would the interval be user-configurable (per-VM)?  If so, could this
setting be changed on the fly (eg, to a live VM and effective immediately)?

Would the statistics survive a restart of libvirtd?  If so, how would they
be persisted (flat text, xml, sqlite database)?

How far back would the stats be kept for?  Would this value be configurable
per-VM?

+1.  I would like to get these kind of stats from virsh list, and very
much like to get CPU and IO usage history from libvirtd.  I'm developing a
simple web interface for managing QEMU and LXC via libvirt (apache,
mod_perl, Sys-Virt, sqlite3, noVnc) for personal use.  I know that libvirt
can give CPU and IO usage info, but not historic info suitable for
assembling a quick graph.  If libvirtd cannot provide historic info, then I
will need to implement a separate daemon to record it, probably by polling
libvirtd at synchronous intervals.

side-note: I should put up a web page with screen-shots of my work in
progress, and provide access to my source code (personal SVN server).  Does
anyone here have an interest in using or examining my little project?

ps- I didn't mean to hijack your thread.  I just wanted to state my desire,
as a user of libvirt, to have this functionality (however tenuously
defined) in place.

Thank you guys and gals for making libvirt excellent.  I've notice the
quality of the feedback given to submitted patches and am impressed.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 8/9] pvs: add storage driver

2012-04-19 Thread Dmitry Guryanov
PVS has one serious discrepancy with libvirt: libvirt stores
domain configuration files always in one place, and storage files
in other places (with API of storage pools and storage volumes).
PVS store all domain data in a single directory, for example, you
may have domain with name fedora-15, which will be located in
'/var/parallels/fedora-15.pvm', and it's hard disk image will be
in '/var/parallels/fedora-15.pvm/harddisk1.hdd'.

I've decided to create storage driver, which produces pseudo-volumes
(xml files with volume description), and they will be 'converted' to
real disk images after attaching to a VM.

So if someone creates VM with one hard disk using virt-manager,
at first virt-manager creates a new volume, and then defines a
domain. We can lookup a volume by path in XML domain definition
and find out location of new domain and size of its hard disk.

This code mostly duplicates code in libvirt's default storage
driver, but I haven't found, how functions from that driver can
be reused. So if it possible I'll be very grateful for the advice,
how to do it.

Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
 src/Makefile.am   |3 +-
 src/pvs/pvs_driver.c  |6 +-
 src/pvs/pvs_driver.h  |5 +
 src/pvs/pvs_storage.c | 1460 +
 src/pvs/pvs_utils.c   |   24 +
 5 files changed, 1495 insertions(+), 3 deletions(-)
 create mode 100644 src/pvs/pvs_storage.c

diff --git a/src/Makefile.am b/src/Makefile.am
index bc9efcf..0765aaf 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -468,7 +468,8 @@ HYPERV_DRIVER_EXTRA_DIST =  
\
 PVS_DRIVER_SOURCES =   
\
pvs/pvs_driver.h
\
pvs/pvs_driver.c
\
-   pvs/pvs_utils.c
+   pvs/pvs_utils.c 
\
+   pvs/pvs_storage.c
 
 NETWORK_DRIVER_SOURCES =   \
network/bridge_driver.h network/bridge_driver.c
diff --git a/src/pvs/pvs_driver.c b/src/pvs/pvs_driver.c
index a7d27f5..68dca1a 100644
--- a/src/pvs/pvs_driver.c
+++ b/src/pvs/pvs_driver.c
@@ -66,13 +66,13 @@ int pvsStart(virDomainObjPtr privdom);
 int pvsKill(virDomainObjPtr privdom);
 int pvsStop(virDomainObjPtr privdom);
 
-static void
+void
 pvsDriverLock(pvsConnPtr driver)
 {
 virMutexLock(driver-lock);
 }
 
-static void
+void
 pvsDriverUnlock(pvsConnPtr driver)
 {
 virMutexUnlock(driver-lock);
@@ -1198,6 +1198,8 @@ pvsRegister(void)
 {
 if (virRegisterDriver(pvsDriver)  0)
 return -1;
+if (pvsStorageRegister())
+return -1;
 
 return 0;
 }
diff --git a/src/pvs/pvs_driver.h b/src/pvs/pvs_driver.h
index 8c2d02b..40ab546 100644
--- a/src/pvs/pvs_driver.h
+++ b/src/pvs/pvs_driver.h
@@ -26,6 +26,7 @@
 
 #include domain_conf.h
 #include storage_conf.h
+#include driver.h
 #include domain_event.h
 
 #include json.h
@@ -59,8 +60,12 @@ typedef struct _pvsConn pvsConn;
 typedef struct _pvsConn *pvsConnPtr;
 
 int pvsRegister(void);
+int pvsStorageRegister(void);
 
 virJSONValuePtr pvsParseOutput(const char *binary, ...);
 int pvsCmdRun(const char *binary, ...);
+char * pvsAddFileExt(const char *path, const char *ext);
+void pvsDriverLock(pvsConnPtr driver);
+void pvsDriverUnlock(pvsConnPtr driver);
 
 #endif
diff --git a/src/pvs/pvs_storage.c b/src/pvs/pvs_storage.c
new file mode 100644
index 000..5d583fb
--- /dev/null
+++ b/src/pvs/pvs_storage.c
@@ -0,0 +1,1460 @@
+/*
+ * pvs_storage.c: core driver functions for managing
+ * Parallels Virtuozzo Server hosts
+ *
+ * Copyright (C) 2012 Parallels, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ */
+
+#include config.h
+
+#include stdlib.h
+#include dirent.h
+#include sys/statvfs.h
+
+#include datatypes.h
+#include memory.h
+#include configmake.h
+#include storage_file.h
+#include virterror_internal.h
+
+#include pvs_driver.h
+
+#define VIR_FROM_THIS VIR_FROM_PVS
+
+static int pvsStorageClose(virConnectPtr conn);
+static virStorageVolDefPtr pvsStorageVolumeDefine(virStoragePoolObjPtr pool,
+  

Re: [libvirt] [PATCH 0/2 v2] use qemu's dump-guest-meory when vm uses host device

2012-04-19 Thread Eric Blake
On 04/18/2012 01:46 AM, Wen Congyang wrote:
 Hi, Eric
 
 Luiz will accept the qemu's patch:
 http://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg02346.html
 
 Do you have any comment about this patchset?

Thanks for the ping.  It's on my list to review, but I'm hoping to tie
up work on my live storage migration work first, so it will probably be
Monday or Tuesday before I can get to your series.  Maybe we'll get
lucky with someone else volunteering some review time?

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Problem with USB pass-through

2012-04-19 Thread Carlos Rodrigues
Thank you for you answer.

In /proc/bus/usb there is a 002/012 but no 002/014.
I will try upgrade kvm and libvirt and see if that solve my problem.

About now, after reboot physical machine, i can do pass-through USB to
virtual machine without a problems.

But i will keep testing if kvm and libvirt solve the problem.

Many thanks,

Best Regards,

-- 
Carlos Rodrigues c...@eurotux.com
 Investigação e Desenvolvimento
 Eurotux Informática, S.A. [http://eurotux.com]



On Thu, 2012-04-19 at 11:49 -0400, Laine Stump wrote:
 On 04/19/2012 10:44 AM, Carlos Rodrigues wrote:
  Hello,
 
  I have a problem with USB pass-through.
 
  I can see the USB device in host machine, i see it in libvirt but when
  attach to VM i get an error:
 
  # lsusb 
  ...
  Bus 002 Device 014: ID 0529:0001 Aladdin Knowledge Systems HASP v0.06
  ...
 
  # virsh nodedev-list
  ...
  usb_device_529_1_noserial
  usb_device_529_1_noserial_if0
  usb_device_529_1_noserial_usbraw
  ...
 
  # virsh dumpxml vm
  ...
 
  hostdev mode='subsystem' type='usb' managed='no'
source
  vendor id='0x0529'/
  product id='0x0001'/
/source
  /hostdev
  ...
 
  # cat /var/log/libvirt/qemu/vm.log
  ...
  LC_ALL=C PATH=/sbin:/usr/sbin:/bin:/usr/bin HOME=/
  QEMU_AUDIO_DRV=none /usr/bin/qemu-kvm -S -M pc -m 3800 -smp 4 -name vm
  -uuid 056dad8b-22a4-4472-cb5e-8cf60215a41b -monitor
  unix:/var/lib/libvirt/qemu/v,.monitor,server,nowait -boot c -drive
  file=/dev/vg_sata2/vol_vm,if=virtio,boot=on,format=raw -net
  nic,macaddr=00:00:00:0b:5b:ec,vlan=0,model=virtio,name=net0 -net
  tap,fd=21,vlan=0,name=hostnet0 -serial none -parallel none -usb
  -usbdevice tablet -vnc 0.0.0.0:4 -k pt -vga cirrus -usbdevice
  host:002.014 -balloon virtio 
  husb: open device 2.12
  /proc/bus/usb/002/012: No such file or directory
  Warning: could not add USB device host:002.014
  ...
 
  I'm using libvirt 0.8.3 with kvm 88.
 
  Anyone have see this problem?
 
 That sounds like this problem:
 
   commit 823a684f8d0495bd5e7b413e1a81fd5a600abef7
   Author: Daniel P. Berrange berra...@redhat.com
   Date:   Thu Feb 11 14:39:13 2010 +
 
   Fix USB device path formatting mixup

   * src/util/hostusb.c: The device path for a USB device wants the
 bus/device IDs in decimal not octal
 
 but the fix was included in libvirt 0.7.7. What is the output of find
 /proc/bus/usb?  I'm guessing that there *is* a 002/014, but no 002/012.
 Since kvm-88's error message lists both, my next guess is that kvm-88
 really *is* expecting the USB device to be given as octal. Are you able
 to upgrade to newer kvm or libvirt? (note that there may be a newer kvm
 available under the package name qemu-kvm or even qemu, depending on
 your distro).
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list


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 2/3] conf: tighten up XML integer parsing

2012-04-19 Thread Eric Blake
On 04/18/2012 07:03 PM, Stefan Berger wrote:
 On 04/18/2012 08:14 PM, Eric Blake wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=617711 reported that
 even with my recent patched to allowmemory unit='G'1/memory,
 people can still get away with tryingmemory1G/memory  and
 silently getmemory unit='KiB'1/memory  instead.  While
 virt-xml-validate catches the error, our C parser was not.

 I always love it when I can reduce lines of code while fixing bugs.

 * src/conf/domain_conf.c (virDomainDefParseXML): Avoid strtoll.
 * src/conf/storage_conf.c (virStorageDefParsePerms): Likewise.
 * src/util/xml.c (virXPathLongBase, virXPathULongBase)
 (virXPathULongLong, virXPathLongLong): Likewise.
 ---
   src/conf/domain_conf.c  |   12 +---
   src/conf/storage_conf.c |6 +++---
   src/util/xml.c  |   36 
   3 files changed, 12 insertions(+), 42 deletions(-)

Phooey.  I fixed memory780Mmemory, but not
currentMemory780M/currentMemory.  It's not enough to change
virXPathULongLong to return -2 instead of 0 on parse errors, but the
caller has to actually check for that return value.  v2 coming up.


 -if (*end || (perms-mode  ~0777)) {
 +int tmp;
 
 Nit: empty line after var decl?
 
 + if (virStrToLong_i(mode, NULL, 8, tmp)  0 || (tmp  ~0777)) {

Sure, I'll fix that as well in v2.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] virsh: avoid strtol

2012-04-19 Thread Eric Blake
On 04/18/2012 06:59 PM, Stefan Berger wrote:
 On 04/18/2012 08:14 PM, Eric Blake wrote:
 We were forgetting to check errno for overflow.

 * tools/virsh.c (get_integer_keycode, vshCommandOptInt)
 (vshCommandOptUInt, vshCommandOptUL, vshCommandOptLongLong)
 (vshCommandOptULongLong): Rewrite to be safer.
 ---
   tools/virsh.c |   66
 
   1 files changed, 19 insertions(+), 47 deletions(-)

 diff --git a/tools/virsh.c b/tools/virsh.c
 index 95ed7bc..3ec853b 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -5758,14 +5758,11 @@ static const vshCmdOptDef opts_send_key[] = {

   static int get_integer_keycode(const char *key_name)
   {
 -long val;
 -char *endptr;
 -
 -val = strtol(key_name,endptr, 0);
 -if (*endptr != '\0' || val  0x || val= 0)

The old code rejected 0,

 - return -1;
 +int ret;

 -return val;
 +if (virStrToLong_i(key_name, NULL, 0,ret)  0 || ret  0x)

but the new code allows it.  v2 coming up.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 0/9] Add basic driver for Parallels Virtuozzo Server

2012-04-19 Thread Eric Blake
On 04/19/2012 10:25 AM, Stefan Berger wrote:
 On 04/19/2012 12:05 PM, Dmitry Guryanov wrote:
 Parallels Virtuozzo Server is a cloud-ready virtualization
 solution that allows users to simultaneously run multiple virtual
 machines and containers on the same physical server.
 
 I should have told you yesterday but forgot ... please run a 'make
 syntax-check' on it. Some '#include' will have to be changed to '#
 include' for example.

Those particular syntax errors are only caught if you have 'cppi' installed.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH V11 2/7] Support for atomic operations on integers

2012-04-19 Thread Eric Blake
On 04/17/2012 08:44 AM, Stefan Berger wrote:
 For threading support, add atomic add and sub operations working on
 integers. Base this on locking support provided by virMutex.
 
 ---
  src/util/viratomic.h |   91 
 +++
  1 file changed, 91 insertions(+)
 
 Index: libvirt-acl/src/util/viratomic.h

 +typedef struct _virAtomicInt virAtomicInt;
 +typedef virAtomicInt *virAtomicIntPtr;
 +
 +struct _virAtomicInt {
 +virMutex lock;
 +int value;
 +};

virMutex is very heavyweight.  I'd love it if we could use gcc
primitives and/or MSVC libc primitives (where they are known to be
available), for a lighter-weight implementation on platforms that
support it.  See this patch proposal (now a year old!) for some ideas
that we should fold in before 0.9.12:

https://www.redhat.com/archives/libvir-list/2011-April/msg00368.html

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH V11 2/7] Support for atomic operations on integers

2012-04-19 Thread Stefan Berger

On 04/19/2012 02:08 PM, Eric Blake wrote:

On 04/17/2012 08:44 AM, Stefan Berger wrote:

For threading support, add atomic add and sub operations working on
integers. Base this on locking support provided by virMutex.


virMutex is very heavyweight.  I'd love it if we could use gcc
primitives and/or MSVC libc primitives (where they are known to be
available), for a lighter-weight implementation on platforms that
support it.  See this patch proposal (now a year old!) for some ideas
that we should fold in before 0.9.12:

https://www.redhat.com/archives/libvir-list/2011-April/msg00368.html

Well, I wasn't aware of this previous proposal and had actually looked 
in pthread for some form of atomic support.
I already pushed this patch now. We can add some compiler- and 
architecture-specific #ifdef's around the few existing functions. So the 
existing implementation probably has a place but can be improved upon.


Stefan


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


[libvirt] [PATCHv2 2/3] conf: tighten up XML integer parsing

2012-04-19 Thread Eric Blake
https://bugzilla.redhat.com/show_bug.cgi?id=617711 reported that
even with my recent patched to allow memory unit='G'1/memory,
people can still get away with trying memory1G/memory and
silently get memory unit='KiB'1/memory instead.  While
virt-xml-validate catches the error, our C parser did not.

Not to mention that it's always fun to fix bugs while reducing
lines of code.  :)

* src/conf/domain_conf.c (virDomainParseMemory): Check for parse error.
(virDomainDefParseXML): Avoid strtoll.
* src/conf/storage_conf.c (virStorageDefParsePerms): Likewise.
* src/util/xml.c (virXPathLongBase, virXPathULongBase)
(virXPathULongLong, virXPathLongLong): Likewise.
---

v2: fix virDomainParseMemory to detect parse errors on optional
arguments, rather than silently ignoring those arguments

 src/conf/domain_conf.c  |   24 ++--
 src/conf/storage_conf.c |7 ---
 src/util/xml.c  |   36 
 3 files changed, 22 insertions(+), 45 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5ab052a..8f352ea 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7609,10 +7609,16 @@ virDomainParseMemory(const char *xpath, 
xmlXPathContextPtr ctxt,
 virReportOOMError();
 goto cleanup;
 }
-if (virXPathULongLong(xpath_full, ctxt, bytes)  0) {
-if (required)
+ret = virXPathULongLong(xpath_full, ctxt, bytes);
+if (ret  0) {
+if (ret == -2)
 virDomainReportError(VIR_ERR_XML_ERROR,
- %s, _(missing memory element));
+ _(could not parse memory element %s),
+ xpath);
+else if (required)
+virDomainReportError(VIR_ERR_XML_ERROR,
+ _(missing memory element %s),
+ xpath);
 else
 ret = 0;
 goto cleanup;
@@ -8086,12 +8092,11 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
 if (STREQ(tmp, reset)) {
 def-clock.data.utc_reset = true;
 } else {
-char *conv = NULL;
-unsigned long long val;
-val = strtoll(tmp, conv, 10);
-if (conv == tmp || *conv != '\0') {
-virDomainReportError(VIR_ERR_INTERNAL_ERROR,
- _(unknown clock adjustment '%s'), 
tmp);
+if (virStrToLong_ll(tmp, NULL, 10,
+def-clock.data.variable.adjustment)  0) 
{
+virDomainReportError(VIR_ERR_XML_ERROR,
+ _(unknown clock adjustment '%s'),
+ tmp);
 goto error;
 }
 switch (def-clock.offset) {
@@ -8103,7 +8108,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
 break;
 }
 def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
-def-clock.data.variable.adjustment = val;
 }
 VIR_FREE(tmp);
 } else {
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 2330fa1..7579327 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -570,14 +570,15 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
 if (!mode) {
 perms-mode = defaultmode;
 } else {
-char *end = NULL;
-perms-mode = strtol(mode, end, 8);
-if (*end || (perms-mode  ~0777)) {
+int tmp;
+
+if (virStrToLong_i(mode, NULL, 8, tmp)  0 || (tmp  ~0777)) {
 VIR_FREE(mode);
 virStorageReportError(VIR_ERR_XML_ERROR,
   %s, _(malformed octal mode));
 goto error;
 }
+perms-mode = tmp;
 VIR_FREE(mode);
 }

diff --git a/src/util/xml.c b/src/util/xml.c
index 79a9d27..7411968 100644
--- a/src/util/xml.c
+++ b/src/util/xml.c
@@ -174,15 +174,8 @@ virXPathLongBase(const char *xpath,
 ctxt-node = relnode;
 if ((obj != NULL)  (obj-type == XPATH_STRING) 
 (obj-stringval != NULL)  (obj-stringval[0] != 0)) {
-char *conv = NULL;
-long val;
-
-val = strtol((const char *) obj-stringval, conv, base);
-if (conv == (const char *) obj-stringval) {
+if (virStrToLong_l((char *) obj-stringval, NULL, base, value)  0)
 ret = -2;
-} else {
-*value = val;
-}
 } else if ((obj != NULL)  (obj-type == XPATH_NUMBER) 
(!(isnan(obj-floatval {
 *value = (long) obj-floatval;
@@ -287,15 +280,8 @@ virXPathULongBase(const char *xpath,
 ctxt-node = relnode;
 if ((obj != NULL)  (obj-type == XPATH_STRING) 
 (obj-stringval != NULL)  (obj-stringval[0] != 0)) {
-char *conv = NULL;
-long val;
-
-val = 

[libvirt] [PATCHv2 3/3] build: avoid strtol and strtod

2012-04-19 Thread Eric Blake
Ensure we don't introduce any more lousy integer parsing in new
code, while avoiding a scrub-down of existing legacy code.

Note that we also need to enable sc_prohibit_atoi_atof (see cfg.mk
local-checks-to-skip) before we are bulletproof, but that also
entails scrubbing I'm not ready to do at the moment.

* src/util/util.c (virStrToLong_i, virStrToLong_ui)
(virStrToLong_l, virStrToLong_ul, virStrToLong_ll)
(virStrToLong_ull, virStrToDouble): Mark exemptions.
* src/util/virmacaddr.c (virMacAddrParse): Likewise.
* cfg.mk (sc_prohibit_strtol): New syntax check.
(exclude_file_name_regexp--sc_prohibit_strtol): Ignore files that
I'm not willing to fix yet.
(local-checks-to-skip): Re-enable sc_prohibit_atoi_atof.
---

v2: no change

 cfg.mk|   14 ++
 src/util/util.c   |   14 +++---
 src/util/virmacaddr.c |2 +-
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 71e9a1d..fb4df2f 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -353,6 +353,17 @@ sc_prohibit_strncmp:
halt='$(ME): use STREQLEN or STRPREFIX instead of str''ncmp'\
  $(_sc_search_regexp)

+# strtol and friends are too easy to misuse
+sc_prohibit_strtol:
+   @prohibit='\bstrto(u?ll?|[ui]max) *\('  \
+   exclude='exempt from syntax-check'  \
+   halt='$(ME): use virStrToLong_*, not strtol variants'   \
+ $(_sc_search_regexp)
+   @prohibit='\bstrto[df] *\(' \
+   exclude='exempt from syntax-check'  \
+   halt='$(ME): use virStrToDouble, not strtod variants'   \
+ $(_sc_search_regexp)
+
 # Use virAsprintf rather than as'printf since *strp is undefined on error.
 sc_prohibit_asprintf:
@prohibit='\v?a[s]printf\'\
@@ -799,6 +810,9 @@ exclude_file_name_regexp--sc_prohibit_sprintf = \
 exclude_file_name_regexp--sc_prohibit_strncpy = \
   ^(src/util/util|tools/virsh)\.c$$

+exclude_file_name_regexp--sc_prohibit_strtol = \
+  ^src/(util/sexpr|(vbox|xen|xenxs)/.*)\.c$$
+
 exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/xml\.c$$

 exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$
diff --git a/src/util/util.c b/src/util/util.c
index 1b39227..48358b2 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1497,7 +1497,7 @@ virStrToLong_i(char const *s, char **end_ptr, int base, 
int *result)
 int err;

 errno = 0;
-val = strtol(s, p, base);
+val = strtol(s, p, base); /* exempt from syntax-check */
 err = (errno || (!end_ptr  *p) || p == s || (int) val != val);
 if (end_ptr)
 *end_ptr = p;
@@ -1516,7 +1516,7 @@ virStrToLong_ui(char const *s, char **end_ptr, int base, 
unsigned int *result)
 int err;

 errno = 0;
-val = strtoul(s, p, base);
+val = strtoul(s, p, base); /* exempt from syntax-check */
 err = (errno || (!end_ptr  *p) || p == s || (unsigned int) val != val);
 if (end_ptr)
 *end_ptr = p;
@@ -1535,7 +1535,7 @@ virStrToLong_l(char const *s, char **end_ptr, int base, 
long *result)
 int err;

 errno = 0;
-val = strtol(s, p, base);
+val = strtol(s, p, base); /* exempt from syntax-check */
 err = (errno || (!end_ptr  *p) || p == s);
 if (end_ptr)
 *end_ptr = p;
@@ -1554,7 +1554,7 @@ virStrToLong_ul(char const *s, char **end_ptr, int base, 
unsigned long *result)
 int err;

 errno = 0;
-val = strtoul(s, p, base);
+val = strtoul(s, p, base); /* exempt from syntax-check */
 err = (errno || (!end_ptr  *p) || p == s);
 if (end_ptr)
 *end_ptr = p;
@@ -1573,7 +1573,7 @@ virStrToLong_ll(char const *s, char **end_ptr, int base, 
long long *result)
 int err;

 errno = 0;
-val = strtoll(s, p, base);
+val = strtoll(s, p, base); /* exempt from syntax-check */
 err = (errno || (!end_ptr  *p) || p == s);
 if (end_ptr)
 *end_ptr = p;
@@ -1592,7 +1592,7 @@ virStrToLong_ull(char const *s, char **end_ptr, int base, 
unsigned long long *re
 int err;

 errno = 0;
-val = strtoull(s, p, base);
+val = strtoull(s, p, base); /* exempt from syntax-check */
 err = (errno || (!end_ptr  *p) || p == s);
 if (end_ptr)
 *end_ptr = p;
@@ -1612,7 +1612,7 @@ virStrToDouble(char const *s,
 int err;

 errno = 0;
-val = strtod(s, p);
+val = strtod(s, p); /* exempt from syntax-check */
 err = (errno || (!end_ptr  *p) || p == s);
 if (end_ptr)
 *end_ptr = p;
diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c
index beb6274..6c0fb24 100644
--- a/src/util/virmacaddr.c
+++ b/src/util/virmacaddr.c
@@ -86,7 +86,7 @@ virMacAddrParse(const char* str, unsigned char *addr)
 if (!c_isxdigit(*str))
 break;

-result = strtoul(str, end_ptr, 16);
+result = strtoul(str, end_ptr, 16); /* exempt from syntax-check */

 if 

[libvirt] [PATCHv2 0/3] tighten up integer parsing

2012-04-19 Thread Eric Blake
v1: https://www.redhat.com/archives/libvir-list/2012-April/msg00954.html

changes in v2: fix regression in patch 1, incomplete change in patch 2

Eric Blake (3):
  virsh: avoid strtol
  conf: tighten up XML integer parsing
  build: avoid strtol and strtod

 cfg.mk  |   14 ++
 src/conf/domain_conf.c  |   24 ++---
 src/conf/storage_conf.c |7 +++--
 src/util/util.c |   14 +-
 src/util/virmacaddr.c   |2 +-
 src/util/xml.c  |   36 +++--
 tools/virsh.c   |   67 ++-
 7 files changed, 64 insertions(+), 100 deletions(-)

-- 
1.7.7.6

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


[libvirt] [PATCHv2 1/3] virsh: avoid strtol

2012-04-19 Thread Eric Blake
We were forgetting to check errno for overflow.

* tools/virsh.c (get_integer_keycode, vshCommandOptInt)
(vshCommandOptUInt, vshCommandOptUL, vshCommandOptLongLong)
(vshCommandOptULongLong): Rewrite to be safer.
---

v2: get_integer_keycode still must reject 0

 tools/virsh.c |   67 +
 1 files changed, 20 insertions(+), 47 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 95ed7bc..0bfb529 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -5756,15 +5756,13 @@ static const vshCmdOptDef opts_send_key[] = {
 {NULL, 0, 0, NULL}
 };

-static int get_integer_keycode(const char *key_name)
+static int
+get_integer_keycode(const char *key_name)
 {
-long val;
-char *endptr;
-
-val = strtol(key_name, endptr, 0);
-if (*endptr != '\0' || val  0x || val = 0)
- return -1;
+unsigned int val;

+if (virStrToLong_ui(key_name, NULL, 0, val)  0 || val  0x || !val)
+return -1;
 return val;
 }

@@ -17973,8 +17971,6 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, 
int *value)
 {
 vshCmdOpt *arg;
 int ret;
-int num;
-char *end_p = NULL;

 ret = vshCommandOpt(cmd, name, arg);
 if (ret = 0)
@@ -17985,12 +17981,9 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, 
int *value)
 return -2;
 }

-num = strtol(arg-data, end_p, 10);
-if (arg-data != end_p  *end_p == 0) {
-*value = num;
-return 1;
-}
-return -1;
+if (virStrToLong_i(arg-data, NULL, 10, value)  0)
+return -1;
+return 1;
 }


@@ -18008,8 +18001,6 @@ vshCommandOptUInt(const vshCmd *cmd, const char *name, 
unsigned int *value)
 {
 vshCmdOpt *arg;
 int ret;
-unsigned int num;
-char *end_p = NULL;

 ret = vshCommandOpt(cmd, name, arg);
 if (ret = 0)
@@ -18020,12 +18011,9 @@ vshCommandOptUInt(const vshCmd *cmd, const char *name, 
unsigned int *value)
 return -2;
 }

-num = strtoul(arg-data, end_p, 10);
-if (arg-data != end_p  *end_p == 0) {
-*value = num;
-return 1;
-}
-return -1;
+if (virStrToLong_ui(arg-data, NULL, 10, value)  0)
+return -1;
+return 1;
 }


@@ -18043,8 +18031,6 @@ vshCommandOptUL(const vshCmd *cmd, const char *name, 
unsigned long *value)
 {
 vshCmdOpt *arg;
 int ret;
-unsigned long num;
-char *end_p = NULL;

 ret = vshCommandOpt(cmd, name, arg);
 if (ret = 0)
@@ -18055,12 +18041,9 @@ vshCommandOptUL(const vshCmd *cmd, const char *name, 
unsigned long *value)
 return -2;
 }

-num = strtoul(arg-data, end_p, 10);
-if (arg-data != end_p  *end_p == 0) {
-*value = num;
-return 1;
-}
-return -1;
+if (virStrToLong_ul(arg-data, NULL, 10, value)  0)
+return -1;
+return 1;
 }

 /**
@@ -18112,8 +18095,6 @@ vshCommandOptLongLong(const vshCmd *cmd, const char 
*name,
 {
 vshCmdOpt *arg;
 int ret;
-long long num;
-char *end_p = NULL;

 ret = vshCommandOpt(cmd, name, arg);
 if (ret = 0)
@@ -18124,12 +18105,9 @@ vshCommandOptLongLong(const vshCmd *cmd, const char 
*name,
 return -2;
 }

-num = strtoll(arg-data, end_p, 10);
-if (arg-data != end_p  *end_p == 0) {
-*value = num;
-return 1;
-}
-return -1;
+if (virStrToLong_ll(arg-data, NULL, 10, value)  0)
+return -1;
+return 1;
 }

 /**
@@ -18147,8 +18125,6 @@ vshCommandOptULongLong(const vshCmd *cmd, const char 
*name,
 {
 vshCmdOpt *arg;
 int ret;
-unsigned long long num;
-char *end_p = NULL;

 ret = vshCommandOpt(cmd, name, arg);
 if (ret = 0)
@@ -18159,12 +18135,9 @@ vshCommandOptULongLong(const vshCmd *cmd, const char 
*name,
 return -2;
 }

-num = strtoull(arg-data, end_p, 10);
-if (arg-data != end_p  *end_p == 0) {
-*value = num;
-return 1;
-}
-return -1;
+if (virStrToLong_ull(arg-data, NULL, 10, value)  0)
+return -1;
+return 1;
 }


-- 
1.7.7.6

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


Re: [libvirt] [PATCH V11 2/7] Support for atomic operations on integers

2012-04-19 Thread Eric Blake
On 04/19/2012 12:19 PM, Stefan Berger wrote:
 On 04/19/2012 02:08 PM, Eric Blake wrote:
 On 04/17/2012 08:44 AM, Stefan Berger wrote:
 For threading support, add atomic add and sub operations working on
 integers. Base this on locking support provided by virMutex.

 virMutex is very heavyweight.  I'd love it if we could use gcc
 primitives and/or MSVC libc primitives (where they are known to be
 available), for a lighter-weight implementation on platforms that
 support it.  See this patch proposal (now a year old!) for some ideas
 that we should fold in before 0.9.12:

 https://www.redhat.com/archives/libvir-list/2011-April/msg00368.html

 Well, I wasn't aware of this previous proposal and had actually looked
 in pthread for some form of atomic support.
 I already pushed this patch now. We can add some compiler- and
 architecture-specific #ifdef's around the few existing functions. So the
 existing implementation probably has a place but can be improved upon.

No problem with the current implementation (don't go reverting anything
just because I came late to the review party!); I'm okay with future
patches for the improvements, especially since they will merely be
optimizations.  But I'm hoping we can get those optimizations in the
tree before 0.9.12, rather than later, now that it is fresh on our minds.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv5 07/23] blockjob: react to active block copy

2012-04-19 Thread Jiri Denemark
On Mon, Apr 16, 2012 at 23:05:58 -0600, Eric Blake wrote:
 For now, disk migration via block copy job is not implemented.  But
 when we do implement it, we have to deal with the fact that qemu does
 not provide an easy way to re-start a qemu process with mirroring
 still intact (it _might_ be possible by using qemu -S then an
 initial 'drive-mirror' with disk reuse before starting the domain,
 but that gets hairy).  Even something like 'virDomainSave' becomes
 hairy, if you realize the implications that 'virDomainRestore' would
 be stuck with recreating the same mirror layout.
 
 But if we step back and look at the bigger picture, we realize that
 the initial client of live storage migration via disk mirroring is
 oVirt, which always uses transient domains, and that if a transient
 domain is destroyed while a mirror exists, oVirt can easily restart
 the storage migration by creating a new domain that visits just the
 source storage, with no loss in data.
 
 We can make life a lot easier by being cowards, and forbidding
 certain operations on a domain.  This patch guarantees that we
 never get in a state where we would have to restart a domain with
 a mirroring block copy, by preventing saves, snapshots, hot
 unplug of a disk in use, and conversion to a persistent domain
 (thankfully, it is still relatively easy to 'virsh undefine' a
 running domain to temporarily make it transient, run tests on
 'virsh blockcopy', then 'virsh define' to restore the persistence).
 
 The change to qemudDomainDefine looks a bit odd for undoing an
 assignment, rather than probing up front to avoid the assignment,
 but this is because of how virDomainAssignDef combines both a
 lookup and assignment into a single function call.
 
 * src/conf/domain_conf.h (virDomainHasDiskMirror): New prototype.
 * src/conf/domain_conf.c (virDomainHasDiskMirror): New function.
 * src/libvirt_private.syms (domain_conf.h): Export it.
 * src/qemu/qemu_driver.c (qemuDomainSaveInternal)
 (qemuDomainSnapshotCreateXML, qemuDomainRevertToSnapshot)
 (qemuDomainBlockJobImpl, qemudDomainDefine): Prevent dangerous
 actions while block copy is already in action.
 * src/qemu/qemu_hotplug.c (qemuDomainDetachDiskDevice): Likewise.
 ---
 
 was 8/18 in v4
 v5: improve commit message
 
  src/conf/domain_conf.c   |   12 
  src/conf/domain_conf.h   |1 +
  src/libvirt_private.syms |1 +
  src/qemu/qemu_driver.c   |   29 +
  src/qemu/qemu_hotplug.c  |7 +++
  5 files changed, 50 insertions(+), 0 deletions(-)

OK, nice commit message improvement.

Jirka

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


[libvirt] [PATCH] Improve on virAtomic implementation

2012-04-19 Thread Stefan Berger

This patch improves on the previously added virAtomicInt operations
by testing for the compiler and if GCC = 4.1 (not found in docs prior to
that) is used on Linux and has the appropriate processor (that I have
access to) then use the implementation based on the gcc-builtins.

I also did not look at other systems (cygwin, win32) that do not need
access to virAtomic right now.

---
 src/util/viratomic.h |   88 
+--

 1 file changed, 72 insertions(+), 16 deletions(-)

Index: libvirt-acl/src/util/viratomic.h
===
--- libvirt-acl.orig/src/util/viratomic.h
+++ libvirt-acl/src/util/viratomic.h
@@ -30,6 +30,22 @@
 typedef struct _virAtomicInt virAtomicInt;
 typedef virAtomicInt *virAtomicIntPtr;

+# define __VIR_ATOMIC_USES_LOCK
+
+# if defined(__GNUC__)
+#  if ((__GNUC__ == 4)  (__GNUC_MINOR__ = 1)) || (__GNUC__  4)
+#   if defined(__linux__)
+#if defined(__i386__) || defined(__x86_64__) || \
+defined(__powerpc64__) || defined(__powerpc__)
+# undef __VIR_ATOMIC_USES_LOCK
+#endif
+#   endif
+#  endif
+# endif
+
+
+# ifdef __VIR_ATOMIC_USES_LOCK
+
 struct _virAtomicInt {
 virMutex lock;
 int value;
@@ -42,22 +58,6 @@ virAtomicIntInit(virAtomicIntPtr vaip)
 return virMutexInit(vaip-lock);
 }

-static inline void
-virAtomicIntSet(virAtomicIntPtr vaip, int value)
-{
- virMutexLock(vaip-lock);
-
- vaip-value = value;
-
- virMutexUnlock(vaip-lock);
-}
-
-static inline int
-virAtomicIntRead(virAtomicIntPtr vaip)
-{
- return vaip-value;
-}
-
 static inline int
 virAtomicIntAdd(virAtomicIntPtr vaip, int add)
 {
@@ -88,4 +88,60 @@ virAtomicIntSub(virAtomicIntPtr vaip, in
 return ret;
 }

+# else /* __VIR_ATOMIC_USES_LOCK */
+
+struct _virAtomicInt {
+int value;
+};
+
+static inline int
+virAtomicIntInit(virAtomicIntPtr vaip)
+{
+vaip-value = 0;
+return 0;
+}
+
+static inline int
+virAtomicIntAdd(virAtomicIntPtr vaip, int add)
+{
+return __sync_add_and_fetch(vaip-value, add);
+}
+
+static inline int
+virAtomicIntSub(virAtomicIntPtr vaip, int sub)
+{
+return __sync_sub_and_fetch(vaip-value, sub);
+}
+
+# endif /* __VIR_ATOMIC_USES_LOCK */
+
+
+
+/* common operations that need no locking or build on others */
+
+
+static inline void
+virAtomicIntSet(virAtomicIntPtr vaip, int value)
+{
+ vaip-value = value;
+}
+
+static inline int
+virAtomicIntRead(virAtomicIntPtr vaip)
+{
+ return *(volatile int *)vaip-value;
+}
+
+static inline int
+virAtomicIntInc(virAtomicIntPtr vaip)
+{
+return virAtomicIntAdd(vaip, 1);
+}
+
+static inline int
+virAtomicIntDec(virAtomicIntPtr vaip)
+{
+return virAtomicIntSub(vaip, 1);
+}
+
 #endif /* __VIR_ATOMIC_H */

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


Re: [libvirt] [PATCHv5 09/23] blockjob: return appropriate event and info

2012-04-19 Thread Jiri Denemark
On Mon, Apr 16, 2012 at 23:06:00 -0600, Eric Blake wrote:
 Handle the new type of block copy event and info.  Of course,
 this patch does nothing until a later patch actually allows the
 creation/abort of a block copy job.  And we'd really love to
 have an event without having to poll for the transition between
 pull and mirroring, but that will have to wait for qemu.
 
 * src/qemu/qemu_monitor_json.c (qemuMonitorJSONHandleBlockJobImpl)
 (qemuMonitorJSONGetBlockJobInfoOne): Translate new job type.
 * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Snoop a successful
 info query to save effort on a pivot request.
 ---
 
 was 10/18 in v4
 no real change
 
  src/qemu/qemu_driver.c   |6 ++
  src/qemu/qemu_monitor_json.c |4 
  2 files changed, 10 insertions(+), 0 deletions(-)

OK

Jirka

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


Re: [libvirt] [PATCHv5 10/23] blockjob: support pivot operation on cancel

2012-04-19 Thread Jiri Denemark
On Mon, Apr 16, 2012 at 23:06:01 -0600, Eric Blake wrote:
 This is the bare minimum to end a copy job (of course, until a
 later patch adds the ability to start a copy job, this patch
 doesn't do much in isolation; I've just split the patches to
 ease the review).
 
 This patch intentionally avoids SELinux, lock manager, and audit
 actions.  Also, if libvirtd restarts at the exact moment that a
 'drive-reopen' is in flight, the only proper way to detect the
 outcome of that 'drive-reopen' would be to first pass in a witness
 fd with 'getfd', then at libvirtd restart, probe whether that file
 is still empty.  This patch is enough to test the common case of
 success when used correctly, while saving the subtleties of proper
 cleanup for worst-case errors for later.
 
 When a mirror job is started, cancelling the job safely reverts back
 to the source disk, regardless of whether the destination is in
 phase 1 (streaming, in which case the destination is worthless) or
 phase 2 (mirroring, in which case the destination is synced up to
 the source at the time of the cancel).  Our existing code does just
 fine in either phase, other than some bookkeeping cleanup.
 
 Pivoting the job requires the use of the new 'drive-reopen' command.
 Here, failure of the command is potentially catastrophic to the
 domain, since the initial qemu implementation rips out the old disk
 before attempting to open the new one; qemu will attempt a recovery
 path of retrying the reopen on the original source, but if that also
 fails, the domain is hosed, with nothing libvirt can do about it.
 If qemu 1.2 ever adds 'drive-reopen' inside 'transaction', then the
 problem will no longer exist (a transaction promises not to close
 the old file until after the new file is proven to work), at which
 point we would add a VIR_DOMAIN_REBASE_COPY_ATOMIC that fails up
 front if we detect an older qemu with the risky drive-reopen.
 
 Interesting side note: while snapshot-create --disk-only creates a
 copy of the disk at a point in time by moving the domain on to a
 new file (the copy is the file now in the just-extended backing
 chain), blockjob --abort of a copy job creates a copy of the disk
 while keeping the domain on the original file.  There may be
 potential improvements to the snapshot code to exploit block copy
 over multiple disks all at one point in time.  And, if
 'block-job-cancel' were made part of 'transaction', you could
 copy multiple disks at the same point in time without pausing
 the domain.  This also implies we may want to add a --quiesce flag
 to the pivot operation, so that when breaking a mirror, the side
 of the mirror that we are abandoning is at least in a stable state
 with regards to guest I/O.
 
 * src/qemu/qemu_driver.c (qemuDomainBlockJobAbort): Accept new flag.
 (qemuDomainBlockPivot): New helper function.
 (qemuDomainBlockJobImpl): Implement it.
 ---
 
 was 11/18 in v4
 v5: no real change, improve commit message
 
  src/qemu/qemu_driver.c |  106 
 +++-
  1 files changed, 105 insertions(+), 1 deletions(-)

OK

Jirka

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


  1   2   >