Re: [libvirt] [PATCH] nodeinfo: Don't fail on non-contiguous NUMA topologies

2013-07-12 Thread Peter Krempa

On 07/11/13 18:08, Daniel P. Berrange wrote:

On Thu, Jul 11, 2013 at 04:09:47PM +0200, Peter Krempa wrote:

From: hejia hejia jiaker...@gmail.com

nodeGetFreeMemory and nodeGetCellsFreeMemory assumed that the NUMA nodes
are contiguous and starting from 0. Unfortunately there are machines
that don't match this assumption:

available: 1 nodes (1)
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
node 1 size: 16340 MB
node 1 free: 11065 MB

Before this patch:
error: internal error Failed to query NUMA free memory
error: internal error Failed to query NUMA free memory for node: 0

After this patch:
Total: 15772580 KiB
0: 0 KiB

Signed-off-by: Peter Krempa pkre...@redhat.com
---
  src/nodeinfo.c | 18 ++
  1 file changed, 6 insertions(+), 12 deletions(-)


...



ACK, looking at the libnuma code, the only reasons why numa_node_size64
would return -1, is if the NUMA node does not exist, or the sysfs file
was not parsable as an integer. The latter is basically not going to
happen, so it is reasonable to skip this error reporting unconditionally.

Daniel



Pushed; Thanks.

Peter

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


Re: [libvirt] ANNOUNCE: libvirt 1.0.5.3 maintenance release

2013-07-12 Thread Ján Tomko
On 07/12/2013 12:32 AM, Cole Robinson wrote:
 libvirt 1.0.5.3 maintenance release is now available. This is
 libvirt 1.0.5 with additional bugfixes that have accumulated
 upstream since the initial release.
 
 This release can be downloaded at:
 
 http://libvirt.org/sources/stable_updates/libvirt-1.0.5.3.tar.gz
 
 Changes in this version:
 
...
 * qemu: Avoid leaking uri in qemuMigrationPrepareDirect

Ouch, this one contains a double free - migration with migrateuri will crash.

I didn't realize I should've pushed
commit 5744d96f211160d406ec0498c2f814a67d1a3fc8
qemu: fix double free in qemuMigrationPrepareDirect

to v1.0.5-maint as well.

Sorry for the inconvenience.

Jan

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


Re: [libvirt] [PATCH 1/3] virAuth: Don't require virConnectPtr to retrieve authentication creds

2013-07-12 Thread Peter Krempa

On 07/11/13 18:10, Daniel P. Berrange wrote:

On Wed, Jul 10, 2013 at 08:42:03AM +0200, Peter Krempa wrote:

Previously a connection object was required to retrieve the auth
credentials. This patch adds the option to call the retrieval functions
only using the connection URI or path to the configuration file. This
will allow to use this toolkit to request passwords for ssh
authentication in the libssh2 connection driver.

Changes:
*virAuthGetConfigFilePathURI(): use URI to retrieve the config file path
*virAuthGetCredential(): Remove the need to propagate conn object

virAuthGetPasswordPath():
*virAuthGetUsernamePath(): New functions, that use config file path
instead of conn object
---
  src/util/virauth.c | 107 +
  src/util/virauth.h |  17 -
  2 files changed, 91 insertions(+), 33 deletions(-)


ACK

I won't force you to write a test case for this, since we don't already
have a test virauth.h file APIs. If you should wish to write one anyway
though.


I will put that on my to-do list. I actually was thinking about testing 
this while writing the code.




Daniel



Series pushed, thanks for the review.

Peter

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


Re: [libvirt] [PATCH] storage: silently ignore missing files on pool refresh

2013-07-12 Thread Ján Tomko
On 07/10/2013 06:07 PM, Daniel P. Berrange wrote:
 On Wed, Jul 10, 2013 at 05:00:59PM +0200, Michal Privoznik wrote:
 On 10.07.2013 16:57, Ján Tomko wrote:
 From: Wei Zhou w.z...@leaseweb.com

 Make virStorageBackendVolOpenCheckMode return -2 instead of
 -1 if volume file is missing.

 https://bugzilla.redhat.com/show_bug.cgi?id=977706
 ---
  src/storage/storage_backend.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
 index e2527c9..f063601 100644
 --- a/src/storage/storage_backend.c
 +++ b/src/storage/storage_backend.c
 @@ -1084,7 +1084,7 @@ virStorageBackendForType(int type)
   * Allows caller to silently ignore files with improper mode
   *
   * Returns -1 on error, -2 if file mode is unexpected or the
 - * volume is a dangling symbolic link.
 + * volume is a dangling symbolic link or file is missing.
   */
  int
  virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags)
 @@ -1097,7 +1097,7 @@ virStorageBackendVolOpenCheckMode(const char *path, 
 unsigned int flags)
  virReportSystemError(errno,
   _(cannot stat file '%s'),
   path);

 If we want to *silently* ignore missing file, why do we
 virReportSystemError() here?

 -return -1;
 +return -2;
  }
 
 Well returning -1 vs -2 from this function isn't ignoring the
 error. It is just providing the caller a way to detect a specific
 error scenario. Thus if the caller decides to ignore the error
 when ret == -2, then the caller should call virResetLastError()
 to clear it.

Does this imply a NACK to v2 [1], where I changed it to VIR_WARN to match the
other cases of returning -2?

Resetting the error won't unlog it. On the other hand, there are paths (like
in virStorageBackendFileSystemVolRefresh) where the return value of '-2' would
get propagated without setting an error (which is pre-existing for the other
cases, but could be a problem in this case).

Jan

[1] https://www.redhat.com/archives/libvir-list/2013-July/msg00639.html

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

Re: [libvirt] invoking qemu with -readconfig

2013-07-12 Thread Daniel P. Berrange
On Thu, Jul 11, 2013 at 12:56:31PM -0600, Jim Fehlig wrote:
 Hi All,
 
 I was recently asked about the possibility of libvirt creating a config
 file and then invoking qemu with '-readconfig file' instead of passing
 all arguments on the command line.  The primary reason for this request
 is a more readable ps output and the ability to inspect the arguments in
 a file (with one arg per line) instead of one long line.

Note: you can already inspect all the arguments in a file

 /var/log/libvirt/qemu/$GUESTNAME.log

always contains the args

 I was certain there was a similar request in the past, but haven't had
 much luck searching the mail archives or with google.  Has this been
 discussed previously?  If so, any pointers or summary of the outcome? 
 If not, what are folks' opinion about this?

I guess my answer depends on how much work it would involve ? If we could
use all our existing command line generator code  simply write the result
to a file, that'd be reasonable. If we had to change all our code to write
stuff in a different format, I'd be less enthusiastic.

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] [java] Build Sources with plain Maven

2013-07-12 Thread Daniel P. Berrange
On Thu, Jul 11, 2013 at 11:37:19PM +0200, philipp.hausslei...@yahoo.de wrote:
 Hi all,
 
 i recently tried to use the libvirt-java Bindings. Since the Build with 
 autobuild / ant / maven seems a little bit odd to me, i started with porting 
 the Build to plain maven. 
 
 The only hard dependency is a installed libvirt. I currently tested it on 
 MacOS and Linux  (i used maven os switches to link in the necessary JNA Lib 
 Paths).
 Are there any people interested in this Changes? 
 I will also try to contact the last contributors for that repository.
 
 ATM the whole thing is a WIP, but Sources compile correct and all existing 
 tests run without any problems.

I don't have an opinion, since I'm not up2date with java world, but in
general it is hard to answer this kind of question without seeing the
actual code changes. So please send your proposed changes to this list
and interested parties can review it.

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


[libvirt] [PATCH] virsh: Mention --driver in man page for nodedev-detach

2013-07-12 Thread Peter Krempa
Commit d923f6c8 introduced the --driver option but didn't document it in
the man page. The docs are borrowed from the public API documentation.
---
 tools/virsh.pod | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 94fe897..51644d9 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2113,7 +2113,7 @@ name or wwn pair in wwnn,wwpn format (only works for 
HBA). Note
 that this makes libvirt quit managing a host device, and may even make
 that device unusable by the rest of the physical host until a reboot.

-=item Bnodedev-detach Inodedev
+=item Bnodedev-detach Inodedev [I--driver backend_driver]

 Detach Inodedev from the host, so that it can safely be used by
 guests via hostdev passthrough.  This is reversed with
@@ -2121,6 +2121,12 @@ Bnodedev-reattach, and is done automatically for 
managed devices.
 For compatibility purposes, this command can also be spelled
 Bnodedev-dettach.

+Different backend drivers expect the device to be bound to different
+dummy devices. For example, QEMU's kvm backend driver (the default)
+expects the device to be bound to pci-stub, but its vfio backend
+driver expects the device to be bound to vfio-pci. The I--driver
+parameter can be used to specify the desired backend driver.
+
 =item Bnodedev-dumpxml Idevice

 Dump a device XML representation for the given node device, including
-- 
1.8.3.2

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


[libvirt] [PATCH] Add a couple of debug statements to LXC driver

2013-07-12 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

When failing to start a container due to inaccessible root
filesystem path, we did not log any meaningful error. Add a
few debug statements to assist diagnosis

Pushed under trivial rule

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/lxc/lxc_container.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index fcd9b74..940233b 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1762,6 +1762,8 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr 
vmDef,
 char *sec_mount_options;
 char *stateDir = NULL;
 
+VIR_DEBUG(Setup pivot root);
+
 if (!(sec_mount_options = 
virSecurityManagerGetMountOptions(securityDriver, vmDef)))
 return -1;
 
@@ -1864,12 +1866,16 @@ static int lxcContainerResolveSymlinks(virDomainDefPtr 
vmDef)
 char *newroot;
 size_t i;
 
+VIR_DEBUG(Resolving symlinks);
+
 for (i = 0; i  vmDef-nfss; i++) {
 virDomainFSDefPtr fs = vmDef-fss[i];
 if (!fs-src)
 continue;
-if (virFileResolveAllLinks(fs-src, newroot)  0)
+if (virFileResolveAllLinks(fs-src, newroot)  0) {
+VIR_DEBUG(Fail to resolve link %s, fs-src);
 return -1;
+}
 
 VIR_DEBUG(Resolved '%s' to %s, fs-src, newroot);
 
-- 
1.8.3.1

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


Re: [libvirt] [PATCH] virsh: Mention --driver in man page for nodedev-detach

2013-07-12 Thread Laine Stump
On 07/12/2013 05:40 AM, Peter Krempa wrote:
 Commit d923f6c8 introduced the --driver option but didn't document it in
 the man page. The docs are borrowed from the public API documentation.
 ---
  tools/virsh.pod | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

ACK. Sorry for the omission.'

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


Re: [libvirt] [PATCH] Fix crash when multiple event callbacks were registered

2013-07-12 Thread Daniel Veillard
On Wed, Jul 10, 2013 at 08:11:14AM -0600, Eric Blake wrote:
 On 07/10/2013 05:02 AM, Daniel P. Berrange wrote:
  On Wed, Jul 10, 2013 at 12:59:48PM +0200, Ján Tomko wrote:
  CVE-2013-2230
  
  This should be in the subject line so it is more visible.
 
 Oh well, it was pushed without the subject line change.  But I noticed
 that DV had added a signed tag to our previous CVE (2013-2218, just
 before 1.1.0), and that is also easily visible if you use 'tig', so I've
 just finished creating lots of other signed tags for CVE fixes over the
 last three years:
 
 CVE-2011-1146   CVE-2012-3411   CVE-2013-0170   CVE-2013-2230
 CVE-2011-1486   CVE-2012-3445   CVE-2013-1962
 CVE-2011-2178   CVE-2012-4423   CVE-2013-2218
 
 Since signed tags can be added after the fact, they are a nice way to
 consistently mark bug fixes, regardless of whether the commit itself was
 aware of a CVE number (for example, some of those tags are on commits
 that were made public long before a CVE was assigned, because no one
 realized the exploit until after the patch was pushed).

  +1 I think at this point it is the best way.

The rationale too, is that sometimes we may commit a fix, and the CVE
about it gets assigned later. With tags we can always add that extra
information. So let's try to be consistent and always use git tags
in the future. Those tags should be PGP signed (as you did :)

  Thanks for doing the history work :-)

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] Notes on configuring Open vSwitch, Linux bridge and Libvirt

2013-07-12 Thread Kashyap Chamarthy
Heya Laine,

Here's some quick notes to associate libvirt guests to Open vSwitch.

Configure Open vSwitch
--

Now that a regular Linux bridge is configured, let's try to configure an
OVS brdige and get IP addresses from that space:

Create an Open vSwitch bridge device called 'ovsbr', and display the
current state of OpenvSwitch database contents:

$ ovs-vsctl add-br ovsbr
$ ovs-vsctl show


Add a virtual ethernet interface called 'veth0' with

$ ip link add name veth0 \
  type veth peer name veth1

Add 'veth0' ethernet device to the Linux bridge 'br0', and enumerate all
bridge devices:

$ brctl addif br0 veth0
$ brctl show

Now, associate virtual ethernet device 'veth1' to the OVS bridge,
and display the current state of OpenvSwitch database contents

$ ovs-vsctl add-port ovsbr veth1
$ ovs-vsctl show


Bring up both the virtual ethernet interfaces 'veth0' and 'veth1'

$ ip link set veth0 up  \
  ip link set veth1 up


Update libvirt guest's bridge source to OVS
---

Install a minimal Fedora guest with Oz (or any other mechanism):

$ wget \
https://github.com/kashyapc/virt-scripts/blob/master/oz/oz-jeos.bash
$ ./oz-jeos f19-min f19

Once install is finished, define the guest XML from the current dir:

   $ virsh define f19-minJul_12_2013-12


Now let's edit libvirt's guest XML file to reflect its bridge source is
OVS bridge:

$ virsh edit f19-min

The contents of the guest XML should reflect something along the below
lines:

$ virsh dumpxml f19-min | grep bridge -A8
interface type='bridge'
  mac address='52:54:00:a6:08:70'/
  source bridge='ovsbr'/
  virtualport type='openvswitch'
parameters interfaceid='ecdff22d-ce80-4ae7-a008-42994415084e'/
  /virtualport
  target dev='vnet2'/
  model type='virtio'/
  alias name='net0'/
  address type='pci' domain='0x' bus='0x00' slot='0x03' 
function='0x0'/
/interface

Start the guest, and check the IP of it:

$ virsh start f19-jeos --console
$ ifconfig eth0


Please note, this is just a simple test, I haven't done any further experiments 
with VLAN
tagging, etc.


Slightly verbose notes:

  http://kashyapc.fedorapeople.org/virt/openvswitch-and-libvirt-kvm.txt


-- 
/kashyap

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


Re: [libvirt] [PATCH] virsh: Mention --driver in man page for nodedev-detach

2013-07-12 Thread Peter Krempa

On 07/12/13 13:57, Laine Stump wrote:

On 07/12/2013 05:40 AM, Peter Krempa wrote:

Commit d923f6c8 introduced the --driver option but didn't document it in
the man page. The docs are borrowed from the public API documentation.
---
  tools/virsh.pod | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)


ACK. Sorry for the omission.'



I've added link to the BZ that complained about this and pushed.

Thanks.

Peter

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


Re: [libvirt] [PATCH v3 02/10] Add qemuMonitorJSONGetObjectProperty() method for QMP qom-get command

2013-07-12 Thread Daniel P. Berrange
On Thu, Jul 11, 2013 at 07:55:52PM -0400, John Ferlan wrote:
 Add a new qemuMonitorJSONGetObjectProperty() method to support invocation
 of the 'qom-get' JSON monitor command with a provided path, property, and
 expected data type return. The qemuMonitorJSONObjectProperty is similar to
 virTypedParameter; however, a future patch will extend it a bit to include
 a void pointer to balloon driver statistic data.
 
 The provided test will execute a qom-get on /machine/i440fx which will
 return a property realized.
 ---
  src/qemu/qemu_monitor_json.c | 86 
 
  src/qemu/qemu_monitor_json.h | 34 ++
  tests/qemumonitorjsontest.c  | 48 +
  3 files changed, 168 insertions(+)

ACK

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

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


Re: [libvirt] [PATCH v3 01/10] Add qemuMonitorJSONGetObjectListPaths() method for QMP qom-list command

2013-07-12 Thread Daniel P. Berrange
On Thu, Jul 11, 2013 at 07:55:51PM -0400, John Ferlan wrote:
 Add a new qemuMonitorJSONGetObjectListPaths() method to support invocation
 of the 'qom-list' JSON monitor command with a provided path.
 
 The returned list of paired data fields of name and type that can
 be used to peruse QOM configuration data and eventually utilize for the
 balloon statistics.
 
 The test does a {execute:qom-list, arguments: { path: /}} which
 returns {return: [{name: machine, type: childcontainer},
 {name: type, type: string}]} resulting in a return of an array
 of 2 elements with [0].name=machine, [0].type=childcontainer.  The [1]
 entry appears to be a header that could be used some day via a command such
 as virsh qemuobject --list to format output.
 ---
  src/qemu/qemu_monitor_json.c | 98 
 
  src/qemu/qemu_monitor_json.h | 16 
  tests/qemumonitorjsontest.c  | 79 +++
  3 files changed, 193 insertions(+)

ACK


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

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


Re: [libvirt] [PATCH v3 03/10] Add qemuMonitorJSONSetObjectProperty() method for QMP qom-set command

2013-07-12 Thread Daniel P. Berrange
On Thu, Jul 11, 2013 at 07:55:53PM -0400, John Ferlan wrote:
 Add a new qemuMonitorJSONSetObjectProperty() method to support invocation
 of the 'qom-set' JSON monitor command with a provided path, property, and
 expected data type to set.
 
 The test code uses the same /machine/i440fx property as the get test and
 attempts to set the realized property to true (which it should be set
 at anyway).
 ---
  src/qemu/qemu_monitor_json.c | 62 
 
  src/qemu/qemu_monitor_json.h |  6 +
  tests/qemumonitorjsontest.c  | 59 +
  3 files changed, 127 insertions(+)

ACK


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

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


Re: [libvirt] [PATCH v3 04/10] Add 'period' for Memballoon statistics gathering capability

2013-07-12 Thread Daniel P. Berrange
On Thu, Jul 11, 2013 at 07:55:54PM -0400, John Ferlan wrote:
 Add a period in seconds to allow/enable statistics gathering from the
 Balloon driver for 'virsh dommemstat domain'.
 ---
  docs/formatdomain.html.in | 10 ++
  docs/schemas/domaincommon.rng |  7 +++
  src/conf/domain_conf.c| 38 +++---
  src/conf/domain_conf.h|  1 +
  4 files changed, 49 insertions(+), 7 deletions(-)

Opps, forgot to mention last time that you need to add a test case
for this new XML - or just add the XML to an existing test data file

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 05/10] Determine whether to start balloon memory stats gathering.

2013-07-12 Thread Daniel P. Berrange
On Thu, Jul 11, 2013 at 07:55:55PM -0400, John Ferlan wrote:
 At vm startup and attach attempt to set the balloon driver statistics
 collection period based on the value found in the domain xml file. This
 is not done at reconnect since it's possible that a collection period
 was set on the live guest and making the set period call would reset to
 whatever value is stored in the config file.
 
 Setting the stats collection period has a side effect of searching through
 the qom-list output for the virtio balloon driver and making sure that it
 has the right properties in order to allow setting of a collection period
 and eventually fetching of statistics.
 
 The walk through the qom-list is expensive and thus the balloonpath will
 be saved in the monitor private structure as well as a flag indicating
 that the initialization has already been attempted (in the event that a
 path is not found, no sense to keep checking).
 
 This processing model conforms to the qom object model model which
 requires setting object properties after device startup. That is, it's
 not possible to pass the period along via the startup code as it won't
 be recognized.
 ---
  src/qemu/qemu_monitor.c  | 130 
 +++
  src/qemu/qemu_monitor.h  |   2 +
  src/qemu/qemu_monitor_json.c |  24 
  src/qemu/qemu_monitor_json.h |   3 +
  src/qemu/qemu_process.c  |  14 -
  5 files changed, 171 insertions(+), 2 deletions(-)
 
 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index 6f9a8fc..a3e250c 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -83,6 +83,10 @@ struct _qemuMonitor {
  
  /* cache of query-command-line-options results */
  virJSONValuePtr options;
 +
 +/* If found, path to the virtio memballoon driver */
 +char *balloonpath;
 +bool ballooninit;
  };
  
  static virClassPtr qemuMonitorClass;
 @@ -248,6 +252,7 @@ static void qemuMonitorDispose(void *obj)
  virCondDestroy(mon-notify);
  VIR_FREE(mon-buffer);
  virJSONValueFree(mon-options);
 +VIR_FREE(mon-balloonpath);
  }
  
  
 @@ -925,6 +930,105 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, 
 virJSONValuePtr options)
  mon-options = options;
  }
  
 +/* Search the qom objects for the balloon driver object by it's known name
 + * of virtio-balloon-pci.  The entry for the driver will be found in the
 + * returned 'type' field using the syntax childvirtio-balloon-pci.
 + *
 + * Once found, check the entry to ensure it has the correct property listed.
 + * If it does not, then obtaining statistics from qemu will not be possible.
 + * This feature was added to qemu 1.5.
 + *
 + * This procedure will be call recursively until found or the qom-list is
 + * exhausted.
 + *
 + * Returns:
 + *
 + *   1  - Found
 + *   0  - Not found still looking
 + *  -1  - Error bail out
 + *
 + * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
 + */
 +static int
 +qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
 + virDomainObjPtr vm,
 + const char *curpath)
 +{
 +size_t i, j, npaths = 0, nprops = 0;
 +int ret = 0;
 +char *nextpath = NULL;
 +qemuMonitorJSONListPathPtr *paths = NULL;
 +qemuMonitorJSONListPathPtr *bprops = NULL;
 +
 +/* Already set and won't change or we already search and failed to find 
 */
 +if (mon-balloonpath || mon-ballooninit)
 +return 1;

This isn't correct logic. You need

   if (mon-balloonpath) {
 return 1;
   } else if (mon-ballooninit) {
  virReportError(VIR_ERR_INTERNAL_ERROR, %s
 _(Cannot determine balloon device path));
  return -1;
   }

 +
 +/* Not supported */
 +if (!vm-def-memballoon ||
 +vm-def-memballoon-model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
 +VIR_DEBUG(Model must be virtio to get memballoon path);

You need to use virReportError here, so the caller sees an error
messages.

 +return -1;
 +}
 +
 +VIR_DEBUG(Searching for Balloon Object Path starting at %s, curpath);
 +
 +npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, paths);
 +
 +for (i = 0; i  npaths  ret == 0; i++) {
 +
 +if (STREQ_NULLABLE(paths[i]-type, linkvirtio-balloon-pci)) {
 +VIR_DEBUG(Path to virtio-balloon-pci is '%s/%s',
 +  curpath, paths[i]-name);
 +if (virAsprintf(nextpath, %s/%s, curpath, paths[i]-name)  
 0) {
 +ret = -1;
 +goto cleanup;
 +}
 +
 +/* Now look at the each of the property entries to determine
 + * whether guest-stats-polling-interval exists.  If not,
 + * then this version of qemu/kvm does not support the feature.
 + */
 +nprops = qemuMonitorJSONGetObjectListPaths(mon, nextpath, 
 bprops);
 +for (j = 0; j  nprops; j++) {
 +if (STREQ(bprops[j]-name, 

Re: [libvirt] [PATCH v3 08/10] Specify remote protocol for virDomainSetMemoryStatsPeriodFlags

2013-07-12 Thread Daniel P. Berrange
On Thu, Jul 11, 2013 at 07:55:58PM -0400, John Ferlan wrote:
 Wire up the remote protocol
 ---
  src/remote/remote_driver.c   |  1 +
  src/remote/remote_protocol.x | 15 ++-
  src/remote_protocol-structs  |  6 ++
  3 files changed, 21 insertions(+), 1 deletion(-)

ACK


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

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


Re: [libvirt] [PATCH v3 06/10] Add capability to fetch balloon stats

2013-07-12 Thread Daniel P. Berrange
On Thu, Jul 11, 2013 at 07:55:56PM -0400, John Ferlan wrote:
 This patch will add the qemuMonitorJSONGetMemoryStats() to execute a
 guest-stats on the balloonpath using get-qom replacing the former
 mechanism which looked through the query-ballon returned data for
 the fields.  The query-balloon code only returns 'actual' memory.
 Rather than duplicating the existing code, have the JSON API use the
 GetBalloonInfo API.
 
 A check in the qemuMonitorGetMemoryStats() will be made to ensure the
 balloon driver path has been set.  Since the underlying JSON code can
 return data not associated with the balloon driver, we don't fail on
 a failure to get the balloonpath.  Of course since we've made the check,
 we can then set the ballooninit flag.  Getting the path here is primarily
 due to the process reconnect path which doesn't attempt to set the
 collection period.
 ---
  src/qemu/qemu_monitor.c  |  10 ++-
  src/qemu/qemu_monitor_json.c | 190 
 ---
  src/qemu/qemu_monitor_json.h |   1 +
  3 files changed, 95 insertions(+), 106 deletions(-)

Can you also extend qemumonitorjsontest.c to cover the new
code in  qemuMonitorJSONGetMemoryStats.

 
 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index a3e250c..14b80e3 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -1483,10 +1483,14 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
  return -1;
  }
  
 -if (mon-json)
 -ret = qemuMonitorJSONGetMemoryStats(mon, stats, nr_stats);
 -else
 +if (mon-json) {
 +ignore_value(qemuMonitorFindBalloonObjectPath(mon, mon-vm, /));
 +mon-ballooninit = true;

Hmm, I think that qemuMonitorFindBalloonObjectPath ought to be
setting 'mon-ballooninit = true' itself, rather than expecting
all the callers to do it.

Actually I should have mentioned this against the previous patch.

 +ret = qemuMonitorJSONGetMemoryStats(mon, mon-balloonpath,
 +stats, nr_stats);
 +} else {
  ret = qemuMonitorTextGetMemoryStats(mon, stats, nr_stats);
 +}
  return ret;
  }
  

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 09/10] Implement the virDomainSetMemoryStatsPeriodFlags for QEMU driver

2013-07-12 Thread Daniel P. Berrange
On Thu, Jul 11, 2013 at 07:55:59PM -0400, John Ferlan wrote:
 Implement the new API that will handle setting the balloon driver statistics
 collection period in order to enable or disable the collection dynamically.
 ---
  src/qemu/qemu_driver.c | 65 
 ++
  1 file changed, 65 insertions(+)

ACK


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

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


Re: [libvirt] [PATCH v3 07/10] Add new public API virDomainSetMemoryStatsPeriodFlags

2013-07-12 Thread Daniel P. Berrange
On Thu, Jul 11, 2013 at 07:55:57PM -0400, John Ferlan wrote:
 Add new API in order to set the balloon memory driver statistics collection
 period in order to allow dynamic period adjustment for the virsh dommemstats 
 to
 display balloon stats data
 ---
  include/libvirt/libvirt.h.in |  3 +++
  src/driver.h |  6 +
  src/libvirt.c| 64 
 
  src/libvirt_public.syms  |  5 
  4 files changed, 78 insertions(+)

ACK


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

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


Re: [libvirt] [PATCH v3 10/10] Allow balloon driver collection to be adjusted dynamically

2013-07-12 Thread Daniel P. Berrange
On Thu, Jul 11, 2013 at 07:56:00PM -0400, John Ferlan wrote:
 Use the virDomainSetMemoryStatsPeriodFlags() to pass a period defined by
 usage of a new --period option in order to set the collection period for the
 balloon driver. This may enable or disable the collection based on the value.
 
 Add the --current, --live,  --config options to dommemstat.
 ---
  docs/formatdomain.html.in| 11 +++-
  tools/virsh-domain-monitor.c | 66 
 ++--
  tools/virsh.pod  | 22 ++-
  3 files changed, 94 insertions(+), 5 deletions(-)
 
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 93d2416..df84ed2 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -4660,7 +4660,16 @@ qemu-kvm -net nic,model=? /dev/null
  p
The optional codeperiod/code allows the QEMU virtio memory
balloon driver to provide statistics through the codevirsh
 -  dommemstat [domain]/code command.
 +  dommemstat [domain]/code command. By default, collection is
 +  not enabled. In order to enable, use the codevirsh dommemstat
 +  [domain] --period [number]/code command or codevirsh 
 edit/code
 +  command to add the option to the XML definition.
 +  The codevirsh dommemstat/code will accept the options
 +  code--live/code, code--current/code, or 
 code--config/code.
 +  If an option is not provided, the change for a running domain will
 +  only be made to the active guest.
 +  If the QEMU driver is not at the right
 +  revision, the attempt to set the period will fail.
span class='since'Since 1.1.1, requires QEMU 1.5/span
  /p
/dd
 diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
 index 5fbd32c..4cbf105 100644
 --- a/tools/virsh-domain-monitor.c
 +++ b/tools/virsh-domain-monitor.c
 @@ -314,6 +314,23 @@ static const vshCmdOptDef opts_dommemstat[] = {
   .flags = VSH_OFLAG_REQ,
   .help = N_(domain name, id or uuid)
  },
 +{.name = period,
 + .type = VSH_OT_DATA,
 + .flags = VSH_OFLAG_EMPTY_OK,
 + .help = N_(period in seconds to set collection)
 +},

Hmm, I think I'd prefer to see the 'period' specified as a flag,
rather than positional arg. eg

  virsh dommemstat --period NNN

 @@ -668,10 +668,30 @@ Both I--live and I--current flags may be given, but 
 I--current is
  exclusive. If no flag is specified, behavior is different depending
  on hypervisor.
  
 -=item Bdommemstat Idomain
 +=item Bdommemstat Idomain [Iperiod Bseconds]
 +[[I--config] [I--live] | [I--current]]
  
  Get memory stats for a running domain.
  
 +Depending on the hypervisor a variety of statistics can be returned
 +
 +For QEMU/KVM with a memory balloon, setting the optional Iperiod to a
 +value larger than 0 in seconds will allow the balloon driver to return
 +additional statistics which will be displayed by subsequent Bdommemstat
 +commands. Setting the Iperiod to 0 will stop the balloon driver collection,
 +but does not clear the statistics in the balloon driver. Requires at least
 +QEMU/KVM 1.5 to be running on the host.
 +
 +The I--live, I--config, and I--current flags are only valid when using
 +the Iperiod option in order to set the collection period for the balloon
 +driver. If I--live is specified, only the running guest collection period
 +is affected. If I--config is specified, affect the next boot of a 
 persistent
 +guest. If I--current is specified, affect the current guest state.
 +
 +Both I--live and I--config flags may be given, but I--current is
 +exclusive. If no flag is specified, behavior is different depending
 +on the guest state.
 +
  =item Bdomblkerror Idomain
  
  Show errors on block devices.  This command usually comes handy when


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] Notes on configuring Open vSwitch, Linux bridge and Libvirt

2013-07-12 Thread Kashyap Chamarthy
On 07/12/2013 05:51 PM, Kashyap Chamarthy wrote:
 Heya Laine,
 
 Here's some quick notes to associate libvirt guests to Open vSwitch.
 
 Configure Open vSwitch
 --
 
 Now that a regular Linux bridge is configured, let's try to configure an
 OVS brdige and get IP addresses from that space:
 
 Create an Open vSwitch bridge device called 'ovsbr', and display the
 current state of OpenvSwitch database contents:
 
 $ ovs-vsctl add-br ovsbr
 $ ovs-vsctl show
 
 
 Add a virtual ethernet interface called 'veth0' with

s/Add a virtual ethernet interface called 'veth0' with/ Add a pair of virtual 
ethernet
interfaces 'veth0' and 'veth1'

 
 $ ip link add name veth0 \
   type veth peer name veth1
 
 Add 'veth0' ethernet device to the Linux bridge 'br0', and enumerate all
 bridge devices:
 
 $ brctl addif br0 veth0
 $ brctl show
 
 Now, associate virtual ethernet device 'veth1' to the OVS bridge,
 and display the current state of OpenvSwitch database contents
 
 $ ovs-vsctl add-port ovsbr veth1
 $ ovs-vsctl show
 
 
 Bring up both the virtual ethernet interfaces 'veth0' and 'veth1'
 
 $ ip link set veth0 up  \
   ip link set veth1 up
 
 
 Update libvirt guest's bridge source to OVS
 ---
 
 Install a minimal Fedora guest with Oz (or any other mechanism):
 
 $ wget \
 https://github.com/kashyapc/virt-scripts/blob/master/oz/oz-jeos.bash
 $ ./oz-jeos f19-min f19
 
 Once install is finished, define the guest XML from the current dir:
 
$ virsh define f19-minJul_12_2013-12
 
 
 Now let's edit libvirt's guest XML file to reflect its bridge source is
 OVS bridge:
 
 $ virsh edit f19-min
 
 The contents of the guest XML should reflect something along the below
 lines:
 
 $ virsh dumpxml f19-min | grep bridge -A8
 interface type='bridge'
   mac address='52:54:00:a6:08:70'/
   source bridge='ovsbr'/
   virtualport type='openvswitch'
 parameters interfaceid='ecdff22d-ce80-4ae7-a008-42994415084e'/
   /virtualport
   target dev='vnet2'/
   model type='virtio'/
   alias name='net0'/
   address type='pci' domain='0x' bus='0x00' slot='0x03' 
 function='0x0'/
 /interface
 
 Start the guest, and check the IP of it:
 
 $ virsh start f19-jeos --console
 $ ifconfig eth0
 
 
 Please note, this is just a simple test, I haven't done any further 
 experiments with VLAN
 tagging, etc.
 
 
 Slightly verbose notes:
 
   http://kashyapc.fedorapeople.org/virt/openvswitch-and-libvirt-kvm.txt


It's probably worth it to make a libvirt networking docs patch.


-- 
/kashyap

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


Re: [libvirt] Notes on configuring Open vSwitch, Linux bridge and Libvirt

2013-07-12 Thread Daniel Veillard
On Fri, Jul 12, 2013 at 05:51:14PM +0530, Kashyap Chamarthy wrote:
 Heya Laine,
 
 Here's some quick notes to associate libvirt guests to Open vSwitch.
 
 Configure Open vSwitch
 --
 
 Now that a regular Linux bridge is configured, let's try to configure an
 OVS brdige and get IP addresses from that space:
 
 Create an Open vSwitch bridge device called 'ovsbr', and display the
 current state of OpenvSwitch database contents:
 
 $ ovs-vsctl add-br ovsbr
 $ ovs-vsctl show
 
 
 Add a virtual ethernet interface called 'veth0' with
 
 $ ip link add name veth0 \
   type veth peer name veth1
 
 Add 'veth0' ethernet device to the Linux bridge 'br0', and enumerate all
 bridge devices:
 
 $ brctl addif br0 veth0
 $ brctl show
 
 Now, associate virtual ethernet device 'veth1' to the OVS bridge,
 and display the current state of OpenvSwitch database contents
 
 $ ovs-vsctl add-port ovsbr veth1
 $ ovs-vsctl show
 
 
 Bring up both the virtual ethernet interfaces 'veth0' and 'veth1'
 
 $ ip link set veth0 up  \
   ip link set veth1 up
 
 
 Update libvirt guest's bridge source to OVS
 ---
 
 Install a minimal Fedora guest with Oz (or any other mechanism):
 
 $ wget \
 https://github.com/kashyapc/virt-scripts/blob/master/oz/oz-jeos.bash
 $ ./oz-jeos f19-min f19
 
 Once install is finished, define the guest XML from the current dir:
 
$ virsh define f19-minJul_12_2013-12
 
 
 Now let's edit libvirt's guest XML file to reflect its bridge source is
 OVS bridge:
 
 $ virsh edit f19-min
 
 The contents of the guest XML should reflect something along the below
 lines:
 
 $ virsh dumpxml f19-min | grep bridge -A8
 interface type='bridge'
   mac address='52:54:00:a6:08:70'/
   source bridge='ovsbr'/
   virtualport type='openvswitch'
 parameters interfaceid='ecdff22d-ce80-4ae7-a008-42994415084e'/
   /virtualport
   target dev='vnet2'/
   model type='virtio'/
   alias name='net0'/
   address type='pci' domain='0x' bus='0x00' slot='0x03' 
 function='0x0'/
 /interface
 
 Start the guest, and check the IP of it:
 
 $ virsh start f19-jeos --console
 $ ifconfig eth0
 
 
 Please note, this is just a simple test, I haven't done any further 
 experiments with VLAN
 tagging, etc.
 
 
 Slightly verbose notes:
 
   http://kashyapc.fedorapeople.org/virt/openvswitch-and-libvirt-kvm.txt
 

  Cool !

It would be good if we could get that on some permanent web
page on libvirt.org :-)

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


Re: [libvirt] Notes on configuring Open vSwitch, Linux bridge and Libvirt

2013-07-12 Thread Daniel P. Berrange
On Fri, Jul 12, 2013 at 05:51:14PM +0530, Kashyap Chamarthy wrote:
 Heya Laine,
 
 Here's some quick notes to associate libvirt guests to Open vSwitch.
 
 Configure Open vSwitch
 --
 
 Now that a regular Linux bridge is configured, let's try to configure an
 OVS brdige and get IP addresses from that space:
 
 Create an Open vSwitch bridge device called 'ovsbr', and display the
 current state of OpenvSwitch database contents:
 
 $ ovs-vsctl add-br ovsbr
 $ ovs-vsctl show
 
 
 Add a virtual ethernet interface called 'veth0' with
 
 $ ip link add name veth0 \
   type veth peer name veth1
 
 Add 'veth0' ethernet device to the Linux bridge 'br0', and enumerate all
 bridge devices:
 
 $ brctl addif br0 veth0
 $ brctl show

I don't really see why you are linking ovs to a traditional software
bridge. You now have the overheads of both bridging  ovs code in
your data path. Surely it is better to connect ovs with your physical
NIC, taking traditional bridges out of the loop completely.

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] domain controller index check

2013-07-12 Thread Ján Tomko
On 07/09/2013 06:13 AM, Jincheng Miao wrote:
 The index of the controller should not be limited in [zero, INT_MAX].
 So use virStrToLong_ui() and check the limit of the controller
 index in virDomainControllerDefParseXML().
 ---
  src/conf/domain_conf.c  | 6 +++---
  src/conf/domain_conf.h  | 2 +-
  src/qemu/qemu_command.c | 2 +-
  src/vmx/vmx.c   | 3 +--
  4 files changed, 6 insertions(+), 7 deletions(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 402e6e9..d4c1054 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -2655,7 +2655,7 @@ virDomainDefRejectDuplicateControllers(virDomainDefPtr 
 def)
  
  for (i = 0; i  def-ncontrollers; i++) {
  cont = def-controllers[i];
 -if (cont-idx  max_idx[cont-type])
 +if ((int)cont-idx  max_idx[cont-type])
^ missing space

  max_idx[cont-type] = cont-idx;
  }
  
 @@ -2663,7 +2663,7 @@ virDomainDefRejectDuplicateControllers(virDomainDefPtr 
 def)
  max_idx[VIR_DOMAIN_CONTROLLER_TYPE_USB] = -1;
  
  for (i = 0; i  VIR_DOMAIN_CONTROLLER_TYPE_LAST; i++) {
 -if (max_idx[i] = 0  !(bitmaps[i] = virBitmapNew(max_idx[i] + 1)))
 +if (max_idx[i] = 0  !(bitmaps[i] = 
 virBitmapNew((size_t)max_idx[i] + 1)))

This cast seems unnecessary.

  goto no_memory;
  nbitmaps++;
  }
 @@ -5593,7 +5593,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
  
  idx = virXMLPropString(node, index);
  if (idx) {
 -if (virStrToLong_i(idx, NULL, 10, def-idx)  0) {
 +if (virStrToLong_ui(idx, NULL, 10, def-idx)  0 || def-idx  
 INT_MAX) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(Cannot parse controller index %s), idx);
  goto error;

If we parse it as unsigned, we should print it as unsigned too. I've split the
line over 80 columns and squashed this in:

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b515887..354131e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14382,7 +14382,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
 }

 virBufferAsprintf(buf,
-  controller type='%s' index='%d',
+  controller type='%s' index='%u',
   type, def-idx);

 if (model) {


 diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
 index da83eb6..7897b4b 100644
 --- a/src/conf/domain_conf.h
 +++ b/src/conf/domain_conf.h
 @@ -790,7 +790,7 @@ struct _virDomainVirtioSerialOpts {
  /* Stores the virtual disk controller configuration */
  struct _virDomainControllerDef {
  int type;
 -int idx;
 +unsigned int idx;
  int model; /* -1 == undef */
  unsigned int queues;
  union {
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 46db28a..7fd1cbf 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -1585,7 +1585,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
  
  for (i = 0; i  def-ncontrollers; i++) {
  if (def-controllers[i]-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) 
 {
 -if (def-controllers[i]-idx  max_idx)
 +if ((int)def-controllers[i]-idx  max_idx)
^ missing space
  max_idx = def-controllers[i]-idx;
  }
  }
 diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
 index 5464d13..deddfaa 100644
 --- a/src/vmx/vmx.c
 +++ b/src/vmx/vmx.c
 @@ -1664,8 +1664,7 @@ virVMXParseConfig(virVMXContext *ctx,
  
  for (controller = 0; controller  def-ncontrollers; ++controller) {
  if (def-controllers[controller]-type == 
 VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
 -if (def-controllers[controller]-idx  0 ||
 -def-controllers[controller]-idx  3) {
 +if (def-controllers[controller]-idx  3) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(SCSI controller index %d out of [0..3] 
 range),
 def-controllers[controller]-idx);
 

ACK and pushed with a test added:

diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-negative-index-invalid.xml
b/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-negative-index-invalid.xml
new file mode 100644
index 000..be3d8f2
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-negative-index-invalid.xml
@@ -0,0 +1,15 @@
+domain type='qemu'
+  namefdr-br/name
+  memory unit='KiB'2097152/memory
+  currentMemory unit='KiB'2097152/currentMemory
+  vcpu placement='static' cpuset='0-1'2/vcpu
+  os
+type arch='x86_64' machine='pc-1.2'hvm/type
+boot dev='hd'/
+  /os
+  devices
+emulator/usr/libexec/qemu-kvm/emulator
+controller type='pci' index='0' model='pci-root'/
+controller type='pci' index='-1' model='pci-bridge'/
+  /devices
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 

[libvirt] [PATCH] conf: reject pci-root controllers with non-zero indexes

2013-07-12 Thread Ján Tomko
https://bugzilla.redhat.com/show_bug.cgi?id=981261
---
 src/conf/domain_conf.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5f0366e..602c9a6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5668,6 +5668,13 @@ virDomainControllerDefParseXML(xmlNodePtr node,
  have an address));
 goto error;
 }
+if (def-idx != 0) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(pci-root controller should have 
+ index 0));
+goto error;
+}
+
 }
 
 default:
-- 
1.8.1.5

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


Re: [libvirt] [PATCH v3 05/10] Determine whether to start balloon memory stats gathering.

2013-07-12 Thread John Ferlan
On 07/12/2013 08:39 AM, Daniel P. Berrange wrote:
 On Thu, Jul 11, 2013 at 07:55:55PM -0400, John Ferlan wrote:
 At vm startup and attach attempt to set the balloon driver statistics
 collection period based on the value found in the domain xml file. This
 is not done at reconnect since it's possible that a collection period
 was set on the live guest and making the set period call would reset to
 whatever value is stored in the config file.

 Setting the stats collection period has a side effect of searching through
 the qom-list output for the virtio balloon driver and making sure that it
 has the right properties in order to allow setting of a collection period
 and eventually fetching of statistics.

 The walk through the qom-list is expensive and thus the balloonpath will
 be saved in the monitor private structure as well as a flag indicating
 that the initialization has already been attempted (in the event that a
 path is not found, no sense to keep checking).

 This processing model conforms to the qom object model model which
 requires setting object properties after device startup. That is, it's
 not possible to pass the period along via the startup code as it won't
 be recognized.
 ---
  src/qemu/qemu_monitor.c  | 130 
 +++
  src/qemu/qemu_monitor.h  |   2 +
  src/qemu/qemu_monitor_json.c |  24 
  src/qemu/qemu_monitor_json.h |   3 +
  src/qemu/qemu_process.c  |  14 -
  5 files changed, 171 insertions(+), 2 deletions(-)

 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index 6f9a8fc..a3e250c 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -83,6 +83,10 @@ struct _qemuMonitor {
  
  /* cache of query-command-line-options results */
  virJSONValuePtr options;
 +
 +/* If found, path to the virtio memballoon driver */
 +char *balloonpath;
 +bool ballooninit;
  };
  
  static virClassPtr qemuMonitorClass;
 @@ -248,6 +252,7 @@ static void qemuMonitorDispose(void *obj)
  virCondDestroy(mon-notify);
  VIR_FREE(mon-buffer);
  virJSONValueFree(mon-options);
 +VIR_FREE(mon-balloonpath);
  }
  
  
 @@ -925,6 +930,105 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, 
 virJSONValuePtr options)
  mon-options = options;
  }
  
 +/* Search the qom objects for the balloon driver object by it's known name
 + * of virtio-balloon-pci.  The entry for the driver will be found in the
 + * returned 'type' field using the syntax childvirtio-balloon-pci.
 + *
 + * Once found, check the entry to ensure it has the correct property listed.
 + * If it does not, then obtaining statistics from qemu will not be possible.
 + * This feature was added to qemu 1.5.
 + *
 + * This procedure will be call recursively until found or the qom-list is
 + * exhausted.
 + *
 + * Returns:
 + *
 + *   1  - Found
 + *   0  - Not found still looking
 + *  -1  - Error bail out
 + *
 + * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
 + */
 +static int
 +qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
 + virDomainObjPtr vm,
 + const char *curpath)
 +{
 +size_t i, j, npaths = 0, nprops = 0;
 +int ret = 0;
 +char *nextpath = NULL;
 +qemuMonitorJSONListPathPtr *paths = NULL;
 +qemuMonitorJSONListPathPtr *bprops = NULL;
 +
 +/* Already set and won't change or we already search and failed to find 
 */
 +if (mon-balloonpath || mon-ballooninit)
 +return 1;
 
 This isn't correct logic. You need
 
if (mon-balloonpath) {
  return 1;
} else if (mon-ballooninit) {
   virReportError(VIR_ERR_INTERNAL_ERROR, %s
  _(Cannot determine balloon device path));
   return -1;
}
 

This is a recursive function - setting ballooninit in here means I have
to know when I'm back at the first call. That's why it's set in the
callers.  Since ballooninit means I've either made the call and was
successful or I've made the call and wasn't successful.

I think an argument could be made that the checking of balloonpath
probably is extraneous.

Since the balloon driver stats are only supported in qemu 1.5 and
beyond, do we really want an error message?

 +
 +/* Not supported */
 +if (!vm-def-memballoon ||
 +vm-def-memballoon-model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
 +VIR_DEBUG(Model must be virtio to get memballoon path);
 
 You need to use virReportError here, so the caller sees an error
 messages.
 

hmm...  Do we really want to virReportError() in either case?  They're
not necessarily specifically asking for the statistics here... Although
with your suggestion to only call SetMemoryStatsPeriod if the period is
defined, I do see the point.  Although the first time someone does a
'virsh dommemstats domain' they'd see this message and perhaps wonder.
Also if the user didn't have the balloon driver configured, then this
would be spit out.


 +   

Re: [libvirt] [PATCH v3 05/10] Determine whether to start balloon memory stats gathering.

2013-07-12 Thread Daniel P. Berrange
On Fri, Jul 12, 2013 at 09:15:40AM -0400, John Ferlan wrote:
 On 07/12/2013 08:39 AM, Daniel P. Berrange wrote:
  On Thu, Jul 11, 2013 at 07:55:55PM -0400, John Ferlan wrote:
  At vm startup and attach attempt to set the balloon driver statistics
  collection period based on the value found in the domain xml file. This
  is not done at reconnect since it's possible that a collection period
  was set on the live guest and making the set period call would reset to
  whatever value is stored in the config file.
 
  Setting the stats collection period has a side effect of searching through
  the qom-list output for the virtio balloon driver and making sure that it
  has the right properties in order to allow setting of a collection period
  and eventually fetching of statistics.
 
  The walk through the qom-list is expensive and thus the balloonpath will
  be saved in the monitor private structure as well as a flag indicating
  that the initialization has already been attempted (in the event that a
  path is not found, no sense to keep checking).
 
  This processing model conforms to the qom object model model which
  requires setting object properties after device startup. That is, it's
  not possible to pass the period along via the startup code as it won't
  be recognized.
  ---
   src/qemu/qemu_monitor.c  | 130 
  +++
   src/qemu/qemu_monitor.h  |   2 +
   src/qemu/qemu_monitor_json.c |  24 
   src/qemu/qemu_monitor_json.h |   3 +
   src/qemu/qemu_process.c  |  14 -
   5 files changed, 171 insertions(+), 2 deletions(-)
 
  diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
  index 6f9a8fc..a3e250c 100644
  --- a/src/qemu/qemu_monitor.c
  +++ b/src/qemu/qemu_monitor.c
  @@ -83,6 +83,10 @@ struct _qemuMonitor {
   
   /* cache of query-command-line-options results */
   virJSONValuePtr options;
  +
  +/* If found, path to the virtio memballoon driver */
  +char *balloonpath;
  +bool ballooninit;
   };
   
   static virClassPtr qemuMonitorClass;
  @@ -248,6 +252,7 @@ static void qemuMonitorDispose(void *obj)
   virCondDestroy(mon-notify);
   VIR_FREE(mon-buffer);
   virJSONValueFree(mon-options);
  +VIR_FREE(mon-balloonpath);
   }
   
   
  @@ -925,6 +930,105 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, 
  virJSONValuePtr options)
   mon-options = options;
   }
   
  +/* Search the qom objects for the balloon driver object by it's known name
  + * of virtio-balloon-pci.  The entry for the driver will be found in the
  + * returned 'type' field using the syntax childvirtio-balloon-pci.
  + *
  + * Once found, check the entry to ensure it has the correct property 
  listed.
  + * If it does not, then obtaining statistics from qemu will not be 
  possible.
  + * This feature was added to qemu 1.5.
  + *
  + * This procedure will be call recursively until found or the qom-list is
  + * exhausted.
  + *
  + * Returns:
  + *
  + *   1  - Found
  + *   0  - Not found still looking
  + *  -1  - Error bail out
  + *
  + * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
  + */
  +static int
  +qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
  + virDomainObjPtr vm,
  + const char *curpath)
  +{
  +size_t i, j, npaths = 0, nprops = 0;
  +int ret = 0;
  +char *nextpath = NULL;
  +qemuMonitorJSONListPathPtr *paths = NULL;
  +qemuMonitorJSONListPathPtr *bprops = NULL;
  +
  +/* Already set and won't change or we already search and failed to 
  find */
  +if (mon-balloonpath || mon-ballooninit)
  +return 1;
  
  This isn't correct logic. You need
  
 if (mon-balloonpath) {
   return 1;
 } else if (mon-ballooninit) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s
   _(Cannot determine balloon device path));
return -1;
 }
  
 
 This is a recursive function - setting ballooninit in here means I have
 to know when I'm back at the first call. That's why it's set in the
 callers.  Since ballooninit means I've either made the call and was
 successful or I've made the call and wasn't successful.
 
 I think an argument could be made that the checking of balloonpath
 probably is extraneous.
 
 Since the balloon driver stats are only supported in qemu 1.5 and
 beyond, do we really want an error message?
 
  +
  +/* Not supported */
  +if (!vm-def-memballoon ||
  +vm-def-memballoon-model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) 
  {
  +VIR_DEBUG(Model must be virtio to get memballoon path);
  
  You need to use virReportError here, so the caller sees an error
  messages.
  
 
 hmm...  Do we really want to virReportError() in either case?  They're
 not necessarily specifically asking for the statistics here... Although
 with your suggestion to only call SetMemoryStatsPeriod if the period is
 defined, I do see the point.  Although 

Re: [libvirt] Notes on configuring Open vSwitch, Linux bridge and Libvirt

2013-07-12 Thread Kashyap Chamarthy
On 07/12/2013 06:32 PM, Daniel P. Berrange wrote:
 On Fri, Jul 12, 2013 at 05:51:14PM +0530, Kashyap Chamarthy wrote:
 Heya Laine,

 Here's some quick notes to associate libvirt guests to Open vSwitch.

 Configure Open vSwitch
 --

 Now that a regular Linux bridge is configured, let's try to configure an
 OVS brdige and get IP addresses from that space:

 Create an Open vSwitch bridge device called 'ovsbr', and display the
 current state of OpenvSwitch database contents:

 $ ovs-vsctl add-br ovsbr
 $ ovs-vsctl show


 Add a virtual ethernet interface called 'veth0' with

 $ ip link add name veth0 \
   type veth peer name veth1

 Add 'veth0' ethernet device to the Linux bridge 'br0', and enumerate all
 bridge devices:

 $ brctl addif br0 veth0
 $ brctl show
 
 I don't really see why you are linking ovs to a traditional software
 bridge. 

I had no specific reason on mind. The only test machine I had free was already 
having a
Linux bridge. I thought I'd try on it anyway.


Meanwhile, from this networking notes page,


http://docs.openstack.org/trunk/openstack-network/admin/content/under_the_hood_openvswitch.html

it appears that OpenStack uses Linux bridge in conjunction with an OVS bridge:

There are four distinct type of virtual networking devices: TAP
devices, veth pairs, Linux bridges, and Open vSwitch bridgesFor an
ethernet frame to travel from eth0 of virtual machine vm01, to the
physical network, it must pass through nine devices inside of the
host: TAP vnet0, Linux bridge qbrXXX, veth pair (qcbXXX, qvoXXX),
Open vSwitch bridge br-int, veth pair (int-br-eth1, phy-br-eth1),
and, finally, the physical network interface card eth1.


And further, it notes the distinction between how a Linux bridge and OVS in 
this context:

A Linux bridge behaves like a hub: you can connect multiple (physical
or virtual) network interfaces devices to a Linux bridge. Any ethernet
frames that come in from one interface attached to the bridge is
transmitted to all of the other devices.

An Open vSwitch bridge behaves like a virtual switch: network interface
devices connect to Open vSwitch bridge's ports, and the ports can be
configured much like a physical switch's ports, including VLAN
configurations.


 You now have the overheads of both bridging  ovs code in
 your data path. Surely it is better to connect ovs with your physical
 NIC, taking traditional bridges out of the loop completely.

That's my next test :)


-- 
/kashyap

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


Re: [libvirt] ANNOUNCE: libvirt 1.0.5.3 maintenance release

2013-07-12 Thread Cole Robinson
On 07/12/2013 03:45 AM, Ján Tomko wrote:
 On 07/12/2013 12:32 AM, Cole Robinson wrote:
 libvirt 1.0.5.3 maintenance release is now available. This is
 libvirt 1.0.5 with additional bugfixes that have accumulated
 upstream since the initial release.

 This release can be downloaded at:

 http://libvirt.org/sources/stable_updates/libvirt-1.0.5.3.tar.gz

 Changes in this version:

 ...
 * qemu: Avoid leaking uri in qemuMigrationPrepareDirect
 
 Ouch, this one contains a double free - migration with migrateuri will crash.
 
 I didn't realize I should've pushed
 commit 5744d96f211160d406ec0498c2f814a67d1a3fc8
 qemu: fix double free in qemuMigrationPrepareDirect
 
 to v1.0.5-maint as well.
 
 Sorry for the inconvenience.
 

No worries, I'll spin up another release.

- Cole

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


Re: [libvirt] Notes on configuring Open vSwitch, Linux bridge and Libvirt

2013-07-12 Thread Daniel P. Berrange
On Fri, Jul 12, 2013 at 07:05:00PM +0530, Kashyap Chamarthy wrote:
 On 07/12/2013 06:32 PM, Daniel P. Berrange wrote:
  On Fri, Jul 12, 2013 at 05:51:14PM +0530, Kashyap Chamarthy wrote:
  Heya Laine,
 
  Here's some quick notes to associate libvirt guests to Open vSwitch.
 
  Configure Open vSwitch
  --
 
  Now that a regular Linux bridge is configured, let's try to configure an
  OVS brdige and get IP addresses from that space:
 
  Create an Open vSwitch bridge device called 'ovsbr', and display the
  current state of OpenvSwitch database contents:
 
  $ ovs-vsctl add-br ovsbr
  $ ovs-vsctl show
 
 
  Add a virtual ethernet interface called 'veth0' with
 
  $ ip link add name veth0 \
type veth peer name veth1
 
  Add 'veth0' ethernet device to the Linux bridge 'br0', and enumerate all
  bridge devices:
 
  $ brctl addif br0 veth0
  $ brctl show
  
  I don't really see why you are linking ovs to a traditional software
  bridge. 
 
 I had no specific reason on mind. The only test machine I had free was 
 already having a
 Linux bridge. I thought I'd try on it anyway.
 
 
 Meanwhile, from this networking notes page,
 
 
 http://docs.openstack.org/trunk/openstack-network/admin/content/under_the_hood_openvswitch.html
 
 it appears that OpenStack uses Linux bridge in conjunction with an OVS bridge:
 
 There are four distinct type of virtual networking devices: TAP
 devices, veth pairs, Linux bridges, and Open vSwitch bridgesFor an
 ethernet frame to travel from eth0 of virtual machine vm01, to the
 physical network, it must pass through nine devices inside of the
 host: TAP vnet0, Linux bridge qbrXXX, veth pair (qcbXXX, qvoXXX),
 Open vSwitch bridge br-int, veth pair (int-br-eth1, phy-br-eth1),
 and, finally, the physical network interface card eth1.

That depends on how you configure openstack to operate. The reason openstack
links ovs to a bridge, is that you can't setup iptables rules with ovs. So
for each guest, openstack creates a separate bridge + veth pair, and then
sets iptables rules on that. This is pretty undesirable from a performance
POV due to the number of devices the traffic must traverse :-(  So I wouldn't
take openstack's usage as an example of good practice here.

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 04/10] Add 'period' for Memballoon statistics gathering capability

2013-07-12 Thread John Ferlan
On 07/12/2013 08:35 AM, Daniel P. Berrange wrote:
 ps, forgot to mention last time that you need to add a test case
 for this new XML - or just add the XML to 

1. Copied qemuxml2argv-balloon-device.xml to
qemuxml2argv-balloon-device-period.xml, adding 
stats period='10'/

2. Copied qemuxml2argv-balloon-device.arg to
qemuxml2argv-balloon-device-period.arg - made no changes
since the period is not part of the command

3. Added the balloon-device-period

I will squash in the following:

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-balloon-device-period.args b/te
new file mode 100644
index 000..48af1c4
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-balloon-device-period.args
@@ -0,0 +1,5 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /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 -hda \
+/dev/HostVG/QEMUGuest1 -device virtio-balloon-pci,id=balloon0,bus=pci.0,\
+addr=0x12
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-balloon-device-period.xml b/tes
new file mode 100644
index 000..c47c9bf
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-balloon-device-period.xml
@@ -0,0 +1,26 @@
+domain type='qemu'
+  nameQEMUGuest1/name
+  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+  memory unit='KiB'219136/memory
+  currentMemory unit='KiB'219136/currentMemory
+  vcpu placement='static'1/vcpu
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+  source dev='/dev/HostVG/QEMUGuest1'/
+  target dev='hda' bus='ide'/
+/disk
+memballoon model='virtio'
+  address type='pci' domain='0' bus='0' slot='18' function='0'/
+  stats period='10'/
+/memballoon
+  /devices
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 7d7332f..0f96eef 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -837,6 +837,7 @@ mymain(void)
 DO_TEST(balloon-device, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
 DO_TEST(balloon-device-auto,
 QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
+DO_TEST(balloon-device-period, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
 DO_TEST(sound, NONE);
 DO_TEST(sound-device,
 QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,


John

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


Re: [libvirt] [PATCH v3 05/10] Determine whether to start balloon memory stats gathering.

2013-07-12 Thread John Ferlan
On 07/12/2013 08:39 AM, Daniel P. Berrange wrote:
 On Thu, Jul 11, 2013 at 07:55:55PM -0400, John Ferlan wrote:
 At vm startup and attach attempt to set the balloon driver statistics
 collection period based on the value found in the domain xml file. This
 is not done at reconnect since it's possible that a collection period
 was set on the live guest and making the set period call would reset to
 whatever value is stored in the config file.

 Setting the stats collection period has a side effect of searching through
 the qom-list output for the virtio balloon driver and making sure that it
 has the right properties in order to allow setting of a collection period
 and eventually fetching of statistics.

 The walk through the qom-list is expensive and thus the balloonpath will
 be saved in the monitor private structure as well as a flag indicating
 that the initialization has already been attempted (in the event that a
 path is not found, no sense to keep checking).

 This processing model conforms to the qom object model model which
 requires setting object properties after device startup. That is, it's
 not possible to pass the period along via the startup code as it won't
 be recognized.
 ---
  src/qemu/qemu_monitor.c  | 130 
 +++
  src/qemu/qemu_monitor.h  |   2 +
  src/qemu/qemu_monitor_json.c |  24 
  src/qemu/qemu_monitor_json.h |   3 +
  src/qemu/qemu_process.c  |  14 -
  5 files changed, 171 insertions(+), 2 deletions(-)

 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index 6f9a8fc..a3e250c 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -83,6 +83,10 @@ struct _qemuMonitor {
  
  /* cache of query-command-line-options results */
  virJSONValuePtr options;
 +
 +/* If found, path to the virtio memballoon driver */
 +char *balloonpath;
 +bool ballooninit;
  };
  
  static virClassPtr qemuMonitorClass;
 @@ -248,6 +252,7 @@ static void qemuMonitorDispose(void *obj)
  virCondDestroy(mon-notify);
  VIR_FREE(mon-buffer);
  virJSONValueFree(mon-options);
 +VIR_FREE(mon-balloonpath);
  }
  
  
 @@ -925,6 +930,105 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, 
 virJSONValuePtr options)
  mon-options = options;
  }
  
 +/* Search the qom objects for the balloon driver object by it's known name
 + * of virtio-balloon-pci.  The entry for the driver will be found in the
 + * returned 'type' field using the syntax childvirtio-balloon-pci.
 + *
 + * Once found, check the entry to ensure it has the correct property listed.
 + * If it does not, then obtaining statistics from qemu will not be possible.
 + * This feature was added to qemu 1.5.
 + *
 + * This procedure will be call recursively until found or the qom-list is
 + * exhausted.
 + *
 + * Returns:
 + *
 + *   1  - Found
 + *   0  - Not found still looking
 + *  -1  - Error bail out
 + *
 + * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
 + */
 +static int
 +qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
 + virDomainObjPtr vm,
 + const char *curpath)
 +{
 +size_t i, j, npaths = 0, nprops = 0;
 +int ret = 0;
 +char *nextpath = NULL;
 +qemuMonitorJSONListPathPtr *paths = NULL;
 +qemuMonitorJSONListPathPtr *bprops = NULL;
 +
 +/* Already set and won't change or we already search and failed to find 
 */
 +if (mon-balloonpath || mon-ballooninit)
 +return 1;
 
 This isn't correct logic. You need
 
if (mon-balloonpath) {
  return 1;
} else if (mon-ballooninit) {
   virReportError(VIR_ERR_INTERNAL_ERROR, %s
  _(Cannot determine balloon device path));
   return -1;
}
 
 +
 +/* Not supported */
 +if (!vm-def-memballoon ||
 +vm-def-memballoon-model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
 +VIR_DEBUG(Model must be virtio to get memballoon path);
 
 You need to use virReportError here, so the caller sees an error
 messages.
 
 +return -1;
 +}
 +
 +VIR_DEBUG(Searching for Balloon Object Path starting at %s, curpath);
 +
 +npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, paths);
 +
 +for (i = 0; i  npaths  ret == 0; i++) {
 +
 +if (STREQ_NULLABLE(paths[i]-type, linkvirtio-balloon-pci)) {
 +VIR_DEBUG(Path to virtio-balloon-pci is '%s/%s',
 +  curpath, paths[i]-name);
 +if (virAsprintf(nextpath, %s/%s, curpath, paths[i]-name)  
 0) {
 +ret = -1;
 +goto cleanup;
 +}
 +
 +/* Now look at the each of the property entries to determine
 + * whether guest-stats-polling-interval exists.  If not,
 + * then this version of qemu/kvm does not support the feature.
 + */
 +nprops = qemuMonitorJSONGetObjectListPaths(mon, nextpath, 
 bprops);
 +for (j = 0; 

Re: [libvirt] [PATCH v3 10/10] Allow balloon driver collection to be adjusted dynamically

2013-07-12 Thread John Ferlan
On 07/12/2013 08:45 AM, Daniel P. Berrange wrote:
 On Thu, Jul 11, 2013 at 07:56:00PM -0400, John Ferlan wrote:
...snip...


 index 5fbd32c..4cbf105 100644
 --- a/tools/virsh-domain-monitor.c
 +++ b/tools/virsh-domain-monitor.c
 @@ -314,6 +314,23 @@ static const vshCmdOptDef opts_dommemstat[] = {
   .flags = VSH_OFLAG_REQ,
   .help = N_(domain name, id or uuid)
  },
 +{.name = period,
 + .type = VSH_OT_DATA,
 + .flags = VSH_OFLAG_EMPTY_OK,
 + .help = N_(period in seconds to set collection)
 +},
 
 Hmm, I think I'd prefer to see the 'period' specified as a flag,
 rather than positional arg. eg
 
   virsh dommemstat --period NNN
 

I've squashed the following:

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 4cbf105..773f96d 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -316,7 +316,7 @@ static const vshCmdOptDef opts_dommemstat[] = {
 },
 {.name = period,
  .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_EMPTY_OK,
+ .flags = VSH_OFLAG_REQ_OPT,
  .help = N_(period in seconds to set collection)
 },
 {.name = config,

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


Re: [libvirt] [PATCH v3 06/10] Add capability to fetch balloon stats

2013-07-12 Thread John Ferlan
On 07/12/2013 08:42 AM, Daniel P. Berrange wrote:
 On Thu, Jul 11, 2013 at 07:55:56PM -0400, John Ferlan wrote:
 This patch will add the qemuMonitorJSONGetMemoryStats() to execute a
 guest-stats on the balloonpath using get-qom replacing the former
 mechanism which looked through the query-ballon returned data for
 the fields.  The query-balloon code only returns 'actual' memory.
 Rather than duplicating the existing code, have the JSON API use the
 GetBalloonInfo API.

 A check in the qemuMonitorGetMemoryStats() will be made to ensure the
 balloon driver path has been set.  Since the underlying JSON code can
 return data not associated with the balloon driver, we don't fail on
 a failure to get the balloonpath.  Of course since we've made the check,
 we can then set the ballooninit flag.  Getting the path here is primarily
 due to the process reconnect path which doesn't attempt to set the
 collection period.
 ---
  src/qemu/qemu_monitor.c  |  10 ++-
  src/qemu/qemu_monitor_json.c | 190 
 ---
  src/qemu/qemu_monitor_json.h |   1 +
  3 files changed, 95 insertions(+), 106 deletions(-)
 
 Can you also extend qemumonitorjsontest.c to cover the new
 code in  qemuMonitorJSONGetMemoryStats.
 

 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index a3e250c..14b80e3 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -1483,10 +1483,14 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
  return -1;
  }
  
 -if (mon-json)
 -ret = qemuMonitorJSONGetMemoryStats(mon, stats, nr_stats);
 -else
 +if (mon-json) {
 +ignore_value(qemuMonitorFindBalloonObjectPath(mon, mon-vm, /));
 +mon-ballooninit = true;
 
 Hmm, I think that qemuMonitorFindBalloonObjectPath ought to be
 setting 'mon-ballooninit = true' itself, rather than expecting
 all the callers to do it.
 
 Actually I should have mentioned this against the previous patch.
 

Was the previous explanation good enough?  Since it's recursive I see
no where in the function that one could safely set the value.

John

 +ret = qemuMonitorJSONGetMemoryStats(mon, mon-balloonpath,
 +stats, nr_stats);
 +} else {
  ret = qemuMonitorTextGetMemoryStats(mon, stats, nr_stats);
 +}
  return ret;
  }
  
 
 Daniel
 

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


Re: [libvirt] Notes on configuring Open vSwitch, Linux bridge and Libvirt

2013-07-12 Thread Kashyap Chamarthy
[...]

 it appears that OpenStack uses Linux bridge in conjunction with an OVS 
 bridge:

 There are four distinct type of virtual networking devices: TAP
 devices, veth pairs, Linux bridges, and Open vSwitch bridgesFor an
 ethernet frame to travel from eth0 of virtual machine vm01, to the
 physical network, it must pass through nine devices inside of the
 host: TAP vnet0, Linux bridge qbrXXX, veth pair (qcbXXX, qvoXXX),
 Open vSwitch bridge br-int, veth pair (int-br-eth1, phy-br-eth1),
 and, finally, the physical network interface card eth1.
 
 That depends on how you configure openstack to operate. The reason openstack
 links ovs to a bridge, is that you can't setup iptables rules with ovs. 

This is useful insight, didn't know about it.

 So
 for each guest, openstack creates a separate bridge + veth pair, and then
 sets iptables rules on that. This is pretty undesirable from a performance
 POV due to the number of devices the traffic must traverse :-( 
 So I wouldn't
 take openstack's usage as an example of good practice here.

Noted. I see your recommendation: from Libvirt guests POV, OVS bridge connected 
to
physical eth0 is the least over-head .

I was wondering does an OVS bridge perform any better than Linux bridge. I had 
a brief
chat with Thomas Graf, he mentions, w.r.t performance, from some numbers they 
did, ovs
/slightly/ did better. However, he hasn't seen any numbers that would indicate 
that ovs is
better in performance but i haven't seen anything the other way around either.


-- 
/kashyap

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


[libvirt] [PATCH v2] qemu: don't use deprecated -no-kvm-pit-reinjection

2013-07-12 Thread Ján Tomko
Since qemu-kvm 1.1 [1] (since 1.3. in upstream QEMU [2])
'-no-kvm-pit-reinjection' has been deprecated.
Use -device kvm-pit,lost_tick_policy=discard instead.

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

[1] http://git.kernel.org/cgit/virt/kvm/qemu-kvm.git/commit/?id=4e4fa39
[2] http://git.qemu.org/?p=qemu.git;a=commitdiff;h=c21fb4f
---
 src/qemu/qemu_capabilities.c   |  5 ++--
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c|  8 --
 .../qemuxml2argv-kvm-pit-delay.args|  4 +++
 .../qemuxml2argv-kvm-pit-delay.xml | 29 ++
 .../qemuxml2argv-kvm-pit-device.args   |  5 
 .../qemuxml2argv-kvm-pit-device.xml| 29 ++
 tests/qemuxml2argvtest.c   |  4 +++
 8 files changed, 81 insertions(+), 4 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index baaaefe..31bd19c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -233,6 +233,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   mlock,
 
   vnc-share-policy, /* 150 */
+  kvm-pit,
 );
 
 struct _virQEMUCaps {
@@ -1381,6 +1382,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { pci-bridge, QEMU_CAPS_DEVICE_PCI_BRIDGE },
 { vfio-pci, QEMU_CAPS_DEVICE_VFIO_PCI },
 { scsi-generic, QEMU_CAPS_DEVICE_SCSI_GENERIC },
+{ kvm-pit, QEMU_CAPS_DEVICE_KVM_PIT },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
@@ -2446,13 +2448,12 @@ virQEMUCapsInitArchQMPBasic(virQEMUCapsPtr qemuCaps,
 
 /*
  * Currently only x86_64 and i686 support PCI-multibus,
- * -no-acpi and -no-kvm-pit-reinjection.
+ * -no-acpi
  */
 if (qemuCaps-arch == VIR_ARCH_X86_64 ||
 qemuCaps-arch == VIR_ARCH_I686) {
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS);
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_ACPI);
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_KVM_PIT);
 }
 
 ret = 0;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 7088747..9c21811 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -189,6 +189,7 @@ enum virQEMUCapsFlags {
 QEMU_CAPS_DRIVE_DISCARD  = 148, /* -drive 
discard=off(ignore)|on(unmap) */
 QEMU_CAPS_MLOCK  = 149, /* -realtime mlock=on|off */
 QEMU_CAPS_VNC_SHARE_POLICY   = 150, /* set display sharing policy */
+QEMU_CAPS_DEVICE_KVM_PIT = 151, /* -device kvm-pit */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0e517f2..0b640c0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6994,11 +6994,15 @@ qemuBuildCommandLine(virConnectPtr conn,
 case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY:
 /* delay is the default if we don't have kernel
(-no-kvm-pit), otherwise, the default is catchup. */
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT))
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_KVM_PIT))
+virCommandAddArgList(cmd, -device,
+ kvm-pit,lost_tick_policy=discard, 
NULL);
+else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT))
 virCommandAddArg(cmd, -no-kvm-pit-reinjection);
 break;
 case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP:
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT)) {
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT) ||
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_KVM_PIT)) {
 /* do nothing - this is default for kvm-pit */
 } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDF)) {
 /* -tdf switches to 'catchup' with userspace pit. */
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args 
b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args
new file mode 100644
index 000..326c582
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args
@@ -0,0 +1,4 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S \
+-M pc -m 214 -smp 2 -nographic -monitor unix:/tmp/test-monitor,server,nowait \
+-no-kvm-pit-reinjection -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 \
+-net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.xml 

[libvirt] [RFC PATCH] qemu: Shorten SCSI hostdev alias to avoid QEMU failure

2013-07-12 Thread Viktor Mihajlovski
The alias for hostdevs of type SCSI can be too long for QEMU if
larger LUNs are encountered. Here's a real life example:

hostdev mode='subsystem' type='scsi' managed='no'
  source
adapter name='scsi_host0'/
address bus='0' target='19' unit='1088634913'/
  /source
  address type='drive' controller='0' bus='0' target='0' unit='0'/
/hostdev

this results in a too long drive id, resulting in QEMU yelling

Property 'scsi-generic.drive' can't find value 
'drive-hostdev-scsi_host0-0-19-1088634913'

This commit changes the alias back to the default hostdev$(index)
scheme.

Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
---

Actually this is highlighting a larger issue. Apparently QEMU ids
are limited to a length of 32 bytes. This means that SCSI based
drives are also in danger to exceed this limit.
We should consider to use a drive index for controller-attached
disks for alias generation instead of the verbose
drive-$bustype-$busaddress schema.

 src/qemu/qemu_command.c|   10 +-
 .../qemuxml2argv-hostdev-scsi-boot.args|4 ++--
 .../qemuxml2argv-hostdev-scsi-lsi.args |4 ++--
 .../qemuxml2argv-hostdev-scsi-readonly.args|4 ++--
 .../qemuxml2argv-hostdev-scsi-virtio-scsi.args |4 ++--
 5 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 879aed8..8c8bef2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -818,16 +818,8 @@ qemuAssignDeviceHostdevAlias(virDomainDefPtr def, 
virDomainHostdevDefPtr hostdev
 }
 }
 
-if (hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
-if (virAsprintf(hostdev-info-alias, hostdev-%s-%d-%d-%d,
-hostdev-source.subsys.u.scsi.adapter,
-hostdev-source.subsys.u.scsi.bus,
-hostdev-source.subsys.u.scsi.target,
-hostdev-source.subsys.u.scsi.unit)  0)
-return -1;
-} else if (virAsprintf(hostdev-info-alias, hostdev%d, idx)  0) {
+if (virAsprintf(hostdev-info-alias, hostdev%d, idx)  0)
 return -1;
-}
 
 return 0;
 }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-boot.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-boot.args
index cd22672..ba1e6c6 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-boot.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-boot.args
@@ -4,6 +4,6 @@ unix:/tmp/test-monitor,server,nowait -no-acpi \
 -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 -usb \
 -drive file=/dev/HostVG/QEMUGuest2,if=none,id=drive-ide0-0-0 \
 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
--drive file=/dev/sg0,if=none,id=drive-hostdev-scsi_host0-0-0-0 \
--device 
scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=8,drive=drive-hostdev-scsi_host0-0-0-0,id=hostdev-scsi_host0-0-0-0,bootindex=1
 \
+-drive file=/dev/sg0,if=none,id=drive-hostdev0 \
+-device 
scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=8,drive=drive-hostdev0,id=hostdev0,bootindex=1
 \
 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi.args
index 06f7938..2cbf9be 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi.args
@@ -4,6 +4,6 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
 -device lsi,id=scsi0,bus=pci.0,addr=0x3 -usb \
 -drive file=/dev/HostVG/QEMUGuest2,if=none,id=drive-ide0-0-0 \
 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
--drive file=/dev/sg0,if=none,id=drive-hostdev-scsi_host0-0-0-0 \
--device 
scsi-generic,bus=scsi0.0,scsi-id=7,drive=drive-hostdev-scsi_host0-0-0-0,id=hostdev-scsi_host0-0-0-0
 \
+-drive file=/dev/sg0,if=none,id=drive-hostdev0 \
+-device scsi-generic,bus=scsi0.0,scsi-id=7,drive=drive-hostdev0,id=hostdev0 \
 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.args
index ea2f7af..8c274a9 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.args
@@ -4,6 +4,6 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
 -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 -usb \
 -drive file=/dev/HostVG/QEMUGuest2,if=none,id=drive-ide0-0-0 \
 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
--drive file=/dev/sg0,if=none,id=drive-hostdev-scsi_host0-0-0-0,readonly=on \
--device 
scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=8,drive=drive-hostdev-scsi_host0-0-0-0,id=hostdev-scsi_host0-0-0-0
 \
+-drive file=/dev/sg0,if=none,id=drive-hostdev0,readonly=on \
+-device 

Re: [libvirt] [PATCH v4 7/9] qemu: Introduce qemuBuildChrDeviceStr

2013-07-12 Thread John Ferlan
On 07/10/2013 01:02 PM, Michal Privoznik wrote:
 The function being introduced is responsible for creating command
 line argument for '-device' for given character device. Based on
 the chardev type, it calls appropriate qemuBuild.*ChrDeviceStr(),
 e.g.  qemuBuildSerialChrDeviceStr() for serial chardev and so on.
 ---
  src/qemu/qemu_command.c | 202 
 +---
  src/qemu/qemu_command.h |  12 +--
  2 files changed, 163 insertions(+), 51 deletions(-)
 
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 6cf46a2..063d76b 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -6593,6 +6593,21 @@ cleanup:
  return ret;
  }
  
 +static int
 +qemuBuildChrDeviceCommandLine(virCommandPtr cmd,
 +  virDomainDefPtr def,
 +  virDomainChrDefPtr chr,
 +  virQEMUCapsPtr qemuCaps)
 +{
 +char *devstr = NULL;
 +
 +if (qemuBuildChrDeviceStr(devstr, def, chr, qemuCaps)  0)
 +return -1;
 +
 +virCommandAddArgList(cmd, -device, devstr, NULL);

make -C tests valgrind reports:

==16114== 1,100 bytes in 1 blocks are definitely lost in loss record 100 of 118
==16114==at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==16114==by 0x4A089F0: realloc (vg_replace_malloc.c:662)
==16114==by 0x4C70BE7: virReallocN (viralloc.c:233)
==16114==by 0x4C73BA0: virBufferGrow (virbuffer.c:129)
==16114==by 0x4C740AE: virBufferVasprintf (virbuffer.c:321)
==16114==by 0x4C74223: virBufferAsprintf (virbuffer.c:294)
==16114==by 0x437DA2: qemuBuildChrDeviceStr (qemu_command.c:8547)
==16114==by 0x438027: qemuBuildChrDeviceCommandLine (qemu_command.c:6607)
==16114==by 0x43BAEF: qemuBuildCommandLine (qemu_command.c:7693)
==16114==by 0x4247EC: testCompareXMLToArgvHelper (qemuxml2argvtest.c:157)
==16114==by 0x425E97: virtTestRun (testutils.c:159)
==16114==by 0x4205F1: mymain (qemuxml2argvtest.c:715)
==16114== 


You need a VIR_FREE(devstr);

 +return 0;
 +}
 +


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


Re: [libvirt] Notes on configuring Open vSwitch, Linux bridge and Libvirt

2013-07-12 Thread Kashyap Chamarthy
On 07/12/2013 06:30 PM, Daniel Veillard wrote:
 On Fri, Jul 12, 2013 at 05:51:14PM +0530, Kashyap Chamarthy wrote:
 Heya Laine,

 Here's some quick notes to associate libvirt guests to Open vSwitch.

 Configure Open vSwitch
 --

 Now that a regular Linux bridge is configured, let's try to configure an
 OVS brdige and get IP addresses from that space:

 Create an Open vSwitch bridge device called 'ovsbr', and display the
 current state of OpenvSwitch database contents:

 $ ovs-vsctl add-br ovsbr
 $ ovs-vsctl show


 Add a virtual ethernet interface called 'veth0' with

 $ ip link add name veth0 \
   type veth peer name veth1

 Add 'veth0' ethernet device to the Linux bridge 'br0', and enumerate all
 bridge devices:

 $ brctl addif br0 veth0
 $ brctl show

 Now, associate virtual ethernet device 'veth1' to the OVS bridge,
 and display the current state of OpenvSwitch database contents

 $ ovs-vsctl add-port ovsbr veth1
 $ ovs-vsctl show


 Bring up both the virtual ethernet interfaces 'veth0' and 'veth1'

 $ ip link set veth0 up  \
   ip link set veth1 up


 Update libvirt guest's bridge source to OVS
 ---

 Install a minimal Fedora guest with Oz (or any other mechanism):

 $ wget \
 https://github.com/kashyapc/virt-scripts/blob/master/oz/oz-jeos.bash
 $ ./oz-jeos f19-min f19

 Once install is finished, define the guest XML from the current dir:

$ virsh define f19-minJul_12_2013-12


 Now let's edit libvirt's guest XML file to reflect its bridge source is
 OVS bridge:

 $ virsh edit f19-min

 The contents of the guest XML should reflect something along the below
 lines:

 $ virsh dumpxml f19-min | grep bridge -A8
 interface type='bridge'
   mac address='52:54:00:a6:08:70'/
   source bridge='ovsbr'/
   virtualport type='openvswitch'
 parameters interfaceid='ecdff22d-ce80-4ae7-a008-42994415084e'/
   /virtualport
   target dev='vnet2'/
   model type='virtio'/
   alias name='net0'/
   address type='pci' domain='0x' bus='0x00' slot='0x03' 
 function='0x0'/
 /interface

 Start the guest, and check the IP of it:

 $ virsh start f19-jeos --console
 $ ifconfig eth0


 Please note, this is just a simple test, I haven't done any further 
 experiments with VLAN
 tagging, etc.


 Slightly verbose notes:

   http://kashyapc.fedorapeople.org/virt/openvswitch-and-libvirt-kvm.txt

 
   Cool !
 
 It would be good if we could get that on some permanent web
 page on libvirt.org :-)

Sure. Will submit a doc patch once I'm done with experimenting a bit with other 
use-cases
like pure OVS bridging, etc.


-- 
/kashyap

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


[libvirt] [PATCH 4/6] LXC: Wire up the virDomainCreate{XML}WithFiles methods

2013-07-12 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Wire up the new virDomainCreate{XML}WithFiles methods in the
LXC driver, so that FDs get passed down to the init process.

The lxc_container code needs to do a little dance in order
to renumber the file descriptors it receives into linear
order, starting from STDERR_FILENO + 1.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/lxc/lxc_container.c  | 136 +--
 src/lxc/lxc_container.h  |   6 ++-
 src/lxc/lxc_controller.c |  36 +++--
 src/lxc/lxc_driver.c |  45 +---
 src/lxc/lxc_process.c|  16 +-
 src/lxc/lxc_process.h|   1 +
 6 files changed, 197 insertions(+), 43 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 940233b..d59de08 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -103,8 +103,10 @@ struct __lxc_child_argv {
 size_t nveths;
 char **veths;
 int monitor;
-char **ttyPaths;
+size_t npassFDs;
+int *passFDs;
 size_t nttyPaths;
+char **ttyPaths;
 int handshakefd;
 };
 
@@ -217,20 +219,28 @@ static virCommandPtr 
lxcContainerBuildInitCmd(virDomainDefPtr vmDef)
 }
 
 /**
- * lxcContainerSetStdio:
+ * lxcContainerSetupFDs:
  * @control: control FD from parent
  * @ttyfd: FD of tty to set as the container console
+ * @npassFDs: number of extra FDs
+ * @passFDs: list of extra FDs
  *
- * Sets the given tty as the primary conosole for the container as well as
- * stdout, stdin and stderr.
+ * Setup file descriptors in the container. @ttyfd is set to be
+ * the container's stdin, stdout  stderr. Any FDs included in
+ * @passFDs, will be dup()'d such that they start from stderr+1
+ * with no gaps.
  *
  * Returns 0 on success or -1 in case of error
  */
-static int lxcContainerSetStdio(int control, int ttyfd, int handshakefd)
+static int lxcContainerSetupFDs(int *ttyfd,
+size_t npassFDs, int *passFDs)
 {
 int rc = -1;
 int open_max;
 int fd;
+int last_fd;
+size_t i;
+size_t j;
 
 if (setsid()  0) {
 virReportSystemError(errno, %s,
@@ -238,44 +248,99 @@ static int lxcContainerSetStdio(int control, int ttyfd, 
int handshakefd)
 goto cleanup;
 }
 
-if (ioctl(ttyfd, TIOCSCTTY, NULL)  0) {
+if (ioctl(*ttyfd, TIOCSCTTY, NULL)  0) {
 virReportSystemError(errno, %s,
  _(ioctl(TIOCSTTY) failed));
 goto cleanup;
 }
 
-/* Just in case someone forget to set FD_CLOEXEC, explicitly
- * close all FDs before executing the container */
-open_max = sysconf(_SC_OPEN_MAX);
-if (open_max  0) {
+if (dup2(*ttyfd, STDIN_FILENO)  0) {
 virReportSystemError(errno, %s,
- _(sysconf(_SC_OPEN_MAX) failed));
+ _(dup2(stdin) failed));
 goto cleanup;
 }
-for (fd = 0; fd  open_max; fd++)
-if (fd != ttyfd  fd != control  fd != handshakefd) {
-int tmpfd = fd;
-VIR_MASS_CLOSE(tmpfd);
-}
 
-if (dup2(ttyfd, 0)  0) {
+if (dup2(*ttyfd, STDOUT_FILENO)  0) {
 virReportSystemError(errno, %s,
- _(dup2(stdin) failed));
+ _(dup2(stdout) failed));
 goto cleanup;
 }
 
-if (dup2(ttyfd, 1)  0) {
+if (dup2(*ttyfd, STDERR_FILENO)  0) {
 virReportSystemError(errno, %s,
- _(dup2(stdout) failed));
+ _(dup2(stderr) failed));
 goto cleanup;
 }
 
-if (dup2(ttyfd, 2)  0) {
+VIR_FORCE_CLOSE(*ttyfd);
+
+/* Any FDs in @passFDs need to be moved around so that
+ * they are numbered, without gaps, starting from
+ * STDERR_FILENO + 1
+ */
+for (i = 0; i  npassFDs; i++) {
+int wantfd;
+
+wantfd = STDERR_FILENO + i + 1;
+VIR_DEBUG(Pass %d onto %d, passFDs[i], wantfd);
+
+/* If we already have desired FD number, life
+ * is easy. Nothing needs renumbering */
+if (passFDs[i] == wantfd)
+continue;
+
+/*
+ * Lets check to see if any later FDs are occupying
+ * our desired FD number. If so, we must move them
+ * out of the way
+ */
+for (j = i + 1; j  npassFDs; j++) {
+if (passFDs[j] == wantfd) {
+VIR_DEBUG(Clash %zu, j);
+int newfd = dup(passFDs[j]);
+if (newfd  0) {
+virReportSystemError(errno,
+ _(Cannot move fd %d out of the way),
+ passFDs[j]);
+goto cleanup;
+}
+/* We're intentionally not closing the
+ * old value of passFDs[j], because we
+ * don't want later iterations of the
+ * loop to take it back. dup2() will
+

[libvirt] [PATCH 0/6] Enable fd passing / socket activation with LXC guest

2013-07-12 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Systemd has a concept of socket activation whereby systemd
will listen on a TCP socket in the host. When a client arrives
on the socket, systemd will run a service, passing it the
pre-opened TCP server socket. The service can then accept the
client connection.

This patch series adds the ability to pass pre-opened file
descriptors into LXC guests. The file descriptors will be
made available to the 'init' process in the container,
starting from STDERR_FILENO + 1.

For example, assuming you have pre-opened a file descriptors
in your shell

 # exec 10/tmp/foo
 # exec 20/tmp/bar
 # exec 30/tmp/wizz

You can then start a container with:

 # virsh -c lxc:/// start --pass-fds 10,20,30  demo

Inside that container the FDs will appear as 3, 4, 5:

 # virsh -c lxc:/// console demo
 Connected to domain demo
 Escape character is ^]
 sh-4.2# lsof -p 1 | grep /tmp
 sh1 root3w   REG   0,320 90226444 /tmp/foo
 sh1 root4w   REG   0,320 90238163 /tmp/bar
 sh1 root5w   REG   0,320 90238164 /tmp/wizz

Finally, if you run systemd inside the container, it can then
use these pre-opened file descriptors, passing them along when
launching services inside the container. So you have end-to-end
socket activation between the host  guest systemd instances.

Daniel P. Berrange (6):
  Introduce new domain create APIs to pass pre-opened FDs to LXC
  Introduce remote protocol support for virDomainCreate{XML}WithFiles
  Fix impl of virDomainCreateWithFlags remote client helper
  LXC: Wire up the virDomainCreate{XML}WithFiles methods
  Enable FD passing when starting guests with virsh
  Merge virCommandPreserveFD / virCommandTransferFD

 daemon/remote.c   | 104 ++
 include/libvirt/libvirt.h.in  |  10 +++
 python/generator.py   |   3 +
 python/libvirt-override-virConnect.py |  30 +++
 python/libvirt-override-virDomain.py  |  38 
 python/libvirt-override.c |  89 +++
 src/driver.h  |  13 +++
 src/fdstream.c|   3 +-
 src/libvirt.c | 154 
 src/libvirt_private.syms  |   3 +-
 src/libvirt_public.syms   |   6 ++
 src/lxc/lxc_container.c   | 136 ++---
 src/lxc/lxc_container.h   |   6 +-
 src/lxc/lxc_controller.c  |  36 +++-
 src/lxc/lxc_driver.c  |  45 --
 src/lxc/lxc_process.c |  20 -
 src/lxc/lxc_process.h |   1 +
 src/qemu/qemu_command.c   |  16 ++--
 src/remote/remote_driver.c|  91 +++
 src/remote/remote_protocol.x  |  32 ++-
 src/remote_protocol-structs   |  16 
 src/uml/uml_conf.c|   3 +-
 src/util/vircommand.c | 159 --
 src/util/vircommand.h |  13 +--
 tests/commandtest.c   |   5 +-
 tools/virsh-domain.c  |  82 +-
 tools/virsh.pod   |  13 ++-
 27 files changed, 960 insertions(+), 167 deletions(-)

-- 
1.8.1.4

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


[libvirt] [PATCH 1/6] Introduce new domain create APIs to pass pre-opened FDs to LXC

2013-07-12 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

With container based virt, it is useful to be able to pass
pre-opened file descriptors to the container init process.
This allows for containers to be auto-activated from incoming
socket connections, passing the active socket into the container.

To do this, introduce a pair of new APIs, virDomainCreateXMLWithFiles
and virDomainCreateWithFiles, which accept an array of file
descriptors. For the LXC driver, UNIX file descriptor passing
will be used to send them to libvirtd, which will them pass
them down to libvirt_lxc, which will then pass them to the container
init process.

This will only be implemented for LXC right now, but the design
is generic enough it could work with other hypervisors, hence
I suggest adding this to libvirt.so, rather than libvirt-lxc.so

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 include/libvirt/libvirt.h.in  |  10 +++
 python/generator.py   |   3 +
 python/libvirt-override-virConnect.py |  30 +++
 python/libvirt-override-virDomain.py  |  38 +
 python/libvirt-override.c |  89 
 src/driver.h  |  13 +++
 src/libvirt.c | 154 ++
 src/libvirt_public.syms   |   6 ++
 8 files changed, 343 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index b87255a..150a231 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1575,6 +1575,11 @@ virConnectPtr   virDomainGetConnect 
(virDomainPtr domain);
 virDomainPtrvirDomainCreateXML  (virConnectPtr conn,
  const char *xmlDesc,
  unsigned int flags);
+virDomainPtrvirDomainCreateXMLWithFiles(virConnectPtr conn,
+const char *xmlDesc,
+unsigned int nfiles,
+int *files,
+unsigned int flags);
 virDomainPtrvirDomainLookupByName   (virConnectPtr conn,
  const char *name);
 virDomainPtrvirDomainLookupByID (virConnectPtr conn,
@@ -2175,6 +2180,11 @@ int virDomainCreate 
(virDomainPtr domain);
 int virDomainCreateWithFlags (virDomainPtr domain,
   unsigned int flags);
 
+int virDomainCreateWithFiles (virDomainPtr domain,
+  unsigned int nfiles,
+  int *files,
+  unsigned int flags);
+
 int virDomainGetAutostart   (virDomainPtr domain,
  int *autostart);
 int virDomainSetAutostart   (virDomainPtr domain,
diff --git a/python/generator.py b/python/generator.py
index da642eb..427cebc 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -513,6 +513,9 @@ skip_function = (
 'virConnectUnregisterCloseCallback', # overriden in virConnect.py
 'virConnectRegisterCloseCallback', # overriden in virConnect.py
 
+'virDomainCreateXMLWithFiles', # overriden in virConnect.py
+'virDomainCreateWithFiles', # overriden in virDomain.py
+
 # 'Ref' functions have no use for bindings users.
 virConnectRef,
 virDomainRef,
diff --git a/python/libvirt-override-virConnect.py 
b/python/libvirt-override-virConnect.py
index 5495b70..a0f579d 100644
--- a/python/libvirt-override-virConnect.py
+++ b/python/libvirt-override-virConnect.py
@@ -310,3 +310,33 @@
 if ret == -1:
 raise libvirtError ('virConnectRegisterCloseCallback() failed', 
conn=self)
 return ret
+
+def createXMLWithFiles(self, xmlDesc, files, flags=0):
+Launch a new guest domain, based on an XML description similar
+to the one returned by virDomainGetXMLDesc()
+This function may require privileged access to the hypervisor.
+The domain is not persistent, so its definition will disappear when it
+is destroyed, or if the host is restarted (see virDomainDefineXML() to
+define persistent domains).
+
+@files provides an array of file descriptors which will be
+made available to the 'init' process of the guest. The file
+handles exposed to the guest will be renumbered to start
+from 3 (ie immediately following stderr). This is only
+supported for guests which use container based virtualization
+technology.
+
+If the VIR_DOMAIN_START_PAUSED flag is set, the guest domain
+will be started, but its CPUs will remain paused. The 

[libvirt] [PATCH 2/6] Introduce remote protocol support for virDomainCreate{XML}WithFiles

2013-07-12 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Since they make use of file descriptor passing, the remote protocol
methods for virDomainCreate{XML}WithFiles must be written by hand.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 daemon/remote.c  | 104 +++
 src/remote/remote_driver.c   |  71 +
 src/remote/remote_protocol.x |  32 -
 src/remote_protocol-structs  |  16 +++
 4 files changed, 222 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 5847e60..a1b571c 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -4891,6 +4891,110 @@ cleanup:
 }
 
 
+static int remoteDispatchDomainCreateXMLWithFiles(
+virNetServerPtr server ATTRIBUTE_UNUSED,
+virNetServerClientPtr client,
+virNetMessagePtr msg ATTRIBUTE_UNUSED,
+virNetMessageErrorPtr rerr,
+remote_domain_create_xml_with_files_args *args,
+remote_domain_create_xml_with_files_ret *ret)
+{
+int rv = -1;
+virDomainPtr dom = NULL;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+int *files = NULL;
+unsigned int nfiles = 0;
+size_t i;
+
+if (!priv-conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
+goto cleanup;
+}
+
+if (VIR_ALLOC_N(files, msg-nfds)  0)
+goto cleanup;
+for (i = 0; i  msg-nfds; i++) {
+if ((files[i] = virNetMessageDupFD(msg, i))  0)
+goto cleanup;
+nfiles++;
+}
+
+if ((dom = virDomainCreateXMLWithFiles(priv-conn, args-xml_desc,
+   nfiles, files,
+   args-flags)) == NULL)
+goto cleanup;
+
+make_nonnull_domain(ret-dom, dom);
+rv = 0;
+
+cleanup:
+for (i = 0; i  nfiles; i++) {
+VIR_FORCE_CLOSE(files[i]);
+}
+VIR_FREE(files);
+if (rv  0)
+virNetMessageSaveError(rerr);
+if (dom)
+virDomainFree(dom);
+return rv;
+}
+
+
+static int remoteDispatchDomainCreateWithFiles(
+virNetServerPtr server ATTRIBUTE_UNUSED,
+virNetServerClientPtr client,
+virNetMessagePtr msg ATTRIBUTE_UNUSED,
+virNetMessageErrorPtr rerr,
+remote_domain_create_with_files_args *args,
+remote_domain_create_with_files_ret *ret)
+{
+int rv = -1;
+virDomainPtr dom = NULL;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+int *files = NULL;
+unsigned int nfiles = 0;
+size_t i;
+
+if (!priv-conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
+goto cleanup;
+}
+
+if (VIR_ALLOC_N(files, msg-nfds)  0)
+goto cleanup;
+for (i = 0; i  msg-nfds; i++) {
+if ((files[i] = virNetMessageDupFD(msg, i))  0)
+goto cleanup;
+nfiles++;
+}
+
+if (!(dom = get_nonnull_domain(priv-conn, args-dom)))
+goto cleanup;
+
+if (virDomainCreateWithFiles(dom,
+ nfiles, files,
+ args-flags)  0)
+goto cleanup;
+
+make_nonnull_domain(ret-dom, dom);
+rv = 0;
+
+cleanup:
+for (i = 0; i  nfiles; i++) {
+VIR_FORCE_CLOSE(files[i]);
+}
+VIR_FREE(files);
+if (rv  0)
+virNetMessageSaveError(rerr);
+if (dom)
+virDomainFree(dom);
+return rv;
+}
+
+
+
 /*- Helpers. -*/
 
 /* get_nonnull_domain and get_nonnull_network turn an on-wire
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 81ecef1..e2764e2 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -6387,6 +6387,75 @@ cleanup:
 }
 
 
+static virDomainPtr
+remoteDomainCreateXMLWithFiles(virConnectPtr conn, const char *xml_desc,
+   unsigned int nfiles, int *files, unsigned int 
flags)
+{
+virDomainPtr rv = NULL;
+struct private_data *priv = conn-privateData;
+remote_domain_create_xml_with_files_args args;
+remote_domain_create_xml_with_files_ret ret;
+
+remoteDriverLock(priv);
+
+args.xml_desc = (char *)xml_desc;
+args.flags = flags;
+
+memset(ret, 0, sizeof(ret));
+
+if (callFull(conn, priv, 0,
+ files, nfiles,
+ NULL, NULL,
+ REMOTE_PROC_DOMAIN_CREATE_XML_WITH_FILES,
+ (xdrproc_t)xdr_remote_domain_create_xml_with_files_args, 
(char *)args,
+ (xdrproc_t)xdr_remote_domain_create_xml_with_files_ret, (char 
*)ret) == -1) {
+goto done;
+}
+
+rv = get_nonnull_domain(conn, ret.dom);
+xdr_free((xdrproc_t)xdr_remote_domain_create_xml_with_files_ret, (char 
*)ret);
+
+done:
+remoteDriverUnlock(priv);
+return rv;
+}
+
+
+static int
+remoteDomainCreateWithFiles(virDomainPtr dom,
+unsigned int nfiles, int *files,
+unsigned int 

[libvirt] [PATCH 3/6] Fix impl of virDomainCreateWithFlags remote client helper

2013-07-12 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

In the following commit:

  commit 03d813bbcd7b4a18360105500672b84d985dd889
  Author: Marek Marczykowski marma...@invisiblethingslab.com
  Date:   Thu May 23 02:01:30 2013 +0200

remote: fix dom-id after virDomainCreateWithFlags

The virDomainCreateWithFlags remote client helper was made to
invoke REMOTE_PROC_DOMAIN_LOOKUP_BY_UUID to refresh the 'id'
of the domain, following the pattern used in the previous
virDomainCreate method impl.

The remote protocol for virDomainCreateWithFlags though did
actually fix the design flaw in virDomainCreate, by directly
returning the new domain info. For some reason, this data was
never used. So we can just use that data now instead.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/remote/remote_driver.c | 20 
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index e2764e2..91ecfe5 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2415,8 +2415,7 @@ remoteDomainCreateWithFlags(virDomainPtr dom, unsigned 
int flags)
 int rv = -1;
 struct private_data *priv = dom-conn-privateData;
 remote_domain_create_with_flags_args args;
-remote_domain_lookup_by_uuid_args args2;
-remote_domain_lookup_by_uuid_ret ret2;
+remote_domain_create_with_flags_args ret;
 
 remoteDriverLock(priv);
 
@@ -2425,23 +2424,12 @@ remoteDomainCreateWithFlags(virDomainPtr dom, unsigned 
int flags)
 
 if (call(dom-conn, priv, 0, REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS,
  (xdrproc_t)xdr_remote_domain_create_with_flags_args, (char 
*)args,
- (xdrproc_t)xdr_void, (char *)NULL) == -1) {
+ (xdrproc_t)xdr_remote_domain_create_with_flags_ret, (char *)ret) 
== -1) {
 goto done;
 }
 
-/* Need to do a lookup figure out ID of newly started guest, because
- * bug in design of REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS means we aren't 
getting
- * it returned.
- */
-memcpy(args2.uuid, dom-uuid, VIR_UUID_BUFLEN);
-memset(ret2, 0, sizeof(ret2));
-if (call(dom-conn, priv, 0, REMOTE_PROC_DOMAIN_LOOKUP_BY_UUID,
- (xdrproc_t) xdr_remote_domain_lookup_by_uuid_args, (char *) 
args2,
- (xdrproc_t) xdr_remote_domain_lookup_by_uuid_ret, (char *) ret2) 
== -1)
-goto done;
-
-dom-id = ret2.dom.id;
-xdr_free((xdrproc_t) xdr_remote_domain_lookup_by_uuid_ret, (char *) 
ret2);
+dom-id = ret.dom.id;
+xdr_free((xdrproc_t) xdr_remote_domain_create_with_flags_ret, (char *) 
ret);
 rv = 0;
 
 done:
-- 
1.8.1.4

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


[libvirt] [PATCH 5/6] Enable FD passing when starting guests with virsh

2013-07-12 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Add a --pass-fds N,M,... arg to the virsh start/create
methods. This allows pre-opened file descriptors from the
shell to be passed on into the guest

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 tools/virsh-domain.c | 82 +---
 tools/virsh.pod  | 13 -
 2 files changed, 90 insertions(+), 5 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index c08b0e9..606bcdf 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3256,9 +3256,61 @@ static const vshCmdOptDef opts_start[] = {
  .type = VSH_OT_BOOL,
  .help = N_(force fresh boot by discarding any managed save)
 },
+{.name = pass-fds,
+ .type = VSH_OT_STRING,
+ .help = N_(pass file descriptors N,M,... to the guest)
+},
 {.name = NULL}
 };
 
+static int
+cmdStartGetFDs(vshControl *ctl,
+   const vshCmd *cmd,
+   size_t *nfdsret,
+   int **fdsret)
+{
+const char *fdopt;
+char **fdlist = NULL;
+int *fds = NULL;
+size_t nfds = 0;
+size_t i;
+
+*nfdsret = 0;
+*fdsret = NULL;
+
+if (vshCommandOptString(cmd, pass-fds, fdopt) = 0)
+return 0;
+
+if (!(fdlist = virStringSplit(fdopt, ,, -1))) {
+vshError(ctl, _(Unable to split FD list '%s'), fdopt);
+return -1;
+}
+
+for (i = 0; fdlist[i] != NULL; i++) {
+int fd;
+if (virStrToLong_i(fdlist[i], NULL, 10, fd)  0) {
+vshError(ctl, _(Unable to parse FD number '%s'), fdlist[i]);
+goto error;
+}
+if (VIR_EXPAND_N(fds, nfds, 1)  0) {
+vshError(ctl, %s, _(Unable to allocate FD list));
+goto error;
+}
+fds[nfds - 1] = fd;
+}
+
+virStringFreeList(fdlist);
+
+*fdsret = fds;
+*nfdsret = nfds;
+return 0;
+
+error:
+virStringFreeList(fdlist);
+VIR_FREE(fds);
+return -1;
+}
+
 static bool
 cmdStart(vshControl *ctl, const vshCmd *cmd)
 {
@@ -3269,6 +3321,8 @@ cmdStart(vshControl *ctl, const vshCmd *cmd)
 #endif
 unsigned int flags = VIR_DOMAIN_NONE;
 int rc;
+size_t nfds = 0;
+int *fds = NULL;
 
 if (!(dom = vshCommandOptDomainBy(ctl, cmd, NULL,
   VSH_BYNAME | VSH_BYUUID)))
@@ -3280,6 +3334,9 @@ cmdStart(vshControl *ctl, const vshCmd *cmd)
 return false;
 }
 
+if (cmdStartGetFDs(ctl, cmd, nfds, fds)  0)
+return false;
+
 if (vshCommandOptBool(cmd, paused))
 flags |= VIR_DOMAIN_START_PAUSED;
 if (vshCommandOptBool(cmd, autodestroy))
@@ -3291,7 +3348,9 @@ cmdStart(vshControl *ctl, const vshCmd *cmd)
 
 /* We can emulate force boot, even for older servers that reject it.  */
 if (flags  VIR_DOMAIN_START_FORCE_BOOT) {
-if (virDomainCreateWithFlags(dom, flags) == 0)
+if ((nfds ?
+ virDomainCreateWithFiles(dom, nfds, fds, flags) :
+ virDomainCreateWithFlags(dom, flags)) == 0)
 goto started;
 if (last_error-code != VIR_ERR_NO_SUPPORT 
 last_error-code != VIR_ERR_INVALID_ARG) {
@@ -3313,8 +3372,9 @@ cmdStart(vshControl *ctl, const vshCmd *cmd)
 }
 
 /* Prefer older API unless we have to pass a flag.  */
-if ((flags ? virDomainCreateWithFlags(dom, flags)
- : virDomainCreate(dom))  0) {
+if ((nfds ? virDomainCreateWithFiles(dom, nfds, fds, flags) :
+ (flags ? virDomainCreateWithFlags(dom, flags)
+  : virDomainCreate(dom)))  0) {
 vshError(ctl, _(Failed to start domain %s), virDomainGetName(dom));
 goto cleanup;
 }
@@ -3331,6 +3391,7 @@ started:
 
 cleanup:
 virDomainFree(dom);
+VIR_FREE(fds);
 return ret;
 }
 
@@ -6397,6 +6458,10 @@ static const vshCmdOptDef opts_create[] = {
  .type = VSH_OT_BOOL,
  .help = N_(automatically destroy the guest when virsh disconnects)
 },
+{.name = pass-fds,
+ .type = VSH_OT_STRING,
+ .help = N_(pass file descriptors N,M,... to the guest)
+},
 {.name = NULL}
 };
 
@@ -6411,6 +6476,8 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd)
 bool console = vshCommandOptBool(cmd, console);
 #endif
 unsigned int flags = VIR_DOMAIN_NONE;
+size_t nfds = 0;
+int *fds = NULL;
 
 if (vshCommandOptStringReq(ctl, cmd, file, from)  0)
 return false;
@@ -6418,12 +6485,18 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd)
 if (virFileReadAll(from, VSH_MAX_XML_FILE, buffer)  0)
 return false;
 
+if (cmdStartGetFDs(ctl, cmd, nfds, fds)  0)
+return false;
+
 if (vshCommandOptBool(cmd, paused))
 flags |= VIR_DOMAIN_START_PAUSED;
 if (vshCommandOptBool(cmd, autodestroy))
 flags |= VIR_DOMAIN_START_AUTODESTROY;
 
-dom = virDomainCreateXML(ctl-conn, buffer, flags);
+if (nfds)
+dom = virDomainCreateXMLWithFiles(ctl-conn, buffer, nfds, fds, flags);
+else
+dom = 

[libvirt] [PATCH 6/6] Merge virCommandPreserveFD / virCommandTransferFD

2013-07-12 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Merge the virCommandPreserveFD / virCommandTransferFD methods
into a single virCommandPasFD method, and use a new
VIR_COMMAND_PASS_FD_CLOSE_PARENT to indicate their difference
in behaviour

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/fdstream.c   |   3 +-
 src/libvirt_private.syms |   3 +-
 src/lxc/lxc_process.c|   6 +-
 src/qemu/qemu_command.c  |  16 +++--
 src/uml/uml_conf.c   |   3 +-
 src/util/vircommand.c| 159 ++-
 src/util/vircommand.h|  13 ++--
 tests/commandtest.c  |   5 +-
 8 files changed, 105 insertions(+), 103 deletions(-)

diff --git a/src/fdstream.c b/src/fdstream.c
index 2ee9bd8..10c7c2a 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -648,7 +648,8 @@ virFDStreamOpenFileInternal(virStreamPtr st,
path,
NULL);
 virCommandAddArgFormat(cmd, %llu, length);
-virCommandTransferFD(cmd, fd);
+virCommandPassFD(cmd, fd,
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
 virCommandAddArgFormat(cmd, %d, fd);
 
 if ((oflags  O_ACCMODE) == O_RDONLY) {
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0ab7632..a9e702c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1228,7 +1228,7 @@ virCommandNew;
 virCommandNewArgList;
 virCommandNewArgs;
 virCommandNonblockingFDs;
-virCommandPreserveFD;
+virCommandPassFD;
 virCommandRequireHandshake;
 virCommandRun;
 virCommandRunAsync;
@@ -1249,7 +1249,6 @@ virCommandSetSELinuxLabel;
 virCommandSetUID;
 virCommandSetWorkingDirectory;
 virCommandToString;
-virCommandTransferFD;
 virCommandWait;
 virCommandWriteArgLog;
 virFork;
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index dd908b8..54eb728 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -853,13 +853,13 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
 for (i = 0; i  nttyFDs; i++) {
 virCommandAddArg(cmd, --console);
 virCommandAddArgFormat(cmd, %d, ttyFDs[i]);
-virCommandPreserveFD(cmd, ttyFDs[i]);
+virCommandPassFD(cmd, ttyFDs[i], 0);
 }
 
 for (i = 0; i  nfiles; i++) {
 virCommandAddArg(cmd, --passfd);
 virCommandAddArgFormat(cmd, %d, files[i]);
-virCommandPreserveFD(cmd, files[i], 0);
+virCommandPassFD(cmd, files[i], 0);
 }
 
 virCommandAddArgPair(cmd, --security,
@@ -873,7 +873,7 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
 virCommandAddArgList(cmd, --veth, veths[i], NULL);
 }
 
-virCommandPreserveFD(cmd, handshakefd);
+virCommandPassFD(cmd, handshakefd, 0);
 
 return cmd;
 cleanup:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 879aed8..4f126e7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -245,7 +245,8 @@ static int 
qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr cfg,
 virCommandAddArgFormat(cmd, --use-vnet);
 virCommandAddArgFormat(cmd, --br=%s, brname);
 virCommandAddArgFormat(cmd, --fd=%d, pair[1]);
-virCommandTransferFD(cmd, pair[1]);
+virCommandPassFD(cmd, pair[1],
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
 virCommandClearCaps(cmd);
 #ifdef CAP_NET_ADMIN
 virCommandAllowCap(cmd, CAP_NET_ADMIN);
@@ -6524,13 +6525,15 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
 }
 
 for (i = 0; i  tapfdSize; i++) {
-virCommandTransferFD(cmd, tapfd[i]);
+virCommandPassFD(cmd, tapfd[i],
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
 if (virAsprintf(tapfdName[i], %d, tapfd[i])  0)
 goto cleanup;
 }
 
 for (i = 0; i  vhostfdSize; i++) {
-virCommandTransferFD(cmd, vhostfd[i]);
+virCommandPassFD(cmd, vhostfd[i],
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
 if (virAsprintf(vhostfdName[i], %d, vhostfd[i])  0)
 goto cleanup;
 }
@@ -8301,7 +8304,8 @@ qemuBuildCommandLine(virConnectPtr conn,
 goto error;
 }
 
-virCommandTransferFD(cmd, configfd);
+virCommandPassFD(cmd, configfd,
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
 }
 }
 virCommandAddArg(cmd, -device);
@@ -8367,7 +8371,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 } else if (STREQ(migrateFrom, stdio)) {
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) {
 virCommandAddArgFormat(cmd, fd:%d, migrateFd);
-virCommandPreserveFD(cmd, migrateFd);
+virCommandPassFD(cmd, migrateFd, 0);
 } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_EXEC)) {
 virCommandAddArg(cmd, exec:cat);
 

Re: [libvirt] [PATCH] add console support in libxl

2013-07-12 Thread Jim Fehlig

On 07/04/2013 05:58 AM, Bamvor Jian Zhang wrote:

this patch introduce the console api in libxl driver for both pv and
hvm guest.  and import and update the libxlMakeChrdevStr function
which was deleted in commit dfa1e1dd.

Signed-off-by: Bamvor Jian Zhang bjzh...@suse.com
---
  src/libxl/libxl_conf.c   |  97 ++
  src/libxl/libxl_conf.h   |   3 ++
  src/libxl/libxl_driver.c | 119 +++
  3 files changed, 219 insertions(+)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index e170357..08095bc 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -332,6 +332,99 @@ error:
  }
  
  static int

+libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
+{
+const char *type = virDomainChrTypeToString(def-source.type);
+int ret;
+
+if (!type) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   %s, _(unexpected chr device type));
+return -1;
+}
+
+switch (def-source.type) {
+case VIR_DOMAIN_CHR_TYPE_NULL:
+case VIR_DOMAIN_CHR_TYPE_STDIO:
+case VIR_DOMAIN_CHR_TYPE_VC:
+case VIR_DOMAIN_CHR_TYPE_PTY:
+if (virAsprintf(buf, %s, type)  0) {
+virReportOOMError();


This will need rebased now that Michal's Introduce OOM reporting to 
virAsprintf patchset is committed.


https://www.redhat.com/archives/libvir-list/2013-July/msg00506.html


+return -1;
+}
+break;
+
+case VIR_DOMAIN_CHR_TYPE_FILE:
+case VIR_DOMAIN_CHR_TYPE_PIPE:
+if (virAsprintf(buf, %s:%s, type,
+def-source.data.file.path)  0) {
+virReportOOMError();
+return -1;
+}
+break;
+
+case VIR_DOMAIN_CHR_TYPE_DEV:
+if (virAsprintf(buf, %s, def-source.data.file.path)  0) {
+virReportOOMError();
+return -1;
+}
+break;
+case VIR_DOMAIN_CHR_TYPE_UDP: {
+const char *connectHost = def-source.data.udp.connectHost;
+const char *bindHost = def-source.data.udp.bindHost;
+const char *bindService  = def-source.data.udp.bindService;
+
+if (connectHost == NULL)
+connectHost = ;
+if (bindHost == NULL)
+bindHost = ;
+if (bindService == NULL)
+bindService = 0;
+
+ret = virAsprintf(buf, udp:%s:%s@%s:%s,
+  connectHost,
+  def-source.data.udp.connectService,
+  bindHost,
+  bindService);
+if ( ret  0) {


Extra space between '(' and 'ret', caught by 'make syntax-check'.


+virReportOOMError();
+return -1;
+}
+break;
+}
+case VIR_DOMAIN_CHR_TYPE_TCP:
+if (def-source.data.tcp.protocol == 
VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET) {
+ret = virAsprintf(buf, telnet:%s:%s%s,
+  def-source.data.tcp.host,
+  def-source.data.tcp.service,
+  def-source.data.tcp.listen ? ,server,nowait : 
);
+} else {
+ret = virAsprintf(buf, tcp:%s:%s%s,
+  def-source.data.tcp.host,
+  def-source.data.tcp.service,
+  def-source.data.tcp.listen ? ,server,nowait : 
);
+}
+if ( ret  0) {


Extra space here too.


+virReportOOMError();
+return -1;
+}
+break;
+
+case VIR_DOMAIN_CHR_TYPE_UNIX:
+ret = virAsprintf(buf, unix:%s%s,
+  def-source.data.nix.path,
+  def-source.data.nix.listen ? ,server,nowait : 
);
+if ( ret  0) {


And here.

BTW, do all of these types work with Xen?  I've only tested with type 'pty', 
which works fine with both pv and hvm guests.



+virReportOOMError();
+return -1;
+}
+break;
+}
+
+return 0;
+}
+
+static int
  libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config)
  {
  libxl_domain_build_info *b_info = d_config-b_info;
@@ -404,6 +497,10 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, 
libxl_domain_config *d_config)
  if (VIR_STRDUP(b_info-u.hvm.boot, bootorder)  0)
  goto error;
  
+if (def-nserials 

+(libxlMakeChrdevStr(def-serials[0], b_info-u.hvm.serial)  0))
+goto error;
+
  /*
   * The following comment and calculation were taken directly from
   * libxenlight's internal function libxl_get_required_shadow_memory():
diff --git a/src/libxl/libxl_conf.h 

Re: [libvirt] [PATCH v2] qemu: don't use deprecated -no-kvm-pit-reinjection

2013-07-12 Thread Eric Blake
On 07/12/2013 08:54 AM, Ján Tomko wrote:
 Since qemu-kvm 1.1 [1] (since 1.3. in upstream QEMU [2])
 '-no-kvm-pit-reinjection' has been deprecated.
 Use -device kvm-pit,lost_tick_policy=discard instead.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=978719
 
 [1] http://git.kernel.org/cgit/virt/kvm/qemu-kvm.git/commit/?id=4e4fa39
 [2] http://git.qemu.org/?p=qemu.git;a=commitdiff;h=c21fb4f
 ---

 @@ -2446,13 +2448,12 @@ virQEMUCapsInitArchQMPBasic(virQEMUCapsPtr qemuCaps,
  
  /*
   * Currently only x86_64 and i686 support PCI-multibus,
 - * -no-acpi and -no-kvm-pit-reinjection.
 + * -no-acpi
   */
  if (qemuCaps-arch == VIR_ARCH_X86_64 ||
  qemuCaps-arch == VIR_ARCH_I686) {
  virQEMUCapsSet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS);
  virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_ACPI);
 -virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_KVM_PIT);

This part seems dubious, like it might break upgrades.  If an older
libvirt says that qemu has a feature, and a VM is running, then when you
upgrade libvirtd and the feature is no longer provided by qemu, then
libvirtd may silently drop the domain definition because it can't find a
qemu binary that supports the same features as were required by the
older libvirtd.  It shouldn't hurt to leave this cap bit set, even if
the rest of this patch is about avoiding the need to rely on this cap bit.

Otherwise, the patch looks okay to me.

-- 
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 v3 01/10] Add qemuMonitorJSONGetObjectListPaths() method for QMP qom-list command

2013-07-12 Thread Eric Blake
On 07/12/2013 06:34 AM, Daniel P. Berrange wrote:
 On Thu, Jul 11, 2013 at 07:55:51PM -0400, John Ferlan wrote:
 Add a new qemuMonitorJSONGetObjectListPaths() method to support invocation
 of the 'qom-list' JSON monitor command with a provided path.

 The returned list of paired data fields of name and type that can
 be used to peruse QOM configuration data and eventually utilize for the
 balloon statistics.

 The test does a {execute:qom-list, arguments: { path: /}} which
 returns {return: [{name: machine, type: childcontainer},
 {name: type, type: string}]} resulting in a return of an array
 of 2 elements with [0].name=machine, [0].type=childcontainer.  The [1]
 entry appears to be a header that could be used some day via a command such
 as virsh qemuobject --list to format output.
 ---
  src/qemu/qemu_monitor_json.c | 98 
 
  src/qemu/qemu_monitor_json.h | 16 
  tests/qemumonitorjsontest.c  | 79 +++
  3 files changed, 193 insertions(+)
 
 ACK

I'm still not convinced we need to touch the .h file; why can't the new
function be static and the data types be local to the .c file?

-- 
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] conf: reject pci-root controllers with non-zero indexes

2013-07-12 Thread Eric Blake
On 07/12/2013 07:08 AM, Ján Tomko wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=981261
 ---
  src/conf/domain_conf.c | 7 +++
  1 file changed, 7 insertions(+)

ACK

 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 5f0366e..602c9a6 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -5668,6 +5668,13 @@ virDomainControllerDefParseXML(xmlNodePtr node,
   have an address));
  goto error;
  }
 +if (def-idx != 0) {
 +virReportError(VIR_ERR_XML_ERROR, %s,
 +   _(pci-root controller should have 
 + index 0));
 +goto error;
 +}
 +
  }
  
  default:
 

-- 
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 v2] qemu: don't use deprecated -no-kvm-pit-reinjection

2013-07-12 Thread Daniel P. Berrange
On Fri, Jul 12, 2013 at 09:47:57AM -0600, Eric Blake wrote:
 On 07/12/2013 08:54 AM, Ján Tomko wrote:
  Since qemu-kvm 1.1 [1] (since 1.3. in upstream QEMU [2])
  '-no-kvm-pit-reinjection' has been deprecated.
  Use -device kvm-pit,lost_tick_policy=discard instead.
  
  https://bugzilla.redhat.com/show_bug.cgi?id=978719
  
  [1] http://git.kernel.org/cgit/virt/kvm/qemu-kvm.git/commit/?id=4e4fa39
  [2] http://git.qemu.org/?p=qemu.git;a=commitdiff;h=c21fb4f
  ---
 
  @@ -2446,13 +2448,12 @@ virQEMUCapsInitArchQMPBasic(virQEMUCapsPtr qemuCaps,
   
   /*
* Currently only x86_64 and i686 support PCI-multibus,
  - * -no-acpi and -no-kvm-pit-reinjection.
  + * -no-acpi
*/
   if (qemuCaps-arch == VIR_ARCH_X86_64 ||
   qemuCaps-arch == VIR_ARCH_I686) {
   virQEMUCapsSet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS);
   virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_ACPI);
  -virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_KVM_PIT);
 
 This part seems dubious, like it might break upgrades.  If an older
 libvirt says that qemu has a feature, and a VM is running, then when you
 upgrade libvirtd and the feature is no longer provided by qemu, then
 libvirtd may silently drop the domain definition because it can't find a
 qemu binary that supports the same features as were required by the
 older libvirtd.  It shouldn't hurt to leave this cap bit set, even if
 the rest of this patch is about avoiding the need to rely on this cap bit.

That shouldn't happen I think. For any runing guest, we have recorded
the original capabilities in the domain status XML file. So any caps
we detect against QEMU binaries upon restart will only impact newly
started guests.

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 03/10] Add qemuMonitorJSONSetObjectProperty() method for QMP qom-set command

2013-07-12 Thread Eric Blake
On 07/12/2013 06:35 AM, Daniel P. Berrange wrote:
 On Thu, Jul 11, 2013 at 07:55:53PM -0400, John Ferlan wrote:
 Add a new qemuMonitorJSONSetObjectProperty() method to support invocation
 of the 'qom-set' JSON monitor command with a provided path, property, and
 expected data type to set.

 The test code uses the same /machine/i440fx property as the get test and
 attempts to set the realized property to true (which it should be set
 at anyway).
 ---
  src/qemu/qemu_monitor_json.c | 62 
 
  src/qemu/qemu_monitor_json.h |  6 +
  tests/qemumonitorjsontest.c  | 59 +
  3 files changed, 127 insertions(+)
 
 ACK

Again, shouldn't this function be marked static, and the .h file left alone?

-- 
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 v2] qemu: don't use deprecated -no-kvm-pit-reinjection

2013-07-12 Thread Eric Blake
On 07/12/2013 09:53 AM, Daniel P. Berrange wrote:

  if (qemuCaps-arch == VIR_ARCH_X86_64 ||
  qemuCaps-arch == VIR_ARCH_I686) {
  virQEMUCapsSet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS);
  virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_ACPI);
 -virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_KVM_PIT);

 This part seems dubious, like it might break upgrades.  If an older
 libvirt says that qemu has a feature, and a VM is running, then when you
 upgrade libvirtd and the feature is no longer provided by qemu, then
 libvirtd may silently drop the domain definition because it can't find a
 qemu binary that supports the same features as were required by the
 older libvirtd.  It shouldn't hurt to leave this cap bit set, even if
 the rest of this patch is about avoiding the need to rely on this cap bit.
 
 That shouldn't happen I think. For any runing guest, we have recorded
 the original capabilities in the domain status XML file. So any caps
 we detect against QEMU binaries upon restart will only impact newly
 started guests.

I seem to recall difficulties in the past, such as when developing on a
RHEL machine, where the upstream and the downstream list of cap bits are
different, and where restarting upstream libvirtd had problems with
domains already started by downstream libvirtd.  I'd feel better if this
were explicitly tested (easy enough to do - build libvirtd without this
patch, start a domain, rebuild libvirtd with the patch, restart
libvirtd, and see if virsh can still control the domain).

-- 
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 v2] qemu: don't use deprecated -no-kvm-pit-reinjection

2013-07-12 Thread Eric Blake
On 07/12/2013 10:00 AM, Eric Blake wrote:

 That shouldn't happen I think. For any runing guest, we have recorded
 the original capabilities in the domain status XML file. So any caps
 we detect against QEMU binaries upon restart will only impact newly
 started guests.
 
 I seem to recall difficulties in the past, such as when developing on a
 RHEL machine, where the upstream and the downstream list of cap bits are
 different, and where restarting upstream libvirtd had problems with
 domains already started by downstream libvirtd.  I'd feel better if this
 were explicitly tested (easy enough to do - build libvirtd without this
 patch, start a domain, rebuild libvirtd with the patch, restart
 libvirtd, and see if virsh can still control the domain).

Or maybe the difficulty I'm recalling is the case of _unknown_ cap bits.
 You may be right that a known, but differently-set bit, behaves nicer
than parsing the XML with a bit that is completely unknown.

-- 
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 v3 03/10] Add qemuMonitorJSONSetObjectProperty() method for QMP qom-set command

2013-07-12 Thread John Ferlan
On 07/12/2013 11:57 AM, Eric Blake wrote:
 On 07/12/2013 06:35 AM, Daniel P. Berrange wrote:
 On Thu, Jul 11, 2013 at 07:55:53PM -0400, John Ferlan wrote:
 Add a new qemuMonitorJSONSetObjectProperty() method to support invocation
 of the 'qom-set' JSON monitor command with a provided path, property, and
 expected data type to set.

 The test code uses the same /machine/i440fx property as the get test and
 attempts to set the realized property to true (which it should be set
 at anyway).
 ---
  src/qemu/qemu_monitor_json.c | 62 
 
  src/qemu/qemu_monitor_json.h |  6 +
  tests/qemumonitorjsontest.c  | 59 +
  3 files changed, 127 insertions(+)

 ACK
 
 Again, shouldn't this function be marked static, and the .h file left alone?
 

I can go either way on this - without the .h changes, we don't have the
test though.  So whichever way is preferred - I'll go with it...

John

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


Re: [libvirt] [PATCH v3 03/10] Add qemuMonitorJSONSetObjectProperty() method for QMP qom-set command

2013-07-12 Thread Eric Blake
On 07/12/2013 10:34 AM, John Ferlan wrote:
 On 07/12/2013 11:57 AM, Eric Blake wrote:
 On 07/12/2013 06:35 AM, Daniel P. Berrange wrote:
 On Thu, Jul 11, 2013 at 07:55:53PM -0400, John Ferlan wrote:
 Add a new qemuMonitorJSONSetObjectProperty() method to support invocation
 of the 'qom-set' JSON monitor command with a provided path, property, and
 expected data type to set.

 The test code uses the same /machine/i440fx property as the get test and
 attempts to set the realized property to true (which it should be set
 at anyway).
 ---
  src/qemu/qemu_monitor_json.c | 62 
 
  src/qemu/qemu_monitor_json.h |  6 +
  tests/qemumonitorjsontest.c  | 59 
 +
  3 files changed, 127 insertions(+)

 ACK

 Again, shouldn't this function be marked static, and the .h file left alone?

 
 I can go either way on this - without the .h changes, we don't have the
 test though.  So whichever way is preferred - I'll go with it...

Ah, so the test case is the reason you are exporting the file.  That's
reason enough to leave the patch body as-is, but do amend the commit
message to mention this.  It might also be worth a comment in the .h
that the function is public only for the testsuite, so we aren't tempted
to use it from qemu_driver.c later.

-- 
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] ANNOUNCE: libvirt 1.0.5.4 maintenance release

2013-07-12 Thread Cole Robinson
libvirt 1.0.5.4 maintenance release is now available. This is
libvirt 1.0.5 with additional bugfixes that have accumulated
upstream since the initial release.

This release can be downloaded at:

http://libvirt.org/sources/stable_updates/libvirt-1.0.5.4.tar.gz

Changes in this version:

* qemu: fix double free in qemuMigrationPrepareDirect

For info about past maintenance releases, see:

http://wiki.libvirt.org/page/Maintenance_Releases

Thanks,
Cole

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


[libvirt] [PATCH v5 1/5] qemuBuildChrDeviceCommandLine: Don't leak devstr

2013-07-12 Thread Michal Privoznik
It's caller's responsibility to free return value of
qemuBuildChrDeviceStr().
---
 src/qemu/qemu_command.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0e517f2..d6ef9cd 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6608,6 +6608,7 @@ qemuBuildChrDeviceCommandLine(virCommandPtr cmd,
 return -1;
 
 virCommandAddArgList(cmd, -device, devstr, NULL);
+VIR_FREE(devstr);
 return 0;
 }
 
-- 
1.8.1.5

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


[libvirt] [PATCH v5 0/5] Chardev hotplug

2013-07-12 Thread Michal Privoznik
I've pushed all ACKed patches from previous rounds. So now just the rest.

Michal Privoznik (5):
  qemuBuildChrDeviceCommandLine: Don't leak devstr
  domain_conf: Auto fill chardev port
  qemu: Implement chardev hotplug on config level
  qemu: Implement chardev hotplug on live level
  qemuhotplugtest: Introduce test for chardev hotplug

 src/conf/domain_conf.c |  52 +-
 src/conf/domain_conf.h |   3 +
 src/libvirt_private.syms   |   1 +
 src/qemu/qemu_command.c|  39 -
 src/qemu/qemu_driver.c |  42 -
 src/qemu/qemu_hotplug.c| 175 +++
 src/qemu/qemu_hotplug.h|  13 ++
 tests/qemuhotplugtest.c| 194 +
 .../qemuhotplug-console-virtio.xml |   5 +
 .../qemuxml2argv-console-compat-2.xml  | 122 +
 10 files changed, 597 insertions(+), 49 deletions(-)
 create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-console-virtio.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml

-- 
1.8.1.5

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


[libvirt] [PATCH v5 5/5] qemuhotplugtest: Introduce test for chardev hotplug

2013-07-12 Thread Michal Privoznik
The test is currently testing just device update function. However,
chardev hotplug is implemented just for device attach and detach. This
fact means, the test needs to be rewritten (the majority of the code is
still shared). Moreover, we are now able to pass VM among multiple test
runs. So for instance, while we add a device in the first run, we can
remove it in the second run.
---
 tests/qemuhotplugtest.c| 194 +
 .../qemuhotplug-console-virtio.xml |   5 +
 .../qemuxml2argv-console-compat-2.xml  | 122 +
 3 files changed, 284 insertions(+), 37 deletions(-)
 create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-console-virtio.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml

diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 15d70d2..d4971c2 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -33,11 +33,20 @@
 
 static virQEMUDriver driver;
 
+enum {
+ATTACH,
+DETACH,
+UPDATE
+};
+
 struct qemuHotplugTestData {
 const char *domain_filename;
 const char *device_filename;
 bool fail;
 const char *const *mon;
+int action;
+bool keep;
+virDomainObjPtr vm;
 };
 
 static int
@@ -46,6 +55,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
  const char *filename)
 {
 int ret = -1;
+qemuDomainObjPrivatePtr priv = NULL;
 
 if (!(*vm = virDomainObjNew(xmlopt)))
 goto cleanup;
@@ -57,12 +67,85 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
  0)))
 goto cleanup;
 
+priv = (*vm)-privateData;
+
+if (!(priv-qemuCaps = virQEMUCapsNew()))
+goto cleanup;
+
+/* for attach  detach qemu must support -device */
+virQEMUCapsSet(priv-qemuCaps, QEMU_CAPS_DEVICE);
+
 ret = 0;
 cleanup:
 return ret;
 }
 
 static int
+testQemuHotplugAttach(virDomainObjPtr vm,
+  virDomainDeviceDefPtr dev)
+{
+int ret = -1;
+
+switch (dev-type) {
+case VIR_DOMAIN_DEVICE_CHR:
+ret = qemuDomainAttachChrDevice(driver, vm, dev-data.chr);
+break;
+default:
+if (virTestGetVerbose())
+fprintf(stderr, device type '%s' cannot be attached,
+virDomainDeviceTypeToString(dev-type));
+break;
+}
+
+return ret;
+}
+
+static int
+testQemuHotplugDetach(virDomainObjPtr vm,
+  virDomainDeviceDefPtr dev)
+{
+int ret = -1;
+
+switch (dev-type) {
+case VIR_DOMAIN_DEVICE_CHR:
+ret = qemuDomainDetachChrDevice(driver, vm, dev-data.chr);
+break;
+default:
+if (virTestGetVerbose())
+fprintf(stderr, device type '%s' cannot be attached,
+virDomainDeviceTypeToString(dev-type));
+break;
+}
+
+return ret;
+}
+
+static int
+testQemuHotplugUpdate(virDomainObjPtr vm,
+  virDomainDeviceDefPtr dev)
+{
+int ret = -1;
+
+/* XXX Ideally, we would call qemuDomainUpdateDeviceLive here.  But that
+ * would require us to provide virConnectPtr and virDomainPtr (they're used
+ * in case of updating a disk device. So for now, we will proceed with
+ * breaking the function into pieces. If we ever learn how to fake those
+ * required object, we can replace this code then. */
+switch (dev-type) {
+case VIR_DOMAIN_DEVICE_GRAPHICS:
+ret = qemuDomainChangeGraphics(driver, vm, dev-data.graphics);
+break;
+default:
+if (virTestGetVerbose())
+fprintf(stderr, device type '%s' cannot be updated,
+virDomainDeviceTypeToString(dev-type));
+break;
+}
+
+return ret;
+}
+
+static int
 testQemuHotplug(const void *data)
 {
 int ret = -1;
@@ -72,6 +155,7 @@ testQemuHotplug(const void *data)
 char *device_xml = NULL;
 const char *const *tmp;
 bool fail = test-fail;
+bool keep = test-keep;
 virDomainObjPtr vm = NULL;
 virDomainDeviceDefPtr dev = NULL;
 virCapsPtr caps = NULL;
@@ -87,17 +171,18 @@ testQemuHotplug(const void *data)
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
 goto cleanup;
 
-if (qemuHotplugCreateObjects(driver.xmlopt, vm, domain_filename)  0)
-goto cleanup;
-
-priv = vm-privateData;
+if (test-vm) {
+vm = test-vm;
+} else {
+if (qemuHotplugCreateObjects(driver.xmlopt, vm, domain_filename)  0)
+goto cleanup;
+}
 
 if (virtTestLoadFile(device_filename, device_xml)  0)
 goto cleanup;
 
 if (!(dev = virDomainDeviceDefParse(device_xml, vm-def,
-caps, driver.xmlopt,
-VIR_DOMAIN_XML_INACTIVE)))
+caps, driver.xmlopt, 0)))
 goto cleanup;
 
 /* Now is the best time to feed the 

[libvirt] [PATCH v5 4/5] qemu: Implement chardev hotplug on live level

2013-07-12 Thread Michal Privoznik
Since previous patches has prepared everything for us, we may now
implement live hotplug of a character device.
---
 src/qemu/qemu_command.c |  38 +-
 src/qemu/qemu_driver.c  |  26 ++--
 src/qemu/qemu_hotplug.c | 103 
 src/qemu/qemu_hotplug.h |   6 +++
 4 files changed, 168 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d6ef9cd..735f300 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -863,8 +863,41 @@ qemuAssignDeviceControllerAlias(virDomainControllerDefPtr 
controller)
 return virAsprintf(controller-info.alias, %s%d, prefix, 
controller-idx);
 }
 
+static ssize_t
+qemuGetNextChrDevIndex(virDomainDefPtr def,
+   virDomainChrDefPtr chr,
+   const char *prefix)
+{
+virDomainChrDefPtr **arrPtr;
+size_t *cntPtr;
+size_t i;
+ssize_t idx = 0;
+const char *prefix2 = NULL;
+
+if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE)
+prefix2 = serial;
+
+virDomainChrGetDomainPtrs(def, chr, arrPtr, cntPtr);
+
+for (i = 0; i  *cntPtr; i++) {
+ssize_t thisidx;
+if (((thisidx = qemuDomainDeviceAliasIndex((*arrPtr)[i]-info, 
prefix))  0) 
+(prefix2 
+ (thisidx = qemuDomainDeviceAliasIndex((*arrPtr)[i]-info, 
prefix2))  0)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Unable to determine device index for character 
device));
+return -1;
+}
+if (thisidx = idx)
+idx = thisidx + 1;
+}
+
+return idx;
+}
+
+
 int
-qemuAssignDeviceChrAlias(virDomainDefPtr def ATTRIBUTE_UNUSED,
+qemuAssignDeviceChrAlias(virDomainDefPtr def,
  virDomainChrDefPtr chr,
  ssize_t idx)
 {
@@ -891,6 +924,9 @@ qemuAssignDeviceChrAlias(virDomainDefPtr def 
ATTRIBUTE_UNUSED,
 return -1;
 }
 
+if (idx == -1  (idx = qemuGetNextChrDevIndex(def, chr, prefix))  0)
+return -1;
+
 return virAsprintf(chr-info.alias, %s%zd, prefix, idx);
 }
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 852db8b..b4a668a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6390,6 +6390,13 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 dev-data.redirdev = NULL;
 break;
 
+case VIR_DOMAIN_DEVICE_CHR:
+ret = qemuDomainAttachChrDevice(driver, vm,
+dev-data.chr);
+if (!ret)
+dev-data.chr = NULL;
+break;
+
 default:
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_(live attach of device '%s' is not supported),
@@ -6477,6 +6484,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
 case VIR_DOMAIN_DEVICE_HOSTDEV:
 ret = qemuDomainDetachHostDevice(driver, vm, dev);
 break;
+case VIR_DOMAIN_DEVICE_CHR:
+ret = qemuDomainDetachChrDevice(driver, vm, dev-data.chr);
+break;
 default:
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_(live detach of device '%s' is not supported),
@@ -6886,7 +6896,7 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, 
const char *xml,
 virDomainDefPtr vmdef = NULL;
 virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
 int ret = -1;
-unsigned int affect;
+unsigned int affect, parse_flags = 0;
 virQEMUCapsPtr qemuCaps = NULL;
 qemuDomainObjPrivatePtr priv;
 virQEMUDriverConfigPtr cfg = NULL;
@@ -6934,9 +6944,13 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, 
const char *xml,
  goto endjob;
 }
 
+if ((flags  VIR_DOMAIN_AFFECT_CONFIG) 
+!(flags  VIR_DOMAIN_AFFECT_LIVE))
+parse_flags |= VIR_DOMAIN_XML_INACTIVE;
+
 dev = dev_copy = virDomainDeviceDefParse(xml, vm-def,
  caps, driver-xmlopt,
- VIR_DOMAIN_XML_INACTIVE);
+ parse_flags);
 if (dev == NULL)
 goto endjob;
 
@@ -7164,7 +7178,7 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, 
const char *xml,
 virDomainDefPtr vmdef = NULL;
 virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
 int ret = -1;
-unsigned int affect;
+unsigned int affect, parse_flags = 0;
 virQEMUCapsPtr qemuCaps = NULL;
 qemuDomainObjPrivatePtr priv;
 virQEMUDriverConfigPtr cfg = NULL;
@@ -7212,9 +7226,13 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, 
const char *xml,
  goto endjob;
 }
 
+if ((flags  VIR_DOMAIN_AFFECT_CONFIG) 
+!(flags  VIR_DOMAIN_AFFECT_LIVE))
+parse_flags |= VIR_DOMAIN_XML_INACTIVE;
+
 dev = dev_copy = virDomainDeviceDefParse(xml, vm-def,
  caps, driver-xmlopt,
-  

[libvirt] [PATCH v5 3/5] qemu: Implement chardev hotplug on config level

2013-07-12 Thread Michal Privoznik
There are two levels on which a device may be hotplugged: config
and live. The config level requires just an insert or remove from
internal domain definition structure, which is exactly what this
patch does. There is currently no implementation for a chardev
update action, as there's not much to be updated. But more
importantly, the only thing that can be updated is path or socket
address by which chardevs are distinguished. So the update action
is currently not supported.
---
 src/conf/domain_conf.c   |  6 ++--
 src/conf/domain_conf.h   |  3 ++
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_driver.c   | 16 +++
 src/qemu/qemu_hotplug.c  | 72 
 src/qemu/qemu_hotplug.h  |  7 +
 6 files changed, 103 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 80ea18b..1d3ba77 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10092,7 +10092,7 @@ virDomainLeaseRemove(virDomainDefPtr def,
 return virDomainLeaseRemoveAt(def, idx);
 }
 
-static bool
+bool
 virDomainChrEquals(virDomainChrDefPtr src,
virDomainChrDefPtr tgt)
 {
@@ -18029,9 +18029,11 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src,
 case VIR_DOMAIN_DEVICE_RNG:
 rc = virDomainRNGDefFormat(buf, src-data.rng, flags);
 break;
+case VIR_DOMAIN_DEVICE_CHR:
+rc = virDomainChrDefFormat(buf, src-data.chr, flags);
+break;
 case VIR_DOMAIN_DEVICE_NONE:
 case VIR_DOMAIN_DEVICE_SMARTCARD:
-case VIR_DOMAIN_DEVICE_CHR:
 case VIR_DOMAIN_DEVICE_MEMBALLOON:
 case VIR_DOMAIN_DEVICE_NVRAM:
 case VIR_DOMAIN_DEVICE_LAST:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ef72d24..c26f4e2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2399,6 +2399,9 @@ virDomainChrGetDomainPtrs(virDomainDefPtr vmdef,
 virDomainChrDefPtr
 virDomainChrFind(virDomainDefPtr def,
  virDomainChrDefPtr target);
+bool
+virDomainChrEquals(virDomainChrDefPtr src,
+   virDomainChrDefPtr tgt);
 int
 virDomainChrInsert(virDomainDefPtr vmdef,
virDomainChrDefPtr chr);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0ab7632..31eae1c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -114,6 +114,7 @@ virDomainChrConsoleTargetTypeToString;
 virDomainChrDefForeach;
 virDomainChrDefFree;
 virDomainChrDefNew;
+virDomainChrEquals;
 virDomainChrFind;
 virDomainChrGetDomainPtrs;
 virDomainChrInsert;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 495867a..852db8b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6686,6 +6686,12 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
 return -1;
 break;
 
+case VIR_DOMAIN_DEVICE_CHR:
+if (qemuDomainChrInsert(vmdef, dev-data.chr)  0)
+return -1;
+dev-data.chr = NULL;
+break;
+
 default:
  virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
 _(persistent attach of device '%s' is not supported),
@@ -6705,6 +6711,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
 virDomainHostdevDefPtr hostdev, det_hostdev;
 virDomainLeaseDefPtr lease, det_lease;
 virDomainControllerDefPtr cont, det_cont;
+virDomainChrDefPtr chr;
 int idx;
 char mac[VIR_MAC_STRING_BUFLEN];
 
@@ -6772,6 +6779,15 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
 
 break;
 
+case VIR_DOMAIN_DEVICE_CHR:
+if (!(chr = qemuDomainChrRemove(vmdef, dev-data.chr)))
+return -1;
+
+virDomainChrDefFree(chr);
+virDomainChrDefFree(dev-data.chr);
+dev-data.chr = NULL;
+break;
+
 default:
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_(persistent detach of device '%s' is not supported),
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e05b4b3..94f29e5 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1162,6 +1162,78 @@ error:
 
 }
 
+int
+qemuDomainChrInsert(virDomainDefPtr vmdef,
+virDomainChrDefPtr chr)
+{
+if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE 
+chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(attaching serial console is not supported));
+return -1;
+}
+
+if (virDomainChrFind(vmdef, chr)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(chardev already exists));
+return -1;
+}
+
+if (virDomainChrInsert(vmdef, chr)  0)
+return -1;
+
+/* Due to some crazy backcompat stuff, the first serial device is an alias
+ * to the first console too. If this is the case, the definition must be
+ * duplicated as first console device. */
+if (vmdef-nserials 

[libvirt] [PATCH v5 2/5] domain_conf: Auto fill chardev port

2013-07-12 Thread Michal Privoznik
Now that we have callbacks, we should auto fill in omitted pieces of
information. It's important for chardev hotplug to fill in the correct
/{serial,parallel,console,channel}/target/@port if no value has been
provided by user.
---
 src/conf/domain_conf.c | 46 +-
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5f0366e..80ea18b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2780,13 +2780,49 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
 
 static int
 virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
-virDomainDefPtr def ATTRIBUTE_UNUSED,
+virDomainDefPtr def,
 virCapsPtr caps ATTRIBUTE_UNUSED)
 {
-if (dev-type == VIR_DOMAIN_DEVICE_CHR 
-dev-data.chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE 
-dev-data.chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE)
-dev-data.chr-targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
+if (dev-type == VIR_DOMAIN_DEVICE_CHR) {
+virDomainChrDefPtr chr = dev-data.chr;
+virDomainChrDefPtr **arrPtr;
+size_t i, *cnt;
+
+virDomainChrGetDomainPtrs(def, chr, arrPtr, cnt);
+
+if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE 
+chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE)
+chr-targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
+
+if (chr-target.port == -1 
+(chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL ||
+ chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL ||
+ chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE)) {
+int maxport = -1;
+
+for (i = 0; i  *cnt; i++) {
+if ((*arrPtr)[i]-target.port  maxport)
+maxport = (*arrPtr)[i]-target.port;
+}
+
+chr-target.port = maxport + 1;
+}
+
+if (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL 
+chr-info.addr.vioserial.port == 0) {
+int maxport = 0;
+
+for (i = 0; i  *cnt; i++) {
+virDomainChrDefPtr thischr = (*arrPtr)[i];
+if (thischr-info.type == 
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL 
+thischr-info.addr.vioserial.controller == 
chr-info.addr.vioserial.controller 
+thischr-info.addr.vioserial.bus == 
chr-info.addr.vioserial.bus 
+(int)thischr-info.addr.vioserial.port  maxport)
+maxport = thischr-info.addr.vioserial.port;
+}
+chr-info.addr.vioserial.port = maxport + 1;
+}
+}
 
 return 0;
 }
-- 
1.8.1.5

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


[libvirt] [PATCH] lxc: hoist supplemental group detection before clone

2013-07-12 Thread Eric Blake
Commit 75c1256 states that virGetGroupList must not be called
between fork and exec, then commit ee777e99 promptly violated
that for lxc.  Hoist the group detection to occur before clone.

* src/lxc/lxc_container.c (__lxc_child_argv): Add members.
(lxcContainerSetID): Adjust signature.
(lxcContainerChild, lxcContainerStart): Adjust callers.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/lxc/lxc_container.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 940233b..50aaa36 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -106,6 +106,8 @@ struct __lxc_child_argv {
 char **ttyPaths;
 size_t nttyPaths;
 int handshakefd;
+gid_t *groups;
+int ngroups;
 };

 static int lxcContainerMountFSBlock(virDomainFSDefPtr fs,
@@ -349,26 +351,20 @@ int lxcContainerWaitForContinue(int control)
  *
  * Returns 0 on success or -1 in case of error
  */
-static int lxcContainerSetID(virDomainDefPtr def)
+static int
+lxcContainerSetID(virDomainDefPtr def, gid_t* groups, int ngroups)
 {
-gid_t *groups;
-int ngroups;
-
 /* Only call virSetUIDGID when user namespace is enabled
  * for this container. And user namespace is only enabled
  * when nuidmapngidmap is not zero */

 VIR_DEBUG(Set UID/GID to 0/0);
-if (def-idmap.nuidmap 
-((ngroups = virGetGroupList(0, 0, groups)  0) ||
- virSetUIDGID(0, 0, groups, ngroups)  0)) {
+if (def-idmap.nuidmap  virSetUIDGID(0, 0, groups, ngroups)  0) {
 virReportSystemError(errno, %s,
  _(setuid or setgid failed));
-VIR_FREE(groups);
 return -1;
 }

-VIR_FREE(groups);
 return 0;
 }

@@ -1981,7 +1977,7 @@ static int lxcContainerChild(void *data)
 cmd = lxcContainerBuildInitCmd(vmDef);
 virCommandWriteArgLog(cmd, 1);

-if (lxcContainerSetID(vmDef)  0)
+if (lxcContainerSetID(vmDef, argv-groups, argv-ngroups)  0)
 goto cleanup;

 root = virDomainGetRootFilesystem(vmDef);
@@ -2054,6 +2050,7 @@ cleanup:
 VIR_FORCE_CLOSE(ttyfd);
 VIR_FORCE_CLOSE(argv-monitor);
 VIR_FORCE_CLOSE(argv-handshakefd);
+VIR_FREE(argv-groups);

 if (ret == 0) {
 /* this function will only return if an error occurred */
@@ -2140,13 +2137,18 @@ int lxcContainerStart(virDomainDefPtr def,
 char *stack, *stacktop;
 lxc_child_argv_t args = { def, securityDriver,
   nveths, veths, control,
-  ttyPaths, nttyPaths, handshakefd};
+  ttyPaths, nttyPaths, handshakefd, NULL, 0 };

 /* allocate a stack for the container */
 if (VIR_ALLOC_N(stack, stacksize)  0)
 return -1;
 stacktop = stack + stacksize;

+if ((args.ngroups = virGetGroupList(0, 0, args.groups))  0) {
+VIR_FREE(stack);
+return -1;
+}
+
 cflags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD;

 if (userns_required(def)) {
@@ -2157,6 +2159,7 @@ int lxcContainerStart(virDomainDefPtr def,
 virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
  _(Kernel doesn't support user namespace));
 VIR_FREE(stack);
+VIR_FREE(args.groups);
 return -1;
 }
 }
@@ -2168,6 +2171,7 @@ int lxcContainerStart(virDomainDefPtr def,

 pid = clone(lxcContainerChild, stacktop, cflags, args);
 VIR_FREE(stack);
+VIR_FREE(args.groups);
 VIR_DEBUG(clone() completed, new container PID is %d, pid);

 if (pid  0) {
-- 
1.8.1.4

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


Re: [libvirt] [PATCH] python: return dictionay without value in case of no blockjob

2013-07-12 Thread Michal Privoznik
On 17.05.2013 08:30, Guannan Ren wrote:


s/dictionay/dictionary/ in $SUBJ

 Currently, when there is no blockjob, dom.blockJobInfo('vda')
 still reports error because it didn't distinguish return value 0 from -1.

s/didn't/doesn't/

 libvirt.libvirtError: virDomainGetBlockJobInfo() failed
 
 virDomainGetBlockJobInfo() API return value:
  -1 in case of failure, 0 when nothing found, 1 found.
 
 And use PyDict_SetItemString instead of PyDict_SetItem when key is
 string type. PyDict_SetItemString increments key/value reference

s/string type/of string type/

 count, so calling Py_DECREF() for value. For key, we don't need to

s/calling/call/

 do this, because PyDict_SetItemString will handle it internally.
 ---
  python/libvirt-override.c | 54 
 ++-
  1 file changed, 39 insertions(+), 15 deletions(-)

ACK with the commit message fixed.

Michal

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


[libvirt] Reminder: KVM Forum 2013 Call for Participation

2013-07-12 Thread KVM-Forum-2013-PC
Reminder, the KVM Forum CFP closes in less than 2 weeks.

Also, thanks to generous support from our sponsors early registrants
get an awesome discount!

=
KVM Forum 2013: Call For Participation
October 21-23, 2013 - Edinburgh International Conference Centre - Edinburgh, UK

(All submissions must be received before midnight July 21, 2013)
=

KVM is an industry leading open source hypervisor that provides an ideal
platform for datacenter virtualization, virtual desktop infrastructure,
and cloud computing.  Once again, it's time to bring together the
community of developers and users that define the KVM ecosystem for
our annual technical conference.  We will discuss the current state of
affairs and plan for the future of KVM, its surrounding infrastructure,
and management tools.  The oVirt Workshop will run in parallel with the
KVM Forum again, bringing in a community focused on enterprise datacenter
virtualization management built on KVM.  For topics which overlap we will
have shared sessions.  So mark your calendar and join us in advancing KVM.

http://events.linuxfoundation.org/events/kvm-forum/

Once again we are colocated with The Linux Foundation's LinuxCon Europe.
KVM Forum attendees will be able to attend oVirt Workshop sessions and
are eligible to attend LinuxCon Europe for a discounted rate.

http://events.linuxfoundation.org/events/kvm-forum/register

We invite you to lead part of the discussion by submitting a speaking
proposal for KVM Forum 2013.

http://events.linuxfoundation.org/cfp

Suggested topics:

 KVM/Kernel
 - Scaling and performance
 - Nested virtualization
 - I/O improvements
 - VFIO, device assignment, SR-IOV
 - Driver domains
 - Time keeping
 - Resource management (cpu, memory, i/o)
 - Memory management (page sharing, swapping, huge pages, etc)
 - Network virtualization
 - Security
 - Architecture ports
 
 QEMU
 - Device model improvements
 - New devices and chipsets
 - Scaling and performance
 - Desktop virtualization
 - Spice
 - Increasing robustness and hardening
 - Security model
 - Management interfaces
 - QMP protocol and implementation
 - Image formats
 - Firmware (SeaBIOS, OVMF, UEFI, etc)
 - Live migration
 - Live snapshots and merging
 - Fault tolerance, high availability, continuous backup
 - Real-time guest support
 
 Virtio
 - Speeding up existing devices
 - Alternatives
 - Virtio on non-Linux or non-virtualized
 
 Management infrastructure
 - oVirt (shared track w/ oVirt Workshop)
 - Libvirt
 - KVM autotest
 - OpenStack
 - Network virtualization management
 - Enterprise storage management
 
 Cloud computing
 - Scalable storage
 - Virtual networking
 - Security
 - Provisioning

SUBMISSION REQUIREMENTS

Abstracts due: July 21, 2013
Notification: August 1, 2013

Please submit a short abstract (~150 words) describing your presentation
proposal.  In your submission please note how long your talk will take.
Slots vary in length up to 45 minutes.  Also include in your proposal
the proposal type -- one of:

- technical talk
- end-user talk
- birds of a feather (BOF) session

Submit your proposal here:

http://events.linuxfoundation.org/cfp

You will receive a notification whether or not your presentation proposal
was accepted by Aug 1st.

END-USER COLLABORATION

One of the big challenges as developers is to know what, where and how
people actually use our software.  We will reserve a few slots for end
users talking about their deployment challenges and achievements.

If you are using KVM in production you are encouraged submit a speaking
proposal.  Simply mark it as an end-user collaboration proposal.  As an
end user, this is a unique opportunity to get your input to developers.

BOF SESSION

We will reserve some slots in the evening after the main conference
tracks, for birds of a feather (BOF) sessions. These sessions will be
less formal than presentation tracks and targetted for people who would
like to discuss specific issues with other developers and/or users.
If you are interested in getting developers and/or uses together to
discuss a specific problem, please submit a BOF proposal.

HOTEL / TRAVEL

The KVM Forum 2013 will be held in Edinburgh, UK at the Edinburgh
International Conference Centre.

http://events.linuxfoundation.org/events/kvm-forum/hotel

Thank you for your interest in KVM.  We're looking forward to your
submissions and seeing you at the KVM Forum 2013 in October!

Thanks,
-your KVM Forum 2013 Program Committee

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


[libvirt] Call for Proposals: 2013 Linux Plumbers Virtualization Microconference

2013-07-12 Thread Alex Williamson

The Call for Proposals for the 2013 Linux Plumbers Virtualization
Microconference is now open.  This uconf is being held as part of Linux
Plumbers Conference in New Orleans, Louisiana, USA September 18-20th and
is co-located with LinuxCon North America.  For more information see:

http://www.linuxplumbersconf.org/2013/

The tentative deadline for proposals is August 1st.  To submit a topic
please email a brief abstract to lpc2013-virt...@codemonkey.ws  If you
require travel assistance (extremely limited) in order to attend, please
note that in your submission.  Also, please keep an eye on:

http://www.linuxplumbersconf.org/2013/submitting-topic/
http://www.linuxplumbersconf.org/2013/participate/

We've setup the above email submission as an interim approach until the
LPC program committee brings the official submission tool online.  I'll
send a follow-up message when that occurs, but please send your
proposals as soon as possible.  Thanks,

Alex

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


[libvirt] [PATCH] security_dac: compute supplemental groups before fork

2013-07-12 Thread Eric Blake
Commit 75c1256 states that virGetGroupList must not be called
between fork and exec, then commit ee777e99 promptly violated
that for lxc's use of virSecurityManagerSetProcessLabel.  Hoist
the supplemental group detection to the time that the security
manager is created.  Qemu is safe, as it uses
virSecurityManagerSetChildProcessLabel which in turn uses
virCommand to determine supplemental groups.

This does not fix the fact that virSecurityManagerSetProcessLabel
calls virSecurityDACParseIds calls parseIds which eventually
calls getpwnam_r, which also violates fork/exec async-signal-safe
safety rules, but so far no one has complained of hitting
deadlock in that case.

* src/security/security_dac.c (virSecurityDACSetUser)
(virSecurityDACSetGroup): Merge...
(virSecurityDACSetUIDGID): ...into new function.
(_virSecurityDACData): Track groups in private data.
(virSecurityDACClose): Clean up new fields.
(virSecurityDACGetIds): Alter signature.
(virSecurityDACSetSecurityHostdevLabelHelper)
(virSecurityDACSetChardevLabel, virSecurityDACSetProcessLabel)
(virSecurityDACSetChildProcessLabel): Update callers.
* src/security/security_dac.h: Update prototypes to match.
* src/security/security_manager.c (virSecurityManagerNewDAC):
Adjust caller.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/security/security_dac.c | 59 ++---
 src/security/security_dac.h |  9 +++
 src/security/security_manager.c | 11 ++--
 3 files changed, 44 insertions(+), 35 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 9b5eaa8..124f270 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -43,23 +43,20 @@ typedef virSecurityDACData *virSecurityDACDataPtr;
 struct _virSecurityDACData {
 uid_t user;
 gid_t group;
+gid_t *groups;
+int ngroups;
 bool dynamicOwnership;
 };

 void
-virSecurityDACSetUser(virSecurityManagerPtr mgr,
-  uid_t user)
+virSecurityDACSetUIDGID(virSecurityManagerPtr mgr,
+uid_t user, gid_t group, gid_t *groups, int ngroups)
 {
 virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
 priv-user = user;
-}
-
-void
-virSecurityDACSetGroup(virSecurityManagerPtr mgr,
-   gid_t group)
-{
-virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
 priv-group = group;
+priv-groups = groups;
+priv-ngroups = ngroups;
 }

 void
@@ -146,7 +143,8 @@ virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, 
gid_t *gidPtr)

 static int
 virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
- uid_t *uidPtr, gid_t *gidPtr)
+ uid_t *uidPtr, gid_t *gidPtr,
+ gid_t **groups, int *ngroups)
 {
 int ret;

@@ -157,8 +155,13 @@ virSecurityDACGetIds(virDomainDefPtr def, 
virSecurityDACDataPtr priv,
 return -1;
 }

-if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) = 0)
+if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) = 0) {
+if (groups)
+*groups = NULL;
+if (ngroups)
+ngroups = 0;
 return ret;
+}

 if (!priv) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -171,6 +174,10 @@ virSecurityDACGetIds(virDomainDefPtr def, 
virSecurityDACDataPtr priv,
 *uidPtr = priv-user;
 if (gidPtr)
 *gidPtr = priv-group;
+if (groups)
+*groups = priv-groups;
+if (ngroups)
+*ngroups = priv-ngroups;

 return 0;
 }
@@ -250,8 +257,10 @@ virSecurityDACOpen(virSecurityManagerPtr mgr 
ATTRIBUTE_UNUSED)
 }

 static int
-virSecurityDACClose(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED)
+virSecurityDACClose(virSecurityManagerPtr mgr)
 {
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+VIR_FREE(priv-groups);
 return 0;
 }

@@ -448,7 +457,7 @@ virSecurityDACSetSecurityHostdevLabelHelper(const char 
*file,
 uid_t user;
 gid_t group;

-if (virSecurityDACGetIds(def, priv, user, group))
+if (virSecurityDACGetIds(def, priv, user, group, NULL, NULL))
 return -1;

 return virSecurityDACSetOwnership(file, user, group);
@@ -702,7 +711,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
 uid_t user;
 gid_t group;

-if (virSecurityDACGetIds(def, priv, user, group))
+if (virSecurityDACGetIds(def, priv, user, group, NULL, NULL))
 return -1;

 switch (dev-type) {
@@ -996,26 +1005,20 @@ virSecurityDACSetProcessLabel(virSecurityManagerPtr mgr,
 {
 uid_t user;
 gid_t group;
-virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
 gid_t *groups;
 int ngroups;
-int ret = -1;
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);

-if (virSecurityDACGetIds(def, priv, user, group))
-return -1;
-ngroups = virGetGroupList(user, group, groups);
-if (ngroups  0)
+if 

[libvirt] [PATCH v3 0/3] libxl: implement some chuncks of the NUMA interface

2013-07-12 Thread Dario Faggioli
Hi everyone again,

3rd round for the initial NUMA support in the libxl driver.

I think I addressed all the comments that have been raised during v2 review
(more details in the signle changelogs). Let me know if there's more.

Thanks and Regards,
Dario

---

Dario Faggioli (3):
  libxl: implement NUMA capabilities reporting
  libxl: advertise the support for VIR_TYPED_PARAM_STRING
  libxl: implement virDomainGetNumaParameters


 src/libxl/libxl_conf.c   |  148 ++
 src/libxl/libxl_driver.c |  161 ++
 2 files changed, 307 insertions(+), 2 deletions(-)

-- 
This happens because I choose it to happen! (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems RD Ltd., Cambridge (UK)

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


[libvirt] [PATCH v3 2/3] libxl: advertise the support for VIR_TYPED_PARAM_STRING

2013-07-12 Thread Dario Faggioli
domainGetNumaParameters has a string typed parameter, hence it
is necessary for the libxl driver to support this.

This change implements the connectSupportsFeature hook for the
libxl driver, advertising that VIR_DRV_FEATURE_TYPED_PARAM_STRING
is supported.

Signed-off-by: Dario Faggioli dario.faggi...@citrix.com
Cc: Eric Blake ebl...@redhat.com
---
Changes from v1:
 * commit message improved, as requested during review;
 * added VIR_TYPED_PARAM_STRING_OKAY handling code in
   libxlDomainGetSchedulerParametersFlags();
---
 src/libxl/libxl_driver.c |   20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 21c5162..a5addab 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4342,7 +4342,10 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom,
 libxl_scheduler sched_id;
 int ret = -1;
 
-virCheckFlags(0, -1);
+virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+/* We don't return strings, and thus trivially support this flag.  */
+flags = ~VIR_TYPED_PARAM_STRING_OKAY;
 
 libxlDriverLock(driver);
 vm = virDomainObjListFindByUUID(driver-domains, dom-uuid);
@@ -4643,6 +4646,20 @@ libxlConnectListAllDomains(virConnectPtr conn,
 return ret;
 }
 
+/* Which features are supported by this driver? */
+static int
+libxlConnectSupportsFeature(virConnectPtr conn, int feature)
+{
+if (virConnectSupportsFeatureEnsureACL(conn)  0)
+return -1;
+
+switch (feature) {
+case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
+return 1;
+default:
+return 0;
+}
+}
 
 
 static virDriver libxlDriver = {
@@ -4723,6 +4740,7 @@ static virDriver libxlDriver = {
 .connectDomainEventRegisterAny = libxlConnectDomainEventRegisterAny, /* 
0.9.0 */
 .connectDomainEventDeregisterAny = libxlConnectDomainEventDeregisterAny, 
/* 0.9.0 */
 .connectIsAlive = libxlConnectIsAlive, /* 0.9.8 */
+.connectSupportsFeature = libxlConnectSupportsFeature, /* 1.1.1 */
 };
 
 static virStateDriver libxlStateDriver = {

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


[libvirt] [PATCH v3 1/3] libxl: implement NUMA capabilities reporting

2013-07-12 Thread Dario Faggioli
Starting from Xen 4.2, libxl has all the bits and pieces in place
for retrieving an adequate amount of information about the host
NUMA topology. It is therefore possible, after a bit of shuffling,
to arrange those information in the way libvirt wants to present
them to the outside world.

Therefore, with this patch, the topology section of the host
capabilities is properly populated, when running on Xen, so that
we can figure out whether or not we're running on a NUMA host,
and what its characteristics are.

[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// capabilities
capabilities
  host
cpu

topology
  cells num='2'
cell id='0'
  memory unit='KiB'6291456/memory
  cpus num='8'
cpu id='0' socket_id='1' core_id='0' siblings='0-1'/
cpu id='1' socket_id='1' core_id='0' siblings='0-1'/
cpu id='2' socket_id='1' core_id='1' siblings='2-3'/
cpu id='3' socket_id='1' core_id='1' siblings='2-3'/
cpu id='4' socket_id='1' core_id='9' siblings='4-5'/
cpu id='5' socket_id='1' core_id='9' siblings='4-5'/
cpu id='6' socket_id='1' core_id='10' siblings='6-7'/
cpu id='7' socket_id='1' core_id='10' siblings='6-7'/
  /cpus
/cell
cell id='1'
  memory unit='KiB'6881280/memory
  cpus num='8'
cpu id='8' socket_id='0' core_id='0' siblings='8-9'/
cpu id='9' socket_id='0' core_id='0' siblings='8-9'/
cpu id='10' socket_id='0' core_id='1' siblings='10-11'/
cpu id='11' socket_id='0' core_id='1' siblings='10-11'/
cpu id='12' socket_id='0' core_id='9' siblings='12-13'/
cpu id='13' socket_id='0' core_id='9' siblings='12-13'/
cpu id='14' socket_id='0' core_id='10' siblings='14-15'/
cpu id='15' socket_id='0' core_id='10' siblings='14-15'/
  /cpus
/cell
  /cells
/topology
  /host
  

Signed-off-by: Dario Faggioli dario.faggi...@citrix.com
---
Changes from v2:
 * iterators turned from int to size_t;
 * fixed wrong sibling maps if on same node but different socket;
 * code motion and error handling, as requested during review.

Changes from v1:
 * fixed a typo in the commit message, as requested during review;
 * fixed coding style (one function parameters per line and no spaces
   between variable definitions), as requested during review;
 * avoid zero-filling memory after VIR_ALLOC_N(), since it does that
   already, as requested during review;
 * improved out of memory error reporting, as requested during review;
 * libxlMakeNumaCapabilities() created, accommodating all the NUMA
   related additions, instead of having them within
   libxlMakeCapabilitiesInternal(), as suggested during review.
---
 src/libxl/libxl_conf.c |  148 
 1 file changed, 147 insertions(+), 1 deletion(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 4a0fba9..c097d1e 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -161,6 +161,117 @@ libxlBuildCapabilities(virArch hostarch,
 }
 
 static virCapsPtr
+libxlMakeNumaCapabilities(libxl_numainfo *numa_info,
+  int nr_nodes,
+  libxl_cputopology *cpu_topo,
+  int nr_cpus,
+  virCapsPtr caps)
+{
+virCapsHostNUMACellCPUPtr *cpus = NULL;
+int *nr_cpus_node = NULL;
+bool numa_failed = false;
+size_t i;
+
+if (VIR_ALLOC_N(cpus, nr_nodes)) {
+virReportOOMError();
+return caps;
+}
+
+if (VIR_ALLOC_N(nr_cpus_node, nr_nodes)) {
+VIR_FREE(cpus);
+virReportOOMError();
+return caps;
+}
+
+/* For each node, prepare a list of CPUs belonging to that node */
+for (i = 0; i  nr_cpus; i++) {
+int node = cpu_topo[i].node;
+
+if (cpu_topo[i].core == LIBXL_CPUTOPOLOGY_INVALID_ENTRY)
+continue;
+
+nr_cpus_node[node]++;
+
+if (nr_cpus_node[node] == 1) {
+if (VIR_ALLOC(cpus[node])  0) {
+virReportOOMError();
+numa_failed = true;
+goto cleanup;
+}
+}
+else {
+if (VIR_REALLOC_N(cpus[node], nr_cpus_node[node])  0) {
+virReportOOMError();
+numa_failed = true;
+goto cleanup;
+}
+}
+
+/* Mapping between what libxl tells and what libvirt wants */
+cpus[node][nr_cpus_node[node]-1].id = i;
+cpus[node][nr_cpus_node[node]-1].socket_id = cpu_topo[i].socket;
+cpus[node][nr_cpus_node[node]-1].core_id = cpu_topo[i].core;
+/* Allocate the siblings maps. We will be filling them later */
+cpus[node][nr_cpus_node[node]-1].siblings = virBitmapNew(nr_cpus);
+if (!cpus[node][nr_cpus_node[node]-1].siblings) {
+virReportOOMError();
+   

[libvirt] [PATCH v3 3/3] libxl: implement virDomainGetNumaParameters

2013-07-12 Thread Dario Faggioli
Although, having it depending on Xen = 4.3 (by using the proper
libxl feature flag).

Xen currently implements a NUMA placement policy which is basically
the same as the 'interleaved' policy of `numactl', although it can
be applied on a subset of the available nodes. We therefore hardcode
interleave as 'numa_mode', and we use the newly introduced libxl
interface to figure out what nodes a domain spans ('numa_nodeset').

With this change, it is now possible to query the NUMA node
affinity of a running domain:

[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// list
 IdName   State

 23F18_x64running

[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// numatune 23
numa_mode  : interleave
numa_nodeset   : 1

Signed-off-by: Dario Faggioli dario.faggi...@citrix.com
---
Changes from v2:
 * iterators turned from int to size_t

Changes from v1:
 * fixed coding style, as requested during review;
 * VIR_TYPED_PARAM_STRING_OKAY handled more properly;
---
 src/libxl/libxl_driver.c |  141 ++
 1 file changed, 141 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a5addab..358d387 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -28,6 +28,7 @@
 
 #include math.h
 #include libxl.h
+#include libxl_utils.h
 #include fcntl.h
 #include regex.h
 
@@ -4499,6 +4500,143 @@ libxlDomainSetSchedulerParameters(virDomainPtr dom, 
virTypedParameterPtr params,
 return libxlDomainSetSchedulerParametersFlags(dom, params, nparams, 0);
 }
 
+/* NUMA node affinity information is available through libxl
+ * starting from Xen 4.3. */
+#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
+
+/* Number of Xen NUMA parameters */
+# define LIBXL_NUMA_NPARAM 2
+
+static int
+libxlDomainGetNumaParameters(virDomainPtr dom,
+ virTypedParameterPtr params,
+ int *nparams,
+ unsigned int flags)
+{
+libxlDriverPrivatePtr driver = dom-conn-privateData;
+libxlDomainObjPrivatePtr priv;
+virDomainObjPtr vm;
+libxl_bitmap nodemap;
+virBitmapPtr nodes = NULL;
+char *nodeset = NULL;
+int rc, ret = -1;
+size_t i, j;
+
+/* In Xen 4.3, it is possible to query the NUMA node affinity of a domain
+ * via libxl, but not to change it. We therefore only allow AFFECT_LIVE. */
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+/* We blindly return a string, and let libvirt.c and remote_driver.c do
+ * the filtering on behalf of older clients that can't parse it. */
+flags = ~VIR_TYPED_PARAM_STRING_OKAY;
+
+libxlDriverLock(driver);
+vm = virDomainObjListFindByUUID(driver-domains, dom-uuid);
+libxlDriverUnlock(driver);
+
+if (!vm) {
+virReportError(VIR_ERR_NO_DOMAIN, %s,
+   _(no domain with matching uuid));
+goto cleanup;
+}
+
+if (virDomainGetNumaParametersEnsureACL(dom-conn, vm-def)  0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(Domain is not running));
+goto cleanup;
+}
+
+priv = vm-privateData;
+
+libxl_bitmap_init(nodemap);
+
+if ((*nparams) == 0) {
+*nparams = LIBXL_NUMA_NPARAM;
+ret = 0;
+goto cleanup;
+}
+
+for (i = 0; i  LIBXL_NUMA_NPARAM  i  *nparams; i++) {
+virMemoryParameterPtr param = params[i];
+
+switch (i) {
+case 0:
+/* NUMA mode */
+
+/* Xen implements something that is really close to numactl's
+ * 'interleave' policy (see `man 8 numactl' for details). */
+if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
+VIR_TYPED_PARAM_INT,
+VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE)  
0)
+goto cleanup;
+
+break;
+
+case 1:
+/* Node affinity */
+
+/* Let's allocate both libxl and libvirt bitmaps */
+if (libxl_node_bitmap_alloc(priv-ctx, nodemap, 0) ||
+!(nodes = virBitmapNew(libxl_get_max_nodes(priv-ctx {
+virReportOOMError();
+goto cleanup;
+}
+
+rc = libxl_domain_get_nodeaffinity(priv-ctx,
+   vm-def-id,
+   nodemap);
+if (rc != 0) {
+virReportSystemError(-rc, %s,
+ _(unable to get numa affinity));
+goto cleanup;
+}
+
+/* First, we convert libxl_bitmap into virBitmap. After that,
+ * we format virBitmap as a string that can be returned. */
+virBitmapClearAll(nodes);
+   

[libvirt] deadlock in function storageVolLookupByKey and storageVolLookupByPath

2013-07-12 Thread Caizhifeng
root@cvknode-141:~/libvirt1.1.0# diff storage_driver.c storage_driver-new.c
1364c1364,1365
 if (virStorageVolLookupByKeyEnsureACL(conn, 
driver-pools.objs[i]-def, vol)  0)
---
 if (virStorageVolLookupByKeyEnsureACL(conn, 
 driver-pools.objs[i]-def, vol)  0) {
 virStoragePoolObjUnlock(driver-pools.objs[i]);
1365a1367
 }
1371a1374
 virStoragePoolObjUnlock(driver-pools.objs[i]);
1424c1427,1428
 if (virStorageVolLookupByPathEnsureACL(conn, 
driver-pools.objs[i]-def, vol)  0)
---
 if (virStorageVolLookupByPathEnsureACL(conn, 
 driver-pools.objs[i]-def, vol)  0) {
 virStoragePoolObjUnlock(driver-pools.objs[i]);
1425a1430
 }
1431a1437
 virStoragePoolObjUnlock(driver-pools.objs[i]);
root@cvknode-141:~/libvirt1.1.0#
-
本邮件及其附件含有杭州华三通信技术有限公司的保密信息,仅限于发送给上面地址中列出
的个人或群组。禁止任何其他人以任何形式使用(包括但不限于全部或部分地泄露、复制、
或散发)本邮件中的信息。如果您错收了本邮件,请您立即电话或邮件通知发件人并删除本
邮件!
This e-mail and its attachments contain confidential information from H3C, 
which is
intended only for the person or entity whose address is listed above. Any use 
of the
information contained herein in any way (including, but not limited to, total 
or partial
disclosure, reproduction, or dissemination) by persons other than the intended
recipient(s) is prohibited. If you receive this e-mail in error, please notify 
the sender
by phone or email immediately and delete it!

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

Re: [libvirt] [PATCH 1/6] Introduce new domain create APIs to pass pre-opened FDs to LXC

2013-07-12 Thread Daniel Veillard
On Fri, Jul 12, 2013 at 04:38:27PM +0100, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 With container based virt, it is useful to be able to pass
 pre-opened file descriptors to the container init process.
 This allows for containers to be auto-activated from incoming
 socket connections, passing the active socket into the container.
 
 To do this, introduce a pair of new APIs, virDomainCreateXMLWithFiles
 and virDomainCreateWithFiles, which accept an array of file
 descriptors. For the LXC driver, UNIX file descriptor passing
 will be used to send them to libvirtd, which will them pass
 them down to libvirt_lxc, which will then pass them to the container
 init process.
 
 This will only be implemented for LXC right now, but the design
 is generic enough it could work with other hypervisors, hence
 I suggest adding this to libvirt.so, rather than libvirt-lxc.so
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
[...]
  /**
 + * virDomainCreateXMLWithFiles:
 + * @conn: pointer to the hypervisor connection
 + * @xmlDesc: string containing an XML description of the domain
 + * @nfiles: number of file descriptors passed
 + * @files: list of file descriptors passed
 + * @flags: bitwise-OR of supported virDomainCreateFlags
 + *
 + * Launch a new guest domain, based on an XML description similar
 + * to the one returned by virDomainGetXMLDesc()
 + * This function may require privileged access to the hypervisor.
 + * The domain is not persistent, so its definition will disappear when it
 + * is destroyed, or if the host is restarted (see virDomainDefineXML() to
 + * define persistent domains).
 + *
 + * @files provides an array of file descriptors which will be
 + * made available to the 'init' process of the guest. The file
 + * handles exposed to the guest will be renumbered to start
 + * from 3 (ie immediately following stderr). This is only
 + * supported for guests which use container based virtualization
 + * technology.

  Hum, at least now the semantic is clear so big improvement from v1,
but can we avoid that magic '3' buried here, and add a firstfile or
start file which would give the start index. This extension is already
very specific to LXC, no need to make it more specific than needed,
maybe other container technologies would appreciate to pass stdin
or start from a higher boundary.

  We must specify what a negative fd passed in the array means, it
could mean don't override the existing fd if opened, or close that fd
etc ... this extensions could then be used for purpose we don't expect
just from as systemd-LXC current use case.

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