[libvirt] [PATCH 3/3] Parallels: Include CPU info in the capabilities XML

[Alexander Burluka]

From: A.Burluka 

Openstack uses (or will start to using) CPU info from the
capabilities XML. So this section is expanded, added CPU info
about arch, type and info about number of cores, sockets and threads.
---
 src/parallels/parallels_driver.c |   28 ++--
 1 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 96d2e45..87e540e 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -118,8 +118,11 @@ parallelsDomObjFreePrivate(void *p)
 static virCapsPtr
 parallelsBuildCapabilities(void)
 {
-virCapsPtr caps;
+virCapsPtr caps = NULL;
+virCPUDefPtr cpu = NULL;
+virCPUDataPtr data = NULL;
 virCapsGuestPtr guest;
+virNodeInfo nodeinfo;
 
 if ((caps = virCapabilitiesNew(virArchFromHost(),
0, 0)) == NULL)
@@ -148,11 +151,32 @@ parallelsBuildCapabilities(void)
   "parallels", NULL, NULL, 0, NULL) == 
NULL)
 goto error;
 
+if (VIR_ALLOC(cpu) < 0)
+goto error;
+
+if (nodeGetInfo(&nodeinfo))
+goto error;
+
+cpu->arch = caps->host.arch;
+cpu->type = VIR_CPU_TYPE_HOST;
+cpu->sockets = nodeinfo.sockets;
+cpu->cores = nodeinfo.cores;
+cpu->threads = nodeinfo.threads;
+
+caps->host.cpu = cpu;
+
+if (!(data = cpuNodeData(cpu->arch))
+|| cpuDecode(cpu, data, NULL, 0, NULL) < 0) {
+goto cleanup;
+}
+
+ cleanup:
+cpuDataFree(data);
 return caps;
 
  error:
 virObjectUnref(caps);
-return NULL;
+goto cleanup;
 }
 
 static char *
-- 
1.7.1

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

[libvirt] [PATCH 0/3] Parallels: patch set

[Alexander Burluka]

From: A.Burluka 

I've fixed my previous patches after Libvirt community reviews 
(thanks to Eric Blake and Daniel P. Berrange).
Parallels team continues working on Libvirt Parallels driver integration 
with OpenStack.

A.Burluka (3):
  Parallels: add domainGetVcpus().
  Parallels: add connectBaselineCPU().
  Parallels: Include CPU info in the capabilities XML

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

[libvirt] [PATCH 1/3] Parallels: add domainGetVcpus().

[Alexander Burluka]

From: A.Burluka 

OpenStack Nova requires this function
to start VM instance. Cpumask info is obtained via prlctl utility.
Unlike KVM, Parallels Cloud Server is unable to set cpu affinity
mask for every VCpu. Mask is unique for all VCpu. You can set it
using 'prlctl set  --cpumask <{n[,n,n1-n2]|all}>'
command. For example, 'prlctl set SomeDomain --cpumask 0,1,5-7'
would set this mask to yy---yyy.
---
 src/parallels/parallels_driver.c |   93 -
 src/parallels/parallels_utils.h  |1 +
 2 files changed, 91 insertions(+), 3 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index ab59599..f6e701d 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -108,6 +108,7 @@ parallelsDomObjFreePrivate(void *p)
 if (!pdom)
 return;
 
+virBitmapFree(pdom->cpumask);
 VIR_FREE(pdom->uuid);
 VIR_FREE(pdom->home);
 VIR_FREE(p);
@@ -650,10 +651,14 @@ parallelsLoadDomain(parallelsConnPtr privconn, 
virJSONValuePtr jobj)
 unsigned int x;
 const char *autostart;
 const char *state;
+int hostcpus;
 
 if (VIR_ALLOC(def) < 0)
 goto cleanup;
 
+if (VIR_ALLOC(pdom) < 0)
+goto cleanup;
+
 def->virtType = VIR_DOMAIN_VIRT_PARALLELS;
 def->id = -1;
 
@@ -716,6 +721,19 @@ parallelsLoadDomain(parallelsConnPtr privconn, 
virJSONValuePtr jobj)
 goto cleanup;
 }
 
+if ((hostcpus = nodeGetCPUCount()) < 0)
+goto cleanup;
+
+if (!(tmp = virJSONValueObjectGetString(jobj3, "mask"))) {
+/* Absence of this field means that all domains cpus are available */
+if (!(pdom->cpumask = virBitmapNew(hostcpus)))
+goto cleanup;
+virBitmapSetAll(pdom->cpumask);
+} else {
+if (virBitmapParse(tmp, 0, &pdom->cpumask, hostcpus) < 0)
+goto cleanup;
+}
+
 if (!(jobj3 = virJSONValueObjectGet(jobj2, "memory"))) {
 parallelsParseError();
 goto cleanup;
@@ -757,9 +775,6 @@ parallelsLoadDomain(parallelsConnPtr privconn, 
virJSONValuePtr jobj)
 
 def->os.arch = VIR_ARCH_X86_64;
 
-if (VIR_ALLOC(pdom) < 0)
-goto cleanup;
-
 if (virJSONValueObjectGetNumberUint(jobj, "EnvID", &x) < 0)
 goto cleanup;
 pdom->id = x;
@@ -2300,6 +2315,77 @@ static int parallelsConnectIsAlive(virConnectPtr conn 
ATTRIBUTE_UNUSED)
 }
 
 
+static int
+parallelsDomainGetVcpus(virDomainPtr domain,
+virVcpuInfoPtr info,
+int maxinfo,
+unsigned char *cpumaps,
+int maplen)
+{
+parallelsConnPtr privconn = domain->conn->privateData;
+parallelsDomObjPtr privdomdata = NULL;
+virDomainObjPtr privdom = NULL;
+size_t i;
+int v, maxcpu, hostcpus;
+int ret = -1;
+
+parallelsDriverLock(privconn);
+privdom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
+parallelsDriverUnlock(privconn);
+
+if (privdom == NULL) {
+parallelsDomNotFoundError(domain);
+goto cleanup;
+}
+
+if (!virDomainObjIsActive(privdom)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s",
+   _("cannot list vcpu pinning for an inactive domain"));
+goto cleanup;
+}
+
+privdomdata = privdom->privateData;
+if ((hostcpus = nodeGetCPUCount()) < 0)
+goto cleanup;
+
+maxcpu = maplen * 8;
+if (maxcpu > hostcpus)
+maxcpu = hostcpus;
+
+if (maxinfo >= 1) {
+if (info != NULL) {
+memset(info, 0, sizeof(*info) * maxinfo);
+for (i = 0; i < maxinfo; i++) {
+info[i].number = i;
+info[i].state = VIR_VCPU_RUNNING;
+}
+}
+if (cpumaps != NULL) {
+unsigned char *tmpmap = NULL;
+int tmpmapLen = 0;
+
+memset(cpumaps, 0, maplen * maxinfo);
+virBitmapToData(privdomdata->cpumask, &tmpmap, &tmpmapLen);
+if (tmpmapLen > maplen)
+tmpmapLen = maplen;
+
+for (v = 0; v < maxinfo; v++) {
+unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v);
+memcpy(cpumap, tmpmap, tmpmapLen);
+}
+VIR_FREE(tmpmap);
+}
+}
+ret = maxinfo;
+
+ cleanup:
+if (privdom)
+virObjectUnlock(privdom);
+return ret;
+}
+
+
 static virDriver parallelsDriver = {
 .no = VIR_DRV_PARALLELS,
 .name = "Parallels",
@@ -2323,6 +2409,7 @@ static virDriver parallelsDriver = {
 .domainGetXMLDesc = parallelsDomainGetXMLDesc,/* 0.10.0 */
 .domainIsPersistent = parallelsDomainIsPersistent,/* 0.10.0 */
 .domainGetAutostart = parallelsDomainGetAutostart,/* 0.10.0 */
+.domainGetVcpus = parallelsDomainGetVcpus, /* 1.2.6 */
 .domainSuspend = parallelsDomainSuspend,/* 0.10.0 */
 .domainResume = parallelsDomain

[libvirt] [PATCH 2/3] Parallels: add connectBaselineCPU().

[Alexander Burluka]

From: A.Burluka 

Openstack Nova (starting at Icehouse release) requires this function
to start VM.
---
 src/parallels/parallels_driver.c |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index f6e701d..96d2e45 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -51,6 +51,7 @@
 #include "nodeinfo.h"
 #include "c-ctype.h"
 #include "virstring.h"
+#include "cpu/cpu.h"
 
 #include "parallels_driver.h"
 #include "parallels_utils.h"
@@ -2315,6 +2316,18 @@ static int parallelsConnectIsAlive(virConnectPtr conn 
ATTRIBUTE_UNUSED)
 }
 
 
+static char *
+parallelsConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED,
+const char **xmlCPUs,
+unsigned int ncpus,
+unsigned int flags)
+{
+virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL);
+
+return cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags);
+}
+
+
 static int
 parallelsDomainGetVcpus(virDomainPtr domain,
 virVcpuInfoPtr info,
@@ -2395,6 +2408,7 @@ static virDriver parallelsDriver = {
 .connectGetHostname = parallelsConnectGetHostname,  /* 0.10.0 */
 .nodeGetInfo = parallelsNodeGetInfo,  /* 0.10.0 */
 .connectGetCapabilities = parallelsConnectGetCapabilities,  /* 0.10.0 
*/
+.connectBaselineCPU = parallelsConnectBaselineCPU, /* 1.2.6 */
 .connectListDomains = parallelsConnectListDomains,  /* 0.10.0 */
 .connectNumOfDomains = parallelsConnectNumOfDomains,/* 0.10.0 */
 .connectListDefinedDomains = parallelsConnectListDefinedDomains,/* 
0.10.0 */
-- 
1.7.1

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

Re: [libvirt] [PATCH V8 2/2] libxl: add migration support

[Jim Fehlig]

Daniel P. Berrange wrote:
> On Wed, Jun 04, 2014 at 02:35:14PM -0600, Jim Fehlig wrote:
>   
>> This patch adds initial migration support to the libxl driver,
>> using the VIR_DRV_FEATURE_MIGRATION_PARAMS family of migration
>> functions.
>>
>> Signed-off-by: Jim Fehlig 
>> ---
>>  po/POTFILES.in  |   1 +
>>  src/Makefile.am |   3 +-
>>  src/libxl/libxl_conf.h  |   6 +
>>  src/libxl/libxl_domain.h|   1 +
>>  src/libxl/libxl_driver.c| 235 ++
>>  src/libxl/libxl_migration.c | 585 
>> 
>>  src/libxl/libxl_migration.h |  79 ++
>>  7 files changed, 909 insertions(+), 1 deletion(-)
>> 
>
> ACK
>   

Thanks, I pushed these patches now.  The ABI stability check was a nice
improvement.  Verified it works when making simple changes like number
of vcpus, max mem, etc.

Regards,
Jim

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

[libvirt] [PATCH] maint: detect VPATH builds when checking for gnulib update

[Eric Blake]

I accidentally typed 'make' in the srcdir of a VPATH build, and
was surprised to see this:

$ make
/bin/sh: s/^[ +-]//;s/ .*//: No such file or directory
INFO: gnulib update required; running ./autogen.sh first
make: -n: Command not found
./autogen.sh
I am going to run ./configure with no arguments - if you wish
to pass any to it, please specify them on the ./autogen.sh command line.
running bootstrap...
./bootstrap: Bootstrapping from checked-out libvirt sources...
./bootstrap: getting gnulib files...

Oops - we're trying to execute some fairly bogus command names,
and then trying to configure in-tree (which breaks all existing
VPATH builds, since automake refuses to do a VPATH build if it
detects an in-tree configure).  The third line (executing "-n")
is fixed by updating to the latest gnulib; the rest of the problem
is fixed by copying the same filtering in our cfg.mk as what
gnulib just added, so that we avoid any $(shell) invocations which
in turn depend on variables that are only populated by a working
Makefile.  With that in place, we are back to the much nicer:

$ make
There seems to be no Makefile in this directory.
You must run ./configure before running 'make'.
make: *** [abort-due-to-no-makefile] Error 1

Additionally, although harder to see - there was a trailing space in
the message warning us that autogen would run an in-tree configure.

* .gnulib: Update to latest, in part for maint.mk improvements.
* cfg.mk (_update_required): Don't check for update in
unconfigured directory.
* autogen.sh (no_git): Drop trailing space.

Signed-off-by: Eric Blake 
---

Pushing under the gnulib maintenance rule.

* .gnulib e8e0eb6...d55899f (35):
  > maintainer-makefile: delete obsolete code
  > maintainer-makefile: avoid spurious error messages
  > rename: avoid unused-but-set-variable compiler warning
  > maint: add ChangeLog entry missing in previous commit
  > rename: mark a label as potentially unused
  > gnulib-common.m4: Fix typo in _GL_UNUSED_LABEL.
  > acl: apply pure attribute to two functions
  > gnulib-common.m4: add _GL_UNUSED_LABEL
  > dup2, fcntl, fcntl-h: port to AIX 7.1
  > printf, config.rpath: Port to FreeBSD 10.
  > ftoastr: work around compiler bug in IBM xlc 12.1
  > valgrind-tests: fixed misleading help message
  > isfinite, isinf, isnan tests: fix for little-endian PowerPC
  > exclude-tests: port to AIX 7.1
  > pthread_sigmask, timer-time: use gl_THREADLIB only if needed
  > gnulib-tool: wget translations using --no-verbose rather than --quiet
  > gnulib-tool: adjust translation wget to avoid a https redirection
  > getlogin_r-tests: check return value rather than errno
  > getlogin_r-tests: fix various issues in recent change
  > fchdir: port 'open' and 'close' redefinitions to AIX 7.1
  > update from texinfo
  > xalloc: don't potentially generate invalid code for xmemdup calls
  > getlogin_r-tests: avoid false failure under sudo/ssh etc.
  > getlogin-tests: avoid false failure under cron
  > mbrtowc.m4: fix a comment typo
  > mbrlen, mbrtowc: fix bug with empty input
  > doc: document mbrtowc and mbrlen problem with empty input
  > doc: document exec* = spawn+exit bug with non-Cygwin Windows platforms
  > autoupdate
  > getlogin-tests: avoid false failure under sudo/ssh etc.
  > mbsstr, quotearg, xstrtol: pacify IRIX 6.5 cc
  > update from texinfo
  > autoupdate
  > autoupdate
  > autoupdate

 .gnulib| 2 +-
 autogen.sh | 2 +-
 cfg.mk | 4 
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/.gnulib b/.gnulib
index e8e0eb6..d55899f 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit e8e0eb6bfb728685ec8d5afd924e41b18e9d928d
+Subproject commit d55899fd2c5794ac85ecb14d5e2f646a89e4b4dd
diff --git a/autogen.sh b/autogen.sh
index 5aa1990..1965f64 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -35,7 +35,7 @@ if test -z "$NOCONFIGURE" ; then
 echo "Running ./configure with $EXTRA_ARGS $@"
   else
 if test -z "$*" && test ! -f "$THEDIR/config.status"; then
-echo "I am going to run ./configure with no arguments - if you wish "
+echo "I am going to run ./configure with no arguments - if you wish"
 echo "to pass any to it, please specify them on the $0 command line."
 fi
   fi
diff --git a/cfg.mk b/cfg.mk
index 4601b35..b5f1fa2 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -939,6 +939,7 @@ sc_require_locale_h:
 # We don't use this feature of maint.mk.
 prev_version_file = /dev/null

+ifneq ($(_gl-Makefile),)
 ifeq (0,$(MAKELEVEL))
   _curr_status = .git-module-status
   # The sed filter accommodates those who check out on a commit from which
@@ -971,6 +972,7 @@ ifeq (0,$(MAKELEVEL))
 maint.mk Makefile: _autogen
   endif
 endif
+endif

 # It is necessary to call autogen any time gnulib changes.  Autogen
 # reruns configure, then we regenerate all Makefiles at once.
@@ -980,7 +982,9 @@ _autogen:
./config.status

 # regenerate HACKING as part of the syntax-check
+ifneq ($(_gl-Makefile),)
 syntax-check: $(top_srcdir)/HACKING bracket

Re: [libvirt] [PATCH V8 2/2] libxl: add migration support

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 02:35:14PM -0600, Jim Fehlig wrote:
> This patch adds initial migration support to the libxl driver,
> using the VIR_DRV_FEATURE_MIGRATION_PARAMS family of migration
> functions.
> 
> Signed-off-by: Jim Fehlig 
> ---
>  po/POTFILES.in  |   1 +
>  src/Makefile.am |   3 +-
>  src/libxl/libxl_conf.h  |   6 +
>  src/libxl/libxl_domain.h|   1 +
>  src/libxl/libxl_driver.c| 235 ++
>  src/libxl/libxl_migration.c | 585 
> 
>  src/libxl/libxl_migration.h |  79 ++
>  7 files changed, 909 insertions(+), 1 deletion(-)

ACK


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

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

Re: [libvirt] [PATCH V8 1/2] libxl: introduce libxlDomainDefCheckABIStability

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 02:35:13PM -0600, Jim Fehlig wrote:
> Introduce a simple libxlDomainDefCheckABIStability() function that
> can be used check ABI stability between two virDomainDef objects.
> 
> Signed-off-by: Jim Fehlig 
> ---
>  src/libxl/libxl_domain.c | 23 +++
>  src/libxl/libxl_domain.h |  5 +
>  2 files changed, 28 insertions(+)

ACK


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

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

Re: [libvirt] [PATCH 2/4] interface_backend_udev: Implement link speed & state

[Daniel P. Berrange]

On Tue, Jun 03, 2014 at 02:22:10PM +0200, Michal Privoznik wrote:
> In previous commit the interface XML is prepared for exporting
> information on NIC link speed and state. This commit implement
> actual logic for getting such info and writing it into XML.
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> Notes:
> For the kernel issue I've posted patch here:
> 
> http://patchwork.ozlabs.org/patch/354928/
> 
> But it wasn't accepted as developers are afraid of breaking backward
> compatibility. Which is weird as in 2.6.X the kernel was reporting -1
> instead of unsigned -1.
> 
>  src/interface/interface_backend_udev.c | 27 +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/src/interface/interface_backend_udev.c 
> b/src/interface/interface_backend_udev.c
> index c5353ea..80ba93e 100644
> --- a/src/interface/interface_backend_udev.c
> +++ b/src/interface/interface_backend_udev.c
> @@ -1036,6 +1036,8 @@ udevGetIfaceDef(struct udev *udev, const char *name)
>  const char *mtu_str;
>  char *vlan_parent_dev = NULL;
>  const char *devtype;
> +const char *operstate;
> +const char *link_speed;
>  
>  /* Allocate our interface definition structure */
>  if (VIR_ALLOC(ifacedef) < 0)
> @@ -1059,6 +1061,31 @@ udevGetIfaceDef(struct udev *udev, const char *name)
> udev_device_get_sysattr_value(dev, "address")) < 0)
>  goto error;
>  
> +operstate = udev_device_get_sysattr_value(dev, "operstate");
> +if ((ifacedef->lnk.state = virInterfaceStateTypeFromString(operstate)) 
> <= 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Unable to parse operstate: %s"),
> +   operstate);
> +goto error;
> +}
> +
> +link_speed = udev_device_get_sysattr_value(dev, "speed");
> +if (link_speed) {
> +if (virStrToLong_ul(link_speed, NULL, 10, &ifacedef->lnk.speed) < 0) 
> {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Unable to parse speed: %s"),
> +   link_speed);
> +goto error;
> +}
> +
> +/* Workaround broken kernel API. If the link is unplugged then
> + * depending on the NIC driver, link speed can be reported as -1.
> + * However, the value is printed out as unsigned integer instead of
> + * signed one. Terrifying but true. */
> +if ((int) ifacedef->lnk.speed == -1)
> +ifacedef->lnk.speed = 0;
> +}

This block of code is duplicated in two other places in your series.
I think it'd be worth having a function in virnetdev

  virNetDevGetLinkInfo(const char *ifname, int *speed, int *state)


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

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

Re: [libvirt] [PATCH 1/4] virInterface: Expose link state & speed

[Daniel P. Berrange]

On Tue, Jun 03, 2014 at 02:22:09PM +0200, Michal Privoznik wrote:
> Currently it is not possible to determine the speed of an interface
> and whether a link is actually detected from the API. Orchestrating
> platforms want to be able to determine when the link has failed and
> where multiple speeds may be available which one the interface is
> actually connected at. This commit introduces an extension to our
> interface XML (without implementation to interface driver backends):
> 
>   
> 
> 
> 
> 
> ...
>   
> 
> Where @speed is negotiated link speed in Mbits per second, and state
> is the current NIC state (can be one of the following:  "unknown",
> "notpresent", "down", "lowerlayerdown","testing", "dormant", "up").
> 
> Signed-off-by: Michal Privoznik 
> ---
>  docs/schemas/basictypes.rng | 25 ++
>  docs/schemas/interface.rng  |  1 +
>  src/conf/device_conf.c  | 62 
> +
>  src/conf/device_conf.h  | 27 ++-
>  src/conf/interface_conf.c   | 11 -
>  src/conf/interface_conf.h   |  2 +
>  src/libvirt_private.syms|  4 ++
>  tests/interfaceschemadata/bridge-no-address.xml |  1 +
>  tests/interfaceschemadata/bridge.xml|  1 +
>  tests/interfaceschemadata/ethernet-dhcp.xml |  1 +
>  10 files changed, 133 insertions(+), 2 deletions(-)

ACK

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

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

Re: [libvirt] [PATCH V7] libxl: add migration support

[Jim Fehlig]

Daniel P. Berrange wrote:
> On Thu, May 08, 2014 at 03:56:51PM -0600, Jim Fehlig wrote:
>   
>> This patch adds initial migration support to the libxl driver,
>> using the VIR_DRV_FEATURE_MIGRATION_PARAMS family of migration
>> functions.
>>
>> Signed-off-by: Jim Fehlig 
>> ---
>>
>> V6 here
>>   https://www.redhat.com/archives/libvir-list/2014-May/msg00017.html
>>
>> In V7:
>> There were no comments on V6, but during testing I noticed that
>> 'virsh migrate --suspend ...' was not being honored.  Fixed that
>> in this version so that the domain is paused on the destination
>> once migration completes.
>>
>>  po/POTFILES.in  |   1 +
>>  src/Makefile.am |   3 +-
>>  src/libxl/libxl_conf.h  |   6 +
>>  src/libxl/libxl_domain.h|   1 +
>>  src/libxl/libxl_driver.c| 235 ++
>>  src/libxl/libxl_migration.c | 581 
>> 
>>  src/libxl/libxl_migration.h |  79 ++
>>  7 files changed, 905 insertions(+), 1 deletion(-)
>> 
>
>
>   
>> @@ -301,6 +303,13 @@ libxlStateInitialize(bool privileged,
>>LIBXL_VNC_PORT_MAX)))
>>  goto error;
>>  
>> +/* Allocate bitmap for migration port reservation */
>> +if (!(libxl_driver->migrationPorts =
>> +  virPortAllocatorNew(_("migration"),
>> +  LIBXL_MIGRATION_PORT_MIN,
>> +  LIBXL_MIGRATION_PORT_MAX)))
>> +goto error;
>> +
>> 
>
> No need todo it in this patch, but experiance with QEMU is that people will
> want this range to be configurable in /etc/libvirt/libxl.conf. So I'd
> suggest adding this later
>   

Nod.  I have an old, dusty patchset that integrates virlockd in the
libxl driver.  The series includes introducing a conf file for the libxl
driver.  I'll get back to working on that before long.

>   
>> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
>> new file mode 100644
>> index 000..e3699c5
>> --- /dev/null
>> +++ b/src/libxl/libxl_migration.c
>> @@ -0,0 +1,581 @@
>> +/*
>> + * libxl_migration.c: methods for handling migration with libxenlight
>> + *
>> + * Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
>> + *
>> + * 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, see
>> + * .
>> + *
>> + * Authors:
>> + * Jim Fehlig 
>> + * Chunyan Liu 
>> + */
>> +
>> +#include 
>> +
>> +#include "internal.h"
>> +#include "virlog.h"
>> +#include "virerror.h"
>> +#include "virconf.h"
>> +#include "datatypes.h"
>> +#include "virfile.h"
>> +#include "viralloc.h"
>> +#include "viruuid.h"
>> +#include "vircommand.h"
>> +#include "virstring.h"
>> +#include "virobject.h"
>> +#include "rpc/virnetsocket.h"
>> +#include "libxl_domain.h"
>> +#include "libxl_driver.h"
>> +#include "libxl_conf.h"
>> +#include "libxl_migration.h"
>> +
>> +#define VIR_FROM_THIS VIR_FROM_LIBXL
>> +
>> +VIR_LOG_INIT("libxl.libxl_migration");
>> +
>> +typedef struct _libxlMigrationDstArgs {
>> +virObject parent;
>> +
>> +virConnectPtr conn;
>> +virDomainObjPtr vm;
>> +unsigned int flags;
>> +
>> +/* for freeing listen sockets */
>> +virNetSocketPtr *socks;
>> +size_t nsocks;
>> +} libxlMigrationDstArgs;
>> +
>> +static virClassPtr libxlMigrationDstArgsClass;
>> +
>> +static void
>> +libxlMigrationDstArgsDispose(void *obj)
>> +{
>> +libxlMigrationDstArgs *args = obj;
>> +
>> +VIR_FREE(args->socks);
>> +}
>> +
>> +static int
>> +libxlMigrationDstArgsOnceInit(void)
>> +{
>> +if (!(libxlMigrationDstArgsClass = virClassNew(virClassForObject(),
>> +   "libxlMigrationDstArgs",
>> +   
>> sizeof(libxlMigrationDstArgs),
>> +   
>> libxlMigrationDstArgsDispose)))
>> +return -1;
>> +
>> +return 0;
>> +}
>> +
>> +VIR_ONCE_GLOBAL_INIT(libxlMigrationDstArgs)
>> +
>> +static void
>> +libxlDoMigrateReceive(virNetSocketPtr sock,
>> +  int events ATTRIBUTE_UNUSED,
>> +  void *opaque)
>> +{
>> +libxlMigrationDstArgs *args = opaque;
>> +virConnectPtr conn = args->conn;
>> +virDomainObjPtr vm = args->vm;
>> +virNetSocketPtr *socks = args->socks;
>> +size_t nsocks = args->nso

[libvirt] [PATCH V8 0/2] libxl: add migration support

[Jim Fehlig]

V7 here
  https://www.redhat.com/archives/libvir-list/2014-May/msg00301.html

In V8:
- Set freed resources to NULL/0.
- Add libxlDomainDefCheckABIStability() function to do simple
  ABI stability checks of two virDomainDef objects.

Jim Fehlig (2):
  libxl: introduce libxlDomainDefCheckABIStability
  libxl: add migration support

 po/POTFILES.in  |   1 +
 src/Makefile.am |   3 +-
 src/libxl/libxl_conf.h  |   6 +
 src/libxl/libxl_domain.c|  23 ++
 src/libxl/libxl_domain.h|   6 +
 src/libxl/libxl_driver.c| 235 ++
 src/libxl/libxl_migration.c | 585 
 src/libxl/libxl_migration.h |  79 ++
 8 files changed, 937 insertions(+), 1 deletion(-)
 create mode 100644 src/libxl/libxl_migration.c
 create mode 100644 src/libxl/libxl_migration.h

-- 
1.8.4.5

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

[libvirt] [PATCH V8 2/2] libxl: add migration support

[Jim Fehlig]

This patch adds initial migration support to the libxl driver,
using the VIR_DRV_FEATURE_MIGRATION_PARAMS family of migration
functions.

Signed-off-by: Jim Fehlig 
---
 po/POTFILES.in  |   1 +
 src/Makefile.am |   3 +-
 src/libxl/libxl_conf.h  |   6 +
 src/libxl/libxl_domain.h|   1 +
 src/libxl/libxl_driver.c| 235 ++
 src/libxl/libxl_migration.c | 585 
 src/libxl/libxl_migration.h |  79 ++
 7 files changed, 909 insertions(+), 1 deletion(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index cff92d9..2ee7225 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -74,6 +74,7 @@ src/lxc/lxc_process.c
 src/libxl/libxl_domain.c
 src/libxl/libxl_driver.c
 src/libxl/libxl_conf.c
+src/libxl/libxl_migration.c
 src/network/bridge_driver.c
 src/network/bridge_driver_linux.c
 src/network/leaseshelper.c
diff --git a/src/Makefile.am b/src/Makefile.am
index d82ca26..01af164 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -707,7 +707,8 @@ XENAPI_DRIVER_SOURCES = 
\
 LIBXL_DRIVER_SOURCES = \
libxl/libxl_conf.c libxl/libxl_conf.h   \
libxl/libxl_domain.c libxl/libxl_domain.h   \
-   libxl/libxl_driver.c libxl/libxl_driver.h
+   libxl/libxl_driver.c libxl/libxl_driver.h   \
+   libxl/libxl_migration.c libxl/libxl_migration.h
 
 UML_DRIVER_SOURCES =   \
uml/uml_conf.c uml/uml_conf.h   \
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 433d6da..6aa36d2 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -43,6 +43,9 @@
 # define LIBXL_VNC_PORT_MIN  5900
 # define LIBXL_VNC_PORT_MAX  65535
 
+# define LIBXL_MIGRATION_PORT_MIN  49152
+# define LIBXL_MIGRATION_PORT_MAX  49216
+
 # define LIBXL_CONFIG_DIR SYSCONFDIR "/libvirt/libxl"
 # define LIBXL_AUTOSTART_DIR LIBXL_CONFIG_DIR "/autostart"
 # define LIBXL_STATE_DIR LOCALSTATEDIR "/run/libvirt/libxl"
@@ -115,6 +118,9 @@ struct _libxlDriverPrivate {
 /* Immutable pointer, self-locking APIs */
 virPortAllocatorPtr reservedVNCPorts;
 
+/* Immutable pointer, self-locking APIs */
+virPortAllocatorPtr migrationPorts;
+
 /* Immutable pointer, lockless APIs*/
 virSysinfoDefPtr hostsysinfo;
 };
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index 6939008..f459fdf 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -69,6 +69,7 @@ struct _libxlDomainObjPrivate {
 virChrdevsPtr devs;
 libxl_evgen_domain_death *deathW;
 libxlDriverPrivatePtr driver;
+unsigned short migrationPort;
 
 struct libxlDomainJobObj job;
 };
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 515d5c9..9feacb1 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -45,6 +45,7 @@
 #include "libxl_domain.h"
 #include "libxl_driver.h"
 #include "libxl_conf.h"
+#include "libxl_migration.h"
 #include "xen_xm.h"
 #include "xen_sxpr.h"
 #include "virtypedparam.h"
@@ -209,6 +210,7 @@ libxlStateCleanup(void)
 virObjectUnref(libxl_driver->xmlopt);
 virObjectUnref(libxl_driver->domains);
 virObjectUnref(libxl_driver->reservedVNCPorts);
+virObjectUnref(libxl_driver->migrationPorts);
 
 virObjectEventStateFree(libxl_driver->domainEventState);
 virSysinfoDefFree(libxl_driver->hostsysinfo);
@@ -301,6 +303,13 @@ libxlStateInitialize(bool privileged,
   LIBXL_VNC_PORT_MAX)))
 goto error;
 
+/* Allocate bitmap for migration port reservation */
+if (!(libxl_driver->migrationPorts =
+  virPortAllocatorNew(_("migration"),
+  LIBXL_MIGRATION_PORT_MIN,
+  LIBXL_MIGRATION_PORT_MAX)))
+goto error;
+
 if (!(libxl_driver->domains = virDomainObjListNew()))
 goto error;
 
@@ -4153,6 +4162,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int 
feature)
 
 switch (feature) {
 case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
+case VIR_DRV_FEATURE_MIGRATION_PARAMS:
 return 1;
 default:
 return 0;
@@ -4331,6 +4341,226 @@ libxlNodeDeviceReset(virNodeDevicePtr dev)
 return ret;
 }
 
+static char *
+libxlDomainMigrateBegin3Params(virDomainPtr domain,
+   virTypedParameterPtr params,
+   int nparams,
+   char **cookieout ATTRIBUTE_UNUSED,
+   int *cookieoutlen ATTRIBUTE_UNUSED,
+   unsigned int flags)
+{
+const char *xmlin = NULL;
+virDomainObjPtr vm = NULL;
+
+virCheckFlags(LIBXL_MIGRATION_FLAGS, NULL);
+if (virTypedParamsValidate(params, nparams, LIBXL_MIGRATION_PARAMETERS) < 
0)
+return NULL;
+
+if (virTypedParamsGetString(

[libvirt] [PATCH V8 1/2] libxl: introduce libxlDomainDefCheckABIStability

[Jim Fehlig]

Introduce a simple libxlDomainDefCheckABIStability() function that
can be used check ABI stability between two virDomainDef objects.

Signed-off-by: Jim Fehlig 
---
 src/libxl/libxl_domain.c | 23 +++
 src/libxl/libxl_domain.h |  5 +
 2 files changed, 28 insertions(+)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index eab789a..73242ac 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1256,3 +1256,26 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 virObjectUnref(cfg);
 return ret;
 }
+
+bool
+libxlDomainDefCheckABIStability(libxlDriverPrivatePtr driver,
+virDomainDefPtr src,
+virDomainDefPtr dst)
+{
+virDomainDefPtr migratableDefSrc = NULL;
+virDomainDefPtr migratableDefDst = NULL;
+libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+bool ret = false;
+
+if (!(migratableDefSrc = virDomainDefCopy(src, cfg->caps, driver->xmlopt, 
true)) ||
+!(migratableDefDst = virDomainDefCopy(dst, cfg->caps, driver->xmlopt, 
true)))
+goto cleanup;
+
+ret = virDomainDefCheckABIStability(migratableDefSrc, migratableDefDst);
+
+ cleanup:
+virDomainDefFree(migratableDefSrc);
+virDomainDefFree(migratableDefDst);
+virObjectUnref(cfg);
+return ret;
+}
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index 979ce2a..6939008 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -139,4 +139,9 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
  bool start_paused,
  int restore_fd);
 
+bool
+libxlDomainDefCheckABIStability(libxlDriverPrivatePtr driver,
+virDomainDefPtr src,
+virDomainDefPtr dst);
+
 #endif /* LIBXL_DOMAIN_H */
-- 
1.8.4.5

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

Re: [libvirt] [PATCH] cfg.mk: Introduce rule for setlocale()

[Eric Blake]

On 06/04/2014 10:24 AM, Michal Privoznik wrote:

>>
 +# Require #include  in all files that call setlocale()
 +sc_require_locale_h:
 +@for i in $$($(VC_LIST_EXCEPT) | grep '\.[chx]$$');
 do\
 +if test -z "$$(grep setlocale $$i)" ; then continue; fi;\
>>>
>>> Why not:
>>>
>>> if ! grep -q setlocale $$i; then continue; fi
>>
>> Even simpler, let maint.mk do it for you:
>>
>> sc_require_locale_h:
>> @require='include.*locale\.h'\
>> containing='setlocale *('\
>> halt='setlocale() requires '\
>>   $(_sc_search_regexp)
>>
> 
> This is the maint.mk-ism I was looking for. Feel free to replace my
> code. Just out of pure curiosity - is your code any faster?

Yes.  Your code spawns 1 or 2 greps per file (one spawn per iteration of
the forloop); while maint.mk's rule does tricks like:

   : Filter by content; \
   test -n "$$files" && test -n "$$containing"  \
 && { files=$$(grep -l "$$containing" $$files); } || :; \

that use a single grep process to drastically filter the set of files
that then need to be tested.

I see you already pushed, so I'll take your message as the ACK to go
ahead and push my replacement.

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



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

Re: [libvirt] Parse commandline for -device option

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 10:27:32AM -0600, Eric Blake wrote:
> On 06/04/2014 03:07 AM, hong-hua@freescale.com wrote:
> > Hi Maintainers,
> > 
> > Currently libvirt could only parse commandline for several legacy devices.
> > For example,
> > -pcidevice: qemuParseCommandLinePCI()
> > -usbdevice: qemuParseCommandLineUSB()
> 
> Yes, the domxml-from-native functionality is woefully under-maintained
> for qemu.  Patches are welcome.
> 
> > Is it complicated to add support to -device option in function 
> > qemuParseCommandLine()?
> > Do you have any thought or plan about it?
> 
> You're welcome to give it a shot to find out how easy or hard it will
> be; it's just that few enough people are relying on it that no one has
> moved it to the top of their priority list to fix it.

It probably isn't that hard - merely alot of tedious work - which is
why it hasn't been done. If you do want to work on it, don't feel you
need to implement everything - even just making a small start for a
particular type of device (eg host PCI devicecs), would be useful and
set a foundation for future work


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

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

Re: [libvirt] Parse commandline for -device option

[Eric Blake]

On 06/04/2014 03:07 AM, hong-hua@freescale.com wrote:
> Hi Maintainers,
> 
> Currently libvirt could only parse commandline for several legacy devices.
> For example,
> -pcidevice:   qemuParseCommandLinePCI()
> -usbdevice:   qemuParseCommandLineUSB()

Yes, the domxml-from-native functionality is woefully under-maintained
for qemu.  Patches are welcome.

> Is it complicated to add support to -device option in function 
> qemuParseCommandLine()?
> Do you have any thought or plan about it?

You're welcome to give it a shot to find out how easy or hard it will
be; it's just that few enough people are relying on it that no one has
moved it to the top of their priority list to fix it.

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



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

Re: [libvirt] [PATCH] cfg.mk: Introduce rule for setlocale()

[Michal Privoznik]

On 04.06.2014 18:21, Eric Blake wrote:

On 06/04/2014 03:15 AM, Martin Kletzander wrote:

On Wed, Jun 04, 2014 at 10:59:08AM +0200, Michal Privoznik wrote:

In the past we had some issues where setlocale() was called without
corresponding include of locale.h. While on some systems this may
work, on others the compilation failed. We should have a syntax-check
rule for that to prevent this from happening again.

Signed-off-by: Michal Privoznik 
---
cfg.mk | 9 +
1 file changed, 9 insertions(+)




+# Require #include  in all files that call setlocale()
+sc_require_locale_h:
+@for i in $$($(VC_LIST_EXCEPT) | grep '\.[chx]$$'); do\
+if test -z "$$(grep setlocale $$i)" ; then continue; fi;\


Why not:

if ! grep -q setlocale $$i; then continue; fi


Even simpler, let maint.mk do it for you:

sc_require_locale_h:
@require='include.*locale\.h'   \
containing='setlocale *('   \
halt='setlocale() requires '\
  $(_sc_search_regexp)



This is the maint.mk-ism I was looking for. Feel free to replace my 
code. Just out of pure curiosity - is your code any faster?


Michal

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

Re: [libvirt] [PATCH] cfg.mk: Introduce rule for setlocale()

[Eric Blake]

On 06/04/2014 03:15 AM, Martin Kletzander wrote:
> On Wed, Jun 04, 2014 at 10:59:08AM +0200, Michal Privoznik wrote:
>> In the past we had some issues where setlocale() was called without
>> corresponding include of locale.h. While on some systems this may
>> work, on others the compilation failed. We should have a syntax-check
>> rule for that to prevent this from happening again.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>> cfg.mk | 9 +
>> 1 file changed, 9 insertions(+)
>>

>> +# Require #include  in all files that call setlocale()
>> +sc_require_locale_h:
>> +@for i in $$($(VC_LIST_EXCEPT) | grep '\.[chx]$$'); do\
>> +if test -z "$$(grep setlocale $$i)" ; then continue; fi;\
> 
> Why not:
> 
> if ! grep -q setlocale $$i; then continue; fi

Even simpler, let maint.mk do it for you:

sc_require_locale_h:
@require='include.*locale\.h'   \
containing='setlocale *('   \
halt='setlocale() requires '  \
  $(_sc_search_regexp)

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



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

Re: [libvirt] [PATCH v2] maint: prohibit empty first lines

[Eric Blake]

On 06/04/2014 07:01 AM, Martin Kletzander wrote:
> Based on discussion with Eric:
> 
> https://www.redhat.com/archives/libvir-list/2014-March/msg01001.html
> 
> Signed-off-by: Martin Kletzander 
> ---
> v2:
>  - rebase on current master and s/FILENAME:1:/FILENAME ":1:"/
> 
>  cfg.mk | 7 +++
>  1 file changed, 7 insertions(+)

The existing sc_prohibit_empty_lines_at_EOF in maint.mk does a similar
check but using perl instead of awk.  But that shouldn't matter.

> 
> diff --git a/cfg.mk b/cfg.mk
> index 675af21..80440b5 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -938,6 +938,13 @@ sc_require_locale_h:
>   fi; 
> \
>   done;
> 
> +sc_prohibit_empty_first_line:
> + @awk 'BEGIN { fail=0; }\
> + FNR == 1 { if ($$0 == "") { print FILENAME ":1:"; fail=1; } } \

why is this to stdout...

> + END { if (fail == 1) { \
> +   print "$(ME): Prohibited empty first line" > "/dev/stderr";  \

...but this to stderr?

> + } exit fail; }' $$($(VC_LIST_EXCEPT) | grep '\.[ch]$$');

I'd use ALL of VC_LIST_EXCEPT (not just the .c and .h files).  But in
general it looks like it does the right thing.


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



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

Re: [libvirt] [PATCH 2/2] formatcaps: Rework and add stubs to document

[Eric Blake]

On 06/04/2014 09:34 AM, Michal Privoznik wrote:
> At the moment we are missing even basic documentation on our
> capabilities XML. Without demand on completeness, I'm
> reorganizing the document structure and adding very basic
> documentation to two major components of the capabilities XML.
> These stubs are intended to be enhanced in the future.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  docs/formatcaps.html.in | 158 
> 
>  1 file changed, 121 insertions(+), 37 deletions(-)
> +
> +  migration
> +  This element expose information on hypervisor's migration

s/expose/exposes/
s/on/on the/

> +  capabilities, like live migration, supported URI transports, and so
> +  on.
> +
> +  topology
> +  This element embodies the host internal topology. Management
> +  applications may want to learn this information when orchestrating new
> +  guests - e.g. due to reduce inter-NUMA node transfers.
> +
> +  secmodel
> +  To find out default security labels for different security models 
> you
> +  need to parse this element. In contrast with the former elements, this 
> is
> +  be repeated for each security model the libvirt daemon currently

s/be //

> +  supports.
> +
> +
> +
> +Guest capabilities
> +
> +While the previous section aims at host
> +capabilities, this one focus on domain's ones. The

s/focus on domain's ones/focuses on capabilities available to a guest
using a given hypervisor/

> + element will typically wrap up the following
> +elements:
> +
> +
> +os_type
> +This express what kind of operating system is the hypervisor able

s/express/expresses/
s/is the hyperviser able/the hypervisor is able/

> +to run. Possible values are:

> +
> +Examples
> +
> +For example in the case of a 64 bits

s/example/example,/
s/64 bits/64-bit/

> +machine with hardware virtualization capabilities enabled in the chip and
> +BIOS you will see:
> +
> +
>
>  
>x86_64

> +The first block (in red) indicates the host hardware capabilities, 
> such
> +as CPU properties and the power management features of the host platform.
> +CPU models are shown as additional features relative to the closest base
> +model, within a feature block (the block is similar to what you will find
> +in a Xen fully virtualized domain description). Further, the power
> +management features supported by the host are shown, such as 
> Suspend-to-RAM
> +(S3), Suspend-to-Disk (S4) and Hybrid-Suspend (a combination of S3 and 
> S4).
> +In case the host does not support any such feature, then an empty
> + tag will be shown. 
>  
> +The second block (in blue) indicates the paravirtualization support of
> +the Xen support, you will see the os_type of xen to indicate a 
> paravirtual
> +kernel, then architecture information and potential features.
> +
> +The third block (in green) gives similar information but when running 
> a
> +32 bit OS fully virtualized with Xen using the hvm support.
> +
> +This section is likely to be updated and augmented in the future, see 
>  +
> href="https://www.redhat.com/archives/libvir-list/2007-March/msg00215.html";>the
> +discussion which led to the capabilities format in the 
> mailing-list
> +archives.

Do we still want to link to a message that old?  I'd rather cover the
information in the docs as a self-sufficient page instead of linking to
7-year old threads.

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



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

Re: [libvirt] [PATCH v2] maint: prohibit empty first lines

[Michal Privoznik]

On 04.06.2014 15:01, Martin Kletzander wrote:

Based on discussion with Eric:

https://www.redhat.com/archives/libvir-list/2014-March/msg01001.html

Signed-off-by: Martin Kletzander 
---
v2:
  - rebase on current master and s/FILENAME:1:/FILENAME ":1:"/

  cfg.mk | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/cfg.mk b/cfg.mk
index 675af21..80440b5 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -938,6 +938,13 @@ sc_require_locale_h:
fi; 
\
done;

+sc_prohibit_empty_first_line:
+   @awk 'BEGIN { fail=0; }\
+   FNR == 1 { if ($$0 == "") { print FILENAME ":1:"; fail=1; } } \
+   END { if (fail == 1) { \
+ print "$(ME): Prohibited empty first line" > "/dev/stderr";  \
+   } exit fail; }' $$($(VC_LIST_EXCEPT) | grep '\.[ch]$$');


Maybe we can check the whole VC_LIST, there are only a few files that 
start with an empty line (typically README.txt) and those can have an 
exception. But there are some that shouldn't start with an empty line, e.g.:

docs/generic.css:1:
src/locking/lockd.conf:1:
src/locking/sanlock.conf:1:

and some testdata under 
tests/nodeinfodata/linux-test3/node/node*/meminfo. These in particular 
should be plain copy of their origins in sysfs which at least on my 
system doesn't start with an empty line:


# head -n1 /sys/devices/system/node/node0/meminfo
Node 0 MemTotal:4004132 kB

But your version is good too. ACK whether you decide to change it the 
way I'm suggesting or not.


Michal

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

Re: [libvirt] [PATCH RFC] Add support for QEMU vhost-user feature

[Daniel P. Berrange]

On Wed, May 28, 2014 at 10:46:27AM +0200, Luke Gorrie wrote:
> vhost-user is a networking backend based on unix domain sockets
> instead of tap devices and ioctl(). This makes it possible for
> userspace networking stacks (vswitches) to provide vhost-networking to
> guests.
> 
> Signed-off-by: Luke Gorrie 

> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 8f17c74..b5ea2f6 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -791,6 +791,7 @@ struct _virDomainFSDef {
>  enum virDomainNetType {
>  VIR_DOMAIN_NET_TYPE_USER,
>  VIR_DOMAIN_NET_TYPE_ETHERNET,
> +VIR_DOMAIN_NET_TYPE_VHOSTUSER,
>  VIR_DOMAIN_NET_TYPE_SERVER,
>  VIR_DOMAIN_NET_TYPE_CLIENT,
>  VIR_DOMAIN_NET_TYPE_MCAST,
> @@ -877,6 +878,10 @@ struct _virDomainNetDef {
>  char *ipaddr;
>  } ethernet;
>  struct {
> +char *socket;
> +char *mode;
> +} vhostuser;
> +struct {

So your QEMU command line generation code shows that QEMU
is actually allowing any chardev here, not restricted to
a UNIX socket. So you should be using a

  virDomainChrSourceDefPtr chardev;

instead of just hardcoding a UNIX socket path + mode.

> +
> +virCommandAddArgList(cmd, "-chardev", virBufferContentAndReset(&buf1),
> + NULL);
> +virCommandAddArgList(cmd, "-netdev", virBufferContentAndReset(&buf2),
> + NULL);

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

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

Re: [libvirt] [PATCH RFC] Add support for QEMU vhost-user feature

[Daniel P. Berrange]

On Wed, May 28, 2014 at 10:46:27AM +0200, Luke Gorrie wrote:
> vhost-user is a networking backend based on unix domain sockets
> instead of tap devices and ioctl(). This makes it possible for
> userspace networking stacks (vswitches) to provide vhost-networking to
> guests.
> 
> Signed-off-by: Luke Gorrie 
> ---
>  docs/schemas/domaincommon.rng  | 23 

Also need to update docs/formatdomain.html.in to describe the
new XML syntax

>  src/conf/domain_conf.c | 42 ++
>  src/conf/domain_conf.h |  5 ++
>  src/lxc/lxc_process.c  |  1 +
>  src/qemu/qemu_command.c| 66 
> +-
>  src/uml/uml_conf.c |  5 ++
>  src/xenxs/xen_sxpr.c   |  1 +
>  tests/qemuxml2argvdata/qemuxml2argv-hugepages.args |  2 +-
>  8 files changed, 142 insertions(+), 3 deletions(-)

You need to introduce a new data file in tests/qemuxml2argvdata/
and add it to tests/qemuxml2argvtest.c and tests/qemuxml2xmltest.c
to validate correct XML parsing, formatting and QEMU CLI arg
generation for the new NIC type.

> @@ -6804,6 +6817,25 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>  }
>  break;
>  
> +case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
> +if (vhostuser_socket == NULL) {
> +  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("No  'path' attribute "

_( should be aligned with the 'VIR_' on the line above - it indented
2 more spaces.

> + "specified with  type='vhostuser'/>"));
> +  goto error;

Indentation is wonky for the goto.

> +}
> +if (vhostuser_mode == NULL) {
> +  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("No  'mode' attribute "

And here too

> + "specified with  type='vhostuser'/>"));
> +  goto error;

And here too

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1d5bce6..755bf9d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7007,6 +7007,62 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr 
> cfg,
>  }
>  
>  static int
> +qemuBuildVhostuserCommandLine(virCommandPtr cmd,
> +  virDomainDefPtr def,
> +  virDomainNetDefPtr net,
> +  virQEMUCapsPtr qemuCaps)
> +{
> +virBuffer buf1 = VIR_BUFFER_INITIALIZER;
> +virBuffer buf2 = VIR_BUFFER_INITIALIZER;
> +
> +char* nic = NULL;
> +
> +if (!qemuDomainSupportsNetdev(def, qemuCaps, net)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   "%s", _("Netdev support unavailable"));
> +goto error;
> +}
> +
> +if (!qemuDomainSupportsNicdev(def, qemuCaps, net)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   "%s", _("Nicdev support unavailable"));
> +goto error;
> +}
> +
> +virBufferAsprintf(&buf1, "socket,id=char%s,path=%s%s",
> +   net->info.alias, net->data.vhostuser.socket,
> +   STRCASEEQ(net->data.vhostuser.mode, "server") ? ",server" : "");

Again these trailing lines should be aligned with the '&buf1' - eg as
is done in the line below:

> +virBufferAsprintf(&buf2, "type=vhost-user,id=host%s,chardev=char%s",
> +  net->info.alias, net->info.alias);
> +
> +if (virBufferError(&buf1) || virBufferError(&buf2)) {
> +virReportOOMError();
> +goto error;
> +}
> +
> +virCommandAddArgList(cmd, "-chardev", virBufferContentAndReset(&buf1),
> + NULL);
> +virCommandAddArgList(cmd, "-netdev", virBufferContentAndReset(&buf2),
> + NULL);
> +
> +if (!(nic = qemuBuildNicDevStr(def, net, -1, 0, 0, qemuCaps))) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   "%s", _("Error generating NIC -device string"));
> +goto error;
> +}
> +
> +virCommandAddArgList(cmd, "-device", nic, NULL);
> +
> +return 0;
> +
> +error:
> +virBufferFreeAndReset(&buf1);
> +virBufferFreeAndReset(&buf2);
> +
> +return -1;
> +}
> +
> +static int
>  qemuBuildInterfaceCommandLine(virCommandPtr cmd,
>virQEMUDriverPtr driver,
>virConnectPtr conn,
> @@ -7029,6 +7085,10 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
>  int actualType = virDomainNetGetActualType(net);
>  size_t i;
>  
> +if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
> +return qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps);
> +}

No need for '{...}' on single line if()s

> +
>  if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
>  /* NET_TYPE_HOSTDEV devices are really hostdev devices, so
>   * their commandlines are

[libvirt] [PATCH 2/2] formatcaps: Rework and add stubs to document

[Michal Privoznik]

At the moment we are missing even basic documentation on our
capabilities XML. Without demand on completeness, I'm
reorganizing the document structure and adding very basic
documentation to two major components of the capabilities XML.
These stubs are intended to be enhanced in the future.

Signed-off-by: Michal Privoznik 
---
 docs/formatcaps.html.in | 158 
 1 file changed, 121 insertions(+), 37 deletions(-)

diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
index d060a5b..b423d0c 100644
--- a/docs/formatcaps.html.in
+++ b/docs/formatcaps.html.in
@@ -4,19 +4,107 @@
   
 Driver capabilities XML format
 
-As new virtualization engine support gets added to libvirt, and to 
handle
-cases like QEmu supporting a variety of emulations, a query interface has
-been added in 0.2.1 allowing to list the set of supported virtualization
-capabilities on the host:
-char * virConnectGetCapabilities (virConnectPtr conn);
-The value returned is an XML document listing the virtualization
-capabilities of the host and virtualization engine to which
-@conn is connected. One can test it using virsh
-command line tool command 'capabilities', it dumps the XML
-associated to the current connection. For example in the case of a 64 bits
-machine with hardware virtualization capabilities enabled in the chip and
-BIOS you will see
-
+
+
+Element and attribute overview
+
+As new virtualization engine support gets added to libvirt, and to
+handle cases like QEMU supporting a variety of emulations, a query
+interface has been added in 0.2.1 allowing to list the set of supported
+virtualization capabilities on the host:
+
+char * virConnectGetCapabilities (virConnectPtr conn);
+
+The value returned is an XML document listing the virtualization
+capabilities of the host and virtualization engine to which
+@conn is connected. One can test it using virsh
+command line tool command 'capabilities', it dumps the XML
+associated to the current connection. 
+
+As can be seen seen in the example, the
+capabilities XML consists of the capabilities element which
+have exactly one host child element to report information on
+host capabilities, and zero or more guest element to express
+the set of architectures the host can run at the moment.
+
+
+Host capabilities
+
+The  element consists of the following child
+elements:
+
+  uuid
+  The host UUID.
+
+  cpu
+  The host CPU architecture and features.
+
+  power_management
+  whether host is capable of memory suspend, disk hibernation, or
+  hybrid suspend.
+
+  migration
+  This element expose information on hypervisor's migration
+  capabilities, like live migration, supported URI transports, and so
+  on.
+
+  topology
+  This element embodies the host internal topology. Management
+  applications may want to learn this information when orchestrating new
+  guests - e.g. due to reduce inter-NUMA node transfers.
+
+  secmodel
+  To find out default security labels for different security models you
+  need to parse this element. In contrast with the former elements, this is
+  be repeated for each security model the libvirt daemon currently
+  supports.
+
+
+
+Guest capabilities
+
+While the previous section aims at host
+capabilities, this one focus on domain's ones. The
+ element will typically wrap up the following
+elements:
+
+
+os_type
+This express what kind of operating system is the hypervisor able
+to run. Possible values are:
+
+  xen
+  for XEN
+
+  linux
+  legacy alias for xen
+
+  hvm
+  Unmodified operating system
+
+  exe
+  Container based virtualization
+
+  uml
+  User Mode Linux
+
+
+
+arch
+This element brings some information on supported guest 
architecture.
+
+features
+This optional element encases possible features that can be used
+with a guest of described type.
+
+
+Examples
+
+For example in the case of a 64 bits
+machine with hardware virtualization capabilities enabled in the chip and
+BIOS you will see:
+
+
   
 
   x86_64
@@ -67,30 +155,26 @@ BIOS you will see
   
   ...
 
-The first block (in red) indicates the host hardware
-  capabilities, such as CPU properties and the power
-  management features of the host platform.  CPU models are
-  shown as additional features relative to the closest base
-  model, within a feature block (the block is similar to what
-  you will find in a Xen fully virtualized domain
-  description). Further

[libvirt] [PATCH 0/2] Couple of virCaps improvements

[Michal Privoznik]

Literally couple.

As usual, the aim is not completeness, but to prepare environment
for future work (if somebody is willing to contribute patches).

Michal Privoznik (2):
  vircapstest: Introduce basic testing
  formatcaps: Rework and add stubs to document

 docs/formatcaps.html.in | 158 +++-
 tests/vircapsdata/vircaps-basic.xml |  66 +++
 tests/vircapstest.c |  77 +-
 3 files changed, 262 insertions(+), 39 deletions(-)
 create mode 100644 tests/vircapsdata/vircaps-basic.xml

-- 
2.0.0

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

[libvirt] [PATCH 1/2] vircapstest: Introduce basic testing

[Michal Privoznik]

For now only one test is introduced. It's purpose in life
is to check we don't break NUMA host distances XML format.

Signed-off-by: Michal Privoznik 
---
 tests/vircapsdata/vircaps-basic.xml | 66 +++
 tests/vircapstest.c | 77 -
 2 files changed, 141 insertions(+), 2 deletions(-)
 create mode 100644 tests/vircapsdata/vircaps-basic.xml

diff --git a/tests/vircapsdata/vircaps-basic.xml 
b/tests/vircapsdata/vircaps-basic.xml
new file mode 100644
index 000..c438c0d
--- /dev/null
+++ b/tests/vircapsdata/vircaps-basic.xml
@@ -0,0 +1,66 @@
+
+
+  
+
+  x86_64
+
+
+
+  
+
+  2097152
+  
+
+
+
+
+  
+  
+
+
+  
+
+
+  2097152
+  
+
+
+
+
+  
+  
+
+
+  
+
+
+  2097152
+  
+
+
+
+
+  
+  
+
+
+  
+
+
+  2097152
+  
+
+
+
+
+  
+  
+
+
+  
+
+  
+
+  
+
+
diff --git a/tests/vircapstest.c b/tests/vircapstest.c
index 3edebba..64a9980 100644
--- a/tests/vircapstest.c
+++ b/tests/vircapstest.c
@@ -41,8 +41,10 @@ buildNUMATopology(int seq)
 {
 virCapsPtr caps;
 virCapsHostNUMACellCPUPtr cell_cpus = NULL;
-int core_id, cell_id;
+virCapsHostNUMACellSiblingInfoPtr siblings = NULL;
+int core_id, cell_id, nsiblings;
 int id;
+size_t i;
 
 if ((caps = virCapabilitiesNew(VIR_ARCH_X86_64, 0, 0)) == NULL)
 goto error;
@@ -63,13 +65,25 @@ buildNUMATopology(int seq)
 }
 id++;
 
+if (VIR_ALLOC_N(siblings, MAX_CELLS) < 0)
+goto error;
+nsiblings = MAX_CELLS;
+
+for (i = 0; i < nsiblings; i++) {
+siblings[i].node = i;
+/* Some magical constants, see virNumaGetDistances()
+ * for their description. */
+siblings[i].distance = cell_id == i ? 10 : 20;
+}
+
 if (virCapabilitiesAddHostNUMACell(caps, cell_id + seq,
MAX_MEM_IN_CELL,
MAX_CPUS_IN_CELL, cell_cpus,
-   0, NULL) < 0)
+   nsiblings, siblings) < 0)
goto error;
 
 cell_cpus = NULL;
+siblings = NULL;
 }
 
 return caps;
@@ -77,6 +91,7 @@ buildNUMATopology(int seq)
  error:
 virCapabilitiesClearHostNUMACellCPUTopology(cell_cpus, MAX_CPUS_IN_CELL);
 VIR_FREE(cell_cpus);
+VIR_FREE(siblings);
 virObjectUnref(caps);
 return NULL;
 
@@ -117,6 +132,55 @@ test_virCapabilitiesGetCpusForNodemask(const void *data 
ATTRIBUTE_UNUSED)
 }
 
 
+struct virCapabilitiesFormatData {
+const char *filename;
+int seq;
+};
+
+static int
+test_virCapabilitiesFormat(const void *opaque)
+{
+struct virCapabilitiesFormatData *data = (struct virCapabilitiesFormatData 
*) opaque;
+virCapsPtr caps = NULL;
+char *capsXML = NULL;
+char *capsFromFile = NULL;
+char *path = NULL;
+int ret = -1;
+
+/*
+ * Build a NUMA topology with cell_id (NUMA node id
+ * being 3(0 + 3),4(1 + 3), 5 and 6
+ */
+if (!(caps = buildNUMATopology(0)))
+goto cleanup;
+
+if (!(capsXML = virCapabilitiesFormatXML(caps))) {
+fprintf(stderr, "Unable to format capabilities XML");
+goto cleanup;
+}
+
+if (virAsprintf(&path, "%s/vircapsdata/vircaps-%s.xml",
+abs_srcdir, data->filename) < 0)
+goto cleanup;
+
+if (virFileReadAll(path, 8192, &capsFromFile) < 0)
+goto cleanup;
+
+if (STRNEQ(capsXML, capsFromFile)) {
+virtTestDifference(stderr, capsXML, capsFromFile);
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(path);
+VIR_FREE(capsFromFile);
+VIR_FREE(capsXML);
+virObjectUnref(caps);
+return ret;
+}
+
 static int
 mymain(void)
 {
@@ -126,6 +190,15 @@ mymain(void)
 test_virCapabilitiesGetCpusForNodemask, NULL) < 0)
 ret = -1;
 
+#define DO_TEST(filename, seq)  \
+do {\
+struct virCapabilitiesFormatData data = {filename, seq};\
+if (virtTestRun(filename, test_virCapabilitiesFormat, &data) < 0)   \
+ret = -1;   \
+} while (0)
+
+DO_TEST("basic", 0);
+
 return ret;
 }
 
-- 
2.0.0

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

Re: [libvirt] [PATCH 6/6] qemu: pass numa node binding preferences to qemu

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 04:56:32PM +0200, Martin Kletzander wrote:
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index b1bfb5a..6415a6d 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -930,8 +932,15 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
>  goto cleanup;
>  }
> 
> +if (virDomainGetMemsForGuestCpu(def, i, &mem_mask) < 0)
> +goto cleanup;
> +
>  /* Set vcpupin in cgroup if vcpupin xml is provided */
>  if (virCgroupHasController(priv->cgroup, 
> VIR_CGROUP_CONTROLLER_CPUSET)) {
> +
> +if (mem_mask && virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 
> 0)
> +goto cleanup;
> +

I'm still pretty unclear on whether we want todo this or not.

IMHO the  is primarily about making guest NUMA nodes
be allocated from specific host NUMA nodes. 

I'm not sure that arbitrary allocations done by the QEMU vCPU
threads should be bound the same way or not. Be nice to have
explicit confirmation from someone knowing KVM to say that we
should force vCPU threads to allocate from the same NUMA
nodes as the guest NUMA the vCPU is in.

>  /* find the right CPU to pin, otherwise
>   * qemuSetupCgroupVcpuPin will fail. */
>  for (j = 0; j < def->cputune.nvcpupin; j++) {
> @@ -946,14 +955,21 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
> 
>  break;
>  }
> +} else if (mem_mask) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("cgroup cpuset is required for "
> + "per-node memory binding"));
> +goto cleanup;
>  }
> 
> +VIR_FREE(mem_mask);
>  virCgroupFree(&cgroup_vcpu);
>  }
> 
>  return 0;
> 
>   cleanup:
> +VIR_FREE(mem_mask);
>  if (cgroup_vcpu) {
>  virCgroupRemove(cgroup_vcpu);
>  virCgroupFree(&cgroup_vcpu);
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index cc6023d..8699eea 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6531,21 +6531,99 @@ qemuBuildSmpArgStr(const virDomainDef *def,
>  return virBufferContentAndReset(&buf);
>  }
> 
> +static const char *
> +qemuNumaPolicyToString(virDomainNumatuneMemMode mode)
> +{
> +const char *policy = NULL;
> +
> +switch (mode) {
> +case VIR_DOMAIN_NUMATUNE_MEM_STRICT:
> +policy = "bind";
> +break;
> +
> +case VIR_DOMAIN_NUMATUNE_MEM_PREFERRED:
> +policy = "preferred";
> +break;
> +
> +case VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE:
> +policy = "interleave";
> +break;
> +
> +case VIR_DOMAIN_NUMATUNE_MEM_LAST:
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("Cannot translate memory policy"));
> +break;
> +}
> +
> +return policy;
> +}

Looks like a candidate for qemu local VIR_ENUM

> +
>  static int
> -qemuBuildNumaArgStr(const virDomainDef *def, virCommandPtr cmd)
> +qemuBuildNumaArgStr(const virDomainDef *def,
> +virCommandPtr cmd,
> +virQEMUDriverPtr driver,
> +virQEMUCapsPtr qemuCaps,
> +virBitmapPtr autoNodemask)
>  {
> +int ret = -1;
>  size_t i;
> -virBuffer buf = VIR_BUFFER_INITIALIZER;
>  char *cpumask = NULL;
> -int ret = -1;
> +virCapsPtr caps = virQEMUDriverGetCapabilities(driver, false);
> +virBuffer buf = VIR_BUFFER_INITIALIZER;
> +char *nodemask = NULL;
> +char *globalNodemask = NULL;
> +const char *globalPolicy = 
> qemuNumaPolicyToString(def->numatune.memory.mode);
> +
> +if (!globalPolicy)
> +goto cleanup;
> +

> 
>  for (i = 0; i < def->cpu->ncells; i++) {
> -VIR_FREE(cpumask);
> +const char *policy = NULL;
>  int cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024);
>  def->cpu->cells[i].mem = cellmem * 1024;
> 
> -if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask)))
> +VIR_FREE(nodemask);
> +if (def->numatune.mem_nodes &&
> +def->numatune.mem_nodes[i].specified) {
> +policy = qemuNumaPolicyToString(def->numatune.mem_nodes[i].mode);
> +if (!policy)
> +goto cleanup;
> +
> +nodemask = virBitmapFormat(def->numatune.mem_nodes[i].nodemask);
> +if (!nodemask) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("Unable to format nodemask"));
> +goto cleanup;
> +}
> +}
> +
> +VIR_FREE(cpumask);
> +
> +if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask))) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("Unable to format nodemask"));
>  goto cleanup;
> +}
> 
>  if (strchr(cpumask, ',')) {
>  virReportE

Re: [libvirt] [PATCH 2/6] conf, schema: add support for numatune memnode element

[Martin Kletzander]

On Wed, Jun 04, 2014 at 04:07:06PM +0100, Daniel P. Berrange wrote:

On Wed, Jun 04, 2014 at 04:56:28PM +0200, Martin Kletzander wrote:

@@ -43,9 +43,17 @@ typedef virNumaTuneDef *virNumaTuneDefPtr;
 struct _virNumaTuneDef {
 struct {
 virBitmapPtr nodemask;
-int mode;
+int mode;   /* enum virDomainNumatuneMemMode */
 int placement_mode; /* enum virNumaTuneMemPlacementMode */
-} memory;
+} memory;   /* pinning for all the memory */
+
+struct mem_node {


Declaring structs inline without typedefs isn't our usual style. There
should be a typedef for virNumaTuneMemNodeDef & virNumaTuneMemNodeDefPtr



Yeah, it should.  Also the numatunedef should be in some conf/ file
with encapsulated insides.  Parsing should be in that file too, of
course.

I took the liberty of proposing it this way and re-factoring
afterwards since all those details were drafting me away of the thing
I wanted to make out of that.  However, I forgot to mention that the
same way I forgot to mention that virGetNumaNodeParameters() API will
follow even though is not part of this patch.

Martin


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

Re: [libvirt] Problem connecting to hypervisor.

[Jim Fehlig]

Vikas Kokare wrote:
> I am trying to establish a connection (using virsh command) to a
> couple of hypervisor hosts, both on SLES 11.3 environments. The host
> from where i am making both the connections is a SLES 10.4 instance.

Hi Vikas,

FYI, this list is for discussing upstream libvirt development, not a
forum for distro support.  Since you are using SLES, it would be best to
enter a support ticket with SUSE.

>
> /kvmh911351246:~ # virsh -c qemu+ssh://9.113.51.247:22/system
> 
> libvir: XML-RPC error : *authentication required*
> error: failed to connect to the hypervisor/
> /
> /
> The SSH authentication to hypervisor host 9.113.51.247 has already
> been established.
>
> /kvmh911351246:~ # *ssh root@9.113.51.247 *
> Last login: Tue Jun  3 11:31:56 2014 from scm-kvmh2-rhel6
> SLES11-51-247:~ #/
> /
> /
> Hence the cause of the authentication failure is not clear.
>
> The second connection to another SLES 11.3 host looks like
> /
> virsh # connect qemu+ssh://9.121.58.19:22/system
> 
> *bash: socat: command not found
> libvir: Remote error : socket closed unexpectedly
> *error: Failed to connect to the hypervisor/

You could try installing socat on the SLES11 SP3 systems.

>
> where the cause of the error is entirely different.
>
> The libvirt packages available on these environments are
>
> *9.113.51.247 (SLES 11.3)*
> /libvirt-cim-0.5.12-0.7.16
> libvirt-1.0.5.1-0.7.10
> libvirt-python-1.0.5.1-0.7.10
> libvirt-client-32bit-1.0.5.1-0.7.10
> libvirt-lock-sanlock-1.0.5.1-0.7.10
> libvirt-client-1.0.5.1-0.7.10/
>
> *9.121.58.19 (SLES 11.3)*
> /libvirt-cim-0.5.12-0.7.16
> libvirt-client-1.0.5.1-0.7.10
> libvirt-python-1.0.5.1-0.7.10
> libvirt-1.0.5.1-0.7.10/

The libvirt packages on your SLES11 SP3 machines are quite old and need
updated.

Regards,
Jim

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

Re: [libvirt] [PATCH 5/6] qemu: memory-ram capability probing

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 04:56:31PM +0200, Martin Kletzander wrote:
> The numa patch series in qemu adds "memory-ram" object type by which
> we can tell whether we can use such objects.
> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/qemu/qemu_capabilities.c | 2 ++
>  src/qemu/qemu_capabilities.h | 1 +
>  2 files changed, 3 insertions(+)

ACK


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

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

Re: [libvirt] [PATCH 4/6] qemu: purely a code movement

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 04:56:30PM +0200, Martin Kletzander wrote:
> to ease the review of commits to follow.
> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/qemu/qemu_command.c | 25 ++---
>  1 file changed, 14 insertions(+), 11 deletions(-)

ACK


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

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

Re: [libvirt] [PATCH 3/6] conf: add virDomainGetMemsForGuestCpu()

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 04:56:29PM +0200, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander 
> ---
>  src/conf/domain_conf.c   | 39 +++
>  src/conf/domain_conf.h   |  4 
>  src/libvirt_private.syms |  1 +
>  3 files changed, 44 insertions(+)

ACK


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

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

[libvirt] [PATCH RFC] network: Bring netdevs online later

[Matthew Rosato]

Defer MAC registration until net devices are actually going
to be used by the guest.  This patch does so by setting the
devices online just before starting guest CPUs.

This approach is an alternative to my previously proposed
'network: Defer online of macvtap during qemu migration'
Laine/Wangrui, is this the sort of thing you had in mind?

Previous thread:
https://www.redhat.com/archives/libvir-list/2014-May/msg00427.html
Associated BZ:
https://bugzilla.redhat.com/show_bug.cgi?id=1081461

Signed-off-by: Matthew Rosato 
---
 src/qemu/qemu_command.c |   45 +++
 src/qemu/qemu_command.h |3 +++
 src/qemu/qemu_process.c |3 +++
 src/util/virnetdevmacvlan.c |5 -
 src/util/virnetdevtap.c |3 ---
 5 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e6acced..c161d73 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -571,6 +571,51 @@ qemuNetworkPrepareDevices(virDomainDefPtr def)
 return ret;
 }
 
+void
+qemuNetworkIfaceUp(virDomainNetDefPtr net)
+{
+if (virNetDevSetOnline(net->ifname, true) < 0) {
+ignore_value(virNetDevTapDelete(net->ifname));
+}
+return;
+}
+
+void
+qemuPhysIfaceUp(virDomainNetDefPtr net)
+{
+if (virNetDevSetOnline(net->ifname, true) < 0) {
+ignore_value(virNetDevVPortProfileDisassociate(net->ifname,
+   
virDomainNetGetActualVirtPortProfile(net),
+   &net->mac,
+   
virDomainNetGetActualDirectDev(net),
+   -1,
+   
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH));
+ignore_value(virNetDevMacVLanDelete(net->ifname));
+}
+return;
+}
+
+void
+qemuNetworkInitializeDevices(virDomainDefPtr def)
+{
+size_t i;
+
+for (i = 0; i < def->nnets; i++) {
+virDomainNetDefPtr net = def->nets[i];
+switch(virDomainNetGetActualType(net)) {
+case VIR_DOMAIN_NET_TYPE_BRIDGE:
+case VIR_DOMAIN_NET_TYPE_NETWORK:
+qemuNetworkIfaceUp(net);
+break;
+case VIR_DOMAIN_NET_TYPE_DIRECT:
+qemuPhysIfaceUp(net);
+break;
+}
+}
+
+return;
+}
+
 static int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info,
   const char *prefix)
 {
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index afbd6ff..4a44464 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -206,6 +206,9 @@ int qemuOpenVhostNet(virDomainDefPtr def,
  int *vhostfdSize);
 
 int qemuNetworkPrepareDevices(virDomainDefPtr def);
+void qemuNetworkIfaceUp(virDomainNetDefPtr net);
+void qemuPhysIfaceUp(virDomainNetDefPtr net);
+void qemuNetworkInitializeDevices(virDomainDefPtr def);
 
 /*
  * NB: def->name can be NULL upon return and the caller
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d719716..bbc11f3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2765,6 +2765,9 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver, 
virDomainObjPtr vm,
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 
+/* Bring up netdevs before starting CPUs */
+qemuNetworkInitializeDevices(vm->def);
+
 VIR_DEBUG("Using lock state '%s'", NULLSTR(priv->lockState));
 if (virDomainLockProcessResume(driver->lockManager, cfg->uri,
vm, priv->lockState) < 0) {
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index cb85b74..3748527 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -898,11 +898,6 @@ int virNetDevMacVLanCreateWithVPortProfile(const char 
*tgifname,
 goto link_del_exit;
 }
 
-if (virNetDevSetOnline(cr_ifname, true) < 0) {
-rc = -1;
-goto disassociate_exit;
-}
-
 if (withTap) {
 if ((rc = virNetDevMacVLanTapOpen(cr_ifname, 10)) < 0)
 goto disassociate_exit;
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 0b444fa..09b9c12 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -574,9 +574,6 @@ int virNetDevTapCreateInBridgePort(const char *brname,
 goto error;
 }
 
-if (virNetDevSetOnline(*ifname, !!(flags & VIR_NETDEV_TAP_CREATE_IFUP)) < 
0)
-goto error;
-
 return 0;
 
  error:
-- 
1.7.9.5

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

Re: [libvirt] [PATCH 2/6] conf, schema: add support for numatune memnode element

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 04:56:28PM +0200, Martin Kletzander wrote:
> This element specifies similar settings as the memory element,
> although memnode can be used per guest NUMA node.
> 
> Signed-off-by: Martin Kletzander 
> ---
>  docs/formatdomain.html.in |  15 +++
>  docs/schemas/domaincommon.rng |  17 
>  src/conf/domain_conf.c| 220 
> +++---
>  src/qemu/qemu_domain.c|  23 -
>  src/qemu/qemu_driver.c|  11 +++
>  src/util/virnuma.h|  14 ++-
>  6 files changed, 260 insertions(+), 40 deletions(-)

The new elements should be added to the 
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml
files you added in the previous commit too.

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

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

Re: [libvirt] [PATCH 1/6] conf, schema: add 'id' field for cells

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 04:56:27PM +0200, Martin Kletzander wrote:
> In XML format, by definition, order of fields should not matter, so
> oder of parsing the elements doesn't affect the end result.  When
> specifying guest NUMA cells, we depend only on the order of the 'cell'
> elements.  With this patch all older domain XMLs are parsed as before,
> but with the 'id' attribute they are parsed and formatted according to
> that field.  This will be useful when we have tuning settings for
> particular guest NUMA node.
> 
> Signed-off-by: Martin Kletzander 
> ---
>  docs/formatdomain.html.in  | 11 +++---
>  docs/schemas/domaincommon.rng  |  5 +++
>  src/conf/cpu_conf.c| 39 
> +++---
>  tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml  |  6 ++--
>  tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml  |  6 ++--
>  tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml  | 25 ++
>  tests/qemuxml2argvtest.c   |  1 +
>  .../qemuxml2xmlout-cpu-numa1.xml   | 28 
>  .../qemuxml2xmlout-cpu-numa2.xml   | 28 
>  tests/qemuxml2xmltest.c|  3 ++
>  10 files changed, 138 insertions(+), 14 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml
>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml

ACK

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

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

Re: [libvirt] [PATCH 2/6] conf, schema: add support for numatune memnode element

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 04:56:28PM +0200, Martin Kletzander wrote:
> @@ -43,9 +43,17 @@ typedef virNumaTuneDef *virNumaTuneDefPtr;
>  struct _virNumaTuneDef {
>  struct {
>  virBitmapPtr nodemask;
> -int mode;
> +int mode;   /* enum virDomainNumatuneMemMode */
>  int placement_mode; /* enum virNumaTuneMemPlacementMode */
> -} memory;
> +} memory;   /* pinning for all the memory */
> +
> +struct mem_node {

Declaring structs inline without typedefs isn't our usual style. There
should be a typedef for virNumaTuneMemNodeDef & virNumaTuneMemNodeDefPtr

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

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

[libvirt] [PATCH 5/6] qemu: memory-ram capability probing

[Martin Kletzander]

The numa patch series in qemu adds "memory-ram" object type by which
we can tell whether we can use such objects.

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 08c3d04..a479035 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -256,6 +256,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "usb-kbd", /* 165 */
   "host-pci-multidomain",
   "msg-timestamp",
+  "memory-ram",
 );


@@ -1440,6 +1441,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { "ich9-intel-hda", QEMU_CAPS_DEVICE_ICH9_INTEL_HDA },
 { "pvpanic", QEMU_CAPS_DEVICE_PANIC },
 { "usb-kbd", QEMU_CAPS_DEVICE_USB_KBD },
+{ "memory-ram", QEMU_CAPS_OBJECT_MEMORY_RAM },
 };

 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d755caa..350c43e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -206,6 +206,7 @@ enum virQEMUCapsFlags {
 QEMU_CAPS_DEVICE_USB_KBD = 165, /* -device usb-kbd */
 QEMU_CAPS_HOST_PCI_MULTIDOMAIN = 166, /* support domain > 0 in host pci 
address */
 QEMU_CAPS_MSG_TIMESTAMP  = 167, /* -msg timestamp */
+QEMU_CAPS_OBJECT_MEMORY_RAM  = 168, /* -object memory-ram */

 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
-- 
2.0.0

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

[libvirt] [PATCH 0/6] Support for per-guest-node binding

[Martin Kletzander]

Currently we are only able to bind the whole domain to some host nodes
using the /domain/numatune/memory element.  Numerous requests were
made to support host<->guest numa node bindings, so this series tries
to pinch an idea on how to do that using /domain/numatune/memnode
elements.

That is incompatible with automatic numa placement (numad) since that
makes no sense.  Also this disables any live changes to numa
parameters (the /domain/numatune/memory settings) since we cannot
change the settings given to qemu.

Martin Kletzander (6):
  conf, schema: add 'id' field for cells
  conf, schema: add support for numatune memnode element
  conf: add virDomainGetMemsForGuestCpu()
  qemu: purely a code movement
  qemu: memory-ram capability probing
  qemu: pass numa node binding preferences to qemu

 docs/formatdomain.html.in  |  26 ++-
 docs/schemas/domaincommon.rng  |  22 ++
 src/conf/cpu_conf.c|  39 +++-
 src/conf/domain_conf.c | 259 ++---
 src/conf/domain_conf.h |   4 +
 src/libvirt_private.syms   |   1 +
 src/qemu/qemu_capabilities.c   |   2 +
 src/qemu/qemu_capabilities.h   |   1 +
 src/qemu/qemu_cgroup.c |  18 +-
 src/qemu/qemu_command.c| 160 +++--
 src/qemu/qemu_command.h|   3 +-
 src/qemu/qemu_domain.c |  23 +-
 src/qemu/qemu_driver.c |  23 +-
 src/qemu/qemu_process.c|   3 +-
 src/util/virnuma.h |  14 +-
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml  |   6 +-
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml  |   6 +-
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml  |  25 ++
 .../qemuxml2argv-numatune-auto-prefer.xml  |  29 +++
 .../qemuxml2argv-numatune-auto.args|   6 +
 .../qemuxml2argv-numatune-auto.xml |  26 +++
 .../qemuxml2argv-numatune-memnode-nocpu.xml|  25 ++
 .../qemuxml2argv-numatune-memnodes-problematic.xml |  31 +++
 .../qemuxml2argv-numatune-memnodes.args|   8 +
 .../qemuxml2argv-numatune-memnodes.xml |  31 +++
 .../qemuxml2argv-numatune-prefer.args  |   6 +
 .../qemuxml2argv-numatune-prefer.xml   |  29 +++
 tests/qemuxml2argvtest.c   |  51 ++--
 .../qemuxml2xmlout-cpu-numa1.xml   |  28 +++
 .../qemuxml2xmlout-cpu-numa2.xml   |  28 +++
 tests/qemuxml2xmltest.c|   4 +
 tests/qemuxmlnstest.c  |   2 +-
 32 files changed, 845 insertions(+), 94 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-prefer.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-prefer.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml

--
2.0.0

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

[libvirt] [PATCH 6/6] qemu: pass numa node binding preferences to qemu

[Martin Kletzander]

Currently, we only bind the whole QEMU domain to memory nodes
specified in nodemask altogether.  That, however, doesn't make much
sense when one wants to control from where the memory for particular
guest nodes should be allocated.  QEMU allows us to do that by
specifying 'host-nodes' parameter for the 'memory-ram' object, so
let's use that.

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_cgroup.c |  18 ++-
 src/qemu/qemu_command.c| 141 +++--
 src/qemu/qemu_command.h|   3 +-
 src/qemu/qemu_driver.c |  12 +-
 src/qemu/qemu_process.c|   3 +-
 .../qemuxml2argv-numatune-auto-prefer.xml  |  29 +
 .../qemuxml2argv-numatune-auto.args|   6 +
 .../qemuxml2argv-numatune-auto.xml |  26 
 .../qemuxml2argv-numatune-memnode-nocpu.xml|  25 
 .../qemuxml2argv-numatune-memnodes-problematic.xml |  31 +
 .../qemuxml2argv-numatune-memnodes.args|   8 ++
 .../qemuxml2argv-numatune-memnodes.xml |  31 +
 .../qemuxml2argv-numatune-prefer.args  |   6 +
 .../qemuxml2argv-numatune-prefer.xml   |  29 +
 tests/qemuxml2argvtest.c   |  50 +---
 tests/qemuxml2xmltest.c|   1 +
 tests/qemuxmlnstest.c  |   2 +-
 17 files changed, 389 insertions(+), 32 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-prefer.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-prefer.xml

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index b1bfb5a..6415a6d 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -23,6 +23,8 @@

 #include 

+#include "domain_audit.h"
+#include "domain_conf.h"
 #include "qemu_cgroup.h"
 #include "qemu_domain.h"
 #include "qemu_process.h"
@@ -30,7 +32,6 @@
 #include "virlog.h"
 #include "viralloc.h"
 #include "virerror.h"
-#include "domain_audit.h"
 #include "virscsi.h"
 #include "virstring.h"
 #include "virfile.h"
@@ -895,6 +896,7 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
 size_t i, j;
 unsigned long long period = vm->def->cputune.period;
 long long quota = vm->def->cputune.quota;
+char *mem_mask = NULL;

 if ((period || quota) &&
 !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
@@ -930,8 +932,15 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
 goto cleanup;
 }

+if (virDomainGetMemsForGuestCpu(def, i, &mem_mask) < 0)
+goto cleanup;
+
 /* Set vcpupin in cgroup if vcpupin xml is provided */
 if (virCgroupHasController(priv->cgroup, 
VIR_CGROUP_CONTROLLER_CPUSET)) {
+
+if (mem_mask && virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 0)
+goto cleanup;
+
 /* find the right CPU to pin, otherwise
  * qemuSetupCgroupVcpuPin will fail. */
 for (j = 0; j < def->cputune.nvcpupin; j++) {
@@ -946,14 +955,21 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)

 break;
 }
+} else if (mem_mask) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("cgroup cpuset is required for "
+ "per-node memory binding"));
+goto cleanup;
 }

+VIR_FREE(mem_mask);
 virCgroupFree(&cgroup_vcpu);
 }

 return 0;

  cleanup:
+VIR_FREE(mem_mask);
 if (cgroup_vcpu) {
 virCgroupRemove(cgroup_vcpu);
 virCgroupFree(&cgroup_vcpu);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cc6023d..8699eea 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6531,21 +6531,99 @@ qemuBuildSmpArgStr(const virDomainDef *def,
 return virBufferContentAndReset(&buf);
 }

+static const char *
+qemuNumaPolicyToString(virDomainNumatuneMemMode mode)
+{
+const char *policy = NULL;
+
+switch (mode) {
+case VIR_DOMAIN_NUMATUNE_MEM_STRICT:
+policy = "bind";
+break;
+
+case VIR_DOMAIN_NUMATUNE_MEM_PREFERRED:
+policy = "preferred";
+break;
+
+case VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE:
+policy = "interleave";
+break;
+
+case VIR_DOMAIN_NUMATUNE_MEM_LAST:
+virReportEr

[libvirt] [PATCH 2/6] conf, schema: add support for numatune memnode element

[Martin Kletzander]

This element specifies similar settings as the memory element,
although memnode can be used per guest NUMA node.

Signed-off-by: Martin Kletzander 
---
 docs/formatdomain.html.in |  15 +++
 docs/schemas/domaincommon.rng |  17 
 src/conf/domain_conf.c| 220 +++---
 src/qemu/qemu_domain.c|  23 -
 src/qemu/qemu_driver.c|  11 +++
 src/util/virnuma.h|  14 ++-
 6 files changed, 260 insertions(+), 40 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 041f70d..2d855ea 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -709,6 +709,8 @@
   ...
   
 
+
+
   
   ...
 
@@ -745,6 +747,19 @@

 Since 0.9.3
   
+  memnode
+  
+Optional memnode elements can specify memory allocation
+policies per each guest NUMA node.  For those nodes having no
+corresponding memnode element, the default from
+element memory will be used.  Attribute 
cellid
+addresses guest NUMA node for which the settings are applied.
+Attributes mode and nodeset have the same
+meaning and syntax as in memory element.
+
+This setting is not compatible with automatic placement.
+QEMU Since 1.2.6
+  
 


diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 0787b5a..a8e3ba0 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -789,6 +789,23 @@
   
 
   
+  
+
+  
+
+  
+  
+
+  strict
+  preferred
+  interleave
+
+  
+  
+
+  
+
+  
 
   

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fe06921..352ba92 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2085,6 +2085,9 @@ void virDomainDefFree(virDomainDefPtr def)
 virDomainVcpuPinDefFree(def->cputune.emulatorpin);

 virBitmapFree(def->numatune.memory.nodemask);
+for (i = 0; i < def->numatune.nmem_nodes; i++)
+virBitmapFree(def->numatune.mem_nodes[i].nodemask);
+VIR_FREE(def->numatune.mem_nodes);

 virSysinfoDefFree(def->sysinfo);

@@ -11232,6 +11235,8 @@ virDomainDefParseXML(xmlDocPtr xml,
 bool usb_other = false;
 bool usb_master = false;
 bool primaryVideo = false;
+bool mem_nodes = false;
+

 if (VIR_ALLOC(def) < 0)
 return NULL;
@@ -11666,6 +11671,33 @@ virDomainDefParseXML(xmlDocPtr xml,
 }
 VIR_FREE(nodes);

+
+/* analysis of cpu handling */
+if ((node = virXPathNode("./cpu[1]", ctxt)) != NULL) {
+xmlNodePtr oldnode = ctxt->node;
+ctxt->node = node;
+def->cpu = virCPUDefParseXML(node, ctxt, VIR_CPU_TYPE_GUEST);
+ctxt->node = oldnode;
+
+if (def->cpu == NULL)
+goto error;
+
+if (def->cpu->sockets &&
+def->maxvcpus >
+def->cpu->sockets * def->cpu->cores * def->cpu->threads) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+   _("Maximum CPUs greater than topology limit"));
+goto error;
+}
+
+if (def->cpu->cells_cpus > def->maxvcpus) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Number of CPUs in  exceeds the"
+ "  count"));
+goto error;
+}
+}
+
 /* Extract numatune if exists. */
 if ((n = virXPathNodeSet("./numatune", ctxt, &nodes)) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -11682,6 +11714,12 @@ virDomainDefParseXML(xmlDocPtr xml,

 if (n) {
 cur = nodes[0]->children;
+if (def->cpu) {
+if (VIR_ALLOC_N(def->numatune.mem_nodes, def->cpu->ncells) < 0)
+goto error;
+def->numatune.nmem_nodes = def->cpu->ncells;
+}
+
 while (cur != NULL) {
 if (cur->type == XML_ELEMENT_NODE) {
 if (xmlStrEqual(cur->name, BAD_CAST "memory")) {
@@ -11764,6 +11802,80 @@ virDomainDefParseXML(xmlDocPtr xml,
 def->placement_mode = 
VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO;

 def->numatune.memory.placement_mode = placement_mode;
+
+} else if (xmlStrEqual(cur->name, BAD_CAST "memnode")) {
+unsigned int cellid;
+struct mem_node *mem_node = NULL;
+
+if (!def->numatune.nmem_nodes) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Element 'memnode' is invalid without 
"
+ "any guest NUMA cells"));
+  

[libvirt] [PATCH 3/6] conf: add virDomainGetMemsForGuestCpu()

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
 src/conf/domain_conf.c   | 39 +++
 src/conf/domain_conf.h   |  4 
 src/libvirt_private.syms |  1 +
 3 files changed, 44 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 352ba92..fef956c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19879,3 +19879,42 @@ virDomainObjSetMetadata(virDomainObjPtr vm,

 return 0;
 }
+
+int
+virDomainGetMemsForGuestCpu(virDomainDefPtr def,
+size_t cpuid,
+char **nodemask)
+{
+ssize_t i = 0;
+
+*nodemask = NULL;
+
+/* nothing to do with no per-node settings */
+if (!def->numatune.nmem_nodes)
+return 0;
+
+/* !!(def->numatune.nmem_nodes) -> !!(def->cpu) */
+for (i = 0; i < def->cpu->ncells; i++) {
+bool result = false;
+
+if (virBitmapGetBit(def->cpu->cells[i].cpumask, cpuid, &result) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Problem accessing domain's "
+ "NUMA cell information"));
+return -1;
+}
+if (!result)
+continue;
+
+if (def->numatune.mem_nodes[i].mode != VIR_DOMAIN_NUMATUNE_MEM_STRICT)
+return 0;
+
+*nodemask = virBitmapFormat(def->numatune.mem_nodes[i].nodemask);
+if (!*nodemask)
+return -1;
+
+return 0;
+}
+
+return 0;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 2de807d..b6e0765 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2711,4 +2711,8 @@ int virDomainObjSetMetadata(virDomainObjPtr vm,
 const char *configDir,
 unsigned int flags);

+int virDomainGetMemsForGuestCpu(virDomainDefPtr def,
+size_t cpuid,
+char **nodemask);
+
 #endif /* __DOMAIN_CONF_H */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d73a9f5..88eae2f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -244,6 +244,7 @@ virDomainFSTypeToString;
 virDomainFSWrpolicyTypeFromString;
 virDomainFSWrpolicyTypeToString;
 virDomainGetFilesystemForTarget;
+virDomainGetMemsForGuestCpu;
 virDomainGraphicsAuthConnectedTypeFromString;
 virDomainGraphicsAuthConnectedTypeToString;
 virDomainGraphicsDefFree;
-- 
2.0.0

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

[libvirt] [PATCH 4/6] qemu: purely a code movement

[Martin Kletzander]

to ease the review of commits to follow.

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_command.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e6acced..cc6023d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6541,11 +6541,22 @@ qemuBuildNumaArgStr(const virDomainDef *def, 
virCommandPtr cmd)

 for (i = 0; i < def->cpu->ncells; i++) {
 VIR_FREE(cpumask);
+int cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024);
+def->cpu->cells[i].mem = cellmem * 1024;
+
+if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask)))
+goto cleanup;
+
+if (strchr(cpumask, ',')) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("disjoint NUMA cpu ranges are not supported "
+ "with this QEMU"));
+goto cleanup;
+}
+
 virCommandAddArg(cmd, "-numa");
 virBufferAsprintf(&buf, "node,nodeid=%d", def->cpu->cells[i].cellid);
 virBufferAddLit(&buf, ",cpus=");
-if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask)))
-goto cleanup;

 /* Up through qemu 1.4, -numa does not accept a cpus
  * argument any more complex than start-stop.
@@ -6553,16 +6564,8 @@ qemuBuildNumaArgStr(const virDomainDef *def, 
virCommandPtr cmd)
  * XXX For qemu 1.5, the syntax has not yet been decided;
  * but when it is, we need a capability bit and
  * translation of our cpumask into the qemu syntax.  */
-if (strchr(cpumask, ',')) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("disjoint NUMA cpu ranges are not supported "
- "with this QEMU"));
-goto cleanup;
-}
 virBufferAdd(&buf, cpumask, -1);
-def->cpu->cells[i].mem = VIR_DIV_UP(def->cpu->cells[i].mem,
-1024) * 1024;
-virBufferAsprintf(&buf, ",mem=%d", def->cpu->cells[i].mem / 1024);
+virBufferAsprintf(&buf, ",mem=%d", cellmem);

 if (virBufferError(&buf)) {
 virReportOOMError();
-- 
2.0.0

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

[libvirt] [PATCH 1/6] conf, schema: add 'id' field for cells

[Martin Kletzander]

In XML format, by definition, order of fields should not matter, so
oder of parsing the elements doesn't affect the end result.  When
specifying guest NUMA cells, we depend only on the order of the 'cell'
elements.  With this patch all older domain XMLs are parsed as before,
but with the 'id' attribute they are parsed and formatted according to
that field.  This will be useful when we have tuning settings for
particular guest NUMA node.

Signed-off-by: Martin Kletzander 
---
 docs/formatdomain.html.in  | 11 +++---
 docs/schemas/domaincommon.rng  |  5 +++
 src/conf/cpu_conf.c| 39 +++---
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml  |  6 ++--
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml  |  6 ++--
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml  | 25 ++
 tests/qemuxml2argvtest.c   |  1 +
 .../qemuxml2xmlout-cpu-numa1.xml   | 28 
 .../qemuxml2xmlout-cpu-numa2.xml   | 28 
 tests/qemuxml2xmltest.c|  3 ++
 10 files changed, 138 insertions(+), 14 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 691a451..041f70d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1030,8 +1030,8 @@
   
 ...
 
-  
-  
+  
+  
 
 ...
   
@@ -1041,8 +1041,11 @@
   Each cell element specifies a NUMA cell or a NUMA node.
   cpus specifies the CPU or range of CPUs that are part of
   the node. memory specifies the node memory in kibibytes
-  (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid
-  or nodeid in the increasing order starting from 0.
+  (i.e. blocks of 1024 bytes). All cells should have id
+  attribute in case referring to some cell is necessary in the code,
+  otherwise the cells are assigned ids in the increasing order starting
+  from 0.  Mixing cells with and without the id attribute
+  is not recommended as it may result in unwanted behaviour.
 

 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index af67123..0787b5a 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3892,6 +3892,11 @@

   
 
+  
+
+  
+
+  
   
 
   
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index ebdaa19..ec80340 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -438,17 +438,47 @@ virCPUDefParseXML(xmlNodePtr node,
 for (i = 0; i < n; i++) {
 char *cpus, *memory;
 int ret, ncpus = 0;
+unsigned int cur_cell;
+char *tmp = NULL;
+
+tmp = virXMLPropString(nodes[i], "id");
+if (!tmp) {
+cur_cell = i;
+} else {
+ret  = virStrToLong_ui(tmp, NULL, 10, &cur_cell);
+VIR_FREE(tmp);
+if (ret == -1) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Invalid 'id' attribute in NUMA cell"));
+goto error;
+}
+}
+
+if (cur_cell >= n) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Exactly one 'cell' element per guest "
+ "NUMA cell allowed, non-contiguous ranges or "
+ "ranges not starting from 0 are not 
allowed"));
+goto error;
+}
+
+if (def->cells[cur_cell].cpustr) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Duplicate NUMA cell info for cell id '%u'"),
+   cur_cell);
+goto error;
+}

-def->cells[i].cellid = i;
+def->cells[cur_cell].cellid = cur_cell;
 cpus = virXMLPropString(nodes[i], "cpus");
 if (!cpus) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("Missing 'cpus' attribute in NUMA cell"));
 goto error;
 }
-def->cells[i].cpustr = cpus;
+def->cells[cur_cell].cpustr = cpus;

-ncpus = virBitmapParse(cpus, 0, &def->cells[i].cpumask,
+ncpus = virBitmapParse(cpus, 0, &def->cells[cur_cell].cpumask,
VIR_DOMAIN_CPUMASK_LEN);
 if (ncpus <= 0)
 

Re: [libvirt] [PATCH] virnuma: Check for numa_bitmask_isbitset presence

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 03:14:11PM +0200, Michal Privoznik wrote:
> On some systems, libnuma can be present but it's so ancient that
> it misses some symbols that virNumaGetDistances() needs. To be
> more precise: numa_bitmask_isbitset() and numa_nodes_ptr are the
> symbols in question. Fortunately, they were both introduced in
> the same release so it's sufficient for us to check for only one
> of them. And the winner is numa_bitmask_isbitset().
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> Notes:
> While this is a build breaker, it's not that critical. Come on -
> who runs libvirt from git on stable enterprise systems? I'm
> sending it to know if the check for numa_bitmask_isbitset() is in
> the right place or should be moved to a different file.
> 
>  m4/virt-numactl.m4 |   4 ++
>  src/util/virnuma.c | 135 
> +++--
>  2 files changed, 73 insertions(+), 66 deletions(-)
> 
> diff --git a/m4/virt-numactl.m4 b/m4/virt-numactl.m4
> index 1dcb029..fa66d24 100644
> --- a/m4/virt-numactl.m4
> +++ b/m4/virt-numactl.m4
> @@ -19,6 +19,10 @@ dnl
>  
>  AC_DEFUN([LIBVIRT_CHECK_NUMACTL],[
>LIBVIRT_CHECK_LIB([NUMACTL], [numa], [numa_available], [numa.h])
> +  AC_CHECK_LIB([numa], [numa_bitmask_isbitset], 
> [have_numa_bitmask_isbitset=yes])
> +  if test "$have_numa_bitmask_isbitset" = "yes"; then
> +AC_DEFINE_UNQUOTED([HAVE_NUMA_BITMASK_ISBITSET], 1, [whether 
> numa_bitmask_isbitset is available])
> +  fi
>  ])
>  
>  AC_DEFUN([LIBVIRT_RESULT_NUMACTL],[
> diff --git a/src/util/virnuma.c b/src/util/virnuma.c
> index 042844b..f979f49 100644
> --- a/src/util/virnuma.c
> +++ b/src/util/virnuma.c
> @@ -217,61 +217,6 @@ virNumaGetMaxNode(void)
>  
>  
>  /**
> - * virNumaGetDistances:
> - * @node: identifier of the requested NUMA node
> - * @distances: array of distances to sibling nodes
> - * @ndistances: size of @distances
> - *
> - * Get array of distances to sibling nodes from @node. If a
> - * distances[x] equals to zero, the node x is not enabled or
> - * doesn't exist. As a special case, if @node itself refers to
> - * disabled or nonexistent NUMA node, then @distances and
> - * @ndistances are set to NULL and zero respectively.
> - *
> - * The distances are a bit of magic. For a local node the value
> - * is 10, for remote it's typically 20 meaning that time penalty
> - * for accessing a remote node is two time bigger than when
> - * accessing a local node.
> - *
> - * Returns 0 on success, -1 otherwise.
> - */
> -int
> -virNumaGetDistances(int node,
> -int **distances,
> -int *ndistances)
> -{
> -int ret = -1;
> -int max_node;
> -size_t i;
> -
> -if (!numa_bitmask_isbitset(numa_nodes_ptr, node)) {
> -VIR_DEBUG("Node %d does not exist", node);
> -*distances = NULL;
> -*ndistances = 0;
> -return 0;
> -}
> -
> -if ((max_node = virNumaGetMaxNode()) < 0)
> -goto cleanup;
> -
> -if (VIR_ALLOC_N(*distances, max_node) < 0)
> -goto cleanup;
> -
> -*ndistances = max_node + 1;
> -
> -for (i = 0; i<= max_node; i++) {
> -if (!numa_bitmask_isbitset(numa_nodes_ptr, i))
> -continue;
> -
> -(*distances)[i] = numa_distance(node, i);
> -}
> -
> -ret = 0;
> - cleanup:
> -return ret;
> -}
> -
> -/**
>   * virNumaGetNodeMemory:
>   * @node: identifier of the requested NUMA node
>   * @memsize: returns the total size of memory in the NUMA node
> @@ -443,17 +388,6 @@ virNumaGetNodeCPUs(int node ATTRIBUTE_UNUSED,
> _("NUMA isn't available on this host"));
>  return -1;
>  }
> -
> -int
> -virNumaGetDistances(int node ATTRIBUTE_UNUSED,
> -int **distances,
> -int *ndistances)
> -{
> -*distances = NULL;
> -*ndistances = 0;
> -VIR_DEBUG("NUMA isn't available on this host");
> -return 0;
> -}
>  #endif
>  
>  
> @@ -469,3 +403,72 @@ virNumaGetMaxCPUs(void)
>  {
>  return NUMA_MAX_N_CPUS;
>  }
> +
> +
> +#ifdef HAVE_NUMA_BITMASK_ISBITSET
> +/**
> + * virNumaGetDistances:
> + * @node: identifier of the requested NUMA node
> + * @distances: array of distances to sibling nodes
> + * @ndistances: size of @distances
> + *
> + * Get array of distances to sibling nodes from @node. If a
> + * distances[x] equals to zero, the node x is not enabled or
> + * doesn't exist. As a special case, if @node itself refers to
> + * disabled or nonexistent NUMA node, then @distances and
> + * @ndistances are set to NULL and zero respectively.
> + *
> + * The distances are a bit of magic. For a local node the value
> + * is 10, for remote it's typically 20 meaning that time penalty
> + * for accessing a remote node is two time bigger than when
> + * accessing a local node.
> + *
> + * Returns 0 on success, -1 otherwise.
> + */
> +int
> +virNumaGetDistances(int node,
> +int **distances,
> +int *ndistances)
> +{

[libvirt] [PATCH] virnuma: Check for numa_bitmask_isbitset presence

[Michal Privoznik]

On some systems, libnuma can be present but it's so ancient that
it misses some symbols that virNumaGetDistances() needs. To be
more precise: numa_bitmask_isbitset() and numa_nodes_ptr are the
symbols in question. Fortunately, they were both introduced in
the same release so it's sufficient for us to check for only one
of them. And the winner is numa_bitmask_isbitset().

Signed-off-by: Michal Privoznik 
---

Notes:
While this is a build breaker, it's not that critical. Come on -
who runs libvirt from git on stable enterprise systems? I'm
sending it to know if the check for numa_bitmask_isbitset() is in
the right place or should be moved to a different file.

 m4/virt-numactl.m4 |   4 ++
 src/util/virnuma.c | 135 +++--
 2 files changed, 73 insertions(+), 66 deletions(-)

diff --git a/m4/virt-numactl.m4 b/m4/virt-numactl.m4
index 1dcb029..fa66d24 100644
--- a/m4/virt-numactl.m4
+++ b/m4/virt-numactl.m4
@@ -19,6 +19,10 @@ dnl
 
 AC_DEFUN([LIBVIRT_CHECK_NUMACTL],[
   LIBVIRT_CHECK_LIB([NUMACTL], [numa], [numa_available], [numa.h])
+  AC_CHECK_LIB([numa], [numa_bitmask_isbitset], 
[have_numa_bitmask_isbitset=yes])
+  if test "$have_numa_bitmask_isbitset" = "yes"; then
+AC_DEFINE_UNQUOTED([HAVE_NUMA_BITMASK_ISBITSET], 1, [whether 
numa_bitmask_isbitset is available])
+  fi
 ])
 
 AC_DEFUN([LIBVIRT_RESULT_NUMACTL],[
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 042844b..f979f49 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -217,61 +217,6 @@ virNumaGetMaxNode(void)
 
 
 /**
- * virNumaGetDistances:
- * @node: identifier of the requested NUMA node
- * @distances: array of distances to sibling nodes
- * @ndistances: size of @distances
- *
- * Get array of distances to sibling nodes from @node. If a
- * distances[x] equals to zero, the node x is not enabled or
- * doesn't exist. As a special case, if @node itself refers to
- * disabled or nonexistent NUMA node, then @distances and
- * @ndistances are set to NULL and zero respectively.
- *
- * The distances are a bit of magic. For a local node the value
- * is 10, for remote it's typically 20 meaning that time penalty
- * for accessing a remote node is two time bigger than when
- * accessing a local node.
- *
- * Returns 0 on success, -1 otherwise.
- */
-int
-virNumaGetDistances(int node,
-int **distances,
-int *ndistances)
-{
-int ret = -1;
-int max_node;
-size_t i;
-
-if (!numa_bitmask_isbitset(numa_nodes_ptr, node)) {
-VIR_DEBUG("Node %d does not exist", node);
-*distances = NULL;
-*ndistances = 0;
-return 0;
-}
-
-if ((max_node = virNumaGetMaxNode()) < 0)
-goto cleanup;
-
-if (VIR_ALLOC_N(*distances, max_node) < 0)
-goto cleanup;
-
-*ndistances = max_node + 1;
-
-for (i = 0; i<= max_node; i++) {
-if (!numa_bitmask_isbitset(numa_nodes_ptr, i))
-continue;
-
-(*distances)[i] = numa_distance(node, i);
-}
-
-ret = 0;
- cleanup:
-return ret;
-}
-
-/**
  * virNumaGetNodeMemory:
  * @node: identifier of the requested NUMA node
  * @memsize: returns the total size of memory in the NUMA node
@@ -443,17 +388,6 @@ virNumaGetNodeCPUs(int node ATTRIBUTE_UNUSED,
_("NUMA isn't available on this host"));
 return -1;
 }
-
-int
-virNumaGetDistances(int node ATTRIBUTE_UNUSED,
-int **distances,
-int *ndistances)
-{
-*distances = NULL;
-*ndistances = 0;
-VIR_DEBUG("NUMA isn't available on this host");
-return 0;
-}
 #endif
 
 
@@ -469,3 +403,72 @@ virNumaGetMaxCPUs(void)
 {
 return NUMA_MAX_N_CPUS;
 }
+
+
+#ifdef HAVE_NUMA_BITMASK_ISBITSET
+/**
+ * virNumaGetDistances:
+ * @node: identifier of the requested NUMA node
+ * @distances: array of distances to sibling nodes
+ * @ndistances: size of @distances
+ *
+ * Get array of distances to sibling nodes from @node. If a
+ * distances[x] equals to zero, the node x is not enabled or
+ * doesn't exist. As a special case, if @node itself refers to
+ * disabled or nonexistent NUMA node, then @distances and
+ * @ndistances are set to NULL and zero respectively.
+ *
+ * The distances are a bit of magic. For a local node the value
+ * is 10, for remote it's typically 20 meaning that time penalty
+ * for accessing a remote node is two time bigger than when
+ * accessing a local node.
+ *
+ * Returns 0 on success, -1 otherwise.
+ */
+int
+virNumaGetDistances(int node,
+int **distances,
+int *ndistances)
+{
+int ret = -1;
+int max_node;
+size_t i;
+
+if (!numa_bitmask_isbitset(numa_nodes_ptr, node)) {
+VIR_DEBUG("Node %d does not exist", node);
+*distances = NULL;
+*ndistances = 0;
+return 0;
+}
+
+if ((max_node = virNumaGetMaxNode()) < 0)
+goto cleanup;
+
+if (VIR_ALLOC_N(*distances, max_node) < 0)
+  

[libvirt] [PATCH v2] maint: prohibit empty first lines

[Martin Kletzander]

Based on discussion with Eric:

https://www.redhat.com/archives/libvir-list/2014-March/msg01001.html

Signed-off-by: Martin Kletzander 
---
v2:
 - rebase on current master and s/FILENAME:1:/FILENAME ":1:"/

 cfg.mk | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/cfg.mk b/cfg.mk
index 675af21..80440b5 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -938,6 +938,13 @@ sc_require_locale_h:
fi; 
\
done;

+sc_prohibit_empty_first_line:
+   @awk 'BEGIN { fail=0; }\
+   FNR == 1 { if ($$0 == "") { print FILENAME ":1:"; fail=1; } } \
+   END { if (fail == 1) { \
+ print "$(ME): Prohibited empty first line" > "/dev/stderr";  \
+   } exit fail; }' $$($(VC_LIST_EXCEPT) | grep '\.[ch]$$');
+
 # We don't use this feature of maint.mk.
 prev_version_file = /dev/null

--
2.0.0

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

Re: [libvirt] [RFC] Atomic Operations

[Martin Kletzander]

On Wed, Jun 04, 2014 at 08:16:16AM -0400, Tucker DiNapoli wrote:

I'm working on job control which involves a lot of locking and unlocking,
often for operations that could be done atomically. I use the existing
atomic integer operations in viratomic.h where possible, but there are a
lot of parameters involved in jobs (such as time, thread id's, progress
information, etc) which don't fit in an integer.



Anything for which you need to implement such operations, you can also
create some lock for that.  If there's an object (struct) you want to
have those operations on, the cleanest way (for libvirt code) is to
create a "class" that derives from virObjectLockable.  That way you
can use virObjectLock/Unlock operations on that object.  Each
operation on that object should be in a function anyway, so the places
for doing lock/unlock should be clear.  Using viratomic is definitely
faster, but, as you said, we don't use that for anything else than
integers.


I have several ideas of ways to update the existing integer atomic
operations to support more types. The easiest way (at least regarding code)
would be to use an existing atomic operations library. My suggestion would
be the libatomic_ops library, which is licensed under an MIT style license
and could be added to the source tree as a git submodule (similar to the
way gnulib is) or a specific version could be used and the library source
itself added to the git repository. The library can be found at
https://github.com/ivmai/libatomic_ops/ .



I don't know how others will respond, but I myself would avoid adding
new dependencies unless completely necessary.  But let's see what
others think.  If we have enough use cases for that it could speed up
some core bottlenecks maybe.


The other way of augmenting atomic operations would be to extend the
existing viratomic.h file to support additional types. Personally I have
experience working with GCC atomic operations and could implement that
myself, but I wouldn't be able to implement the windows versions.

Again I think the easiest and most maintainable way of supporting atomic
operations is via an external library, but I am not sure of the feasibility
of adding another dependency to libvirt. In general atomic operations are
an optimization, but they can help make multithreaded programming easier
and I think libvirt would be enhanced by having more comprehensive support
for atomic operations.

Tucker DiNapoli


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

Re: [libvirt] [libvirt-sandbox] virt-sandbox-service: fixed /lib/ into /usr/lib for searching unit files

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 02:22:05PM +0200, Cedric Bosdonnat wrote:
> On Wed, 2014-06-04 at 12:51 +0100, Daniel P. Berrange wrote:
> > On Wed, Jun 04, 2014 at 01:24:17PM +0200, Cedric Bosdonnat wrote:
> > > On Wed, 2014-06-04 at 11:24 +0200, Christophe Fergeau wrote:
> > > > On Tue, May 13, 2014 at 10:10:36AM +0200, Cédric Bosdonnat wrote:
> > > > > ---
> > > > >  bin/virt-sandbox-service | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
> > > > > index 2dcbfb8..9ed37e0 100755
> > > > > --- a/bin/virt-sandbox-service
> > > > > +++ b/bin/virt-sandbox-service
> > > > > @@ -1071,7 +1071,7 @@ class CheckUnit(argparse.Action):
> > > > >  src = "/etc/systemd/system/" + unit
> > > > >  if os.path.exists(src):
> > > > >  return src
> > > > > -src = "/lib/systemd/system/" + unit
> > > > > +src = "/usr/lib/systemd/system/" + unit
> > > > 
> > > > This makes sense to me as systemd installs unit files in
> > > > $rootdir/lib/systemd, with $rootdir set to /usr at least upstream and in
> > > > fedora.
> > > > Are you having issues withusing /lib instead of /usr/lib? Or is
> > > > it just cleaner this way?
> > > 
> > > Yes, we don't have a /lib symlink pointing to /usr/lib: so failing here
> > > on openSUSE.
> > 
> > Oh I thought systemd required that / and /usr were always one and
> > the same location ?
> 
> Well, I'm pointed to this page for the details of the story:
> http://freedesktop.org/wiki/Software/systemd/separate-usr-is-broken/

 "On the Fedora distribution we have succeeded to clean up the situation
  and the confusion the current split between / and /usr has created. We
  have moved all tools that over time have been moved to / back to /usr
  (where they belong), and the root file system only contains compatibility
  symlinks for /bin and /sbin into /usr. All binaries of the system are
  exclusively located within the /usr hierarchy. "

So from that it makes sense that we use /usr/lib as the canonical
path location. So ACK to your patch


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

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

Re: [libvirt] [RFC] Atomic Operations

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 08:16:16AM -0400, Tucker DiNapoli wrote:
> I'm working on job control which involves a lot of locking and unlocking,
> often for operations that could be done atomically. I use the existing
> atomic integer operations in viratomic.h where possible, but there are a
> lot of parameters involved in jobs (such as time, thread id's, progress
> information, etc) which don't fit in an integer.

If there are many different data items which need updating then I'd
really think this is better done with mutexes. I wouldn't even assume
that atomic ops are neccessarily going to be faster. When I optimized
our logging routines recently I tried to make use of atomic ops for
a bunch of variables but found it was actually quicker to use mutexes
since one mutex lock can protect many variable writes, whereas every
atomic op read/write has a cost.

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

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

Re: [libvirt] [libvirt-sandbox] virt-sandbox-service: fixed /lib/ into /usr/lib for searching unit files

[Cedric Bosdonnat]

On Wed, 2014-06-04 at 12:51 +0100, Daniel P. Berrange wrote:
> On Wed, Jun 04, 2014 at 01:24:17PM +0200, Cedric Bosdonnat wrote:
> > On Wed, 2014-06-04 at 11:24 +0200, Christophe Fergeau wrote:
> > > On Tue, May 13, 2014 at 10:10:36AM +0200, Cédric Bosdonnat wrote:
> > > > ---
> > > >  bin/virt-sandbox-service | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
> > > > index 2dcbfb8..9ed37e0 100755
> > > > --- a/bin/virt-sandbox-service
> > > > +++ b/bin/virt-sandbox-service
> > > > @@ -1071,7 +1071,7 @@ class CheckUnit(argparse.Action):
> > > >  src = "/etc/systemd/system/" + unit
> > > >  if os.path.exists(src):
> > > >  return src
> > > > -src = "/lib/systemd/system/" + unit
> > > > +src = "/usr/lib/systemd/system/" + unit
> > > 
> > > This makes sense to me as systemd installs unit files in
> > > $rootdir/lib/systemd, with $rootdir set to /usr at least upstream and in
> > > fedora.
> > > Are you having issues withusing /lib instead of /usr/lib? Or is
> > > it just cleaner this way?
> > 
> > Yes, we don't have a /lib symlink pointing to /usr/lib: so failing here
> > on openSUSE.
> 
> Oh I thought systemd required that / and /usr were always one and
> the same location ?

Well, I'm pointed to this page for the details of the story:
http://freedesktop.org/wiki/Software/systemd/separate-usr-is-broken/

--
Cedric

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

[libvirt] [RFC] Atomic Operations

[Tucker DiNapoli]

I'm working on job control which involves a lot of locking and unlocking,
often for operations that could be done atomically. I use the existing
atomic integer operations in viratomic.h where possible, but there are a
lot of parameters involved in jobs (such as time, thread id's, progress
information, etc) which don't fit in an integer.

I have several ideas of ways to update the existing integer atomic
operations to support more types. The easiest way (at least regarding code)
would be to use an existing atomic operations library. My suggestion would
be the libatomic_ops library, which is licensed under an MIT style license
and could be added to the source tree as a git submodule (similar to the
way gnulib is) or a specific version could be used and the library source
itself added to the git repository. The library can be found at
https://github.com/ivmai/libatomic_ops/ .

The other way of augmenting atomic operations would be to extend the
existing viratomic.h file to support additional types. Personally I have
experience working with GCC atomic operations and could implement that
myself, but I wouldn't be able to implement the windows versions.

Again I think the easiest and most maintainable way of supporting atomic
operations is via an external library, but I am not sure of the feasibility
of adding another dependency to libvirt. In general atomic operations are
an optimization, but they can help make multithreaded programming easier
and I think libvirt would be enhanced by having more comprehensive support
for atomic operations.

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

Re: [libvirt] [libvirt-sandbox] virt-sandbox-service: fixed /lib/ into /usr/lib for searching unit files

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 01:24:17PM +0200, Cedric Bosdonnat wrote:
> On Wed, 2014-06-04 at 11:24 +0200, Christophe Fergeau wrote:
> > On Tue, May 13, 2014 at 10:10:36AM +0200, Cédric Bosdonnat wrote:
> > > ---
> > >  bin/virt-sandbox-service | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
> > > index 2dcbfb8..9ed37e0 100755
> > > --- a/bin/virt-sandbox-service
> > > +++ b/bin/virt-sandbox-service
> > > @@ -1071,7 +1071,7 @@ class CheckUnit(argparse.Action):
> > >  src = "/etc/systemd/system/" + unit
> > >  if os.path.exists(src):
> > >  return src
> > > -src = "/lib/systemd/system/" + unit
> > > +src = "/usr/lib/systemd/system/" + unit
> > 
> > This makes sense to me as systemd installs unit files in
> > $rootdir/lib/systemd, with $rootdir set to /usr at least upstream and in
> > fedora.
> > Are you having issues withusing /lib instead of /usr/lib? Or is
> > it just cleaner this way?
> 
> Yes, we don't have a /lib symlink pointing to /usr/lib: so failing here
> on openSUSE.

Oh I thought systemd required that / and /usr were always one and
the same location ?

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

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

Re: [libvirt] [libvirt-sandbox] virt-sandbox-service: fixed /lib/ into /usr/lib for searching unit files

[Christophe Fergeau]

On Wed, Jun 04, 2014 at 01:24:17PM +0200, Cedric Bosdonnat wrote:
> On Wed, 2014-06-04 at 11:24 +0200, Christophe Fergeau wrote:
> > On Tue, May 13, 2014 at 10:10:36AM +0200, Cédric Bosdonnat wrote:
> > > ---
> > >  bin/virt-sandbox-service | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
> > > index 2dcbfb8..9ed37e0 100755
> > > --- a/bin/virt-sandbox-service
> > > +++ b/bin/virt-sandbox-service
> > > @@ -1071,7 +1071,7 @@ class CheckUnit(argparse.Action):
> > >  src = "/etc/systemd/system/" + unit
> > >  if os.path.exists(src):
> > >  return src
> > > -src = "/lib/systemd/system/" + unit
> > > +src = "/usr/lib/systemd/system/" + unit
> > 
> > This makes sense to me as systemd installs unit files in
> > $rootdir/lib/systemd, with $rootdir set to /usr at least upstream and in
> > fedora.
> > Are you having issues withusing /lib instead of /usr/lib? Or is
> > it just cleaner this way?
> 
> Yes, we don't have a /lib symlink pointing to /usr/lib: so failing here
> on openSUSE.

Ah, this would be useful info to have in the commit log imo. ACK from
me, though I'm not familiar at all with all of this ;)

Christophe


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

Re: [libvirt] [libvirt-sandbox] virt-sandbox-service: fixed /lib/ into /usr/lib for searching unit files

[Cedric Bosdonnat]

On Wed, 2014-06-04 at 11:24 +0200, Christophe Fergeau wrote:
> On Tue, May 13, 2014 at 10:10:36AM +0200, Cédric Bosdonnat wrote:
> > ---
> >  bin/virt-sandbox-service | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
> > index 2dcbfb8..9ed37e0 100755
> > --- a/bin/virt-sandbox-service
> > +++ b/bin/virt-sandbox-service
> > @@ -1071,7 +1071,7 @@ class CheckUnit(argparse.Action):
> >  src = "/etc/systemd/system/" + unit
> >  if os.path.exists(src):
> >  return src
> > -src = "/lib/systemd/system/" + unit
> > +src = "/usr/lib/systemd/system/" + unit
> 
> This makes sense to me as systemd installs unit files in
> $rootdir/lib/systemd, with $rootdir set to /usr at least upstream and in
> fedora.
> Are you having issues withusing /lib instead of /usr/lib? Or is
> it just cleaner this way?

Yes, we don't have a /lib symlink pointing to /usr/lib: so failing here
on openSUSE.

--
Cedric

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

Re: [libvirt] storage conf: Add key-value options to storage pools

[Daniel P. Berrange]

On Tue, Jun 03, 2014 at 05:33:13PM +0200, Ján Tomko wrote:
> On 05/28/2014 11:38 AM, Wido den Hollander wrote:
> > This series of patches adds the ability to pass down options to the
> > storage pool drivers.
> > 
> > In the case of NFS users can specify mount options and in the case of
> > RBD users can specify options for librados to influence some behavior.
> > 
> > All options and values are checked on input validity to prevent injection
> > of malicious commands.
> > 
> 
> Should we allow arbitrary options?

No, we shouldn't - at least not in this way. Libvirt's goal is always to
to provide a clearly defined mapping, not do arbitrary unchecked argument
passthrough. History has shown repeatedly that tools/libraries change their
syntax and libvirt has demonstrated its value in adapting to these changes
without breaking application compatibility.

The only way I'd support passthrough is if it were done in he same way
as QEMU passthrough where it used a separate XML namespace which was clearly
marked "use at your own risk, unsupported if it breaks".


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

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

Re: [libvirt] storage conf: Add key-value options to storage pools

[Michael Chapman]

On Tue, 3 Jun 2014, Ján Tomko wrote:

On 05/28/2014 11:38 AM, Wido den Hollander wrote:

This series of patches adds the ability to pass down options to the
storage pool drivers.

In the case of NFS users can specify mount options and in the case of
RBD users can specify options for librados to influence some behavior.

All options and values are checked on input validity to prevent injection
of malicious commands.



Should we allow arbitrary options?


Ceph in particular has a tendency to add new options with each release, 
and I'm not really sure that it's feasible for libvirt to know about all 
of these options natively.


For instance, I've been running libvirt with a local patch that let's me 
attach arbitrary options to RBD disks. This lets me configure things like 
"rbd cache size" and "rbd cache writethrough until flush" on a per-disk 
basis. Previous discussion on this list has indicated that some people 
would find Ceph's debugging options useful as well.


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

[libvirt] [PATCH] virnuma: Implement virNumaGetDistances stub for non-NUMA

[Michal Privoznik]

In case the libvirt is built without numactl support, we're
missing the virNumaGetDistances() stub so the linking fails:

  CCLD libvirt_lxc
libvirt_lxc-nodeinfo.o: In function `virNodeCapsGetSiblingInfo':
/home/zippy/tmp/libvirt.git/src/nodeinfo.c:1763: undefined reference to 
`virNumaGetDistances'
collect2: error: ld returned 1 exit status
make[3]: *** [libvirt_lxc] Error 1

The issue was introduced in 77c830d8c4.

Signed-off-by: Michal Privoznik 
---

Notes:
Pushing under build-breaker and trivial rules.

 src/util/virnuma.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index e8ceec8..042844b 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -443,6 +443,17 @@ virNumaGetNodeCPUs(int node ATTRIBUTE_UNUSED,
_("NUMA isn't available on this host"));
 return -1;
 }
+
+int
+virNumaGetDistances(int node ATTRIBUTE_UNUSED,
+int **distances,
+int *ndistances)
+{
+*distances = NULL;
+*ndistances = 0;
+VIR_DEBUG("NUMA isn't available on this host");
+return 0;
+}
 #endif
 
 
-- 
2.0.0

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

[libvirt] [PATCHv4 3/4] virsh: Reject negative numbers in vshCommandOptULongLong

[Peter Krempa]

To follow the new semantics of the vshCommandOptToU* functions convert
this one to reject negative numbers too. To allow using -1 for "maximum"
semantics for the vol-*load two bandwidth functions that use this helper
introduce vshCommandOptULongLongWrap.
---
 tools/virsh-volume.c |  8 
 tools/virsh.c| 51 ---
 tools/virsh.h|  3 +++
 3 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index f284fa5..724a86b 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -677,12 +677,12 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
 const char *name = NULL;
 unsigned long long offset = 0, length = 0;

-if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) {
+if (vshCommandOptULongLongWrap(cmd, "offset", &offset) < 0) {
 vshError(ctl, _("Unable to parse integer"));
 return false;
 }

-if (vshCommandOptULongLong(cmd, "length", &length) < 0) {
+if (vshCommandOptULongLongWrap(cmd, "length", &length) < 0) {
 vshError(ctl, _("Unable to parse integer"));
 return false;
 }
@@ -787,12 +787,12 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
 unsigned long long offset = 0, length = 0;
 bool created = false;

-if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) {
+if (vshCommandOptULongLongWrap(cmd, "offset", &offset) < 0) {
 vshError(ctl, _("Unable to parse integer"));
 return false;
 }

-if (vshCommandOptULongLong(cmd, "length", &length) < 0) {
+if (vshCommandOptULongLongWrap(cmd, "length", &length) < 0) {
 vshError(ctl, _("Unable to parse integer"));
 return false;
 }
diff --git a/tools/virsh.c b/tools/virsh.c
index 77a4f99..82669b0 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1699,31 +1699,60 @@ vshCommandOptLongLong(const vshCmd *cmd, const char 
*name,
 return 1;
 }

+static int
+vshCommandOptULongLongInternal(const vshCmd *cmd,
+   const char *name,
+   unsigned long long *value,
+   bool wrap)
+{
+vshCmdOpt *arg;
+int ret;
+
+if ((ret = vshCommandOpt(cmd, name, &arg, true)) <= 0)
+return ret;
+
+if (wrap) {
+if (virStrToLong_ull(arg->data, NULL, 10, value) < 0)
+return -1;
+} else {
+if (virStrToLong_ullp(arg->data, NULL, 10, value) < 0)
+return -1;
+}
+
+return 1;
+}
+
 /**
  * vshCommandOptULongLong:
  * @cmd command reference
  * @name option name
  * @value result
  *
- * Returns option as long long
+ * Returns option as long long, rejects negative numbers
  * See vshCommandOptInt()
  */
 int
 vshCommandOptULongLong(const vshCmd *cmd, const char *name,
unsigned long long *value)
 {
-vshCmdOpt *arg;
-int ret;
-
-ret = vshCommandOpt(cmd, name, &arg, true);
-if (ret <= 0)
-return ret;
-
-if (virStrToLong_ull(arg->data, NULL, 10, value) < 0)
-return -1;
-return 1;
+return vshCommandOptULongLongInternal(cmd, name, value, false);
 }

+/**
+ * vshCommandOptULongLongWrap:
+ * @cmd command reference
+ * @name option name
+ * @value result
+ *
+ * Returns option as long long, wraps negative numbers to positive
+ * See vshCommandOptInt()
+ */
+int
+vshCommandOptULongLongWrap(const vshCmd *cmd, const char *name,
+   unsigned long long *value)
+{
+return vshCommandOptULongLongInternal(cmd, name, value, true);
+}

 /**
  * vshCommandOptScaledInt:
diff --git a/tools/virsh.h b/tools/virsh.h
index 4f5c336..7656407 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -307,6 +307,9 @@ int vshCommandOptLongLong(const vshCmd *cmd, const char 
*name,
 int vshCommandOptULongLong(const vshCmd *cmd, const char *name,
unsigned long long *value)
 ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
+int vshCommandOptULongLongWrap(const vshCmd *cmd, const char *name,
+   unsigned long long *value)
+ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
 int vshCommandOptScaledInt(const vshCmd *cmd, const char *name,
unsigned long long *value, int scale,
unsigned long long max)
-- 
1.9.3

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

[libvirt] [PATCHv4 0/4] virsh: Clean unsigned value parsing

[Peter Krempa]

Jincheng Miao (1):
  virsh: forbid negative vcpu argument to vcpupin

Peter Krempa (3):
  virsh: Reject negative numbers in vshCommandOptUInt
  virsh: Reject negative numbers in vshCommandOptUL
  virsh: Reject negative numbers in vshCommandOptULongLong

 tests/vcpupin|  29 ++-
 tools/virsh-domain.c |  39 ---
 tools/virsh-volume.c |   8 +--
 tools/virsh.c| 137 +--
 tools/virsh.h|   9 
 5 files changed, 172 insertions(+), 50 deletions(-)

-- 
1.9.3

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

[libvirt] [PATCHv4 2/4] virsh: Reject negative numbers in vshCommandOptUL

[Peter Krempa]

To follow the new semantics of the vshCommandOptToU* functions convert
this one to reject negative numbers too. To allow using -1 for "maximum"
semantics for the two bandwidth functions that use this helper introduce
vshCommandOptULWrap. Although currently the migrate-setspeed function
for the qemu driver will reject -1 as maximum.
---
 tools/virsh-domain.c |  4 ++--
 tools/virsh.c| 46 +-
 tools/virsh.h|  3 +++
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 84a6706..d76865b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1457,7 +1457,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
 if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
 goto cleanup;

-if (vshCommandOptUL(cmd, "bandwidth", &bandwidth) < 0) {
+if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) {
 vshError(ctl, "%s", _("bandwidth must be a number"));
 goto cleanup;
 }
@@ -9223,7 +9223,7 @@ cmdMigrateSetMaxSpeed(vshControl *ctl, const vshCmd *cmd)
 if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
 return false;

-if (vshCommandOptUL(cmd, "bandwidth", &bandwidth) < 0) {
+if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) {
 vshError(ctl, "%s", _("migrate: Invalid bandwidth"));
 goto done;
 }
diff --git a/tools/virsh.c b/tools/virsh.c
index cbab6b0..77a4f99 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1547,6 +1547,28 @@ vshCommandOptUIntWrap(const vshCmd *cmd, const char 
*name, unsigned int *value)
 return vshCommandOptUIntInternal(cmd, name, value, true);
 }

+static int
+vshCommandOptULInternal(const vshCmd *cmd,
+const char *name,
+unsigned long *value,
+bool wrap)
+{
+vshCmdOpt *arg;
+int ret;
+
+if ((ret = vshCommandOpt(cmd, name, &arg, true)) <= 0)
+return ret;
+
+if (wrap) {
+if (virStrToLong_ul(arg->data, NULL, 10, value) < 0)
+return -1;
+} else {
+if (virStrToLong_ulp(arg->data, NULL, 10, value) < 0)
+return -1;
+}
+
+return 1;
+}

 /*
  * vshCommandOptUL:
@@ -1560,16 +1582,22 @@ vshCommandOptUIntWrap(const vshCmd *cmd, const char 
*name, unsigned int *value)
 int
 vshCommandOptUL(const vshCmd *cmd, const char *name, unsigned long *value)
 {
-vshCmdOpt *arg;
-int ret;
-
-ret = vshCommandOpt(cmd, name, &arg, true);
-if (ret <= 0)
-return ret;
+return vshCommandOptULInternal(cmd, name, value, false);
+}

-if (virStrToLong_ul(arg->data, NULL, 10, value) < 0)
-return -1;
-return 1;
+/**
+ * vshCommandOptULWrap:
+ * @cmd command reference
+ * @name option name
+ * @value result
+ *
+ * Convert option to unsigned long, wraps negative numbers to positive
+ * See vshCommandOptInt()
+ */
+int
+vshCommandOptULWrap(const vshCmd *cmd, const char *name, unsigned long *value)
+{
+return vshCommandOptULInternal(cmd, name, value, true);
 }

 /**
diff --git a/tools/virsh.h b/tools/virsh.h
index 9afbbb5..4f5c336 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -291,6 +291,9 @@ int vshCommandOptUIntWrap(const vshCmd *cmd, const char 
*name,
 int vshCommandOptUL(const vshCmd *cmd, const char *name,
 unsigned long *value)
 ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
+int vshCommandOptULWrap(const vshCmd *cmd, const char *name,
+unsigned long *value)
+ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
 int vshCommandOptString(const vshCmd *cmd, const char *name,
 const char **value)
 ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
-- 
1.9.3

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

[libvirt] [PATCHv4 4/4] virsh: forbid negative vcpu argument to vcpupin

[Peter Krempa]

From: Jincheng Miao 

The vcpupin command allowed specifying a negative number for the --vcpu
argument. This would the overflow when the underlying virDomainPinVcpu
API was called.

 $ virsh vcpupin r7 -1 0
 error: numerical overflow: input too large: 4294967295

Switch the vCPU variable to a unsigned int and parse it using the
corresponding function.

Also improve the vcpupin test to cover all the defects.

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

Signed-off-by: Jincheng Miao 
Signed-off-by: Peter Krempa 
---
 tests/vcpupin| 29 -
 tools/virsh-domain.c | 35 ++-
 2 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/tests/vcpupin b/tests/vcpupin
index f1fb038..9f34ec0 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -34,7 +34,7 @@ fail=0
 $abs_top_builddir/tools/virsh --connect test:///default vcpupin test a 0,1 > 
out 2>&1
 test $? = 1 || fail=1
 cat <<\EOF > exp || fail=1
-error: vcpupin: Invalid or missing vCPU number.
+error: vcpupin: Invalid vCPU number.

 EOF
 compare exp out || fail=1
@@ -43,9 +43,36 @@ compare exp out || fail=1
 $abs_top_builddir/tools/virsh --connect test:///default vcpupin test 100 0,1 > 
out 2>&1
 test $? = 1 || fail=1
 cat <<\EOF > exp || fail=1
+error: vcpupin: vCPU index out of range.
+
+EOF
+compare exp out || fail=1
+
+# Negative number
+$abs_top_builddir/tools/virsh --connect test:///default vcpupin test -100 0,1 
> out 2>&1
+test $? = 1 || fail=1
+cat <<\EOF > exp || fail=1
 error: vcpupin: Invalid vCPU number.

 EOF
 compare exp out || fail=1

+# missing argument
+$abs_top_builddir/tools/virsh --connect test:///default vcpupin test --cpulist 
0,1 > out 2>&1
+test $? = 1 || fail=1
+cat <<\EOF > exp || fail=1
+error: vcpupin: Missing vCPU number in pin mode.
+
+EOF
+compare exp out || fail=1
+
+# without arguments. This should succeed but the backend function in the
+# test driver isn't implemented
+$abs_top_builddir/tools/virsh --connect test:///default vcpupin test > out 2>&1
+test $? = 1 || fail=1
+cat <<\EOF > exp || fail=1
+error: this function is not supported by the connection driver: 
virDomainGetVcpuPinInfo
+
+EOF
+compare exp out || fail=1
 (exit $fail); exit $fail
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d76865b..e43c380 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5797,7 +5797,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
 {
 virDomainInfo info;
 virDomainPtr dom;
-int vcpu = -1;
+unsigned int vcpu = 0;
 const char *cpulist = NULL;
 bool ret = false;
 unsigned char *cpumap = NULL;
@@ -5809,6 +5809,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
 bool live = vshCommandOptBool(cmd, "live");
 bool current = vshCommandOptBool(cmd, "current");
 bool query = false; /* Query mode if no cpulist */
+int got_vcpu;
 unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;

 VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
@@ -5830,29 +5831,29 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)

 query = !cpulist;

-/* In query mode, "vcpu" is optional */
-if (vshCommandOptInt(cmd, "vcpu", &vcpu) < !query) {
-vshError(ctl, "%s",
- _("vcpupin: Invalid or missing vCPU number."));
-virDomainFree(dom);
-return false;
+if ((got_vcpu = vshCommandOptUInt(cmd, "vcpu", &vcpu)) < 0) {
+vshError(ctl, "%s", _("vcpupin: Invalid vCPU number."));
+goto cleanup;
 }

-if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0) {
-virDomainFree(dom);
-return false;
+/* In pin mode, "vcpu" is necessary */
+if (!query && got_vcpu == 0) {
+vshError(ctl, "%s", _("vcpupin: Missing vCPU number in pin mode."));
+goto cleanup;
 }

 if (virDomainGetInfo(dom, &info) != 0) {
 vshError(ctl, "%s", _("vcpupin: failed to get domain information."));
-virDomainFree(dom);
-return false;
+goto cleanup;
 }

 if (vcpu >= info.nrVirtCpu) {
-vshError(ctl, "%s", _("vcpupin: Invalid vCPU number."));
-virDomainFree(dom);
-return false;
+vshError(ctl, "%s", _("vcpupin: vCPU index out of range."));
+goto cleanup;
+}
+
+if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0) {
+goto cleanup;
 }

 cpumaplen = VIR_CPU_MAPLEN(maxcpu);
@@ -5871,7 +5872,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
 vshPrintExtra(ctl, "%s %s\n", _("VCPU:"), _("CPU Affinity"));
 vshPrintExtra(ctl, "--\n");
 for (i = 0; i < ncpus; i++) {
-   if (vcpu != -1 && i != vcpu)
+   if (got_vcpu && i != vcpu)
continue;

vshPrint(ctl, "%4zu: ", i);
@@ -5880,8 +5881,8 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
if (!ret)
break;
 }
-
 }
+
 VIR_FREE(cpumaps);

[libvirt] [PATCHv4 1/4] virsh: Reject negative numbers in vshCommandOptUInt

[Peter Krempa]

Use virStrToLong_uip instead of virStrToLong_ui to reject negative
numbers in the helper. None of the callers expects the wraparound
"feature" for negative numbers.

Also add a function that allows wrapping of negative numbers as it might
be used in the future and be explicit about the new semantics in the
function docs.
---
 tools/virsh.c | 48 ++--
 tools/virsh.h |  3 +++
 2 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 15f3025..cbab6b0 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1494,6 +1494,28 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, 
int *value)
 return 1;
 }

+static int
+vshCommandOptUIntInternal(const vshCmd *cmd,
+  const char *name,
+  unsigned int *value,
+  bool wrap)
+{
+vshCmdOpt *arg;
+int ret;
+
+if ((ret = vshCommandOpt(cmd, name, &arg, true)) <= 0)
+return ret;
+
+if (wrap) {
+if (virStrToLong_ui(arg->data, NULL, 10, value) < 0)
+return -1;
+} else {
+if (virStrToLong_uip(arg->data, NULL, 10, value) < 0)
+return -1;
+}
+
+return 1;
+}

 /**
  * vshCommandOptUInt:
@@ -1501,22 +1523,28 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, 
int *value)
  * @name option name
  * @value result
  *
- * Convert option to unsigned int
+ * Convert option to unsigned int, reject negative numbers
  * See vshCommandOptInt()
  */
 int
 vshCommandOptUInt(const vshCmd *cmd, const char *name, unsigned int *value)
 {
-vshCmdOpt *arg;
-int ret;
-
-ret = vshCommandOpt(cmd, name, &arg, true);
-if (ret <= 0)
-return ret;
+return vshCommandOptUIntInternal(cmd, name, value, false);
+}

-if (virStrToLong_ui(arg->data, NULL, 10, value) < 0)
-return -1;
-return 1;
+/**
+ * vshCommandOptUIntWrap:
+ * @cmd command reference
+ * @name option name
+ * @value result
+ *
+ * Convert option to unsigned int, wraps negative numbers to positive
+ * See vshCommandOptInt()
+ */
+int
+vshCommandOptUIntWrap(const vshCmd *cmd, const char *name, unsigned int *value)
+{
+return vshCommandOptUIntInternal(cmd, name, value, true);
 }


diff --git a/tools/virsh.h b/tools/virsh.h
index 1ffc003..9afbbb5 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -285,6 +285,9 @@ int vshCommandOptInt(const vshCmd *cmd, const char *name, 
int *value)
 int vshCommandOptUInt(const vshCmd *cmd, const char *name,
   unsigned int *value)
 ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
+int vshCommandOptUIntWrap(const vshCmd *cmd, const char *name,
+  unsigned int *value)
+ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
 int vshCommandOptUL(const vshCmd *cmd, const char *name,
 unsigned long *value)
 ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
-- 
1.9.3

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

Re: [libvirt] libvirt, libxl and

[Daniel P. Berrange]

On Wed, Jun 04, 2014 at 09:11:59AM +, Dave Scott wrote:
> Hi,
> 
> Two of the applications I’d like to use with libvirt (cloudstack
> and oVirt) make use of “” in the domain XML, like this:
> 
> 
> 
> 
> 
> 
> 
> I don’t believe these are currently supported by libvirt + libxl
> — I’d like to see what it would take to hook these up.
> 
> I chatted with Daniel Berrange at the Xen hackathon last week,
> and if I understood correctly these channels are analogous to
> serial ports used for low-bandwidth communication to (e.g.)
> guest agents. Daniel suggested that the xen console mechanism
> ought to be adequate to power these things. The other option
> (if higher bandwidth was required) would be to use the Xen
> vchan protocol.

Yep, the only really relevant difference between a console and
a channel, is that a channel has an string name associated with
it. The distinction between console & channel serves a few purposes

 1. A guest OS can be set to automatically spawn getty login
process on all (paravirt based)  devices safe in
the knowledge no application is expecting to use them for
some higher level protocol.

 2. A guest agent can reliably identify which  it is
supposed to be using from its name. ie QEMU guest agent
knows that it should always open /dev/virtio-port/org.qemu.guest_agent.0
and no other app will touch that.

 3. The named port can be used to write udev rules that automatically
spawn the correct guest agent when the port with the matching
name appears in the guest. eg here we trigger a systemd service
matching on QEMU guest agent port

$ cat /lib/udev/rules.d/99-qemu-guest-agent.rules
  SUBSYSTEM=="virtio-ports", ATTR{name}=="org.qemu.guest_agent.0", \
 TAG+="systemd" ENV{SYSTEMD_WANTS}="qemu-guest-agent.service"

> I think the behaviour is:
> * bind a unix domain socket on the host (‘/var/lib/libvirt/qemu/…')
> * connect a bidirectional low-bandwidth channel to the guest
> * manifest the channel in the guest as a ‘/dev/vport/s-4-VM.vport’ device (?)

Yep, that's pretty much it. For virtio-serial the dir is /dev/virtio-ports.
If Xen uses a different non-virtio-serial device type, it can provide a
suitable guest udev rule to setup named devices in a dir that you think
is best.

> So an application on the host can connect() to the host socket, an
> application in the guest can open() the guest device and they can
> talk privately. [Have I got this right?]

Yes.

> I had a quick read of the libxl code and I think the consoles are
> considered an internal detail: there is a function libxl_console_get_tty
> to retrieve a console’s endpoint in dom0 but I couldn’t see a way to
> request additional consoles are created. The libxl_domain_config has
> disks, nics, pcidevs, vfbs, vkbs, and vtpms but no consoles. (Have I
> missed something?)
> 
> Bypassing libxl I was able to manually create a 
> /local/domain/%d/device/console/1
> which was recognised by the VM as /dev/hvc1. As an aside, I notice that there
> are 2 console backends now: xenconsoled seems to only watch for the initial
> console, while a per-domain qemu process is used for all subsequent consoles,
> so any enhancements to the dom0 end would have to go into qemu?
> 
> So to implement channels via consoles I would need to:
> 
> 1. check if qemu when acting as a console server in dom0 is able to connect
>the console to a suitably named Unix domain socket in dom0 (signalled via
>xenstore in the usual way)
> 
> 2. modify libxl to support consoles as configurable devices alongside disks,
>nics etc

I'd suggest you also want to support a non-tty backend, specifically UNIX
sockets. This means that the process connecting to the backend on the host
does not need to query libvirt to discover what random /dev/pts/NN was
allocated to it - it can just inotify watch /var/lib/libvirt/libxl/ for
new files appearing and connect to it straight off based on the name of
the file.

I'd really strongly recommend the ability to give names to the devices
if you want to re-use the xen console device type, so that the guest
agents can automatically determine the correct device and so that the
guest OS can tell which console instances it should launch a getty on
vs ignore.

> 3. add support to libvirt’s libxl driver
> 
> 4. see if I can write something like a udev rule in the guest to
>notice the console, look up the ‘name’ from xenstore and make a
>suitably-named file?
> 
> What do you think?

Sounds reasonable to me.

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

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

[libvirt] libvirt, libxl and

[Dave Scott]

Hi,

Two of the applications I’d like to use with libvirt (cloudstack and oVirt) 
make use of “” in the domain XML, like this:







I don’t believe these are currently supported by libvirt + libxl — I’d like to 
see what it would take to hook these up.

I chatted with Daniel Berrange at the Xen hackathon last week, and if I 
understood correctly these channels are analogous to serial ports used for 
low-bandwidth communication to (e.g.) guest agents. Daniel suggested that the 
xen console mechanism ought to be adequate to power these things. The other 
option (if higher bandwidth was required) would be to use the Xen vchan 
protocol.

I think the behaviour is:
* bind a unix domain socket on the host (‘/var/lib/libvirt/qemu/…')
* connect a bidirectional low-bandwidth channel to the guest
* manifest the channel in the guest as a ‘/dev/vport/s-4-VM.vport’ device (?)

So an application on the host can connect() to the host socket, an application 
in the guest can open() the guest device and they can talk privately. [Have I 
got this right?]

I had a quick read of the libxl code and I think the consoles are considered an 
internal detail: there is a function libxl_console_get_tty to retrieve a 
console’s endpoint in dom0 but I couldn’t see a way to request additional 
consoles are created. The libxl_domain_config has disks, nics, pcidevs, vfbs, 
vkbs, and vtpms but no consoles. (Have I missed something?)

Bypassing libxl I was able to manually create a 
/local/domain/%d/device/console/1 which was recognised by the VM as /dev/hvc1. 
As an aside, I notice that there are 2 console backends now: xenconsoled seems 
to only watch for the initial console, while a per-domain qemu process is used 
for all subsequent consoles, so any enhancements to the dom0 end would have to 
go into qemu?

So to implement channels via consoles I would need to:

1. check if qemu when acting as a console server in dom0 is able to connect the 
console to a suitably named Unix domain socket in dom0 (signalled via xenstore 
in the usual way)

2. modify libxl to support consoles as configurable devices alongside disks, 
nics etc

3. add support to libvirt’s libxl driver

4. see if I can write something like a udev rule in the guest to notice the 
console, look up the ‘name’ from xenstore and make a suitably-named file?

What do you think?

Cheers,
Dave


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

Re: [libvirt] [libvirt-sandbox] virt-sandbox-service: fixed /lib/ into /usr/lib for searching unit files

[Christophe Fergeau]

On Tue, May 13, 2014 at 10:10:36AM +0200, Cédric Bosdonnat wrote:
> ---
>  bin/virt-sandbox-service | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
> index 2dcbfb8..9ed37e0 100755
> --- a/bin/virt-sandbox-service
> +++ b/bin/virt-sandbox-service
> @@ -1071,7 +1071,7 @@ class CheckUnit(argparse.Action):
>  src = "/etc/systemd/system/" + unit
>  if os.path.exists(src):
>  return src
> -src = "/lib/systemd/system/" + unit
> +src = "/usr/lib/systemd/system/" + unit

This makes sense to me as systemd installs unit files in
$rootdir/lib/systemd, with $rootdir set to /usr at least upstream and in
fedora.
Are you having issues withusing /lib instead of /usr/lib? Or is
it just cleaner this way?

Christophe


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

Re: [libvirt] [libvirt-glib] spec: Move .vapi files to -devel RPMs

[Christophe Fergeau]

On Tue, Jun 03, 2014 at 03:00:17PM +0100, Daniel P. Berrange wrote:
> On Tue, Jun 03, 2014 at 03:50:50PM +0200, Christophe Fergeau wrote:
> > .vapi files are only needed when building vala programs using
> > libvirt-glib, so they belong to the -devel RPMs, not to the non-devel
> > RPMs which only hold files needed at runtime.
> > ---
> >  libvirt-glib.spec.in | 20 ++--
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> ACK

Thanks, pushed.

Christophe


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

Re: [libvirt] [PATCH] cfg.mk: Introduce rule for setlocale()

[Martin Kletzander]

On Wed, Jun 04, 2014 at 10:59:08AM +0200, Michal Privoznik wrote:

In the past we had some issues where setlocale() was called without
corresponding include of locale.h. While on some systems this may
work, on others the compilation failed. We should have a syntax-check
rule for that to prevent this from happening again.

Signed-off-by: Michal Privoznik 
---
cfg.mk | 9 +
1 file changed, 9 insertions(+)

diff --git a/cfg.mk b/cfg.mk
index 9e8fcec..36395c6 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -929,6 +929,15 @@ sc_prohibit_mixed_case_abbreviations:
halt='Use PCI, USB, SCSI, not Pci, Usb, Scsi'   \
  $(_sc_search_regexp)

+# Require #include  in all files that call setlocale()
+sc_require_locale_h:
+   @for i in $$($(VC_LIST_EXCEPT) | grep '\.[chx]$$'); do  
\
+   if test -z "$$(grep setlocale $$i)" ; then continue; fi;  \


Why not:

if ! grep -q setlocale $$i; then continue; fi

?  It shouldn't be any bashism and I think it works everywhere. I just
tried it in dash and it works.  And if I may nitpick, you can also
s/setlocale/\bsetlocale(/.


+   if test -z "$$(grep 'include ' $$i)" ; then   
\


The same here.  '# *include...' for the maximum bonus ;)


+   echo '$(ME): missing locale.h include in' $$i 1>&2; 
exit 1;  \
+   fi; 
\
+   done;
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null



ACK with the changes mentioned.

Martin


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

Re: [libvirt] [libvirt-glib] [PATCH] build: fix race in vapi/ subdirectory

[Christophe Fergeau]

On Mon, Jun 02, 2014 at 05:23:00PM -0500, Michael Catanzaro wrote:
> On Mon, 2014-06-02 at 16:12 -0600, Eric Blake wrote:
> > This hit the list with line-wrapping , making it a bit harder to tell
> > what changed. 
> 
> Sorry, patch attached to avoid line wrapping.

I have pushed this patch

Christophe


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

[libvirt] Parse commandline for -device option

[hong-hua....@freescale.com]

Hi Maintainers,

Currently libvirt could only parse commandline for several legacy devices.
For example,
-pcidevice: qemuParseCommandLinePCI()
-usbdevice: qemuParseCommandLineUSB()

It also provides test cases for host device assignment.
DO_TEST("hostdev-usb-address");
DO_TEST("hostdev-pci-address");

But for other hostdev with -device option, it failed to convert into  
tag.

-device vfio-pci,host=06:12.5,id=hostdev0,bus=pci.0,addr=0x3
$ virsh domxml-from-native qemu-argv kvm.args >kvm.xml
  


  

Expected XML file should like below:

  
  

  


Maybe it is not very critical. 
Is it complicated to add support to -device option in function 
qemuParseCommandLine()?
Do you have any thought or plan about it?

Best Regards,
Olivia

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

[libvirt] [PATCH] cfg.mk: Introduce rule for setlocale()

[Michal Privoznik]

In the past we had some issues where setlocale() was called without
corresponding include of locale.h. While on some systems this may
work, on others the compilation failed. We should have a syntax-check
rule for that to prevent this from happening again.

Signed-off-by: Michal Privoznik 
---
 cfg.mk | 9 +
 1 file changed, 9 insertions(+)

diff --git a/cfg.mk b/cfg.mk
index 9e8fcec..36395c6 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -929,6 +929,15 @@ sc_prohibit_mixed_case_abbreviations:
halt='Use PCI, USB, SCSI, not Pci, Usb, Scsi'   \
  $(_sc_search_regexp)
 
+# Require #include  in all files that call setlocale()
+sc_require_locale_h:
+   @for i in $$($(VC_LIST_EXCEPT) | grep '\.[chx]$$'); do  
\
+   if test -z "$$(grep setlocale $$i)" ; then continue; fi;
\
+   if test -z "$$(grep 'include ' $$i)" ; then   
\
+   echo '$(ME): missing locale.h include in' $$i 1>&2; 
exit 1; \
+   fi; 
\
+   done;
+
 # We don't use this feature of maint.mk.
 prev_version_file = /dev/null
 
-- 
2.0.0

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

[libvirt] Problem connecting to hypervisor.

[Vikas Kokare]

I am trying to establish a connection (using virsh command) to a couple of
hypervisor hosts, both on SLES 11.3 environments. The host from where i am
making both the connections is a SLES 10.4 instance.



*kvmh911351246:~ # virsh -c qemu+ssh://9.113.51.247:22/system
libvir: XML-RPC error : authentication
requirederror: failed to connect to the hypervisor*

The SSH authentication to hypervisor host 9.113.51.247 has already been
established.



*kvmh911351246:~ # ssh root@9.113.51.247 Last login: Tue
Jun  3 11:31:56 2014 from scm-kvmh2-rhel6 SLES11-51-247:~ #*

Hence the cause of the authentication failure is not clear.

The second connection to another SLES 11.3 host looks like




*virsh # connect qemu+ssh://9.121.58.19:22/system
 bash: socat: command not foundlibvir: Remote
error : socket closed unexpectedlyerror: Failed to connect to the
hypervisor*

where the cause of the error is entirely different.

The libvirt packages available on these environments are

*9.113.51.247 (SLES 11.3)*





*libvirt-cim-0.5.12-0.7.16libvirt-1.0.5.1-0.7.10libvirt-python-1.0.5.1-0.7.10libvirt-client-32bit-1.0.5.1-0.7.10
libvirt-lock-sanlock-1.0.5.1-0.7.10libvirt-client-1.0.5.1-0.7.10*

*9.121.58.19 (SLES 11.3)*



*libvirt-cim-0.5.12-0.7.16libvirt-client-1.0.5.1-0.7.10libvirt-python-1.0.5.1-0.7.10
libvirt-1.0.5.1-0.7.10*

*9.113.51.246 ( SLES 10.4)*




*libvirt-python-0.3.3-18.22.1libvirt-devel-0.3.3-18.22.1libvirt-0.3.3-18.22.1*
How can these connection failures be debugged? Is there a way to know more
information about them?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-sandbox PATCH] virt-sandbox-service: fixed /lib/ into /usr/lib for searching unit files

[Cedric Bosdonnat]

Hi all,

Is that patch now burried too far away in the mail stack? Could someone
review it?

--
Cedric

On Tue, 2014-05-13 at 10:10 +0200, Cédric Bosdonnat wrote:
> ---
>  bin/virt-sandbox-service | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
> index 2dcbfb8..9ed37e0 100755
> --- a/bin/virt-sandbox-service
> +++ b/bin/virt-sandbox-service
> @@ -1071,7 +1071,7 @@ class CheckUnit(argparse.Action):
>  src = "/etc/systemd/system/" + unit
>  if os.path.exists(src):
>  return src
> -src = "/lib/systemd/system/" + unit
> +src = "/usr/lib/systemd/system/" + unit
>  if os.path.exists(src):
>  return src
>  return None


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