Re: [libvirt] [PATCHv4 8/8] storage: probe qcow2 volumes in gluster pool

2013-11-26 Thread Daniel P. Berrange
On Mon, Nov 25, 2013 at 01:45:54PM -0700, Eric Blake wrote:
 On 11/25/2013 08:59 AM, Daniel P. Berrange wrote:
  On Fri, Nov 22, 2013 at 08:20:30PM -0700, Eric Blake wrote:
  Putting together pieces from previous patches, it is now possible
  for 'virsh vol-dumpxml --pool gluster volname' to report metadata
  about a qcow2 file stored on gluster.  The backing file is still
  treated as raw; to fix that, more patches are needed to make the
  storage backing chain analysis recursive rather than halting at
  a network protocol name, but that work will not need any further
  calls into libgfapi so much as just reusing this code, and that
  should be the only code outside of the storage driver that needs
  any help from libgfapi.  Any additional use of libgfapi within
  libvirt should only be needed for implementing storage pool APIs
  such as volume creation or resizing, where backing chain analysis
  should be unaffected.
 
 
  +while (maxlen) {
  +ssize_t r = glfs_read(fd, s, maxlen, 0);
  +if (r  0  errno == EINTR)
  +continue;
  +if (r  0) {
  +VIR_FREE(*buf);
  +virReportSystemError(errno, _(unable to read '%s'), name);
  +return r;
  +}
  
  Further down you're requesting O_NONBLOCK, and here you are
  not handling EAGAIN explicitly. Is is desirable that we turn
  EAGAIN into a fatal error, or should we remove the O_NONBLOCK
  flag ?
 
 Hmm.  I was copying from directory pools, which also use O_NONBLOCK then
 call into virFileReadHeaderFD.  So that code needs to be fixed (separate
 patch).  For gluster, I think it's easiest to just drop O_NONBLOCK from
 the code (I just verified that attempts to use 'mkfifo' inside a gluster
 volume fail with EACCES); for directory pools you DO want to open
 O_NONBLOCK (otherwise opening a fifo for read would hang waiting for a
 writer), then use virSetNonBlock() after verifying file type but before
 reading the header (since we already reject fifos as unable to be used
 as a storage volume).

Fortunately with local directory based pools, the only effect that
O_NONBLOCK has is to prevent you blocking on fifos. If you use
the O_NONBLOCK flag in conjunction with plain files, you'll still
block waiting for I/O, to the annoyance of programmers everywhere.

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-test-API][PATCH] Fix utils.exec_cmd output problem

2013-11-26 Thread Jincheng Miao
Thanks for review, 
yes, I missed this situation: stdout is not the subprocess.PIPE.

Since the stderr is always subprocess.PIPE, my another way is err after 
Popen.communicate().

The patch looks like:
---
 utils/utils.py | 2 ++---
 1 file changed, 2 insertions(+)

diff --git a/utils/utils.py b/utils/utils.py
index 147c1ef..d107cbd 100644
--- a/utils/utils.py
+++ b/utils/utils.py
@@ -412,6 +412,8 @@ def exec_cmd(command, sudo=False, cwd=None, infile=None, 
outfile=None, shell=Fal
 p = subprocess.Popen(command, shell=shell, close_fds=True, cwd=cwd,
 stdin=infile, stdout=outfile, stderr=subprocess.PIPE)
 (out, err) = p.communicate(data)
 if out == None:
 # Prevent splitlines() from barfing later on
 out = 
+if err != :
+out += err
 return (p.returncode, out.splitlines())
 
 def remote_exec_pexpect(hostname, username, password, cmd):
-- 
1.8.3.1

- Original Message -
 Sorry I missed your patch.
 
 The p.returncode can indicate the result of executing command unless you
 want the standard error.
 The subprocess.PIPE can ensure the variable out is always string type,
 but if the stdout is not the
 subprocess.PIPE, the variable out possibly be the type of None.
 so I think it is necessary to use the following code
 if out == None:
 out = 
 
 If you want to get standard error in the case of executing command
 failure. we need to consider other
 way.
 
 Guannan
 

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


Re: [libvirt] [libvirt-users] Help required in simulating libvirt TLS server

2013-11-26 Thread Daniel P. Berrange
On Mon, Nov 25, 2013 at 11:29:42PM +0530, Arun Viswanath wrote:
 Hi All,
 
 Will some one explain how is this tls libvirt server is implemented. For my
 testing purpose I need to implement the similar TLS server in Java or
 Python and this server is capable to receive all libvirt calls like
 getCapabilities, hostname etc and return response as I'm configured.
 Basically I need to simulate the libvirt TLS server. I tried creating many
 TLS server but none of the my TLS server implemenation is capable to do
 proper handshake with python libvirt client and do successful calls. Any
 ideas or help will be appreciable.

I really strongly recommend against trying to re-implement the libvirt
RPC protocol again. The protocol is pretty complex  easy to screw up.
We consider it a private implementation detail of libvirt and we won't
support use of libvirt client against any 3rd party server impl or vica
verca.

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] Entering freeze for 1.2.0, rc1 available

2013-11-26 Thread Daniel P. Berrange
On Tue, Nov 26, 2013 at 11:56:09AM +0800, Daniel Veillard wrote:
   So as planned we are entering freeze for libvirt-1.2.0,
 I just tagged the git trees - plural as we now use libvirt-python
 separate git for this binding - and pushed tarballs and rpms to
 the usual place at:
 
ftp://libvirt.org/libvirt/

I think you want to add a /python subdirectory there as we have
for the other language bindings, rather than putting it all in
the top level which is getting to be quite a large directory

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

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


Re: [libvirt] Entering freeze for 1.2.0, rc1 available

2013-11-26 Thread Daniel Veillard
On Tue, Nov 26, 2013 at 09:25:51AM +, Daniel P. Berrange wrote:
 On Tue, Nov 26, 2013 at 11:56:09AM +0800, Daniel Veillard wrote:
So as planned we are entering freeze for libvirt-1.2.0,
  I just tagged the git trees - plural as we now use libvirt-python
  separate git for this binding - and pushed tarballs and rpms to
  the usual place at:
  
 ftp://libvirt.org/libvirt/
 
 I think you want to add a /python subdirectory there as we have
 for the other language bindings, rather than putting it all in
 the top level which is getting to be quite a large directory

 Yes, definitely, but I think that for this release I will put
a symlinking back to the main dir, time for people to adapt :-)

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
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] [libvirt-python PATCH] Make setup.py executable

2013-11-26 Thread Martin Kletzander
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 setup.py | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 = 100755 setup.py

diff --git a/setup.py b/setup.py
old mode 100644
new mode 100755
-- 
1.8.4.3

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


[libvirt] [libvirt-python PATCH] setup: Make libvirt API XML path configurable

2013-11-26 Thread Martin Kletzander
Adding a support for LIBVIRT_API_PATH evironment variable, which can
control where the script should look for the 'libvirt-api.xml' file.
This allows building libvirt-python against different libvirt than the
one installed in the system.  This may be used for example in autotest
or by packagers without the need to install libvirt into the system.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 setup.py | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index 17b4722..566c210 100755
--- a/setup.py
+++ b/setup.py
@@ -109,7 +109,17 @@ class my_build(build):
 Check with pkg-config that libvirt is present and extract
 the API XML file paths we need from it

-libvirt_api = get_pkgconfig_data([--variable, libvirt_api], 
libvirt)
+libvirt_api = os.getenv(LIBVIRT_API_PATH)
+
+if libvirt_api:
+if not libvirt_api.endswith(-api.xml):
+raise ValueError(Invalid path '%s' for API XML % libvirt_api)
+if not os.path.exists(libvirt_api):
+raise ValueError(API XML '%s' does not exist, 
+ have you built libvirt? % libvirt_api)
+else:
+libvirt_api = get_pkgconfig_data([--variable, libvirt_api],
+ libvirt)

 offset = libvirt_api.index(-api.xml)
 libvirt_qemu_api = libvirt_api[0:offset] + -qemu-api.xml
-- 
1.8.4.3

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


Re: [libvirt] [PATCHv6 1/5] Add a hostdev PCI backend type

2013-11-26 Thread Laine Stump
On 11/26/2013 04:09 AM, Jim Fehlig wrote:
 When Chunyan originally submitted the patch adding PCI passthrough
 support in the libxl driver, it was noted that a lot of the code is
 not qemu-specific, based on the amount of copy-and-paste, and hence
 the request for common code :-).


Yes. It's definitely the right way to do it.


 Do you think a lot of the bugs in this code
 are specific to the qemu driver? I don't have a feel for the number of
 common vs hypervisor-specific bugs.



The bugs that I was thinking of were related to either vfio and/or to
binding/unbinding drivers to the devices, so they would be
hypervisor-agnostic.

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


Re: [libvirt] [libvirt-python PATCH] setup: Make libvirt API XML path configurable

2013-11-26 Thread Daniel P. Berrange
On Tue, Nov 26, 2013 at 10:58:25AM +0100, Martin Kletzander wrote:
 Adding a support for LIBVIRT_API_PATH evironment variable, which can
 control where the script should look for the 'libvirt-api.xml' file.
 This allows building libvirt-python against different libvirt than the
 one installed in the system.  This may be used for example in autotest
 or by packagers without the need to install libvirt into the system.
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  setup.py | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)
 
 diff --git a/setup.py b/setup.py
 index 17b4722..566c210 100755
 --- a/setup.py
 +++ b/setup.py
 @@ -109,7 +109,17 @@ class my_build(build):
  Check with pkg-config that libvirt is present and extract
  the API XML file paths we need from it
 
 -libvirt_api = get_pkgconfig_data([--variable, libvirt_api], 
 libvirt)
 +libvirt_api = os.getenv(LIBVIRT_API_PATH)
 +
 +if libvirt_api:
 +if not libvirt_api.endswith(-api.xml):
 +raise ValueError(Invalid path '%s' for API XML % 
 libvirt_api)
 +if not os.path.exists(libvirt_api):
 +raise ValueError(API XML '%s' does not exist, 
 + have you built libvirt? % libvirt_api)
 +else:
 +libvirt_api = get_pkgconfig_data([--variable, libvirt_api],
 + libvirt)
 
  offset = libvirt_api.index(-api.xml)
  libvirt_qemu_api = libvirt_api[0:offset] + -qemu-api.xml

NACK, setting pkg-config already takes care of this. See the
build-many.sh scrpit attached to this mail which demonstrates
use of PKG_CONFIG_PATH to build against every version of libvirt
back to 0.9.11

https://www.redhat.com/archives/libvir-list/2013-November/msg00933.html

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-python PATCH] Make setup.py executable

2013-11-26 Thread Daniel P. Berrange
On Tue, Nov 26, 2013 at 10:58:13AM +0100, Martin Kletzander wrote:
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  setup.py | 0
  1 file changed, 0 insertions(+), 0 deletions(-)
  mode change 100644 = 100755 setup.py
 
 diff --git a/setup.py b/setup.py
 old mode 100644
 new mode 100755

Normal python usage is documented as 'python setup.py' but I guess
this doesn't hurt.

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] VirtIO-SCSI disks limitation

2013-11-26 Thread Osier Yang

[[ TO libvir-list ]]

Hi, Daniel,

I'm going to share the thread to public list for further discussion. 
Hope you

don't mind.

On 26/11/13 02:37, Daniel Erez wrote:

Hi Osier,

It seems there's a limitation in libvirt that allows up to six disks in a
virtio-scsi controller. I.e. when sending more than six disks, libvirt
automatically creates a new controller but of type virtual LSI Logic SCSI.
Is this behavior a known issue?


For narrow SCSI bus, we allow 6 disks indeed.

For wide SCSI bus,  we allow 15 disks (not including the controller
itself on unit 7).

I'm doubting if we have problem on detecting if it supports wide SCSI
bus though, since as far as I see from the user cases, it's always
narrow SCSI bus.


Shouldn't libvirt allow up to 256 disks
per controller or at least create a new controller of type virtio-scsi when 
needed?


The controller model for virtio-scsi controller is lsilogic, which we can't
change simply, since it might affect the existing guests.

There was the similar discussion in libvir-list before [1].

But auto generation for controller is quite old, which I'm also not quite
clear about. I'd like see another discussion to make it more clear whether
we should do some work for upper layer app (e.g. oVirt).

Basicly two points:

*  Should we do some changes on the maximum units for a SCSI controller,
I.e. Should 7 (narrow bus) 16 (wide bus) be changed to other numbers?
I'm afraid the changes could affect existing guests though.

*  Do we really want to put the burden on users, I.E, let them create the
   controller explicitly. For use cases like one wants to add many 
disks for

   a guest, they need to know whether it's narrow SCSI bus or wide SCSI
   bus first (which we don't expose outside), and then do the calculation
   to know when to create a new SCSI controller.

@Daniel, am I correct on your problems?  Please comments if it doesn't
cover all your thoughts.

[1] http://www.redhat.com/archives/libvir-list/2012-November/msg00537.html

Regards,
Osier



[the issue as been discussed as part of: http://gerrit.ovirt.org/#/c/20630]

Thanks,
Daniel


- Original Message -

From: Dave Allan dal...@redhat.com
To: Daniel Erez de...@redhat.com
Cc: Ayal Baron aba...@redhat.com, Osier Yang jy...@redhat.com, John Ferlan 
jfer...@redhat.com
Sent: Monday, November 25, 2013 8:19:42 PM
Subject: Re: VirtIO-SCSI disks limitation

Hi Daniel,

Talk to Osier Yang and John Ferlan (cc'd).

Dave


On Mon, Nov 25, 2013 at 12:48:45PM -0500, Daniel Erez wrote:

Hi Dave,

I'm an engineer at oVirt team and I'm working on VirtIO-SCSI integration.
I would appreciate it if you could refer me to a point of contact at
libvirt.
In specific, I need to know if there's any hardcoded limitation for the
number of disks per VirtIO-SCSI controller.

Best Regards,
Daniel


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


Re: [libvirt] VirtIO-SCSI disks limitation

2013-11-26 Thread Daniel P. Berrange
On Tue, Nov 26, 2013 at 06:24:02PM +0800, Osier Yang wrote:
 [[ TO libvir-list ]]
 
 Hi, Daniel,
 
 I'm going to share the thread to public list for further discussion.
 Hope you
 don't mind.
 
 On 26/11/13 02:37, Daniel Erez wrote:
 Hi Osier,
 
 It seems there's a limitation in libvirt that allows up to six disks in a
 virtio-scsi controller. I.e. when sending more than six disks, libvirt
 automatically creates a new controller but of type virtual LSI Logic SCSI.
 Is this behavior a known issue?
 
 For narrow SCSI bus, we allow 6 disks indeed.
 
 For wide SCSI bus,  we allow 15 disks (not including the controller
 itself on unit 7).
 
 I'm doubting if we have problem on detecting if it supports wide SCSI
 bus though, since as far as I see from the user cases, it's always
 narrow SCSI bus.
 
 Shouldn't libvirt allow up to 256 disks
 per controller or at least create a new controller of type virtio-scsi when 
 needed?
 
 The controller model for virtio-scsi controller is lsilogic, which we can't
 change simply, since it might affect the existing guests.
 
 There was the similar discussion in libvir-list before [1].
 
 But auto generation for controller is quite old, which I'm also not quite
 clear about. I'd like see another discussion to make it more clear whether
 we should do some work for upper layer app (e.g. oVirt).
 
 Basicly two points:
 
 *  Should we do some changes on the maximum units for a SCSI controller,
 I.e. Should 7 (narrow bus) 16 (wide bus) be changed to other numbers?
 I'm afraid the changes could affect existing guests though.
 
 *  Do we really want to put the burden on users, I.E, let them create the
controller explicitly. For use cases like one wants to add many
 disks for
a guest, they need to know whether it's narrow SCSI bus or wide SCSI
bus first (which we don't expose outside), and then do the calculation
to know when to create a new SCSI controller.
 
 @Daniel, am I correct on your problems?  Please comments if it doesn't
 cover all your thoughts.

The logic for auto-assignment of disks to controllers is never going
to make everyone happy. If people really care about the mapping they
should define it explicitly by providing a controller address with
their disk XML.

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] Fix three minor typos

2013-11-26 Thread Osier Yang

On 26/11/13 15:15, Yuri Chornoivan wrote:



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


ACK and pushed. Thanks for the patch, but please consider posting
the patch with git-email.

Regards,
Osier

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


Re: [libvirt] [PATCH 0/3] SASL valgrind fixes

2013-11-26 Thread Christophe Fergeau
On Fri, Nov 22, 2013 at 01:00:59PM -0700, Eric Blake wrote:
 On 11/22/2013 10:00 AM, Christophe Fergeau wrote:
  Hey,
  
  While running virsh through valgrind for some SASL tests, I triggered some
  leaks/invalid reads, this patch series fixes these.
 
 ACK series.

I did not get to push these before the freeze, I assume this is still ok as
they were ACK'ed before, and as they also are bug fixes? I prefer to
double-check it's fine before pushing :)

Christophe


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

Re: [libvirt] [PATCH 0/3] SASL valgrind fixes

2013-11-26 Thread Daniel P. Berrange
On Tue, Nov 26, 2013 at 11:41:12AM +0100, Christophe Fergeau wrote:
 On Fri, Nov 22, 2013 at 01:00:59PM -0700, Eric Blake wrote:
  On 11/22/2013 10:00 AM, Christophe Fergeau wrote:
   Hey,
   
   While running virsh through valgrind for some SASL tests, I triggered some
   leaks/invalid reads, this patch series fixes these.
  
  ACK series.
 
 I did not get to push these before the freeze, I assume this is still ok as
 they were ACK'ed before, and as they also are bug fixes? I prefer to
 double-check it's fine before pushing :)

Yes, for bugfixes like this it is fine.


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 v3] sasl: Fix authentication when using PLAIN mechanism

2013-11-26 Thread Christophe Fergeau
On Fri, Nov 22, 2013 at 12:58:27PM -0700, Eric Blake wrote:
 On 11/22/2013 10:26 AM, Christophe Fergeau wrote:
  With some authentication mechanism (PLAIN for example), sasl_client_start()
  can return SASL_OK, which translates to virNetSASLSessionClientStart()
  returning VIR_NET_SASL_COMPLETE.
  cyrus-sasl documentation is a bit vague as to what to do in such situation,
  but upstream clarified this a bit in
  http://asg.andrew.cmu.edu/archive/message.php?mailbox=archive.cyrus-saslmsg=10104
  
  When we got VIR_NET_SASL_COMPLETE after virNetSASLSessionClientStart() and
  if the remote also tells us that authentication is complete, then we should
  end the authentication procedure rather than forcing a call to
  virNetSASLSessionClientStep(). Without this patch, when trying to use SASL
  PLAIN, I get:
  error :authentication failed : Failed to step SASL negotiation: -1
  (SASL(-1): generic failure: Unable to find a callback: 32775)
  
  This patch is based on a spice-gtk patch by Dietmar Maurer.
  ---
  Change since v2:
- move the added test out of the for(;;) loop
 
   /* Loop-the-loop...
  - * Even if the server has completed, the client must *always* do at 
  least one step
  - * in this loop to verify the server isn't lying about something. 
  Mutual auth */
  + * Even if the server has completed, the client must loop until 
  sasl_client_start() or
  + * sasl_client_step() return SASL_OK to verify the server isn't lying
  + * about something. Mutual auth
  + * */
   for (;;) {
  +
 
 This blank line seems spurious
 
   restep:
 
 now that you aren't modifying the head of the loop, you could follow my
 earlier suggestion of dropping the 'restep' label and replacing 'goto
 restep' with 'continue'.  But that's trivial, so I don't care either
 way, and don't need to see a v4 if you choose to change before pushing.
 
 ACK.

I've pushed this after removing the blank line, and I've pushed an
additional patch replacing restep with continue;

Christophe


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

Re: [libvirt] [PATCH 0/3] SASL valgrind fixes

2013-11-26 Thread Christophe Fergeau
On Tue, Nov 26, 2013 at 10:43:15AM +, Daniel P. Berrange wrote:
 On Tue, Nov 26, 2013 at 11:41:12AM +0100, Christophe Fergeau wrote:
  On Fri, Nov 22, 2013 at 01:00:59PM -0700, Eric Blake wrote:
   On 11/22/2013 10:00 AM, Christophe Fergeau wrote:
Hey,

While running virsh through valgrind for some SASL tests, I triggered 
some
leaks/invalid reads, this patch series fixes these.
   
   ACK series.
  
  I did not get to push these before the freeze, I assume this is still ok as
  they were ACK'ed before, and as they also are bug fixes? I prefer to
  double-check it's fine before pushing :)
 
 Yes, for bugfixes like this it is fine.

Thanks, I've pushed this now.

Christophe


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

Re: [libvirt] [RFC PATCH] libvirt support to force convergence of live guest migration

2013-11-26 Thread Jiri Denemark
On Thu, Nov 21, 2013 at 17:47:24 -0800, Chegu Vinod wrote:
 Busy enterprise workloads hosted on large sized VM's tend to dirty
 memory faster than the transfer rate achieved via live guest migration.
 Despite some good recent improvements ( using dedicated 10Gig NICs
 between hosts) the live migration may NOT converge.
 
 Recently support was added in qemu (version 1.6) to allow a user to
 choose if they wish to force convergence of their migration via a
 new migration capability : auto-converge. This feature allows for qemu
 to auto-detect lack of convergence and trigger a throttle-down of the
 VCPUs. 
 
 This RFC patch includes the libvirt support needed to trigger this
 feature. (Testing is still in progress)
 
 Signed-off-by:  Chegu Vinod chegu_vi...@hp.com
 ---
  include/libvirt/libvirt.h.in |1 +
  src/qemu/qemu_migration.c|   44 
 ++
  src/qemu/qemu_migration.h|1 +
  src/qemu/qemu_monitor.c  |2 +-
  src/qemu/qemu_monitor.h  |1 +
  tools/virsh-domain.c |7 ++
  6 files changed, 55 insertions(+), 1 deletions(-)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 146a59b..13b0bfc 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -1192,6 +1192,7 @@ typedef enum {
  VIR_MIGRATE_OFFLINE   = (1  10), /* offline migrate */
  VIR_MIGRATE_COMPRESSED= (1  11), /* compress data during 
 migration */
  VIR_MIGRATE_ABORT_ON_ERROR= (1  12), /* abort migration on I/O 
 errors happened during migration */
 +VIR_MIGRATE_AUTO_CONVERGE = (1  13), /* force auto-convergence 
 during during migration */
  } virDomainMigrateFlags;

I feel like there must be a better name we could use for this flag but
I'm not able to come up with one... :-)

 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index e87ea85..8cc0c56 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -1565,6 +1565,40 @@ cleanup:
  }
  
  static int
 +qemuMigrationSetAutoConverge(virQEMUDriverPtr driver,
 +virDomainObjPtr vm,
 +enum qemuDomainAsyncJob job)

Nicely copiedpasted from qemuMigrationSetCompression but you forgot to
fix indentation :-)

 +{
 +qemuDomainObjPrivatePtr priv = vm-privateData;
 +int ret;
 +
 +if (qemuDomainObjEnterMonitorAsync(driver, vm, job)  0)
 +return -1;
 +
 +ret = qemuMonitorGetMigrationCapability(
 +priv-mon,
 +QEMU_MONITOR_MIGRATION_CAPS_AUTO_CONVERGE);
 +
 +if (ret  0) {
 +goto cleanup;
 +} else if (ret == 0) {
 +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
 +   _(Auto-Converge migration is not supported by 
 + QEMU binary));

Reduce indentation by one level. Also I think migration is not really
needed in the error message, I'd just change it to
Auto-converge is not supported by QEMU binary.

 +ret = -1;
 +goto cleanup;
 +}
 +
 +ret = qemuMonitorSetMigrationCapability(
 +priv-mon,
 +QEMU_MONITOR_MIGRATION_CAPS_AUTO_CONVERGE);
 +
 +cleanup:
 +qemuDomainObjExitMonitor(driver, vm);
 +return ret;
 +}
 +
 +static int
  qemuMigrationWaitForSpice(virQEMUDriverPtr driver,
virDomainObjPtr vm)
  {
 @@ -2389,6 +2423,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
  QEMU_ASYNC_JOB_MIGRATION_IN)  0)
  goto stop;
  
 +if (flags  VIR_MIGRATE_AUTO_CONVERGE 
 +qemuMigrationSetAutoConverge(driver, vm,
 +QEMU_ASYNC_JOB_MIGRATION_IN)  0)
 +goto stop;
 +

Hmm, I know you are just following what I did with
VIR_MIGRATE_COMPRESSED, but setting auto-converge on destination doesn't
make any sense. And it doesn't even make a lot of sense to set
compression on destination (other than checking the destination supports
compression) so I'm wondering why I did so.

  if (mig-lockState) {
  VIR_DEBUG(Received lockstate %s, mig-lockState);
  VIR_FREE(priv-lockState);
 @@ -3181,6 +3220,11 @@ qemuMigrationRun(virQEMUDriverPtr driver,
  QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
  goto cleanup;
  
 +if (flags  VIR_MIGRATE_AUTO_CONVERGE 
 +qemuMigrationSetAutoConverge(driver, vm,
 +QEMU_ASYNC_JOB_MIGRATION_OUT)  0)

Indentation is off by one space.

 +goto cleanup;
 +
  if (qemuDomainObjEnterMonitorAsync(driver, vm,
 QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
  goto cleanup;
...

So except for the small issues, the patch looks good to me. However, do
I remember correctly that this feature can be turned on dynamically for
an already running migration? If so, I think we want a second patch
adding a 

Re: [libvirt] [RFC PATCH] libvirt support to force convergence of live guest migration

2013-11-26 Thread Paolo Bonzini
Il 26/11/2013 13:24, Jiri Denemark ha scritto:
 Hmm, I know you are just following what I did with
 VIR_MIGRATE_COMPRESSED, but setting auto-converge on destination doesn't
 make any sense. And it doesn't even make a lot of sense to set
 compression on destination (other than checking the destination supports
 compression) so I'm wondering why I did so.

FWIW, when new capabilities pop up, I'm trying to enforce that they only
need to be set in the source and that (if they affect the destination at
all) the destination should automatically discover them through the
migration stream.

query-migrate-capabilities can be used to find out if the destination
knows about the capability.

Paolo

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


Re: [libvirt] VirtIO-SCSI disks limitation

2013-11-26 Thread Daniel Erez


- Original Message -
 From: Osier Yang jy...@redhat.com
 To: libvir-list@redhat.com
 Cc: Daniel Erez de...@redhat.com
 Sent: Tuesday, November 26, 2013 12:24:02 PM
 Subject: Re: VirtIO-SCSI disks limitation
 
 [[ TO libvir-list ]]
 
 Hi, Daniel,
 
 I'm going to share the thread to public list for further discussion.
 Hope you
 don't mind.
 
 On 26/11/13 02:37, Daniel Erez wrote:
  Hi Osier,
 
  It seems there's a limitation in libvirt that allows up to six disks in a
  virtio-scsi controller. I.e. when sending more than six disks, libvirt
  automatically creates a new controller but of type virtual LSI Logic SCSI.
  Is this behavior a known issue?
 
 For narrow SCSI bus, we allow 6 disks indeed.
 
 For wide SCSI bus,  we allow 15 disks (not including the controller
 itself on unit 7).
 
 I'm doubting if we have problem on detecting if it supports wide SCSI
 bus though, since as far as I see from the user cases, it's always
 narrow SCSI bus.
 
  Shouldn't libvirt allow up to 256 disks
  per controller or at least create a new controller of type virtio-scsi when
  needed?
 
 The controller model for virtio-scsi controller is lsilogic, which we can't
 change simply, since it might affect the existing guests.
 
 There was the similar discussion in libvir-list before [1].
 
 But auto generation for controller is quite old, which I'm also not quite
 clear about. I'd like see another discussion to make it more clear whether
 we should do some work for upper layer app (e.g. oVirt).
 
 Basicly two points:
 
 *  Should we do some changes on the maximum units for a SCSI controller,
  I.e. Should 7 (narrow bus) 16 (wide bus) be changed to other numbers?
  I'm afraid the changes could affect existing guests though.
 
 *  Do we really want to put the burden on users, I.E, let them create the
 controller explicitly. For use cases like one wants to add many
 disks for
 a guest, they need to know whether it's narrow SCSI bus or wide SCSI
 bus first (which we don't expose outside), and then do the calculation
 to know when to create a new SCSI controller.
 
 @Daniel, am I correct on your problems?  Please comments if it doesn't
 cover all your thoughts.

So, IIUC, we can't reach the limitation of 256 disks per controller?
I.e. we must explicitly ask for a new controller every 6/15 disks?
BTW, how do I define in the XML the bus type (narrow/wide)?

 
 [1] http://www.redhat.com/archives/libvir-list/2012-November/msg00537.html
 
 Regards,
 Osier
 
 
  [the issue as been discussed as part of: http://gerrit.ovirt.org/#/c/20630]
 
  Thanks,
  Daniel
 
 
  - Original Message -
  From: Dave Allan dal...@redhat.com
  To: Daniel Erez de...@redhat.com
  Cc: Ayal Baron aba...@redhat.com, Osier Yang jy...@redhat.com,
  John Ferlan jfer...@redhat.com
  Sent: Monday, November 25, 2013 8:19:42 PM
  Subject: Re: VirtIO-SCSI disks limitation
 
  Hi Daniel,
 
  Talk to Osier Yang and John Ferlan (cc'd).
 
  Dave
 
 
  On Mon, Nov 25, 2013 at 12:48:45PM -0500, Daniel Erez wrote:
  Hi Dave,
 
  I'm an engineer at oVirt team and I'm working on VirtIO-SCSI integration.
  I would appreciate it if you could refer me to a point of contact at
  libvirt.
  In specific, I need to know if there's any hardcoded limitation for the
  number of disks per VirtIO-SCSI controller.
 
  Best Regards,
  Daniel
 
 

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


Re: [libvirt] [libvirt-python PATCH] Make setup.py executable

2013-11-26 Thread Martin Kletzander
On Tue, Nov 26, 2013 at 10:15:50AM +, Daniel P. Berrange wrote:
 On Tue, Nov 26, 2013 at 10:58:13AM +0100, Martin Kletzander wrote:
  Signed-off-by: Martin Kletzander mklet...@redhat.com
  ---
   setup.py | 0
   1 file changed, 0 insertions(+), 0 deletions(-)
   mode change 100644 = 100755 setup.py
  
  diff --git a/setup.py b/setup.py
  old mode 100644
  new mode 100755
 
 Normal python usage is documented as 'python setup.py' but I guess
 this doesn't hurt.
 

I could've mentioned that this is just for an ease of use when doing
something by hand.  Some projects use it, some don't.

I pushed it, thanks.

Martin


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

Re: [libvirt] [PATCH v2] Pin guest to memory node on NUMA system

2013-11-26 Thread Shivaprasad bhat
Thanks Martin. I use stgit. I need to figure out how to put the notes
in the format you mentioned.

Sending you the V3 addressing your comments.

Regards,
Shiva

On Mon, Nov 25, 2013 at 3:01 PM, Martin Kletzander mklet...@redhat.com wrote:
 On Tue, Nov 19, 2013 at 09:36:49AM -0500, Shivaprasad G Bhat wrote:
 Version 2:
 Fixed the string formatting errors in v1.

 Version 1:
 The patch contains the fix for defect 1009880 reported at redhat bugzilla.


 Version changes are great to have, but it's good to add them as a
 'footnote' or as 'notes' (separated from the commit with three dashes
 on a line, i.e. '---\n').  You can also use git-notes(1) and then use
 git-format-patch(1) with the '--notes' option, which adds it there
 properly.

 The root cause is, ever since the subcpusets(vcpu,emulator) were introduced, 
 the paren cpuset cannot be modified to remove the nodes that are in use by 
 the subcpusets.
 The fix is to break the memory node modification into three steps as to 
 assign new nodes into the parent first. Change the nodes in the child nodes. 
 Then remove the old nodes on the parent node.


 Please make sure your EDITOR wraps lines, so it's better readable and
 it also looks better in git log.

 Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com
 ---
  src/qemu/qemu_driver.c |  129 
 
  1 file changed, 129 insertions(+)

 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index e8bc04d..2435b75 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -8171,7 +8171,11 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
  }
  } else if (STREQ(param-field, VIR_DOMAIN_NUMA_NODESET)) {
  virBitmapPtr nodeset = NULL;
 +virBitmapPtr old_nodeset = NULL;
 +virBitmapPtr temp_nodeset = NULL;
  char *nodeset_str = NULL;
 +char *old_nodeset_str = NULL;
 +char *temp_nodeset_str = NULL;

  if (virBitmapParse(params[i].value.s,
 0, nodeset,
 @@ -8181,6 +8185,10 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
  }

  if (flags  VIR_DOMAIN_AFFECT_LIVE) {
 +size_t j;
 +virCgroupPtr cgroup_vcpu = NULL;
 +virCgroupPtr cgroup_emulator = NULL;
 +
  if (vm-def-numatune.memory.mode !=
  VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
  virReportError(VIR_ERR_OPERATION_INVALID, %s,
 @@ -8200,7 +8208,128 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
  continue;
  }

 +if (virCgroupGetCpusetMems(priv-cgroup, old_nodeset_str) 
  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Failed to get current system nodeset 
 values));

 No need to report second error since virCgroup...() already set the error.

 +virBitmapFree(nodeset);
 +VIR_FREE(nodeset_str);
 +ret = -1;
 +continue;
 +}
 +
 +if (virBitmapParse(old_nodeset_str, 0, old_nodeset,
 +   VIR_DOMAIN_CPUMASK_LEN)  0) {
 +virBitmapFree(nodeset);
 +VIR_FREE(nodeset_str);
 +VIR_FREE(old_nodeset_str);
 +ret = -1;
 +continue;
 +}
 +
 +if ((temp_nodeset = virBitmapNewCopy(old_nodeset)) == NULL) 
 {
 +virBitmapFree(nodeset);
 +VIR_FREE(nodeset_str);
 +virBitmapFree(old_nodeset);
 +VIR_FREE(old_nodeset_str);
 +ret = -1;
 +continue;

 Just skimming through, I see a lot of unnecessary code duplication
 here.  Can you break it to different function?  Or rework the code to
 free/clean the data in one place, since there is more than a few lines
 of code in this workpath?

 Martin

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


[libvirt] [PATCH v3] Pin guest to memory node on NUMA system

2013-11-26 Thread Shivaprasad G Bhat
Version 3:
Addressed comments on V2.

Version 2:
Fixed the string formatting errors in v1.

The patch contains the fix for defect 1009880 reported at redhat bugzilla.
The root cause is, ever since the subcpusets(vcpu,emulator) were introduced, the
parent cpuset cannot be modified to remove the nodes that are in use by the
subcpusets.
The fix is to break the memory node modification into three steps as to assign
new nodes into the parent first. Change the nodes in the child nodes. Then
remove the old nodes on the parent node.

Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com
---
 src/qemu/qemu_driver.c |  115 ++--
 1 file changed, 110 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8a1eefd..4bc9d1d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8132,6 +8132,47 @@ cleanup:
 }
 
 static int
+qemuSetVcpuCpusetMems(virDomainObjPtr vm,
+  char *nodeset_str)
+{
+size_t j = 0;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+virCgroupPtr cgroup_vcpu = NULL;
+
+for (j = 0; j  priv-nvcpupids; j++) {
+if (virCgroupNewVcpu(priv-cgroup, j, false, cgroup_vcpu)  0) {
+return -1;
+}
+if (virCgroupSetCpusetMems(cgroup_vcpu, nodeset_str)  0) {
+virCgroupFree(cgroup_vcpu);
+return -1;
+}
+virCgroupFree(cgroup_vcpu);
+}
+
+return 0;
+}
+
+static int
+qemuSetEmulatorCpusetMems(virDomainObjPtr vm,
+  char *nodeset_str)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+virCgroupPtr cgroup_emulator = NULL;
+
+if (virCgroupNewEmulator(priv-cgroup, false, cgroup_emulator)  0) {
+return -1;
+}
+if (virCgroupSetCpusetMems(cgroup_emulator, nodeset_str)  0) {
+virCgroupFree(cgroup_emulator);
+return -1;
+}
+virCgroupFree(cgroup_emulator);
+
+return 0;
+}
+
+static int
 qemuDomainSetNumaParameters(virDomainPtr dom,
 virTypedParameterPtr params,
 int nparams,
@@ -8198,7 +8239,11 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
 }
 } else if (STREQ(param-field, VIR_DOMAIN_NUMA_NODESET)) {
 virBitmapPtr nodeset = NULL;
+virBitmapPtr old_nodeset = NULL;
+virBitmapPtr temp_nodeset = NULL;
 char *nodeset_str = NULL;
+char *old_nodeset_str = NULL;
+char *temp_nodeset_str = NULL;
 
 if (virBitmapParse(params[i].value.s,
0, nodeset,
@@ -8208,32 +8253,92 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
 }
 
 if (flags  VIR_DOMAIN_AFFECT_LIVE) {
+size_t j;
+
 if (vm-def-numatune.memory.mode !=
 VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(change of nodeset for running domain 
  requires strict numa mode));
-virBitmapFree(nodeset);
 ret = -1;
-continue;
+goto next;
 }
 
 /* Ensure the cpuset string is formated before passing to 
cgroup */
 if (!(nodeset_str = virBitmapFormat(nodeset))) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Failed to format nodeset));
-virBitmapFree(nodeset);
 ret = -1;
-continue;
+goto next;
+}
+
+/*Get Exisitng nodeset values */
+if (virCgroupGetCpusetMems(priv-cgroup, old_nodeset_str)  
0) {
+ret = -1;
+goto next;
+}
+if (virBitmapParse(old_nodeset_str, 0, old_nodeset,
+   VIR_DOMAIN_CPUMASK_LEN)  0){
+ret = -1;
+goto next;
+}
+
+/* Merge the existing and new nodeset values */
+if ((temp_nodeset = virBitmapNewCopy(old_nodeset)) == NULL) {
+ret = -1;
+goto next;
+}
+
+for (j = 0; j  caps-host.nnumaCell; j++) {
+bool result;
+if (virBitmapGetBit(nodeset, j, result)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Failed to get cpuset bit values));
+ret = -1;
+goto next;
+}
+if (result  (virBitmapSetBit(temp_nodeset, j)  0)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Failed to set 

Re: [libvirt] [Qemu-devel] [PATCH] virtio-rng: correct the default limit rate

2013-11-26 Thread Eric Blake
[adding libvirt]

On 11/26/2013 06:58 AM, Paolo Bonzini wrote:
 Il 26/11/2013 14:43, Amos Kong ha scritto:
 /* Set a default rate limit of 2^47 bytes per minute or roughly 2TB/s.  If
  * you have an entropy source capable of generating more entropy than this
  * and you can pass it through via virtio-rng, then hats off to you.  Until
  * then, this is unlimited for all practical purposes.
  */

 But the current rate is (INT64_MAX) bytes per (1  16) ms, it's 128,000 TB/s
 
 You are changing:
 
 * max-bytes from 2^63 to 2^47
 
 * period from 65536 to 6
 
 For a user, changing only period would have no effect, the limit rate
 would remain effectively infinite.  Changing max-bytes would give a 7%
 higher rate after your patch.
 
 Not a big deal, and max-bytes is easier to explain after your patch
 (bytes/minute) than before (bytes/65536ms).
 
 Reviewed-by: Paolo Bonzini pbonz...@redhat.com
 

Hmm.  Libvirt is already converting a user's rate of bytes/period into
the qemu parameters, defaulting to 1 second as its default period.  Am I
correct that as long as libvirt specified both rate AND period, then
this change has no impact (and that the 7% change occurs if you specify
period while leaving max-bytes alone)?  Or is this an ABI change where
libvirt will have to be taught to be smart enough to know whether it is
old qemu or new qemu to adjust how libvirt does its calculations when
converting the user's rate into qemu terms?

-- 
Eric Blake   eblake 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] [libvirt-python PATCH] setup: Make libvirt API XML path configurable

2013-11-26 Thread Martin Kletzander
On Tue, Nov 26, 2013 at 10:14:36AM +, Daniel P. Berrange wrote:
 On Tue, Nov 26, 2013 at 10:58:25AM +0100, Martin Kletzander wrote:
  Adding a support for LIBVIRT_API_PATH evironment variable, which can
  control where the script should look for the 'libvirt-api.xml' file.
  This allows building libvirt-python against different libvirt than the
  one installed in the system.  This may be used for example in autotest
  or by packagers without the need to install libvirt into the system.
  
  Signed-off-by: Martin Kletzander mklet...@redhat.com
  ---
   setup.py | 12 +++-
   1 file changed, 11 insertions(+), 1 deletion(-)
  
  diff --git a/setup.py b/setup.py
  index 17b4722..566c210 100755
  --- a/setup.py
  +++ b/setup.py
  @@ -109,7 +109,17 @@ class my_build(build):
   Check with pkg-config that libvirt is present and extract
   the API XML file paths we need from it
  
  -libvirt_api = get_pkgconfig_data([--variable, libvirt_api], 
  libvirt)
  +libvirt_api = os.getenv(LIBVIRT_API_PATH)
  +
  +if libvirt_api:
  +if not libvirt_api.endswith(-api.xml):
  +raise ValueError(Invalid path '%s' for API XML % 
  libvirt_api)
  +if not os.path.exists(libvirt_api):
  +raise ValueError(API XML '%s' does not exist, 
  + have you built libvirt? % libvirt_api)
  +else:
  +libvirt_api = get_pkgconfig_data([--variable, libvirt_api],
  + libvirt)
  
   offset = libvirt_api.index(-api.xml)
   libvirt_qemu_api = libvirt_api[0:offset] + -qemu-api.xml
 
 NACK, setting pkg-config already takes care of this. See the
 build-many.sh scrpit attached to this mail which demonstrates
 use of PKG_CONFIG_PATH to build against every version of libvirt
 back to 0.9.11
 

This still means you have to configure libvirt with different prefix,
install it and then you can use PKG_CONFIG_PATH.  This variable (which
is unused if unset) makes it easier to use in case you have it built
with default prefix etc.  It would help me a lot, but if everyone else
is OK with installing libvirt in order to build python bindings just
to test something, I'll keep this in my git.

Martin


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

Re: [libvirt] [Qemu-devel] [PATCH] virtio-rng: correct the default limit rate

2013-11-26 Thread Paolo Bonzini
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 26/11/2013 15:32, Eric Blake ha scritto:
 Hmm.  Libvirt is already converting a user's rate of bytes/period
 into the qemu parameters, defaulting to 1 second as its default
 period.  Am I correct that as long as libvirt specified both rate
 AND period, then this change has no impact (and that the 7% change
 occurs if you specify period while leaving max-bytes alone)?  Or is
 this an ABI change where libvirt will have to be taught to be smart
 enough to know whether it is old qemu or new qemu to adjust how
 libvirt does its calculations when converting the user's rate into
 qemu terms?

Yes, it is the former---i.e. you're right.

Paolo
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSlLPjAAoJEBvWZb6bTYbyDg8QAIk8BIh5ZY0w4aF7AHufxpmn
XtvbGIAdb+ztNBtPrL5HJXAvaOvbZNfktmKDA7Cee1Cq8opJPUZQDpwmv7xUshpG
yc1YQu3Djb0/kCDV0oNiMQARSwxJ3XMR94SSLfmlI3b7lJVDeviouhV4UdjS3eal
ZNWUXya+1bloOC+zX4P0atB5hyrpVhcVHddWLsIs4+6EYcOatHPgl5CK5f5uxq+P
5qkAe1FJTHMkBOJ86kis3s57ZTTqIlkvFiqtJY4IViifHjbMR4PJraO6EQigOxf6
VdV+MIkPm3GA6F3SYOoZNacITBsaRXTJ0kjD+BjUUAFONB7X8//Uob5gSUgmkED7
ATqiDnqSq1jwyV4FAVwYDB397ViLAZYd9k/ai4QZLaclyMqHQKx+RCNN7dmFwCuL
EYK0Z9AkoVkJJQQUcERO3ps/T89+tlBVQhBKpae1V2ZKJqCGxLaf0344S+kFicYk
bqp87ra5L5J8Qdn+/eBMJsQNa5NDWjKU0TvZSr3tGGoGKKamzwFmnsTzcZ9DHoTZ
IVcdHAbToCYnqbnJzORoxFVycSrY8B2AhZOqIz5YQgNV1LDPcK7hzcXZ+rpBu5mU
zF9MFHBKrGJo7l4GyC4jFa5osDfGOyGzMyoSvlMuFTG8CUXN/QLmcpzE+NL1QlCj
lL7TLkbhYKrQxUPw37+P
=mb5e
-END PGP SIGNATURE-

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


[libvirt] newer libvirt version issues

2013-11-26 Thread Franky Van Liedekerke


Hi,

since we're running more than 20 hosts per KVM server, we needed to 
update libvirt to at least 1.1.2 for virtlockd to be able to cope with 
this (due to an old hardcoded limit that was in there before).
But where 1.1.0 compiles and runs just fine on a fully-patched CentOS 
6.4 server, newer version have all kinds of issues:


Every version of libvirt = 1.1.2 crashes with a segfault on up-to-date 
CentOS 6.4 servers (see below).
Also, versions of libvirt = 1.1.3 need a change in the spec file to be 
able to compile:
apparently the %doc entries that are different with 1.1.2 prevent 
rpmbuild to succeed on
CentOS servers. Using the %doc lines as per the 1.1.2 spec file results 
in a working rpmbuild.
The 1.1.4 version has issues with a symbol not found after compiling 
and letting it run:


error : virDriverLoadModule:78 : failed to load module 
/usr/lib64/libvirt/connection-driver/libvirt_driver_network.so 
/usr/lib64/libvirt/connection-driver/libvirt_driver_network.so: 
undefined symbol: virNetworkList
error : virDriverLoadModule:78 : failed to load module 
/usr/lib64/libvirt/connection-driver/libvirt_driver_storage.so 
/usr/lib64/libvirt/connection-driver/libvirt_driver_storage.so: 
undefined symbol: virAsprintf
error : virDriverLoadModule:78 : failed to load module 
/usr/lib64/libvirt/connection-driver/libvirt_driver_nodedev.so 
/usr/lib64/libvirt/connection-driver/libvirt_driver_nodedev.so: 
undefined symbol: virNodeDeviceList
31677: error : virDriverLoadModule:78 : failed to load module 
/usr/lib64/libvirt/connection-driver/libvirt_driver_secret.so 
/usr/lib64/libvirt/connection-driver/libvirt_driver_secret.so: undefined 
symbol: virAsprintf
31677: error : virDriverLoadModule:78 : failed to load module 
/usr/lib64/libvirt/connection-driver/libvirt_driver_nwfilter.so 
/usr/lib64/libvirt/connection-driver/libvirt_driver_nwfilter.so: 
undefined symbol: virAsprintf
31677: error : virDriverLoadModule:78 : failed to load module 
/usr/lib64/libvirt/connection-driver/libvirt_driver_interface.so 
/usr/lib64/libvirt/connection-driver/libvirt_driver_interface.so: 
undefined symbol: virAsprintf
31677: error : virDriverLoadModule:78 : failed to load module 
/usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so 
/usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so: undefined 
symbol: networkAllocateActualDevice
31677: error : virDriverLoadModule:78 : failed to load module 
/usr/lib64/libvirt/connection-driver/libvirt_driver_lxc.so 
/usr/lib64/libvirt/connection-driver/libvirt_driver_lxc.so: undefined 
symbol: networkAllocateActualDevice


And every time libvirt segfaults, there's this in the logs:
debug : virJSONValueToString:1133 : 
result={id:libvirt-6,error:{class:CommandNotFound,desc:The 
command qom-list has not been found,data:{name:qom-list}}}
debug : virEventPollRunOnce:627 : EVENT_POLL_RUN: nhandles=32 
timeout=1684
9479: debug : qemuMonitorJSONCheckError:341 : unable to execute QEMU 
command 
{execute:qom-list,arguments:{path:/},id:libvirt-6}: 
{id:libvirt-6,error:{class:CommandNotFound,desc:The command 
qom-list has not been found,data:{name:qom-list}}}
9479: error : qemuMonitorJSONCheckError:352 : internal error: unable to 
execute QEMU command 'qom-list': The command qom-list has not been found


Any insights on any of these issues?

Franky


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


Re: [libvirt] newer libvirt version issues

2013-11-26 Thread Eric Blake
On 11/26/2013 08:06 AM, Franky Van Liedekerke wrote:

 apparently the %doc entries that are different with 1.1.2 prevent
 rpmbuild to succeed on
 CentOS servers. Using the %doc lines as per the 1.1.2 spec file results
 in a working rpmbuild.

Should be fixed in caaeb69.

 The 1.1.4 version has issues with a symbol not found after compiling and
 letting it run:
 
 error : virDriverLoadModule:78 : failed to load module
 /usr/lib64/libvirt/connection-driver/libvirt_driver_network.so
 /usr/lib64/libvirt/connection-driver/libvirt_driver_network.so:
 undefined symbol: virNetworkList

Not sure what you're seeing here.

 
 And every time libvirt segfaults, there's this in the logs:
 debug : virJSONValueToString:1133 :
 result={id:libvirt-6,error:{class:CommandNotFound,desc:The
 command qom-list has not been found,data:{name:qom-list}}}
 debug : virEventPollRunOnce:627 : EVENT_POLL_RUN: nhandles=32 timeout=1684
 9479: debug : qemuMonitorJSONCheckError:341 : unable to execute QEMU
 command
 {execute:qom-list,arguments:{path:/},id:libvirt-6}:
 {id:libvirt-6,error:{class:CommandNotFound,desc:The command
 qom-list has not been found,data:{name:qom-list}}}
 9479: error : qemuMonitorJSONCheckError:352 : internal error: unable to
 execute QEMU command 'qom-list': The command qom-list has not been found

Should be fixed in 730af8f.

 
 Any insights on any of these issues?

Please try again with 1.2.0-rc1 (contains both fixes mentioned above,
and several others), so we can determine whether we need more patches
prior to actually releasing 1.2.0.

-- 
Eric Blake   eblake 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] [Qemu-devel] [PATCH] virtio-rng: correct the default limit rate

2013-11-26 Thread Amos Kong
On Tue, Nov 26, 2013 at 07:32:37AM -0700, Eric Blake wrote:
 [adding libvirt]
 
 On 11/26/2013 06:58 AM, Paolo Bonzini wrote:
  Il 26/11/2013 14:43, Amos Kong ha scritto:
  /* Set a default rate limit of 2^47 bytes per minute or roughly 2TB/s.  If
   * you have an entropy source capable of generating more entropy than this
   * and you can pass it through via virtio-rng, then hats off to you.  Until
   * then, this is unlimited for all practical purposes.
   */
 
  But the current rate is (INT64_MAX) bytes per (1  16) ms, it's 128,000 
  TB/s
  
  You are changing:
  
  * max-bytes from 2^63 to 2^47
  
  * period from 65536 to 6
  
  For a user, changing only period would have no effect, the limit rate
  would remain effectively infinite.  Changing max-bytes would give a 7%
  higher rate after your patch.
  
  Not a big deal, and max-bytes is easier to explain after your patch
  (bytes/minute) than before (bytes/65536ms).
  
  Reviewed-by: Paolo Bonzini pbonz...@redhat.com
  
 
 Hmm.  Libvirt is already converting a user's rate of bytes/period into
 the qemu parameters, defaulting to 1 second as its default period.

So libvirt will always pass a fixed 1 second period to qemu?

 Am I
 correct that as long as libvirt specified both rate AND period, then
 this change has no impact (and that the 7% change occurs if you specify
 period while leaving max-bytes alone)?

  Or is this an ABI change where
 libvirt will have to be taught to be smart enough to know whether it is
 old qemu or new qemu to adjust how libvirt does its calculations when
 converting the user's rate into qemu terms?

Nothing need to do in Libvirt for _this patch_

No API change here, just change default rate from 1.9 TB/s to 2.1 TB/s
This patch didn't change another limit logic.


== Effect of the period parameter ==

When we set a fixed ratespeed, we can use different periods. The
period still effect the stable of system IO. 
If the period is too large, and we set a higher speed, then IO will wave.

Example:  the IO max speed is 20 M/s, and we test 5 mins
  it clear that the first period is better

_Theory_ Condition:

* period 1:   20M / 1s
  from 0 ~ 20 second, read packets,
  from 21 ~ 100 second, wait ...

  from 0 ~ 20 second, read packets,
  from 21 ~ 100 second, wait ...

  from 260 ~ 281 second, read packets,
  from 281 ~ 300 second, wait ...

* period 2:   100M / 5
  from 0 ~ 60 second, read packets,
  from 61 ~ 300 second, wait ...

Smaller period is better, but smaller period will cause more timer
expired, and IO wave will be balance by scheduling other process.

So Libvirt should pass period  max-bytes in xml to qemu without
converting.

-- 
Amos.


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

Re: [libvirt] [Qemu-devel] [PATCH] virtio-rng: correct the default limit rate

2013-11-26 Thread Eric Blake
On 11/26/2013 08:23 AM, Amos Kong wrote:


 Hmm.  Libvirt is already converting a user's rate of bytes/period into
 the qemu parameters, defaulting to 1 second as its default period.
 
 So libvirt will always pass a fixed 1 second period to qemu?

You forced me to check the code :)

We're safe.  Libvirt lets the user control both rate and period in the
XML.  If the user specifies neither, then libvirt doesn't pass max-bytes
or period (but that's the default for unlimited, and you argued that
even with your changes, the default is still effectively unlimited); the
same is true if the user specifies period but not rate (a different
period with no maximum is still effectively unlimited throughput, so it
didn't really matter that we don't pass period on through).  It is only
when the user specifies rate that libvirt passes anything on to qemu;
and in that case, libvirt always passes both max-bytes and period
(either with a period set by user, or with the period forced to 1000).

 
 Nothing need to do in Libvirt for _this patch_

Agreed.  Phew.

-- 
Eric Blake   eblake 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] newer libvirt version issues

2013-11-26 Thread Michal Privoznik
On 26.11.2013 16:06, Franky Van Liedekerke wrote:
 
 Hi,
 
 since we're running more than 20 hosts per KVM server, we needed to
 update libvirt to at least 1.1.2 for virtlockd to be able to cope with
 this (due to an old hardcoded limit that was in there before).
 But where 1.1.0 compiles and runs just fine on a fully-patched CentOS
 6.4 server, newer version have all kinds of issues:
 
 Every version of libvirt = 1.1.2 crashes with a segfault on up-to-date
 CentOS 6.4 servers (see below).
 Also, versions of libvirt = 1.1.3 need a change in the spec file to be
 able to compile:
 apparently the %doc entries that are different with 1.1.2 prevent
 rpmbuild to succeed on
 CentOS servers. Using the %doc lines as per the 1.1.2 spec file results
 in a working rpmbuild.
 The 1.1.4 version has issues with a symbol not found after compiling and
 letting it run:
 
 error : virDriverLoadModule:78 : failed to load module
 /usr/lib64/libvirt/connection-driver/libvirt_driver_network.so
 /usr/lib64/libvirt/connection-driver/libvirt_driver_network.so:
 undefined symbol: virNetworkList
 error : virDriverLoadModule:78 : failed to load module
 /usr/lib64/libvirt/connection-driver/libvirt_driver_storage.so
 /usr/lib64/libvirt/connection-driver/libvirt_driver_storage.so:
 undefined symbol: virAsprintf
 error : virDriverLoadModule:78 : failed to load module
 /usr/lib64/libvirt/connection-driver/libvirt_driver_nodedev.so
 /usr/lib64/libvirt/connection-driver/libvirt_driver_nodedev.so:
 undefined symbol: virNodeDeviceList
 31677: error : virDriverLoadModule:78 : failed to load module
 /usr/lib64/libvirt/connection-driver/libvirt_driver_secret.so
 /usr/lib64/libvirt/connection-driver/libvirt_driver_secret.so: undefined
 symbol: virAsprintf
 31677: error : virDriverLoadModule:78 : failed to load module
 /usr/lib64/libvirt/connection-driver/libvirt_driver_nwfilter.so
 /usr/lib64/libvirt/connection-driver/libvirt_driver_nwfilter.so:
 undefined symbol: virAsprintf
 31677: error : virDriverLoadModule:78 : failed to load module
 /usr/lib64/libvirt/connection-driver/libvirt_driver_interface.so
 /usr/lib64/libvirt/connection-driver/libvirt_driver_interface.so:
 undefined symbol: virAsprintf
 31677: error : virDriverLoadModule:78 : failed to load module
 /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so
 /usr/lib64/libvirt/connection-driver/libvirt_driver_qemu.so: undefined
 symbol: networkAllocateActualDevice
 31677: error : virDriverLoadModule:78 : failed to load module
 /usr/lib64/libvirt/connection-driver/libvirt_driver_lxc.so
 /usr/lib64/libvirt/connection-driver/libvirt_driver_lxc.so: undefined
 symbol: networkAllocateActualDevice
 
 And every time libvirt segfaults, there's this in the logs:
 debug : virJSONValueToString:1133 :
 result={id:libvirt-6,error:{class:CommandNotFound,desc:The
 command qom-list has not been found,data:{name:qom-list}}}
 debug : virEventPollRunOnce:627 : EVENT_POLL_RUN: nhandles=32 timeout=1684
 9479: debug : qemuMonitorJSONCheckError:341 : unable to execute QEMU
 command
 {execute:qom-list,arguments:{path:/},id:libvirt-6}:
 {id:libvirt-6,error:{class:CommandNotFound,desc:The command
 qom-list has not been found,data:{name:qom-list}}}
 9479: error : qemuMonitorJSONCheckError:352 : internal error: unable to
 execute QEMU command 'qom-list': The command qom-list has not been found
 
 Any insights on any of these issues?

As Eric said it should all be fixed now. However I'm not sure about the
last one - it wasn't SIGSEGV I was seeing when writing the patch, was
it? Can't recall right now, honestly. Anyway, if you're seeing a
SIGSEGV-ing daemon, is it possible for you to get a stacktrace and share
it with us? Maybe we have more bugs than that one I've fixed :)

Moreover, if you're running libvirt under production you almost
certainly don't want run upstream libvirt there but rather backport the
important patches instead. In this case - backport 9f5b4b1f.

Michal

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


[libvirt] [PATCHv1.5 00/27] Gluster snapshot support (RFC) and refactors

2013-11-26 Thread Peter Krempa
This version was rebased on top of Eric's gluster pool series and a few
small mistakes were fixed. As nobody actually reviewed the original posting
I did not bother putting the differences in the patches.

Peter Krempa (27):
  conf: Export virStorageVolType enum helper functions
  test: Implement fake storage pool driver in qemuxml2argv test
  qemuxml2argv: Add test to verify correct usage of disk type=volume
  qemuxml2argv: Add test for disk type='volume' with iSCSI pools
  qemu: Split out formatting of network disk source URI
  qemu: Simplify call pattern of qemuBuildDriveURIString
  qemu: Use qemuBuildNetworkDriveURI to handle http/ftp and friends
  qemu: Migrate sheepdog source generation into common function
  qemu: Split out NBD command generation
  qemu: Unify formatting of RBD sources
  qemu: Refactor disk source string formatting
  conf: Support disk source formatting without needing a
virDomainDiskDefPtr
  conf: Clean up virDomainDiskSourceDefFormatInternal
  conf: Split out seclabel formating code for disk source
  conf: Export disk source formatter and parser
  snapshot: conf: Use common parsing and formatting functions for source
  snapshot: conf: Fix NULL dereference when driver element is empty
  conf: Add functions to copy and free network disk source definitions
  qemu: snapshot: Detect internal snapshots also for sheepdog and RBD
  conf: Add helper do clear disk source authentication struct
  qemu: Clear old translated pool source
  qemu: snapshot: Touch up error message
  qemu: snapshot: Add functions similar to disk source pool translation
  qemu: snapshots: Declare supported and unsupported snapshot configs
  RFC: snapshot: Add support for specifying snapshot disk backing type
  RFC: conf: snapshot: Parse more snapshot information
  RFC: qemu: snapshot: Add support for external active snapshots on
gluster

 src/conf/domain_conf.c | 261 ++---
 src/conf/domain_conf.h |  25 +
 src/conf/snapshot_conf.c   |  69 ++-
 src/conf/snapshot_conf.h   |  14 +-
 src/libvirt_private.syms   |   6 +
 src/qemu/qemu_command.c| 652 +++--
 src/qemu/qemu_command.h|  15 +
 src/qemu/qemu_conf.c   | 156 +++--
 src/qemu/qemu_conf.h   |   8 +
 src/qemu/qemu_driver.c | 429 --
 .../qemuxml2argv-disk-source-pool-mode.args|  10 +
 .../qemuxml2argv-disk-source-pool-mode.xml |   4 +-
 .../qemuxml2argv-disk-source-pool.args |   8 +
 .../qemuxml2argv-disk-source-pool.xml  |   2 +-
 tests/qemuxml2argvtest.c   | 166 ++
 15 files changed, 1298 insertions(+), 527 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args

-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 01/27] conf: Export virStorageVolType enum helper functions

2013-11-26 Thread Peter Krempa
Export string conversion from and to the virStorageVolType enum.
---
 src/libvirt_private.syms | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a705c56..205fe56 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -701,6 +701,8 @@ virStorageVolDefFree;
 virStorageVolDefParseFile;
 virStorageVolDefParseNode;
 virStorageVolDefParseString;
+virStorageVolTypeFromString;
+virStorageVolTypeToString;


 # conf/storage_encryption_conf.h
-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 02/27] test: Implement fake storage pool driver in qemuxml2argv test

2013-11-26 Thread Peter Krempa
To support testing of volume disk backing, we need to implement a few
disk driver backend functions.

The fake storage driver uses files in storagepoolxml2xmlout/POOLNAME.xml
as XML files for pool definitions and volume names are in format
VOL_TYPE+VOL_PATH. By default type block is assumed (for iSCSI test
compatibility).

The choice of this approach along with implemented functions was made so
that disk type='volume' can be tested in the xml2argv test.
---
 tests/qemuxml2argvtest.c | 162 +++
 1 file changed, 162 insertions(+)

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a290062..a4cef84 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -18,6 +18,7 @@
 # include qemu/qemu_command.h
 # include qemu/qemu_domain.h
 # include datatypes.h
+# include conf/storage_conf.h
 # include cpu/cpu_map.h
 # include virstring.h

@@ -75,6 +76,161 @@ static virSecretDriver fakeSecretDriver = {
 .secretUndefine = NULL,
 };

+
+# define STORAGE_POOL_XML_PATH storagepoolxml2xmlout/
+static const unsigned char fakeUUID[VIR_UUID_BUFLEN] = fakeuuid;
+
+static virStoragePoolPtr
+fakeStoragePoolLookupByName(virConnectPtr conn,
+const char *name)
+{
+char *xmlpath = NULL;
+virStoragePoolPtr ret = NULL;
+
+if (STRNEQ(name, inactive)) {
+if (virAsprintf(xmlpath, %s%s.xml,
+STORAGE_POOL_XML_PATH,
+name)  0)
+return NULL;
+
+if (!virFileExists(xmlpath)) {
+virReportError(VIR_ERR_NO_STORAGE_POOL,
+   File '%s' not found, xmlpath);
+goto cleanup;
+}
+}
+
+ret = virGetStoragePool(conn, name, fakeUUID, NULL, NULL);
+
+cleanup:
+VIR_FREE(xmlpath);
+return ret;
+}
+
+
+static virStorageVolPtr
+fakeStorageVolLookupByName(virStoragePoolPtr pool,
+   const char *name)
+{
+char **volinfo = NULL;
+virStorageVolPtr ret = NULL;
+
+if (STREQ(pool-name, inactive)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   storage pool '%s' is not active, pool-name);
+return NULL;
+}
+
+if (STREQ(name, nonexistent)) {
+virReportError(VIR_ERR_NO_STORAGE_VOL,
+   no storage vol with matching name '%s', name);
+return NULL;
+}
+
+if (!strchr(name, '+'))
+goto fallback;
+
+if (!(volinfo = virStringSplit(name, +, 2)))
+return NULL;
+
+if (!volinfo[1])
+goto fallback;
+
+ret = virGetStorageVol(pool-conn, pool-name, volinfo[1], volinfo[0],
+   NULL, NULL);
+
+cleanup:
+virStringFreeList(volinfo);
+return ret;
+
+fallback:
+ret = virGetStorageVol(pool-conn, pool-name, name, block, NULL, NULL);
+goto cleanup;
+}
+
+static int
+fakeStorageVolGetInfo(virStorageVolPtr vol,
+  virStorageVolInfoPtr info)
+{
+memset(info, 0, sizeof(*info));
+
+info-type = virStorageVolTypeFromString(vol-key);
+
+if (info-type  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   Invalid volume type '%s', vol-key);
+return -1;
+}
+
+return 0;
+}
+
+
+static char *
+fakeStorageVolGetPath(virStorageVolPtr vol)
+{
+char *ret = NULL;
+
+ignore_value(virAsprintf(ret, /some/%s/device/%s, vol-key, vol-name));
+
+return ret;
+}
+
+
+static char *
+fakeStoragePoolDumpXML(virStoragePoolPtr pool,
+   unsigned int flags_unused ATTRIBUTE_UNUSED)
+{
+char *xmlpath = NULL;
+char *xmlbuf = NULL;
+
+if (STREQ(pool-name, inactive)) {
+virReportError(VIR_ERR_NO_STORAGE_POOL, NULL);
+return NULL;
+}
+
+if (virAsprintf(xmlpath, %s%s.xml,
+STORAGE_POOL_XML_PATH,
+pool-name)  0)
+return NULL;
+
+if (virtTestLoadFile(xmlpath, xmlbuf)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   failed to load XML file '%s',
+   xmlpath);
+goto cleanup;
+}
+
+cleanup:
+VIR_FREE(xmlpath);
+
+return xmlbuf;
+}
+
+static int
+fakeStorageClose(virConnectPtr conn ATTRIBUTE_UNUSED)
+{
+return 0;
+}
+
+static int
+fakeStoragePoolIsActive(virStoragePoolPtr pool ATTRIBUTE_UNUSED)
+{
+return 1;
+}
+
+
+static virStorageDriver fakeStorageDriver = {
+.name = fake_storage,
+.storageClose = fakeStorageClose,
+.storagePoolLookupByName = fakeStoragePoolLookupByName,
+.storageVolLookupByName = fakeStorageVolLookupByName,
+.storagePoolGetXMLDesc = fakeStoragePoolDumpXML,
+.storageVolGetPath = fakeStorageVolGetPath,
+.storageVolGetInfo = fakeStorageVolGetInfo,
+.storagePoolIsActive = fakeStoragePoolIsActive,
+};
+
 typedef enum {
 FLAG_EXPECT_ERROR   = 1  0,
 FLAG_EXPECT_FAILURE = 1  1,
@@ -103,6 +259,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
 

[libvirt] [PATCHv1.5 05/27] qemu: Split out formatting of network disk source URI

2013-11-26 Thread Peter Krempa
The snapshot code will need to use qemu-style formatted URIs of network
disks. Split out the code to avoid duplication.
---
 src/qemu/qemu_command.c | 146 
 src/qemu/qemu_command.h |   6 ++
 2 files changed, 91 insertions(+), 61 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b8129a3..36bdc15 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3626,106 +3626,125 @@ error:
 return -1;
 }

-static int
-qemuBuildDriveURIString(virConnectPtr conn,
-virDomainDiskDefPtr disk, virBufferPtr opt,
-const char *scheme, virSecretUsageType secretUsageType)
-{
-int ret = -1;
-int port = 0;
-char *secret = NULL;

-char *tmpscheme = NULL;
-char *volimg = NULL;
-char *sock = NULL;
-char *user = NULL;
-char *builturi = NULL;
-const char *transp = NULL;
-virURI uri = {
-.port = port /* just to clear rest of bits */
-};
+char *
+qemuBuildNetworkDriveURI(int protocol,
+ const char *src,
+ size_t nhosts,
+ virDomainDiskHostDefPtr hosts,
+ const char *username,
+ const char *secret)
+{
+char *ret = NULL;
+virURIPtr uri = NULL;

-if (disk-nhosts != 1) {
+if (nhosts != 1) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(%s accepts only one host), scheme);
-return -1;
+   _(protocol '%s' accepts only one host),
+   virDomainDiskProtocolTypeToString(protocol));
+return NULL;
 }

-virBufferAddLit(opt, file=);
-transp = 
virDomainDiskProtocolTransportTypeToString(disk-hosts-transport);
+if (VIR_ALLOC(uri)  0)
+return NULL;

-if (disk-hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) {
-if (VIR_STRDUP(tmpscheme, scheme)  0)
+if (hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) {
+if (VIR_STRDUP(uri-scheme, 
virDomainDiskProtocolTypeToString(protocol))  0)
 goto cleanup;
 } else {
-if (virAsprintf(tmpscheme, %s+%s, scheme, transp)  0)
+if (virAsprintf(uri-scheme, %s+%s,
+virDomainDiskProtocolTypeToString(protocol),
+
virDomainDiskProtocolTransportTypeToString(hosts-transport))  0)
 goto cleanup;
 }

-if (disk-src  virAsprintf(volimg, /%s, disk-src)  0)
+if (src 
+virAsprintf(uri-path, /%s, src)  0)
+goto cleanup;
+
+if (hosts-port 
+virStrToLong_i(hosts-port, NULL, 10, uri-port)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(failed to parse port number '%s'),
+   hosts-port);
+goto cleanup;
+}
+
+if (hosts-socket 
+virAsprintf(uri-query, socket=%s, hosts-socket)  0)
 goto cleanup;

-if (disk-hosts-port) {
-port = atoi(disk-hosts-port);
+if (username) {
+if (secret) {
+if (virAsprintf(uri-user, %s:%s, username, secret)  0)
+goto cleanup;
+} else {
+if (VIR_STRDUP(uri-user, username)  0)
+goto cleanup;
+}
 }

-if (disk-hosts-socket 
-virAsprintf(sock, socket=%s, disk-hosts-socket)  0)
+if (VIR_STRDUP(uri-server, hosts-name)  0)
 goto cleanup;

+ret = virURIFormat(uri);
+
+cleanup:
+virURIFree(uri);
+
+return ret;
+}
+
+
+static int
+qemuBuildDriveURIString(virConnectPtr conn,
+virDomainDiskDefPtr disk,
+virBufferPtr opt,
+int protocol,
+virSecretUsageType secretUsageType)
+{
+char *secret = NULL;
+char *builturi = NULL;
+int ret = -1;
+
+virBufferAddLit(opt, file=);
+
 if (disk-auth.username  secretUsageType != VIR_SECRET_USAGE_TYPE_NONE) {
 /* Get the secret string using the virDomainDiskDef */
-if (!(secret = qemuGetSecretString(conn, scheme, false,
+if (!(secret = qemuGetSecretString(conn,
+   
virDomainDiskProtocolTypeToString(protocol),
+   false,
disk-auth.secretType,
disk-auth.username,
disk-auth.secret.uuid,
disk-auth.secret.usage,
secretUsageType)))
 goto cleanup;
-if (virAsprintf(user, %s:%s, disk-auth.username, secret)  0)
-goto cleanup;
 }

-uri.scheme = tmpscheme; /* gluster+transport */
-uri.server = disk-hosts-name;
-uri.user = user;
-uri.port = port;
-uri.path = volimg;
-uri.query = sock;
+if (!(builturi = 

[libvirt] [PATCHv1.5 06/27] qemu: Simplify call pattern of qemuBuildDriveURIString

2013-11-26 Thread Peter Krempa
Automatically assign secret type from the disk source definition and
pull in adding of the comma. Then update callers to keep generated
output the same.
---
 src/qemu/qemu_command.c | 34 +-
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 36bdc15..2326221 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3699,9 +3699,7 @@ cleanup:
 static int
 qemuBuildDriveURIString(virConnectPtr conn,
 virDomainDiskDefPtr disk,
-virBufferPtr opt,
-int protocol,
-virSecretUsageType secretUsageType)
+virBufferPtr opt)
 {
 char *secret = NULL;
 char *builturi = NULL;
@@ -3709,20 +3707,22 @@ qemuBuildDriveURIString(virConnectPtr conn,

 virBufferAddLit(opt, file=);

-if (disk-auth.username  secretUsageType != VIR_SECRET_USAGE_TYPE_NONE) {
+if (disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI 
+disk-auth.username) {
 /* Get the secret string using the virDomainDiskDef */
 if (!(secret = qemuGetSecretString(conn,
-   
virDomainDiskProtocolTypeToString(protocol),
+   
virDomainDiskProtocolTypeToString(disk-protocol),
false,
disk-auth.secretType,
disk-auth.username,
disk-auth.secret.uuid,
disk-auth.secret.usage,
-   secretUsageType)))
+   VIR_SECRET_USAGE_TYPE_ISCSI)))
 goto cleanup;
 }

-if (!(builturi = qemuBuildNetworkDriveURI(protocol,
+
+if (!(builturi = qemuBuildNetworkDriveURI(disk-protocol,
   disk-src,
   disk-nhosts,
   disk-hosts,
@@ -3731,6 +3731,7 @@ qemuBuildDriveURIString(virConnectPtr conn,
 goto cleanup;

 virBufferEscape(opt, ',', ,, %s, builturi);
+virBufferAddChar(opt, ',');

 ret = 0;

@@ -3760,9 +3761,7 @@ qemuBuildNBDString(virConnectPtr conn, 
virDomainDiskDefPtr disk, virBufferPtr op
  !disk-hosts-name)
 || (disk-hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX
  disk-hosts-socket  disk-hosts-socket[0] != '/'))
-return qemuBuildDriveURIString(conn, disk, opt,
-   VIR_DOMAIN_DISK_PROTOCOL_NBD,
-   VIR_SECRET_USAGE_TYPE_NONE);
+return qemuBuildDriveURIString(conn, disk, opt);

 virBufferAddLit(opt, file=nbd:);

@@ -3792,6 +3791,8 @@ qemuBuildNBDString(virConnectPtr conn, 
virDomainDiskDefPtr disk, virBufferPtr op
 if (disk-src)
 virBufferEscape(opt, ',', ,, :exportname=%s, disk-src);

+virBufferAddChar(opt, ',');
+
 return 0;
 }

@@ -3921,7 +3922,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 case VIR_DOMAIN_DISK_PROTOCOL_NBD:
 if (qemuBuildNBDString(conn, disk, opt)  0)
 goto error;
-virBufferAddChar(opt, ',');
 break;
 case VIR_DOMAIN_DISK_PROTOCOL_RBD:
 virBufferAddLit(opt, file=);
@@ -3929,19 +3929,11 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 goto error;
 virBufferAddChar(opt, ',');
 break;
+
 case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
-if (qemuBuildDriveURIString(conn, disk, opt,
-VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
-VIR_SECRET_USAGE_TYPE_NONE)  0)
-goto error;
-virBufferAddChar(opt, ',');
-break;
 case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
-if (qemuBuildDriveURIString(conn, disk, opt,
-VIR_DOMAIN_DISK_PROTOCOL_ISCSI,
-VIR_SECRET_USAGE_TYPE_ISCSI)  0)
+if (qemuBuildDriveURIString(conn, disk, opt)  0)
 goto error;
-virBufferAddChar(opt, ',');
 break;

 case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 03/27] qemuxml2argv: Add test to verify correct usage of disk type=volume

2013-11-26 Thread Peter Krempa
Tweak the existing file to test command line generator too.
---
 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args | 8 
 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml  | 2 +-
 tests/qemuxml2argvtest.c  | 2 ++
 3 files changed, 11 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args
new file mode 100644
index 000..da87ad9
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args
@@ -0,0 +1,8 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \
+file=/some/block/device/cdrom,if=none,media=cdrom,id=drive-ide0-0-1 -device \
+ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -drive \
+file=/tmp/idedisk.img,if=none,id=drive-ide0-0-2 -device \
+ide-drive,bus=ide.0,unit=2,drive=drive-ide0-0-2,id=ide0-0-2 -device \
+virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
index 465a539..6ca5cf7 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
@@ -15,7 +15,7 @@
   devices
 emulator/usr/bin/qemu/emulator
 disk type='volume' device='cdrom'
-  source pool='blk-pool0' volume='blk-pool0-vol0' 
startupPolicy='optional'
+  source pool='pool-disk' volume='block+cdrom'
 seclabel model='selinux' relabel='yes'
   labelsystem_u:system_r:public_content_t:s0/label
 /seclabel
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a4cef84..1c50732 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -770,6 +770,8 @@ mymain(void)
 DO_TEST(disk-aio,
 QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_AIO,
 QEMU_CAPS_DRIVE_CACHE_V2, QEMU_CAPS_DRIVE_FORMAT);
+DO_TEST(disk-source-pool,
+QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
 DO_TEST(disk-ioeventfd,
 QEMU_CAPS_DRIVE, QEMU_CAPS_VIRTIO_IOEVENTFD,
 QEMU_CAPS_VIRTIO_TX_ALG, QEMU_CAPS_DEVICE,
-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 04/27] qemuxml2argv: Add test for disk type='volume' with iSCSI pools

2013-11-26 Thread Peter Krempa
Tweak the existing file so that it can be tested for command line
corectness.
---
 src/conf/domain_conf.h |   1 +
 src/libvirt_private.syms   |   1 +
 src/qemu/qemu_command.c|  76 +---
 src/qemu/qemu_conf.c   | 129 ++---
 src/qemu/qemu_conf.h   |   2 +
 .../qemuxml2argv-disk-source-pool-mode.args|  10 ++
 .../qemuxml2argv-disk-source-pool-mode.xml |   4 +-
 tests/qemuxml2argvtest.c   |   2 +
 8 files changed, 112 insertions(+), 113 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4561ccc..a5ef2ca 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -686,6 +686,7 @@ struct _virDomainDiskSourcePoolDef {
 char *volume; /* volume name */
 int voltype; /* enum virStorageVolType, internal only */
 int pooltype; /* enum virStoragePoolType, internal only */
+int actualtype; /* enum virDomainDiskType, internal only */
 int mode; /* enum virDomainDiskSourcePoolMode */
 };
 typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 205fe56..aeb3568 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -693,6 +693,7 @@ virStoragePoolSourceFree;
 virStoragePoolSourceListFormat;
 virStoragePoolSourceListNewSource;
 virStoragePoolTypeFromString;
+virStoragePoolTypeToString;
 virStorageVolDefFindByKey;
 virStorageVolDefFindByName;
 virStorageVolDefFindByPath;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 763417f..b8129a3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3775,69 +3775,6 @@ qemuBuildNBDString(virConnectPtr conn, 
virDomainDiskDefPtr disk, virBufferPtr op
 return 0;
 }

-static int
-qemuBuildVolumeString(virConnectPtr conn,
-  virDomainDiskDefPtr disk,
-  virBufferPtr opt)
-{
-int ret = -1;
-
-switch ((virStorageVolType) disk-srcpool-voltype) {
-case VIR_STORAGE_VOL_DIR:
-if (!disk-readonly) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(cannot create virtual FAT disks in read-write 
mode));
-goto cleanup;
-}
-if (disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
-virBufferEscape(opt, ',', ,, file=fat:floppy:%s,, disk-src);
-else
-virBufferEscape(opt, ',', ,, file=fat:%s,, disk-src);
-break;
-case VIR_STORAGE_VOL_BLOCK:
-if (disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
-   _(tray status 'open' is invalid for 
- block type volume));
-goto cleanup;
-}
-if (disk-srcpool-pooltype == VIR_STORAGE_POOL_ISCSI) {
-if (disk-srcpool-mode ==
-VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) {
-if (qemuBuildISCSIString(conn, disk, opt)  0)
-goto cleanup;
-virBufferAddChar(opt, ',');
-} else if (disk-srcpool-mode ==
-   VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) {
-virBufferEscape(opt, ',', ,, file=%s,, disk-src);
-}
-} else {
-virBufferEscape(opt, ',', ,, file=%s,, disk-src);
-}
-break;
-case VIR_STORAGE_VOL_FILE:
-if (disk-auth.username) {
-if (qemuBuildISCSIString(conn, disk, opt)  0)
-goto cleanup;
-virBufferAddChar(opt, ',');
-} else {
-virBufferEscape(opt, ',', ,, file=%s,, disk-src);
-}
-break;
-case VIR_STORAGE_VOL_NETWORK:
-case VIR_STORAGE_VOL_NETDIR:
-case VIR_STORAGE_VOL_LAST:
-/* Keep the compiler quiet, qemuTranslateDiskSourcePool already
- * reported the unsupported error.
- */
-goto cleanup;
-}
-
-ret = 0;
-
-cleanup:
-return ret;
-}

 char *
 qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
@@ -3851,6 +3788,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 virDomainDiskGeometryTransTypeToString(disk-geometry.trans);
 int idx = virDiskNameToIndex(disk-dst);
 int busid = -1, unitid = -1;
+int actualType = qemuDiskGetActualType(disk);

 if (idx  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -3934,12 +3872,13 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,

 /* disk-src is NULL when we use nbd disks */
 if ((disk-src ||
-(disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK 
+(actualType == VIR_DOMAIN_DISK_TYPE_NETWORK 
  disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_NBD)) 
 !((disk-device == 

[libvirt] [PATCHv1.5 10/27] qemu: Unify formatting of RBD sources

2013-11-26 Thread Peter Krempa
---
 src/qemu/qemu_command.c | 155 +++-
 1 file changed, 61 insertions(+), 94 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 15a6e9b..799209d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3253,72 +3253,6 @@ cleanup:
 return secret;
 }

-static int
-qemuBuildRBDString(virConnectPtr conn,
-   virDomainDiskDefPtr disk,
-   virBufferPtr opt)
-{
-size_t i;
-int ret = 0;
-char *secret = NULL;
-
-if (strchr(disk-src, ':')) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _(':' not allowed in RBD source volume name '%s'),
-   disk-src);
-return -1;
-}
-
-virBufferEscape(opt, ',', ,, rbd:%s, disk-src);
-if (disk-auth.username) {
-virBufferEscape(opt, '\\', :, :id=%s, disk-auth.username);
-/* Get the secret string using the virDomainDiskDef
- * NOTE: qemu/librbd wants it base64 encoded
- */
-if (!(secret = qemuGetSecretString(conn, rbd, true,
-   disk-auth.secretType,
-   disk-auth.username,
-   disk-auth.secret.uuid,
-   disk-auth.secret.usage,
-   VIR_SECRET_USAGE_TYPE_CEPH)))
-goto error;
-
-
-virBufferEscape(opt, '\\', :,
-:key=%s:auth_supported=cephx\\;none,
-secret);
-} else {
-virBufferAddLit(opt, :auth_supported=none);
-}
-
-if (disk-nhosts  0) {
-virBufferAddLit(opt, :mon_host=);
-for (i = 0; i  disk-nhosts; ++i) {
-if (i) {
-virBufferAddLit(opt, \\;);
-}
-
-/* assume host containing : is ipv6 */
-if (strchr(disk-hosts[i].name, ':')) {
-virBufferEscape(opt, '\\', :, [%s], disk-hosts[i].name);
-} else {
-virBufferAsprintf(opt, %s, disk-hosts[i].name);
-}
-if (disk-hosts[i].port) {
-virBufferAsprintf(opt, \\:%s, disk-hosts[i].port);
-}
-}
-}
-
-cleanup:
-VIR_FREE(secret);
-
-return ret;
-
-error:
-ret = -1;
-goto cleanup;
-}

 static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport)
 {
@@ -3693,6 +3627,7 @@ qemuBuildNetworkDriveURI(int protocol,
 char *ret = NULL;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 virURIPtr uri = NULL;
+size_t i;

 switch ((enum virDomainDiskProtocol) protocol) {
 case VIR_DOMAIN_DISK_PROTOCOL_NBD:
@@ -3835,10 +3770,51 @@ qemuBuildNetworkDriveURI(int protocol,
 break;

 case VIR_DOMAIN_DISK_PROTOCOL_RBD:
+if (strchr(src, ':')) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(':' not allowed in RBD source volume name 
'%s'),
+   src);
+goto cleanup;
+}
+
+virBufferStrcat(buf, rbd:, src, NULL);
+
+if (username) {
+virBufferEscape(buf, '\\', :, :id=%s, username);
+virBufferEscape(buf, '\\', :,
+:key=%s:auth_supported=cephx\\;none,
+secret);
+} else {
+virBufferAddLit(buf, :auth_supported=none);
+}
+
+if (nhosts  0) {
+virBufferAddLit(buf, :mon_host=);
+for (i = 0; i  nhosts; i++) {
+if (i)
+virBufferAddLit(buf, \\;);
+
+/* assume host containing : is ipv6 */
+if (strchr(hosts[i].name, ':'))
+virBufferEscape(buf, '\\', :, [%s], 
hosts[i].name);
+else
+virBufferAsprintf(buf, %s, hosts[i].name);
+
+if (hosts[i].port)
+virBufferAsprintf(buf, \\:%s, hosts[i].port);
+}
+}
+
+if (virBufferError(buf)  0) {
+virReportOOMError();
+goto cleanup;
+}
+
+ret = virBufferContentAndReset(buf);
+break;
+
+
 case VIR_DOMAIN_DISK_PROTOCOL_LAST:
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(network disk protocol '%s' not supported),
-   virDomainDiskProtocolTypeToString(protocol));
 goto cleanup;
 }

@@ -3861,17 +3837,26 @@ qemuBuildDriveURIString(virConnectPtr conn,

 virBufferAddLit(opt, file=);

-if (disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI 
+if ((disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI ||
+ disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) 
 disk-auth.username) {
-  

[libvirt] [PATCHv1.5 17/27] snapshot: conf: Fix NULL dereference when driver element is empty

2013-11-26 Thread Peter Krempa
Consider the following valid snapshot XML as the driver element is
allowed to be empty in the domainsnapshot.rng schema:

$ cat snap.xml
domainsnapshot
  disks
disk name='vda' snapshot='external'
  source file='/tmp/foo'/
  driver/
/disk
  /disks
/domainsnapshot

produces the following error:

$ virsh snapshot-create domain snap.xml
error: internal error: unknown disk snapshot driver '(null)'

The driver type is parsed as NULL from the XML as the attribute is not
present and then directly used to produce the error message.

With this patch the attempt to parse the driver type is skipped if not
present to avoid changing the schema to forbid the empty driver element.
---
 src/conf/snapshot_conf.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 7258daa..5958f13 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -154,15 +154,17 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
 } else if (!def-format 
xmlStrEqual(cur-name, BAD_CAST driver)) {
 char *driver = virXMLPropString(cur, type);
-def-format = virStorageFileFormatTypeFromString(driver);
-if (def-format = 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(unknown disk snapshot driver '%s'),
-   driver);
+if (driver) {
+def-format = virStorageFileFormatTypeFromString(driver);
+if (def-format = 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(unknown disk snapshot driver '%s'),
+   driver);
+VIR_FREE(driver);
+goto cleanup;
+}
 VIR_FREE(driver);
-goto cleanup;
 }
-VIR_FREE(driver);
 }
 }

-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 09/27] qemu: Split out NBD command generation

2013-11-26 Thread Peter Krempa
---
 src/qemu/qemu_command.c | 117 +---
 1 file changed, 61 insertions(+), 56 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6ba8df9..15a6e9b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3680,7 +3680,7 @@ qemuNetworkDriveGetPort(int protocol,
 return -1;
 }

-
+#define QEMU_DEFAULT_NBD_PORT 10809

 char *
 qemuBuildNetworkDriveURI(int protocol,
@@ -3691,9 +3691,67 @@ qemuBuildNetworkDriveURI(int protocol,
  const char *secret)
 {
 char *ret = NULL;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
 virURIPtr uri = NULL;

 switch ((enum virDomainDiskProtocol) protocol) {
+case VIR_DOMAIN_DISK_PROTOCOL_NBD:
+if (nhosts != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(protocol '%s' accepts only one host),
+   virDomainDiskProtocolTypeToString(protocol));
+goto cleanup;
+}
+
+if (!((hosts-name  strchr(hosts-name, ':')) ||
+  (hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP 
+   !hosts-name) ||
+  (hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX 
+   hosts-socket 
+   hosts-socket[0] != '/'))) {
+
+virBufferAddLit(buf, nbd:);
+
+switch (hosts-transport) {
+case VIR_DOMAIN_DISK_PROTO_TRANS_TCP:
+virBufferStrcat(buf, hosts-name, NULL);
+virBufferAsprintf(buf, :%s,
+  hosts-port ? hosts-port :
+  QEMU_DEFAULT_NBD_PORT);
+break;
+
+case VIR_DOMAIN_DISK_PROTO_TRANS_UNIX:
+if (!hosts-socket) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(socket attribute required for 
+ nix transport));
+goto cleanup;
+}
+
+virBufferAsprintf(buf, unix:%s, hosts-socket);
+break;
+
+default:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(nbd does not support transport '%s'),
+   
virDomainDiskProtocolTransportTypeToString(hosts-transport));
+goto cleanup;
+}
+
+if (src)
+virBufferAsprintf(buf, :exportname=%s, src);
+
+if (virBufferError(buf)  0) {
+virReportOOMError();
+goto cleanup;
+}
+
+ret = virBufferContentAndReset(buf);
+goto cleanup;
+}
+/* fallthrough */
+/* NBD code uses same formatting scheme as others in some cases */
+
 case VIR_DOMAIN_DISK_PROTOCOL_HTTP:
 case VIR_DOMAIN_DISK_PROTOCOL_HTTPS:
 case VIR_DOMAIN_DISK_PROTOCOL_FTP:
@@ -3701,7 +3759,6 @@ qemuBuildNetworkDriveURI(int protocol,
 case VIR_DOMAIN_DISK_PROTOCOL_TFTP:
 case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
 case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
-case VIR_DOMAIN_DISK_PROTOCOL_NBD:
 if (nhosts != 1) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(protocol '%s' accepts only one host),
@@ -3786,6 +3843,7 @@ qemuBuildNetworkDriveURI(int protocol,
 }

 cleanup:
+virBufferFreeAndReset(buf);
 virURIFree(uri);

 return ret;
@@ -3839,56 +3897,6 @@ cleanup:
 }


-#define QEMU_DEFAULT_NBD_PORT 10809
-
-static int
-qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr 
opt)
-{
-const char *transp;
-
-if (disk-nhosts != 1) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(nbd accepts only one host));
-return -1;
-}
-
-if ((disk-hosts-name  strchr(disk-hosts-name, ':')) ||
-(disk-hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP  
!disk-hosts-name) ||
-(disk-hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX  
disk-hosts-socket  disk-hosts-socket[0] != '/'))
-return qemuBuildDriveURIString(conn, disk, opt);
-
-virBufferAddLit(opt, file=nbd:);
-
-switch (disk-hosts-transport) {
-case VIR_DOMAIN_DISK_PROTO_TRANS_TCP:
-if (disk-hosts-name)
-virBufferEscape(opt, ',', ,, %s, disk-hosts-name);
-virBufferEscape(opt, ',', ,, :%s,
-disk-hosts-port ? disk-hosts-port :
-QEMU_DEFAULT_NBD_PORT);
-break;
-case VIR_DOMAIN_DISK_PROTO_TRANS_UNIX:
-if (!disk-hosts-socket) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(socket attribute required for unix transport));

[libvirt] [PATCHv1.5 12/27] conf: Support disk source formatting without needing a virDomainDiskDefPtr

2013-11-26 Thread Peter Krempa
The source element formatting function was expecting a
virDomainDiskDefPtr to store the data. As snapshots are not using this
data structure to hold the data, we need to add an internal function
which splits out individual fields separately.
---
 src/conf/domain_conf.c | 129 ++---
 1 file changed, 78 insertions(+), 51 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 140eb80..afeadd7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14352,28 +14352,38 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf,
 }

 static int
-virDomainDiskSourceDefFormat(virBufferPtr buf,
- virDomainDiskDefPtr def,
- unsigned int flags)
+virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
+ int type,
+ const char *src,
+ int policy,
+ int protocol,
+ size_t nhosts,
+ virDomainDiskHostDefPtr hosts,
+ size_t nseclabels,
+ virSecurityDeviceLabelDefPtr *seclabels,
+ virDomainDiskSourcePoolDefPtr srcpool,
+ unsigned int flags)
 {
-int n;
-const char *startupPolicy = 
virDomainStartupPolicyTypeToString(def-startupPolicy);
+size_t n;
+const char *startupPolicy = NULL;

-if (def-src || def-nhosts  0 || def-srcpool ||
-def-startupPolicy) {
-switch (def-type) {
+if (policy)
+startupPolicy = virDomainStartupPolicyTypeToString(policy);
+
+if (src || nhosts  0 || srcpool || startupPolicy) {
+switch (type) {
 case VIR_DOMAIN_DISK_TYPE_FILE:
 virBufferAddLit(buf,   source);
-if (def-src)
-virBufferEscapeString(buf,  file='%s', def-src);
-if (def-startupPolicy)
+if (src)
+virBufferEscapeString(buf,  file='%s', src);
+if (startupPolicy)
 virBufferEscapeString(buf,  startupPolicy='%s',
   startupPolicy);
-if (def-nseclabels) {
+if (nseclabels) {
 virBufferAddLit(buf, \n);
 virBufferAdjustIndent(buf, 8);
-for (n = 0; n  def-nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, def-seclabels[n],
+for (n = 0; n  nseclabels; n++)
+virSecurityDeviceLabelDefFormat(buf, seclabels[n],
 flags);
 virBufferAdjustIndent(buf, -8);
 virBufferAddLit(buf,   /source\n);
@@ -14383,15 +14393,15 @@ virDomainDiskSourceDefFormat(virBufferPtr buf,
 break;
 case VIR_DOMAIN_DISK_TYPE_BLOCK:
 virBufferAddLit(buf,   source);
-virBufferEscapeString(buf,  dev='%s', def-src);
-if (def-startupPolicy)
+virBufferEscapeString(buf,  dev='%s', src);
+if (startupPolicy)
 virBufferEscapeString(buf,  startupPolicy='%s',
   startupPolicy);
-if (def-nseclabels) {
+if (nseclabels) {
 virBufferAddLit(buf, \n);
 virBufferAdjustIndent(buf, 8);
-for (n = 0; n  def-nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, def-seclabels[n],
+for (n = 0; n  nseclabels; n++)
+virSecurityDeviceLabelDefFormat(buf, seclabels[n],
 flags);
 virBufferAdjustIndent(buf, -8);
 virBufferAddLit(buf,   /source\n);
@@ -14400,41 +14410,38 @@ virDomainDiskSourceDefFormat(virBufferPtr buf,
 }
 break;
 case VIR_DOMAIN_DISK_TYPE_DIR:
-virBufferEscapeString(buf,   source dir='%s',
-  def-src);
-if (def-startupPolicy)
+virBufferEscapeString(buf,   source dir='%s', src);
+if (startupPolicy)
 virBufferEscapeString(buf,  startupPolicy='%s',
   startupPolicy);
 virBufferAddLit(buf, /\n);
 break;
 case VIR_DOMAIN_DISK_TYPE_NETWORK:
 virBufferAsprintf(buf,   source protocol='%s',
-  
virDomainDiskProtocolTypeToString(def-protocol));
-if (def-src) {
-virBufferEscapeString(buf,  name='%s', def-src);
-}
-if (def-nhosts == 0) {
+  virDomainDiskProtocolTypeToString(protocol));
+if (src)
+

[libvirt] [PATCHv1.5 07/27] qemu: Use qemuBuildNetworkDriveURI to handle http/ftp and friends

2013-11-26 Thread Peter Krempa
Prepare the function to integrate other protocols and start folding
other network protocols into a common place.
---
 src/qemu/qemu_command.c | 199 +---
 1 file changed, 122 insertions(+), 77 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2326221..09ebd00 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3627,6 +3627,61 @@ error:
 }


+static int
+qemuNetworkDriveGetPort(int protocol,
+const char *port)
+{
+int ret = 0;
+
+if (port) {
+if (virStrToLong_i(port, NULL, 10, ret)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(failed to parse port number '%s'),
+   port);
+return -1;
+}
+
+return ret;
+}
+
+switch ((enum virDomainDiskProtocol) protocol) {
+case VIR_DOMAIN_DISK_PROTOCOL_HTTP:
+return 80;
+
+case VIR_DOMAIN_DISK_PROTOCOL_HTTPS:
+return 443;
+
+case VIR_DOMAIN_DISK_PROTOCOL_FTP:
+return 21;
+
+case VIR_DOMAIN_DISK_PROTOCOL_FTPS:
+return 990;
+
+case VIR_DOMAIN_DISK_PROTOCOL_TFTP:
+return 69;
+
+case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
+return 7000;
+
+case VIR_DOMAIN_DISK_PROTOCOL_NBD:
+return 10809;
+
+case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
+case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+/* no default port specified */
+return 0;
+
+case VIR_DOMAIN_DISK_PROTOCOL_RBD:
+case VIR_DOMAIN_DISK_PROTOCOL_LAST:
+/* not aplicable */
+return -1;
+}
+
+return -1;
+}
+
+
+
 char *
 qemuBuildNetworkDriveURI(int protocol,
  const char *src,
@@ -3638,56 +3693,74 @@ qemuBuildNetworkDriveURI(int protocol,
 char *ret = NULL;
 virURIPtr uri = NULL;

-if (nhosts != 1) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(protocol '%s' accepts only one host),
-   virDomainDiskProtocolTypeToString(protocol));
-return NULL;
-}
-
-if (VIR_ALLOC(uri)  0)
-return NULL;
+switch ((enum virDomainDiskProtocol) protocol) {
+case VIR_DOMAIN_DISK_PROTOCOL_HTTP:
+case VIR_DOMAIN_DISK_PROTOCOL_HTTPS:
+case VIR_DOMAIN_DISK_PROTOCOL_FTP:
+case VIR_DOMAIN_DISK_PROTOCOL_FTPS:
+case VIR_DOMAIN_DISK_PROTOCOL_TFTP:
+case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
+case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+case VIR_DOMAIN_DISK_PROTOCOL_NBD:
+if (nhosts != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(protocol '%s' accepts only one host),
+   virDomainDiskProtocolTypeToString(protocol));
+goto cleanup;
+}

-if (hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) {
-if (VIR_STRDUP(uri-scheme, 
virDomainDiskProtocolTypeToString(protocol))  0)
-goto cleanup;
-} else {
-if (virAsprintf(uri-scheme, %s+%s,
-virDomainDiskProtocolTypeToString(protocol),
-
virDomainDiskProtocolTransportTypeToString(hosts-transport))  0)
-goto cleanup;
-}
+if (VIR_ALLOC(uri)  0)
+goto cleanup;

-if (src 
-virAsprintf(uri-path, /%s, src)  0)
-goto cleanup;
+if (hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) {
+if (VIR_STRDUP(uri-scheme,
+   virDomainDiskProtocolTypeToString(protocol))  
0)
+goto cleanup;
+} else {
+if (virAsprintf(uri-scheme, %s+%s,
+virDomainDiskProtocolTypeToString(protocol),
+
virDomainDiskProtocolTransportTypeToString(hosts-transport))  0)
+goto cleanup;
+}

-if (hosts-port 
-virStrToLong_i(hosts-port, NULL, 10, uri-port)  0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(failed to parse port number '%s'),
-   hosts-port);
-goto cleanup;
-}
+if ((uri-port = qemuNetworkDriveGetPort(protocol, hosts-port))  
0)
+goto cleanup;

-if (hosts-socket 
-virAsprintf(uri-query, socket=%s, hosts-socket)  0)
-goto cleanup;
+if (src 
+virAsprintf(uri-path, %s%s,
+src[0] == '/' ?  : /,
+src)  0)
+goto cleanup;

-if (username) {
-if (secret) {
-if (virAsprintf(uri-user, %s:%s, username, secret)  0)
+if (hosts-socket 
+virAsprintf(uri-query, socket=%s, hosts-socket)  0)
 goto cleanup;
-} else {
-

[libvirt] [PATCHv1.5 11/27] qemu: Refactor disk source string formatting

2013-11-26 Thread Peter Krempa
---
 src/qemu/qemu_command.c | 128 
 1 file changed, 86 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 799209d..0738de2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3827,19 +3827,62 @@ cleanup:


 static int
-qemuBuildDriveURIString(virConnectPtr conn,
-virDomainDiskDefPtr disk,
-virBufferPtr opt)
+qemuGetDriveSourceString(int type,
+ const char *src,
+ int protocol,
+ size_t nhosts,
+ virDomainDiskHostDefPtr hosts,
+ const char *username,
+ const char *secret,
+ char **path)
 {
+*path = NULL;
+
+switch ((enum virDomainDiskType) type) {
+case VIR_DOMAIN_DISK_TYPE_BLOCK:
+case VIR_DOMAIN_DISK_TYPE_FILE:
+case VIR_DOMAIN_DISK_TYPE_DIR:
+if (!src)
+return 1;
+
+if (VIR_STRDUP(*path, src)  0)
+return -1;
+
+break;
+
+case VIR_DOMAIN_DISK_TYPE_NETWORK:
+if (!(*path = qemuBuildNetworkDriveURI(protocol,
+   src,
+   nhosts,
+   hosts,
+   username,
+   secret)))
+return -1;
+break;
+
+case VIR_DOMAIN_DISK_TYPE_VOLUME:
+case VIR_DOMAIN_DISK_TYPE_LAST:
+break;
+}
+
+return 0;
+}
+
+static int
+qemuDomainDiskGetSourceString(virConnectPtr conn,
+  virDomainDiskDefPtr disk,
+  char **source)
+{
+int actualType = qemuDiskGetActualType(disk);
 char *secret = NULL;
-char *builturi = NULL;
 int ret = -1;

-virBufferAddLit(opt, file=);
+*source = NULL;

-if ((disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI ||
- disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) 
-disk-auth.username) {
+if (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK 
+disk-auth.username 
+(disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI ||
+ disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD)) {
 bool encode = false;
 int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;

@@ -3860,32 +3903,23 @@ qemuBuildDriveURIString(virConnectPtr conn,
 goto cleanup;
 }

-
-if (!(builturi = qemuBuildNetworkDriveURI(disk-protocol,
-  disk-src,
-  disk-nhosts,
-  disk-hosts,
-  disk-auth.username,
-  secret)))
-goto cleanup;
-
-virBufferEscape(opt, ',', ,, %s, builturi);
-virBufferAddChar(opt, ',');
-
-ret = 0;
+ret = qemuGetDriveSourceString(qemuDiskGetActualType(disk),
+   disk-src,
+   disk-protocol,
+   disk-nhosts,
+   disk-hosts,
+   disk-auth.username,
+   secret,
+   source);

 cleanup:
 VIR_FREE(secret);
-VIR_FREE(builturi);
-
 return ret;
 }


-
-
 char *
-qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
+qemuBuildDriveStr(virConnectPtr conn,
   virDomainDiskDefPtr disk,
   bool bootable,
   virQEMUCapsPtr qemuCaps)
@@ -3896,6 +3930,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 virDomainDiskGeometryTransTypeToString(disk-geometry.trans);
 int idx = virDiskNameToIndex(disk-dst);
 int busid = -1, unitid = -1;
+char *source = NULL;
 int actualType = qemuDiskGetActualType(disk);

 if (idx  0) {
@@ -3978,15 +4013,18 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 break;
 }

-/* disk-src is NULL when we use nbd disks */
-if ((disk-src ||
-(actualType == VIR_DOMAIN_DISK_TYPE_NETWORK 
- disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_NBD)) 
+if (qemuDomainDiskGetSourceString(conn, disk, source)  0)
+goto error;
+
+if (source 
 !((disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM) 
   disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) {

-if (actualType == VIR_DOMAIN_DISK_TYPE_DIR) {
+virBufferAddLit(opt, file=);
+
+switch (actualType) {
+case VIR_DOMAIN_DISK_TYPE_DIR:
 /* QEMU only supports magic FAT format for now */
 if (disk-format  0  disk-format != VIR_STORAGE_FILE_FAT) {
 

[libvirt] [PATCHv1.5 22/27] qemu: snapshot: Touch up error message

2013-11-26 Thread Peter Krempa
---
 src/qemu/qemu_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d2dbaf5..96bc87e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11858,8 +11858,8 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, 
virDomainSnapshotDefPtr def,
  * offline snapshots */
 if (found_internal  external) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
-   _(mixing internal and external snapshots is not 
- supported yet));
+   _(mixing internal and external targets for a snapshot 
+ is not yet supported));
 goto cleanup;
 }

-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 08/27] qemu: Migrate sheepdog source generation into common function

2013-11-26 Thread Peter Krempa
---
 src/qemu/qemu_command.c | 37 -
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 09ebd00..6ba8df9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3754,6 +3754,29 @@ qemuBuildNetworkDriveURI(int protocol,
 break;

 case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
+if (!src) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(missing disk source for 'sheepdog' 
protocol));
+goto cleanup;
+}
+
+if (nhosts == 0) {
+if (virAsprintf(ret, sheepdog:%s, src)  0)
+goto cleanup;
+} else if (nhosts == 1) {
+if (virAsprintf(ret, sheepdog:%s:%s:%s,
+hosts-name,
+hosts-port ? hosts-port : 7000,
+src)  0)
+goto cleanup;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(protocol 'sheepdog' accepts up to one 
host));
+goto cleanup;
+}
+
+break;
+
 case VIR_DOMAIN_DISK_PROTOCOL_RBD:
 case VIR_DOMAIN_DISK_PROTOCOL_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -4001,6 +4024,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 virBufferAddChar(opt, ',');
 break;

+case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
 case VIR_DOMAIN_DISK_PROTOCOL_TFTP:
 case VIR_DOMAIN_DISK_PROTOCOL_FTPS:
 case VIR_DOMAIN_DISK_PROTOCOL_FTP:
@@ -4011,19 +4035,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 if (qemuBuildDriveURIString(conn, disk, opt)  0)
 goto error;
 break;
-
-case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
-if (disk-nhosts == 0) {
-virBufferEscape(opt, ',', ,, file=sheepdog:%s,,
-disk-src);
-} else {
-/* only one host is supported now */
-virBufferAsprintf(opt, file=sheepdog:%s:%s:,
-  disk-hosts-name,
-  disk-hosts-port ? disk-hosts-port : 
7000);
-virBufferEscape(opt, ',', ,, %s,, disk-src);
-}
-break;
 }
 } else {
 if ((actualType == VIR_DOMAIN_DISK_TYPE_BLOCK) 
-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 20/27] conf: Add helper do clear disk source authentication struct

2013-11-26 Thread Peter Krempa
Add virDomainDiskAuthClear to help cleaning out the struct in other
places too.
---
 src/conf/domain_conf.c   | 17 ++---
 src/conf/domain_conf.h   |  1 +
 src/libvirt_private.syms |  1 +
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e0d6903..454b21c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1201,12 +1201,9 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
 VIR_FREE(def-driverName);
 virStorageFileFreeMetadata(def-backingChain);
 VIR_FREE(def-mirror);
-VIR_FREE(def-auth.username);
 VIR_FREE(def-wwn);
 VIR_FREE(def-vendor);
 VIR_FREE(def-product);
-if (def-auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
-VIR_FREE(def-auth.secret.usage);
 virStorageEncryptionFree(def-encryption);
 virDomainDeviceInfoClear(def-info);

@@ -1217,10 +1214,24 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
 }

 virDomainDiskHostDefFree(def-nhosts, def-hosts);
+virDomainDiskAuthClear(def);

 VIR_FREE(def);
 }

+
+void
+virDomainDiskAuthClear(virDomainDiskDefPtr def)
+{
+VIR_FREE(def-auth.username);
+
+if (def-auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
+VIR_FREE(def-auth.secret.usage);
+
+def-auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_NONE;
+}
+
+
 void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def)
 {
 if (!def)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ee018f0..4934911 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2210,6 +2210,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr 
def);
 void virDomainInputDefFree(virDomainInputDefPtr def);
 void virDomainDiskDefFree(virDomainDiskDefPtr def);
 void virDomainLeaseDefFree(virDomainLeaseDefPtr def);
+void virDomainDiskAuthClear(virDomainDiskDefPtr def);
 void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def);
 void virDomainDiskHostDefFree(size_t nhosts, virDomainDiskHostDefPtr hosts);
 virDomainDiskHostDefPtr virDomainDiskHostDefCopy(size_t nhosts,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f952a12..f8e774c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -180,6 +180,7 @@ virDomainDeviceFindControllerModel;
 virDomainDeviceInfoCopy;
 virDomainDeviceInfoIterate;
 virDomainDeviceTypeToString;
+virDomainDiskAuthClear;
 virDomainDiskBusTypeToString;
 virDomainDiskCacheTypeFromString;
 virDomainDiskCacheTypeToString;
-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 13/27] conf: Clean up virDomainDiskSourceDefFormatInternal

2013-11-26 Thread Peter Krempa
Avoid if statements when used with virBufferEscapeString which
automaticaly omits the whole string. Also add some line breaks to
visualy separate the code.
---
 src/conf/domain_conf.c | 48 
 1 file changed, 20 insertions(+), 28 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index afeadd7..05f8964 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14374,53 +14374,49 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
 switch (type) {
 case VIR_DOMAIN_DISK_TYPE_FILE:
 virBufferAddLit(buf,   source);
-if (src)
-virBufferEscapeString(buf,  file='%s', src);
-if (startupPolicy)
-virBufferEscapeString(buf,  startupPolicy='%s',
-  startupPolicy);
+virBufferEscapeString(buf,  file='%s', src);
+virBufferEscapeString(buf,  startupPolicy='%s', startupPolicy);
+
 if (nseclabels) {
 virBufferAddLit(buf, \n);
 virBufferAdjustIndent(buf, 8);
 for (n = 0; n  nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, seclabels[n],
-flags);
+virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
 virBufferAdjustIndent(buf, -8);
 virBufferAddLit(buf,   /source\n);
 } else {
 virBufferAddLit(buf, /\n);
 }
 break;
+
 case VIR_DOMAIN_DISK_TYPE_BLOCK:
 virBufferAddLit(buf,   source);
 virBufferEscapeString(buf,  dev='%s', src);
-if (startupPolicy)
-virBufferEscapeString(buf,  startupPolicy='%s',
-  startupPolicy);
+virBufferEscapeString(buf,  startupPolicy='%s', startupPolicy);
+
 if (nseclabels) {
 virBufferAddLit(buf, \n);
 virBufferAdjustIndent(buf, 8);
 for (n = 0; n  nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, seclabels[n],
-flags);
+virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
 virBufferAdjustIndent(buf, -8);
 virBufferAddLit(buf,   /source\n);
 } else {
 virBufferAddLit(buf, /\n);
 }
 break;
+
 case VIR_DOMAIN_DISK_TYPE_DIR:
-virBufferEscapeString(buf,   source dir='%s', src);
-if (startupPolicy)
-virBufferEscapeString(buf,  startupPolicy='%s',
-  startupPolicy);
+virBufferAddLit(buf,   source);
+virBufferEscapeString(buf,  dir='%s', src);
+virBufferEscapeString(buf,  startupPolicy='%s', startupPolicy);
 virBufferAddLit(buf, /\n);
 break;
+
 case VIR_DOMAIN_DISK_TYPE_NETWORK:
 virBufferAsprintf(buf,   source protocol='%s',
   virDomainDiskProtocolTypeToString(protocol));
-if (src)
-virBufferEscapeString(buf,  name='%s', src);
+virBufferEscapeString(buf,  name='%s', src);

 if (nhosts == 0) {
 virBufferAddLit(buf, /\n);
@@ -14428,25 +14424,21 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
 virBufferAddLit(buf, \n);
 for (n = 0; n  nhosts; n++) {
 virBufferAddLit(buf, host);
-if (hosts[n].name)
-virBufferEscapeString(buf,  name='%s', 
hosts[n].name);
-
-if (hosts[n].port)
-virBufferEscapeString(buf,  port='%s',
-  hosts[n].port);
+virBufferEscapeString(buf,  name='%s', hosts[n].name);
+virBufferEscapeString(buf,  port='%s', hosts[n].port);

 if (hosts[n].transport)
 virBufferAsprintf(buf,  transport='%s',
   
virDomainDiskProtocolTransportTypeToString(hosts[n].transport));

-if (hosts[n].socket)
-virBufferEscapeString(buf,  socket='%s', 
hosts[n].socket);
+virBufferEscapeString(buf,  socket='%s', 
hosts[n].socket);

 virBufferAddLit(buf, /\n);
 }
 virBufferAddLit(buf,   /source\n);
 }
 break;
+
 case VIR_DOMAIN_DISK_TYPE_VOLUME:
 virBufferAddLit(buf,   source);

@@ -14457,8 +14449,7 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
 virBufferAsprintf(buf,  mode='%s',
  

Re: [libvirt] [PATCH v2 1/2] LXC: fix the problem that libvirt lxc fail to start on latest kernel

2013-11-26 Thread Daniel P. Berrange
On Wed, Nov 20, 2013 at 10:11:08AM +0800, Gao feng wrote:
 After kernel commit 5ff9d8a65ce80efb509ce4e8051394e9ed2cd942
 vfs: Lock in place mounts from more privileged users,
 
 unprivileged user has no rights to move the mounts that
 inherited from parent mountns. we use this feature to move
 the /stateDir/domain-name.{dev, devpts} to the /dev/ and
 /dev/pts directroy of container. this commit breaks libvirt lxc.
 
 this patch changes the behavior to bind these mounts when
 user namespace is enabled and move these mounts when user
 namespace is disabled.
 
 Signed-off-by: Gao feng gaof...@cn.fujitsu.com
 ---
  src/lxc/lxc_container.c | 13 -
  1 file changed, 8 insertions(+), 5 deletions(-)

ACK, and pushed.

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] [PATCHv1.5 14/27] conf: Split out seclabel formating code for disk source

2013-11-26 Thread Peter Krempa
The code is common for all the various disk types. Split it out to a
common function.
---
 src/conf/domain_conf.c | 62 --
 1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 05f8964..8f45f2e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14351,6 +14351,32 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf,
 }
 }

+
+/* virDomainDiskSourceDefFormatSeclabel:
+ *
+ * This function automaticaly closes the source element and formats any
+ * possible seclabels.
+ */
+static void
+virDomainDiskSourceDefFormatSeclabel(virBufferPtr buf,
+ size_t nseclabels,
+ virSecurityDeviceLabelDefPtr *seclabels,
+ unsigned int flags)
+{
+size_t n;
+
+if (nseclabels) {
+virBufferAddLit(buf, \n);
+virBufferAdjustIndent(buf, 8);
+for (n = 0; n  nseclabels; n++)
+virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
+virBufferAdjustIndent(buf, -8);
+virBufferAddLit(buf,   /source\n);
+} else {
+virBufferAddLit(buf, /\n);
+}
+}
+
 static int
 virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
  int type,
@@ -14377,33 +14403,15 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
 virBufferEscapeString(buf,  file='%s', src);
 virBufferEscapeString(buf,  startupPolicy='%s', startupPolicy);

-if (nseclabels) {
-virBufferAddLit(buf, \n);
-virBufferAdjustIndent(buf, 8);
-for (n = 0; n  nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
-virBufferAdjustIndent(buf, -8);
-virBufferAddLit(buf,   /source\n);
-} else {
-virBufferAddLit(buf, /\n);
-}
-break;
+virDomainDiskSourceDefFormatSeclabel(buf, nseclabels, seclabels, 
flags);
+   break;

 case VIR_DOMAIN_DISK_TYPE_BLOCK:
 virBufferAddLit(buf,   source);
 virBufferEscapeString(buf,  dev='%s', src);
 virBufferEscapeString(buf,  startupPolicy='%s', startupPolicy);

-if (nseclabels) {
-virBufferAddLit(buf, \n);
-virBufferAdjustIndent(buf, 8);
-for (n = 0; n  nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
-virBufferAdjustIndent(buf, -8);
-virBufferAddLit(buf,   /source\n);
-} else {
-virBufferAddLit(buf, /\n);
-}
+virDomainDiskSourceDefFormatSeclabel(buf, nseclabels, seclabels, 
flags);
 break;

 case VIR_DOMAIN_DISK_TYPE_DIR:
@@ -14451,17 +14459,7 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
 }
 virBufferEscapeString(buf,  startupPolicy='%s', startupPolicy);

-if (nseclabels) {
-virBufferAddLit(buf, \n);
-virBufferAdjustIndent(buf, 8);
-for (n = 0; n  nseclabels; n++)
-virSecurityDeviceLabelDefFormat(buf, seclabels[n],
-flags);
-virBufferAdjustIndent(buf, -8);
-virBufferAddLit(buf,   /source\n);
-} else {
-virBufferAddLit(buf, /\n);
-}
+virDomainDiskSourceDefFormatSeclabel(buf, nseclabels, seclabels, 
flags);
 break;

 default:
-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 16/27] snapshot: conf: Use common parsing and formatting functions for source

2013-11-26 Thread Peter Krempa
Disk source elements for snapshots were using separate code from our
config parser. As snapshots can be stored on more than just regular
files, we will need the universal parser to allow us to expose a variety
of snapshot disk targets. This patch reuses the config parsers and
formatters to do the job.

This initial support only changes the code without any visible XML
change.
---
 src/conf/snapshot_conf.c | 70 +---
 src/conf/snapshot_conf.h |  1 +
 2 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 94a74d2..7258daa 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -128,27 +128,42 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
 }
 }

-cur = node-children;
-while (cur) {
-if (cur-type == XML_ELEMENT_NODE) {
-if (!def-file 
-xmlStrEqual(cur-name, BAD_CAST source)) {
-def-file = virXMLPropString(cur, file);
-} else if (!def-format 
-   xmlStrEqual(cur-name, BAD_CAST driver)) {
-char *driver = virXMLPropString(cur, type);
-def-format = virStorageFileFormatTypeFromString(driver);
-if (def-format = 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(unknown disk snapshot driver '%s'),
-   driver);
-VIR_FREE(driver);
-goto cleanup;
-}
+def-type = -1;
+
+for (cur = node-children; cur; cur = cur-next) {
+if (cur-type != XML_ELEMENT_NODE)
+continue;
+
+if (!def-file 
+xmlStrEqual(cur-name, BAD_CAST source)) {
+
+int backingtype = def-type;
+
+if (backingtype  0)
+backingtype = VIR_DOMAIN_DISK_TYPE_FILE;
+
+if (virDomainDiskSourceDefParse(cur,
+backingtype,
+def-file,
+NULL,
+NULL,
+NULL,
+NULL)  0)
+goto cleanup;
+
+} else if (!def-format 
+   xmlStrEqual(cur-name, BAD_CAST driver)) {
+char *driver = virXMLPropString(cur, type);
+def-format = virStorageFileFormatTypeFromString(driver);
+if (def-format = 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(unknown disk snapshot driver '%s'),
+   driver);
 VIR_FREE(driver);
+goto cleanup;
 }
+VIR_FREE(driver);
 }
-cur = cur-next;
 }

 if (!def-snapshot  (def-file || def-format))
@@ -577,6 +592,8 @@ static void
 virDomainSnapshotDiskDefFormat(virBufferPtr buf,
virDomainSnapshotDiskDefPtr disk)
 {
+int type = disk-type;
+
 if (!disk-name)
 return;

@@ -584,6 +601,13 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
 if (disk-snapshot  0)
 virBufferAsprintf(buf,  snapshot='%s',
   
virDomainSnapshotLocationTypeToString(disk-snapshot));
+
+if (type  0)
+type = VIR_DOMAIN_DISK_TYPE_FILE;
+else
+virBufferAsprintf(buf,  type='%s',
+  virDomainDiskTypeToString(type));
+
 if (!disk-file  disk-format == 0) {
 virBufferAddLit(buf, /\n);
 return;
@@ -591,12 +615,14 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,

 virBufferAddLit(buf, \n);

-virBufferAdjustIndent(buf, 6);
 if (disk-format  0)
-virBufferEscapeString(buf, driver type='%s'/\n,
+virBufferEscapeString(buf,   driver type='%s'/\n,
   virStorageFileFormatTypeToString(disk-format));
-virBufferEscapeString(buf, source file='%s'/\n, disk-file);
-virBufferAdjustIndent(buf, -6);
+virDomainDiskSourceDefFormatInternal(buf,
+ type,
+ disk-file,
+ 0, 0, 0, NULL, 0, NULL, NULL, 0);
+
 virBufferAddLit(buf, /disk\n);
 }

diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index ff3dca2..241d63c 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -51,6 +51,7 @@ struct _virDomainSnapshotDiskDef {
 char *name; /* name matching the target dev='...' of the domain */
 int index; /* index within snapshot-dom-disks that matches name */
 int snapshot; /* enum virDomainSnapshotLocation */
+int type; /* enum virDomainDiskType */
 char *file; /* new source file when snapshot is external */
 int format; /* enum virStorageFileFormat 

[libvirt] [PATCHv1.5 21/27] qemu: Clear old translated pool source

2013-11-26 Thread Peter Krempa
Clear the old data to avoid leaking it when attempting to re-translate a
pool on the same domain object.
---
 src/qemu/qemu_conf.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 58a0500..639e2ff 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1360,6 +1360,10 @@ qemuTranslateDiskSourcePool(virConnectPtr conn,
 goto cleanup;
 }

+VIR_FREE(def-src);
+virDomainDiskHostDefFree(def-nhosts, def-hosts);
+virDomainDiskAuthClear(def);
+
 switch ((enum virStoragePoolType) pooldef-type) {
 case VIR_STORAGE_POOL_DIR:
 case VIR_STORAGE_POOL_FS:
-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 15/27] conf: Export disk source formatter and parser

2013-11-26 Thread Peter Krempa
This code will be reused in the snapshot disk definition parser.
---
 src/conf/domain_conf.c |  4 ++--
 src/conf/domain_conf.h | 20 
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8f45f2e..0561d9d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4737,7 +4737,7 @@ cleanup:
 }


-static int
+int
 virDomainDiskSourceDefParse(xmlNodePtr node,
 int type,
 char **source,
@@ -14377,7 +14377,7 @@ virDomainDiskSourceDefFormatSeclabel(virBufferPtr buf,
 }
 }

-static int
+int
 virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
  int type,
  const char *src,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a5ef2ca..e9800a5 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2345,6 +2345,18 @@ int virDomainDefFormatInternal(virDomainDefPtr def,
unsigned int flags,
virBufferPtr buf);

+int virDomainDiskSourceDefFormatInternal(virBufferPtr buf,
+ int type,
+ const char *src,
+ int policy,
+ int protocol,
+ size_t nhosts,
+ virDomainDiskHostDefPtr hosts,
+ size_t nseclabels,
+ virSecurityDeviceLabelDefPtr 
*seclabels,
+ virDomainDiskSourcePoolDefPtr srcpool,
+ unsigned int flags);
+
 int virDomainDefCompatibleDevice(virDomainDefPtr def,
  virDomainDeviceDefPtr dev);

@@ -2379,6 +2391,14 @@ virDomainDiskDefPtr
 virDomainDiskRemove(virDomainDefPtr def, size_t i);
 virDomainDiskDefPtr
 virDomainDiskRemoveByName(virDomainDefPtr def, const char *name);
+int virDomainDiskSourceDefParse(xmlNodePtr node,
+int type,
+char **source,
+int *proto,
+size_t *nhosts,
+virDomainDiskHostDefPtr *hosts,
+virDomainDiskSourcePoolDefPtr *srcpool);
+
 bool virDomainHasDiskMirror(virDomainObjPtr vm);

 int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net);
-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 23/27] qemu: snapshot: Add functions similar to disk source pool translation

2013-11-26 Thread Peter Krempa
To avoid future pain, add placeholder functions to get the actual
snapshot disk type.
---
 src/qemu/qemu_conf.c | 23 +++
 src/qemu/qemu_conf.h |  6 ++
 2 files changed, 29 insertions(+)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 639e2ff..210608e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1453,3 +1453,26 @@ cleanup:
 virStoragePoolDefFree(pooldef);
 return ret;
 }
+
+
+int
+qemuSnapshotDiskGetActualType(virDomainSnapshotDiskDefPtr def)
+{
+if (def-type == -1)
+return VIR_DOMAIN_DISK_TYPE_FILE;
+
+return def-type;
+}
+
+
+int
+qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn ATTRIBUTE_UNUSED,
+virDomainSnapshotDiskDefPtr def)
+{
+if (def-type != VIR_DOMAIN_DISK_TYPE_VOLUME)
+return 0;
+
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Snapshots are not yet supported with 'pool' volumes));
+return -1;
+}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index b9786b1..0cb7901 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -29,6 +29,7 @@
 # include capabilities.h
 # include network_conf.h
 # include domain_conf.h
+# include snapshot_conf.h
 # include domain_event.h
 # include virthread.h
 # include security/security_manager.h
@@ -309,4 +310,9 @@ int qemuDiskGetActualType(virDomainDiskDefPtr def);
 int qemuTranslateDiskSourcePool(virConnectPtr conn,
 virDomainDiskDefPtr def);

+int qemuSnapshotDiskGetActualType(virDomainSnapshotDiskDefPtr def);
+
+int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn,
+virDomainSnapshotDiskDefPtr def);
+
 #endif /* __QEMUD_CONF_H */
-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 18/27] conf: Add functions to copy and free network disk source definitions

2013-11-26 Thread Peter Krempa
To simplify operations on virDomainDiskHostDef arrays we will need deep
copy and freeing functions. Add and properly export them.
---
 src/conf/domain_conf.c   | 55 +---
 src/conf/domain_conf.h   |  3 +++
 src/libvirt_private.syms |  2 ++
 3 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0561d9d..e0d6903 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1216,9 +1216,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
 VIR_FREE(def-seclabels);
 }

-for (i = 0; i  def-nhosts; i++)
-virDomainDiskHostDefClear(def-hosts[i]);
-VIR_FREE(def-hosts);
+virDomainDiskHostDefFree(def-nhosts, def-hosts);

 VIR_FREE(def);
 }
@@ -1233,6 +1231,57 @@ void virDomainDiskHostDefClear(virDomainDiskHostDefPtr 
def)
 VIR_FREE(def-socket);
 }

+
+void
+virDomainDiskHostDefFree(size_t nhosts,
+ virDomainDiskHostDefPtr hosts)
+{
+size_t i;
+
+if (!hosts)
+return;
+
+for (i = 0; i  nhosts; i++)
+virDomainDiskHostDefClear(hosts[i]);
+
+VIR_FREE(hosts);
+}
+
+
+virDomainDiskHostDefPtr
+virDomainDiskHostDefCopy(size_t nhosts,
+ virDomainDiskHostDefPtr hosts)
+{
+virDomainDiskHostDefPtr ret = NULL;
+size_t i;
+
+if (VIR_ALLOC_N(ret, nhosts)  0)
+goto error;
+
+for (i = 0; i  nhosts; i++) {
+virDomainDiskHostDefPtr src = hosts[i];
+virDomainDiskHostDefPtr dst = ret[i];
+
+dst-transport = src-transport;
+
+if (VIR_STRDUP(dst-name, src-name)  0)
+goto error;
+
+if (VIR_STRDUP(dst-port, src-port)  0)
+goto error;
+
+if (VIR_STRDUP(dst-socket, src-socket)  0)
+goto error;
+}
+
+return ret;
+
+error:
+virDomainDiskHostDefFree(nhosts, ret);
+return NULL;
+}
+
+
 void virDomainControllerDefFree(virDomainControllerDefPtr def)
 {
 if (!def)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e9800a5..ee018f0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2211,6 +2211,9 @@ void virDomainInputDefFree(virDomainInputDefPtr def);
 void virDomainDiskDefFree(virDomainDiskDefPtr def);
 void virDomainLeaseDefFree(virDomainLeaseDefPtr def);
 void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def);
+void virDomainDiskHostDefFree(size_t nhosts, virDomainDiskHostDefPtr hosts);
+virDomainDiskHostDefPtr virDomainDiskHostDefCopy(size_t nhosts,
+ virDomainDiskHostDefPtr 
hosts);
 int virDomainDeviceFindControllerModel(virDomainDefPtr def,
virDomainDeviceInfoPtr info,
int controllerType);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index aeb3568..f952a12 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -198,6 +198,8 @@ virDomainDiskFindByBusAndDst;
 virDomainDiskGeometryTransTypeFromString;
 virDomainDiskGeometryTransTypeToString;
 virDomainDiskHostDefClear;
+virDomainDiskHostDefCopy;
+virDomainDiskHostDefFree;
 virDomainDiskIndexByName;
 virDomainDiskInsert;
 virDomainDiskInsertPreAlloced;
-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 19/27] qemu: snapshot: Detect internal snapshots also for sheepdog and RBD

2013-11-26 Thread Peter Krempa
When doing an internal snapshot on a VM with sheepdog or RBD disks we
would not set a flag to mark the domain is using internal snapshots and
might end up creating a mixed snapshot. Move the setting of the variable
to avoid this problem.
---
 src/qemu/qemu_driver.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8a1eefd..d2dbaf5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11764,6 +11764,8 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, 
virDomainSnapshotDefPtr def,

 switch (disk-snapshot) {
 case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL:
+found_internal = true;
+
 if (def-state != VIR_DOMAIN_DISK_SNAPSHOT 
 dom_disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK 
 (dom_disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG ||
@@ -11787,7 +11789,6 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, 
virDomainSnapshotDefPtr def,
disk-name);
 goto cleanup;
 }
-found_internal = true;
 break;

 case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL:
-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 26/27] RFC: conf: snapshot: Parse more snapshot information

2013-11-26 Thread Peter Krempa
Add fields to hold information about network volumes for snapshots.

RFC: Missing schema and tests.
---
 src/conf/snapshot_conf.c | 12 
 src/conf/snapshot_conf.h | 15 +--
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index f6f170e..7ad605f 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -153,9 +153,9 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
 if (virDomainDiskSourceDefParse(cur,
 backingtype,
 def-file,
-NULL,
-NULL,
-NULL,
+def-protocol,
+def-nhosts,
+def-hosts,
 NULL)  0)
 goto cleanup;

@@ -632,7 +632,11 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
 virDomainDiskSourceDefFormatInternal(buf,
  type,
  disk-file,
- 0, 0, 0, NULL, 0, NULL, NULL, 0);
+ 0,
+ disk-protocol,
+ disk-nhosts,
+ disk-hosts,
+ 0, NULL, NULL, 0);

 virBufferAddLit(buf, /disk\n);
 }
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 241d63c..bcd92dc 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -48,12 +48,15 @@ enum virDomainSnapshotState {
 typedef struct _virDomainSnapshotDiskDef virDomainSnapshotDiskDef;
 typedef virDomainSnapshotDiskDef *virDomainSnapshotDiskDefPtr;
 struct _virDomainSnapshotDiskDef {
-char *name; /* name matching the target dev='...' of the domain */
-int index; /* index within snapshot-dom-disks that matches name */
-int snapshot; /* enum virDomainSnapshotLocation */
-int type; /* enum virDomainDiskType */
-char *file; /* new source file when snapshot is external */
-int format; /* enum virStorageFileFormat */
+char *name; /* name matching the target dev='...' of the domain */
+int index;  /* index within snapshot-dom-disks that matches name */
+int snapshot;   /* enum virDomainSnapshotLocation */
+int type;   /* enum virDomainDiskType */
+char *file; /* new source file when snapshot is external */
+int format; /* enum virStorageFileFormat */
+int protocol;   /* network source protocol */
+size_t nhosts;  /* network source hosts count */
+virDomainDiskHostDefPtr hosts; /* network source hosts */
 };

 /* Stores the complete snapshot metadata */
-- 
1.8.4.3

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


[libvirt] [PATCHv1.5 24/27] qemu: snapshots: Declare supported and unsupported snapshot configs

2013-11-26 Thread Peter Krempa
Currently the snapshot code did not check if it actually supports
snapshots on various disk backends for domains. To avoid future problems
add checkers that whitelist the supported configurations.
---
 src/qemu/qemu_driver.c | 264 +++--
 1 file changed, 234 insertions(+), 30 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 96bc87e..b9c270b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11738,13 +11738,226 @@ endjob:
 }

 static int
-qemuDomainSnapshotPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
+qemuDomainSnapshotPrepareDiskExternalBacking(virDomainDiskDefPtr disk)
+{
+int actualType = qemuDiskGetActualType(disk);
+
+switch ((enum virDomainDiskType) actualType) {
+case VIR_DOMAIN_DISK_TYPE_BLOCK:
+case VIR_DOMAIN_DISK_TYPE_FILE:
+return 0;
+
+case VIR_DOMAIN_DISK_TYPE_NETWORK:
+switch ((enum virDomainDiskProtocol) disk-protocol) {
+case VIR_DOMAIN_DISK_PROTOCOL_NBD:
+case VIR_DOMAIN_DISK_PROTOCOL_RBD:
+case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
+case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
+case VIR_DOMAIN_DISK_PROTOCOL_HTTP:
+case VIR_DOMAIN_DISK_PROTOCOL_HTTPS:
+case VIR_DOMAIN_DISK_PROTOCOL_FTP:
+case VIR_DOMAIN_DISK_PROTOCOL_FTPS:
+case VIR_DOMAIN_DISK_PROTOCOL_TFTP:
+case VIR_DOMAIN_DISK_PROTOCOL_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(external inactive snapshots are not supported on 

+ 'network' disks using '%s' protocol),
+   virDomainDiskProtocolTypeToString(disk-protocol));
+return -1;
+}
+break;
+
+case VIR_DOMAIN_DISK_TYPE_DIR:
+case VIR_DOMAIN_DISK_TYPE_VOLUME:
+case VIR_DOMAIN_DISK_TYPE_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(external inactive snapshots are not supported on 
+ '%s' disks), virDomainDiskTypeToString(actualType));
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr 
disk)
+{
+int actualType = qemuSnapshotDiskGetActualType(disk);
+
+switch ((enum virDomainDiskType) actualType) {
+case VIR_DOMAIN_DISK_TYPE_BLOCK:
+case VIR_DOMAIN_DISK_TYPE_FILE:
+return 0;
+
+case VIR_DOMAIN_DISK_TYPE_NETWORK:
+case VIR_DOMAIN_DISK_TYPE_DIR:
+case VIR_DOMAIN_DISK_TYPE_VOLUME:
+case VIR_DOMAIN_DISK_TYPE_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(external active snapshots are not supported on 
+ '%s' disks), virDomainDiskTypeToString(actualType));
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+qemuDomainSnapshotPrepareDiskExternalOverlayInactive(virDomainSnapshotDiskDefPtr
 disk)
+{
+int actualType = qemuSnapshotDiskGetActualType(disk);
+
+switch ((enum virDomainDiskType) actualType) {
+case VIR_DOMAIN_DISK_TYPE_BLOCK:
+case VIR_DOMAIN_DISK_TYPE_FILE:
+return 0;
+
+case VIR_DOMAIN_DISK_TYPE_NETWORK:
+case VIR_DOMAIN_DISK_TYPE_DIR:
+case VIR_DOMAIN_DISK_TYPE_VOLUME:
+case VIR_DOMAIN_DISK_TYPE_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(external inactive snapshots are not supported on 
+ '%s' disks), virDomainDiskTypeToString(actualType));
+return -1;
+}
+
+return 0;
+}
+
+
+
+static int
+qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
+  virDomainDiskDefPtr disk,
+  virDomainSnapshotDiskDefPtr snapdisk,
+  bool active,
+  bool reuse)
+{
+int actualType;
+struct stat st;
+
+if (qemuTranslateSnapshotDiskSourcePool(conn, snapdisk)  0)
+return -1;
+
+if (!active) {
+if (qemuTranslateDiskSourcePool(conn, disk)  0)
+return -1;
+
+if (qemuDomainSnapshotPrepareDiskExternalBacking(disk)  0)
+return -1;
+
+if (qemuDomainSnapshotPrepareDiskExternalOverlayInactive(snapdisk)  0)
+return -1;
+} else {
+if (qemuDomainSnapshotPrepareDiskExternalOverlayActive(snapdisk)  0)
+return -1;
+}
+
+actualType = qemuSnapshotDiskGetActualType(snapdisk);
+
+switch ((enum virDomainDiskType) actualType) {
+case VIR_DOMAIN_DISK_TYPE_BLOCK:
+case VIR_DOMAIN_DISK_TYPE_FILE:
+if (stat(snapdisk-file, st)  0) {
+if (errno != ENOENT) {
+virReportSystemError(errno,
+ _(unable to stat for disk %s: %s),
+ snapdisk-name, snapdisk-file);
+return -1;
+} 

[libvirt] [PATCHv1.5 27/27] RFC: qemu: snapshot: Add support for external active snapshots on gluster

2013-11-26 Thread Peter Krempa
This is a basic implementation of snapshots on gluster. Many things are
lacking, especially cleanup after a failed snapshot.
---
 src/qemu/qemu_command.c |   2 +-
 src/qemu/qemu_command.h |   9 
 src/qemu/qemu_driver.c  | 106 
 3 files changed, 109 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0738de2..5e8348f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3826,7 +3826,7 @@ cleanup:
 }


-static int
+int
 qemuGetDriveSourceString(int type,
  const char *src,
  int protocol,
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 66c23cc..ec944ea 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -312,4 +312,13 @@ qemuParseKeywords(const char *str,
   int *retnkeywords,
   int allowEmptyValue);

+int qemuGetDriveSourceString(int type,
+ const char *src,
+ int protocol,
+ size_t nhosts,
+ virDomainDiskHostDefPtr hosts,
+ const char *username,
+ const char *secret,
+ char **path);
+
 #endif /* __QEMU_COMMAND_H__*/
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e6d4f47..4aa88c8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11412,6 +11412,24 @@ cleanup:
 return ret;
 }

+
+static int
+qemuDomainSnapshotDiskGetSourceString(virDomainSnapshotDiskDefPtr disk,
+  char **source)
+{
+*source = NULL;
+
+return qemuGetDriveSourceString(qemuSnapshotDiskGetActualType(disk),
+disk-file,
+disk-protocol,
+disk-nhosts,
+disk-hosts,
+NULL,
+NULL,
+source);
+}
+
+
 typedef enum {
 VIR_DISK_CHAIN_NO_ACCESS,
 VIR_DISK_CHAIN_READ_ONLY,
@@ -11792,6 +11810,29 @@ 
qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
 return 0;

 case VIR_DOMAIN_DISK_TYPE_NETWORK:
+switch ((enum virDomainDiskProtocol) disk-protocol) {
+case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
+return 0;
+
+case VIR_DOMAIN_DISK_PROTOCOL_NBD:
+case VIR_DOMAIN_DISK_PROTOCOL_RBD:
+case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
+case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
+case VIR_DOMAIN_DISK_PROTOCOL_HTTP:
+case VIR_DOMAIN_DISK_PROTOCOL_HTTPS:
+case VIR_DOMAIN_DISK_PROTOCOL_FTP:
+case VIR_DOMAIN_DISK_PROTOCOL_FTPS:
+case VIR_DOMAIN_DISK_PROTOCOL_TFTP:
+case VIR_DOMAIN_DISK_PROTOCOL_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(external active snapshots are not supported on 
+ 'network' disks using '%s' protocol),
+   virDomainDiskProtocolTypeToString(disk-protocol));
+return -1;
+
+}
+break;
+
 case VIR_DOMAIN_DISK_TYPE_DIR:
 case VIR_DOMAIN_DISK_TYPE_VOLUME:
 case VIR_DOMAIN_DISK_TYPE_LAST:
@@ -12101,6 +12142,9 @@ 
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = vm-privateData;
 char *device = NULL;
 char *source = NULL;
+char *newsource = NULL;
+virDomainDiskHostDefPtr newhosts = NULL;
+virDomainDiskHostDefPtr persistHosts = NULL;
 int format = snap-format;
 const char *formatStr = NULL;
 char *persistSource = NULL;
@@ -12114,8 +12158,7 @@ 
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
 return -1;
 }

-if (virAsprintf(device, drive-%s, disk-info.alias)  0 ||
-(persistDisk  VIR_STRDUP(persistSource, source)  0))
+if (virAsprintf(device, drive-%s, disk-info.alias)  0)
 goto cleanup;

 /* XXX Here, we know we are about to alter disk-backingChain if
@@ -12126,14 +12169,22 @@ 
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
 virStorageFileFreeMetadata(disk-backingChain);
 disk-backingChain = NULL;

+if (qemuDomainSnapshotDiskGetSourceString(snap, source)  0)
+goto cleanup;
+
+if (VIR_STRDUP(newsource, snap-file)  0)
+goto cleanup;
+
+if (persistDisk 
+VIR_STRDUP(persistSource, snap-file)  0)
+goto cleanup;
+
 switch (snap-type) {
 case VIR_DOMAIN_DISK_TYPE_BLOCK:
 reuse = true;
 /* fallthrough */
 case -1: /* type was not provided in snapshot conf */
 case VIR_DOMAIN_DISK_TYPE_FILE:
-if (VIR_STRDUP(source, snap-file)  0)
-goto cleanup;

 /* create the stub file and 

[libvirt] [PATCHv1.5 25/27] RFC: snapshot: Add support for specifying snapshot disk backing type

2013-11-26 Thread Peter Krempa
Add support for specifying various types when doing snapshots. This will
later allow to do snapshots on network backed volumes.

RFC: This patch is lacking tests and domain schema touch up.
---
 src/conf/snapshot_conf.c |  9 
 src/qemu/qemu_driver.c   | 56 +++-
 2 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 5958f13..f6f170e 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -108,6 +108,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
 {
 int ret = -1;
 char *snapshot = NULL;
+char *type = NULL;
 xmlNodePtr cur;

 def-name = virXMLPropString(node, name);
@@ -129,6 +130,13 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
 }

 def-type = -1;
+if ((type = virXMLPropString(node, type))) {
+if ((def-type = virDomainDiskTypeFromString(type))  0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(unknown disk snapshot type '%s'), type);
+goto cleanup;
+}
+}

 for (cur = node-children; cur; cur = cur-next) {
 if (cur-type != XML_ELEMENT_NODE)
@@ -174,6 +182,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
 ret = 0;
 cleanup:
 VIR_FREE(snapshot);
+VIR_FREE(type);
 if (ret  0)
 virDomainSnapshotDiskDefClear(def);
 return ret;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b9c270b..e6d4f47 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12115,33 +12115,48 @@ 
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
 }

 if (virAsprintf(device, drive-%s, disk-info.alias)  0 ||
-VIR_STRDUP(source, snap-file)  0 ||
 (persistDisk  VIR_STRDUP(persistSource, source)  0))
 goto cleanup;

-/* create the stub file and set selinux labels; manipulate disk in
- * place, in a way that can be reverted on failure. */
-if (!reuse) {
-fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT,
-  need_unlink, NULL);
-if (fd  0)
-goto cleanup;
-VIR_FORCE_CLOSE(fd);
-}
-
 /* XXX Here, we know we are about to alter disk-backingChain if
- * successful, so we nuke the existing chain so that future
- * commands will recompute it.  Better would be storing the chain
- * ourselves rather than reprobing, but this requires modifying
- * domain_conf and our XML to fully track the chain across
- * libvirtd restarts.  */
+ * successful, so we nuke the existing chain so that future commands will
+ * recompute it.  Better would be storing the chain ourselves rather than
+ * reprobing, but this requires modifying domain_conf and our XML to fully
+ * track the chain across libvirtd restarts.  */
 virStorageFileFreeMetadata(disk-backingChain);
 disk-backingChain = NULL;

-if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
-  VIR_DISK_CHAIN_READ_WRITE)  0) {
-qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
-  VIR_DISK_CHAIN_NO_ACCESS);
+switch (snap-type) {
+case VIR_DOMAIN_DISK_TYPE_BLOCK:
+reuse = true;
+/* fallthrough */
+case -1: /* type was not provided in snapshot conf */
+case VIR_DOMAIN_DISK_TYPE_FILE:
+if (VIR_STRDUP(source, snap-file)  0)
+goto cleanup;
+
+/* create the stub file and set selinux labels; manipulate disk in
+ * place, in a way that can be reverted on failure. */
+if (!reuse) {
+fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT,
+  need_unlink, NULL);
+if (fd  0)
+goto cleanup;
+VIR_FORCE_CLOSE(fd);
+}
+
+if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
+  VIR_DISK_CHAIN_READ_WRITE)  0) {
+qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
+  VIR_DISK_CHAIN_NO_ACCESS);
+goto cleanup;
+}
+break;
+
+default:
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+   _(snapshots are not supported on '%s' volumes),
+   virDomainDiskTypeToString(snap-type));
 goto cleanup;
 }

@@ -12160,6 +12175,7 @@ 
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
 disk-src = source;
 source = NULL;
 disk-format = format;
+disk-type = snap-type;
 if (persistDisk) {
 VIR_FREE(persistDisk-src);
 persistDisk-src = persistSource;
-- 
1.8.4.3

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


[libvirt] [PATCH python] Don't include virDomainSnapshotRef in python API

2013-11-26 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The reference counting API is for internal use only. Attempts
to use it from python application code will cause havoc.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 generator.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/generator.py b/generator.py
index ea5b8a1..c769ed0 100755
--- a/generator.py
+++ b/generator.py
@@ -517,7 +517,8 @@ skip_function = (
 virNWFilterRef,
 virStoragePoolRef,
 virStorageVolRef,
-'virStreamRef',
+virStreamRef,
+virDomainSnapshotRef,
 
 # This functions shouldn't be called via the bindings (and even the docs
 # contain an explicit warning to that effect). The equivalent should be
-- 
1.8.3.1

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


Re: [libvirt] [PATCH v2 2/2] LXC: don't unmount mounts for shared root

2013-11-26 Thread Daniel P. Berrange
On Wed, Nov 20, 2013 at 10:11:09AM +0800, Gao feng wrote:
 Also after commit 5ff9d8a65ce80efb509ce4e8051394e9ed2cd942
 vfs: Lock in place mounts from more privileged users,
 
 unprivileged user has no rights to umount the mounts that
 inherited from parent mountns.
 
 right now, I have no good idea to fix this problem, we need
 to do more research. this patch just skip unmounting these
 mounts for shared root.
 
 BTW, I think when libvirt lxc enables user namespace, the
 configuation that shares root with host is very rara.
 
 Signed-off-by: Gao feng gaof...@cn.fujitsu.com
 ---
  src/lxc/lxc_container.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
 index 3d9b491..fbce8e8 100644
 --- a/src/lxc/lxc_container.c
 +++ b/src/lxc/lxc_container.c
 @@ -1664,7 +1664,9 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr 
 vmDef,
  if (lxcContainerPivotRoot(root)  0)
  goto cleanup;
  
 -if (STREQ(root-src, /) 
 +/* FIXME: we should find a way to unmount these mounts for container
 + * even user namespace is enabled. */
 +if (STREQ(root-src, /)  (!vmDef-idmap.nuidmap) 
  lxcContainerUnmountForSharedRoot(stateDir, vmDef-name)  0)
  goto cleanup;

ACK, this sucks but we have no choice for now. Fortunately not unmounting
these things isn't really harmful - just clutter in /proc/mounts.


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] [PATCH python] Make block pull event dispatcher private

2013-11-26 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The method dispatchDomainEventBlockPullCallback which is
used internally to dispatch block pull events to the python
application code was missing the leading '_', to denote that
it was private.  All other event callback helpers have a
leading '_'. No application should have been using this so
it is justifiable to rename it.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 libvirt-override-virConnect.py | 2 +-
 libvirt-override.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libvirt-override-virConnect.py b/libvirt-override-virConnect.py
index 4ba3d30..23fadfd 100644
--- a/libvirt-override-virConnect.py
+++ b/libvirt-override-virConnect.py
@@ -113,7 +113,7 @@
authScheme, subject, opaque)
 return 0
 
-def dispatchDomainEventBlockPullCallback(self, dom, path, type, status, 
cbData):
+def _dispatchDomainEventBlockPullCallback(self, dom, path, type, status, 
cbData):
 Dispatches events to python user domain blockJob event callbacks
 
 try:
diff --git a/libvirt-override.c b/libvirt-override.c
index 4cc64b7..d3802de 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -5998,7 +5998,7 @@ 
libvirt_virConnectDomainEventBlockJobCallback(virConnectPtr conn ATTRIBUTE_UNUSE
 
 /* Call the Callback Dispatcher */
 pyobj_ret = PyObject_CallMethod(pyobj_conn,
-
(char*)dispatchDomainEventBlockPullCallback,
+
(char*)_dispatchDomainEventBlockPullCallback,
 (char*)OsiiO,
 pyobj_dom, path, type, status, 
pyobj_cbData);
 
-- 
1.8.3.1

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


[libvirt] [PATCH python] Avoid generating the migrate methods in multiple classes

2013-11-26 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The python code generator tries to figure out what class a
method should be in by looking at the list of arguments for
any which are object types. Unfortunately missing break
statements meant that methods which have multiple object
arguments (eg migrate as a virDomainPtr followed by a
virConnectPtr) got added to multiple classes. The migrate
methods should only be visible in the virDomain class, and
the versions in the virConnect class have fubar arguments.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 generator.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/generator.py b/generator.py
index c769ed0..ab2a97f 100755
--- a/generator.py
+++ b/generator.py
@@ -1264,12 +1264,14 @@ def buildWrappers(module):
 func = nameFixup(name, classe, type, file)
 info = (0, func, name, ret, args, file, mod)
 function_classes[classe].append(info)
+break
 elif name[0:3] == vir and len(args) = 2 and args[1][1] == type \
 and file != python_accessor and not name in 
function_skip_index_one:
 found = 1
 func = nameFixup(name, classe, type, file)
 info = (1, func, name, ret, args, file, mod)
 function_classes[classe].append(info)
+break
 if found == 1:
 continue
 func = nameFixup(name, None, file, file)
-- 
1.8.3.1

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


Re: [libvirt] [PATCH] storage: don't read storage volumes in nonblock mode

2013-11-26 Thread Michal Privoznik
On 25.11.2013 22:49, Eric Blake wrote:
 Commit 348b4e2 introduced a potential problem (thankfully not
 in any release): we are attempting to use virFileReadHeaderFD()
 on a file that was opened with O_NONBLOCK.  While this
 shouldn't be a problem in practice (because O_NONBLOCK
 typically doesn't affect regular or block files, and fifos and
 sockets cannot be storage volumes), it's better to play it safe
 to avoid races from opening an unexpected file type while also
 avoiding problems with having to handle EAGAIN while read()ing.
 
 Based on a report by Dan Berrange.
 
 * src/storage/storage_backend.c
 (virStorageBackendVolOpenCheckMode): Fix up fd after avoiding race.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
  src/storage/storage_backend.c | 25 +
  1 file changed, 21 insertions(+), 4 deletions(-)
 
 diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
 index 57c1728..bde39d6 100644
 --- a/src/storage/storage_backend.c
 +++ b/src/storage/storage_backend.c
 @@ -1179,6 +1179,12 @@ virStorageBackendVolOpenCheckMode(const char *path, 
 struct stat *sb,
  return -2;
  }
 
 +/* O_NONBLOCK should only matter during open() for fifos and
 + * sockets, which we already filtered; but using it prevents a
 + * TOCTTOU race.  However, later on we will want to read() the
 + * header from this fd, and virFileRead* routines require a
 + * blocking fd, so fix it up after verifying we avoided a
 + * race.  */
  if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY))  0) {
  if ((errno == ENOENT || errno == ELOOP) 
  S_ISLNK(sb-st_mode)) {
 @@ -1200,13 +1206,13 @@ virStorageBackendVolOpenCheckMode(const char *path, 
 struct stat *sb,
  return -1;
  }
 
 -if (S_ISREG(sb-st_mode))
 +if (S_ISREG(sb-st_mode)) {
  mode = VIR_STORAGE_VOL_OPEN_REG;
 -else if (S_ISCHR(sb-st_mode))
 +} else if (S_ISCHR(sb-st_mode)) {
  mode = VIR_STORAGE_VOL_OPEN_CHAR;
 -else if (S_ISBLK(sb-st_mode))
 +} else if (S_ISBLK(sb-st_mode)) {
  mode = VIR_STORAGE_VOL_OPEN_BLOCK;
 -else if (S_ISDIR(sb-st_mode)) {
 +} else if (S_ISDIR(sb-st_mode)) {
  mode = VIR_STORAGE_VOL_OPEN_DIR;
 
  if (STREQ(base, .) ||
 @@ -1215,6 +1221,17 @@ virStorageBackendVolOpenCheckMode(const char *path, 
 struct stat *sb,
  VIR_INFO(Skipping special dir '%s', base);
  return -2;
  }
 +} else {
 +VIR_WARN(ignoring unexpected type for file '%s', path);
 +VIR_FORCE_CLOSE(fd);
 +return -2;
 +}
 +
 +if (virSetBlocking(fd, true)  0) {
 +virReportSystemError(errno, _(unable to set blocking mode for 
 '%s'),
 + path);
 +VIR_FORCE_CLOSE(fd);
 +return -2;
  }
 
  if (!(mode  flags)) {
 

ACK

Michal

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


Re: [libvirt] [PATCHv1.5 06/27] qemu: Simplify call pattern of qemuBuildDriveURIString

2013-11-26 Thread Michal Privoznik
On 26.11.2013 17:48, Peter Krempa wrote:
 Automatically assign secret type from the disk source definition and
 pull in adding of the comma. Then update callers to keep generated
 output the same.
 ---
  src/qemu/qemu_command.c | 34 +-
  1 file changed, 13 insertions(+), 21 deletions(-)
 
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 36bdc15..2326221 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -3699,9 +3699,7 @@ cleanup:
  static int
  qemuBuildDriveURIString(virConnectPtr conn,
  virDomainDiskDefPtr disk,
 -virBufferPtr opt,
 -int protocol,
 -virSecretUsageType secretUsageType)
 +virBufferPtr opt)
  {
  char *secret = NULL;
  char *builturi = NULL;
 @@ -3709,20 +3707,22 @@ qemuBuildDriveURIString(virConnectPtr conn,
 
  virBufferAddLit(opt, file=);
 
 -if (disk-auth.username  secretUsageType != 
 VIR_SECRET_USAGE_TYPE_NONE) {
 +if (disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI 
 +disk-auth.username) {
  /* Get the secret string using the virDomainDiskDef */
  if (!(secret = qemuGetSecretString(conn,
 -   
 virDomainDiskProtocolTypeToString(protocol),
 +   
 virDomainDiskProtocolTypeToString(disk-protocol),
 false,
 disk-auth.secretType,
 disk-auth.username,
 disk-auth.secret.uuid,
 disk-auth.secret.usage,
 -   secretUsageType)))
 +   VIR_SECRET_USAGE_TYPE_ISCSI)))

I know we were currently using this only for _ISCSI, but when touching
this how about making it more robust and choosing the correct
VIR_SECRET_USAGE_TYPE_ here? The @disk is passed anyway so the decision
can be made here.

  goto cleanup;
  }
 
 -if (!(builturi = qemuBuildNetworkDriveURI(protocol,
 +
 +if (!(builturi = qemuBuildNetworkDriveURI(disk-protocol,
disk-src,
disk-nhosts,
disk-hosts,
 @@ -3731,6 +3731,7 @@ qemuBuildDriveURIString(virConnectPtr conn,
  goto cleanup;
 
  virBufferEscape(opt, ',', ,, %s, builturi);
 +virBufferAddChar(opt, ',');
 
  ret = 0;
 
 @@ -3760,9 +3761,7 @@ qemuBuildNBDString(virConnectPtr conn, 
 virDomainDiskDefPtr disk, virBufferPtr op
   !disk-hosts-name)
  || (disk-hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX
   disk-hosts-socket  disk-hosts-socket[0] != '/'))
 -return qemuBuildDriveURIString(conn, disk, opt,
 -   VIR_DOMAIN_DISK_PROTOCOL_NBD,
 -   VIR_SECRET_USAGE_TYPE_NONE);
 +return qemuBuildDriveURIString(conn, disk, opt);
 
  virBufferAddLit(opt, file=nbd:);
 
 @@ -3792,6 +3791,8 @@ qemuBuildNBDString(virConnectPtr conn, 
 virDomainDiskDefPtr disk, virBufferPtr op
  if (disk-src)
  virBufferEscape(opt, ',', ,, :exportname=%s, disk-src);
 
 +virBufferAddChar(opt, ',');
 +
  return 0;
  }
 
 @@ -3921,7 +3922,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
  case VIR_DOMAIN_DISK_PROTOCOL_NBD:
  if (qemuBuildNBDString(conn, disk, opt)  0)
  goto error;
 -virBufferAddChar(opt, ',');
  break;
  case VIR_DOMAIN_DISK_PROTOCOL_RBD:
  virBufferAddLit(opt, file=);
 @@ -3929,19 +3929,11 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
  goto error;
  virBufferAddChar(opt, ',');
  break;
 +
  case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
 -if (qemuBuildDriveURIString(conn, disk, opt,
 -VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
 -VIR_SECRET_USAGE_TYPE_NONE)  0)
 -goto error;
 -virBufferAddChar(opt, ',');
 -break;
  case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
 -if (qemuBuildDriveURIString(conn, disk, opt,
 -VIR_DOMAIN_DISK_PROTOCOL_ISCSI,
 -VIR_SECRET_USAGE_TYPE_ISCSI)  0)
 +if (qemuBuildDriveURIString(conn, disk, opt)  0)
  goto error;
 -virBufferAddChar(opt, ',');
  break;
 
  case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
 

Weak ACK due to comment above.


Re: [libvirt] [PATCHv1.5 07/27] qemu: Use qemuBuildNetworkDriveURI to handle http/ftp and friends

2013-11-26 Thread Michal Privoznik
On 26.11.2013 17:48, Peter Krempa wrote:
 Prepare the function to integrate other protocols and start folding
 other network protocols into a common place.
 ---
  src/qemu/qemu_command.c | 199 
 +---
  1 file changed, 122 insertions(+), 77 deletions(-)

ACK

Michal

(this is the point where I have to stop for today. I'll continue
tomorrow unless somebody reviews the rest)

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


Re: [libvirt] [PATCHv1.5 05/27] qemu: Split out formatting of network disk source URI

2013-11-26 Thread Michal Privoznik
On 26.11.2013 17:48, Peter Krempa wrote:
 The snapshot code will need to use qemu-style formatted URIs of network
 disks. Split out the code to avoid duplication.
 ---
  src/qemu/qemu_command.c | 146 
 
  src/qemu/qemu_command.h |   6 ++
  2 files changed, 91 insertions(+), 61 deletions(-)
 

ACK

Michal

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


Re: [libvirt] [PATCHv1.5 03/27] qemuxml2argv: Add test to verify correct usage of disk type=volume

2013-11-26 Thread Michal Privoznik
On 26.11.2013 17:48, Peter Krempa wrote:
 Tweak the existing file to test command line generator too.
 ---
  tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args | 8 
  tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml  | 2 +-
  tests/qemuxml2argvtest.c  | 2 ++
  3 files changed, 11 insertions(+), 1 deletion(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args

ACK

Michal

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


Re: [libvirt] [PATCHv1.5 01/27] conf: Export virStorageVolType enum helper functions

2013-11-26 Thread Michal Privoznik
On 26.11.2013 17:48, Peter Krempa wrote:
 Export string conversion from and to the virStorageVolType enum.
 ---
  src/libvirt_private.syms | 2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index a705c56..205fe56 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -701,6 +701,8 @@ virStorageVolDefFree;
  virStorageVolDefParseFile;
  virStorageVolDefParseNode;
  virStorageVolDefParseString;
 +virStorageVolTypeFromString;
 +virStorageVolTypeToString;
 
 
  # conf/storage_encryption_conf.h
 

ACK, although it can be squashed into 02/27 where the functions are
first used outside storage driver.

Michal

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


Re: [libvirt] [PATCHv1.5 04/27] qemuxml2argv: Add test for disk type='volume' with iSCSI pools

2013-11-26 Thread Michal Privoznik
On 26.11.2013 17:48, Peter Krempa wrote:
 Tweak the existing file so that it can be tested for command line
 corectness.
 ---
  src/conf/domain_conf.h |   1 +
  src/libvirt_private.syms   |   1 +
  src/qemu/qemu_command.c|  76 +---
  src/qemu/qemu_conf.c   | 129 
 ++---
  src/qemu/qemu_conf.h   |   2 +
  .../qemuxml2argv-disk-source-pool-mode.args|  10 ++
  .../qemuxml2argv-disk-source-pool-mode.xml |   4 +-
  tests/qemuxml2argvtest.c   |   2 +
  8 files changed, 112 insertions(+), 113 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args
 
 diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
 index 4561ccc..a5ef2ca 100644
 --- a/src/conf/domain_conf.h
 +++ b/src/conf/domain_conf.h
 @@ -686,6 +686,7 @@ struct _virDomainDiskSourcePoolDef {
  char *volume; /* volume name */
  int voltype; /* enum virStorageVolType, internal only */
  int pooltype; /* enum virStoragePoolType, internal only */
 +int actualtype; /* enum virDomainDiskType, internal only */
  int mode; /* enum virDomainDiskSourcePoolMode */
  };
  typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr;
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 205fe56..aeb3568 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -693,6 +693,7 @@ virStoragePoolSourceFree;
  virStoragePoolSourceListFormat;
  virStoragePoolSourceListNewSource;
  virStoragePoolTypeFromString;
 +virStoragePoolTypeToString;
  virStorageVolDefFindByKey;
  virStorageVolDefFindByName;
  virStorageVolDefFindByPath;
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 763417f..b8129a3 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -3775,69 +3775,6 @@ qemuBuildNBDString(virConnectPtr conn, 
 virDomainDiskDefPtr disk, virBufferPtr op
  return 0;
  }
 
 -static int
 -qemuBuildVolumeString(virConnectPtr conn,
 -  virDomainDiskDefPtr disk,
 -  virBufferPtr opt)
 -{
 -int ret = -1;
 -
 -switch ((virStorageVolType) disk-srcpool-voltype) {
 -case VIR_STORAGE_VOL_DIR:
 -if (!disk-readonly) {
 -virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 -   _(cannot create virtual FAT disks in read-write 
 mode));
 -goto cleanup;
 -}
 -if (disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
 -virBufferEscape(opt, ',', ,, file=fat:floppy:%s,, disk-src);
 -else
 -virBufferEscape(opt, ',', ,, file=fat:%s,, disk-src);
 -break;
 -case VIR_STORAGE_VOL_BLOCK:
 -if (disk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) {
 -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 -   _(tray status 'open' is invalid for 
 - block type volume));
 -goto cleanup;
 -}
 -if (disk-srcpool-pooltype == VIR_STORAGE_POOL_ISCSI) {
 -if (disk-srcpool-mode ==
 -VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) {
 -if (qemuBuildISCSIString(conn, disk, opt)  0)
 -goto cleanup;
 -virBufferAddChar(opt, ',');
 -} else if (disk-srcpool-mode ==
 -   VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) {
 -virBufferEscape(opt, ',', ,, file=%s,, disk-src);
 -}
 -} else {
 -virBufferEscape(opt, ',', ,, file=%s,, disk-src);
 -}
 -break;
 -case VIR_STORAGE_VOL_FILE:
 -if (disk-auth.username) {
 -if (qemuBuildISCSIString(conn, disk, opt)  0)
 -goto cleanup;
 -virBufferAddChar(opt, ',');
 -} else {
 -virBufferEscape(opt, ',', ,, file=%s,, disk-src);
 -}
 -break;
 -case VIR_STORAGE_VOL_NETWORK:
 -case VIR_STORAGE_VOL_NETDIR:
 -case VIR_STORAGE_VOL_LAST:
 -/* Keep the compiler quiet, qemuTranslateDiskSourcePool already
 - * reported the unsupported error.
 - */
 -goto cleanup;
 -}
 -
 -ret = 0;
 -
 -cleanup:
 -return ret;
 -}
 
  char *
  qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 @@ -3851,6 +3788,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
  virDomainDiskGeometryTransTypeToString(disk-geometry.trans);
  int idx = virDiskNameToIndex(disk-dst);
  int busid = -1, unitid = -1;
 +int actualType = qemuDiskGetActualType(disk);
 
  if (idx  0) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 @@ -3934,12 +3872,13 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 
  /* disk-src is NULL when we use nbd disks */
  if ((disk-src ||
 -(disk-type == 

[libvirt] [PATCH python] Add missing binding of security model/label APIs

2013-11-26 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The virNodeGetSecurityModel, virDomainGetSecurityLabel and
virDomainGetSecurityLabelList methods were disabled in the
python binding for inexplicable reasons.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 generator.py |  6 ++--
 libvirt-override-api.xml | 15 +
 libvirt-override.c   | 82 
 typewrappers.c   |  9 ++
 typewrappers.h   |  1 +
 5 files changed, 110 insertions(+), 3 deletions(-)

diff --git a/generator.py b/generator.py
index ab2a97f..273efbd 100755
--- a/generator.py
+++ b/generator.py
@@ -381,6 +381,9 @@ skip_impl = (
 'virDomainGetJobInfo',
 'virDomainGetJobStats',
 'virNodeGetInfo',
+'virNodeGetSecurityModel',
+'virDomainGetSecurityLabel',
+'virDomainGetSecurityLabelList',
 'virDomainGetUUID',
 'virDomainGetUUIDString',
 'virDomainLookupByUUID',
@@ -476,9 +479,6 @@ skip_function = (
 'virCopyLastError', # Python API is called virGetLastError instead
 'virConnectOpenAuth', # Python C code is manually written
 'virDefaultErrorFunc', # Python virErrorFuncHandler impl calls this from C
-'virDomainGetSecurityLabel', # Needs investigation...
-'virDomainGetSecurityLabelList', # Needs investigation...
-'virNodeGetSecurityModel', # Needs investigation...
 'virConnectDomainEventRegister',   # overridden in virConnect.py
 'virConnectDomainEventDeregister', # overridden in virConnect.py
 'virConnectDomainEventRegisterAny',   # overridden in virConnect.py
diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
index 337d0a0..d5b25b5 100644
--- a/libvirt-override-api.xml
+++ b/libvirt-override-api.xml
@@ -104,6 +104,21 @@
   return type='char *' info='the list of information or None in case of 
error'/
   arg name='conn' type='virConnectPtr' info='pointer to the hypervisor 
connection'/
 /function
+function name='virNodeGetSecurityModel' file='python'
+  infoExtract information about the host security model/info
+  return type='char *' info='the list of information or None in case of 
error'/
+  arg name='conn' type='virConnectPtr' info='pointer to the hypervisor 
connection'/
+/function
+function name='virDomainGetSecurityLabel' file='python'
+  infoExtract information about the domain security label. Only the 
first label will be returned./info
+  return type='char *' info='the list of information or None in case of 
error'/
+  arg name='domain' type='virDomainPtr' info='a domain object'/
+/function
+function name='virDomainGetSecurityLabelList' file='python'
+  infoExtract information about the domain security label. A list of all 
labels will be returned./info
+  return type='char *' info='the list of information or None in case of 
error'/
+  arg name='domain' type='virDomainPtr' info='a domain object'/
+/function
 function name='virNodeGetCPUStats' file='python'
   infoExtract node's CPU statistics./info
   return type='char *' info='dictionary mapping field names to values or 
None in case of error'/
diff --git a/libvirt-override.c b/libvirt-override.c
index d3802de..42e253e 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -2865,6 +2865,83 @@ libvirt_virNodeGetInfo(PyObject *self ATTRIBUTE_UNUSED, 
PyObject *args) {
 }
 
 static PyObject *
+libvirt_virNodeGetSecurityModel(PyObject *self ATTRIBUTE_UNUSED, PyObject 
*args) {
+PyObject *py_retval;
+int c_retval;
+virConnectPtr conn;
+PyObject *pyobj_conn;
+virSecurityModel model;
+
+if (!PyArg_ParseTuple(args, (char *)O:virDomainGetSecurityModel, 
pyobj_conn))
+return NULL;
+conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_retval = virNodeGetSecurityModel(conn, model);
+LIBVIRT_END_ALLOW_THREADS;
+if (c_retval  0)
+return VIR_PY_NONE;
+py_retval = PyList_New(2);
+PyList_SetItem(py_retval, 0, libvirt_constcharPtrWrap(model.model[0]));
+PyList_SetItem(py_retval, 1, libvirt_constcharPtrWrap(model.doi[0]));
+return py_retval;
+}
+
+static PyObject *
+libvirt_virDomainGetSecurityLabel(PyObject *self ATTRIBUTE_UNUSED, PyObject 
*args) {
+PyObject *py_retval;
+int c_retval;
+virDomainPtr dom;
+PyObject *pyobj_dom;
+virSecurityLabel label;
+
+if (!PyArg_ParseTuple(args, (char *)O:virDomainGetSecurityLabel, 
pyobj_dom))
+return NULL;
+dom = (virDomainPtr) PyvirDomain_Get(pyobj_dom);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_retval = virDomainGetSecurityLabel(dom, label);
+LIBVIRT_END_ALLOW_THREADS;
+if (c_retval  0)
+return VIR_PY_NONE;
+py_retval = PyList_New(2);
+PyList_SetItem(py_retval, 0, libvirt_constcharPtrWrap(label.label[0]));
+PyList_SetItem(py_retval, 1, libvirt_boolWrap(label.enforcing));
+return py_retval;
+}
+
+#if LIBVIR_CHECK_VERSION(0, 9, 

Re: [libvirt] [PATCHv1.5 02/27] test: Implement fake storage pool driver in qemuxml2argv test

2013-11-26 Thread Michal Privoznik
On 26.11.2013 17:48, Peter Krempa wrote:
 To support testing of volume disk backing, we need to implement a few
 disk driver backend functions.
 
 The fake storage driver uses files in storagepoolxml2xmlout/POOLNAME.xml
 as XML files for pool definitions and volume names are in format
 VOL_TYPE+VOL_PATH. By default type block is assumed (for iSCSI test
 compatibility).
 
 The choice of this approach along with implemented functions was made so
 that disk type='volume' can be tested in the xml2argv test.
 ---
  tests/qemuxml2argvtest.c | 162 
 +++
  1 file changed, 162 insertions(+)
 

ACK

Michal

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


[libvirt] [PATCH python] Improve quality of sanitytest check

2013-11-26 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Validate that every public API method is mapped into the python
and that every python method has a sane C API.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 sanitytest.py | 309 +++---
 setup.py  |  35 +++
 2 files changed, 294 insertions(+), 50 deletions(-)
 mode change 100755 = 100644 sanitytest.py

diff --git a/sanitytest.py b/sanitytest.py
old mode 100755
new mode 100644
index 517054b..9e4c261
--- a/sanitytest.py
+++ b/sanitytest.py
@@ -1,40 +1,283 @@
 #!/usr/bin/python
 
 import sys
+import lxml
+import lxml.etree
+import string
 
+# Munge import path to insert build location for libvirt mod
 sys.path.insert(0, sys.argv[1])
-
 import libvirt
+import libvirtmod
+
+# Path to the libvirt API XML file
+xml = sys.argv[2]
+
+f = open(xml, r)
+tree = lxml.etree.parse(f)
+
+verbose = False
+
+wantenums = []
+wantfunctions = []
+
+# Phase 1: Identify all functions and enums in public API
+set = tree.xpath('/api/files/file/exports[@type=function]/@symbol')
+for n in set:
+wantfunctions.append(n)
+
+set = tree.xpath('/api/files/file/exports[@type=enum]/@symbol')
+for n in set:
+wantenums.append(n)
+
+
+# Phase 2: Identify all classes and methods in the 'libvirt' python module
+gotenums = []
+gottypes = []
+gotfunctions = { libvirt: [] }
+
+for name in dir(libvirt):
+if name[0] == '_':
+continue
+thing = getattr(libvirt, name)
+if type(thing) == int:
+gotenums.append(name)
+elif type(thing) == type:
+gottypes.append(name)
+gotfunctions[name] = []
+elif callable(thing):
+gotfunctions[libvirt].append(name)
+else:
+   pass
+
+for klassname in gottypes:
+klassobj = getattr(libvirt, klassname)
+for name in dir(klassobj):
+if name[0] == '_':
+continue
+thing = getattr(klassobj, name)
+if callable(thing):
+gotfunctions[klassname].append(name)
+else:
+pass
+
+
+# Phase 3: First cut at mapping of C APIs to python classes + methods
+basicklassmap = {}
+
+for cname in wantfunctions:
+name = cname
+# Some virConnect APIs have stupid names
+if name[0:7] == virNode and name[0:13] != virNodeDevice:
+name = virConnect + name[7:]
+if name[0:7] == virConn and name[0:10] != virConnect:
+name = virConnect + name[7:]
+
+# The typed param APIs are only for internal use
+if name[0:14] == virTypedParams:
+continue
+
+# These aren't functions, they're callback signatures
+if name in [virConnectAuthCallbackPtr, virConnectCloseFunc,
+virStreamSinkFunc, virStreamSourceFunc, 
virStreamEventCallback,
+virEventHandleCallback, virEventTimeoutCallback, 
virFreeCallback]:
+continue
+if name[0:21] == virConnectDomainEvent and name[-8:] == Callback:
+continue
+
+
+# virEvent APIs go into main 'libvirt' namespace not any class
+if name[0:8] == virEvent:
+if name[-4:] == Func:
+continue
+basicklassmap[name] = [libvirt, name, cname]
+else:
+found = False
+# To start with map APIs to classes based on the
+# naming prefix. Mistakes will be fixed in next
+# loop
+for klassname in gottypes:
+klen = len(klassname)
+if name[0:klen] == klassname:
+found = True
+if name not in basicklassmap:
+basicklassmap[name] = [klassname, name[klen:], cname]
+elif len(basicklassmap[name])  klassname:
+basicklassmap[name] = [klassname, name[klen:], cname]
+
+# Anything which can't map to a class goes into the
+# global namespaces
+if not found:
+basicklassmap[name] = [libvirt, name[3:], cname]
+
+
+# Phase 4: Deal with oh so many special cases in C - python mapping
+finalklassmap = {}
+
+for name in sorted(basicklassmap):
+klass = basicklassmap[name][0]
+func = basicklassmap[name][1]
+cname = basicklassmap[name][2]
+
+# The object lifecycle APIs are irrelevant since they're
+# used inside the object constructors/destructors.
+if func in [Ref, Free, New, GetConnect, GetDomain]:
+if klass == virStream and func == New:
+klass = virConnect
+func = NewStream
+else:
+continue
+
+
+# All the error handling methods need special handling
+if klass == libvirt:
+if func in [CopyLastError, DefaultErrorFunc,
+ErrorFunc, FreeError,
+SaveLastError, ResetError]:
+continue
+elif func in [GetLastError, GetLastErrorMessage, ResetLastError, 
Initialize]:
+func = vir + func
+elif func == SetErrorFunc:
+func = RegisterErrorHandler
+elif klass == virConnect:
+if func in [CopyLastError, SetErrorFunc]:
+  

[libvirt] [libvirt-python] Call virGetLastError from mod rather than py wrapper

2013-11-26 Thread Doug Goldstein
All other code always calls the methods from the mod rather than using
the python wrapper so this matches the state of all other callers.
---
 libvirt-override.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-override.py b/libvirt-override.py
index ccfec48..87996f8 100644
--- a/libvirt-override.py
+++ b/libvirt-override.py
@@ -20,7 +20,7 @@ class libvirtError(Exception):
 
 # Never call virConnGetLastError().
 # virGetLastError() is now thread local
-err = virGetLastError()
+err = libvirtmod.virGetLastError()
 if err is None:
 msg = defmsg
 else:
-- 
1.8.3.2

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


Re: [libvirt] [PATCH python] Improve quality of sanitytest check

2013-11-26 Thread Doug Goldstein
On Tue, Nov 26, 2013 at 12:32 PM, Daniel P. Berrange
berra...@redhat.com wrote:
 From: Daniel P. Berrange berra...@redhat.com

 Validate that every public API method is mapped into the python
 and that every python method has a sane C API.

Looks like we had the same idea and even a similar approach as well.


 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  sanitytest.py | 309 
 +++---
  setup.py  |  35 +++
  2 files changed, 294 insertions(+), 50 deletions(-)
  mode change 100755 = 100644 sanitytest.py

 diff --git a/sanitytest.py b/sanitytest.py
 old mode 100755
 new mode 100644
 index 517054b..9e4c261
 --- a/sanitytest.py
 +++ b/sanitytest.py
 @@ -1,40 +1,283 @@
  #!/usr/bin/python

  import sys
 +import lxml
 +import lxml.etree
 +import string

 +# Munge import path to insert build location for libvirt mod
  sys.path.insert(0, sys.argv[1])
 -
  import libvirt
 +import libvirtmod

I wouldn't directly import libvirtmod due to Cygwin. I'd just use
libvirt.libvirtmod which is what its available as.

 +
 +# Path to the libvirt API XML file
 +xml = sys.argv[2]
 +
 +f = open(xml, r)
 +tree = lxml.etree.parse(f)
 +
 +verbose = False
 +
 +wantenums = []
 +wantfunctions = []
 +
 +# Phase 1: Identify all functions and enums in public API
 +set = tree.xpath('/api/files/file/exports[@type=function]/@symbol')
 +for n in set:
 +wantfunctions.append(n)
 +
 +set = tree.xpath('/api/files/file/exports[@type=enum]/@symbol')
 +for n in set:
 +wantenums.append(n)
 +

Maybe its a bit ugly but I actually grabbed the typedef's as well to
check the various namespaces (e.g. virConnect, virDomain) but not sure
if we want that.

 +
 +# Phase 2: Identify all classes and methods in the 'libvirt' python module
 +gotenums = []
 +gottypes = []
 +gotfunctions = { libvirt: [] }
 +
 +for name in dir(libvirt):
 +if name[0] == '_':
 +continue
 +thing = getattr(libvirt, name)
 +if type(thing) == int:
 +gotenums.append(name)
 +elif type(thing) == type:
 +gottypes.append(name)
 +gotfunctions[name] = []
 +elif callable(thing):
 +gotfunctions[libvirt].append(name)
 +else:
 +   pass

Could the body of this be made into a function reused below?

 +
 +for klassname in gottypes:
 +klassobj = getattr(libvirt, klassname)
 +for name in dir(klassobj):
 +if name[0] == '_':
 +continue
 +thing = getattr(klassobj, name)
 +if callable(thing):
 +gotfunctions[klassname].append(name)
 +else:
 +pass
 +
 +
 +# Phase 3: First cut at mapping of C APIs to python classes + methods
 +basicklassmap = {}
 +
 +for cname in wantfunctions:
 +name = cname
 +# Some virConnect APIs have stupid names
 +if name[0:7] == virNode and name[0:13] != virNodeDevice:
 +name = virConnect + name[7:]
 +if name[0:7] == virConn and name[0:10] != virConnect:
 +name = virConnect + name[7:]
 +
 +# The typed param APIs are only for internal use
 +if name[0:14] == virTypedParams:
 +continue
 +
 +# These aren't functions, they're callback signatures
 +if name in [virConnectAuthCallbackPtr, virConnectCloseFunc,
 +virStreamSinkFunc, virStreamSourceFunc, 
 virStreamEventCallback,
 +virEventHandleCallback, virEventTimeoutCallback, 
 virFreeCallback]:
 +continue
 +if name[0:21] == virConnectDomainEvent and name[-8:] == Callback:
 +continue
 +
 +
 +# virEvent APIs go into main 'libvirt' namespace not any class
 +if name[0:8] == virEvent:
 +if name[-4:] == Func:
 +continue
 +basicklassmap[name] = [libvirt, name, cname]
 +else:
 +found = False
 +# To start with map APIs to classes based on the
 +# naming prefix. Mistakes will be fixed in next
 +# loop
 +for klassname in gottypes:
 +klen = len(klassname)
 +if name[0:klen] == klassname:
 +found = True
 +if name not in basicklassmap:
 +basicklassmap[name] = [klassname, name[klen:], cname]
 +elif len(basicklassmap[name])  klassname:
 +basicklassmap[name] = [klassname, name[klen:], cname]
 +
 +# Anything which can't map to a class goes into the
 +# global namespaces
 +if not found:
 +basicklassmap[name] = [libvirt, name[3:], cname]
 +
 +
 +# Phase 4: Deal with oh so many special cases in C - python mapping
 +finalklassmap = {}
 +
 +for name in sorted(basicklassmap):
 +klass = basicklassmap[name][0]
 +func = basicklassmap[name][1]
 +cname = basicklassmap[name][2]
 +
 +# The object lifecycle APIs are irrelevant since they're
 +# used inside the object constructors/destructors.
 +if func in [Ref, Free, New, GetConnect, GetDomain]:
 +if klass == virStream and func == New:
 +  

Re: [libvirt] [PATCH python] Don't include virDomainSnapshotRef in python API

2013-11-26 Thread Eric Blake
On 11/26/2013 09:55 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The reference counting API is for internal use only. Attempts
 to use it from python application code will cause havoc.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  generator.py | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

ACK.

 
 diff --git a/generator.py b/generator.py
 index ea5b8a1..c769ed0 100755
 --- a/generator.py
 +++ b/generator.py
 @@ -517,7 +517,8 @@ skip_function = (
  virNWFilterRef,
  virStoragePoolRef,
  virStorageVolRef,
 -'virStreamRef',
 +virStreamRef,
 +virDomainSnapshotRef,
  
  # This functions shouldn't be called via the bindings (and even the docs
  # contain an explicit warning to that effect). The equivalent should be
 

-- 
Eric Blake   eblake 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 python] Make block pull event dispatcher private

2013-11-26 Thread Eric Blake
On 11/26/2013 09:57 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The method dispatchDomainEventBlockPullCallback which is
 used internally to dispatch block pull events to the python
 application code was missing the leading '_', to denote that
 it was private.  All other event callback helpers have a
 leading '_'. No application should have been using this so
 it is justifiable to rename it.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  libvirt-override-virConnect.py | 2 +-
  libvirt-override.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

ACK.

-- 
Eric Blake   eblake 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 python] Avoid generating the migrate methods in multiple classes

2013-11-26 Thread Eric Blake
On 11/26/2013 10:00 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The python code generator tries to figure out what class a
 method should be in by looking at the list of arguments for
 any which are object types. Unfortunately missing break
 statements meant that methods which have multiple object
 arguments (eg migrate as a virDomainPtr followed by a
 virConnectPtr) got added to multiple classes. The migrate
 methods should only be visible in the virDomain class, and
 the versions in the virConnect class have fubar arguments.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  generator.py | 2 ++
  1 file changed, 2 insertions(+)

ACK.  Wow - this repo split has forced us to address some long-standing
bugs.

-- 
Eric Blake   eblake 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 python] Add missing binding of security model/label APIs

2013-11-26 Thread Eric Blake
On 11/26/2013 11:32 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The virNodeGetSecurityModel, virDomainGetSecurityLabel and
 virDomainGetSecurityLabelList methods were disabled in the
 python binding for inexplicable reasons.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---

 +++ b/libvirt-override.c
 @@ -2865,6 +2865,83 @@ libvirt_virNodeGetInfo(PyObject *self 
 ATTRIBUTE_UNUSED, PyObject *args) {
  }
  
  static PyObject *
 +libvirt_virNodeGetSecurityModel(PyObject *self ATTRIBUTE_UNUSED, PyObject 
 *args) {

{ on its own line (Hmm, we aren't very consistent in this file)

 +PyObject *py_retval;
 +int c_retval;
 +virConnectPtr conn;
 +PyObject *pyobj_conn;
 +virSecurityModel model;
 +
 +if (!PyArg_ParseTuple(args, (char *)O:virDomainGetSecurityModel, 
 pyobj_conn))
 +return NULL;
 +conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
 +
 +LIBVIRT_BEGIN_ALLOW_THREADS;
 +c_retval = virNodeGetSecurityModel(conn, model);
 +LIBVIRT_END_ALLOW_THREADS;
 +if (c_retval  0)
 +return VIR_PY_NONE;
 +py_retval = PyList_New(2);

If PyList_New() fails, py_retval is NULL,...

 +PyList_SetItem(py_retval, 0, libvirt_constcharPtrWrap(model.model[0]));

...and this will crash.  Instead, you should just return NULL early.
(Then again, we have LOTS of places where we aren't checking the return
of PyList_SetItem for failure, which is a major cleanup in its own right).

 +PyList_SetItem(py_retval, 1, libvirt_constcharPtrWrap(model.doi[0]));
 +return py_retval;
 +}
 +
 +static PyObject *
 +libvirt_virDomainGetSecurityLabel(PyObject *self ATTRIBUTE_UNUSED, PyObject 
 *args) {

{ placement

 +py_retval = PyList_New(2);
 +PyList_SetItem(py_retval, 0, libvirt_constcharPtrWrap(label.label[0]));

Same comment on error handling.

 +PyList_SetItem(py_retval, 1, libvirt_boolWrap(label.enforcing));
 +return py_retval;
 +}
 +
 +#if LIBVIR_CHECK_VERSION(0, 9, 13)
 +static PyObject *
 +libvirt_virDomainGetSecurityLabelList(PyObject *self ATTRIBUTE_UNUSED, 
 PyObject *args) {

{ placement

 +py_retval = PyList_New(0);
 +for (i = 0 ; i  c_retval ; i++) {

No space before semicolon (hmm, we lost our syntax checker in the split)

 +PyObject *entry = PyList_New(2);
 +PyList_SetItem(entry, 0, 
 libvirt_constcharPtrWrap(labels[i].label[0]));

Again, can't libvirt_constcharPtrWrap() return NULL (on OOM situations),
which will crash PyList_SetItem()?

Only findings are style-related or part of a bigger cleanup needed for
proper error handling, so ACK.

-- 
Eric Blake   eblake 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] [libvirt-python] Call virGetLastError from mod rather than py wrapper

2013-11-26 Thread Eric Blake
On 11/26/2013 12:00 PM, Doug Goldstein wrote:
 All other code always calls the methods from the mod rather than using
 the python wrapper so this matches the state of all other callers.
 ---
  libvirt-override.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

ACK

 
 diff --git a/libvirt-override.py b/libvirt-override.py
 index ccfec48..87996f8 100644
 --- a/libvirt-override.py
 +++ b/libvirt-override.py
 @@ -20,7 +20,7 @@ class libvirtError(Exception):
  
  # Never call virConnGetLastError().
  # virGetLastError() is now thread local
 -err = virGetLastError()
 +err = libvirtmod.virGetLastError()
  if err is None:
  msg = defmsg
  else:
 

-- 
Eric Blake   eblake 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] [RFC PATCH] libvirt support to force convergence of live guest migration

2013-11-26 Thread Chegu Vinod

On 11/26/2013 4:24 AM, Jiri Denemark wrote:

On Thu, Nov 21, 2013 at 17:47:24 -0800, Chegu Vinod wrote:

Busy enterprise workloads hosted on large sized VM's tend to dirty
memory faster than the transfer rate achieved via live guest migration.
Despite some good recent improvements ( using dedicated 10Gig NICs
between hosts) the live migration may NOT converge.

Recently support was added in qemu (version 1.6) to allow a user to
choose if they wish to force convergence of their migration via a
new migration capability : auto-converge. This feature allows for qemu
to auto-detect lack of convergence and trigger a throttle-down of the
VCPUs.

This RFC patch includes the libvirt support needed to trigger this
feature. (Testing is still in progress)

Signed-off-by:  Chegu Vinod chegu_vi...@hp.com
---
  include/libvirt/libvirt.h.in |1 +
  src/qemu/qemu_migration.c|   44 ++
  src/qemu/qemu_migration.h|1 +
  src/qemu/qemu_monitor.c  |2 +-
  src/qemu/qemu_monitor.h  |1 +
  tools/virsh-domain.c |7 ++
  6 files changed, 55 insertions(+), 1 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 146a59b..13b0bfc 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1192,6 +1192,7 @@ typedef enum {
  VIR_MIGRATE_OFFLINE   = (1  10), /* offline migrate */
  VIR_MIGRATE_COMPRESSED= (1  11), /* compress data during 
migration */
  VIR_MIGRATE_ABORT_ON_ERROR= (1  12), /* abort migration on I/O 
errors happened during migration */
+VIR_MIGRATE_AUTO_CONVERGE = (1  13), /* force auto-convergence 
during during migration */
  } virDomainMigrateFlags;

I feel like there must be a better name we could use for this flag but
I'm not able to come up with one... :-)


diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index e87ea85..8cc0c56 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1565,6 +1565,40 @@ cleanup:
  }
  
  static int

+qemuMigrationSetAutoConverge(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+enum qemuDomainAsyncJob job)

Nicely copiedpasted from qemuMigrationSetCompression but you forgot to
fix indentation :-)
Yes being very new to libvirt...I did leverage from the previous 
changes :)

'will fix the indentation.


+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int ret;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, job)  0)
+return -1;
+
+ret = qemuMonitorGetMigrationCapability(
+priv-mon,
+QEMU_MONITOR_MIGRATION_CAPS_AUTO_CONVERGE);
+
+if (ret  0) {
+goto cleanup;
+} else if (ret == 0) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+   _(Auto-Converge migration is not supported by 
+ QEMU binary));

Reduce indentation by one level. Also I think migration is not really
needed in the error message, I'd just change it to
Auto-converge is not supported by QEMU binary.


Ok got it.




+ret = -1;
+goto cleanup;
+}
+
+ret = qemuMonitorSetMigrationCapability(
+priv-mon,
+QEMU_MONITOR_MIGRATION_CAPS_AUTO_CONVERGE);
+
+cleanup:
+qemuDomainObjExitMonitor(driver, vm);
+return ret;
+}
+
+static int
  qemuMigrationWaitForSpice(virQEMUDriverPtr driver,
virDomainObjPtr vm)
  {
@@ -2389,6 +2423,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
  QEMU_ASYNC_JOB_MIGRATION_IN)  0)
  goto stop;
  
+if (flags  VIR_MIGRATE_AUTO_CONVERGE 

+qemuMigrationSetAutoConverge(driver, vm,
+QEMU_ASYNC_JOB_MIGRATION_IN)  0)
+goto stop;
+

Hmm, I know you are just following what I did with
VIR_MIGRATE_COMPRESSED, but setting auto-converge on destination doesn't
make any sense. And it doesn't even make a lot of sense to set
compression on destination (other than checking the destination supports
compression) so I'm wondering why I did so.


  if (mig-lockState) {
  VIR_DEBUG(Received lockstate %s, mig-lockState);
  VIR_FREE(priv-lockState);
@@ -3181,6 +3220,11 @@ qemuMigrationRun(virQEMUDriverPtr driver,
  QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
  goto cleanup;
  
+if (flags  VIR_MIGRATE_AUTO_CONVERGE 

+qemuMigrationSetAutoConverge(driver, vm,
+QEMU_ASYNC_JOB_MIGRATION_OUT)  0)

Indentation is off by one space.

Ok

+goto cleanup;
+
  if (qemuDomainObjEnterMonitorAsync(driver, vm,
 QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
  goto cleanup;

...

So except for the small issues, the patch looks good to me. However, do
I remember correctly that this feature can be 

Re: [libvirt] [PATCH] storage: don't read storage volumes in nonblock mode

2013-11-26 Thread Eric Blake
On 11/26/2013 10:19 AM, Michal Privoznik wrote:
 On 25.11.2013 22:49, Eric Blake wrote:
 Commit 348b4e2 introduced a potential problem (thankfully not
 in any release): we are attempting to use virFileReadHeaderFD()
 on a file that was opened with O_NONBLOCK.  While this
 shouldn't be a problem in practice (because O_NONBLOCK
 typically doesn't affect regular or block files, and fifos and
 sockets cannot be storage volumes), it's better to play it safe
 to avoid races from opening an unexpected file type while also
 avoiding problems with having to handle EAGAIN while read()ing.

 Based on a report by Dan Berrange.

 * src/storage/storage_backend.c
 (virStorageBackendVolOpenCheckMode): Fix up fd after avoiding race.

 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
  src/storage/storage_backend.c | 25 +
  1 file changed, 21 insertions(+), 4 deletions(-)

 
 ACK

Thanks; pushed.

-- 
Eric Blake   eblake 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

[libvirt] [PATCH] look for numad in /usr/sbin

2013-11-26 Thread Jim Fehlig
When looking for numad with AC_PATH_PROG, include /usr/sbin in
the search path.
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 4942e07..f494f46 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1454,7 +1454,7 @@ AC_ARG_WITH([numad],
 if test $with_numad != no ; then
   fail=0
 
-  AC_PATH_PROG([NUMAD], [numad], [], [/bin:/usr/bin])
+  AC_PATH_PROG([NUMAD], [numad], [], [/bin:/usr/bin:/usr/sbin])
 
   if test $with_numad = check; then
 test $with_numactl = yes || fail=1
-- 
1.8.0.1

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


[libvirt] [PATCH] Memory Leak Fix: Check def-info-alias before assigning

2013-11-26 Thread Nehal J Wani
On running the command make -C tests valgrind, there used to be a bunch of
memory leaks shown by valgrind. Specifically, one can check it by running:
libtool --mode=execute valgrind --quiet --leak-check=full 
--suppressions=./.valgrind.supp qemuhotplugtest
The issue was that def-info-alias was already malloc'ed by xmlStrndup in
virDomainDeviceInfoParseXML (domain_conf.c:3439). The new alias was being
assigned again without freeing the old one in qemuAssignDeviceAliases().
This patch checks if the entity exists, and frees accordingly, hence making
valgrind cry lesser.

---
 src/qemu/qemu_command.c | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 763417f..bbec1d4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -979,6 +979,8 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr 
qemuCaps)
 size_t i;
 
 for (i = 0; i  def-ndisks; i++) {
+if (def-disks[i]-info.alias)
+VIR_FREE(def-disks[i]-info.alias);
 if (qemuAssignDeviceDiskAlias(def, def-disks[i], qemuCaps)  0)
 return -1;
 }
@@ -988,6 +990,9 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr 
qemuCaps)
 /* type='hostdev' interfaces are also on the hostdevs list,
  * and will have their alias assigned with other hostdevs.
  */
+if (def-nets[i]-info.alias)
+VIR_FREE(def-nets[i]-info.alias);
+
 if (virDomainNetGetActualType(def-nets[i])
 != VIR_DOMAIN_NET_TYPE_HOSTDEV 
 qemuAssignDeviceNetAlias(def, def-nets[i], i)  0) {
@@ -1000,70 +1005,104 @@ qemuAssignDeviceAliases(virDomainDefPtr def, 
virQEMUCapsPtr qemuCaps)
 return 0;
 
 for (i = 0; i  def-nfss; i++) {
+if (def-fss[i]-info.alias)
+VIR_FREE(def-fss[i]-info.alias);
 if (virAsprintf(def-fss[i]-info.alias, fs%zu, i)  0)
 return -1;
 }
 for (i = 0; i  def-nsounds; i++) {
+if (def-sounds[i]-info.alias)
+VIR_FREE(def-sounds[i]-info.alias);
 if (virAsprintf(def-sounds[i]-info.alias, sound%zu, i)  0)
 return -1;
 }
 for (i = 0; i  def-nhostdevs; i++) {
+if (def-hostdevs[i]-info-alias)
+VIR_FREE(def-hostdevs[i]-info-alias);
 if (qemuAssignDeviceHostdevAlias(def, def-hostdevs[i], i)  0)
 return -1;
 }
 for (i = 0; i  def-nredirdevs; i++) {
+if (def-redirdevs[i]-info.alias)
+VIR_FREE(def-redirdevs[i]-info.alias);
 if (qemuAssignDeviceRedirdevAlias(def, def-redirdevs[i], i)  0)
 return -1;
 }
 for (i = 0; i  def-nvideos; i++) {
+if (def-videos[i]-info.alias)
+VIR_FREE(def-videos[i]-info.alias);
 if (virAsprintf(def-videos[i]-info.alias, video%zu, i)  0)
 return -1;
 }
 for (i = 0; i  def-ncontrollers; i++) {
+if (def-controllers[i]-info.alias)
+VIR_FREE(def-controllers[i]-info.alias);
 if (qemuAssignDeviceControllerAlias(def-controllers[i])  0)
 return -1;
 }
 for (i = 0; i  def-ninputs; i++) {
+if (def-inputs[i]-info.alias)
+VIR_FREE(def-inputs[i]-info.alias);
 if (virAsprintf(def-inputs[i]-info.alias, input%zu, i)  0)
 return -1;
 }
 for (i = 0; i  def-nparallels; i++) {
+if (def-parallels[i]-info.alias)
+VIR_FREE(def-parallels[i]-info.alias);
 if (qemuAssignDeviceChrAlias(def, def-parallels[i], i)  0)
 return -1;
 }
 for (i = 0; i  def-nserials; i++) {
+if (def-serials[i]-info.alias)
+VIR_FREE(def-serials[i]-info.alias);
 if (qemuAssignDeviceChrAlias(def, def-serials[i], i)  0)
 return -1;
 }
 for (i = 0; i  def-nchannels; i++) {
+if (def-channels[i]-info.alias)
+VIR_FREE(def-channels[i]-info.alias);
 if (qemuAssignDeviceChrAlias(def, def-channels[i], i)  0)
 return -1;
 }
 for (i = 0; i  def-nconsoles; i++) {
+if (def-consoles[i]-info.alias)
+VIR_FREE(def-consoles[i]-info.alias);
 if (qemuAssignDeviceChrAlias(def, def-consoles[i], i)  0)
 return -1;
 }
 for (i = 0; i  def-nhubs; i++) {
+if (def-hubs[i]-info.alias)
+VIR_FREE(def-hubs[i]-info.alias);
 if (virAsprintf(def-hubs[i]-info.alias, hub%zu, i)  0)
 return -1;
 }
 for (i = 0; i  def-nsmartcards; i++) {
+if (def-smartcards[i]-info.alias)
+VIR_FREE(def-smartcards[i]-info.alias);
 if (virAsprintf(def-smartcards[i]-info.alias, smartcard%zu, i)  
0)
 return -1;
 }
 if (def-watchdog) {
+if (def-watchdog-info.alias)
+VIR_FREE(def-watchdog-info.alias);
 if (virAsprintf(def-watchdog-info.alias, watchdog%d, 0)  0)
  

[libvirt] virDomain{Attach, Detach, Update}DeviceFlags filesystem support for qemu driver

2013-11-26 Thread Jesse Cook
It does not currently appear to be possible to attach/detach a filesystem
with kvm through the C API or virsh. Looking at
src/qemu/qemu_driver.c:qemuDomain{Attach,Detach,Update}Config,
VIR_DOMAIN_DEVICE_FS is not a case within the switch statement for
dev-type.

Is support for this planned for the near future or in progress? If not, how
much would have to be implemented outside these functions to add support
for attaching, detaching, and modifying filesystems? I would like to gauge
whether I should carve out time to add support, or work around it by using
disks in place of filesystems. Thanks!

--
Jesse J. Cook
Camber Corporation
Lead Software Developer
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [test-API][PATCH V2] Add blockjob related test functions and cases

2013-11-26 Thread Jincheng Miao
  * repos/domain/blkstatsflags.py
  * repos/domain/block_iotune.py
  * repos/domain/block_peek.py
  * repos/domain/block_resize.py
  * repos/domain/domain_blkio.py
  * cases/basic_blockjob.conf

  V1-V2:
  Removed diskpath params for block_resize, this hard code disk device is
  not convenient for users.

---
 cases/basic_blockjob.conf |  83 +
 repos/domain/blkstatsflags.py |  63 
 repos/domain/block_iotune.py  | 118 ++
 repos/domain/block_peek.py|  69 ++
 repos/domain/block_resize.py  |  94 
 repos/domain/domain_blkio.py  | 163 ++
 6 files changed, 590 insertions(+)
 create mode 100644 cases/basic_blockjob.conf
 create mode 100644 repos/domain/blkstatsflags.py
 create mode 100644 repos/domain/block_iotune.py
 create mode 100644 repos/domain/block_peek.py
 create mode 100644 repos/domain/block_resize.py
 create mode 100644 repos/domain/domain_blkio.py

diff --git a/cases/basic_blockjob.conf b/cases/basic_blockjob.conf
new file mode 100644
index 000..21332dc
--- /dev/null
+++ b/cases/basic_blockjob.conf
@@ -0,0 +1,83 @@
+domain:install_linux_cdrom
+guestname
+$defaultname
+guestos
+$defaultos
+guestarch
+$defaultarch
+vcpu
+$defaultvcpu
+memory
+$defaultmem
+hddriver
+$defaulthd
+nicdriver
+$defaultnic
+macaddr
+54:52:00:45:c3:8a
+
+domain:install_linux_check
+guestname
+$defaultname
+virt_type
+$defaulthv
+hddriver
+$defaulthd
+nicdriver
+$defaultnic
+
+domain:block_iotune
+guestname
+$defaultname
+bytes_sec
+10
+iops_sec
+0
+
+domain:block_iotune
+guestname
+$defaultname
+bytes_sec
+0
+iops_sec
+1000
+
+domain:block_peek
+guestname
+$defaultname
+
+domain:block_peek
+guestname
+$defaultname
+
+domain:block_resize
+guestname
+$defaultname
+disksize
+1G
+
+domain:blkstats
+guestname
+$defaultname
+
+domain:blkstatsflags
+guestname
+$defaultname
+flags
+0
+
+domain:domain_blkinfo
+guestname
+$defaultname
+
+domain:domain_blkio
+guestname
+$defaultname
+weight
+500
+
+domain:undefine
+guestname
+$defaultname
+
+options cleanup=enable
diff --git a/repos/domain/blkstatsflags.py b/repos/domain/blkstatsflags.py
new file mode 100644
index 000..4c84a18
--- /dev/null
+++ b/repos/domain/blkstatsflags.py
@@ -0,0 +1,63 @@
+#!/usr/bin/evn python
+# To test domain block device statistics with flags
+
+import time
+import libxml2
+
+import libvirt
+from libvirt import libvirtError
+
+from src import sharedmod
+
+required_params = ('guestname', 'flags')
+optional_params = {}
+
+def check_guest_status(domobj):
+Check guest current status
+state = domobj.info()[0]
+if state == libvirt.VIR_DOMAIN_SHUTOFF or state == 
libvirt.VIR_DOMAIN_SHUTDOWN:
+# add check function
+return False
+else:
+return True
+
+def check_blkstats():
+Check block device statistic result
+pass
+
+def blkstatsflags(params):
+Domain block device statistic
+logger = params['logger']
+guestname = params['guestname']
+flags = int(params['flags'])
+
+conn = sharedmod.libvirtobj['conn']
+
+domobj = conn.lookupByName(guestname)
+
+# Check domain block status
+if check_guest_status(domobj):
+pass
+else:
+domobj.create()
+time.sleep(90)
+try:
+xml = domobj.XMLDesc(0)
+doc = libxml2.parseDoc(xml)
+cont = doc.xpathNewContext()
+devs = cont.xpathEval(/domain/devices/disk/target/@dev)
+
+for dev in devs:
+path = dev.content
+blkstats = domobj.blockStatsFlags(path, flags)
+# check_blkstats()
+logger.debug(blkstats)
+for entry in blkstats.keys():
+logger.info(%s %s %s %(path, entry, blkstats[entry]))
+
+except libvirtError, e:
+logger.error(API error message: %s, error code is %s
+ % (e.message, e.get_error_code()))
+return 1
+
+return 0
diff --git a/repos/domain/block_iotune.py b/repos/domain/block_iotune.py
new file mode 100644
index 000..26e4823
--- /dev/null
+++ b/repos/domain/block_iotune.py
@@ -0,0 +1,118 @@
+#!/usr/bin/evn python
+# To test domain block device iotune
+
+import time
+import libxml2
+import libvirt
+from libvirt import libvirtError
+
+from src import sharedmod
+
+required_params = ('guestname', 'bytes_sec', 'iops_sec')
+optional_params = {}
+
+def check_guest_status(domobj):
+Check guest current status
+state = domobj.info()[0]
+if state == libvirt.VIR_DOMAIN_SHUTOFF or \
+state == libvirt.VIR_DOMAIN_SHUTDOWN:
+# add check function
+

[libvirt] [test-API][PATCH V2] Modify previous test cases by invoking api

2013-11-26 Thread Jincheng Miao
  * repos/domain/blkstats.py
  * repos/domain/domain_blkinfo.py
  Function blkstats and domain_blkinfo used virsh commands to test result,
  that is not fit for testing api, so I replace commands with invoking api.

  V1-V2:
  Removed blockdev params for domain_blkinfo, this hard code disk device is
  not convenient for users.
---
 repos/domain/blkstats.py   |  2 -
 repos/domain/domain_blkinfo.py | 94 --
 2 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/repos/domain/blkstats.py b/repos/domain/blkstats.py
index 0254922..27c2a46 100644
--- a/repos/domain/blkstats.py
+++ b/repos/domain/blkstats.py
@@ -1,8 +1,6 @@
 #!/usr/bin/evn python
 # To test domain block device statistics
 
-import os
-import sys
 import time
 import libxml2
 
diff --git a/repos/domain/domain_blkinfo.py b/repos/domain/domain_blkinfo.py
index b6051aa..81035ed 100644
--- a/repos/domain/domain_blkinfo.py
+++ b/repos/domain/domain_blkinfo.py
@@ -1,22 +1,17 @@
 #!/usr/bin/env python
-# To test virsh domblkinfo command
+# To test domain's blockkinfo API
 
-import os
-import sys
-import re
 import commands
-
+import libxml2
 import libvirt
 from libvirt import libvirtError
 
 from src import sharedmod
 
-GET_DOMBLKINFO_MAC = virsh domblkinfo %s %s | awk '{print $2}'
 GET_CAPACITY = du -b %s | awk '{print $1}'
 GET_PHYSICAL_K =  du -B K %s | awk '{print $1}'
-VIRSH_DOMBLKINFO = virsh domblkinfo %s %s
 
-required_params = ('guestname', 'blockdev',)
+required_params = ('guestname', )
 optional_params = {}
 
 def get_output(command, logger):
@@ -32,8 +27,8 @@ def check_domain_exists(conn, guestname, logger):
  check if the domain exists, may or may not be active 
 guest_names = []
 ids = conn.listDomainsID()
-for id in ids:
-obj = conn.lookupByID(id)
+for domain_id in ids:
+obj = conn.lookupByID(domain_id)
 guest_names.append(obj.name())
 
 guest_names += conn.listDefinedDomains()
@@ -44,17 +39,27 @@ def check_domain_exists(conn, guestname, logger):
 else:
 return True
 
+def check_guest_status(domobj):
+Check guest current status
+state = domobj.info()[0]
+if state == libvirt.VIR_DOMAIN_SHUTOFF or \
+state == libvirt.VIR_DOMAIN_SHUTDOWN:
+# add check function
+return False
+else:
+return True
+
 def check_block_data(blockdev, blkdata, logger):
  check data about capacity,allocation,physical 
 status, apparent_size = get_output(GET_CAPACITY % blockdev, logger)
 if not status:
-if apparent_size == blkdata[0]:
-logger.info(the capacity of '%s' is %s, checking succeeded % \
-(blockdev, apparent_size))
+if apparent_size == str(blkdata[0]):
+logger.info(the capacity of '%s' is %s, checking succeeded
+% (blockdev, apparent_size))
 else:
-logger.error(apparent-size from 'du' is %s, \n\
- but from 'domblkinfo' is %s, checking failed % \
-(apparent_size, blkdata[0]))
+logger.error(apparent-size from 'du' is %s % apparent_size)
+logger.error(but from 'domain blockinfo' is %d, checking failed
+ % blkdata[0])
 return 1
 else:
 return 1
@@ -64,14 +69,15 @@ def check_block_data(blockdev, blkdata, logger):
 block_size_b = int(block_size_k[:-1]) * 1024
 # Temporarily, we only test the default case, assuming
 # Allocation value is equal to Physical value
-if str(block_size_b) == blkdata[1] and str(block_size_b) == blkdata[2]:
-logger.info(the block size of '%s' is %s, same with \n\
-Allocation and Physical value, checking succeeded % \
- (blockdev, block_size_b))
+if block_size_b == blkdata[1] and block_size_b == blkdata[2]:
+logger.info(the block size of '%s' is %s
+% (blockdev, block_size_b))
+logger.info(Allocation and Physical value's checking succeeded)
 else:
-logger.error(the block size from 'du' is %s, \n\
-  the Allocation value is %s, Physical value is %s, \n\
-  checking failed % (block_size_b, blkdata[1], 
blkdata[2]))
+logger.error(the block size from 'du' is %d % block_size_b)
+logger.error(the Allocation value is %d, Physical value is %d
+ % (blkdata[1], blkdata[2]))
+logger.error(checking failed)
 return 1
 
 return 0
@@ -79,14 +85,12 @@ def check_block_data(blockdev, blkdata, logger):
 
 def domain_blkinfo(params):
  using du command to check the data
-in the output of virsh domblkinfo
+in the output of API blockinfo
 
 logger = params['logger']
 guestname = params.get('guestname')
-blockdev = params.get('blockdev')
 
 

Re: [libvirt] [PATCH] look for numad in /usr/sbin

2013-11-26 Thread Eric Blake
On 11/26/2013 03:00 PM, Jim Fehlig wrote:
 When looking for numad with AC_PATH_PROG, include /usr/sbin in
 the search path.
 ---
  configure.ac | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

ACK.

 
 diff --git a/configure.ac b/configure.ac
 index 4942e07..f494f46 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -1454,7 +1454,7 @@ AC_ARG_WITH([numad],
  if test $with_numad != no ; then
fail=0
  
 -  AC_PATH_PROG([NUMAD], [numad], [], [/bin:/usr/bin])
 +  AC_PATH_PROG([NUMAD], [numad], [], [/bin:/usr/bin:/usr/sbin])
  
if test $with_numad = check; then
  test $with_numactl = yes || fail=1
 

-- 
Eric Blake   eblake 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

[libvirt] [PATCH] storage: skip selinux cleanup when fd not available

2013-11-26 Thread Eric Blake
When attempting to backport gluster pools to an older version
where there is no VIR_STRDUP, I got a crash from calling
strdup(,NULL).  Rather than relying on the current else branch
safely doing nothing when there is no fd, it is easier to just
skip it. While at it, there's no need to explicitly set
perms.label to NULL after a VIR_FREE().

* src/storage/storage_backend.c
(virStorageBackendUpdateVolTargetInfoFD): Minor optimization.

Signed-off-by: Eric Blake ebl...@redhat.com
---

 src/storage/storage_backend.c | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index bde39d6..b08d646 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1383,28 +1383,26 @@ 
virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target,

 VIR_FREE(target-perms.label);

 #if WITH_SELINUX
 /* XXX: make this a security driver call */
-if (fd = 0  fgetfilecon_raw(fd, filecon) == -1) {
-if (errno != ENODATA  errno != ENOTSUP) {
-virReportSystemError(errno,
- _(cannot get file context of '%s'),
- target-path);
-return -1;
+if (fd = 0) {
+if (fgetfilecon_raw(fd, filecon) == -1) {
+if (errno != ENODATA  errno != ENOTSUP) {
+virReportSystemError(errno,
+ _(cannot get file context of '%s'),
+ target-path);
+return -1;
+}
 } else {
-target-perms.label = NULL;
-}
-} else {
-if (VIR_STRDUP(target-perms.label, filecon)  0) {
+if (VIR_STRDUP(target-perms.label, filecon)  0) {
+freecon(filecon);
+return -1;
+}
 freecon(filecon);
-return -1;
 }
-freecon(filecon);
 }
-#else
-target-perms.label = NULL;
 #endif

 return 0;
 }

-- 
1.8.3.1

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


Re: [libvirt] [PATCH] look for numad in /usr/sbin

2013-11-26 Thread Jim Fehlig
Eric Blake wrote:
 On 11/26/2013 03:00 PM, Jim Fehlig wrote:
   
 When looking for numad with AC_PATH_PROG, include /usr/sbin in
 the search path.
 ---
  configure.ac | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 

 ACK.
   

Thanks, pushed.

Regards,
Jim

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


Re: [libvirt] [libvirt-test-API][PATCH] Fix utils.exec_cmd output problem

2013-11-26 Thread Guannan Ren

On 2013年11月26日 17:17, Jincheng Miao wrote:

Thanks for review,
yes, I missed this situation: stdout is not the subprocess.PIPE.

Since the stderr is always subprocess.PIPE, my another way is err after 
Popen.communicate().

The patch looks like:
---
  utils/utils.py | 2 ++---
  1 file changed, 2 insertions(+)

diff --git a/utils/utils.py b/utils/utils.py
index 147c1ef..d107cbd 100644
--- a/utils/utils.py
+++ b/utils/utils.py
@@ -412,6 +412,8 @@ def exec_cmd(command, sudo=False, cwd=None, infile=None, 
outfile=None, shell=Fal
  p = subprocess.Popen(command, shell=shell, close_fds=True, cwd=cwd,
  stdin=infile, stdout=outfile, stderr=subprocess.PIPE)
  (out, err) = p.communicate(data)
  if out == None:
  # Prevent splitlines() from barfing later on
  out = 
+if err != :
+out += err
  return (p.returncode, out.splitlines())
  
  def remote_exec_pexpect(hostname, username, password, cmd):


Why to append standard error to standard output. It is not right in 
semantics.
In order to get the standard error if executing command failed,  the 
following change is enough:


 if out == None:
 # Prevent splitlines() from barfing later on
 out = 
+if p.returncode:
+out = err
 return (p.returncode, out.splitlines())


Guannan

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

Re: [libvirt] [libvirt-test-API][PATCH] Fix utils.exec_cmd output problem

2013-11-26 Thread Jincheng Miao
- Original Message -
 Why to append standard error to standard output. It is not right in
 semantics.

I think test-api should replace all commands modules with utils.exec_cmd.
For commands modules, it merged stderr with stdout:

def getstatusoutput(cmd):
Return (status, output) of executing cmd in a shell.
import os
pipe = os.popen('{ ' + cmd + '; } 21', 'r')

So for compatible, utils.exec_cmd should do the same thing.

 In order to get the standard error if executing command failed,  the
 following change is enough:
 
   if out == None:
   # Prevent splitlines() from barfing later on
   out = 
 +if p.returncode:
 +out = err
   return (p.returncode, out.splitlines())
 
 
 Guannan
 

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


Re: [libvirt] [PATCH] Memory Leak Fix: Check def-info-alias before assigning

2013-11-26 Thread Osier Yang

On 27/11/13 06:31, Nehal J Wani wrote:

On running the command make -C tests valgrind, there used to be a bunch of
memory leaks shown by valgrind. Specifically, one can check it by running:
libtool --mode=execute valgrind --quiet --leak-check=full 
--suppressions=./.valgrind.supp qemuhotplugtest
The issue was that def-info-alias was already malloc'ed by xmlStrndup in
virDomainDeviceInfoParseXML (domain_conf.c:3439). The new alias was being
assigned again without freeing the old one in qemuAssignDeviceAliases().
This patch checks if the entity exists, and frees accordingly, hence making
valgrind cry lesser.

---
  src/qemu/qemu_command.c | 39 +++
  1 file changed, 39 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 763417f..bbec1d4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -979,6 +979,8 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr 
qemuCaps)
  size_t i;
  
  for (i = 0; i  def-ndisks; i++) {

+if (def-disks[i]-info.alias)
+VIR_FREE(def-disks[i]-info.alias);


Instead of free'ing it, it should be used to avoid calculate/strdup the
alias again.  Since the alias from virDomainDeviceInfoParseXML only
works for active domain's XML:

snip
if (alias == NULL 
!(flags  VIR_DOMAIN_XML_INACTIVE) 
xmlStrEqual(cur-name, BAD_CAST alias)) {
alias = cur;
/snip

And I don't think using the alias existing in the XML could cause any
conflicts for an active domain.

Regards,
Osier

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


Re: [libvirt] [libvirt-test-API][PATCH] Fix utils.exec_cmd output problem

2013-11-26 Thread Guannan Ren

On 2013年11月27日 14:11, Jincheng Miao wrote:

- Original Message -

Why to append standard error to standard output. It is not right in
semantics.

I think test-api should replace all commands modules with utils.exec_cmd.
For commands modules, it merged stderr with stdout:

def getstatusoutput(cmd):
 Return (status, output) of executing cmd in a shell.
 import os
 pipe = os.popen('{ ' + cmd + '; } 21', 'r')

So for compatible, utils.exec_cmd should do the same thing.



Good idea, I will do the work soon.

Guannan

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

Re: [libvirt] [PATCH] storage: skip selinux cleanup when fd not available

2013-11-26 Thread Osier Yang

On 27/11/13 12:00, Eric Blake wrote:

When attempting to backport gluster pools to an older version
where there is no VIR_STRDUP, I got a crash from calling
strdup(,NULL).  Rather than relying on the current else branch
safely doing nothing when there is no fd, it is easier to just
skip it. While at it, there's no need to explicitly set
perms.label to NULL after a VIR_FREE().

* src/storage/storage_backend.c
(virStorageBackendUpdateVolTargetInfoFD): Minor optimization.

Signed-off-by: Eric Blake ebl...@redhat.com
---

  src/storage/storage_backend.c | 26 --
  1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index bde39d6..b08d646 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1383,28 +1383,26 @@ 
virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target,

  VIR_FREE(target-perms.label);

  #if WITH_SELINUX
  /* XXX: make this a security driver call */
-if (fd = 0  fgetfilecon_raw(fd, filecon) == -1) {
-if (errno != ENODATA  errno != ENOTSUP) {
-virReportSystemError(errno,
- _(cannot get file context of '%s'),
- target-path);
-return -1;
+if (fd = 0) {
+if (fgetfilecon_raw(fd, filecon) == -1) {
+if (errno != ENODATA  errno != ENOTSUP) {
+virReportSystemError(errno,
+ _(cannot get file context of '%s'),
+ target-path);
+return -1;
+}
  } else {
-target-perms.label = NULL;
-}
-} else {
-if (VIR_STRDUP(target-perms.label, filecon)  0) {
+if (VIR_STRDUP(target-perms.label, filecon)  0) {
+freecon(filecon);
+return -1;
+}
  freecon(filecon);
-return -1;
  }
-freecon(filecon);
  }
-#else
-target-perms.label = NULL;
  #endif

  return 0;
  }



ACK

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


[libvirt] [PATCH] libvirt-perl: Fix the wrong binding of virNodeDeviceLookupSCSIHostByWWN

2013-11-26 Thread Osier Yang
The second parameter for virNodeDeviceLookupSCSIHostByWWN is wwnn
instead of wwpn, and the API supports flags.
---
 AUTHORS| 1 +
 lib/Sys/Virt/NodeDevice.pm | 5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index d53f191..c2af49a 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -12,5 +12,6 @@ Patches contributed by:
Stepan Kasal  skasal-at-redhat-dot-com
Ludwig Nussel ludwig-dot-nussel-at-suse-dot-de
Zhe Peng  zpeng-at-redhat-dot-com
+   Osier Yangjyang-at-redhat-dot-com
 
 -- End
diff --git a/lib/Sys/Virt/NodeDevice.pm b/lib/Sys/Virt/NodeDevice.pm
index 984910d..29ceac7 100644
--- a/lib/Sys/Virt/NodeDevice.pm
+++ b/lib/Sys/Virt/NodeDevice.pm
@@ -51,10 +51,11 @@ sub _new {
 my $self;
 if (exists $params{name}) {
$self = Sys::Virt::NodeDevice::_lookup_by_name($con,  $params{name});
-} elsif (exists $params{wwpn}) {
+} elsif (exists $params{wwnn}) {
$self = Sys::Virt::NodeDevice::_lookup_scsihost_by_wwn($con,
+  $params{wwnn},
   $params{wwpn},
-  $params{wwnn});
+   $params{flags});
 } elsif (exists $params{xml}) {
$self = Sys::Virt::NodeDevice::_create_xml($con, $params{xml});
 } else {
-- 
1.8.1.4

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


Re: [libvirt] [PATCHv1.5 02/27] test: Implement fake storage pool driver in qemuxml2argv test

2013-11-26 Thread Osier Yang

On 27/11/13 00:48, Peter Krempa wrote:

To support testing of volume disk backing, we need to implement a few
disk driver backend functions.

The fake storage driver uses files in storagepoolxml2xmlout/POOLNAME.xml
as XML files for pool definitions and volume names are in format
VOL_TYPE+VOL_PATH. By default type block is assumed (for iSCSI test
compatibility).

The choice of this approach along with implemented functions was made so
that disk type='volume' can be tested in the xml2argv test.
---
  tests/qemuxml2argvtest.c | 162 +++
  1 file changed, 162 insertions(+)

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a290062..a4cef84 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -18,6 +18,7 @@
  # include qemu/qemu_command.h
  # include qemu/qemu_domain.h
  # include datatypes.h
+# include conf/storage_conf.h
  # include cpu/cpu_map.h
  # include virstring.h

@@ -75,6 +76,161 @@ static virSecretDriver fakeSecretDriver = {
  .secretUndefine = NULL,
  };

+
+# define STORAGE_POOL_XML_PATH storagepoolxml2xmlout/


This will cause build failure when building with VPATH.


+static const unsigned char fakeUUID[VIR_UUID_BUFLEN] = fakeuuid;
+
+static virStoragePoolPtr
+fakeStoragePoolLookupByName(virConnectPtr conn,
+const char *name)
+{
+char *xmlpath = NULL;
+virStoragePoolPtr ret = NULL;
+
+if (STRNEQ(name, inactive)) {
+if (virAsprintf(xmlpath, %s%s.xml,
+STORAGE_POOL_XML_PATH,
+name)  0)
+return NULL;
+
+if (!virFileExists(xmlpath)) {
+virReportError(VIR_ERR_NO_STORAGE_POOL,
+   File '%s' not found, xmlpath);
+goto cleanup;
+}
+}
+
+ret = virGetStoragePool(conn, name, fakeUUID, NULL, NULL);


Looks like fakeUUID is only used here, so generating a random UUID
might work.


+
+cleanup:
+VIR_FREE(xmlpath);
+return ret;
+}
+
+
+static virStorageVolPtr
+fakeStorageVolLookupByName(virStoragePoolPtr pool,
+   const char *name)
+{
+char **volinfo = NULL;
+virStorageVolPtr ret = NULL;
+
+if (STREQ(pool-name, inactive)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   storage pool '%s' is not active, pool-name);
+return NULL;
+}
+
+if (STREQ(name, nonexistent)) {


It will be better if it has document what these magic strings mean.


+virReportError(VIR_ERR_NO_STORAGE_VOL,
+   no storage vol with matching name '%s', name);
+return NULL;
+}
+
+if (!strchr(name, '+'))
+goto fallback;
+
+if (!(volinfo = virStringSplit(name, +, 2)))
+return NULL;
+
+if (!volinfo[1])
+goto fallback;
+
+ret = virGetStorageVol(pool-conn, pool-name, volinfo[1], volinfo[0],
+   NULL, NULL);
+
+cleanup:
+virStringFreeList(volinfo);
+return ret;
+
+fallback:
+ret = virGetStorageVol(pool-conn, pool-name, name, block, NULL, NULL);
+goto cleanup;
+}
+
+static int
+fakeStorageVolGetInfo(virStorageVolPtr vol,
+  virStorageVolInfoPtr info)
+{
+memset(info, 0, sizeof(*info));
+
+info-type = virStorageVolTypeFromString(vol-key);
+
+if (info-type  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   Invalid volume type '%s', vol-key);
+return -1;
+}
+
+return 0;
+}
+
+
+static char *
+fakeStorageVolGetPath(virStorageVolPtr vol)
+{
+char *ret = NULL;
+
+ignore_value(virAsprintf(ret, /some/%s/device/%s, vol-key, vol-name));
+
+return ret;
+}
+
+
+static char *
+fakeStoragePoolDumpXML(virStoragePoolPtr pool,


Better to rename it as fakeStoragePoolGetXMLDesc to keep consistent.
DumpXML is used in virsh.

Osier

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