date:20120816

[libvirt] [PATCH v4] network: use firewalld instead of iptables, when available

2012-08-16 Thread Laine Stump

From: Thomas Woerner twoer...@redhat.com

(This is Thomas v3 version of 1/2 of the firewalld patches, modified
to check for firewall-cmd and firewalld state only once, rather than
every time an iptables rule is added or removed. It's not intended to
be pushed, because I'm still having issues with it, at least on my
machine. I'm mostly concerned with item (1) on the list below; the
others could be solved later or tolerated.)

* configure.ac, spec file: firewalld defaults to enabled if dbus is
  available, otherwise is disabled. If --with_firewalld is explicitly
  requested and dbus is not available, configure will fail.

* bridge_driver: add dbus filters to get the FirewallD1.Reloaded
  signal and DBus.NameOwnerChanged on org.fedoraproject.FirewallD1.
  When these are encountered, reload all the iptables reuls of all
  libvirt's virtual networks (similar to what happens when libvirtd is
  restarted).

* iptables, ebtables: use firewall-cmd's direct passthrough interface
  when available, otherwise use iptables and ebtables commands. This
  decision is made once the first time libvirt calls
  iptables/ebtables, and that decision is maintained for the life of
  libvirtd.

* Note that the nwfilter part of this patch was separated out into
  another patch by Stefan in V2, so that needs to be revised and
  re-reviewed as well.



All the configure.ac and specfile changes are unchanged from Thomas'
V3.

V3 re-ran firewall-cmd --state every time a new rule was added,
which was extremely inefficient.  V4 uses VIR_ONCE_GLOBAL_INIT to set
up a one-time initialization function.

The VIR_ONCE_GLOBAL_INIT(x) macro references a static function called
vir(Ip|Eb)OnceInit(), which will then be called the first time that
the static function vir(Ip|Eb)TablesInitialize() is called (that
function is defined for you by the macro). This is
thread-safe, so there is no chance of any race.

IMPORTANT NOTE: I've left the VIR_DEBUG messages in these two init
functions (one for iptables, on for ebtables) as VIR_WARN so that I
don't have to turn on all the other debug message just to see
these. Even if this patch doesn't need any other modification, those
messages need to be changed to VIR_DEBUG before pushing.

This one-time initialization works well. However, I've encountered
problems with testing:

1) Whenever I have enabled the firewalld service, *all* attempts to
call firewall-cmd from within libvirtd end with firewall-cmd hanging
internally somewhere. This is *not* the case if firewall-cmd returns
non-0 in response to firewall-cmd --state (i.e. *that* command runs
and returns to libvirt successfully.)

2) If I start libvirtd while firewalld is stopped, then start
firewalld later, this triggers libvirtd to reload its iptables rules,
however it also spits out a *ton* of complaints about deletion failing
(I suppose because firewalld has nuked all of libvirt's rules). I
guess we need to suppress those messages (which is a more annoying
problem to fix than you might think, but that's another story).

3) I noticed a few times during this long line of errors that
firewalld made a complaint about Resource Temporarily
unavailable. Having libvirtd access iptables commands directly at the
same time as firewalld is doing so is apparently problematic.

4) In general, I'm concerned about the set it once and never change
it method - if firewalld is disabled at libvirtd startup, causing
libvirtd to always use iptables/ebtables directly, this won't cause
*terrible* problems, but if libvirtd decides to use firewall-cmd and
firewalld is later disabled, libvirtd will not be able to recover.
---
 AUTHORS |  2 +-
 configure.ac| 17 +++
 libvirt.spec.in | 11 ++
 src/Makefile.am |  4 ++--
 src/network/bridge_driver.c | 49 ++
 src/util/ebtables.c | 52 -
 src/util/iptables.c | 49 +++---
 7 files changed, 177 insertions(+), 7 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 8581aea..5dec3a2 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -257,7 +257,7 @@ Patches have also been contributed by:
   Frido Roose  frido.ro...@gmail.com
   Asad Saeed   asad.sa...@acidseed.com
   Sukadev Bhattiprolu  suka...@linux.vnet.ibm.com
-
+  Thomas Woerner   twoer...@redhat.com
   [send patches to get your name here]
 
 The libvirt Logo was designed by Diana Fong
diff --git a/configure.ac b/configure.ac
index ba5a3cd..0150f99 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1321,6 +1321,22 @@ AM_CONDITIONAL([HAVE_POLKIT1], [test x$with_polkit1 = 
xyes])
 AC_SUBST([POLKIT_CFLAGS])
 AC_SUBST([POLKIT_LIBS])
 
+dnl firewalld
+AC_ARG_WITH([firewalld],
+  AC_HELP_STRING([--with-firewalld], [enable firewalld support 
@:@default=check@:@]),
+  [],
+  [with_firewalld=check])
+if test x$with_firewalld = xcheck ; then
+

Re: [libvirt] [PATCHv2] build: avoid warnings on older gcc

2012-08-16 Thread Viktor Mihajlovski


On 08/15/2012 09:25 PM, Eric Blake wrote:

The previous patch pulled in a newer version of stat-time.h from
gnulib, which tries to suppress some stupid gcc warnings (see
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54113) by exploiting
a feature of newer gcc.  But we still aim to compile on RHEL 6.3,
with gcc 4.4.6 (not to mention even older devel platforms like
RHEL 5), which fails to build thanks to our development -Werror.

It took me a while to figure out how to make our set of warnings
smaller on older gcc without losing the benefit of the warnings
when using newer gcc (such as the one on Fedora 17), but this
should do the trick.

* m4/virt-compile-warnings.m4 (LIBVIRT_COMPILE_WARNINGS): Avoid
warnings that gnulib can't silence on older gcc.
---

v2: Use AC_CACHE_CHECK, add -Werror, and test further.
Pushing under the build-breaker rule.

  m4/virt-compile-warnings.m4 | 18 ++
  1 file changed, 18 insertions(+)

diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
index 9dee000..26f6134 100644
--- a/m4/virt-compile-warnings.m4
+++ b/m4/virt-compile-warnings.m4
@@ -58,6 +58,24 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
  # Gnulib's stat-time.h violates this
  dontwarn=$dontwarn -Waggregate-return

+# Gnulib uses '#pragma GCC diagnostic push' to silence some
+# warnings, but older gcc doesn't support this.
+AC_CACHE_CHECK([whether pragma GCC diagnostic push works],
+  [lv_cv_gcc_pragma_push_works], [
+  save_CFLAGS=$CFLAGS
+  CFLAGS='-Wunknown-pragmas -Werror'
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
+#pragma GCC diagnostic push
+#pragma GCC diagnostic pop
+  ]])],
+  [lv_cv_gcc_pragma_push_works=yes],
+  [lv_cv_gcc_pragma_push_works=no])
+  CFLAGS=$save_CFLAGS])
+if test $lv_cv_gcc_pragma_push_works = no; then
+  dontwarn=$dontwarn -Wmissing-prototypes
+  dontwarn=$dontwarn -Wmissing-declarations
+fi
+
  # We might fundamentally need some of these disabled forever, but
  # ideally we'd turn many of them on
  dontwarn=$dontwarn -Wfloat-equal



Works for me now -- thanks!

--

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research  Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

Re: [libvirt] [PATCH 1/2] util: properly save/restore original vlan tag for VFs

2012-08-16 Thread Osier Yang


On 2012年08月16日 12:34, Laine Stump wrote:

When a network device that is a VF of an SR-IOV card was assigned to a
guest usinginterface type='hostdev', only the MAC address was being
saved/restored, but the VLAN tag was left untouched. Up to now we
haven't actually used vlan tags on SR-IOV devices, so the guest would
have used whatever was set, and left it the same at the end.

The patch following this one will hook up thevlan  element from the
interface config, so save/restore of the device state needs to also
include the vlan tag.

MAC address is being saved as a simple ASCII string in a file named
for the device under /var/run.  The VLAN tag is now just added at the
end of that file, after a newline. It might be nicer if the file was
XML (in case it ever gets more complicated) but at the moment there's
nothing else on the horizon, and this makes backward compatibility
easier.
---
  src/util/virnetdev.c | 56 +---
  1 file changed, 40 insertions(+), 16 deletions(-)

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index f1ee0a4..25bdf01 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1533,33 +1533,41 @@ virNetDevReplaceVfConfig(const char *pflinkdev, int vf,
   int vlanid,
   const char *stateDir)
  {
+int ret = -1;
  virMacAddr oldmac;
  int oldvlanid = -1;
  char *path = NULL;
  char macstr[VIR_MAC_STRING_BUFLEN];
+char *fileData = NULL;
  int ifindex = -1;

  if (virNetDevGetVfConfig(pflinkdev, vf,oldmac,oldvlanid)  0)
-return -1;
+goto cleanup;

  if (virAsprintf(path, %s/%s_vf%d,
  stateDir, pflinkdev, vf)  0) {
  virReportOOMError();
-return -1;
+goto cleanup;
  }

-virMacAddrFormat(oldmac, macstr);
-if (virFileWriteStr(path, macstr, O_CREAT|O_TRUNC|O_WRONLY)  0) {
-virReportSystemError(errno, _(Unable to preserve mac for pf = %s,
-  vf = %d), pflinkdev, vf);
-VIR_FREE(path);
-return -1;
+if (virAsprintf(fileData, %s\n%d\n,
+virMacAddrFormat(oldmac, macstr), oldvlanid)  0) {
+virReportOOMError();
+goto cleanup;
+}
+if (virFileWriteStr(path, fileData, O_CREAT|O_TRUNC|O_WRONLY)  0) {
+virReportSystemError(errno, _(Unable to preserve mac/vlan tag 
+  for pf = %s, vf = %d), pflinkdev, vf);
+goto cleanup;
  }

-VIR_FREE(path);
-
-return virNetDevSetVfConfig(pflinkdev, ifindex, vf, true,
+ret = virNetDevSetVfConfig(pflinkdev, ifindex, vf, true,
  macaddress, vlanid, NULL);
+
+cleanup:
+VIR_FREE(path);
+VIR_FREE(fileData);
+return ret;
  }

  static int
@@ -1567,8 +1575,9 @@ virNetDevRestoreVfConfig(const char *pflinkdev, int vf,
   const char *stateDir)
  {
  int rc = -1;
-char *macstr = NULL;
  char *path = NULL;
+char *fileData = NULL;
+char *vlan = NULL;
  virMacAddr oldmac;
  int vlanid = -1;
  int ifindex = -1;
@@ -1579,14 +1588,29 @@ virNetDevRestoreVfConfig(const char *pflinkdev, int vf,
  return rc;
  }

-if (virFileReadAll(path, VIR_MAC_STRING_BUFLEN,macstr)  0) {
+if (virFileReadAll(path, 128,fileData)  0) {
  goto cleanup;
  }

-if (virMacAddrParse(macstr,oldmac) != 0) {
+if ((vlan = strchr(fileData, '\n'))) {
+char *endptr;
+
+*vlan++ = 0; /* NULL terminate the mac address */


s/terminate/terminates/


+if (*vlan) {
+if ((virStrToLong_i(vlan,endptr, 10,vlanid)  0) ||
+(endptr  *endptr != '\n'  *endptr != 0)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Cannot parse vlan tag from '%s'),
+   vlan);
+goto cleanup;
+}
+}
+}
+
+if (virMacAddrParse(fileData,oldmac) != 0) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(Cannot parse MAC address from '%s'),
-   macstr);
+   fileData);
  goto cleanup;
  }

@@ -1597,7 +1621,7 @@ virNetDevRestoreVfConfig(const char *pflinkdev, int vf,

  cleanup:
  VIR_FREE(path);
-VIR_FREE(macstr);
+VIR_FREE(fileData);

  return rc;
  }


Looks good for me, ACK.

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

Re: [libvirt] [PATCH 2/2] qemu: support setting vlan tag for interface type='hostdev'

2012-08-16 Thread Osier Yang


On 2012年08月16日 12:34, Laine Stump wrote:

The underlying function to set the vlan tag of an SR-IOV network
device was already in place (although an extra patch to save/restore
the original vlan tag was needed), and recent patches added the
ability to configure a vlan tag. This patch just ties those two
together.

An SR-IOV device doesn't support vlan trunking, so if anyone tries to
configure more than a single tag, or set the trunk flag, and error is
logged.
---
  src/qemu/qemu_hostdev.c | 40 ++--
  1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 7619fd0..46c84b5 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -301,6 +301,7 @@ qemuDomainHostdevNetConfigReplace(virDomainHostdevDefPtr 
hostdev,
char *stateDir)
  {
  char *linkdev = NULL;
+virNetDevVlanPtr vlan;
  virNetDevVPortProfilePtr virtPort;
  int ret = -1;
  int vf = -1;
@@ -319,19 +320,46 @@ qemuDomainHostdevNetConfigReplace(virDomainHostdevDefPtr 
hostdev,
  if (qemuDomainHostdevNetDevice(hostdev,linkdev,vf)  0)
  return ret;

+vlan = virDomainNetGetActualVlan(hostdev-parent.data.net);
  virtPort = virDomainNetGetActualVirtPortProfile(
   hostdev-parent.data.net);
-if (virtPort)
+if (virtPort) {
+if (vlan) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(direct setting of the vlan tag is not allowed 
+ for hostdev devices using %s mode),
+   virNetDevVPortTypeToString(virtPort-virtPortType));
+goto cleanup;
+}
  ret = qemuDomainHostdevNetConfigVirtPortProfile(linkdev, vf,
  virtPort,hostdev-parent.data.net-mac, uuid,
  port_profile_associate);
-else
-/* Set only mac */
+} else {
+/* Set only mac and vlan */
+if (vlan) {
+if (vlan-nTags != 1 || vlan-trunk) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(vlan trunking is not supported 
+ by SR-IOV network devices));
+goto cleanup;
+}
+if (vf == -1) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(vlan can only be set for SR-IOV VFs, but 
+ %s is not a VF), linkdev);
+goto cleanup;
+}
+vlanid = vlan-tag[0];
+} else  if (vf= 0) {
+vlanid = 0; /* assure any current vlan tag is reset */
+}
+
  ret = virNetDevReplaceNetConfig(linkdev, vf,
-hostdev-parent.data.net-mac, vlanid,
-stateDir);
+hostdev-parent.data.net-mac,
+vlanid, stateDir);
+}
+cleanup:
  VIR_FREE(linkdev);
-
  return ret;
  }



ACK.

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

Re: [libvirt] [PATCH] qemu: Ensure the cpuset is formated as expected before passing to cgroup

2012-08-16 Thread Osier Yang


On 2012年08月16日 13:06, Eric Blake wrote:

On 08/15/2012 09:35 PM, Osier Yang wrote:

The parameter value for cpuset could be in special format like
0-10,^7, which is not recongnized by cgroup. This patch is to


s/recongnized/recognized/


ensure the cpuset is formated as expected before passing it to


s/formated/formatted/


cgroup. As a side effect, after the patch, it parses the cpuset
early before cgroup setting, to avoid the rollback if cpuset
parsing fails afterwards.
---
  src/qemu/qemu_driver.c |   77 +--
  1 files changed, 41 insertions(+), 36 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 369e8ed..fc412a1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6952,8 +6952,23 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
  }
  } else if (STREQ(param-field, VIR_DOMAIN_NUMA_NODESET)) {
  int rc;
-bool savedmask;
-char oldnodemask[VIR_DOMAIN_CPUMASK_LEN];
+char *nodeset = NULL;
+char *nodeset_str = NULL;
+
+if (VIR_ALLOC_N(nodeset, VIR_DOMAIN_CPUMASK_LEN)  0) {
+virReportOOMError();
+ret = -1;
+goto cleanup;
+};
+
+if (virDomainCpuSetParse(params[i].value.s,
+ 0, nodeset,
+ VIR_DOMAIN_CPUMASK_LEN)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Failed to parse nodeset));
+ret = -1;
+continue;
+}


We have enough of this open-coded conversions in the code base that it
would be worth common helper routines to make the conversion much easier
to use in the clients (for a later patch).  But for the purposes of
fixing the bug at hand, your patch does the trick.



  if (flags  VIR_DOMAIN_AFFECT_LIVE) {
  if (vm-def-numatune.memory.mode !=
@@ -6961,72 +6976,62 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
  virReportError(VIR_ERR_OPERATION_INVALID, %s,
 _(change of nodeset for running domain 
   requires strict numa mode));
+VIR_FREE(nodeset);
  ret = -1;
  continue;
  }
-rc = virCgroupSetCpusetMems(group, params[i].value.s);
-if (rc != 0) {
+
+/* Ensure the cpuset string is formated before passing to 
cgroup */


s/formated/formatted/


hum, you corrected my typos on recongnized and formated several
times. /shy.



ACK with nits fixed.


Thanks, pushed with the nits fixed.

Regards,
Osier

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

[libvirt] [test-API][PATCH 1/2] New get_conn function in utils

2012-08-16 Thread Wayne Sun

  The get_conn function return connection object from libvirt module.
  This function could be used by both framework and testcases.
  The patch includes:
  * get_conn in utils/utils.py
  * sync env_inspect.py using the new function

Signed-off-by: Wayne Sun g...@redhat.com
---
 src/env_inspect.py |   22 ++
 utils/utils.py |   27 +++
 2 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/src/env_inspect.py b/src/env_inspect.py
index b260ff8..2c1a701 100644
--- a/src/env_inspect.py
+++ b/src/env_inspect.py
@@ -20,6 +20,7 @@
 import commands
 import libvirt
 import sharedmod
+from utils import utils
 
 def check_libvirt(logger):
 virsh = 'virsh -v'
@@ -68,20 +69,6 @@ def hostinfo(logger):
 return 1
 return 0
 
-def request_credentials(credentials, user_data):
-for credential in credentials:
-if credential[0] == libvirt.VIR_CRED_AUTHNAME:
-credential[4] = user_data[0]
-
-if len(credential[4]) == 0:
-credential[4] = credential[3]
-elif credential[0] == libvirt.VIR_CRED_PASSPHRASE:
-credential[4] = user_data[1]
-else:
-return -1
-
-return 0
-
 def sharemod_init(env_parser, logger):
  get connection object from libvirt module
 initialize sharemod for use by testcases
@@ -89,12 +76,7 @@ def sharemod_init(env_parser, logger):
 uri = env_parser.get_value('variables', 'defaulturi')
 username = env_parser.get_value('variables', 'username')
 password = env_parser.get_value('variables', 'password')
-user_data = [username, password]
-auth = [[libvirt.VIR_CRED_AUTHNAME, libvirt.VIR_CRED_PASSPHRASE], 
request_credentials, user_data]
-conn = libvirt.openAuth(uri, auth, 0)
-if not conn:
-logger.error(Failed to setup libvirt connection);
-return 1
+conn = utils.get_conn(uri, username, password)
 
 # initialize conn object in sharedmod
 sharedmod.libvirtobj.clear()
diff --git a/utils/utils.py b/utils/utils.py
index be87cdc..eade10d 100644
--- a/utils/utils.py
+++ b/utils/utils.py
@@ -29,6 +29,7 @@ import struct
 import pexpect
 import string
 import subprocess
+import libvirt
 from xml.dom import minidom
 from urlparse import urlparse
 
@@ -57,6 +58,32 @@ def get_uri(ip):
 uri = qemu+ssh://%s/system % ip
 return uri
 
+def request_credentials(credentials, user_data):
+for credential in credentials:
+if credential[0] == libvirt.VIR_CRED_AUTHNAME:
+credential[4] = user_data[0]
+
+if len(credential[4]) == 0:
+credential[4] = credential[3]
+elif credential[0] == libvirt.VIR_CRED_PASSPHRASE:
+credential[4] = user_data[1]
+else:
+return -1
+
+return 0
+
+def get_conn(uri='', username='', password=''):
+ get connection object from libvirt module
+
+user_data = [username, password]
+auth = [[libvirt.VIR_CRED_AUTHNAME, libvirt.VIR_CRED_PASSPHRASE], 
request_credentials, user_data]
+conn = libvirt.openAuth(uri, auth, 0)
+if not conn:
+logger.error(Failed to setup libvirt connection);
+sys.exit(1)
+else:
+return conn
+
 def parse_uri(uri):
 # This is a simple parser for uri
 return urlparse(uri)
-- 
1.7.1

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

[libvirt] [test-API][PATCH 2/2] Update cases with libvirtd restart problem

2012-08-16 Thread Wayne Sun

  Restart libvirtd during test will break conn from framework. Update
  cases with:
  * Add notes in case description
  * Using get_conn to reconnect in cases

Signed-off-by: Wayne Sun g...@redhat.com
---
 repos/domain/ownership_test.py  |6 --
 repos/libvirtd/qemu_hang.py |1 -
 repos/libvirtd/restart.py   |3 +++
 repos/libvirtd/upstart.py   |3 +++
 repos/sVirt/domain_nfs_start.py |   11 ---
 5 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/repos/domain/ownership_test.py b/repos/domain/ownership_test.py
index b479708..33b57e2 100644
--- a/repos/domain/ownership_test.py
+++ b/repos/domain/ownership_test.py
@@ -3,6 +3,9 @@
 # check the ownership of saved domain file. Test could
 # be on local or root_squash nfs. The default owner of
 # the saved domain file is qemu:qemu in this case.
+#
+# NOTES: Libvirtd will be restarted during test, better run this
+# case alone.
 
 import os
 import re
@@ -11,7 +14,6 @@ import sys
 import libvirt
 from libvirt import libvirtError
 
-from src import sharedmod
 from utils import utils
 
 required_params = ('guestname', 'dynamic_ownership', 'use_nfs',)
@@ -180,7 +182,7 @@ def ownership_test(params):
 logger.error(failed to prepare the environment)
 return 1
 
-conn = sharedmod.libvirtobj['conn']
+conn = utils.get_conn()
 
 # save domain to the file
 logger.info(save domain %s to the file %s % (guestname, SAVE_FILE))
diff --git a/repos/libvirtd/qemu_hang.py b/repos/libvirtd/qemu_hang.py
index 7a58f50..9127ed6 100644
--- a/repos/libvirtd/qemu_hang.py
+++ b/repos/libvirtd/qemu_hang.py
@@ -17,7 +17,6 @@ required_params = ('guestname',)
 optional_params = {}
 
 VIRSH_LIST = virsh list --all
-RESTART_CMD = service libvirtd restart
 
 def check_domain_running(conn, guestname, logger):
  check if the domain exists, may or may not be active 
diff --git a/repos/libvirtd/restart.py b/repos/libvirtd/restart.py
index 803fa2e..e66f30a 100644
--- a/repos/libvirtd/restart.py
+++ b/repos/libvirtd/restart.py
@@ -2,6 +2,9 @@
 # Restart libvirtd testing. A running guest is required in
 # this test. During libvirtd restart, the guest remains
 # running and not affected by libvirtd restart.
+#
+# NOTES: Libvirtd will be restarted during test, better run this
+# case alone.
 
 import os
 import re
diff --git a/repos/libvirtd/upstart.py b/repos/libvirtd/upstart.py
index 13cb349..c57ba1c 100644
--- a/repos/libvirtd/upstart.py
+++ b/repos/libvirtd/upstart.py
@@ -1,5 +1,8 @@
 #!/usr/bin/env python
 # Upstart libvirtd testing
+#
+# NOTES: Libvirtd will be restarted during test, better run this
+# case alone.
 
 import os
 import re
diff --git a/repos/sVirt/domain_nfs_start.py b/repos/sVirt/domain_nfs_start.py
index 88d349c..5ce9a7a 100644
--- a/repos/sVirt/domain_nfs_start.py
+++ b/repos/sVirt/domain_nfs_start.py
@@ -5,6 +5,9 @@
 # check whether the guest can be started or not. The nfs could
 # be root_squash or no_root_squash. SElinux should be enabled
 # and enforcing on host.
+#
+# NOTES: Libvirtd will be restarted during test, better run this
+# case alone.
 
 import os
 import re
@@ -12,7 +15,6 @@ import sys
 
 import libvirt
 from libvirt import libvirtError
-
 from src import sharedmod
 from utils import utils
 from shutil import copy
@@ -171,6 +173,9 @@ def domain_nfs_start(params):
 logger.error(failed to prepare the environment)
 return 1
 
+# reconnect libvirt
+conn = utils.get_conn()
+
 domobj = conn.lookupByName(guestname)
 
 logger.info(begin to test start domain from nfs storage)
@@ -283,7 +288,7 @@ def domain_nfs_start(params):
 logger.error(Error: fail to get domain %s state % guestname)
 return 1
 
-if state != shutoff:
+if state != libvirt.VIR_DOMAIN_SHUTOFF:
 logger.info(shut down the domain %s % guestname)
 try:
 domobj.destroy()
@@ -407,7 +412,7 @@ def domain_nfs_start_clean(params):
 
 
 # Connect to local hypervisor connection URI
-conn = sharedmod.libvirtobj['conn']
+conn = utils.get_conn()
 domobj = conn.lookupByName(guestname)
 
 if domobj.info()[0] != libvirt.VIR_DOMAIN_SHUTOFF:
-- 
1.7.1

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

[libvirt] libvirtd crash when attach-disk to VM

2012-08-16 Thread Wangpan

Hi all,
I got a depressed problem(libvirtd crash with SIGABRT or SIGSEGV sometimes) 
when attach a nbd disk to a VM by using cmd as follow:
virsh attach-disk 228 --source /dev/nbd0 --target vdd --sourcetype block 
--driver qemu --subdriver raw
or just using  virsh attach-disk 228 /dev/nbd0 vdd.
and this problem occurs everytime when I attach a nbd disk to VM.

the device nbd0 is create by qemu-nbd cmd: qemu-nbd -c /dev/nbd0 
/home/hzwangpan/zero
root@114-113-199-15:/home/hzwangpan# qemu-nbd -V
qemu-nbd version 0.0.1
the 'zero' file was created by qemu-img cmd: qemu-img create -f raw zero 512M
qemu-img version 1.1.0, Copyright (c) 2004-2008 Fabrice Bellard

Some info of packages' version is listed below:
root@114-113-199-15:/home/hzwangpan# dpkg -l | grep libvi
ii  libvirt-bin0.9.12-4 
programs for the libvirt library
ii  libvirt-dev0.9.12-4 
development files for the libvirt library
ii  libvirt0   0.9.12-4 
library for interfacing with different virtualization systems
ii  libvirt0-dbg   0.9.12-4 
library for interfacing with different virtualization systems

root@114-113-199-15:/home/hzwangpan# virsh version
Compiled against library: libvir 0.9.12
Using library: libvir 0.9.12
Using API: QEMU 0.9.12
Running hypervisor: QEMU 1.1.0

root@114-113-199-15:/home/hzwangpan# kvm -version
QEMU emulator version 1.1.0 (qemu-kvm-1.1.0+dfsg-3, Debian), Copyright (c) 
2003-2008 Fabrice Bellard

root@114-113-199-15:/home/hzwangpan# uname -a
Linux 114-113-199-15 3.2.0-3-amd64 #1 SMP Thu Jun 28 09:07:26 UTC 2012 x86_64 
GNU/Linux

root@114-113-199-15:/home/hzwangpan# cat /etc/issue
Debian GNU/Linux wheezy/sid \n \l

the backtrace info when libvirtd crash is list here:
1) one situation:
*** glibc detected *** /usr/sbin/libvirtd: malloc(): memory corruption (fast): 
0x0087bc10 ***
=== Backtrace: =
/lib/x86_64-linux-gnu/libc.so.6(+0x75b46)[0x74608b46]
/lib/x86_64-linux-gnu/libc.so.6(+0x79428)[0x7460c428]
/lib/x86_64-linux-gnu/libc.so.6(__libc_malloc+0x70)[0x7460d960]
/lib/x86_64-linux-gnu/libc.so.6(__strdup+0x22)[0x74612912]
/usr/lib/libvirt.so.0(virJSONValueObjectAppend+0x39)[0x777b75c9]
/usr/lib/libvirt.so.0(virJSONValueObjectAppendString+0x37)[0x777b7e87]
/usr/sbin/libvirtd[0x4aa884]
/usr/sbin/libvirtd[0x4ac3a7]
/usr/sbin/libvirtd[0x49d303]
/usr/sbin/libvirtd[0x4a8bfe]
/usr/sbin/libvirtd[0x4b0814]
/usr/sbin/libvirtd[0x484a5d]
/usr/sbin/libvirtd[0x461d9e]
/usr/lib/libvirt.so.0(virDomainAttachDevice+0xdd)[0x77846f5d]
/usr/sbin/libvirtd[0x43ccfe]
/usr/lib/libvirt.so.0(+0x129866)[0x7788a866]
/usr/lib/libvirt.so.0(+0x1254d1)[0x778864d1]
/usr/lib/libvirt.so.0(+0x6273e)[0x777c373e]
/usr/lib/libvirt.so.0(+0x621c9)[0x777c31c9]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x6b50)[0x74d27b50]
/lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)[0x7466b6dd]
=== Memory map: 
0040-0053d000 r-xp  08:01 69457  
/usr/sbin/libvirtd
0073c000-0073e000 r--p 0013c000 08:01 69457  
/usr/sbin/libvirtd
0073e000-00746000 rw-p 0013e000 08:01 69457  
/usr/sbin/libvirtd
00746000-00aa1000 rw-p  00:00 0  [heap]
7fffe000-7fffe021f000 rw-p  00:00 0 
7fffe021f000-7fffe400 ---p  00:00 0 
7fffe77ff000-7fffe780 ---p  00:00 0 
7fffe780-7fffe800 rw-p  00:00 0 
7fffe800-7fffe8175000 rw-p  00:00 0 
7fffe8175000-7fffec00 ---p  00:00 0 
7fffec138000-7fffec139000 ---p  00:00 0 
7fffec139000-7fffec939000 rw-p  00:00 0 
7fffec939000-7fffec96e000 r--s  08:06 286722 
/var/cache/nscd/passwd
7fffec96e000-7fffec99a000 rw-p  00:00 0 
7fffec99a000-7fffec99b000 ---p  00:00 0 
7fffec99b000-7fffed19b000 rw-p  00:00 0 
7fffed19b000-7fffed315000 r-xp  08:01 124989 
/usr/lib/x86_64-linux-gnu/libdb-5.1.so
7fffed315000-7fffed515000 ---p 0017a000 08:01 124989 
/usr/lib/x86_64-linux-gnu/libdb-5.1.so
7fffed515000-7fffed51b000 r--p 0017a000 08:01 124989 
/usr/lib/x86_64-linux-gnu/libdb-5.1.so
7fffed51b000-7fffed51e000 rw-p 0018 08:01 124989 
/usr/lib/x86_64-linux-gnu/libdb-5.1.so
7fffed51e000-7fffed524000 r-xp  08:01 133549 
/usr/lib/x86_64-linux-gnu/sasl2/libsasldb.so.2.0.25
7fffed524000-7fffed723000 ---p 6000 08:01 133549 
/usr/lib/x86_64-linux-gnu/sasl2/libsasldb.so.2.0.25
7fffed723000-7fffed724000 r--p 5000 08:01 133549 
/usr/lib/x86_64-linux-gnu/sasl2/libsasldb.so.2.0.25
7fffed724000-7fffed725000 rw-p 6000 08:01 133549

Re: [libvirt] QEMU 1.2 Test Day - August 16 2012

2012-08-16 Thread Stefan Hajnoczi

On Tue, Aug 14, 2012 at 2:37 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Thu, Aug 2, 2012 at 1:22 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 I have set up the QEMU 1.2 Testing wiki page and suggest August 16 as
 the Test Day:

 http://wiki.qemu.org/Planning/1.2/Testing

 QEMU 1.2 Test Day is only 2 days away!

 Please help test QEMU 1.2-rc and add yourself to the wiki:
 http://wiki.qemu.org/Planning/1.2/Testing

 More about QEMU 1.2 Test Day from my original email:

An update on Test Day and the QEMU 1.2 release:

Postponing Test Day until -rc0 is tagged.

v1.2-rc0 has not been tagged yet.  Release blockers being discussed
and will be resolved before -rc0 is tagged.

If you are hitting build failures or critical bugs in qemu.git/master
at this point, please discuss on qemu-de...@nongnu.org as soon as
possible and mention 1.2 in the email subject line.

Stefan

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

Re: [libvirt] [test-API][PATCH 1/2] New get_conn function in utils

2012-08-16 Thread Osier Yang


On 2012年08月16日 17:00, Wayne Sun wrote:

   The get_conn function return connection object from libvirt module.
   This function could be used by both framework and testcases.
   The patch includes:
   * get_conn in utils/utils.py
   * sync env_inspect.py using the new function

Signed-off-by: Wayne Sung...@redhat.com
---
  src/env_inspect.py |   22 ++
  utils/utils.py |   27 +++
  2 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/src/env_inspect.py b/src/env_inspect.py
index b260ff8..2c1a701 100644
--- a/src/env_inspect.py
+++ b/src/env_inspect.py
@@ -20,6 +20,7 @@
  import commands
  import libvirt
  import sharedmod
+from utils import utils

  def check_libvirt(logger):
  virsh = 'virsh -v'
@@ -68,20 +69,6 @@ def hostinfo(logger):
  return 1
  return 0

-def request_credentials(credentials, user_data):
-for credential in credentials:
-if credential[0] == libvirt.VIR_CRED_AUTHNAME:
-credential[4] = user_data[0]
-
-if len(credential[4]) == 0:
-credential[4] = credential[3]
-elif credential[0] == libvirt.VIR_CRED_PASSPHRASE:
-credential[4] = user_data[1]
-else:
-return -1
-
-return 0
-
  def sharemod_init(env_parser, logger):
   get connection object from libvirt module
  initialize sharemod for use by testcases
@@ -89,12 +76,7 @@ def sharemod_init(env_parser, logger):
  uri = env_parser.get_value('variables', 'defaulturi')
  username = env_parser.get_value('variables', 'username')
  password = env_parser.get_value('variables', 'password')
-user_data = [username, password]
-auth = [[libvirt.VIR_CRED_AUTHNAME, libvirt.VIR_CRED_PASSPHRASE], 
request_credentials, user_data]
-conn = libvirt.openAuth(uri, auth, 0)
-if not conn:
-logger.error(Failed to setup libvirt connection);
-return 1
+conn = utils.get_conn(uri, username, password)

  # initialize conn object in sharedmod
  sharedmod.libvirtobj.clear()
diff --git a/utils/utils.py b/utils/utils.py
index be87cdc..eade10d 100644
--- a/utils/utils.py
+++ b/utils/utils.py
@@ -29,6 +29,7 @@ import struct
  import pexpect
  import string
  import subprocess
+import libvirt
  from xml.dom import minidom
  from urlparse import urlparse

@@ -57,6 +58,32 @@ def get_uri(ip):
  uri = qemu+ssh://%s/system % ip
  return uri

+def request_credentials(credentials, user_data):
+for credential in credentials:
+if credential[0] == libvirt.VIR_CRED_AUTHNAME:
+credential[4] = user_data[0]
+
+if len(credential[4]) == 0:
+credential[4] = credential[3]
+elif credential[0] == libvirt.VIR_CRED_PASSPHRASE:
+credential[4] = user_data[1]
+else:
+return -1
+
+return 0
+
+def get_conn(uri='', username='', password=''):
+ get connection object from libvirt module
+
+user_data = [username, password]
+auth = [[libvirt.VIR_CRED_AUTHNAME, libvirt.VIR_CRED_PASSPHRASE], 
request_credentials, user_data]
+conn = libvirt.openAuth(uri, auth, 0)
+if not conn:
+logger.error(Failed to setup libvirt connection);
+sys.exit(1)
+else:
+return conn


Isn't there a shared 'conn' in sharemod.py?


+
  def parse_uri(uri):
  # This is a simple parser for uri
  return urlparse(uri)


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

Re: [libvirt] [test-API][PATCH 1/2] New get_conn function in utils

2012-08-16 Thread Wayne Sun


On 08/16/2012 06:07 PM, Osier Yang wrote:

On 2012年08月16日 17:00, Wayne Sun wrote:

   The get_conn function return connection object from libvirt module.
   This function could be used by both framework and testcases.
   The patch includes:
   * get_conn in utils/utils.py
   * sync env_inspect.py using the new function

Signed-off-by: Wayne Sung...@redhat.com
---
  src/env_inspect.py |   22 ++
  utils/utils.py |   27 +++
  2 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/src/env_inspect.py b/src/env_inspect.py
index b260ff8..2c1a701 100644
--- a/src/env_inspect.py
+++ b/src/env_inspect.py
@@ -20,6 +20,7 @@
  import commands
  import libvirt
  import sharedmod
+from utils import utils

  def check_libvirt(logger):
  virsh = 'virsh -v'
@@ -68,20 +69,6 @@ def hostinfo(logger):
  return 1
  return 0

-def request_credentials(credentials, user_data):
-for credential in credentials:
-if credential[0] == libvirt.VIR_CRED_AUTHNAME:
-credential[4] = user_data[0]
-
-if len(credential[4]) == 0:
-credential[4] = credential[3]
-elif credential[0] == libvirt.VIR_CRED_PASSPHRASE:
-credential[4] = user_data[1]
-else:
-return -1
-
-return 0
-
  def sharemod_init(env_parser, logger):
   get connection object from libvirt module
  initialize sharemod for use by testcases
@@ -89,12 +76,7 @@ def sharemod_init(env_parser, logger):
  uri = env_parser.get_value('variables', 'defaulturi')
  username = env_parser.get_value('variables', 'username')
  password = env_parser.get_value('variables', 'password')
-user_data = [username, password]
-auth = [[libvirt.VIR_CRED_AUTHNAME, 
libvirt.VIR_CRED_PASSPHRASE], request_credentials, user_data]

-conn = libvirt.openAuth(uri, auth, 0)
-if not conn:
-logger.error(Failed to setup libvirt connection);
-return 1
+conn = utils.get_conn(uri, username, password)

  # initialize conn object in sharedmod
  sharedmod.libvirtobj.clear()
diff --git a/utils/utils.py b/utils/utils.py
index be87cdc..eade10d 100644
--- a/utils/utils.py
+++ b/utils/utils.py
@@ -29,6 +29,7 @@ import struct
  import pexpect
  import string
  import subprocess
+import libvirt
  from xml.dom import minidom
  from urlparse import urlparse

@@ -57,6 +58,32 @@ def get_uri(ip):
  uri = qemu+ssh://%s/system % ip
  return uri

+def request_credentials(credentials, user_data):
+for credential in credentials:
+if credential[0] == libvirt.VIR_CRED_AUTHNAME:
+credential[4] = user_data[0]
+
+if len(credential[4]) == 0:
+credential[4] = credential[3]
+elif credential[0] == libvirt.VIR_CRED_PASSPHRASE:
+credential[4] = user_data[1]
+else:
+return -1
+
+return 0
+
+def get_conn(uri='', username='', password=''):
+ get connection object from libvirt module
+
+user_data = [username, password]
+auth = [[libvirt.VIR_CRED_AUTHNAME, 
libvirt.VIR_CRED_PASSPHRASE], request_credentials, user_data]

+conn = libvirt.openAuth(uri, auth, 0)
+if not conn:
+logger.error(Failed to setup libvirt connection);
+sys.exit(1)
+else:
+return conn


Isn't there a shared 'conn' in sharemod.py?
Yes, but it will be broke when restart libvirtd and some case do need 
restart libvirtd. In those cases need a to get a new 'conn', so add this 
function for this. The 'conn' in sharemod.py is from sharemod_init in 
env_inspect.py, extract the get connection method from there to utils 
for benefit of the framework and testcases.


Wayne Sun



+
  def parse_uri(uri):
  # This is a simple parser for uri
  return urlparse(uri)




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

[libvirt] [PATCH v2] qemu: Set swap_hard_limit before hard_limit

2012-08-16 Thread Osier Yang

Setting hard_limit larger than previous swap_hard_limit must fail,
it's not that good if one wants to change the swap_hard_limit
and hard_limit together. E.g.

% virsh memtune rhel6
hard_limit : 100
soft_limit : 100
swap_hard_limit: 100

% virsh memtune rhel6 --hard-limit 120 --soft-limit 120 \
--swap-hard-limit 120 --live

This patch reoder the limits setting to set the swap_hard_limit
first, hard_limit then, and soft_limit last if it's greater than
current swap_hard_limit. And soft_limit first, hard_limit then,
swap_hard_limit last, if not.
---
 src/qemu/qemu_driver.c |   59 +--
 1 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 80cfa84..a26d3cd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6651,11 +6651,18 @@ qemuDomainSetMemoryParameters(virDomainPtr dom,
 virDomainDefPtr persistentDef = NULL;
 virCgroupPtr group = NULL;
 virDomainObjPtr vm = NULL;
+virTypedParameterPtr hard_limit = NULL;
+virTypedParameterPtr soft_limit = NULL;
+virTypedParameterPtr swap_hard_limit = NULL;
+virTypedParameterPtr sorted_params[nparams];
+unsigned long long val = 0;
+
 int ret = -1;
 int rc;
 
 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
   VIR_DOMAIN_AFFECT_CONFIG, -1);
+
 if (virTypedParameterArrayValidate(params, nparams,
VIR_DOMAIN_MEMORY_HARD_LIMIT,
VIR_TYPED_PARAM_ULLONG,
@@ -6694,13 +6701,49 @@ qemuDomainSetMemoryParameters(virDomainPtr dom,
 }
 }
 
+for (i = 0; i  nparams; i++) {
+if (STREQ(params[i].field, VIR_DOMAIN_MEMORY_HARD_LIMIT))
+hard_limit = params[i];
+else if (STREQ(params[i].field, VIR_DOMAIN_MEMORY_SOFT_LIMIT))
+soft_limit = params[i];
+else if (STREQ(params[i].field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT))
+swap_hard_limit = params[i];
+}
+
+/* It will fail if hard limit greater than swap hard limit anyway */
+if (swap_hard_limit 
+hard_limit 
+(hard_limit-value.ul  swap_hard_limit-value.ul)) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+   _(hard limit must be lower than swap hard limit));
+goto cleanup;
+}
+
+/* Get current swap hard limit */
+rc = virCgroupGetMemSwapHardLimit(group, val);
+if (rc != 0) {
+virReportSystemError(-rc, %s,
+ _(unable to get swap hard limit));
+goto cleanup;
+}
+
+if (swap_hard_limit-value.ul  val) {
+sorted_params[0] = swap_hard_limit;
+sorted_params[1] = hard_limit;
+sorted_params[2] = soft_limit;
+} else {
+sorted_params[0] = soft_limit;
+sorted_params[1] = hard_limit;
+sorted_params[2] = swap_hard_limit;
+}
+
 ret = 0;
 for (i = 0; i  nparams; i++) {
-virTypedParameterPtr param = params[i];
+virTypedParameterPtr param = sorted_params[i];
 
 if (STREQ(param-field, VIR_DOMAIN_MEMORY_HARD_LIMIT)) {
 if (flags  VIR_DOMAIN_AFFECT_LIVE) {
-rc = virCgroupSetMemoryHardLimit(group, params[i].value.ul);
+rc = virCgroupSetMemoryHardLimit(group, 
sorted_params[i]-value.ul);
 if (rc != 0) {
 virReportSystemError(-rc, %s,
  _(unable to set memory hard_limit 
tunable));
@@ -6709,11 +6752,11 @@ qemuDomainSetMemoryParameters(virDomainPtr dom,
 }
 
 if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
-persistentDef-mem.hard_limit = params[i].value.ul;
+persistentDef-mem.hard_limit = sorted_params[i]-value.ul;
 }
 } else if (STREQ(param-field, VIR_DOMAIN_MEMORY_SOFT_LIMIT)) {
 if (flags  VIR_DOMAIN_AFFECT_LIVE) {
-rc = virCgroupSetMemorySoftLimit(group, params[i].value.ul);
+rc = virCgroupSetMemorySoftLimit(group, 
sorted_params[i]-value.ul);
 if (rc != 0) {
 virReportSystemError(-rc, %s,
  _(unable to set memory soft_limit 
tunable));
@@ -6722,11 +6765,11 @@ qemuDomainSetMemoryParameters(virDomainPtr dom,
 }
 
 if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
-persistentDef-mem.soft_limit = params[i].value.ul;
+persistentDef-mem.soft_limit = sorted_params[i]-value.ul;
 }
 } else if (STREQ(param-field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) {
 if (flags  VIR_DOMAIN_AFFECT_LIVE) {
-rc = virCgroupSetMemSwapHardLimit(group, params[i].value.ul);
+rc = virCgroupSetMemSwapHardLimit(group, 
sorted_params[i]-value.ul);
 if (rc != 0) {
 virReportSystemError(-rc, %s,

Re: [libvirt] [PATCH v2] qemu: Set swap_hard_limit before hard_limit

2012-08-16 Thread Martin Kletzander

On 08/16/2012 12:37 PM, Osier Yang wrote:
 Setting hard_limit larger than previous swap_hard_limit must fail,
 it's not that good if one wants to change the swap_hard_limit
 and hard_limit together. E.g.
 
 % virsh memtune rhel6
 hard_limit : 100
 soft_limit : 100
 swap_hard_limit: 100
 
 % virsh memtune rhel6 --hard-limit 120 --soft-limit 120 \
 --swap-hard-limit 120 --live
 
 This patch reoder the limits setting to set the swap_hard_limit
 first, hard_limit then, and soft_limit last if it's greater than
 current swap_hard_limit. And soft_limit first, hard_limit then,
 swap_hard_limit last, if not.
 ---
  src/qemu/qemu_driver.c |   59 +--
  1 files changed, 51 insertions(+), 8 deletions(-)
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 80cfa84..a26d3cd 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -6651,11 +6651,18 @@ qemuDomainSetMemoryParameters(virDomainPtr dom,
  virDomainDefPtr persistentDef = NULL;
  virCgroupPtr group = NULL;
  virDomainObjPtr vm = NULL;
 +virTypedParameterPtr hard_limit = NULL;
 +virTypedParameterPtr soft_limit = NULL;
 +virTypedParameterPtr swap_hard_limit = NULL;
 +virTypedParameterPtr sorted_params[nparams];

You allocate the correct number of params here, but ...

 @@ -6694,13 +6701,49 @@ qemuDomainSetMemoryParameters(virDomainPtr dom,
  }
  }
  
 +for (i = 0; i  nparams; i++) {
 +if (STREQ(params[i].field, VIR_DOMAIN_MEMORY_HARD_LIMIT))
 +hard_limit = params[i];
 +else if (STREQ(params[i].field, VIR_DOMAIN_MEMORY_SOFT_LIMIT))
 +soft_limit = params[i];
 +else if (STREQ(params[i].field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT))
 +swap_hard_limit = params[i];
 +}
 +
 +/* It will fail if hard limit greater than swap hard limit anyway */
 +if (swap_hard_limit 
 +hard_limit 
 +(hard_limit-value.ul  swap_hard_limit-value.ul)) {
 +virReportError(VIR_ERR_INVALID_ARG, %s,
 +   _(hard limit must be lower than swap hard limit));
 +goto cleanup;
 +}
 +
 +/* Get current swap hard limit */
 +rc = virCgroupGetMemSwapHardLimit(group, val);
 +if (rc != 0) {
 +virReportSystemError(-rc, %s,
 + _(unable to get swap hard limit));
 +goto cleanup;
 +}
 +
 +if (swap_hard_limit-value.ul  val) {
 +sorted_params[0] = swap_hard_limit;
 +sorted_params[1] = hard_limit;
 +sorted_params[2] = soft_limit;
 +} else {
 +sorted_params[0] = soft_limit;
 +sorted_params[1] = hard_limit;
 +sorted_params[2] = swap_hard_limit;
 +}

... you always access all 3 of them and you go through this list.
Unfortunately this means that in case you want to change only one value
(e.g. hard limit), you will dereference NULL couple of lines later and
it will crash libvirtd (tried). To test this, you can try running 'virsh
memtune rhel6 --hard-limit 120' in your previous scenario.

Martin.

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

Re: [libvirt] [PATCH 2/7] add qemuAgentArbitraryCommand() for general qemu agent command.

2012-08-16 Thread Martin Kletzander

On 08/15/2012 03:36 AM, MATSUDA Daiki wrote:
 add qemuAgentArbitraryCommand() for general qemu agent command.


Twice the same line in the message, could be less brief, maybe :)

 Signed-off-by: MATSUDA Daiki matsuda...@intellilink.co.jp
 ---
  src/qemu/qemu_agent.c |   31 +++
  src/qemu/qemu_agent.h |5 +
  2 files changed, 36 insertions(+), 0 deletions(-)

 diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
 index 26c2726..c634c23 100644
 --- a/src/qemu/qemu_agent.c
 +++ b/src/qemu/qemu_agent.c
 @@ -1408,3 +1408,34 @@ qemuAgentSuspend(qemuAgentPtr mon,
  virJSONValueFree(reply);
  return ret;
  }
 +
 +int
 +qemuAgentArbitraryCommand(qemuAgentPtr mon,
 +  const char *cmd_str,
 +  char **result,
 +  int timeout)
 +{
 +int ret = -1;
 +virJSONValuePtr cmd;
 +virJSONValuePtr reply = NULL;
 +
 +if (timeout  -2)
 +return ret;

I think you can leave the test to only one layer of the code and that
would be 'qemuAgentSend'.

 +
 +cmd = virJSONValueFromString(cmd_str);
 +if (!cmd)
 +return ret;
 +
 +ret = qemuAgentCommand(mon, cmd, reply, timeout);
 +
 +if (ret == 0) {
 +ret = qemuAgentCheckError(cmd, reply);
 +*result = virJSONValueToString(reply);
 +} else {
 +result = NULL;

First of all, you probably wanted to do '*result = NULL', because
'result = NULL' won't do anything here, however in case of any other
failure (timeout == -3, fail in virJSONValuFromSting) you don't
guarantee the *result to be NULL, so I don't think you need to do this here.

Martin

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

Re: [libvirt] [PATCH 1/7] Add @seconds vaiable to qemuAgentSend()

2012-08-16 Thread Martin Kletzander

On 08/15/2012 03:36 AM, MATSUDA Daiki wrote:
 
  Add @seconds variable to qemuAgentSend().
  When @tiemout is true, @seconds controls how long to wait for a response
  (if @seconds is VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT, default to
  QEMU_AGENT_WAIT_TIME).
  If @timeout is false, @seconds is ignored.
 
 
 Signed-off-by: MATSUDA Daiki matsuda...@intellilink.co.jp
 ---
  include/libvirt/libvirt-qemu.h |6 ++
  src/qemu/qemu_agent.c  |   30 --
  2 files changed, 26 insertions(+), 10 deletions(-)
 
 diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
 index a37f897..013ed5a 100644
 --- a/include/libvirt/libvirt-qemu.h
 +++ b/include/libvirt/libvirt-qemu.h
 @@ -44,6 +44,12 @@ virDomainPtr virDomainQemuAttach(virConnectPtr domain,
   unsigned int pid_value,
   unsigned int flags);
  
 +typedef enum {
 +VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2,

Correct me if I'm wrong, but isn't this the same as setting timeout to
false?

 +VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1,
 +VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
 +} virDomainQemuAgentCommandTimeoutValues;
 +
  # ifdef __cplusplus
  }
  # endif
 diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
 index 15af758..26c2726 100644
 --- a/src/qemu/qemu_agent.c
 +++ b/src/qemu/qemu_agent.c
 @@ -837,6 +837,8 @@ void qemuAgentClose(qemuAgentPtr mon)
   * @mon: Monitor
   * @msg: Message
   * @timeout: use timeout?
 + * @seconds: timeout seconds. if VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT and
 + *   @timeout is true, use default value.

If my previous assumption is correct, than the timeout parameter can be
completely thrown away.

   *
   * Send @msg to agent @mon.
   * Wait max QEMU_AGENT_WAIT_TIME for agent
 @@ -848,7 +850,8 @@ void qemuAgentClose(qemuAgentPtr mon)
   */
  static int qemuAgentSend(qemuAgentPtr mon,
   qemuAgentMessagePtr msg,
 - bool timeout)
 + bool timeout,
 + int seconds)
  {
  int ret = -1;
  unsigned long long now, then = 0;
 @@ -864,7 +867,8 @@ static int qemuAgentSend(qemuAgentPtr mon,
  if (timeout) {
  if (virTimeMillisNow(now)  0)
  return -1;
 -then = now + QEMU_AGENT_WAIT_TIME;
 +then = now + (seconds == VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT ?
 +  QEMU_AGENT_WAIT_TIME : seconds * 1000ull);
  }

Also if seconds == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK, then this causes
'then' to be smaller than now. I'm not sure what that would do with
pthread_cond_timedwait, but that's definitely not wanted behavior.

Martin

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

Re: [libvirt] [PATCH 3/7] add virAgentCommand()

2012-08-16 Thread Martin Kletzander

On 08/15/2012 03:36 AM, MATSUDA Daiki wrote:
 diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
 index 013ed5a..60b83ef 100644
 --- a/include/libvirt/libvirt-qemu.h
 +++ b/include/libvirt/libvirt-qemu.h
 @@ -50,6 +50,9 @@ typedef enum {
  VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
  } virDomainQemuAgentCommandTimeoutValues;
  
 +char *virAgentCommand(virDomainPtr domain, const char *cmd,
 +  int timeout, unsigned int flags);
 +

I wondered why this is type 'char *', when we can use 'int' as usual and
have '**result' (same as monitor command does).

 diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c
 index 78480bb..49dfc20 100644
 --- a/src/libvirt-qemu.c
 +++ b/src/libvirt-qemu.c
 @@ -185,3 +185,54 @@ error:
  virDispatchError(conn);
  return NULL;
  }
 +
 +/**
 + * virAgentCommand:
 + * @domain: a domain object
 + * @cmd: the guest agent command string
 + * @timeout: timeout seconds
 + * @flags: execution flags
 + *
 + * Execute an arbitrary Guest Agent command.
 + *
 + * Issue @cmd to the guest agent running in @domain.
 + * If @result is NULL, then don't wait for a result (and @timeout

And then I noticed that there is no @result here, so maybe you wanted to
make it that way and didn't change something?

 + * must be 0).  Otherwise, wait for @timeout seconds for a
 + * @timeout must be -2, -1, 0 or positive.
 + * VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK(-2): meaning to block forever waiting 
 for
 + * a result.
 + * VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT(-1): use default timeout value.
 + * VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT(0): does not wait.
 + * positive value: wait for @timeout seconds
 + *
 + * Returns strings if success, NULL in failure.
 + */
 +char *
 +virAgentCommand(virDomainPtr domain,
 +const char *cmd,
 +int timeout,
 +unsigned int flags)
 +{
 +virConnectPtr conn;
 +
 +VIR_DEBUG(domain=%p, cmd=%s, timeout=%d, flags=%x,
 +  domain, cmd, timeout, flags);
 +
 +if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
 +virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
 +virDispatchError(NULL);
 +return NULL;
 +}
 +
 +conn = domain-conn;
 +
 +if (conn-driver-qemuAgentCommand) {
 +return conn-driver-qemuAgentCommand(domain, cmd, timeout, flags);

Shouldn't this be allowed only for read-only connections?

 +}
 +
 +virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
 +
 +/* Copy to connection error object for back compatability */

s/compatability/compatibility/

 +virDispatchError(domain-conn);

You have 'domain-conn' in 'conn' already.

Martin

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

Re: [libvirt] [PATCH 5/7] add qemuAgentCommand() for .qemuAgentCommand to remote driver

2012-08-16 Thread Martin Kletzander

On 08/15/2012 03:36 AM, MATSUDA Daiki wrote:
 diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
 index 353a153..3c60709 100644
 --- a/src/remote/remote_driver.c
 +++ b/src/remote/remote_driver.c
 @@ -5368,6 +5368,7 @@ static virDriver remote_driver = {
  .domainSetMetadata = remoteDomainSetMetadata, /* 0.9.10 */
  .domainGetMetadata = remoteDomainGetMetadata, /* 0.9.10 */
  .domainGetHostname = remoteDomainGetHostname, /* 0.10.0 */
 +.qemuAgentCommand = qemuAgentCommand, /* 0.10.0 */

You probably want to call some function like
'remoteQemuDomainAgentCommand' or something like that since
qemuAgentCommand works locally and not remotely.

I haven't had a look at the rest of the series. Please feel free to
correct me in case I've misunderstood the code somewhere.

Martin

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

Re: [libvirt] [PATCH 1/2] util: properly save/restore original vlan tag for VFs

2012-08-16 Thread Eric Blake

On 08/16/2012 02:34 AM, Osier Yang wrote:


 -if (virMacAddrParse(macstr,oldmac) != 0) {
 +if ((vlan = strchr(fileData, '\n'))) {
 +char *endptr;
 +
 +*vlan++ = 0; /* NULL terminate the mac address */
 
 s/terminate/terminates/

Actually, 'terminate' is correct - this is a comment describing what the
assignment is good for (convert arbitrary contents into 0 in order to
NUL-terminate the vlan variable), not the vlan format itself (a valid
vlan exists when NUL terminates the string).

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



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

Re: [libvirt] [User question] Huge buffer size on KVM host

2012-08-16 Thread Avi Kivity

On 08/16/2012 05:54 PM, Martin Wawro wrote:
 
 On Aug 15, 2012, at 2:57 PM, Avi Kivity wrote:
 
 
 We are using logical volumes and the cache is set to 'none'.
 
 Strange, that should work without any buffering.
 
 What the contents of
 
  /sys/block/sda/queue/hw_sector_size
 
 and
 
  /sys/block/sda/queue/logical_block_size
 
 ?
 
 
 Hi Avi,
 
 It seems that the kernel on that particular machine is too old, those entries 
 are
 not featured. We checked on a comparable setup with a newer kernel and both 
 entries
 were set to 512.
 
 We also did have a third more thorough look on the caching. It turns out that 
 the
 virt-manager does not seem to honor the caching adjusted in the GUI correctly.
 We disabled caching on all virtual devices for this particular VM and checking
 with ps -fxal revealed, that only one of those devices (and a rather small 
 one too)
 had this set. We corrected this in the XML file directly and the buffer size
 currently resides at around 1.8 GB after rebooting the VM (the only virtio 
 device
 not having the cache=none option set is now the (non-mounted) cdrom).
 

cc += libvirt-list

Is there a reason that cdroms don't get cache=none?


-- 
error compiling committee.c: too many arguments to function

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

[libvirt] [PATCH 2/7] network: helper function to create interface pool from PF

2012-08-16 Thread Shradha Shah

  Existing code that creates a list of forwardIfs from a single PF
  was moved to the new utility function networkCreateInterfacePool.
  No functional change.

Signed-off-by: Shradha Shah ss...@solarflare.com
---
 src/network/bridge_driver.c |   82 ++-
 1 files changed, 50 insertions(+), 32 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 474bbfa..ff17c7f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2771,6 +2771,55 @@ int networkRegister(void) {
  * backend function table.
  */
 
+/* networkCreateInterfacePool:
+ * @netdef: the original NetDef from the network
+ *
+ * Creates an implicit interface pool of VF's when a PF dev is given
+ */
+static int 
+networkCreateInterfacePool(virNetworkDefPtr netdef) {
+unsigned int num_virt_fns = 0;
+char **vfname = NULL;
+int ret = -1, ii = 0;
+
+if ((virNetDevGetVirtualFunctions(netdef-forwardPfs-dev,
+  vfname, num_virt_fns))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Could not get Virtual functions on %s),
+   netdef-forwardPfs-dev);
+goto finish;
+}
+
+if (num_virt_fns == 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(No Vf's present on SRIOV PF %s),
+   netdef-forwardPfs-dev);
+   goto finish;
+}
+
+if ((VIR_ALLOC_N(netdef-forwardIfs, num_virt_fns))  0) {
+virReportOOMError();
+goto finish;
+}
+
+netdef-nForwardIfs = num_virt_fns;
+
+for (ii = 0; ii  netdef-nForwardIfs; ii++) {
+netdef-forwardIfs[ii].dev = strdup(vfname[ii]);
+if (!netdef-forwardIfs[ii].dev) {
+virReportOOMError();
+goto finish;
+}
+}
+
+ret = 0;
+finish:
+for (ii = 0; ii  num_virt_fns; ii++)
+VIR_FREE(vfname[ii]);
+VIR_FREE(vfname);
+return ret;
+}
+
 /* networkAllocateActualDevice:
  * @iface: the original NetDef from the domain
  *
@@ -2793,8 +2842,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
 virNetDevVPortProfilePtr virtport = iface-virtPortProfile;
 virNetDevVlanPtr vlan = NULL;
 virNetworkForwardIfDefPtr dev = NULL;
-unsigned int num_virt_fns = 0;
-char **vfname = NULL;
 int ii;
 int ret = -1;
 
@@ -2969,35 +3016,9 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
  */
 if (netdef-forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) {
 if ((netdef-nForwardPfs  0)  (netdef-nForwardIfs = 0)) {
-if ((virNetDevGetVirtualFunctions(netdef-forwardPfs-dev,
-  vfname, num_virt_fns)) 
 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(Could not get Virtual functions on 
%s),
-   netdef-forwardPfs-dev);
+if ((networkCreateInterfacePool(netdef))  0) {
 goto error;
 }
-
-if (num_virt_fns == 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(No Vf's present on SRIOV PF %s),
-   netdef-forwardPfs-dev);
-goto error;
-}
-
-if ((VIR_ALLOC_N(netdef-forwardIfs, num_virt_fns))  0) {
-virReportOOMError();
-goto error;
-}
-
-netdef-nForwardIfs = num_virt_fns;
-
-for (ii = 0; ii  netdef-nForwardIfs; ii++) {
-netdef-forwardIfs[ii].dev = strdup(vfname[ii]);
-if (!netdef-forwardIfs[ii].dev) {
-virReportOOMError();
-goto error;
-}
-}
 }
 
 /* pick first dev with 0 connections */
@@ -3105,9 +3126,6 @@ validate:
 ret = 0;
 
 cleanup:
-for (ii = 0; ii  num_virt_fns; ii++)
-VIR_FREE(vfname[ii]);
-VIR_FREE(vfname);
 if (network)
 virNetworkObjUnlock(network);
 return ret;
-- 
1.7.4.4


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

[libvirt] [PATCH 1/7] conf: move DevicePCIAddress functions to separate file

2012-08-16 Thread Shradha Shah

Move the functions the parse/format, and validate PCI addresses to
their own file so they can be conveniently used in other places
besides device_conf.c

Refactoring existing code without causing any functional changes to
prepare for new code.

This patch makes the code reusable.

Signed-off-by: Shradha Shah ss...@solarflare.com
---
 include/libvirt/virterror.h  |1 +
 src/Makefile.am  |7 ++-
 src/conf/device_conf.c   |  131 ++
 src/conf/device_conf.h   |   65 +
 src/conf/domain_conf.c   |  114 +
 src/conf/domain_conf.h   |   25 +---
 src/libvirt_private.syms |   10 ++-
 src/qemu/qemu_command.c  |   13 ++--
 src/qemu/qemu_hotplug.c  |7 +-
 src/qemu/qemu_monitor.c  |   14 ++--
 src/qemu/qemu_monitor.h  |   17 +++---
 src/qemu/qemu_monitor_json.c |   14 ++--
 src/qemu/qemu_monitor_json.h |   14 ++--
 src/qemu/qemu_monitor_text.c |   16 +++---
 src/qemu/qemu_monitor_text.h |   14 ++--
 src/util/virterror.c |3 +-
 src/xen/xend_internal.c  |3 +-
 17 files changed, 287 insertions(+), 181 deletions(-)

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 913fc5d..d0af43d 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -110,6 +110,7 @@ typedef enum {
 VIR_FROM_AUTH = 46, /* Error from auth handling */
 VIR_FROM_DBUS = 47, /* Error from DBus */
 VIR_FROM_PARALLELS = 48,/* Error from Parallels */
+VIR_FROM_DEVICE = 49,   /* Error from Device */
 
 # ifdef VIR_ENUM_SENTINELS
 VIR_ERR_DOMAIN_LAST
diff --git a/src/Makefile.am b/src/Makefile.am
index b5f8056..ad24534 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -199,6 +199,10 @@ CPU_CONF_SOURCES = 
\
 CONSOLE_CONF_SOURCES = \
conf/virconsole.c conf/virconsole.h
 
+# Device Helper APIs
+DEVICE_CONF_SOURCES =   \
+   conf/device_conf.c conf/device_conf.h
+
 CONF_SOURCES = \
$(NETDEV_CONF_SOURCES)  \
$(DOMAIN_CONF_SOURCES)  \
@@ -211,7 +215,8 @@ CONF_SOURCES =  
\
$(INTERFACE_CONF_SOURCES)   \
$(SECRET_CONF_SOURCES)  \
$(CPU_CONF_SOURCES) \
-   $(CONSOLE_CONF_SOURCES)
+   $(CONSOLE_CONF_SOURCES) \
+   $(DEVICE_CONF_SOURCES)
 
 # The remote RPC driver, covering domains, storage, networks, etc
 REMOTE_DRIVER_GENERATED = \
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
new file mode 100644
index 000..ca600c5
--- /dev/null
+++ b/src/conf/device_conf.c
@@ -0,0 +1,131 @@
+/*
+ * device_conf.c: device XML handling
+ *
+ * Copyright (C) 2006-2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Author: Daniel P. Berrange berra...@redhat.com
+ */
+
+#include config.h
+#include virterror_internal.h
+#include datatypes.h
+#include memory.h
+#include xml.h
+#include uuid.h
+#include util.h
+#include buf.h
+#include device_conf.h
+
+#define VIR_FROM_THIS VIR_FROM_DEVICE
+
+VIR_ENUM_IMPL(virDeviceAddressPciMulti,
+  VIR_DEVICE_ADDRESS_PCI_MULTI_LAST,
+  default,
+  on,
+  off)
+
+int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr)
+{
+/* PCI bus has 32 slots and 8 functions per slot */
+if (addr-slot = 32 || addr-function = 8)
+return 0;
+return addr-domain || addr-bus || addr-slot;
+}
+
+
+int
+virDevicePCIAddressParseXML(xmlNodePtr node,
+virDevicePCIAddressPtr addr)
+{
+char *domain, *slot, *bus, *function, *multi;
+int ret = -1;
+
+memset(addr, 0, sizeof(*addr));
+
+domain   = virXMLPropString(node, domain);
+bus  = virXMLPropString(node, bus);
+slot = virXMLPropString(node, slot);
+function = virXMLPropString(node, function);
+multi

Re: [libvirt] [PATCH 0/8] Honour current process label when generating SELinux labels

2012-08-16 Thread Viktor Mihajlovski


On 08/10/2012 03:47 PM, Daniel P. Berrange wrote:

This patch series makes a number of changes to the SELinux label
generation code. This is intended to make it fully honour the
current process label when generating VM labels, so that dynamic
label generation works better with custom policies, or confined
user accounts.

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



Unfortunately I am not selinux-savvy enough to understand exactly why, 
but I cannot start guests any more after pulling master.


The issue is that the virtual disk's security context (a block device in 
this case) cannot be set, message shown below.


012-08-16 15:02:18.891+: 1536: error : 
virSecuritySELinuxSetFileconHelper:652 : unable to set security context 
'system_u:system_r:svirt_image_t:s0:c786,c986' on 
'/dev/disk/by-path/ccw-0.0.3770-part1': Invalid argument


Prior to that the security context would have looked like this
system_u:object_r:svirt_image_t:s0:c153,c923, i.e. using object_r 
instead of system_r.


I am running on RHEL 6.2, not sure whether this is relevant.

--

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research  Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

[libvirt] [PATCH 6/7] Forward Mode Hostdev network driver Implementation

2012-08-16 Thread Shradha Shah

This patch updates the network driver to properly utilize the new
attributes/elements that are now in virNetworkDef

Signed-off-by: Shradha Shah ss...@solarflare.com
---
 docs/formatnetwork.html.in  |   62 ++
 src/network/bridge_driver.c |  282 ++-
 2 files changed, 284 insertions(+), 60 deletions(-)

diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index ed9f7a9..5ab5f3c 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -223,6 +223,37 @@
 (usually either a domain start, or a hotplug interface
 attach to a domain).span class=sinceSince 0.9.4/span
   /dd
+  dtcodehostdev/code/dt
+  dd
+This network facilitates PCI Passthrough of a network device.
+A network device is chosen from the interface pool and 
+directly assigned to the guest using generic device
+passthrough, after first optionally setting the device's MAC
+address to the configured value, and optionally associating 
+the device with an 802.1Qbh capable switch using an optionally 
+specified codelt;virtualportgt;/code element. 
+Note that - due to limitations in standard single-port PCI 
+ethernet card driver design - only SR-IOV (Single Root I/O 
+Virtualization) virtual function (VF) devices can be assigned 
+in this manner; to assign a standard single-port PCI or PCIe 
+ethernet card to a guest, use the traditional codelt;
+hostdevgt;/code device definition. span class=since
+Since 0.10.0/span
+
+pNote that this intelligent passthrough of network devices is
+very similar to the functionality of a standard codelt;
+hostdevgt;/code device, the difference being that this 
+method allows specifying a MAC address and codelt;virtualport
+gt;/code for the passed-through device. If these capabilities 
+are not required, if you have a standard single-port PCI, PCIe, 
+or USB network card that doesn't support SR-IOV (and hence would 
+anyway lose the configured MAC address during reset after being 
+assigned to the guest domain), or if you are using a version of 
+libvirt older than 0.10.0, you should use a standard 
+codelt;hostdevgt;/code definition to assign the device to 
+the guest instead of codelt;forward mode='hostdev'/gt;/code.
+/p
+  /dd
 /dl
 As mentioned above, a codelt;forwardgt;/code element can
 have multiple codelt;interfacegt;/code subelements, each
@@ -272,6 +303,37 @@
 particular, 'passthrough' mode, and 'private' mode when using
 802.1Qbh), libvirt will choose an unused physical interface
 or, if it can't find an unused interface, fail the operation./p
+
+span class=sincesince 0.9.12/span and when using forward mode 
+'hostdev' we specify the interface pool by using the 
+codelt;addressgt;/code element and codelt;
+typegt;/code codelt;domaingt;/code codelt;busgt;/code
+codelt;slotgt;/code and codelt;functiongt;/code
+sub-elements.
+
+pre
+...
+  lt;forward mode='hostdev' managed='yes'gt;
+lt;address type='pci' domain='0' bus='4' slot='0' function='1'/gt;
+lt;address type='pci' domain='0' bus='4' slot='0' function='2'/gt;
+lt;address type='pci' domain='0' bus='4' slot='0' function='3'/gt;
+  lt;/forwardgt; 
+...
+/pre
+
+Alternatively the interface pool can also be defined using a
+single physical function  codelt;pfgt;/code subelement to 
+call out the  corresponding physical interface associated with 
+multiple virtual interfaces (similar to passthrough mode):
+
+pre
+...
+  lt;forward mode='hostdev' managed='yes'gt;
+lt;pf dev='eth0'/gt;
+  lt;/forwardgt;
+...
+/pre
+
   /dd
 /dl
 h5a name=elementQoSQuality of service/a/h5
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 38f6d12..065af3e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -47,6 +47,7 @@
 #include datatypes.h
 #include bridge_driver.h
 #include network_conf.h
+#include device_conf.h
 #include driver.h
 #include buf.h
 #include virpidfile.h
@@ -1935,7 +1936,7 @@ networkStartNetworkExternal(struct network_driver *driver 
ATTRIBUTE_UNUSED,
 virNetworkObjPtr network ATTRIBUTE_UNUSED)
 {
 /* put anything here that needs to be done each time a network of
- * type BRIDGE, PRIVATE, VEPA, or PASSTHROUGH is started. On
+ * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is started. On
  * failure, undo anything you've done, and return -1. On success
  * return 0.
  */
@@ -1946,7 +1947,7 @@ static int

[libvirt] [PATCH 5/7] Add function virDevicePCIAddressEqual

2012-08-16 Thread Shradha Shah

This function is needed by the network driver in a later commit.
This function is useful in functions like networkNotifyActualDevice
and networkReleaseActualDevice
---
 src/conf/device_conf.c   |   16 
 src/conf/device_conf.h   |3 +++
 src/libvirt_private.syms |1 +
 3 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index ca600c5..8edcc0a 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -129,3 +129,19 @@ virDevicePCIAddressFormat(virBufferPtr buf,
   addr.function);
 return 0;
 }
+
+int
+virDevicePCIAddressEqual(virDevicePCIAddress addr1,
+ virDevicePCIAddress addr2)
+{
+int ret = -1;
+
+if (addr1.domain == addr2.domain 
+addr1.bus == addr2.bus 
+addr1.slot == addr2.slot 
+addr1.function == addr2.function) {
+ret = 0;
+}
+
+return ret;
+}
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index c679bce..7c4d356 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -59,6 +59,9 @@ int virDevicePCIAddressFormat(virBufferPtr buf,
   virDevicePCIAddress addr,
   bool includeTypeInAddr);
 
+int virDevicePCIAddressEqual(virDevicePCIAddress addr1,
+ virDevicePCIAddress addr2);
+
 
 VIR_ENUM_DECL(virDeviceAddressPciMulti)
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1f32f8e..063d0bc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -224,6 +224,7 @@ virDeviceAddressPciMultiTypeToString;
 virDevicePCIAddressIsValid;
 virDevicePCIAddressParseXML;
 virDevicePCIAddressFormat;
+virDevicePCIAddressEqual;
 
 # dnsmasq.h
 dnsmasqAddDhcpHost;
-- 
1.7.4.4


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

[libvirt] [PATCH 3/7] RNG updates, new xml parser/formatter code to support forward mode=hostdev

2012-08-16 Thread Shradha Shah

This patch introduces the new forward mode='hostdev' along with attribute 
managed
Includes updates to the network RNG and new xml parser/formatter code.

Signed-off-by: Shradha Shah ss...@solarflare.com
---
 docs/schemas/basictypes.rng|   46 +++
 docs/schemas/domaincommon.rng  |   44 ---
 docs/schemas/network.rng   |   53 ++---
 src/conf/network_conf.c|  130 +++-
 src/conf/network_conf.h|   26 ++-
 src/network/bridge_driver.c|   18 ++--
 tests/networkxml2xmlin/hostdev-pf.xml  |7 ++
 tests/networkxml2xmlin/hostdev.xml |   10 +++
 tests/networkxml2xmlout/hostdev-pf.xml |7 ++
 tests/networkxml2xmlout/hostdev.xml|   10 +++
 tests/networkxml2xmltest.c |2 +
 11 files changed, 266 insertions(+), 87 deletions(-)

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 9dbda4a..766f9a0 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -54,6 +54,31 @@
 /choice
   /define
 
+  define name=pciaddress
+optional
+  attribute name=domain
+ref name=pciDomain/
+  /attribute
+/optional
+attribute name=bus
+  ref name=pciBus/
+/attribute
+attribute name=slot
+  ref name=pciSlot/
+/attribute
+attribute name=function
+  ref name=pciFunc/
+/attribute
+optional
+  attribute name=multifunction
+choice
+  valueon/value
+  valueoff/value
+/choice
+  /attribute
+/optional
+  /define
+
   !-- a 6 byte MAC address in ASCII-hex format, eg 12:34:56:78:9A:BC --
   !-- The lowest bit of the 1st byte is the multicast bit. a --
   !-- uniMacAddr requires that bit to be 0, and a multiMacAddr --
@@ -167,4 +192,25 @@
 ref name='unsignedLong'/
   /define
 
+  define name=pciDomain
+data type=string
+  param name=pattern(0x)?[0-9a-fA-F]{1,4}/param
+/data
+  /define
+  define name=pciBus
+data type=string
+  param name=pattern(0x)?[0-9a-fA-F]{1,2}/param
+/data
+  /define
+  define name=pciSlot
+data type=string
+  param name=pattern(0x)?[0-1]?[0-9a-fA-F]/param
+/data
+  /define
+  define name=pciFunc
+data type=string
+  param name=pattern(0x)?[0-7]/param
+/data
+  /define
+
 /grammar
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4903ca6..35e9f82 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2652,30 +2652,6 @@
   /attribute
 /optional
   /define
-  define name=pciaddress
-optional
-  attribute name=domain
-ref name=pciDomain/
-  /attribute
-/optional
-attribute name=bus
-  ref name=pciBus/
-/attribute
-attribute name=slot
-  ref name=pciSlot/
-/attribute
-attribute name=function
-  ref name=pciFunc/
-/attribute
-optional
-  attribute name=multifunction
-choice
-  valueon/value
-  valueoff/value
-/choice
-  /attribute
-/optional
-  /define
   define name=driveaddress
 optional
   attribute name=controller
@@ -3376,26 +3352,6 @@
   param 
name=pattern((0x)?[0-9a-fA-F]{1,3}\.){0,3}(0x)?[0-9a-fA-F]{1,3}/param
 /data
   /define
-  define name=pciDomain
-data type=string
-  param name=pattern(0x)?[0-9a-fA-F]{1,4}/param
-/data
-  /define
-  define name=pciBus
-data type=string
-  param name=pattern(0x)?[0-9a-fA-F]{1,2}/param
-/data
-  /define
-  define name=pciSlot
-data type=string
-  param name=pattern(0x)?[0-1]?[0-9a-fA-F]/param
-/data
-  /define
-  define name=pciFunc
-data type=string
-  param name=pattern(0x)?[0-7]/param
-/data
-  /define
   define name=driveController
 data type=string
   param name=pattern[0-9]{1,2}/param
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index e55105a..4abfd91 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -87,22 +87,51 @@
   valuepassthrough/value
   valueprivate/value
   valuevepa/value
+  valuehostdev/value
+/choice
+  /attribute
+/optional
+
+optional
+  attribute name=managed
+choice
+  valueyes/value
+  valueno/value
 /choice
   /attribute
 /optional
 interleave
-  zeroOrMore
-element name='interface'
-  attribute name='dev'
-ref name='deviceName'/
-  /attribute
-  optional
-attribute name=connections
-  data type=unsignedInt/
-/attribute
-  /optional
-/element
-  /zeroOrMore
+  choice
+

[libvirt] [PATCH 4/7] Code to return interface name or pci_addr of the VF in actualDevice

2012-08-16 Thread Shradha Shah

The network pool should be able to keep track of both, network device
names nad PCI addresses, and return the appropriate one in the actualDevice
when networkAllocateActualDevice is called.

Signed-off-by: Shradha Shah ss...@solarflare.com
---
 src/network/bridge_driver.c |   60 +--
 src/util/virnetdev.c|   25 -
 src/util/virnetdev.h|4 ++-
 3 files changed, 50 insertions(+), 39 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index ddd66e5..38f6d12 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -59,6 +59,7 @@
 #include dnsmasq.h
 #include configmake.h
 #include virnetdev.h
+#include pci.h
 #include virnetdevbridge.h
 #include virnetdevtap.h
 #include virnetdevvportprofile.h
@@ -2780,10 +2781,11 @@ static int
 networkCreateInterfacePool(virNetworkDefPtr netdef) {
 unsigned int num_virt_fns = 0;
 char **vfname = NULL;
+struct pci_config_address **virt_fns;
 int ret = -1, ii = 0;
 
 if ((virNetDevGetVirtualFunctions(netdef-forwardPfs-dev,
-  vfname, num_virt_fns))  0) {
+  vfname, virt_fns, num_virt_fns))  0) 
{ 
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Could not get Virtual functions on %s),
netdef-forwardPfs-dev);
@@ -2805,18 +2807,34 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) {
 netdef-nForwardIfs = num_virt_fns;
 
 for (ii = 0; ii  netdef-nForwardIfs; ii++) {
-netdef-forwardIfs[ii].device.dev = strdup(vfname[ii]);
-if (!netdef-forwardIfs[ii].device.dev) {
-virReportOOMError();
-goto finish;
+if ((netdef-forwardType == VIR_NETWORK_FORWARD_BRIDGE) ||
+(netdef-forwardType == VIR_NETWORK_FORWARD_PRIVATE) ||
+(netdef-forwardType == VIR_NETWORK_FORWARD_VEPA) ||
+(netdef-forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH)) {
+netdef-forwardIfs[ii].type = 
VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV;
+if(vfname[ii]) {
+netdef-forwardIfs[ii].device.dev = strdup(vfname[ii]);
+if (!netdef-forwardIfs[ii].device.dev) {
+virReportOOMError();
+goto finish;
+}
+}
+else {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Direct mode types requires interface 
names));
+goto finish;
+}
 }
 }
 
 ret = 0;
 finish:
-for (ii = 0; ii  num_virt_fns; ii++)
+for (ii = 0; ii  num_virt_fns; ii++) {
 VIR_FREE(vfname[ii]);
+VIR_FREE(virt_fns[ii]);
+}
 VIR_FREE(vfname);
+VIR_FREE(virt_fns);
 return ret;
 }
 
@@ -3008,31 +3026,23 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
 } else {
 /* pick an interface from the pool */
 
+if ((netdef-nForwardPfs  0)  (netdef-nForwardIfs = 0)) {
+if ((networkCreateInterfacePool(netdef))  0) {
+goto error;
+}
+}
+
 /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both
  * require exclusive access to a device, so current
  * connections count must be 0.  Other modes can share, so
  * just search for the one with the lowest number of
  * connections.
  */
-if (netdef-forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) {
-if ((netdef-nForwardPfs  0)  (netdef-nForwardIfs = 0)) {
-if ((networkCreateInterfacePool(netdef))  0) {
-goto error;
-}
-}
-
-/* pick first dev with 0 connections */
-
-for (ii = 0; ii  netdef-nForwardIfs; ii++) {
-if (netdef-forwardIfs[ii].connections == 0) {
-dev = netdef-forwardIfs[ii];
-break;
-}
-}
-} else if ((netdef-forwardType == VIR_NETWORK_FORWARD_PRIVATE) 
-   iface-data.network.actual-virtPortProfile 
-   
(iface-data.network.actual-virtPortProfile-virtPortType
-== VIR_NETDEV_VPORT_PROFILE_8021QBH)) {
+if ((netdef-forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) || 
+((netdef-forwardType == VIR_NETWORK_FORWARD_PRIVATE) 
+ iface-data.network.actual-virtPortProfile 
+ (iface-data.network.actual-virtPortProfile-virtPortType
+  == VIR_NETDEV_VPORT_PROFILE_8021QBH))) {
 
 /* pick first dev with 0 connections */
 for (ii = 0; ii  netdef-nForwardIfs; ii++) {
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 25bdf01..f9eba1a

[libvirt] [PATCH 7/7] Forward Mode 'Hostdev' qemu driver implementation

2012-08-16 Thread Shradha Shah

For a network with forward mode='hostdev', there is a need to
add the newly minted hostdev to the hostdevs array.

In this case we also call qemuPrepareHostDevicesas it has already been
called by the time we get to here and are building the qemu commandline

Signed-off-by: Shradha Shah ss...@solarflare.com
---
 src/qemu/qemu_command.c |   39 +--
 1 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6f6c6cd..1470edd 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -24,6 +24,7 @@
 #include config.h
 
 #include qemu_command.h
+#include qemu_hostdev.h
 #include qemu_capabilities.h
 #include qemu_bridge_filter.h
 #include cpu/cpu.h
@@ -5221,12 +5222,38 @@ qemuBuildCommandLine(virConnectPtr conn,
 
 actualType = virDomainNetGetActualType(net);
 if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
-/* type='hostdev' interfaces are handled in codepath
- * for standard hostdev (NB: when there is a network
- * with forward mode='hostdev', there will need to be
- * code here that adds the newly minted hostdev to the
- * hostdevs array).
- */
+if (net-type == VIR_DOMAIN_NET_TYPE_NETWORK) {
+virDomainHostdevDefPtr hostdev = 
virDomainNetGetActualHostdev(net);
+virDomainHostdevDefPtr found;
+/* For a network with forward mode='hostdev', there is a 
need to 
+ * add the newly minted hostdev to the hostdevs array.
+ */
+if (qemuAssignDeviceHostdevAlias(def, hostdev,
+ (def-nhostdevs-1))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Could not assign alias to Net 
Hostdev));
+goto error;
+}
+
+if (virDomainHostdevFind(def, hostdev, found)  0) {
+if (virDomainHostdevInsert(def, hostdev)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Hostdev not inserted into the 
array));
+goto error;
+}
+if (qemuPrepareHostdevPCIDevices(driver, def-name, 
def-uuid, 
+ hostdev, 1)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Prepare Hostdev PCI Devices 
failed));
+goto error;
+} 
+}
+else {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(The Hostdev is being used by some 
other device));
+goto error;
+}
+}
 continue;
 }
 
-- 
1.7.4.4

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

[libvirt] [PATCH 0/7] Forward mode='hostdev' patch series

2012-08-16 Thread Shradha Shah

This patch series supports the forward mode='hostdev'. The functionality
of this mode is the same as interface type='hostdev' but with the added
benefit of using interface pools.

The patch series also contains a patch to support use of interface names
and PCI device addresses interchangeably in a network xml, and return
the appropriate one in actualDevice when networkAllocateActualDevice is
called.

At the top level managed attribute can be specified with identical results 
as when it's specified for a hostdev.

Currently forward mode='hostdev' does not support USB devices.

Shradha Shah (7):
  conf: move DevicePCIAddress functions to separate file
  network: helper function to create interface pool from PF
  RNG updates, new xml parser/formatter code to support forward
mode=hostdev
  Code to return interface name or pci_addr of the VF in actualDevice
  Add function virDevicePCIAddressEqual
  Forward Mode Hostdev network driver Implementation
  Forward Mode 'Hostdev' qemu driver implementation

 docs/formatnetwork.html.in |   62 +
 docs/schemas/basictypes.rng|   46 
 docs/schemas/domaincommon.rng  |   44 
 docs/schemas/network.rng   |   53 -
 include/libvirt/virterror.h|1 +
 src/Makefile.am|7 +-
 src/conf/device_conf.c |  147 +++
 src/conf/device_conf.h |   68 ++
 src/conf/domain_conf.c |  114 +
 src/conf/domain_conf.h |   25 +--
 src/conf/network_conf.c|  130 +--
 src/conf/network_conf.h|   26 ++-
 src/libvirt_private.syms   |   11 +-
 src/network/bridge_driver.c|  414 +++-
 src/qemu/qemu_command.c|   52 +++-
 src/qemu/qemu_hotplug.c|7 +-
 src/qemu/qemu_monitor.c|   14 +-
 src/qemu/qemu_monitor.h|   17 +-
 src/qemu/qemu_monitor_json.c   |   14 +-
 src/qemu/qemu_monitor_json.h   |   14 +-
 src/qemu/qemu_monitor_text.c   |   16 +-
 src/qemu/qemu_monitor_text.h   |   14 +-
 src/util/virnetdev.c   |   25 +-
 src/util/virnetdev.h   |4 +-
 src/util/virterror.c   |3 +-
 src/xen/xend_internal.c|3 +-
 tests/networkxml2xmlin/hostdev-pf.xml  |7 +
 tests/networkxml2xmlin/hostdev.xml |   10 +
 tests/networkxml2xmlout/hostdev-pf.xml |7 +
 tests/networkxml2xmlout/hostdev.xml|   10 +
 tests/networkxml2xmltest.c |2 +
 31 files changed, 976 insertions(+), 391 deletions(-)
 create mode 100644 src/conf/device_conf.c
 create mode 100644 src/conf/device_conf.h
 create mode 100644 tests/networkxml2xmlin/hostdev-pf.xml
 create mode 100644 tests/networkxml2xmlin/hostdev.xml
 create mode 100644 tests/networkxml2xmlout/hostdev-pf.xml
 create mode 100644 tests/networkxml2xmlout/hostdev.xml

-- 
1.7.4.4

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

Re: [libvirt] [PATCH 2/7] network: helper function to create interface pool from PF

2012-08-16 Thread Laine Stump

On 08/16/2012 11:41 AM, Shradha Shah wrote:
   Existing code that creates a list of forwardIfs from a single PF
   was moved to the new utility function networkCreateInterfacePool.
   No functional change.

 Signed-off-by: Shradha Shah ss...@solarflare.com

ACK. Just one small change to the code (and a shorter title :-) had been
requested, and both have been resolved.

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

Re: [libvirt] [PATCH 1/7] conf: move DevicePCIAddress functions to separate file

2012-08-16 Thread Laine Stump

On 08/16/2012 11:41 AM, Shradha Shah wrote:
 Move the functions the parse/format, and validate PCI addresses to
 their own file so they can be conveniently used in other places
 besides device_conf.c

 Refactoring existing code without causing any functional changes to
 prepare for new code.

 This patch makes the code reusable.

 Signed-off-by: Shradha Shah ss...@solarflare.com

ACK. You've taken care of all my nits from the previous version.

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

Re: [libvirt] [PATCH 3/7] RNG updates, new xml parser/formatter code to support forward mode=hostdev

2012-08-16 Thread Laine Stump

On 08/16/2012 11:41 AM, Shradha Shah wrote:
 This patch introduces the new forward mode='hostdev' along with attribute 
 managed
 Includes updates to the network RNG and new xml parser/formatter code.

 Signed-off-by: Shradha Shah ss...@solarflare.com

I'm in tears!

ACK. You've taken care of every one of my points in the last review,
*AND* rebased on top of my conflicting changes! (Thanks for putting up
with that, btw :-)

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

Re: [libvirt] [PATCH 08/23] Refactor the way new clients are registered with the server

2012-08-16 Thread Eric Blake

On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Currently the virNetServerDispatchNewClient both creates the
 virNetServerClientPtr instance and registers it with the
 virNetServerPtr internal state. Split the client registration
 code out into a separate virNetServerAddClient method to
 allow future reuse from other contexts
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/rpc/virnetserver.c | 46 --
  1 file changed, 28 insertions(+), 18 deletions(-)

ACK.

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



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

Re: [libvirt] [PATCH 09/23] Refactor impl of the virNetServerClientNew method

2012-08-16 Thread Eric Blake

On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 In preparation for adding further constructors, refactor
 the virNetServerClientNew method to move most of the code
 into a common virNetServerClientNewInternal helper API.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/rpc/virnetserverclient.c | 52 
 +---
  1 file changed, 34 insertions(+), 18 deletions(-)
 

ACK.

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



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

Re: [libvirt] [PATCH 5/7] Add function virDevicePCIAddressEqual

2012-08-16 Thread Laine Stump

On 08/16/2012 11:42 AM, Shradha Shah wrote:
 This function is needed by the network driver in a later commit.
 This function is useful in functions like networkNotifyActualDevice
 and networkReleaseActualDevice
 ---
  src/conf/device_conf.c   |   16 
  src/conf/device_conf.h   |3 +++
  src/libvirt_private.syms |1 +
  3 files changed, 20 insertions(+), 0 deletions(-)

 diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
 index ca600c5..8edcc0a 100644
 --- a/src/conf/device_conf.c
 +++ b/src/conf/device_conf.c
 @@ -129,3 +129,19 @@ virDevicePCIAddressFormat(virBufferPtr buf,
addr.function);
  return 0;
  }
 +
 +int
 +virDevicePCIAddressEqual(virDevicePCIAddress addr1,
 + virDevicePCIAddress addr2)
 +{
 +int ret = -1;


The other xxxEqual() functions in libvirt return a bool true/false
rather than 0 / -1. ACK with that fix (don't bother re-sending - I'll
just fix it up (along with the places you call it in later patches)
before I push.

 +
 +if (addr1.domain == addr2.domain 
 +addr1.bus == addr2.bus 
 +addr1.slot == addr2.slot 
 +addr1.function == addr2.function) {
 +ret = 0;
 +}
 +
 +return ret;
 +}
 diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
 index c679bce..7c4d356 100644
 --- a/src/conf/device_conf.h
 +++ b/src/conf/device_conf.h
 @@ -59,6 +59,9 @@ int virDevicePCIAddressFormat(virBufferPtr buf,
virDevicePCIAddress addr,
bool includeTypeInAddr);
  
 +int virDevicePCIAddressEqual(virDevicePCIAddress addr1,
 + virDevicePCIAddress addr2);
 +
  
  VIR_ENUM_DECL(virDeviceAddressPciMulti)
  
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 1f32f8e..063d0bc 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -224,6 +224,7 @@ virDeviceAddressPciMultiTypeToString;
  virDevicePCIAddressIsValid;
  virDevicePCIAddressParseXML;
  virDevicePCIAddressFormat;
 +virDevicePCIAddressEqual;
  
  # dnsmasq.h
  dnsmasqAddDhcpHost;

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

Re: [libvirt] [PATCH 10/23] Add support for creating sockets RPC servers from a pre-opened fd

2012-08-16 Thread Eric Blake

On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 In order to support systemd socket based activation, it needs to
 be possible to create virNetSocketPtr and virNetServerServicePtr
 instance from a pre-opened file descriptor
 ---
  src/libvirt_private.syms  |  2 ++
  src/rpc/virnetserverservice.c | 49 
 +++
  src/rpc/virnetserverservice.h |  5 +
  src/rpc/virnetsocket.c| 20 ++
  src/rpc/virnetsocket.h|  3 +++
  5 files changed, 79 insertions(+)
 

ACK.

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



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

Re: [libvirt] [PATCH 0/8] Honour current process label when generating SELinux labels

2012-08-16 Thread Daniel J Walsh

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 08/16/2012 11:41 AM, Viktor Mihajlovski wrote:
 On 08/10/2012 03:47 PM, Daniel P. Berrange wrote:
 This patch series makes a number of changes to the SELinux label 
 generation code. This is intended to make it fully honour the current
 process label when generating VM labels, so that dynamic label generation
 works better with custom policies, or confined user accounts.
 
 -- libvir-list mailing list libvir-list@redhat.com 
 https://www.redhat.com/mailman/listinfo/libvir-list
 
 
 Unfortunately I am not selinux-savvy enough to understand exactly why, but
 I cannot start guests any more after pulling master.
 
 The issue is that the virtual disk's security context (a block device in
 this case) cannot be set, message shown below.
 
 012-08-16 15:02:18.891+: 1536: error : 
 virSecuritySELinuxSetFileconHelper:652 : unable to set security context 
 'system_u:system_r:svirt_image_t:s0:c786,c986' on 
 '/dev/disk/by-path/ccw-0.0.3770-part1': Invalid argument
 
 Prior to that the security context would have looked like this 
 system_u:object_r:svirt_image_t:s0:c153,c923, i.e. using object_r instead
 of system_r.
 
 I am running on RHEL 6.2, not sure whether this is relevant.
 

Yes the security context should be system_u:object_r:svirt_image_t:s0:c786,c986
These patches should have just affected the Process label not the file label.
 On the file label we should alter the role on the file label to include 
object_r.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlAtMVIACgkQrlYvE4MpobMYqQCgz+d7yeXhYXTz0IGFIsRYUqJl
GGgAniHHX21m7D5BHZgeMHskS8zww4B1
=Ex2S
-END PGP SIGNATURE-

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

Re: [libvirt] [PATCH 1/7] conf: move DevicePCIAddress functions to separate file

2012-08-16 Thread Laine Stump

On 08/16/2012 12:12 PM, Laine Stump wrote:
 On 08/16/2012 11:41 AM, Shradha Shah wrote:
 Move the functions the parse/format, and validate PCI addresses to
 their own file so they can be conveniently used in other places
 besides device_conf.c

 Refactoring existing code without causing any functional changes to
 prepare for new code.

 This patch makes the code reusable.

 Signed-off-by: Shradha Shah ss...@solarflare.com
 ACK. You've taken care of all my nits from the previous version.


Actually, make check was failing on a bunch of cases. But it turned out
to just be that the address line was indented 4 extra spaces instead
of two. I'll squash in this change when I push:

diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index ca600c5..aefffec 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -120,7 +120,7 @@ virDevicePCIAddressFormat(virBufferPtr buf,
   virDevicePCIAddress addr,
   bool includeTypeInAddr)
 {
-virBufferAsprintf(buf, address %sdomain='0x%.4x' bus='0x%.2x' 
+virBufferAsprintf(buf, address %sdomain='0x%.4x' bus='0x%.2x' 
   slot='0x%.2x' function='0x%.1x'/\n,
   includeTypeInAddr ? type='pci'  : ,
   addr.domain,
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 78d5685..ff225e6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11435,18 +11435,19 @@ virDomainHostdevSourceFormat(virBufferPtr buf,
  bool includeTypeInAddr)
 {
 virBufferAddLit(buf, source\n);
+virBufferAdjustIndent(buf, 2);
 switch (def-source.subsys.type)
 {
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
 if (def-source.subsys.u.usb.vendor) {
-virBufferAsprintf(buf,   vendor id='0x%.4x'/\n,
+virBufferAsprintf(buf, vendor id='0x%.4x'/\n,
   def-source.subsys.u.usb.vendor);
-virBufferAsprintf(buf,   product id='0x%.4x'/\n,
+virBufferAsprintf(buf, product id='0x%.4x'/\n,
   def-source.subsys.u.usb.product);
 }
 if (def-source.subsys.u.usb.bus ||
 def-source.subsys.u.usb.device) {
-virBufferAsprintf(buf,   address %sbus='%d' device='%d'/\n,
+virBufferAsprintf(buf, address %sbus='%d' device='%d'/\n,
   includeTypeInAddr ? type='usb'  : ,
   def-source.subsys.u.usb.bus,
   def-source.subsys.u.usb.device);
@@ -11463,14 +11464,14 @@ virDomainHostdevSourceFormat(virBufferPtr buf,
 (def-origstates.states.pci.unbind_from_stub ||
  def-origstates.states.pci.remove_slot ||
  def-origstates.states.pci.reprobe)) {
-virBufferAddLit(buf,   origstates\n);
+virBufferAddLit(buf, origstates\n);
 if (def-origstates.states.pci.unbind_from_stub)
-virBufferAddLit(buf, unbind/\n);
+virBufferAddLit(buf,   unbind/\n);
 if (def-origstates.states.pci.remove_slot)
-virBufferAddLit(buf, removeslot/\n);
+virBufferAddLit(buf,   removeslot/\n);
 if (def-origstates.states.pci.reprobe)
-virBufferAddLit(buf, reprobe/\n);
-virBufferAddLit(buf,   /origstates\n);
+virBufferAddLit(buf,   reprobe/\n);
+virBufferAddLit(buf, /origstates\n);
 }
 break;
 default:
@@ -11480,6 +11481,7 @@ virDomainHostdevSourceFormat(virBufferPtr buf,
 return -1;
 }
 
+virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, /source\n);
 return 0;
 }

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

Re: [libvirt] [PATCH 4/7] Code to return interface name or pci_addr of the VF in actualDevice

2012-08-16 Thread Laine Stump

On 08/16/2012 11:41 AM, Shradha Shah wrote:
 The network pool should be able to keep track of both, network device
 names nad PCI addresses, and return the appropriate one in the actualDevice
 when networkAllocateActualDevice is called.

 Signed-off-by: Shradha Shah ss...@solarflare.com


ACK again. It's good that the autogeneration of the device pool from pf
now works for PRIVATE and BRIDGE modes too.

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

[libvirt] [PATCH] build: ship stamp files

2012-08-16 Thread Eric Blake

'make distcheck' fails because the generated ESX and HyperV files
are (intentionally) marked read-only, but since the stamp file was
missing, make assumes they need to be rebuilt.  Shipping the stamp
file solves the problem.

* src/Makefile.am (EXTRA_DIST): Ship stamp files.
---

Pushing under the build-breaker rule.

 src/Makefile.am | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/Makefile.am b/src/Makefile.am
index ac5a27f..213bd83 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -930,6 +930,8 @@ BUILT_SOURCES += $(ESX_DRIVER_GENERATED)

 ESX_GENERATED_STAMP = .esx_vi_generator.stamp

+EXTRA_DIST += $(ESX_GENERATED_STAMP)
+
 $(ESX_DRIVER_GENERATED): $(ESX_GENERATED_STAMP)

 $(ESX_GENERATED_STAMP): $(srcdir)/esx/esx_vi_generator.input \
@@ -956,6 +958,8 @@ BUILT_SOURCES += $(HYPERV_DRIVER_GENERATED)

 HYPERV_GENERATED_STAMP = .hyperv_wmi_generator.stamp

+EXTRA_DIST += $(HYPERV_GENERATED_STAMP)
+
 $(HYPERV_DRIVER_GENERATED): $(HYPERV_GENERATED_STAMP)

 $(HYPERV_GENERATED_STAMP): $(srcdir)/hyperv/hyperv_wmi_generator.input \
-- 
1.7.11.2

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

Re: [libvirt] [PATCH 5/7] Add function virDevicePCIAddressEqual

2012-08-16 Thread Eric Blake

On 08/16/2012 09:42 AM, Shradha Shah wrote:
 This function is needed by the network driver in a later commit.
 This function is useful in functions like networkNotifyActualDevice
 and networkReleaseActualDevice
 ---
  src/conf/device_conf.c   |   16 
  src/conf/device_conf.h   |3 +++
  src/libvirt_private.syms |1 +
  3 files changed, 20 insertions(+), 0 deletions(-)

 +++ b/src/libvirt_private.syms
 @@ -224,6 +224,7 @@ virDeviceAddressPciMultiTypeToString;
  virDevicePCIAddressIsValid;
  virDevicePCIAddressParseXML;
  virDevicePCIAddressFormat;
 +virDevicePCIAddressEqual;

Several patches have now failed to sort this section of the file.  I'm
okay with either rebasing to touch it up as you go, or one big cleanup
at the end of the series.

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



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

Re: [libvirt] distcheck issues on maint and master

2012-08-16 Thread Eric Blake

On 08/13/2012 10:19 AM, Eric Blake wrote:
 On 08/13/2012 08:04 AM, Cole Robinson wrote:
 On 08/13/2012 09:59 AM, Daniel P. Berrange wrote:
 On Mon, Aug 13, 2012 at 09:56:49AM -0400, Cole Robinson wrote:
 I just pushed a bunch of patches to -maint branches, but both are giving me
 distcheck errors that seem related to a gnulib update:

 ERROR: files left in build directory after distclean:
 ./.sc-start-sc_vulnerable_makefile_CVE-2012-3386
 
 'make maintainer-clean' doesn't clean up leftover .sc-start files from
 one failed syntax check, even if you resolve the syntax check in the
 meantime.  I don't know why the .sc file is getting bundled into the
 tarball, but it should not be; and that would possibly explain the error
 you are seeing.  There may be further improvements needed on gnulib's
 side (hence the cc), but for now, I think the workaround for you is to
 just manually delete that .sc-start* file prior to running distcheck.

Found the root of the issue - it's libvirt's fault.  Gnulib's maint.mk
takes the initial definition of local-checks-to-skip, and from that,
creates a macro 'local-checks' using a := rule:

local-check :=  \
  $(patsubst sc_%, sc_%.z,  \
$(filter-out $(local-checks-to-skip), $(local-checks-available)))

But libvirt's cfg.mk is conditionally running the local-checks-to-skip
rule, via:

# Most developers don't run 'make distcheck'.  We want the official
# dist to be secure, but don't want to penalize other developers
# using a distro that has not yet picked up the automake fix.
# FIXME remove this ifeq (making the syntax check unconditional)
# once fixed automake (1.11.6 or 1.12.2+) is more common.
ifeq ($(filter dist%, $(MAKECMDGOALS)), )
local-checks-to-skip += sc_vulnerable_makefile_CVE-2012-3386
else
distdir: sc_vulnerable_makefile_CVE-2012-3386
endif

Because distdir depends on the full sc_ name, rather than the sc_.z
rewrite, maint.mk's timing rules don't get properly run, so the
.sc-start-* file doesn't get cleaned up.  I think with a bit more
tweaking to libvirt's cfg.mk, I can get this working again.

Meanwhile, would gnulib like to incorporate this hack from libvirt?
After all, the current Automake vulnerability only affects you if you
run 'make dist' or 'make distcheck'; it does not impact normal
day-to-day development.  Therefore, running the syntax check only in the
vulnerable cases, and in such a way that the syntax check stops make
before the vulnerability can actually be triggered, without penalizing
day-to-day development for people relying on their distro rather than
using a hand-built automake, seems like it would be nice to share among
multiple packages.

[It's a shame that more than a month after the CVE was reported and both
Fedora 17 and RHEL 6.3 are still vulnerable, but that's a story for
another day.]

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



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

[libvirt] [PATCH] build: fix syntax check during 'make distcheck'

2012-08-16 Thread Eric Blake

'make distcheck' was failing because a syntax check file,
.sc-start-sc_vulnerable_makefile_CVE-2012-3386, got left
behind.  I traced it to the 'distdir' rule depending on a
shortcut syntax-check name rather than the full rule name
normally used during 'local-check' from maint.mk.

* cfg.mk (distdir): Depend on full rule, not shorthand name.
---

Pushing under the build-breaker rule.

 cfg.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cfg.mk b/cfg.mk
index 1318593..e9138a8 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -85,7 +85,7 @@ local-checks-to-skip =\
 ifeq ($(filter dist%, $(MAKECMDGOALS)), )
 local-checks-to-skip +=sc_vulnerable_makefile_CVE-2012-3386
 else
-distdir: sc_vulnerable_makefile_CVE-2012-3386
+distdir: sc_vulnerable_makefile_CVE-2012-3386.z
 endif

 # Files that should never cause syntax check failures.
-- 
1.7.11.2

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

Re: [libvirt] [PATCH 11/23] Introduce an internal API for handling file based lockspaces

2012-08-16 Thread Eric Blake

On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The previously introduced virFile{Lock,Unlock} APIs provide a
 way to acquire/release fcntl() locks on individual files. For
 unknown reason though, the POSIX spec says that fcntl() locks
 are released when *any* file handle refering to the same path

s/refering/referring/

 is closed. In the following sequence
 
   threadA: fd1 = open(foo)
   threadB: fd2 = open(foo)
   threadA: virFileLock(fd1)
   threadB: virFileLock(fd2)
   threadB: close(fd2)
 
 you'd expect threadA to come out holding a lock on 'foo', and
 indeed it does hold a lock for a very short time. Unfortunately
 when threadB does close(fd2) this releases the lock associated
 with fd1. For the current libvirt use case for virFileLock -
 pidfiles - this doesn't matter since the lock is acquired
 at startup while single threaded an never released until
 exit.
 
 To provide a more generally useful API though, it is neccessary

s/neccessary/necessary/

 to introduce a slightly higher level abstraction, which is to
 be referred to as a lockspace.  This is to be provided by
 a virLockSpacePtr object in src/util/virlockspace.{c,h}. The
 core idea is that the lockspace keeps track of what files are
 already open+locked. This means that when a 2nd thread comes
 along and tries to acquire a lock, it doesn't end up opening
 and closing a new FD. The lockspace just checks the current
 list of held locks and immediately returns VIR_ERR_RESOURCE_BUSY.
 
 NB, the API as it stands is designed on the basis that the
 files being locked are not being otherwise opened and used
 by the application code. One approach to using this API is to
 acquire locks based on a hash of the filepath.
 
 eg to lock /var/lib/libvirt/images/foo.img the application
 might do
 
virLockSpacePtr lockspace = virLockSpaceNew(/var/lib/libvirt/imagelocks);
lockname = md5sum(/var/lib/libvirt/images/foo.img)
virLockSpaceAcquireLock(lockspace, lockname)

s/)$/),/g

Don't you want to ensure that the canonical name is used (that is,
symlinks can allow two different names to resolve to the same file, but
you want to hash the name without symlinks).

 
 It is also possible to do locks directly on resources by
 using a NULL lockspace directory and then using the file
 path as the lock name eg
 
virLockSpacePtr lockspace = virLockSpaceNew(NULL)
virLockSpaceAcquireLock(lockspace, /var/lib/libvirt/images/foo.img)

more missing semicolons

 
 This is only safe todo though if no other part of the proces

s/todo/to do/; s/proces/process/

 will be opening the files. This will be the case when this
 code is used inside the soon-to-be-reposted virlockd daemon
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---

This patch failed to apply directly for me; you'll have to rebase it
again.  I was able to figure out the changes, though (a new error in
virterror.[ch]).

 +
 +
 +static char *virLockSpaceGetResourcePath(virLockSpacePtr lockspace,
 + const char *resname)
 +{
 +char *ret;
 +if (lockspace-dir) {
 +if (virAsprintf(ret, %s/%s, lockspace-dir, resname)  0) {
 +virReportOOMError();
 +return NULL;
 +}
 +} else {
 +if (!(ret = strdup(resname))) {
 +virReportOOMError();
 +return NULL;
 +}
 +}

Fine as-is, but could be written shorter as:

if (lockspace-dir)
ignore_value(virAsprintf(ret, %s/%s, lockspace-dir, resname));
else
ret = strdup(resname);
if (!ret) {
virReportOOMError();
return NULL;
}

 +static void virLockSpaceResourceFree(virLockSpaceResourcePtr res)
 +{
 +if (!res)
 +return;
 +
 +if (res-lockHeld 
 +(res-flags  VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE)) {
 +if (res-flags  VIR_LOCK_SPACE_ACQUIRE_SHARED) {
 +/* We must upgrade to an exclusive lock to ensure
 + * no one else still has it before trying to delete */
 +if (virFileLock(res-fd, false, 0, 1)  0) {
 +VIR_DEBUG(Could not upgrade shared lease to exclusive, not 
 deleting);
 +} else {
 +unlink(res-path);

Should we log failures to unlink()?

 +static virLockSpaceResourcePtr
 +virLockSpaceResourceNew(virLockSpacePtr lockspace,

 +VIR_DEBUG(Resource '%s' disappeared: %s,
 +  res-path, virStrerror(errno, ebuf, sizeof(ebuf)));
 +VIR_FORCE_CLOSE(res-fd);
 +/* Someone else must be racing with us, so try agin */

s/agin/again/

 +
 +void virLockSpaceFree(virLockSpacePtr lockspace)
 +{
 +if (!lockspace)
 +return;
 +
 +virHashFree(lockspace-resources);
 +VIR_FREE(lockspace-dir);

Should we first try to rmdir() the directory, and silently ignore errors
related to the directory still being full?

 +++ b/tests/virlockspacetest.c

 +
 +static int testLockSpaceCreate(const void *args

Re: [libvirt] [PATCH 12/23] Add JSON serialization of virLockSpacePtr objects for process re-exec()

2012-08-16 Thread Eric Blake

On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Add two new APIs virLockSpaceNewPostExecRestart and
 virLockSpacePreExecRestart which allow a virLockSpacePtr
 object to be created from a JSON object and saved to a
 JSON object, for the purposes of re-exec'ing a process.
 
 As well as saving the state in JSON format, the second
 method will disable the O_CLOEXEC flag so that the open
 file descriptors are preserved across the process re-exec()
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---

Is virLockSpacePreExecRestart called in the parent prior to forking
(mostly good, with one caveat) or in the child between fork and exec
(bad, since you malloc and do a lot of other non-async-safe stuff)?  If
the latter, we risk deadlock; if the former, then you have at least one
bug...

 +
 +virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr object)
 +{
 +virLockSpacePtr lockspace;
 +virJSONValuePtr resources;
 +int n;
 +size_t i;
 +
 +VIR_DEBUG(object=%p, object);

Would it be any better to pretty-print the contents of object instead of
just listing its address?


 +if (virJSONValueObjectGetNumberInt(child, fd, res-fd)  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Missing resource fd in JSON document));
 +virLockSpaceResourceFree(res);
 +goto error;
 +}
 +if (virSetInherit(res-fd, false)  0) {
 +virReportSystemError(errno, %s,
 + _(Cannot enable close-on-exec flag));
 +goto error;

Is the reparse loop called while the exec'd process is still
single-threaded, so as to avoid any races with the fds that are not yet
re-marked CLOEXEC?

 +
 +virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr lockspace)
 +{

 +
 +if (virSetInherit(res-fd, true)  0) {
 +virReportSystemError(errno, %s,
 + _(Cannot disable close-on-exec flag));
 +goto error;
 +}

...clearing O_CLOEXEC in the parent is wrong.  You cannot clear
O_CLOEXEC until you are in the child (so as not to leak into any other
child process spawned by another thread), which really means that you
have to use virCommandPreserveFD instead of virSetInherit (virCommand
will then call virSetInherit in the child, between fork and exec, on
your behalf).  But that makes it sound like you need to pass a
virCommandPtr in to this function, in addition to the lockspace.

Overall, looks like a mostly sane serialization - simpler than coding it
up in XML, and still robust to pass between parent and child, but you do
need to solve the CLOEXEC safety issues.

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



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

Re: [libvirt] [PATCH 13/23] Add JSON serialization of virNetSocketPtr objects for process re-exec()

2012-08-16 Thread Eric Blake

On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Add two new APIs virNetSocketNewPostExecRestart and
 virNetSocketPreExecRestart which allow a virNetSocketPtr
 object to be created from a JSON object and saved to a
 JSON object, for the purpose of re-exec'ing a process.
 
 As well as saving the state in JSON format, the second
 method will disable the O_CLOEXEC flag so that the open
 file descriptors are preserved across the process re-exec()

Same problem as 12/23 regarding _when_ you clear O_CLOEXEC.

 
 Since it is not possible to serialize SASL or TLS encryption
 state, an error will be raised if attempting to perform
 serialization on non-raw sockets
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/libvirt_private.syms |   2 +
  src/rpc/virnetsocket.c   | 108 
 +++
  src/rpc/virnetsocket.h   |   6 +++
  3 files changed, 116 insertions(+)
 

  
 +virNetSocketPtr virNetSocketNewPostExecRestart(virJSONValuePtr object)
 +{
 +virSocketAddr localAddr;
 +virSocketAddr remoteAddr;
 +int fd, thepid, errfd;
 +bool isClient;
 +
 +if (virJSONValueObjectGetNumberInt(object, fd, fd)  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Missing fd data in JSON document));
 +return NULL;
 +}
 +
 +if (virJSONValueObjectGetNumberInt(object, pid, thepid)  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Missing pid data in JSON document));
 +return NULL;
 +}
 +
 +if (virJSONValueObjectGetNumberInt(object, errfd, errfd)  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Missing errfd data in JSON document));
 +return NULL;
 +}

Do you need to re-enable FD_CLOEXEC on fd and errfd at this point?

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



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

Re: [libvirt] [PATCH 14/23] Add JSON serialization of virNetServerServicePtr objects for process re-exec()

2012-08-16 Thread Eric Blake

On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Add two new APIs virNetServerServiceNewPostExecRestart and
 virNetServerServicePreExecRestart which allow a virNetServerServicePtr
 object to be created from a JSON object and saved to a
 JSON object, for the purpose of re-exec'ing a process.
 
 This includes serialization of the listening sockets associated
 with the service
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  po/POTFILES.in|   1 +
  src/libvirt_private.syms  |   2 +
  src/rpc/virnetserverservice.c | 124 
 ++
  src/rpc/virnetserverservice.h |   4 ++
  4 files changed, 131 insertions(+)
 

May need some signature changes if the solution to 12 and 13 requires
passing a virCommandPtr through the PreExecRestart commands.

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



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

Re: [libvirt] [PATCH 15/23] Add JSON serialization of virNetServerClientPtr objects for process re-exec()

2012-08-16 Thread Eric Blake

On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Add two new APIs virNetServerClientNewPostExecRestart and
 virNetServerClientPreExecRestart which allow a virNetServerClientPtr
 object to be created from a JSON object and saved to a
 JSON object, for the purpose of re-exec'ing a process.
 
 This includes serialization of the connected socket associated
 with the client
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---

 +virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr 
 object,
 +   
 virNetServerClientPrivNewPostExecRestart privNew,
 +   
 virNetServerClientPrivPreExecRestart privPreExecRestart,
 +   virFreeCallback 
 privFree,
 +   void *privOpaque)
 +{
 +virJSONValuePtr child;
 +virNetServerClientPtr client = NULL;
 +virNetSocketPtr sock;
 +const char *identity = NULL;
 +int auth;
 +bool readonly;
 +unsigned int nrequests_max;
 +
 +if (virJSONValueObjectGetNumberInt(object, auth, auth)  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Missing auth field in JSON state document));
 +return NULL;
 +}
 +if (virJSONValueObjectGetBoolean(object, readonly, readonly)  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Missing readonly field in JSON state document));
 +return NULL;
 +}
 +if (virJSONValueObjectGetNumberUint(object, nrequests_max,
 +(unsigned int *)nrequests_max)  0) 
 {

Why the cast?  nrequests_max is already (unsigned int *).

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



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

Re: [libvirt] [PATCH 16/23] Add JSON serialization of virNetServerPtr objects for process re-exec()

2012-08-16 Thread Eric Blake

On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Add two new APIs virNetServerNewPostExecRestart and
 virNetServerPreExecRestart which allow a virNetServerPtr
 object to be created from a JSON object and saved to a
 JSON object, for the purpose of re-exec'ing a process.
 
 This includes serialization of all registered services
 and clients
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/libvirt_private.syms |   2 +
  src/rpc/virnetserver.c   | 252 
 +++
  src/rpc/virnetserver.h   |  10 ++
  3 files changed, 264 insertions(+)
 

Looks reasonable, but may be impacted by any changes to earlier patches.

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



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

Re: [libvirt] [PATCH 5/7] Add function virDevicePCIAddressEqual

2012-08-16 Thread Laine Stump

On 08/16/2012 04:07 PM, Eric Blake wrote:
 On 08/16/2012 09:42 AM, Shradha Shah wrote:
 This function is needed by the network driver in a later commit.
 This function is useful in functions like networkNotifyActualDevice
 and networkReleaseActualDevice
 ---
  src/conf/device_conf.c   |   16 
  src/conf/device_conf.h   |3 +++
  src/libvirt_private.syms |1 +
  3 files changed, 20 insertions(+), 0 deletions(-)
 +++ b/src/libvirt_private.syms
 @@ -224,6 +224,7 @@ virDeviceAddressPciMultiTypeToString;
  virDevicePCIAddressIsValid;
  virDevicePCIAddressParseXML;
  virDevicePCIAddressFormat;
 +virDevicePCIAddressEqual;
 Several patches have now failed to sort this section of the file.  I'm
 okay with either rebasing to touch it up as you go, or one big cleanup
 at the end of the series.


Now that you've pointed it out, I'm just squashing in the simple fixes
to keep the order alphabetic as they go (along with a few other minor
changes like extra spaces at the end of lines, etc). (Doing a make
check and make syntax-check after applying each patch, I also found
some problems in 3/7 that cause networkxml2xmltest to fail - that
required enough changes that I'll be posting a patch for review (and
squashing into 3/7 before pushing) rather than just silently squashing
in the changes.

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

[libvirt] [PATCH] Add support for setting VLANs on Open vSwitch ports

2012-08-16 Thread Kyle Mestery

Add the ability to support VLAN tags for Open vSwitch
virtual port types. To accomplish this, modify
virNetDevOpenvswitchAddPort and
virNetDevTapCreateInBridgePort to take a virNetDevVlanPtr
argument. When adding the port to the OVS bridge, setup
either a single VLAN or a trunk port based on the
configuration from the virNetDevVlanPtr.

Signed-off-by: Kyle Mestery kmest...@cisco.com
---
 .gnulib |  2 +-
 src/lxc/lxc_process.c   |  2 +-
 src/network/bridge_driver.c |  2 +-
 src/qemu/qemu_command.c |  1 +
 src/uml/uml_conf.c  |  1 +
 src/util/virnetdevopenvswitch.c | 34 +++---
 src/util/virnetdevopenvswitch.h |  4 +++-
 src/util/virnetdevtap.c |  3 ++-
 src/util/virnetdevtap.h |  2 ++
 9 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/.gnulib b/.gnulib
index 271dd74..dbd9144 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 271dd74fdf54ec2a03e73a5173b0b5697f6088f1
+Subproject commit dbd914496c99c52220e5f5ba4121d6cb55fb3beb
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 046ed86..dc34bef 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -325,7 +325,7 @@ static int virLXCProcessSetupInterfaceBridged(virConnectPtr 
conn,
 
 if (vport  vport-virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)
 ret = virNetDevOpenvswitchAddPort(brname, parentVeth, net-mac,
-  vm-uuid, vport);
+  vm-uuid, vport, net-vlan);
 else
 ret = virNetDevBridgeAddPort(brname, parentVeth);
 if (ret  0)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 474bbfa..a78e3b6 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1763,7 +1763,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
 }
 if (virNetDevTapCreateInBridgePort(network-def-bridge,
macTapIfName, network-def-mac,
-   NULL, NULL, NULL,
+   NULL, NULL, NULL, NULL,

VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE)  0) {
 VIR_FREE(macTapIfName);
 goto err0;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9383530..e0062a1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -255,6 +255,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
 err = virNetDevTapCreateInBridgePort(brname, net-ifname, net-mac,
  def-uuid, tapfd,
  
virDomainNetGetActualVirtPortProfile(net),
+ net-vlan,
  tap_create_flags);
 virDomainAuditNetDevice(def, net, /dev/net/tun, tapfd = 0);
 if (err  0) {
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index 4c299d8..5461b42 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -141,6 +141,7 @@ umlConnectTapDevice(virConnectPtr conn,
 if (virNetDevTapCreateInBridgePort(bridge, net-ifname, net-mac,
vm-uuid, NULL,

virDomainNetGetActualVirtPortProfile(net),
+   net-vlan,
VIR_NETDEV_TAP_CREATE_IFUP)  0) {
 if (template_ifname)
 VIR_FREE(net-ifname);
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index b57532b..8a31e77 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -46,9 +46,11 @@
 int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
const virMacAddrPtr macaddr,
const unsigned char *vmuuid,
-   virNetDevVPortProfilePtr ovsport)
+   virNetDevVPortProfilePtr ovsport,
+   virNetDevVlanPtr virtVlan)
 {
 int ret = -1;
+int i = 0;
 virCommandPtr cmd = NULL;
 char macaddrstr[VIR_MAC_STRING_BUFLEN];
 char ifuuidstr[VIR_UUID_STRING_BUFLEN];
@@ -57,6 +59,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const 
char *ifname,
 char *ifaceid_ex_id = NULL;
 char *profile_ex_id = NULL;
 char *vmid_ex_id = NULL;
+virBufferPtr buf;
 
 virMacAddrFormat(macaddr, macaddrstr);
 virUUIDFormat(ovsport-interfaceID, ifuuidstr);
@@ -76,11 +79,35 @@ int virNetDevOpenvswitchAddPort(const char *brname, const 
char *ifname,
 ovsport-profileID)  0)
 goto out_of_memory;
 }
+if (virtVlan) {
+if (VIR_ALLOC(buf)  0)
+goto out_of_memory;
+
+/* Trunk port first */
+if (virtVlan-trunk) {
+

[libvirt] [PATCH 3.5/7] fixes to get 3/7 past make check

2012-08-16 Thread Laine Stump


Patch 3/7 fails make check, so I need to sqaush in the following to
fix it. It seemed a bit too much to squash in without getting somebody
to review it first:


1) The indentation was off for the new use of
   virDevicePCIAddressFormat in network_conf.c. Rather than just
   hacking in even more tweaks of virBufferAdjustIndent(), I corrected
   virNetworkDefFormat and the other format functions it calls
   (i.e. the rest of the file) to properly use the buffer indent
   everywhere.

2) the accessor function virNetworkDefForwardIf() needed to be updated
   to not access the newly unionized member device.dev unless the type
   was NETDEV. This caused a segfault when non-pointer data was
   interpreted as a pointer.

3) Some of the network XML test cases didn't match the output format
   (PCI addresses are always printed in hex with a 0x at the beginning).

4) Previously it was legal to put both a pf and a list of
   interface elements inside a forward element. That has now been
   made illegal, so it can't show up even in the *input* XML.

---
 src/conf/network_conf.c   | 83 +--
 src/conf/network_conf.h   |  3 +-
 tests/networkxml2xmlin/hostdev-pf.xml |  2 +-
 tests/networkxml2xmlin/hostdev.xml| 10 ++--
 tests/networkxml2xmlin/passthrough-pf.xml |  2 -
 tests/networkxml2xmlout/hostdev-pf.xml|  4 +-
 tests/networkxml2xmlout/hostdev.xml   | 10 ++--
 7 files changed, 61 insertions(+), 53 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ef519f6..9d53d8e 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1388,17 +1388,18 @@ virNetworkDNSDefFormat(virBufferPtr buf,
 if (def == NULL)
 goto out;
 
-virBufferAddLit(buf,   dns\n);
+virBufferAddLit(buf, dns\n);
+virBufferAdjustIndent(buf, 2);
 
 for (i = 0 ; i  def-ntxtrecords ; i++) {
-virBufferAsprintf(buf, txt name='%s' value='%s' /\n,
+virBufferAsprintf(buf, txt name='%s' value='%s' /\n,
   def-txtrecords[i].name,
   def-txtrecords[i].value);
 }
 
 for (i = 0 ; i  def-nsrvrecords ; i++) {
 if (def-srvrecords[i].service  def-srvrecords[i].protocol) {
-virBufferAsprintf(buf, srv service='%s' protocol='%s',
+virBufferAsprintf(buf, srv service='%s' protocol='%s',
   def-srvrecords[i].service,
   def-srvrecords[i].protocol);
 
@@ -1423,18 +1424,19 @@ virNetworkDNSDefFormat(virBufferPtr buf,
 for (ii = 0 ; ii  def-nhosts; ii++) {
 char *ip = virSocketAddrFormat(def-hosts[ii].ip);
 
-virBufferAsprintf(buf, host ip='%s'\n, ip);
-
+virBufferAsprintf(buf, host ip='%s'\n, ip);
+virBufferAdjustIndent(buf, 2);
 for (j = 0; j  def-hosts[ii].nnames; j++)
-virBufferAsprintf(buf,   hostname%s/hostname\n,
-   def-hosts[ii].names[j]);
+virBufferAsprintf(buf, hostname%s/hostname\n,
+  def-hosts[ii].names[j]);
 
-virBufferAsprintf(buf, /host\n);
+virBufferAdjustIndent(buf, -2);
+virBufferAsprintf(buf, /host\n);
 VIR_FREE(ip);
 }
 }
-
-virBufferAddLit(buf,   /dns\n);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, /dns\n);
 out:
 return result;
 }
@@ -1445,7 +1447,7 @@ virNetworkIpDefFormat(virBufferPtr buf,
 {
 int result = -1;
 
-virBufferAddLit(buf,   ip);
+virBufferAddLit(buf, ip);
 
 if (def-family) {
 virBufferAsprintf(buf,  family='%s', def-family);
@@ -1468,14 +1470,17 @@ virNetworkIpDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, prefix='%u', def-prefix);
 }
 virBufferAddLit(buf, \n);
+virBufferAdjustIndent(buf, 2);
 
 if (def-tftproot) {
-virBufferEscapeString(buf, tftp root='%s' /\n,
+virBufferEscapeString(buf, tftp root='%s' /\n,
   def-tftproot);
 }
 if ((def-nranges || def-nhosts)) {
 int ii;
-virBufferAddLit(buf, dhcp\n);
+virBufferAddLit(buf, dhcp\n);
+virBufferAdjustIndent(buf, 2);
+
 for (ii = 0 ; ii  def-nranges ; ii++) {
 char *saddr = virSocketAddrFormat(def-ranges[ii].start);
 if (!saddr)
@@ -1485,13 +1490,13 @@ virNetworkIpDefFormat(virBufferPtr buf,
 VIR_FREE(saddr);
 goto error;
 }
-virBufferAsprintf(buf,   range start='%s' end='%s' /\n,
+virBufferAsprintf(buf, range start='%s' end='%s' /\n,
   saddr, eaddr);
 VIR_FREE(saddr);
 VIR_FREE(eaddr);
 }
 for (ii = 0 ; ii  def-nhosts ; ii++) {
-virBufferAddLit(buf,   host );
+

51 matches

Mail list logo