[libvirt] [PATCH] virsh: fix domfsinfo wrong output in quiet mode

[Luyao Huang]

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

When run domfsinfo in quiet mode, we cannot get any
useful information (just get \n), this is because
we didn't use vshPrint to print useful information.

Signed-off-by: Luyao Huang 
---
 tools/virsh-domain.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a61656f..4988ba2 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12848,10 +12848,10 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
   _("Mountpoint"), _("Name"), _("Type"), _("Target"));
 vshPrintExtra(ctl, 
"---\n");
 for (i = 0; i < ret; i++) {
-vshPrintExtra(ctl, "%-36s %-8s %-8s ",
-  info[i]->mountpoint, info[i]->name, info[i]->fstype);
+vshPrint(ctl, "%-36s %-8s %-8s ",
+ info[i]->mountpoint, info[i]->name, info[i]->fstype);
 for (j = 0; j < info[i]->ndevAlias; j++) {
-vshPrintExtra(ctl, "%s", info[i]->devAlias[j]);
+vshPrint(ctl, "%s", info[i]->devAlias[j]);
 if (j != info[i]->ndevAlias - 1)
 vshPrint(ctl, ",");
 }
-- 
1.8.3.1

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

Re: [libvirt] [PATCH 3/3] qemu: Return true pining info when using numad

[John Ferlan]

$SUBJ

s/pining/pinning

Or perhaps - "qemu: Use numad information when getting pin information""

On 07/26/2015 12:57 PM, Martin Kletzander wrote:
> Pinning information returned for emulatorpin and vcpupin calls is being
> returned from our data without querying cgroups for some time.  However,
> not all the data were utilized.  When automatic placement is used the
> information is not returned for the calls mentioned above.  Since the
> numad hint in private data is properly saved/restored, we can safely use
> it to return true information.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1162947
> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/qemu/qemu_driver.c | 11 +++
>  1 file changed, 11 insertions(+)
> 

Should qemuDomainGetIOThreadsConfig be adjusted as well?  In the for
loop that's fetching/filling in the iothreadid there's a filling of the
cpumask as well.

Patches seem reasonable otherwise, although patch2 could have a wee bit
more information in the commit log to explain what's being done...
Beyond that does that value matter if placement_mode !=
VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO? or if
!virDomainDefNeedsPlacementAdvice (from qemuProcessStart)?  Was checking
where it was set and if it's set to something reasonable...

John

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 40c882c4ba88..1e090bb5c36b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5224,6 +5224,7 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom,
>  int ret = -1;
>  int hostcpus, vcpu;
>  virBitmapPtr allcpumap = NULL;
> +qemuDomainObjPrivatePtr priv = NULL;
> 
>  virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>VIR_DOMAIN_AFFECT_CONFIG, -1);
> @@ -5244,6 +5245,7 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom,
>  goto cleanup;
> 
>  virBitmapSetAll(allcpumap);
> +priv = vm->privateData;
> 
>  /* Clamp to actual number of vcpus */
>  if (ncpumaps > def->vcpus)
> @@ -5262,6 +5264,9 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom,
> 
>  if (pininfo && pininfo->cpumask)
>  bitmap = pininfo->cpumask;
> +else if (vm->def->placement_mode == 
> VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO &&
> + priv->autoCpuset)
> +bitmap = priv->autoCpuset;
>  else
>  bitmap = allcpumap;
> 
> @@ -5412,6 +5417,7 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom,
>  int hostcpus;
>  virBitmapPtr cpumask = NULL;
>  virBitmapPtr bitmap = NULL;
> +qemuDomainObjPrivatePtr priv = NULL;
> 
>  virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>VIR_DOMAIN_AFFECT_CONFIG, -1);
> @@ -5428,10 +5434,15 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom,
>  if ((hostcpus = nodeGetCPUCount(NULL)) < 0)
>  goto cleanup;
> 
> +priv = vm->privateData;
> +
>  if (def->cputune.emulatorpin) {
>  cpumask = def->cputune.emulatorpin;
>  } else if (def->cpumask) {
>  cpumask = def->cpumask;
> +} else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO 
> &&
> +   priv->autoCpuset) {
> +cpumask = priv->autoCpuset;
>  } else {
>  if (!(bitmap = virBitmapNew(hostcpus)))
>  goto cleanup;
> 

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

Re: [libvirt] Libvirt error in Openstack Tempest attaching disk on arm64 system

[Clark Laughlin]

I was just recently told I should try using virtio-scsi on arm64 -- that
QEMU 2.4.0 has everything needed for block device hotplug on arm64 using
virtio-scsi.  I see that nova appears to have support as of Feb 2014 (
https://review.openstack.org/#/c/70263/).  I am currently using the
distro-provided libvirt on trusty and vivid.  Would this be new enough to
have what I need?

Thank you,
Clark

On Wed, Jul 22, 2015 at 3:58 PM, Daniel P. Berrange 
wrote:

> On Wed, Jul 22, 2015 at 02:53:31PM -0500, Clark Laughlin wrote:
> > I am running Openstack Tempest on an arm64 platform and am seeing some
> > test failures related to attaching volumes to an instance.  This is an
> > example of the disk XML generated by one of the tests:
> >
> >  > cache="none"/ >
> dev="/dev/disk/by-path/ip-10.7.1.2:3260-iscsi-iqn.2010-10.org.openstack:volume-5a204339-80cb-4d06-aecf-2a8a2c970b0e-lun-1"/> > bus="virtio"
> dev="vdb"/>5a204339-80cb-4d06-aecf-2a8a2c970b0e
> >
> > The test is failing with the error "XML error: No PCI buses
> > available".  I am trying to find the relevent source locations for
> > this functionality in either in the nova libvirt driver or in the
> > libvirt source itself.  I am not sure why I am getting an error about
> > no PCI buses when the bus specified in the XML is "virtio".
> >
> > I would appreciate any pointers / help.
>
> virtio is just a guest/host device communication protocol that
> can be run over multiple different transport. On x86 virtio has
> always used PCI, but on s390 it uses either s390 or ccw bus,
> and on arm7/aarch64 it uses mmio. I'm not sure that the mmio
> bus supports hotplug, which could be why you see the error in
> question. Very latest upstream QEMU does now support PCI with
> aarch64 so in the near future it should have parity of functionality
> with x86
>
> 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] [sandbox PATCH v2 03/19] Image: Add Hooking Mechanism

[Eren Yagdiran]

Any custom source provider can be added to virt-sandbox-image as a source
---
 .gitignore   |  1 +
 bin/Makefile.am  | 16 
 bin/virt-sandbox-image.in|  3 +++
 configure.ac |  2 ++
 virt-sandbox-image/Makefile.am   | 13 +
 virt-sandbox-image/sources/Source.py | 31 +++
 virt-sandbox-image/sources/__init__.py   | 29 +
 virt-sandbox-image/virt-sandbox-image.py | 13 -
 8 files changed, 103 insertions(+), 5 deletions(-)
 create mode 100644 bin/virt-sandbox-image.in
 create mode 100644 virt-sandbox-image/Makefile.am
 create mode 100644 virt-sandbox-image/sources/Source.py
 create mode 100644 virt-sandbox-image/sources/__init__.py
 mode change 100644 => 100755 virt-sandbox-image/virt-sandbox-image.py

diff --git a/.gitignore b/.gitignore
index f77ea12..ef5b5aa 100644
--- a/.gitignore
+++ b/.gitignore
@@ -56,3 +56,4 @@ bin/virt-sandbox
 bin/virt-sandbox-service-util
 build/
 bin/*.1
+bin/virt-sandbox-image
diff --git a/bin/Makefile.am b/bin/Makefile.am
index 416f86f..df4c7dc 100644
--- a/bin/Makefile.am
+++ b/bin/Makefile.am
@@ -3,7 +3,11 @@ bin_PROGRAMS = virt-sandbox
 
 libexec_PROGRAMS = virt-sandbox-service-util
 
-bin_SCRIPTS = virt-sandbox-service
+bin_SCRIPTS = virt-sandbox-service \
+  virt-sandbox-image
+
+virt-sandbox-image: virt-sandbox-image.in
+   sed -e 's,[@]pkgpythondir[@],$(pkgpythondir),g' < $< >$@
 
 virtsandboxcompdir = $(datarootdir)/bash-completion/completions/
 
@@ -20,8 +24,11 @@ POD_FILES = \
virt-sandbox-service-reload.pod \
virt-sandbox-service-upgrade.pod \
$(NULL)
-EXTRA_DIST = $(bin_SCRIPTS) $(POD_FILES) 
virt-sandbox-service-bash-completion.sh virt-sandbox-service.logrotate
-EXTRA_DIST += virt-sandbox-service-bash-completion.sh
+EXTRA_DIST = virt-sandbox-service \
+ virt-sandbox-image.in \
+ $(POD_FILES) \
+ virt-sandbox-service-bash-completion.sh \
+ virt-sandbox-service.logrotate
 
 man1_MANS = \
virt-sandbox.1 \
@@ -64,7 +71,8 @@ virt-sandbox-service-reload.1: 
virt-sandbox-service-reload.pod Makefile
 virt-sandbox-service-upgrade.1: virt-sandbox-service-upgrade.pod Makefile
$(AM_V_GEN)$(POD2MAN) $< $(srcdir)/$@
 
-CLEANFILES = $(man1_MANS)
+CLEANFILES = $(man1_MANS) \
+ virt-sandbox-image
 
 virt_sandbox_SOURCES = virt-sandbox.c
 virt_sandbox_CFLAGS = \
diff --git a/bin/virt-sandbox-image.in b/bin/virt-sandbox-image.in
new file mode 100644
index 000..732bb38
--- /dev/null
+++ b/bin/virt-sandbox-image.in
@@ -0,0 +1,3 @@
+#!/bin/sh
+
+exec "@pkgpythondir@/virt-sandbox-image.py" "$@"
diff --git a/configure.ac b/configure.ac
index 8f6da04..69b5870 100644
--- a/configure.ac
+++ b/configure.ac
@@ -124,11 +124,13 @@ dnl Should be in m4/virt-gettext.m4 but intltoolize is too
 dnl dumb to find it there
 IT_PROG_INTLTOOL([0.35.0])
 
+AM_PATH_PYTHON
 
 AC_OUTPUT(Makefile
   libvirt-sandbox/Makefile
   libvirt-sandbox/tests/Makefile
   bin/Makefile
+  virt-sandbox-image/Makefile
   examples/Makefile
   docs/Makefile
   docs/libvirt-sandbox/Makefile
diff --git a/virt-sandbox-image/Makefile.am b/virt-sandbox-image/Makefile.am
new file mode 100644
index 000..5ab4d2e
--- /dev/null
+++ b/virt-sandbox-image/Makefile.am
@@ -0,0 +1,13 @@
+
+EXTRA_DIST = \
+   virt-sandbox-image.py \
+   sources
+
+install-data-local:
+   $(mkinstalldirs) $(DESTDIR)/$(pkgpythondir)/sources
+   $(INSTALL) -m 0755 $(srcdir)/virt-sandbox-image.py 
$(DESTDIR)$(pkgpythondir)
+   $(INSTALL) -m 0644 $(srcdir)/sources/__init__.py 
$(DESTDIR)$(pkgpythondir)/sources
+   $(INSTALL) -m 0644 $(srcdir)/sources/Source.py 
$(DESTDIR)$(pkgpythondir)/sources
+
+uninstall-local:
+   rm -f $(DESTDIR)$(pkgpythondir)
diff --git a/virt-sandbox-image/sources/Source.py 
b/virt-sandbox-image/sources/Source.py
new file mode 100644
index 000..43f0720
--- /dev/null
+++ b/virt-sandbox-image/sources/Source.py
@@ -0,0 +1,31 @@
+'''
+*
+* libvirt-sandbox-config-diskaccess.h: libvirt sandbox configuration
+*
+* Copyright (C) 2015 Universitat Polit??cnica de Catalunya.
+*
+* This library is free software; you can redistribute it and/or
+* modify it under the terms of the GNU Lesser General Public
+* License as published by the Free Software Foundation; either
+* version 2.1 of the License, or (at your option) any later version.
+*
+* This library is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+* Lesser General Public License for more details.
+*
+* You should have received a copy of the GNU Lesser General Public
+* License along with this library; if not, write to the Free Software
+* Foundation, Inc., 51 Fr

[libvirt] [sandbox PATCH v2 01/19] Add virt-sandbox-image

[Eren Yagdiran]

From: Daniel P Berrange 

virt-sandbox-image.py is a python script that lets you download Docker
images easily. It is a proof of concept code and consumes Docker Rest API.
---
 po/POTFILES.in   |   1 +
 virt-sandbox-image/virt-sandbox-image.py | 394 +++
 2 files changed, 395 insertions(+)
 create mode 100644 virt-sandbox-image/virt-sandbox-image.py

diff --git a/po/POTFILES.in b/po/POTFILES.in
index afcb050..7204112 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -11,3 +11,4 @@ libvirt-sandbox/libvirt-sandbox-context-interactive.c
 libvirt-sandbox/libvirt-sandbox-init-common.c
 libvirt-sandbox/libvirt-sandbox-rpcpacket.c
 libvirt-sandbox/libvirt-sandbox-util.c
+virt-sandbox-image/virt-sandbox-image.py
diff --git a/virt-sandbox-image/virt-sandbox-image.py 
b/virt-sandbox-image/virt-sandbox-image.py
new file mode 100644
index 000..4f5443b
--- /dev/null
+++ b/virt-sandbox-image/virt-sandbox-image.py
@@ -0,0 +1,394 @@
+#!/usr/bin/python -Es
+#
+# Authors: Daniel P. Berrange 
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program 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 General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+#
+
+import argparse
+import gettext
+import hashlib
+import json
+import os
+import os.path
+import shutil
+import sys
+import urllib2
+import subprocess
+
+default_index_server = "index.docker.io"
+default_template_dir = "/var/lib/libvirt/templates"
+
+debug = True
+verbose = True
+
+gettext.bindtextdomain("libvirt-sandbox", "/usr/share/locale")
+gettext.textdomain("libvirt-sandbox")
+try:
+gettext.install("libvirt-sandbox",
+localedir="/usr/share/locale",
+unicode=False,
+codeset = 'utf-8')
+except IOError:
+import __builtin__
+__builtin__.__dict__['_'] = unicode
+
+
+def debug(msg):
+sys.stderr.write(msg)
+
+def info(msg):
+sys.stdout.write(msg)
+
+def get_url(server, path, headers):
+url = "https://"; + server + path
+debug("  Fetching %s..." % url)
+req = urllib2.Request(url=url)
+
+if json:
+req.add_header("Accept", "application/json")
+
+for h in headers.keys():
+req.add_header(h, headers[h])
+
+return urllib2.urlopen(req)
+
+def get_json(server, path, headers):
+try:
+res = get_url(server, path, headers)
+data = json.loads(res.read())
+debug("OK\n")
+return (data, res)
+except Exception, e:
+debug("FAIL %s\n" % str(e))
+raise
+
+def save_data(server, path, headers, dest, checksum=None, datalen=None):
+try:
+res = get_url(server, path, headers)
+
+csum = None
+if checksum is not None:
+csum = hashlib.sha256()
+
+pattern = [".", "o", "O", "o"]
+patternIndex = 0
+donelen = 0
+
+with open(dest, "w") as f:
+while 1:
+buf = res.read(1024*64)
+if not buf:
+break
+if csum is not None:
+csum.update(buf)
+f.write(buf)
+
+if datalen is not None:
+donelen = donelen + len(buf)
+debug("\x1b[s%s (%5d Kb of %5d Kb)\x1b8" % (
+pattern[patternIndex], (donelen/1024), (datalen/1024)
+))
+patternIndex = (patternIndex + 1) % 4
+
+debug("\x1b[K")
+if csum is not None:
+csumstr = "sha256:" + csum.hexdigest()
+if csumstr != checksum:
+debug("FAIL checksum '%s' does not match '%s'" % (csumstr, 
checksum))
+os.remove(dest)
+raise IOError("Checksum '%s' for data does not match '%s'" % 
(csumstr, checksum))
+debug("OK\n")
+return res
+except Exception, e:
+debug("FAIL %s\n" % str(e))
+raise
+
+
+def download_template(name, server, destdir):
+tag = "latest"
+
+offset = name.find(':')
+if offset != -1:
+tag = name[offset + 1:]
+name = name[0:offset]
+
+# First we must ask the index server about the image name. THe
+# index server will return an auth token we can use when talking
+# to the registry server. We need this token even when anonymous
+try:
+(data, res) = get_json(server, "/v1/repositories/" + name + "/images",
+

[libvirt] [sandbox PATCH v2 19/19] Image: Add custom environment support

[Eren Yagdiran]

Any custom key=value pair can be used as a custom environment variable
in virt-sandbox-image.
e.g virt-sandbox-image run ubuntu /var/lib/libvirt/templates -c lxc:/// -i 
/bin/bash -e key1=val1
---
 virt-sandbox-image/sources/DockerSource.py | 10 ++
 virt-sandbox-image/sources/Source.py   |  4 
 virt-sandbox-image/virt-sandbox-image.py   | 19 +++
 3 files changed, 33 insertions(+)

diff --git a/virt-sandbox-image/sources/DockerSource.py 
b/virt-sandbox-image/sources/DockerSource.py
index 44bc238..54b68b9 100644
--- a/virt-sandbox-image/sources/DockerSource.py
+++ b/virt-sandbox-image/sources/DockerSource.py
@@ -48,6 +48,12 @@ class DockerConfParser():
   for key,value in volumes.iteritems():
 volumelist.append(key)
 return volumelist
+def getEnvs(self):
+lst = self.json_data['container_config']['Env']
+if lst is not None and isinstance(lst,list):
+  return lst
+else:
+  return []
 
 class DockerSource(Source):
 default_index_server = "index.docker.io"
@@ -411,5 +417,9 @@ class DockerSource(Source):
 configParser = DockerConfParser(configfile)
 return configParser.getVolumes()
 
+def get_environment(self,configfile):
+configParser = DockerConfParser(configfile)
+return configParser.getEnvs()
+
 def debug(msg):
 sys.stderr.write(msg)
diff --git a/virt-sandbox-image/sources/Source.py 
b/virt-sandbox-image/sources/Source.py
index 6898c15..ad82986 100644
--- a/virt-sandbox-image/sources/Source.py
+++ b/virt-sandbox-image/sources/Source.py
@@ -53,3 +53,7 @@ class Source():
 @abstractmethod
 def get_volume(self,**args):
   pass
+
+@abstractmethod
+def get_env(self,**args):
+  pass
diff --git a/virt-sandbox-image/virt-sandbox-image.py 
b/virt-sandbox-image/virt-sandbox-image.py
index b12b99b..25c8dfa 100755
--- a/virt-sandbox-image/virt-sandbox-image.py
+++ b/virt-sandbox-image/virt-sandbox-image.py
@@ -118,10 +118,12 @@ def run(args):
 cmd.append("-c")
 cmd.append(args.connect)
 params = ['-m','host-image:/=%s,format=%s' %(diskfile,format)]
+
 networkArgs = args.network
 if networkArgs is not None:
 params.append('-N')
 params.append(networkArgs)
+
 allVolumes = source.get_volume(configfile)
 volumeArgs = args.volume
 if volumeArgs is not None:
@@ -141,6 +143,20 @@ def run(args):
 pass
 params.append("--mount")
 params.append("host-bind:%s=%s" %(guestPath,hostPath))
+
+allEnvs = source.get_environment(configfile)
+envArgs = args.env
+if envArgs is not None:
+allEnvs = allEnvs + envArgs
+for env in allEnvs:
+envsplit = env.split("=")
+envlen = len(envsplit)
+if envlen == 2:
+params.append("--env")
+params.append(env)
+else:
+pass
+
 params.append('--')
 params.append(commandToRun)
 cmd = cmd + params
@@ -215,6 +231,9 @@ def gen_run_args(subparser):
 help=_("Network params for running template"))
 parser.add_argument("-v","--volume",action="append",
 help=_("Volume params for running template"))
+parser.add_argument("-e","--env",action="append",
+help=_("Environment params for running template"))
+
 parser.set_defaults(func=run)
 
 if __name__ == '__main__':
-- 
2.1.0

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

[libvirt] [sandbox PATCH v2 07/19] Image: Add get_command function to Source

[Eren Yagdiran]

Provide a way to know how a template can be started depending on the used source
DockerSource will need to parse the topmost config file in order to find the 
igniter command
---
 virt-sandbox-image/sources/DockerSource.py | 14 ++
 virt-sandbox-image/sources/Source.py   |  4 
 2 files changed, 18 insertions(+)

diff --git a/virt-sandbox-image/sources/DockerSource.py 
b/virt-sandbox-image/sources/DockerSource.py
index 03f50aa..2f4646f 100644
--- a/virt-sandbox-image/sources/DockerSource.py
+++ b/virt-sandbox-image/sources/DockerSource.py
@@ -32,6 +32,15 @@ import os
 import subprocess
 import shutil
 
+class DockerConfParser():
+
+def __init__(self,jsonfile):
+with open(jsonfile) as json_file:
+self.json_data = json.load(json_file)
+def getRunCommand(self):
+cmd = self.json_data['container_config']['Cmd'][2]
+return cmd[cmd.index('"') + 1:cmd.rindex('"')]
+
 class DockerSource(Source):
 default_index_server = "index.docker.io"
 default_template_dir = "/var/lib/libvirt/templates"
@@ -376,5 +385,10 @@ class DockerSource(Source):
 parent = None
 imagetagid = parent
 
+def get_command(self,configfile):
+configParser = DockerConfParser(configfile)
+commandToRun = configParser.getRunCommand()
+return commandToRun
+
 def debug(msg):
 sys.stderr.write(msg)
diff --git a/virt-sandbox-image/sources/Source.py 
b/virt-sandbox-image/sources/Source.py
index ceda58f..01f8560 100644
--- a/virt-sandbox-image/sources/Source.py
+++ b/virt-sandbox-image/sources/Source.py
@@ -41,3 +41,7 @@ class Source():
 @abstractmethod
 def delete_template(self,**args):
   pass
+
+@abstractmethod
+def get_command(self,**args):
+  pass
-- 
2.1.0

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

[libvirt] [sandbox PATCH v2 06/19] Image: Add delete function

[Eren Yagdiran]

Refactoring delete function from virt-sandbox-image to DockerSource. Delete 
function
can delete templates by name.
---
 virt-sandbox-image/sources/DockerSource.py | 53 +++
 virt-sandbox-image/sources/Source.py   |  4 ++
 virt-sandbox-image/virt-sandbox-image.py   | 59 --
 3 files changed, 65 insertions(+), 51 deletions(-)

diff --git a/virt-sandbox-image/sources/DockerSource.py 
b/virt-sandbox-image/sources/DockerSource.py
index 9cd0080..03f50aa 100644
--- a/virt-sandbox-image/sources/DockerSource.py
+++ b/virt-sandbox-image/sources/DockerSource.py
@@ -323,5 +323,58 @@ class DockerSource(Source):
 cmd = cmd + params
 subprocess.call(cmd)
 
+def delete_template(self,**args):
+imageusage = {}
+imageparent = {}
+imagenames = {}
+name = args['name']
+destdir = args['imagepath']
+destdir = destdir if destdir is not None else default_template_dir
+imagedirs = os.listdir(destdir)
+for imagetagid in imagedirs:
+indexfile = destdir + "/" + imagetagid + "/index.json"
+if os.path.exists(indexfile):
+with open(indexfile,"r") as f:
+index = json.load(f)
+imagenames[index["name"]] = imagetagid
+jsonfile = destdir + "/" + imagetagid + "/template.json"
+if os.path.exists(jsonfile):
+with open(jsonfile,"r") as f:
+template = json.load(f)
+
+parent = template.get("parent",None)
+if parent:
+if parent not in imageusage:
+imageusage[parent] = []
+imageusage[parent].append(imagetagid)
+imageparent[imagetagid] = parent
+
+
+if not name in imagenames:
+raise ValueError(["Image %s does not exist locally" %name])
+
+imagetagid = imagenames[name]
+while imagetagid != None:
+debug("Remove %s\n" % imagetagid)
+parent = imageparent.get(imagetagid,None)
+
+indexfile = destdir + "/" + imagetagid + "/index.json"
+if os.path.exists(indexfile):
+   os.remove(indexfile)
+jsonfile = destdir + "/" + imagetagid + "/template.json"
+if os.path.exists(jsonfile):
+os.remove(jsonfile)
+datafile = destdir + "/" + imagetagid + "/template.tar.gz"
+if os.path.exists(datafile):
+os.remove(datafile)
+imagedir = destdir + "/" + imagetagid
+shutil.rmtree(imagedir)
+
+if parent:
+if len(imageusage[parent]) != 1:
+debug("Parent %s is shared\n" % parent)
+parent = None
+imagetagid = parent
+
 def debug(msg):
 sys.stderr.write(msg)
diff --git a/virt-sandbox-image/sources/Source.py 
b/virt-sandbox-image/sources/Source.py
index 1a63428..ceda58f 100644
--- a/virt-sandbox-image/sources/Source.py
+++ b/virt-sandbox-image/sources/Source.py
@@ -37,3 +37,7 @@ class Source():
 @abstractmethod
 def create_template(self,**args):
   pass
+
+@abstractmethod
+def delete_template(self,**args):
+  pass
diff --git a/virt-sandbox-image/virt-sandbox-image.py 
b/virt-sandbox-image/virt-sandbox-image.py
index 6f65445..ea7ab02 100755
--- a/virt-sandbox-image/virt-sandbox-image.py
+++ b/virt-sandbox-image/virt-sandbox-image.py
@@ -67,55 +67,6 @@ def debug(msg):
 def info(msg):
 sys.stdout.write(msg)
 
-def delete_template(name, destdir):
-imageusage = {}
-imageparent = {}
-imagenames = {}
-imagedirs = os.listdir(destdir)
-for imagetagid in imagedirs:
-indexfile = destdir + "/" + imagetagid + "/index.json"
-if os.path.exists(indexfile):
-with open(indexfile, "r") as f:
-index = json.load(f)
-imagenames[index["name"]] = imagetagid
-jsonfile = destdir + "/" + imagetagid + "/template.json"
-if os.path.exists(jsonfile):
-with open(jsonfile, "r") as f:
-template = json.load(f)
-
-parent = template.get("parent", None)
-if parent:
-if parent not in imageusage:
-imageusage[parent] = []
-imageusage[parent].append(imagetagid)
-imageparent[imagetagid] = parent
-
-if not name in imagenames:
-raise ValueError(["Image %s does not exist locally" % name])
-
-imagetagid = imagenames[name]
-while imagetagid != None:
-debug("Remove %s\n" %  imagetagid)
-parent = imageparent.get(imagetagid, None)
-
-indexfile = destdir + "/" + imagetagid + "/index.json"
-if os.path.exists(indexfile):
-os.remove(indexfile)
-jsonfile = destdir + "/" + imagetagid + "/template.json"
-if os.path.exists(jsonfile):
-os.remove(jsonfile)
-

[libvirt] [sandbox PATCH v2 16/19] Add environment parameter to virt-sandbox

[Eren Yagdiran]

Allow users to add custom environment variables to their sandbox.
---
 bin/virt-sandbox.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c
index 195515f..e90b698 100644
--- a/bin/virt-sandbox.c
+++ b/bin/virt-sandbox.c
@@ -64,6 +64,7 @@ int main(int argc, char **argv) {
 GError *error = NULL;
 gchar *name = NULL;
 gchar **disks = NULL;
+gchar **envs = NULL;
 gchar **mounts = NULL;
 gchar **includes = NULL;
 gchar *includefile = NULL;
@@ -95,6 +96,8 @@ int main(int argc, char **argv) {
   N_("root directory of the sandbox"), "DIR" },
 { "disk", ' ', 0, G_OPTION_ARG_STRING_ARRAY, &disks,
   N_("add a disk in the guest"), "TYPE:TAGNAME=SOURCE,format=FORMAT" },
+{ "env", 'e', 0, G_OPTION_ARG_STRING_ARRAY, &envs,
+  N_("add a environment variable for the sandbox"), "KEY=VALUE" },
 { "mount", 'm', 0, G_OPTION_ARG_STRING_ARRAY, &mounts,
   N_("mount a filesystem in the guest"), "TYPE:TARGET=SOURCE" },
 { "include", 'i', 0, G_OPTION_ARG_STRING_ARRAY, &includes,
@@ -185,6 +188,13 @@ int main(int argc, char **argv) {
 gvir_sandbox_config_set_username(cfg, "root");
 }
 
+if (envs &&
+!gvir_sandbox_config_add_env_strv(cfg, envs, &error)) {
+g_printerr(_("Unable to parse custom environment variables: %s\n"),
+   error && error->message ? error->message : _("Unknown 
failure"));
+goto cleanup;
+}
+
 if (disks &&
 !gvir_sandbox_config_add_disk_strv(cfg, disks, &error)) {
 g_printerr(_("Unable to parse disks: %s\n"),
@@ -329,6 +339,10 @@ inheriting the host's root filesystem.
 NB. C must contain a matching install of the libvirt-sandbox
 package. This restriction may be lifted in a future version.
 
+=item B<--env key=value>
+
+Sets up a custom environment variable on a running sandbox.
+
 =item B<--disk TYPE:TAGNAME=SOURCE,format=FORMAT>
 
 Sets up a disk inside the sandbox by using B with a symlink named as 
B
-- 
2.1.0

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

[libvirt] [sandbox PATCH v2 05/19] Image: Refactor create function

[Eren Yagdiran]

Move the docker-related code to the DockerSource and use
the Source mechanism
---
 virt-sandbox-image/sources/DockerSource.py | 100 +
 virt-sandbox-image/sources/Source.py   |   4 ++
 virt-sandbox-image/virt-sandbox-image.py   |  70 
 3 files changed, 118 insertions(+), 56 deletions(-)

diff --git a/virt-sandbox-image/sources/DockerSource.py 
b/virt-sandbox-image/sources/DockerSource.py
index cf81ffe..9cd0080 100644
--- a/virt-sandbox-image/sources/DockerSource.py
+++ b/virt-sandbox-image/sources/DockerSource.py
@@ -223,5 +223,105 @@ class DockerSource(Source):
 debug("FAIL %s\n" % str(e))
 raise
 
+def create_template(self,**args):
+name = args['name']
+connect = args['connect']
+path = args['imagepath']
+path = path if path is not None else self.default_image_path
+format = args['format']
+format = format if format is not None else self.default_disk_format
+
+self._create_template(name,
+   connect,
+   path,
+   format,
+   path)
+
+def _create_template(self,name,connect,image_path,format,destdir):
+self._check_disk_format(format)
+imagelist = self._get_image_list(name,destdir)
+imagelist.reverse()
+
+parentImage = None
+for imagetagid in imagelist:
+templateImage = destdir + "/" + imagetagid + "/template." + format
+cmd = ["qemu-img","create","-f","qcow2"]
+if parentImage is not None:
+cmd.append("-o")
+cmd.append("backing_fmt=qcow2,backing_file=%s" % parentImage)
+cmd.append(templateImage)
+if parentImage is None:
+cmd.append("10G")
+subprocess.call(cmd)
+
+if parentImage is None:
+self._format_disk(templateImage,format,connect)
+
+self._extract_tarballs(destdir + "/" + imagetagid + 
"/template.",format,connect)
+parentImage = templateImage
+
+
+def _check_disk_format(self,format):
+supportedFormats = ['qcow2']
+if not format in supportedFormats:
+raise ValueError(["Unsupported image format %s" % format])
+
+def _get_image_list(self,name,destdir):
+imageparent = {}
+imagenames = {}
+imagedirs = os.listdir(destdir)
+for imagetagid in imagedirs:
+indexfile = destdir + "/" + imagetagid + "/index.json"
+if os.path.exists(indexfile):
+with open(indexfile,"r") as f:
+index = json.load(f)
+imagenames[index["name"]] = imagetagid
+jsonfile = destdir + "/" + imagetagid + "/template.json"
+if os.path.exists(jsonfile):
+with open(jsonfile,"r") as f:
+template = json.load(f)
+parent = template.get("parent",None)
+if parent:
+imageparent[imagetagid] = parent
+if not name in imagenames:
+raise ValueError(["Image %s does not exist locally" %name])
+imagetagid = imagenames[name]
+imagelist = []
+while imagetagid != None:
+imagelist.append(imagetagid)
+parent = imageparent.get(imagetagid,None)
+imagetagid = parent
+return imagelist
+
+def _format_disk(self,disk,format,connect):
+cmd = ['virt-sandbox']
+if connect is not None:
+cmd.append("-c")
+cmd.append(connect)
+params = ['--disk=file:disk_image=%s,format=%s' %(disk,format),
+  '/sbin/mkfs.ext3',
+  '/dev/disk/by-tag/disk_image']
+cmd = cmd + params
+subprocess.call(cmd)
+
+def _extract_tarballs(self,directory,format,connect):
+tempdir = "/mnt"
+tarfile = directory + "tar.gz"
+diskfile = directory + "qcow2"
+cmd = ['virt-sandbox']
+if connect is not None:
+cmd.append("-c")
+cmd.append(connect)
+params = ['-m',
+  'host-image:/mnt=%s,format=%s' %(diskfile,format),
+  '--',
+  '/bin/tar',
+  'zxvf',
+  '%s' %tarfile,
+  '-C',
+  '/mnt']
+cmd = cmd + params
+subprocess.call(cmd)
+
 def debug(msg):
 sys.stderr.write(msg)
diff --git a/virt-sandbox-image/sources/Source.py 
b/virt-sandbox-image/sources/Source.py
index 08bf335..1a63428 100644
--- a/virt-sandbox-image/sources/Source.py
+++ b/virt-sandbox-image/sources/Source.py
@@ -33,3 +33,7 @@ class Source():
 @abstractmethod
 def download_template(self,**args):
 pass
+
+@abstractmethod
+def create_template(self,**args):
+  pass
diff --git a/virt-sandbox-image/virt-sandbox-image.py 
b/virt-sandbox-ima

[libvirt] [sandbox PATCH v2 18/19] Add testcase for custom environment variables

[Eren Yagdiran]

"make check" now includes testcase for environment variables
---
 libvirt-sandbox/tests/test-config.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/libvirt-sandbox/tests/test-config.c 
b/libvirt-sandbox/tests/test-config.c
index da05187..ac10bab 100644
--- a/libvirt-sandbox/tests/test-config.c
+++ b/libvirt-sandbox/tests/test-config.c
@@ -58,6 +58,13 @@ int main(int argc, char **argv)
 "host-bind:/tmp=",
 NULL
 };
+
+ const gchar *envs[] = {
+"key1=val1",
+"key2=val2",
+NULL
+};
+
 const gchar *disks[] = {
 "file:dbdata=/tmp/img.blah,format=qcow2",
 "file:cache=/tmp/img.qcow2",
@@ -103,6 +110,9 @@ int main(int argc, char **argv)
 if (!gvir_sandbox_config_add_mount_strv(cfg1, (gchar**)mounts, &err))
 goto cleanup;
 
+if (!gvir_sandbox_config_add_env_strv(cfg1, (gchar**)envs, &err))
+goto cleanup;
+
 if (!gvir_sandbox_config_add_disk_strv(cfg1, (gchar**)disks, &err))
 goto cleanup;
 
-- 
2.1.0

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

[libvirt] [sandbox PATCH v2 04/19] Image: Add download function

[Eren Yagdiran]

Refactor download function from virt-sandbox-image to use
the newly introduced Source abstract class. The docker-specific
download code is moved to a new DockerSource class.
---
 virt-sandbox-image/Makefile.am |   1 +
 virt-sandbox-image/sources/DockerSource.py | 227 +
 virt-sandbox-image/sources/Source.py   |   4 +
 virt-sandbox-image/virt-sandbox-image.py   | 199 -
 4 files changed, 259 insertions(+), 172 deletions(-)
 create mode 100644 virt-sandbox-image/sources/DockerSource.py

diff --git a/virt-sandbox-image/Makefile.am b/virt-sandbox-image/Makefile.am
index 5ab4d2e..8188c80 100644
--- a/virt-sandbox-image/Makefile.am
+++ b/virt-sandbox-image/Makefile.am
@@ -8,6 +8,7 @@ install-data-local:
$(INSTALL) -m 0755 $(srcdir)/virt-sandbox-image.py 
$(DESTDIR)$(pkgpythondir)
$(INSTALL) -m 0644 $(srcdir)/sources/__init__.py 
$(DESTDIR)$(pkgpythondir)/sources
$(INSTALL) -m 0644 $(srcdir)/sources/Source.py 
$(DESTDIR)$(pkgpythondir)/sources
+   $(INSTALL) -m 0644 $(srcdir)/sources/DockerSource.py 
$(DESTDIR)$(pkgpythondir)/sources
 
 uninstall-local:
rm -f $(DESTDIR)$(pkgpythondir)
diff --git a/virt-sandbox-image/sources/DockerSource.py 
b/virt-sandbox-image/sources/DockerSource.py
new file mode 100644
index 000..cf81ffe
--- /dev/null
+++ b/virt-sandbox-image/sources/DockerSource.py
@@ -0,0 +1,227 @@
+'''
+*
+* libvirt-sandbox-config-diskaccess.h: libvirt sandbox configuration
+*
+* Copyright (C) 2015 Universitat Polit??cnica de Catalunya.
+*
+* This library is free software; you can redistribute it and/or
+* modify it under the terms of the GNU Lesser General Public
+* License as published by the Free Software Foundation; either
+* version 2.1 of the License, or (at your option) any later version.
+*
+* This library is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+* Lesser General Public License for more details.
+*
+* You should have received a copy of the GNU Lesser General Public
+* License along with this library; if not, write to the Free Software
+* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  
USA
+*
+* Author: Eren Yagdiran 
+*
+'''
+#!/usr/bin/python
+
+from Source import Source
+import urllib2
+import sys
+import json
+import traceback
+import os
+import subprocess
+import shutil
+
+class DockerSource(Source):
+default_index_server = "index.docker.io"
+default_template_dir = "/var/lib/libvirt/templates"
+default_image_path = "/var/lib/libvirt/templates"
+default_disk_format = "qcow2"
+
+www_auth_username = None
+www_auth_password = None
+
+def 
__init__(self,server="index.docker.io",destdir="/var/lib/libvirt/templates"):
+self.default_index_server = server
+self.default_template_dir = destdir
+
+def _check_cert_validate(self):
+major = sys.version_info.major
+SSL_WARNING = "SSL certificates couldn't be validated by default. You 
need to have 2.7.9/3.4.3 or higher"
+SSL_WARNING +="\nSee https://bugs.python.org/issue22417";
+py2_7_9_hexversion = 34015728
+py3_4_3_hexversion = 50594800
+if  (major == 2 and sys.hexversion < py2_7_9_hexversion) or (major == 
3 and sys.hexversion < py3_4_3_hexversion):
+print SSL_WARNING
+
+def _check_dir_writable(self,path):
+if not os.access(path,os.W_OK):
+raise ValueError(["%s is not writable for saving template data" 
%path])
+
+def download_template(self,**args):
+name = args['name']
+registry = args['registry'] if args['registry'] is not None else 
self.default_index_server
+username = args['username']
+password = args['password']
+templatedir = args['templatedir'] if args['templatedir'] is not None 
else self.default_template_dir
+self._download_template(name,registry,username,password,templatedir)
+
+def _download_template(self,name, server,username,password,destdir):
+
+if username is not None:
+self.www_auth_username = username
+self.www_auth_password = password
+
+self._check_cert_validate()
+self._check_dir_writable(destdir)
+tag = "latest"
+offset = name.find(':')
+if offset != -1:
+tag = name[offset + 1:]
+name = name[0:offset]
+try:
+(data, res) = self._get_json(server, "/v1/repositories/" + name + 
"/images",
+   {"X-Docker-Token": "true"})
+except urllib2.HTTPError, e:
+raise ValueError(["Image '%s' does not exist" % name])
+
+registryserver = res.info().getheader('X-Docker-Endpoints')
+token = res.info().getheader('X-Docker-Token')
+checksums = {}
+for layer in data:
+pass
+(data, res) = 

[libvirt] [sandbox PATCH v2 02/19] Fix virt-sandbox-image

[Eren Yagdiran]

Authentication fix for Docker REST API.
---
 virt-sandbox-image/virt-sandbox-image.py | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/virt-sandbox-image/virt-sandbox-image.py 
b/virt-sandbox-image/virt-sandbox-image.py
index 4f5443b..a9cb0ff 100644
--- a/virt-sandbox-image/virt-sandbox-image.py
+++ b/virt-sandbox-image/virt-sandbox-image.py
@@ -1,8 +1,10 @@
 #!/usr/bin/python -Es
 #
 # Authors: Daniel P. Berrange 
+#  Eren Yagdiran 
 #
 # Copyright (C) 2013 Red Hat, Inc.
+# Copyright (C) 2015 Universitat Polit??cnica de Catalunya.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -166,7 +168,7 @@ def download_template(name, server, destdir):
 # or more parents, in a linear stack. Here we are getting the list
 # of layers for the image with the tag we used.
 (data, res) = get_json(registryserver, "/v1/images/" + imagetagid + 
"/ancestry",
-   { "Cookie": cookie })
+   { "Authorization": "Token " + token })
 
 if data[0] != imagetagid:
 raise ValueError(["Expected first layer id '%s' to match image id 
'%s'",
@@ -188,9 +190,9 @@ def download_template(name, server, destdir):
 if not os.path.exists(jsonfile) or not os.path.exists(datafile):
 # The '/json' URL gives us some metadata about the layer
 res = save_data(registryserver, "/v1/images/" + layerid + 
"/json",
-{ "Cookie": cookie }, jsonfile)
+{ "Authorization": "Token " + token }, 
jsonfile)
 createdFiles.append(jsonfile)
-layersize = int(res.info().getheader("x-docker-size"))
+layersize = int(res.info().getheader("Content-Length"))
 
 datacsum = None
 if layerid in checksums:
@@ -199,7 +201,7 @@ def download_template(name, server, destdir):
 # and the '/layer' URL is the actual payload, provided
 # as a tar.gz archive
 save_data(registryserver, "/v1/images/" + layerid + "/layer",
-  { "Cookie": cookie }, datafile, datacsum, layersize)
+  { "Authorization": "Token " + token }, datafile, 
datacsum, layersize)
 createdFiles.append(datafile)
 
 # Strangely the 'json' data for a layer doesn't include
-- 
2.1.0

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

[libvirt] [sandbox PATCH v2 13/19] Image: Add Volume Support

[Eren Yagdiran]

Volumes let user to map host-paths into guest. Docker containers need volumes 
because its
filesystem read-only by default.
---
 virt-sandbox-image/sources/DockerSource.py | 12 
 virt-sandbox-image/sources/Source.py   |  4 
 virt-sandbox-image/virt-sandbox-image.py   | 22 ++
 3 files changed, 38 insertions(+)

diff --git a/virt-sandbox-image/sources/DockerSource.py 
b/virt-sandbox-image/sources/DockerSource.py
index 74feb3e..44bc238 100644
--- a/virt-sandbox-image/sources/DockerSource.py
+++ b/virt-sandbox-image/sources/DockerSource.py
@@ -31,6 +31,7 @@ import traceback
 import os
 import subprocess
 import shutil
+import collections
 
 class DockerConfParser():
 
@@ -40,6 +41,13 @@ class DockerConfParser():
 def getRunCommand(self):
 cmd = self.json_data['container_config']['Cmd'][2]
 return cmd[cmd.index('"') + 1:cmd.rindex('"')]
+def getVolumes(self):
+volumes = self.json_data['container_config']['Volumes']
+volumelist = []
+if isinstance(volumes,collections.Iterable):
+  for key,value in volumes.iteritems():
+volumelist.append(key)
+return volumelist
 
 class DockerSource(Source):
 default_index_server = "index.docker.io"
@@ -399,5 +407,9 @@ class DockerSource(Source):
 commandToRun = configParser.getRunCommand()
 return commandToRun
 
+def get_volume(self,configfile):
+configParser = DockerConfParser(configfile)
+return configParser.getVolumes()
+
 def debug(msg):
 sys.stderr.write(msg)
diff --git a/virt-sandbox-image/sources/Source.py 
b/virt-sandbox-image/sources/Source.py
index 6e2f5fb..6898c15 100644
--- a/virt-sandbox-image/sources/Source.py
+++ b/virt-sandbox-image/sources/Source.py
@@ -49,3 +49,7 @@ class Source():
 @abstractmethod
 def get_disk(self,**args):
   pass
+
+@abstractmethod
+def get_volume(self,**args):
+  pass
diff --git a/virt-sandbox-image/virt-sandbox-image.py 
b/virt-sandbox-image/virt-sandbox-image.py
index 5fc7f44..b12b99b 100755
--- a/virt-sandbox-image/virt-sandbox-image.py
+++ b/virt-sandbox-image/virt-sandbox-image.py
@@ -103,6 +103,7 @@ def check_connect(connectstr):
 
 def run(args):
 try:
+default_dir = "/var/lib/libvirt/storage"
 if args.connect is not None:
 check_connect(args.connect)
 source = dynamic_source_loader(args.source)
@@ -121,6 +122,25 @@ def run(args):
 if networkArgs is not None:
 params.append('-N')
 params.append(networkArgs)
+allVolumes = source.get_volume(configfile)
+volumeArgs = args.volume
+if volumeArgs is not None:
+allVolumes = allVolumes + volumeArgs
+for volume in allVolumes:
+volumeSplit = volume.split(":")
+volumelen = len(volumeSplit)
+if volumelen == 2:
+hostPath = volumeSplit[0]
+guestPath = volumeSplit[1]
+elif volumelen == 1:
+guestPath = volumeSplit[0]
+hostPath = default_dir + guestPath
+if not os.path.exists(hostPath):
+os.makedirs(hostPath)
+else:
+pass
+params.append("--mount")
+params.append("host-bind:%s=%s" %(guestPath,hostPath))
 params.append('--')
 params.append(commandToRun)
 cmd = cmd + params
@@ -193,6 +213,8 @@ def gen_run_args(subparser):
 help=_("Igniter command for image"))
 parser.add_argument("-n","--network",
 help=_("Network params for running template"))
+parser.add_argument("-v","--volume",action="append",
+help=_("Volume params for running template"))
 parser.set_defaults(func=run)
 
 if __name__ == '__main__':
-- 
2.1.0

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

[libvirt] [sandbox PATCH v2 14/19] Image: man file for virt-sandbox-image

[Eren Yagdiran]

---
 bin/Makefile.am|   5 ++
 bin/virt-sandbox-image.pod | 172 +
 2 files changed, 177 insertions(+)
 create mode 100644 bin/virt-sandbox-image.pod

diff --git a/bin/Makefile.am b/bin/Makefile.am
index df4c7dc..5d7ff8a 100644
--- a/bin/Makefile.am
+++ b/bin/Makefile.am
@@ -23,6 +23,7 @@ POD_FILES = \
virt-sandbox-service-delete.pod \
virt-sandbox-service-reload.pod \
virt-sandbox-service-upgrade.pod \
+   virt-sandbox-image.pod \
$(NULL)
 EXTRA_DIST = virt-sandbox-service \
  virt-sandbox-image.in \
@@ -40,6 +41,7 @@ man1_MANS = \
virt-sandbox-service-delete.1 \
virt-sandbox-service-reload.1 \
virt-sandbox-service-upgrade.1 \
+   virt-sandbox-image.1 \
$(NULL)
 
 POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)"
@@ -71,6 +73,9 @@ virt-sandbox-service-reload.1: 
virt-sandbox-service-reload.pod Makefile
 virt-sandbox-service-upgrade.1: virt-sandbox-service-upgrade.pod Makefile
$(AM_V_GEN)$(POD2MAN) $< $(srcdir)/$@
 
+virt-sandbox-image.1: virt-sandbox-image.pod Makefile
+   $(AM_V_GEN)$(POD2MAN) $< $(srcdir)/$@
+
 CLEANFILES = $(man1_MANS) \
  virt-sandbox-image
 
diff --git a/bin/virt-sandbox-image.pod b/bin/virt-sandbox-image.pod
new file mode 100644
index 000..a85fcd9
--- /dev/null
+++ b/bin/virt-sandbox-image.pod
@@ -0,0 +1,172 @@
+=head1 NAME
+
+virt-sandbox-image - Sandbox Container Image Tool
+
+=head1 SYNOPSIS
+
+  {download,create,run,delete}
+
+  commands:
+
+download  Download template data
+
+createCreate image from template data
+
+run   Run an already built image
+
+deleteDelete template data
+
+=head1 DESCRIPTION
+
+virt-sandbox-image.py is a sandbox container image tool developed in python.
+This tool can download,create,run and delete templates which are provided by
+different sources. This tool comes with Docker source by default. Other sources
+can be implemented by extending source class
+
+=head1 OPTIONS
+
+=over 4
+
+=item B
+
+Download a template by given name with a specified source.
+
+=over 6
+
+=item B
+
+Template name to download
+
+=item B<-s or --source>
+
+Source parameter will try load source module under sources/ directory. Each 
source has to implement Source.py base class and register itself with a proper 
name
+Default source is Docker.
+
+=item B<-r or --registry>
+
+Custom registry url for downloading data. This might need privileged 
credentials which can be specified by --username and --password parameters.
+
+=item B<-u or --username>
+
+Username for custom registry authentication
+
+=item B<-p or --password>
+
+Password for custom registry authentication
+
+=item B<-t or --template-dir>
+
+Custom directory for downloading template data
+
+=back
+
+=item B
+
+Create already downloaded template into image with given format.
+
+=over 5
+
+=item B
+
+Template name to download.
+
+=item B
+
+Image path where template image will be stored.
+
+=item B
+
+Image format e.g qcow2
+
+=item B<-s or --source>
+
+Source parameter will try load source module under sources/ directory. Each 
source has to implement Source.py base class and register itself with a proper 
name
+Default source is Docker.
+
+=item B<-d or --driver>
+
+Driver parameter can be specified with only supported driver by 
libvirt-sandbox. These are lxc:///, qemu:///session, qemu:///system.
+
+=back
+
+=item B
+
+Run already built image.
+
+=over 6
+
+=item B
+
+Template name to download.
+
+=item B
+
+Image path where template image will be stored.
+
+=item B<-c or --command>
+
+Command for running a image. If it is not specified, virt-sandbox-image will 
try to load command params from specified source. E.g /bin/bash
+
+=item B<-n or --network>
+
+Network params will be passed directly to the virt-sandbox. More information 
about network params, See C
+
+=item B<-v or --volume>
+
+Volume params are for binding host-paths to the guest. E.g -v /home:/home will 
map /home directory from host to the guest.
+
+=item B<-d or --driver>
+
+Driver parameter can be specified with only supported driver by 
libvirt-sandbox. These are lxc:///, qemu:///session, qemu:///system.
+
+=back
+
+=item B
+
+Delete downloaded template data and its built image.
+
+=over 3
+
+=item B
+
+Template name to delete.
+
+=item B
+
+Image path where template data or image stays.
+
+=item B<-s or --source>
+
+Source parameter will try load source module under sources/ directory. Each 
source has to implement Source.py base class and register itself with a proper 
name
+Default source is Docker.
+
+=back
+
+=back
+
+=head1 SEE ALSO
+
+C
+
+=head1 FILES
+
+Container content will be stored in subdirectories of
+/var/lib/libvirt/templates, by default.
+
+=head1 AUTHORS
+
+Daniel P. Berrange 
+
+Eren Yagdiran 
+
+=head1 COPYRIGHT
+
+Copyright (C) 2013 Red Hat, Inc.
+Copyright (C) 2015 U

[libvirt] [sandbox PATCH v2 17/19] Common-init: Exporting custom environment variables

[Eren Yagdiran]

Common-init reads config file and export custom environment
variables from config file and apply them to the running sandbox.
---
 libvirt-sandbox/libvirt-sandbox-init-common.c | 30 +++
 1 file changed, 30 insertions(+)

diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c 
b/libvirt-sandbox/libvirt-sandbox-init-common.c
index d35f760..0b0aa98 100644
--- a/libvirt-sandbox/libvirt-sandbox-init-common.c
+++ b/libvirt-sandbox/libvirt-sandbox-init-common.c
@@ -336,6 +336,33 @@ static gboolean setup_network(GVirSandboxConfig *config, 
GError **error)
 }
 
 
+static gboolean setup_custom_env(GVirSandboxConfig *config, GError **error)
+{
+GList *envs, *tmp;
+gboolean ret = FALSE;
+gchar *key = NULL;
+gchar *value = NULL;
+
+envs = tmp = gvir_sandbox_config_get_envs(config);
+
+while (tmp) {
+GVirSandboxConfigEnv *env = GVIR_SANDBOX_CONFIG_ENV(tmp->data);
+key = g_strdup(gvir_sandbox_config_env_get_key(env));
+value = g_strdup(gvir_sandbox_config_env_get_value(env));
+if(setenv(key,value,1)!=0)
+goto cleanup;
+g_free(key);
+g_free(value);
+tmp = tmp->next;
+}
+
+ret = TRUE;
+ cleanup:
+g_list_foreach(envs, (GFunc)g_object_unref, NULL);
+g_list_free(envs);
+return ret;
+}
+
 static int change_user(const gchar *user,
uid_t uid,
gid_t gid,
@@ -1262,6 +1289,9 @@ int main(int argc, char **argv) {
 if (!setup_disk_tags())
 exit(EXIT_FAILURE);
 
+if (!setup_custom_env(config, &error))
+goto error;
+
 if (!setup_network(config, &error))
 goto error;
 
-- 
2.1.0

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

[libvirt] [sandbox PATCH v2 11/19] Image: Add run function

[Eren Yagdiran]

Run an already-built template
If there is no execution command specified by user, source.get_command will
find the command to invoke
---
 virt-sandbox-image/virt-sandbox-image.py | 24 
 1 file changed, 24 insertions(+)

diff --git a/virt-sandbox-image/virt-sandbox-image.py 
b/virt-sandbox-image/virt-sandbox-image.py
index 4c19fa8..e20ce22 100755
--- a/virt-sandbox-image/virt-sandbox-image.py
+++ b/virt-sandbox-image/virt-sandbox-image.py
@@ -101,6 +101,30 @@ def check_connect(connectstr):
 raise ValueError("%s is not supported by Virt-sandbox" %connectstr)
 return True
 
+def run(args):
+try:
+if args.connect is not None:
+check_connect(args.connect)
+source = dynamic_source_loader(args.source)
+diskfile,configfile = 
source.get_disk(name=args.name,path=args.imagepath)
+
+format = "qcow2"
+commandToRun = args.igniter
+if commandToRun is None:
+commandToRun = source.get_command(configfile)
+cmd = ['virt-sandbox']
+if args.connect is not None:
+cmd.append("-c")
+cmd.append(args.connect)
+params = ['-m','host-image:/=%s,format=%s' %(diskfile,format),
+   '--',
+   commandToRun]
+cmd = cmd + params
+subprocess.call(cmd)
+
+except Exception,e:
+print "Run Error %s" % str(e)
+
 def requires_name(parser):
 parser.add_argument("name",
 help=_("name of the template"))
-- 
2.1.0

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

[libvirt] [sandbox PATCH v2 09/19] Image: Add check_connect function

[Eren Yagdiran]

Check if user-specified connect argument is valid
---
 virt-sandbox-image/virt-sandbox-image.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/virt-sandbox-image/virt-sandbox-image.py 
b/virt-sandbox-image/virt-sandbox-image.py
index feee849..4c19fa8 100755
--- a/virt-sandbox-image/virt-sandbox-image.py
+++ b/virt-sandbox-image/virt-sandbox-image.py
@@ -95,6 +95,12 @@ def create(args):
 except Exception,e:
 print "Create Error %s" % str(e)
 
+def check_connect(connectstr):
+supportedDrivers = ['lxc:///','qemu:///session','qemu:///system']
+if not connectstr in supportedDrivers:
+raise ValueError("%s is not supported by Virt-sandbox" %connectstr)
+return True
+
 def requires_name(parser):
 parser.add_argument("name",
 help=_("name of the template"))
-- 
2.1.0

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

[libvirt] [sandbox PATCH v2 08/19] Image: Add run args

[Eren Yagdiran]

Commandline parameters for running a template
---
 virt-sandbox-image/virt-sandbox-image.py | 13 +
 1 file changed, 13 insertions(+)

diff --git a/virt-sandbox-image/virt-sandbox-image.py 
b/virt-sandbox-image/virt-sandbox-image.py
index ea7ab02..feee849 100755
--- a/virt-sandbox-image/virt-sandbox-image.py
+++ b/virt-sandbox-image/virt-sandbox-image.py
@@ -147,6 +147,18 @@ def gen_create_args(subparser):
 help=_("format format for image"))
 parser.set_defaults(func=create)
 
+def gen_run_args(subparser):
+parser = subparser.add_parser("run",
+  help=_("Run a already built image"))
+requires_name(parser)
+requires_source(parser)
+requires_connect(parser)
+parser.add_argument("imagepath",
+help=_("path for image"))
+parser.add_argument("-i","--igniter",
+help=_("Igniter command for image"))
+parser.set_defaults(func=run)
+
 if __name__ == '__main__':
 parser = argparse.ArgumentParser(description='Sandbox Container Image 
Tool')
 
@@ -154,6 +166,7 @@ if __name__ == '__main__':
 gen_download_args(subparser)
 gen_delete_args(subparser)
 gen_create_args(subparser)
+gen_run_args(subparser)
 
 try:
 args = parser.parse_args()
-- 
2.1.0

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

[libvirt] [sandbox PATCH v2 15/19] Add configuration object for environment variables

[Eren Yagdiran]

Add the config gobject to store custom environment variables.
This will allow creating custom environment variables on a sandbox
with a parameter formatted like --env key1=val1
---
 libvirt-sandbox/Makefile.am  |   2 +
 libvirt-sandbox/libvirt-sandbox-config-all.h |   1 +
 libvirt-sandbox/libvirt-sandbox-config-env.c | 199 +++
 libvirt-sandbox/libvirt-sandbox-config-env.h |  78 +++
 libvirt-sandbox/libvirt-sandbox-config.c | 187 -
 libvirt-sandbox/libvirt-sandbox-config.h |  12 ++
 libvirt-sandbox/libvirt-sandbox.h|   1 +
 libvirt-sandbox/libvirt-sandbox.sym  |   6 +
 8 files changed, 485 insertions(+), 1 deletion(-)
 create mode 100644 libvirt-sandbox/libvirt-sandbox-config-env.c
 create mode 100644 libvirt-sandbox/libvirt-sandbox-config-env.h

diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am
index 597803e..5383b0d 100644
--- a/libvirt-sandbox/Makefile.am
+++ b/libvirt-sandbox/Makefile.am
@@ -53,6 +53,7 @@ SANDBOX_RPC_FILES = \
 SANDBOX_CONFIG_HEADER_FILES = \
libvirt-sandbox-config.h \
libvirt-sandbox-config-disk.h \
+   libvirt-sandbox-config-env.h \
libvirt-sandbox-config-network.h \
libvirt-sandbox-config-network-address.h \
libvirt-sandbox-config-network-filterref-parameter.h \
@@ -92,6 +93,7 @@ SANDBOX_CONFIG_SOURCE_FILES = \
libvirt-sandbox-util.c \
libvirt-sandbox-config.c \
libvirt-sandbox-config-disk.c \
+   libvirt-sandbox-config-env.c \
libvirt-sandbox-config-network.c \
libvirt-sandbox-config-network-address.c \
libvirt-sandbox-config-network-filterref.c \
diff --git a/libvirt-sandbox/libvirt-sandbox-config-all.h 
b/libvirt-sandbox/libvirt-sandbox-config-all.h
index 8cb25c4..756bb3e 100644
--- a/libvirt-sandbox/libvirt-sandbox-config-all.h
+++ b/libvirt-sandbox/libvirt-sandbox-config-all.h
@@ -32,6 +32,7 @@
 /* Local includes */
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/libvirt-sandbox/libvirt-sandbox-config-env.c 
b/libvirt-sandbox/libvirt-sandbox-config-env.c
new file mode 100644
index 000..eaf0fb2
--- /dev/null
+++ b/libvirt-sandbox/libvirt-sandbox-config-env.c
@@ -0,0 +1,199 @@
+/*
+ * libvirt-sandbox-config-env.c: libvirt sandbox configuration
+ *
+ * Copyright (C) 2015 Universitat Polit??cnica de Catalunya.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  
USA
+ *
+ * Author: Eren Yagdiran 
+ */
+
+#include 
+#include 
+
+#include "libvirt-sandbox/libvirt-sandbox-config-all.h"
+
+/**
+ * SECTION: libvirt-sandbox-config-env
+ * @short_description: Disk attachment configuration details
+ * @include: libvirt-sandbox/libvirt-sandbox.h
+ * @see_aloso: #GVirSandboxConfig
+ *
+ * Provides an object to store information about a environment variable in the 
sandbox
+ *
+ */
+
+#define GVIR_SANDBOX_CONFIG_ENV_GET_PRIVATE(obj)  \
+(G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_SANDBOX_TYPE_CONFIG_ENV, 
GVirSandboxConfigEnvPrivate))
+
+
+struct _GVirSandboxConfigEnvPrivate
+{
+gchar *key;
+gchar *value;
+};
+
+G_DEFINE_TYPE(GVirSandboxConfigEnv, gvir_sandbox_config_env, G_TYPE_OBJECT);
+
+
+enum {
+PROP_0,
+PROP_KEY,
+PROP_VALUE
+};
+
+enum {
+LAST_SIGNAL
+};
+
+
+
+static void gvir_sandbox_config_env_get_property(GObject *object,
+  guint prop_id,
+  GValue *value,
+  GParamSpec *pspec)
+{
+GVirSandboxConfigEnv *config = GVIR_SANDBOX_CONFIG_ENV(object);
+GVirSandboxConfigEnvPrivate *priv = config->priv;
+
+switch (prop_id) {
+case PROP_KEY:
+g_value_set_string(value, priv->key);
+break;
+case PROP_VALUE:
+g_value_set_string(value, priv->value);
+break;
+   default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
+}
+}
+
+
+static void gvir_sandbox_config_env_set_property(GObject *object,
+  

[libvirt] [sandbox PATCH v2 12/19] Image: Add network support

[Eren Yagdiran]

Virt-sandbox-image will pass exact network arguments to virt-sandbox
---
 virt-sandbox-image/virt-sandbox-image.py | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/virt-sandbox-image/virt-sandbox-image.py 
b/virt-sandbox-image/virt-sandbox-image.py
index e20ce22..5fc7f44 100755
--- a/virt-sandbox-image/virt-sandbox-image.py
+++ b/virt-sandbox-image/virt-sandbox-image.py
@@ -116,9 +116,13 @@ def run(args):
 if args.connect is not None:
 cmd.append("-c")
 cmd.append(args.connect)
-params = ['-m','host-image:/=%s,format=%s' %(diskfile,format),
-   '--',
-   commandToRun]
+params = ['-m','host-image:/=%s,format=%s' %(diskfile,format)]
+networkArgs = args.network
+if networkArgs is not None:
+params.append('-N')
+params.append(networkArgs)
+params.append('--')
+params.append(commandToRun)
 cmd = cmd + params
 subprocess.call(cmd)
 
@@ -187,6 +191,8 @@ def gen_run_args(subparser):
 help=_("path for image"))
 parser.add_argument("-i","--igniter",
 help=_("Igniter command for image"))
+parser.add_argument("-n","--network",
+help=_("Network params for running template"))
 parser.set_defaults(func=run)
 
 if __name__ == '__main__':
-- 
2.1.0

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

[libvirt] [sandbox PATCH v2 00/19] *** Virt-sandbox-image ***

[Eren Yagdiran]

virt-sandbox-image.py is a python script that lets you download and run 
templates
from supported sources using virt-sandbox.
Component-based archictecture is accomplished through Source base class.
Docker image support is added through DockerSource.
DockerSource is capable of downloading and running Docker images by consuming 
Docker Registry API.

**Changes for v2**
*Trailing spaces are gone forever. make syntax-check now is ok.
*Python version < 2.7.9 or < 3.4.3 now gives a warring when downloading a 
docker image from ssl.
*Dynamic resource loader has been changed. Now it uses class naming convention 
in order to load
custom sources. In previous patch series, custom sources used to register 
themselves into a 
common area, so we can load from them
* -c/--connect parameter is for providing URI to the libvirt.
* Private methods now starts with a single underscore instead of double 
underscores
* virt-sandbox-image/sources/__init__.py is added
* Network params can be passed to running sandbox.
* Custom volume support is added through host-bind.
* Custom environment variables can be passed into virt-sandbox
* Custom environment support for virt-sandbox-image


Daniel P Berrange (1):
  Add virt-sandbox-image

Eren Yagdiran (18):
  Fix virt-sandbox-image
  Image: Add Hooking Mechanism
  Image: Add download function
  Image: Refactor create function
  Image: Add delete function
  Image: Add get_command function to Source
  Image: Add run args
  Image: Add check_connect function
  Image: Add get_disk function to Source
  Image: Add run function
  Image: Add network support
  Image: Add Volume Support
  Image: man file for virt-sandbox-image
  Add configuration object for environment variables
  Add environment parameter to virt-sandbox
  Common-init: Exporting custom environment variables
  Add testcase for custom environment variables
  Image: Add custom environment support

 .gitignore|   1 +
 bin/Makefile.am   |  21 +-
 bin/virt-sandbox-image.in |   3 +
 bin/virt-sandbox-image.pod| 172 +++
 bin/virt-sandbox.c|  14 +
 configure.ac  |   2 +
 libvirt-sandbox/Makefile.am   |   2 +
 libvirt-sandbox/libvirt-sandbox-config-all.h  |   1 +
 libvirt-sandbox/libvirt-sandbox-config-env.c  | 199 
 libvirt-sandbox/libvirt-sandbox-config-env.h  |  78 +
 libvirt-sandbox/libvirt-sandbox-config.c  | 187 +++-
 libvirt-sandbox/libvirt-sandbox-config.h  |  12 +
 libvirt-sandbox/libvirt-sandbox-init-common.c |  30 ++
 libvirt-sandbox/libvirt-sandbox.h |   1 +
 libvirt-sandbox/libvirt-sandbox.sym   |   6 +
 libvirt-sandbox/tests/test-config.c   |  10 +
 po/POTFILES.in|   1 +
 virt-sandbox-image/Makefile.am|  14 +
 virt-sandbox-image/sources/DockerSource.py| 425 ++
 virt-sandbox-image/sources/Source.py  |  59 
 virt-sandbox-image/sources/__init__.py|  29 ++
 virt-sandbox-image/virt-sandbox-image.py  | 267 
 22 files changed, 1529 insertions(+), 5 deletions(-)
 create mode 100644 bin/virt-sandbox-image.in
 create mode 100644 bin/virt-sandbox-image.pod
 create mode 100644 libvirt-sandbox/libvirt-sandbox-config-env.c
 create mode 100644 libvirt-sandbox/libvirt-sandbox-config-env.h
 create mode 100644 virt-sandbox-image/Makefile.am
 create mode 100644 virt-sandbox-image/sources/DockerSource.py
 create mode 100644 virt-sandbox-image/sources/Source.py
 create mode 100644 virt-sandbox-image/sources/__init__.py
 create mode 100755 virt-sandbox-image/virt-sandbox-image.py

-- 
2.1.0

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

[libvirt] [sandbox PATCH v2 10/19] Image: Add get_disk function to Source

[Eren Yagdiran]

Provide a way to know which disk image to use for the sandbox depending on the 
used source
DockerSource will need to locate the topmost disk image among all the layers 
images
---
 virt-sandbox-image/sources/DockerSource.py | 9 +
 virt-sandbox-image/sources/Source.py   | 4 
 2 files changed, 13 insertions(+)

diff --git a/virt-sandbox-image/sources/DockerSource.py 
b/virt-sandbox-image/sources/DockerSource.py
index 2f4646f..74feb3e 100644
--- a/virt-sandbox-image/sources/DockerSource.py
+++ b/virt-sandbox-image/sources/DockerSource.py
@@ -385,6 +385,15 @@ class DockerSource(Source):
 parent = None
 imagetagid = parent
 
+def get_disk(self,**args):
+name = args['name']
+destdir = args['path']
+imageList = self._get_image_list(name,destdir)
+toplayer = imageList[0]
+diskfile = destdir + "/" + toplayer + "/template.qcow2"
+configfile = destdir + "/" + toplayer + "/template.json"
+return (diskfile,configfile)
+
 def get_command(self,configfile):
 configParser = DockerConfParser(configfile)
 commandToRun = configParser.getRunCommand()
diff --git a/virt-sandbox-image/sources/Source.py 
b/virt-sandbox-image/sources/Source.py
index 01f8560..6e2f5fb 100644
--- a/virt-sandbox-image/sources/Source.py
+++ b/virt-sandbox-image/sources/Source.py
@@ -45,3 +45,7 @@ class Source():
 @abstractmethod
 def get_command(self,**args):
   pass
+
+@abstractmethod
+def get_disk(self,**args):
+  pass
-- 
2.1.0

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

Re: [libvirt] [PATCH] conf: Resolve Coverity FORWARD_NULL

[John Ferlan]

On 08/04/2015 11:42 AM, Michal Privoznik wrote:
> On 04.08.2015 13:11, John Ferlan wrote:
>> The recent changes to perform SCSI device address checks during the
>> post parse callbacks ran afoul of the Coverity checker since the changes
>> assumed that the 'xmlopt' parameter to virDomainDeviceDefPostParse
>> would be non NULL (commit id 'ca2cf74e87'); however, what was missed
>> is there was an "if (xmlopt &&" check being made, so Coverity believed
>> that it could be possible for a NULL 'xmlopt'.
>>
>> Checking the various calling paths seemingly disproves that. If called
>> from virDomainDeviceDefParse, there were two other possible calls that
>> would end up dereffing, so that path could not be NULL. If called via
>> virDomainDefPostParseDeviceIterator via virDomainDefPostParse there
>> are two callers (virDomainDefParseXML and qemuParseCommandLine)
>> which deref xmlopt either directly or through another call.
>>
>> So I'm removing the check for non-NULL xmlopt.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/conf/domain_conf.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 77a50c3..dd5ebd7 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -4145,7 +4145,7 @@ virDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>>  {
>>  int ret;
>>  
>> -if (xmlopt && xmlopt->config.devicesPostParseCallback) {
>> +if (xmlopt->config.devicesPostParseCallback) {
>>  ret = xmlopt->config.devicesPostParseCallback(dev, def, caps,
>>xmlopt->config.priv);
>>  if (ret < 0)
>>
> 
> ACK
> 
> Although with the virDomainDefPostParse() it should be the same story.
> @xmlopt can't be NULL there too. But that can be saved for a follow up
> patch if you want.
> 
> Michal
> 

Myopia strikes again...  I need a larger workspace!

John

Perhaps it'd be better to squash in the following rather than have a
separate patch since it's same issue even though Coverity only complained
about the initial one. Although if it's preferred to have a separate
patch that's fine by me too:

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7d3a24d..5eaeb21 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4190,7 +4190,7 @@ virDomainDefPostParse(virDomainDefPtr def,
 };
 
 /* call the domain config callback */
-if (xmlopt && xmlopt->config.domainPostParseCallback) {
+if (xmlopt->config.domainPostParseCallback) {
 ret = xmlopt->config.domainPostParseCallback(def, caps,
  xmlopt->config.priv);
 if (ret < 0)


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

Re: [libvirt] [PATCH] conf: Resolve Coverity FORWARD_NULL

[Michal Privoznik]

On 04.08.2015 13:11, John Ferlan wrote:
> The recent changes to perform SCSI device address checks during the
> post parse callbacks ran afoul of the Coverity checker since the changes
> assumed that the 'xmlopt' parameter to virDomainDeviceDefPostParse
> would be non NULL (commit id 'ca2cf74e87'); however, what was missed
> is there was an "if (xmlopt &&" check being made, so Coverity believed
> that it could be possible for a NULL 'xmlopt'.
> 
> Checking the various calling paths seemingly disproves that. If called
> from virDomainDeviceDefParse, there were two other possible calls that
> would end up dereffing, so that path could not be NULL. If called via
> virDomainDefPostParseDeviceIterator via virDomainDefPostParse there
> are two callers (virDomainDefParseXML and qemuParseCommandLine)
> which deref xmlopt either directly or through another call.
> 
> So I'm removing the check for non-NULL xmlopt.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/conf/domain_conf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 77a50c3..dd5ebd7 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4145,7 +4145,7 @@ virDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>  {
>  int ret;
>  
> -if (xmlopt && xmlopt->config.devicesPostParseCallback) {
> +if (xmlopt->config.devicesPostParseCallback) {
>  ret = xmlopt->config.devicesPostParseCallback(dev, def, caps,
>xmlopt->config.priv);
>  if (ret < 0)
> 

ACK

Although with the virDomainDefPostParse() it should be the same story.
@xmlopt can't be NULL there too. But that can be saved for a follow up
patch if you want.

Michal

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

Re: [libvirt] [PATCH 2/2] netlink: add support for multi-part netlink messages.

[Dmitry Guryanov]

On 07/31/2015 07:35 PM, Maxim Perevedentsev wrote:

Such messages do not have NLMSG_ERROR or NLMSG_DONE type
but they are valid responses. We test 'multi-partness'
by looking for NLM_F_MULTI flag.


This patch looks OK to me except for comment style.


---
  src/util/virnetlink.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0052ef9..105a604 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -386,7 +386,9 @@ virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int 
recvbuflen)
  break;

  default:
-goto malformed_resp;
+// We allow multipart messages.




+if (!(resp->nlmsg_flags & NLM_F_MULTI))
+goto malformed_resp;
  }

  return result;
--
Sincerely,
Maxim Perevedentsev

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


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

Re: [libvirt] [PATCH 1/2] network: added waiting for DAD to finish for bridge address.

[Dmitry Guryanov]

On 07/31/2015 07:35 PM, Maxim Perevedentsev wrote:

This is a fix for commit db488c79173b240459c7754f38c3c6af9b432970
dnsmasq main process exits without waiting for DAD, this is dnsmasq
daemon's task. So we periodically poll the kernel using netlink and
check whether there are any IPv6 addresses assigned to bridge
which have 'tentative' state. After DAD is finished, execution continues.
I guess that is what dnsmasq was assumed to do.


There are some comments on style, but unfortunately I'm not very 
confident in this part of libvirt, so it would be great if someone else 
reviewed these patches.



---
  src/network/bridge_driver.c | 109 +++-
  1 file changed, 108 insertions(+), 1 deletion(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 3d6721b..6a7a505 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2025,6 +2025,107 @@ networkAddRouteToBridge(virNetworkObjPtr network,
  return 0;
  }

+/* return whether DAD is in progress */
Could you, please, specify, which value mean 'DAD is in progress', it's 
unclear from the comment?

+static int
+networkParseDadStatus(struct nlmsghdr *nlh, int len,
+  virNetworkObjPtr network)
+{
+struct ifaddrmsg *ifaddrmsg_ptr;
+unsigned int ifaddrmsg_len;
+struct rtattr *rtattr_ptr;
+size_t i;
+virNetworkIpDefPtr ipdef;
+unsigned char prefix;
+struct in6_addr *addr;
+for (; NLMSG_OK(nlh, len); nlh = NLMSG_NEXT(nlh, len)) {
+if (NLMSG_PAYLOAD(nlh, 0) < sizeof(struct ifaddrmsg))
+// Message without payload is the last one.


Please, change comments to C89-style, /* */.
Also add braces, libvirt's coding style requires putting braces around 
multi-line

body even if it's a single line of code with a comment.


+break;
+
+ifaddrmsg_ptr = (struct ifaddrmsg *)NLMSG_DATA(nlh);
+if (!(ifaddrmsg_ptr->ifa_flags & IFA_F_TENTATIVE))
+// Not tentative: we are not interested in this entry.
+continue;


The same, braces and comment.


+
+ifaddrmsg_len = IFA_PAYLOAD(nlh);
+rtattr_ptr = (struct rtattr *) IFA_RTA(ifaddrmsg_ptr);
+for (; RTA_OK(rtattr_ptr, ifaddrmsg_len);
+rtattr_ptr = RTA_NEXT(rtattr_ptr, ifaddrmsg_len)) {
+if (RTA_PAYLOAD(rtattr_ptr) != sizeof(struct in6_addr))
+// No address: ignore.
+continue;
+

Braces and comment, also please, fix all such things in code below.


+// We check only known addresses.
+for (i = 0;
+ (ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET6, 
i));
+ i++) {
+
+prefix = virNetworkIpDefPrefix(ipdef);
+addr = &ipdef->address.data.inet6.sin6_addr;
+
+if (prefix == ifaddrmsg_ptr->ifa_prefixlen &&
+!memcmp(addr, RTA_DATA(rtattr_ptr),
+sizeof(struct in6_addr)))
+// We found matching tentative address.
+return 1;
+}
+}
+}
+return 0;
+}
+
+/* return after DAD finishes for all known IPv6 addresses or an error */
+static int
+networkWaitDadFinish(virNetworkObjPtr network)


I'd put this function to src/util/virnetlink.c


+{
+struct nl_msg *nlmsg = NULL;
+struct ifaddrmsg ifa;
+struct nlmsghdr *resp = NULL;
+unsigned int recvbuflen;
+int ret = -1, dad = 1;
+
+if (!(nlmsg = nlmsg_alloc_simple(RTM_GETADDR,
+NLM_F_REQUEST | NLM_F_DUMP))) {


style:

if (!(nlmsg = nlmsg_alloc_simple(RTM_GETADDR,
 NLM_F_REQUEST | NLM_F_DUMP))) {



+virReportOOMError();
+return -1;
+}
+
+memset(&ifa, 0, sizeof(ifa));
+// DAD is for IPv6 adresses only.
+ifa.ifa_family = AF_INET6;
+if (nlmsg_append(nlmsg, &ifa, sizeof(ifa), NLMSG_ALIGNTO) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("allocated netlink buffer is too small"));

please, align second line to the brace


+goto cleanup;
+}
+
+// Periodically query netlink until DAD finishes on all known addresses.
+while (dad) {
+if (virNetlinkCommand(nlmsg, &resp, &recvbuflen, 0, 0,
+  NETLINK_ROUTE, 0) < 0)
+goto cleanup;
+
+if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) {
+virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
+_("error reading DAD state information"));

the same


+goto cleanup;
+}
+
+//Parse response.
+dad = networkParseDadStatus(resp, recvbuflen, network);
+if (dad)
+usleep(1000 * 10);

What if an interface will remain in transient state forever?


+
+VIR_FREE(resp);
+}
+ret = 0;
+
+ cleanup:
+VIR_FREE(resp);
+nlmsg_free(nlmsg);
+return ret;
+}
+
  static int
  networ

Re: [libvirt] [PATCH] examples: Add example polkit ACL rules

[Daniel P. Berrange]

On Tue, Aug 04, 2015 at 05:01:26PM +0200, Jiri Denemark wrote:
> Creating ACL rules is not exactly easy and existing examples are pretty
> simple. This patch adds a somewhat complex example which defines three
> roles (user, operator, admin) with different permissions.
> 

> +/* Basic operations and monitoring. */
> +var user = new Role("user");
> +user.users = ["user1", "user2", "user3"];
> +user.groups = ["group1", "group2"];
> +
> +/* Same as users plus some privileged operations.  */
> +var operator = new Role("operator");
> +operator.users = ["powerUser1", "powerUser2"];
> +operator.groups = ["powerGroup1", "powerGroup2", "powerGroup3"];
> +
> +/* Full access. */
> +var admin = new Role("admin");
> +admin.users = ["adminUser1"];
> +admin.groups = ["adminGroup1"];

What is the aim in differentiating operator vs admin ? 


> +operator.actions = [
> +"domain.delete",
> +"domain.migrate",
> +"domain.read-secure",
> +"domain.write",

Once you give out domain.write (or any other $object.write) to the
operator, it is pretty much game over for security - they'd be
able to elevate privileges to admin without any real trouble.

> +"network.delete",
> +"network.getattr",
> +"network.read",
> +"network.save",
> +"network.start",
> +"network.stop",
> +"network.write",
> +"nwfilter.delete",
> +"nwfilter.getattr",
> +"nwfilter.read",
> +"nwfilter.save",
> +"nwfilter.write",
> +"secret.delete",
> +"secret.getattr",
> +"secret.read",
> +"secret.read-secure",
> +"secret.save",
> +"secret.write",
> +"storage-pool.refresh",
> +"storage-vol.create",
> +"storage-vol.data-read",
> +"storage-vol.data-write",
> +"storage-vol.delete",
> +"storage-vol.format",
> +"storage-vol.getattr",
> +"storage-vol.read",
> +"storage-vol.resize"
> +];

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] examples: Add example polkit ACL rules

[Jiri Denemark]

Creating ACL rules is not exactly easy and existing examples are pretty
simple. This patch adds a somewhat complex example which defines three
roles (user, operator, admin) with different permissions.

Signed-off-by: Jiri Denemark 
---
 Makefile.am   |   2 +-
 configure.ac  |   1 +
 examples/polkit/Makefile.am   |  17 ++
 examples/polkit/libvirt-acl.rules | 124 ++
 libvirt.spec.in   |   3 +
 5 files changed, 146 insertions(+), 1 deletion(-)
 create mode 100644 examples/polkit/Makefile.am
 create mode 100644 examples/polkit/libvirt-acl.rules

diff --git a/Makefile.am b/Makefile.am
index 91b943b..d338d5a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -23,7 +23,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs 
gnulib/tests \
   tests po examples/object-events examples/hellolibvirt \
   examples/dominfo examples/domsuspend examples/apparmor \
   examples/xml/nwfilter examples/openauth examples/systemtap \
-  tools/wireshark examples/dommigrate \
+  tools/wireshark examples/dommigrate examples/polkit \
   examples/lxcconvert examples/domtop
 
 ACLOCAL_AMFLAGS = -I m4
diff --git a/configure.ac b/configure.ac
index 46c80ce..d506c28 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2805,6 +2805,7 @@ AC_CONFIG_FILES([\
 examples/systemtap/Makefile \
 examples/xml/nwfilter/Makefile \
 examples/lxcconvert/Makefile \
+examples/polkit/Makefile \
 tools/wireshark/Makefile \
 tools/wireshark/src/Makefile])
 AC_OUTPUT
diff --git a/examples/polkit/Makefile.am b/examples/polkit/Makefile.am
new file mode 100644
index 000..4d213e8
--- /dev/null
+++ b/examples/polkit/Makefile.am
@@ -0,0 +1,17 @@
+## Copyright (C) 2015 Red Hat, Inc.
+##
+## This library is free software; you can redistribute it and/or
+## modify it under the terms of the GNU Lesser General Public
+## License as published by the Free Software Foundation; either
+## version 2.1 of the License, or (at your option) any later version.
+##
+## This library is distributed in the hope that it will be useful,
+## but WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+## Lesser General Public License for more details.
+##
+## You should have received a copy of the GNU Lesser General Public
+## License along with this library.  If not, see
+## .
+
+EXTRA_DIST = libvirt-acl.rules
diff --git a/examples/polkit/libvirt-acl.rules 
b/examples/polkit/libvirt-acl.rules
new file mode 100644
index 000..6a93206
--- /dev/null
+++ b/examples/polkit/libvirt-acl.rules
@@ -0,0 +1,124 @@
+function Role(name) {
+this.name = name;
+
+this.actions = [];
+this.users = [];
+this.groups = [];
+
+this.lookup = function(subject) {
+if (this.users.indexOf(subject.user) >= 0)
+return true;
+
+for (var i = 0; i < subject.groups.length; i++) {
+if (this.groups.indexOf(subject.groups[i]) >= 0)
+return true;
+}
+
+return false;
+};
+
+this.lookupAction = function(action) {
+action = action.id.replace("org.libvirt.api.", "");
+if (this.actions.indexOf(action) >= 0)
+return true;
+else
+return false;
+};
+}
+
+
+/* Basic operations and monitoring. */
+var user = new Role("user");
+user.users = ["user1", "user2", "user3"];
+user.groups = ["group1", "group2"];
+
+/* Same as users plus some privileged operations.  */
+var operator = new Role("operator");
+operator.users = ["powerUser1", "powerUser2"];
+operator.groups = ["powerGroup1", "powerGroup2", "powerGroup3"];
+
+/* Full access. */
+var admin = new Role("admin");
+admin.users = ["adminUser1"];
+admin.groups = ["adminGroup1"];
+
+
+user.actions = [
+"domain.core-dump",
+"domain.fs-freeze",
+"domain.fs-trim",
+"domain.getattr",
+"domain.hibernate",
+"domain.init-control",
+"domain.inject-nmi",
+"domain.open-device",
+"domain.open-graphics",
+"domain.pm-control",
+"domain.read",
+"domain.reset",
+"domain.save",
+"domain.screenshot",
+"domain.send-input",
+"domain.send-signal",
+"domain.set-password",
+"domain.set-time",
+"domain.snapshot",
+"domain.start",
+"domain.stop",
+"domain.suspend"
+];
+operator.actions = [
+"domain.delete",
+"domain.migrate",
+"domain.read-secure",
+"domain.write",
+"network.delete",
+"network.getattr",
+"network.read",
+"network.save",
+"network.start",
+"network.stop",
+"network.write",
+"nwfilter.delete",
+"nwfilter.getattr",
+"nwfilter.read",
+"nwfilter.save",
+"nwfilter.write",
+"secret.delete",
+"secret.getattr",
+"secret.read",
+"secret.read-secure",
+"secret.save",
+"secret.write",
+"storage-pool.refresh",
+"storage-vol.crea

Re: [libvirt] [PATCH] qemuProcessStart: Be tolerant to relabel errors for session mode

[Michal Privoznik]

On 20.07.2015 15:50, Daniel P. Berrange wrote:
> On Wed, Jul 15, 2015 at 03:02:13PM +0200, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1124841
>>
>> When the daemon is running under unprivileged user, that is under
>> qemu:///session, there are plenty of operations we can't do. What
>> we can do is to go with best effort. One of such cases is
>> relabeling domain resources (be it disks, sockets, regular files,
>> etc.) during domain startup process. While we may successfully set
>> DAC labels, we can be fairly certain that any attempt to change
>> SELinux labels will fail. Therefore we should tolerate relabelling
>> errors and just let qemu to try access the resources. If it fails,
>> our error reporting system is strong enough to articulate the
>> exact error to the user anyway.
> 
> Errr, isn't it entirely the opposite to what you say. Running as
> an unprivileged user ID has no bearing on whether you are allowed
> to set SELinux labels. If the user acount is unconfined_t it can
> set any SELinux labels it wants. It will only fail if the libvird
> process is confined in some way.  IMHO we shold not be ignoring
> such failures.
> 
> What *will* fail is any attempt to set DAC labels, since you need
> CAP_CHOWN capability, but we shouldn't have the DAC security
> maanger running when in session mode.
> 
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index 1c0c734..58ed631 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -4856,8 +4856,13 @@ int qemuProcessStart(virConnectPtr conn,
>>  
>>  VIR_DEBUG("Setting domain security labels");
>>  if (virSecurityManagerSetAllLabel(driver->securityManager,
>> -  vm->def, stdin_path) < 0)
>> -goto cleanup;
>> +  vm->def, stdin_path) < 0) {
>> +/* Be tolerant to relabel errors if we are running unprivileged. */
>> +if (virQEMUDriverIsPrivileged(driver))
>> +goto cleanup;
>> +else
>> +VIR_DEBUG("Ignoring relabel errors for unprivileged daemon");
>> +}
> 
> I really don't think we should do this here as it affects all
> security managers.
> 
> What is the failure you are actually seeing without this ?  SElinux
> label changes should be succeeding in session mode and we should not
> even be applying DAC labels

Unable to complete install: 'unable to set security context 
'system_u:object_r:virt_content_t:s0' on 
'/usr/share/virtio-win/virtio-win.iso': Operation not permitted'

Traceback (most recent call last):
  File "/usr/share/virt-manager/virtManager/asyncjob.py", line 89, in 
cb_wrapper
  callback(asyncjob, *args, **kwargs)
File "/usr/share/virt-manager/virtManager/create.py", line 1873, in 
do_install
  guest.start_install(meter=meter)
File "/usr/share/virt-manager/virtinst/guest.py", line 417, in 
start_install
  noboot)
File "/usr/share/virt-manager/virtinst/guest.py", line 481, in 
_create_guest
  dom = self.conn.createLinux(start_xml or final_xml, 0)
File "/usr/lib64/python2.7/site-packages/libvirt.py", line 3585, in 
createLinux
  if ret is None:raise libvirtError('virDomainCreateLinux() failed', 
conn=self)
  libvirtError: unable to set security context 
'system_u:object_r:virt_content_t:s0' on 
'/usr/share/virtio-win/virtio-win.iso': Operation not permitted


# ls -lZ /usr/share/virtio-win/virtio-win.iso
lrwxrwxrwx. root root system_u:object_r:usr_t:s0   
/usr/share/virtio-win/virtio-win.iso -> virtio-win-1.7.4.iso

# ls -lZ /usr/share/virtio-win/virtio-win-1.7.4.iso
-rw-r--r--. root root system_u:object_r:usr_t:s0   
/usr/share/virtio-win/virtio-win-1.7.4.iso


So maybe we should ignore just selinux labelling errors when running as 
unprivileged?

Michal

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

Re: [libvirt] mass create vm errors

[Michal Privoznik]

On 04.08.2015 14:31, Vasiliy Tolstov wrote:
> 2015-08-03 10:01 GMT+03:00 Michal Privoznik :
>> There has not been much movement in that particular area since 1.2.16.
>> There's inherent race though: by the time that libvirt detects that a
>> certain port is free and qemu binds to it. During this window another
>> process may just allocate the port which will result in the error
>> message you're seeing.
> 
> This is very bad thing in case of mass start , for example when node
> dies i have 100-150 vm started at the same time, and sometimes they
> running on single host.
> Does it possible to create lock for this port when port passed to
> qemu, so another process when detect new port don't use it?

No. It would require change in other apps so that they use the lock file.

> 
>> One of the solutions may be to use static ports, or make sure that other
>> programs don't interfere with the range that's reserved for graphical
>> sessions (the range can be configured too in qemu.conf).
> But if i specify port ranges in qemu.conf, how this solve my issue? as
> i understand libvirt again randomly allocated port for vm...
> 

No, libvirt does not randomly allocate port. In fact, libvirt does not
allocate port at all. It merely checks on domain startup that the
graphics port is free. Or it searches for next free port within the
range set in qemu.conf (in case of autoport). Then, the port number is
put on qemu command line and qemu binds to it. However during this
window other apps may step in and allocate the port.
But if you choose a different range, unused one, you may be more
successful. BTW: to view which app has allocated the 5904 port you can
use: 'netstat -ltnp | grep 5904'.

Of course, the internal bitmap of free and taken ports is protected by a
mutex, so libvirt should not assign the same port to two domains.

Michal

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

Re: [libvirt] [PATCH v2 12/12] conf: Check for hostdev conflicts when assign default disk address

[John Ferlan]

On 08/03/2015 09:57 AM, Ján Tomko wrote:
> On Wed, Jul 22, 2015 at 10:54:34AM -0400, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1210587  (completed)
>>
>> When generating the default drive address for a SCSI  device,
>> check the generated address to ensure it doesn't conflict with a SCSI
>>  address. The  address generation algorithm uses the
>>  "dev" name in order to determine which controller and unit
>> in order to place the device. Since a SCSI  device doesn't
>> require a target device name, its placement on the guest SCSI address
>> "could" conflict.  For instance, if a SCSI  exists at
>> controller=0 unit=0 and an attempt to hotplug 'sda' into the guest
>> made, there would be a conflict if the  is already using
>> /dev/sda.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/conf/domain_conf.c  | 16 ++--
>>  src/conf/domain_conf.h  |  3 ++-
>>  src/qemu/qemu_command.c |  4 ++--
>>  src/vmx/vmx.c   | 22 --
>>  src/vmx/vmx.h   |  3 ++-
>>  5 files changed, 32 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 39a4cf8..0dac60c 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -4079,7 +4079,7 @@ 
>> virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
>>  }
>>  
>>  if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
>> -virDomainDiskDefAssignAddress(xmlopt, disk) < 0)
>> +virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0)
>>  return -1;
>>  }
>>  
>> @@ -5860,7 +5860,8 @@ virDomainDiskDiffersSourceOnly(virDomainDiskDefPtr 
>> disk,
>>  
>>  int
>>  virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt,
>> -  virDomainDiskDefPtr def)
>> +  virDomainDiskDefPtr def,
>> +  const virDomainDef *vmdef)
> 
> This function does not really do any 'real' assignment, it just
> translates the target name to a fixed address.
> Unlike PCI address assignment, where we find unoccupied slots based on
> the domain defition, the domain definition is not really needed here.
> 
> Moving the address check conflict to virDomainDefPostParse, after
> the addresses have been filled out by virDomainDeviceDefPostParse,
> would remove the need to change all the function prototypes and it would
> be a good place to check for address conflicts between disks too - after
> this series, two drives with the same addresses are allowed, as long as
> they have different target names.
> 

After looking through the code and thinking more about this - using
virDomainDefPostParse won't work for the hotplug case since it's only
going through virDomainDeviceDefParse and virDomainDeviceDefPostParse
code in order to validate whether the address (whether provided or
generated) is duplicated.

Leaving the vmdef in virDomainDiskDefAssignAddress allows for the check
of the generated device address based on the name to be compared against
known hostdev addresses to ensure there isn't a conflict. Since the
knowledge of what device type is being used is there - it just seems
more natural to make the check there rather than repeating the same
check in multiple callers.

With respect to the other issue you note - that someone can provide
duplicate drive addresses for either disk or hostdev - that's a slightly
different issue, but is resolvable as well. Of course it's perhaps also
true of other address types - that is I'm not sure that problem is
'drive' specific. It seems a separate patch could use the
virDomainDefHasDeviceAddressIterator in some way to check all addresses
for duplicates.

I believe the existing patches should remain as is:

 Patch 9 - checks that the provided 'drive' address for a SCSI hostdev
doesn't conflict with any current disk address.  Since the disks would
be parsed first this works to ensure there are no generated or provided
address conflicts

 Patch 11 - just sets up to make patch 12 appear cleaner

 Patch 12 - checks to ensure the generated address based on name doesn't
conflict with some defined SCSI hostdev address.

If you have some specific suggestions for ways to handle this - I'm
certainly open to reconsidering, but because of the existing hotplug
paths which don't call virDomainDefPostParse I don't see it as a viable
solution to the "whole" problem.

John

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

Re: [libvirt] mass create vm errors

[Vasiliy Tolstov]

2015-08-03 10:01 GMT+03:00 Michal Privoznik :
> There has not been much movement in that particular area since 1.2.16.
> There's inherent race though: by the time that libvirt detects that a
> certain port is free and qemu binds to it. During this window another
> process may just allocate the port which will result in the error
> message you're seeing.

This is very bad thing in case of mass start , for example when node
dies i have 100-150 vm started at the same time, and sometimes they
running on single host.
Does it possible to create lock for this port when port passed to
qemu, so another process when detect new port don't use it?

> One of the solutions may be to use static ports, or make sure that other
> programs don't interfere with the range that's reserved for graphical
> sessions (the range can be configured too in qemu.conf).
But if i specify port ranges in qemu.conf, how this solve my issue? as
i understand libvirt again randomly allocated port for vm...



-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

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

Re: [libvirt] [PATCH 0/2] Adjustments for configuring volume lun device

[John Ferlan]

On 07/18/2015 07:43 AM, John Ferlan wrote:
> Resolve a couple of issues regarding failures seen configuring a disk
> to be a type='volume' device='lun'.  The doc patch just indicates that
> using an NPIV storage/source pool is a valid option. The second patch
> allows for a "clearer" error message to be reported.
> 
> 
> John Ferlan (2):
>   docs: Add Fibre Channel NPIV supported option for volume lun config
>   conf: Allow error reporting in virDomainDiskSourceIsBlockType
> 
>  docs/formatdomain.html.in |  6 --
>  src/conf/domain_conf.c| 21 ++---
>  src/conf/domain_conf.h|  2 +-
>  src/lxc/lxc_cgroup.c  |  2 +-
>  src/lxc/lxc_driver.c  |  6 ++
>  src/qemu/qemu_command.c   |  5 +
>  src/qemu/qemu_conf.c  |  6 +++---
>  7 files changed, 30 insertions(+), 18 deletions(-)
> 

Adjusted the lxc_driver call to be true from patch 2 and pushed.

Tks,

John

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

Re: [libvirt] tc ingress rule of VM B disappear when reboot VM A

[ychen]

finally, I have found that it is ovs bug.
when reboot vm, libvirt will call ovs-vsctl del-port, 
this cmd will cause other port's tc ingress rule automatically remove.


http://openvswitch.org/pipermail/discuss/2015-July/018318.html
I have reported a bug for ovs, and they have admitted it.


anyway, thank you for your reply.
At 2015-08-03 15:34:11, "Michal Privoznik"  wrote:
>On 20.07.2015 10:43, ychen wrote:
>> hi:
>> when I use openstack devstack to test QOS, wired phenomenon appeared, 
>> I set qos ingress rule in tapB, but when I reboot tapA, the ingress rule of 
>> tapB automatically removed, but the egress rule is still exist.
>> Test enviroment:
>> Linux: ubuntu 14.04.1 LTS
>> kernel: 3.13.0-32-generic
>> libvirt: 1.2.2
>> openstack: havana
>> 
>> 
>> 1. use nova to create vm A and vm B. for the configuration of the libvirt 
>> xml, see the last paragraph in the end.
>> 2. use tc cmd to create qos rule for vm A and vm B
>>tc qdisc add dev tap3d0d2c4a-0b ingress  //vmA
>>tc qdisc add dev tap896d5066-69 ingress //vmB
>> 
>> 
>> 3. then use cmd 
>>   "sudo virsh destory 142a08db-6e25-4a03-be13-7073104b0745 "  to first 
>> shutdown vm1
>>   then I see ingress rule of vmB disappeared :( 
>> 
>
>Interesting. With the latest libvirt I am unable to reproduce - there's
>a check and QoS is cleared out only if set in domain XML. Since your
>domain XML lacks it, I guess it's either already fixed or not libvirt fault.
>
>Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] tests: extend workaround for gnutls private key loading failure

[Daniel P. Berrange]

In gnutls 3.4.3 there is a regression in the loading of private
keys via gnutls_x509_privkey_import. We already have a workaround
to deal with failures on older gnutls, but the error code that
the new gnutls returns is different. Extend the workaround so that
is checks for GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE too.

See also gnutls https://bugzilla.redhat.com/show_bug.cgi?id=1250020

Signed-off-by: Daniel P. Berrange 
---
 tests/virnettlshelpers.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Pushed as a build fix

diff --git a/tests/virnettlshelpers.c b/tests/virnettlshelpers.c
index 6e667d1..531d0b9 100644
--- a/tests/virnettlshelpers.c
+++ b/tests/virnettlshelpers.c
@@ -84,7 +84,8 @@ static gnutls_x509_privkey_t testTLSLoadKey(void)
 
 if ((err = gnutls_x509_privkey_import(key, &data,
   GNUTLS_X509_FMT_PEM)) < 0) {
-if (err != GNUTLS_E_BASE64_UNEXPECTED_HEADER_ERROR) {
+if (err != GNUTLS_E_BASE64_UNEXPECTED_HEADER_ERROR &&
+err != GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
 VIR_WARN("Failed to import key %s", gnutls_strerror(err));
 abort();
 }
-- 
2.4.3

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

[libvirt] [PATCH] conf: Resolve Coverity FORWARD_NULL

[John Ferlan]

The recent changes to perform SCSI device address checks during the
post parse callbacks ran afoul of the Coverity checker since the changes
assumed that the 'xmlopt' parameter to virDomainDeviceDefPostParse
would be non NULL (commit id 'ca2cf74e87'); however, what was missed
is there was an "if (xmlopt &&" check being made, so Coverity believed
that it could be possible for a NULL 'xmlopt'.

Checking the various calling paths seemingly disproves that. If called
from virDomainDeviceDefParse, there were two other possible calls that
would end up dereffing, so that path could not be NULL. If called via
virDomainDefPostParseDeviceIterator via virDomainDefPostParse there
are two callers (virDomainDefParseXML and qemuParseCommandLine)
which deref xmlopt either directly or through another call.

So I'm removing the check for non-NULL xmlopt.

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 77a50c3..dd5ebd7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4145,7 +4145,7 @@ virDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
 {
 int ret;
 
-if (xmlopt && xmlopt->config.devicesPostParseCallback) {
+if (xmlopt->config.devicesPostParseCallback) {
 ret = xmlopt->config.devicesPostParseCallback(dev, def, caps,
   xmlopt->config.priv);
 if (ret < 0)
-- 
2.1.0

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

[libvirt] PING-2: [PATCH v5 0/4] qemu: Allow PCI virtio on ARM "virt" machine

[Pavel Fedin]

 Hello! Can anybody tell me what is wrong with this? Ignored for a long time.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


> -Original Message-
> From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] 
> On Behalf Of Pavel
> Fedin
> Sent: Friday, July 17, 2015 2:28 PM
> To: libvir-list@redhat.com
> Cc: Peter Krempa
> Subject: [libvirt] [PATCH v5 0/4] qemu: Allow PCI virtio on ARM "virt" machine
> 
> Virt machine in qemu since v2.3.0 has PCI generic host controller, and
> can use PCI devices. This provides performance improvement as well as
> vhost-net with irqfd support for virtio-net. However libvirt currently
> does not allow ARM virt machine to have PCI devices. This patchset adds
> the necessary support.
> 
> Changes since v4:
> - Rebased onto current master
> - Added possibility to plug virtio-net-pci adapter directly into PCIe bus.
>   This is necessary for irqfds to work in qemu.
> Changes since v3:
> - Capability is based not on qemu version but on support of "gpex-pcihost"
>   device by qemu
> - Added a workaround, allowing to pass "make check". The problem is that
>   test suite does not build capabilities cache. Unfortunately this means
>   that correct unit-test for the new functionality currently cannot be
>   written. Test suite framework needs to be improved.
> Changes since v2:
> Complete rework, use different approach
> - Correctly model PCI Express bus on the machine. It is now possible to
>   explicitly specify  with attributes. This allows to
>   attach not only virtio, but any other PCI device to the model.
> - Default is not changed and still mmio, for backwards compatibility with
>   existing installations. PCI bus has to be explicitly specified.
> - Check for the capability in correct place, in v2 it actually did not
>   work
> Changes since v1:
> - Added capability based on qemu version number
> - Recognize also "virt-" prefix
> 
> Pavel Fedin (4):
>   qemu: Introduce QEMU_CAPS_OBJECT_GPEX
>   Add PCI-Express root to ARM virt machine
>   Build correct command line for PCI NICs on ARM
>   Allow to plug virtio-net-pci into PCIe slot
> 
>  src/qemu/qemu_capabilities.c | 10 ++
>  src/qemu/qemu_capabilities.h |  1 +
>  src/qemu/qemu_command.c  | 11 ++-
>  src/qemu/qemu_domain.c   | 17 +
>  4 files changed, 34 insertions(+), 5 deletions(-)
> 
> --
> 1.9.5.msysgit.0
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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

Re: [libvirt] [PATCH v2 2/3] Avoid starting a PowerPC VM with floppy disk

[Ján Tomko]

On Mon, Aug 03, 2015 at 11:27:56PM +0530, madhu pavan wrote:
> 
> 
> On 08/03/2015 06:23 PM, Ján Tomko wrote:
> > On Thu, Jul 30, 2015 at 07:55:36AM -0400, Kothapally Madhu Pavan wrote:
> >> PowerPC pseries based VMs do not support a floppy disk controller.
> > Here the commit message mentions a controller, but the code only checks
> > for the  device,
> > not for 
> >
> > Should we forbid that one too? AFAIK we've auto-added it to the XML
> > only if there was at least floppy.
> We need not forbid fdc, as checking for floppy device will fulfill the 
> need. Yes, fdc is added only if there was at least one floppy device.
> 

Okay, I've squashed the test together with 2/3 and pushed the series.

Jan


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

[libvirt] [PATCH 15/18] cpu: Add PVR mask to CPU map XML for ppc64 models

[Andrea Bolognani]

The code currently assumes that the mask will be 0x, which
is not always the case - it is for the models listed so far, though.
---
 src/cpu/cpu_map.xml | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index b3c4477..0895ada 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -1360,30 +1360,30 @@
 
 
   
-  
+  
 
 
 
   
-  
-  
+  
+  
 
 
 
   
-  
-  
+  
+  
 
 
 
 
   
-  
+  
 
 
 
   
-  
+  
 
   
 
-- 
2.4.3

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

[libvirt] [PATCH 17/18] cpu: Add POWER8 NVL information to CPU map XML

[Andrea Bolognani]

This is yet another variation of POWER8. The PVR information comes
from arch/powerpc/kernel/cputable.c in the Linux kernel tree.
---
 src/cpu/cpu_map.xml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 0895ada..d2469ee 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -1372,6 +1372,7 @@
 
   
   
+  
   
 
 
-- 
2.4.3

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

[libvirt] [PATCH 18/18] tests: Add a bunch of ppc64 cases to the cpu test

[Andrea Bolognani]

New test cases cover the cpuCompare() and cpuBaseline() implementation.
---
 tests/cputest.c  | 10 ++
 tests/cputestdata/ppc64-baseline-incompatible-models.xml | 14 ++
 tests/cputestdata/ppc64-baseline-same-model-result.xml   |  3 +++
 tests/cputestdata/ppc64-baseline-same-model.xml  | 14 ++
 tests/cputestdata/ppc64-host-better.xml  |  6 ++
 tests/cputestdata/ppc64-host-incomp-arch.xml |  6 ++
 tests/cputestdata/ppc64-host-no-vendor.xml   |  5 +
 tests/cputestdata/ppc64-host-worse.xml   |  6 ++
 8 files changed, 64 insertions(+)
 create mode 100644 tests/cputestdata/ppc64-baseline-incompatible-models.xml
 create mode 100644 tests/cputestdata/ppc64-baseline-same-model-result.xml
 create mode 100644 tests/cputestdata/ppc64-baseline-same-model.xml
 create mode 100644 tests/cputestdata/ppc64-host-better.xml
 create mode 100644 tests/cputestdata/ppc64-host-incomp-arch.xml
 create mode 100644 tests/cputestdata/ppc64-host-no-vendor.xml
 create mode 100644 tests/cputestdata/ppc64-host-worse.xml

diff --git a/tests/cputest.c b/tests/cputest.c
index 82999f8..5f17145 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -563,6 +563,13 @@ mymain(void)
 DO_TEST_COMPARE("x86", "host", "host-no-vendor", 
VIR_CPU_COMPARE_IDENTICAL);
 DO_TEST_COMPARE("x86", "host-no-vendor", "host", 
VIR_CPU_COMPARE_INCOMPATIBLE);
 
+DO_TEST_COMPARE("ppc64", "host", "host", VIR_CPU_COMPARE_IDENTICAL);
+DO_TEST_COMPARE("ppc64", "host", "host-better", 
VIR_CPU_COMPARE_INCOMPATIBLE);
+DO_TEST_COMPARE("ppc64", "host", "host-worse", 
VIR_CPU_COMPARE_INCOMPATIBLE);
+DO_TEST_COMPARE("ppc64", "host", "host-incomp-arch", 
VIR_CPU_COMPARE_INCOMPATIBLE);
+DO_TEST_COMPARE("ppc64", "host", "host-no-vendor", 
VIR_CPU_COMPARE_IDENTICAL);
+DO_TEST_COMPARE("ppc64", "host-no-vendor", "host", 
VIR_CPU_COMPARE_INCOMPATIBLE);
+
 /* guest to host comparison */
 DO_TEST_COMPARE("x86", "host", "bogus-model", VIR_CPU_COMPARE_ERROR);
 DO_TEST_COMPARE("x86", "host", "bogus-feature", VIR_CPU_COMPARE_ERROR);
@@ -619,6 +626,9 @@ mymain(void)
 
 DO_TEST_BASELINE("ppc64", "incompatible-vendors", 0, -1);
 DO_TEST_BASELINE("ppc64", "no-vendor", 0, 0);
+DO_TEST_BASELINE("ppc64", "incompatible-models", 0, -1);
+DO_TEST_BASELINE("ppc64", "same-model", 0, 0);
+
 /* CPU features */
 DO_TEST_HASFEATURE("x86", "host", "vmx", YES);
 DO_TEST_HASFEATURE("x86", "host", "lm", YES);
diff --git a/tests/cputestdata/ppc64-baseline-incompatible-models.xml 
b/tests/cputestdata/ppc64-baseline-incompatible-models.xml
new file mode 100644
index 000..7e7b9a6
--- /dev/null
+++ b/tests/cputestdata/ppc64-baseline-incompatible-models.xml
@@ -0,0 +1,14 @@
+
+
+  ppc64
+  POWER7
+  IBM
+  
+
+
+  ppc64
+  POWER8
+  IBM
+  
+
+
diff --git a/tests/cputestdata/ppc64-baseline-same-model-result.xml 
b/tests/cputestdata/ppc64-baseline-same-model-result.xml
new file mode 100644
index 000..dc0c862
--- /dev/null
+++ b/tests/cputestdata/ppc64-baseline-same-model-result.xml
@@ -0,0 +1,3 @@
+
+  POWER8
+
diff --git a/tests/cputestdata/ppc64-baseline-same-model.xml 
b/tests/cputestdata/ppc64-baseline-same-model.xml
new file mode 100644
index 000..dceae83
--- /dev/null
+++ b/tests/cputestdata/ppc64-baseline-same-model.xml
@@ -0,0 +1,14 @@
+
+
+  ppc64
+  POWER8
+  IBM
+  
+
+
+  ppc64
+  POWER8
+  IBM
+  
+
+
diff --git a/tests/cputestdata/ppc64-host-better.xml 
b/tests/cputestdata/ppc64-host-better.xml
new file mode 100644
index 000..af87412
--- /dev/null
+++ b/tests/cputestdata/ppc64-host-better.xml
@@ -0,0 +1,6 @@
+
+  ppc64
+  POWER8
+  IBM
+  
+
diff --git a/tests/cputestdata/ppc64-host-incomp-arch.xml 
b/tests/cputestdata/ppc64-host-incomp-arch.xml
new file mode 100644
index 000..195f436
--- /dev/null
+++ b/tests/cputestdata/ppc64-host-incomp-arch.xml
@@ -0,0 +1,6 @@
+
+  x86_64
+  POWER7
+  IBM
+  
+
diff --git a/tests/cputestdata/ppc64-host-no-vendor.xml 
b/tests/cputestdata/ppc64-host-no-vendor.xml
new file mode 100644
index 000..de73006
--- /dev/null
+++ b/tests/cputestdata/ppc64-host-no-vendor.xml
@@ -0,0 +1,5 @@
+
+  ppc64
+  POWER7
+  
+
diff --git a/tests/cputestdata/ppc64-host-worse.xml 
b/tests/cputestdata/ppc64-host-worse.xml
new file mode 100644
index 000..ba1806b
--- /dev/null
+++ b/tests/cputestdata/ppc64-host-worse.xml
@@ -0,0 +1,6 @@
+
+  ppc64
+  POWER6
+  IBM
+  
+
-- 
2.4.3

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

[libvirt] [PATCH 11/18] cpu: Use ppc64Compute() to implement ppc64DriverCompare()

[Andrea Bolognani]

This ensures comparison of two CPU definitions will be consistent
regardless of the fact that it is performed using cpuCompare() or
cpuGuestData(). The x86 driver uses the same exact code.
---
 src/cpu/cpu_ppc64.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 09ff386..4428a55 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -449,16 +449,22 @@ ppc64DriverCompare(virCPUDefPtr host,
virCPUDefPtr cpu,
bool failIncompatible)
 {
-if ((cpu->arch == VIR_ARCH_NONE || host->arch == cpu->arch) &&
-STREQ(host->model, cpu->model))
-return VIR_CPU_COMPARE_IDENTICAL;
+virCPUCompareResult ret;
+char *message = NULL;
 
-if (failIncompatible) {
-virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL);
-return VIR_CPU_COMPARE_ERROR;
-} else {
-return VIR_CPU_COMPARE_INCOMPATIBLE;
+ret = ppc64Compute(host, cpu, NULL, &message);
+
+if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) {
+ret = VIR_CPU_COMPARE_ERROR;
+if (message) {
+virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message);
+} else {
+virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL);
+}
 }
+VIR_FREE(message);
+
+return ret;
 }
 
 static int
-- 
2.4.3

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

[libvirt] [PATCH 16/18] cpu: Parse and use PVR masks in the ppc64 driver

[Andrea Bolognani]

Instead of relying on a hard-coded mask value, read it from the CPU
map XML and use it when looking up models by PVR.

Rewrite ppc64DriverNodeData() to use this feature.
---
 src/cpu/cpu_ppc64.c  | 74 
 src/cpu/cpu_ppc64_data.h |  1 +
 2 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 9351729..add5ede 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -84,8 +84,10 @@ ppc64DataCopy(virCPUppc64Data *data)
 
 copy->len = data->len;
 
-for (i = 0; i < data->len; i++)
+for (i = 0; i < data->len; i++) {
 copy->pvr[i].value = data->pvr[i].value;
+copy->pvr[i].mask = data->pvr[i].mask;
+}
 
 return copy;
 
@@ -185,20 +187,12 @@ ppc64ModelFindPVR(const struct ppc64_map *map,
 model = map->models;
 while (model) {
 for (i = 0; i < model->data->len; i++)
-if (model->data->pvr[i].value == pvr)
+if ((pvr & model->data->pvr[i].mask) == model->data->pvr[i].value)
 return model;
 
 model = model->next;
 }
 
-/* PowerPC Processor Version Register is interpreted as follows :
- * Higher order 16 bits : Power ISA generation.
- * Lower order 16 bits : CPU chip version number.
- * If the exact CPU isn't found, return the nearest matching CPU generation
- */
-if (pvr & 0xul)
-return ppc64ModelFindPVR(map, (pvr & 0xul));
-
 return NULL;
 }
 
@@ -364,6 +358,24 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
 model->data->pvr[i].value = pvr;
 
 VIR_FREE(prop);
+
+if (!(prop = virXMLPropString(nodes[i], "mask"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Missing PVR mask in CPU model %s"),
+   model->name);
+goto ignore;
+}
+
+if (virStrToLong_ul(prop, NULL, 16, &pvr) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Invalid PVR mask in CPU model %s"),
+   model->name);
+goto ignore;
+}
+
+model->data->pvr[i].mask = pvr;
+
+VIR_FREE(prop);
 }
 
 if (!map->models) {
@@ -607,33 +619,33 @@ ppc64DriverFree(virCPUDataPtr data)
 static virCPUDataPtr
 ppc64DriverNodeData(virArch arch)
 {
-virCPUDataPtr cpuData;
-
-if (VIR_ALLOC(cpuData) < 0)
-goto error;
-
-if (VIR_ALLOC(cpuData->data.ppc64) < 0)
-goto error;
-
-if (VIR_ALLOC_N(cpuData->data.ppc64->pvr, 1) < 0)
-goto error;
-
-cpuData->data.ppc64->len = 1;
-
-cpuData->arch = arch;
+virCPUDataPtr nodeData = NULL;
+struct ppc64_map *map = NULL;
+struct ppc64_model *model = NULL;
+uint32_t pvr = 0;
 
 #if defined(__powerpc__) || defined(__powerpc64__)
 asm("mfpvr %0"
-: "=r" (cpuData->data.ppc64->pvr[0].value));
+: "=r" (pvr));
 #endif
 
-return cpuData;
+if (!(map = ppc64LoadMap()))
+goto cleanup;
 
- error:
-if (cpuData)
-ppc64DataFree(cpuData->data.ppc64);
-VIR_FREE(cpuData);
-return NULL;
+if (!(model = ppc64ModelFindPVR(map, pvr))) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("Cannot find CPU model with PVR 0x%08x"),
+   pvr);
+goto cleanup;
+}
+
+if (!(nodeData = ppc64MakeCPUData(arch, model->data)))
+goto cleanup;
+
+ cleanup:
+ppc64MapFree(map);
+
+return nodeData;
 }
 
 static virCPUCompareResult
diff --git a/src/cpu/cpu_ppc64_data.h b/src/cpu/cpu_ppc64_data.h
index 0d3cb0b..c0a130e 100644
--- a/src/cpu/cpu_ppc64_data.h
+++ b/src/cpu/cpu_ppc64_data.h
@@ -29,6 +29,7 @@
 typedef struct _virCPUppc64PVR virCPUppc64PVR;
 struct _virCPUppc64PVR {
 uint32_t value;
+uint32_t mask;
 };
 
 typedef struct _virCPUppc64Data virCPUppc64Data;
-- 
2.4.3

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

[libvirt] [PATCH 04/18] cpu: Use a different name for the copy in ppc64ModelFromCPU()

[Andrea Bolognani]

While the previous code was correct, it looked wrong at first
sight because the same variable used to store the result of a
map lookup is later used to store a copy of said result. The
copy is deallocated on error, but due to the fact that a single
variable is used, it looks like the result of the lookup is
deallocated instead, which would be a bug.

Using a separate variable for the copy means the code is just
as correct but much less likely to result confusing.

No functional changes.
---
 src/cpu/cpu_ppc64.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index dd02a3f..c769221 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -162,6 +162,7 @@ ppc64ModelFromCPU(const virCPUDef *cpu,
   const struct ppc64_map *map)
 {
 struct ppc64_model *model;
+struct ppc64_model *copy;
 
 if (!(model = ppc64ModelFind(map, cpu->model))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -169,13 +170,13 @@ ppc64ModelFromCPU(const virCPUDef *cpu,
 goto error;
 }
 
-if (!(model = ppc64ModelCopy(model)))
+if (!(copy = ppc64ModelCopy(model)))
 goto error;
 
-return model;
+return copy;
 
  error:
-ppc64ModelFree(model);
+ppc64ModelFree(copy);
 return NULL;
 }
 
-- 
2.4.3

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

[libvirt] [PATCH 10/18] cpu: CPU model names have to match on ppc64

[Andrea Bolognani]

Limitations of the POWER architecture mean that you can't run
eg. a POWER7 guest on a POWER8 host when using KVM. This applies
to all guests, not just those using VIR_CPU_MATCH_STRICT in the
CPU definition; in fact, exact and strict CPU matching are
basically the same on ppc64.

This means, of course, that hosts using different CPUs have to be
considered incompatible as well.

Change ppc64Compute(), called by cpuGuestData(), to reflect this
fact and update test cases accordingly.
---
 src/cpu/cpu_ppc64.c  | 5 +
 tests/cputest.c  | 4 ++--
 tests/cputestdata/ppc64-guest.xml| 2 +-
 tests/cputestdata/ppc64-host+guest,ppc_models-result.xml | 2 +-
 .../ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml| 5 -
 5 files changed, 5 insertions(+), 13 deletions(-)
 delete mode 100644 
tests/cputestdata/ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 633515f..09ff386 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -361,7 +361,6 @@ ppc64Compute(virCPUDefPtr host,
  const virCPUDef *cpu,
  virCPUDataPtr *guestData,
  char **message)
-
 {
 struct ppc64_map *map = NULL;
 struct ppc64_model *host_model = NULL;
@@ -418,9 +417,7 @@ ppc64Compute(virCPUDefPtr host,
 !(guest_model = ppc64ModelFromCPU(cpu, map)))
 goto cleanup;
 
-if (cpu->type == VIR_CPU_TYPE_GUEST &&
-cpu->match == VIR_CPU_MATCH_STRICT &&
-STRNEQ(guest_model->name, host_model->name)) {
+if (STRNEQ(guest_model->name, host_model->name)) {
 VIR_DEBUG("host CPU model does not match required CPU model %s",
   guest_model->name);
 if (message &&
diff --git a/tests/cputest.c b/tests/cputest.c
index 5b7de0f..ec867a7 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -493,7 +493,7 @@ static const char *model486[]   = { "486" };
 static const char *nomodel[]= { "nomodel" };
 static const char *models[] = { "qemu64", "core2duo", "Nehalem" };
 static const char *haswell[]= { "SandyBridge", "Haswell" };
-static const char *ppc_models[] = { "POWER7", "POWER7_v2.1", 
"POWER8_v1.0"};
+static const char *ppc_models[] = { "POWER7", "POWER7_v2.1", "POWER7_v2.3", 
"POWER8_v1.0"};
 
 static int
 mymain(void)
@@ -654,7 +654,7 @@ mymain(void)
   NULL, "Haswell-noTSX", 0);
 
 DO_TEST_GUESTDATA("ppc64", "host", "guest", ppc_models, NULL, 0);
-DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, 
"POWER7_v2.1", 0);
+DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, 
"POWER7_v2.1", -1);
 
 return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
diff --git a/tests/cputestdata/ppc64-guest.xml 
b/tests/cputestdata/ppc64-guest.xml
index ac81ec0..9e91501 100644
--- a/tests/cputestdata/ppc64-guest.xml
+++ b/tests/cputestdata/ppc64-guest.xml
@@ -1,4 +1,4 @@
 
-  POWER8_v1.0
+  POWER7_v2.3
   
 
diff --git a/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml 
b/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml
index 0cb0830..3e55f68 100644
--- a/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml
+++ b/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml
@@ -1,5 +1,5 @@
 
   ppc64
-  POWER8_v1.0
+  POWER7_v2.3
   IBM
 
diff --git 
a/tests/cputestdata/ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml
 
b/tests/cputestdata/ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml
deleted file mode 100644
index 7e58361..000
--- 
a/tests/cputestdata/ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml
+++ /dev/null
@@ -1,5 +0,0 @@
-
-  ppc64
-  POWER7_v2.1
-  IBM
-
-- 
2.4.3

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

[libvirt] [PATCH 01/18] cpu: Mark driver functions in ppc64 driver

[Andrea Bolognani]

Use the ppc64Driver prefix for all functions that are used to
fill in the cpuDriverPPC64 structure, ie. those that are going
to be called by the generic CPU code.

This makes it clear which functions are exported and which are
implementation details; it also gets rid of the ambiguity that
affected the ppc64DataFree() function which, despite what the
name suggested, was not related to ppc64DataCopy() and could
not be used to release the memory allocated for a
virCPUppc64Data* instance.

No functional changes.
---
 src/cpu/cpu_ppc64.c | 62 ++---
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index c3a51fb..5140297 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -448,9 +448,9 @@ ppc64Compute(virCPUDefPtr host,
 }
 
 static virCPUCompareResult
-ppc64Compare(virCPUDefPtr host,
- virCPUDefPtr cpu,
- bool failIncompatible)
+ppc64DriverCompare(virCPUDefPtr host,
+   virCPUDefPtr cpu,
+   bool failIncompatible)
 {
 if ((cpu->arch == VIR_ARCH_NONE || host->arch == cpu->arch) &&
 STREQ(host->model, cpu->model))
@@ -465,12 +465,12 @@ ppc64Compare(virCPUDefPtr host,
 }
 
 static int
-ppc64Decode(virCPUDefPtr cpu,
-const virCPUData *data,
-const char **models,
-unsigned int nmodels,
-const char *preferred ATTRIBUTE_UNUSED,
-unsigned int flags)
+ppc64DriverDecode(virCPUDefPtr cpu,
+  const virCPUData *data,
+  const char **models,
+  unsigned int nmodels,
+  const char *preferred ATTRIBUTE_UNUSED,
+  unsigned int flags)
 {
 int ret = -1;
 struct ppc64_map *map;
@@ -510,7 +510,7 @@ ppc64Decode(virCPUDefPtr cpu,
 
 
 static void
-ppc64DataFree(virCPUDataPtr data)
+ppc64DriverFree(virCPUDataPtr data)
 {
 if (data == NULL)
 return;
@@ -519,7 +519,7 @@ ppc64DataFree(virCPUDataPtr data)
 }
 
 static virCPUDataPtr
-ppc64NodeData(virArch arch)
+ppc64DriverNodeData(virArch arch)
 {
 virCPUDataPtr cpuData;
 
@@ -537,17 +537,17 @@ ppc64NodeData(virArch arch)
 }
 
 static virCPUCompareResult
-ppc64GuestData(virCPUDefPtr host,
-   virCPUDefPtr guest,
-   virCPUDataPtr *data,
-   char **message)
+ppc64DriverGuestData(virCPUDefPtr host,
+ virCPUDefPtr guest,
+ virCPUDataPtr *data,
+ char **message)
 {
 return ppc64Compute(host, guest, data, message);
 }
 
 static int
-ppc64Update(virCPUDefPtr guest,
-const virCPUDef *host)
+ppc64DriverUpdate(virCPUDefPtr guest,
+  const virCPUDef *host)
 {
 switch ((virCPUMode) guest->mode) {
 case VIR_CPU_MODE_HOST_MODEL:
@@ -569,11 +569,11 @@ ppc64Update(virCPUDefPtr guest,
 }
 
 static virCPUDefPtr
-ppc64Baseline(virCPUDefPtr *cpus,
-  unsigned int ncpus,
-  const char **models ATTRIBUTE_UNUSED,
-  unsigned int nmodels ATTRIBUTE_UNUSED,
-  unsigned int flags)
+ppc64DriverBaseline(virCPUDefPtr *cpus,
+unsigned int ncpus,
+const char **models ATTRIBUTE_UNUSED,
+unsigned int nmodels ATTRIBUTE_UNUSED,
+unsigned int flags)
 {
 struct ppc64_map *map = NULL;
 const struct ppc64_model *model;
@@ -653,7 +653,7 @@ ppc64Baseline(virCPUDefPtr *cpus,
 }
 
 static int
-ppc64GetModels(char ***models)
+ppc64DriverGetModels(char ***models)
 {
 struct ppc64_map *map;
 struct ppc64_model *model;
@@ -699,14 +699,14 @@ struct cpuArchDriver cpuDriverPPC64 = {
 .name   = "ppc64",
 .arch   = archs,
 .narch  = ARRAY_CARDINALITY(archs),
-.compare= ppc64Compare,
-.decode = ppc64Decode,
+.compare= ppc64DriverCompare,
+.decode = ppc64DriverDecode,
 .encode = NULL,
-.free   = ppc64DataFree,
-.nodeData   = ppc64NodeData,
-.guestData  = ppc64GuestData,
-.baseline   = ppc64Baseline,
-.update = ppc64Update,
+.free   = ppc64DriverFree,
+.nodeData   = ppc64DriverNodeData,
+.guestData  = ppc64DriverGuestData,
+.baseline   = ppc64DriverBaseline,
+.update = ppc64DriverUpdate,
 .hasFeature = NULL,
-.getModels  = ppc64GetModels,
+.getModels  = ppc64DriverGetModels,
 };
-- 
2.4.3

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

[libvirt] [PATCH 13/18] cpu: Support multiple PVRs in the ppc64 driver

[Andrea Bolognani]

This will allow us to perform PVR matching more broadly, eg. consider
both POWER8 and POWER8E CPUs to be the same even though they have
different PVR values.
---
 src/cpu/cpu_ppc64.c  | 73 
 src/cpu/cpu_ppc64_data.h |  8 +-
 2 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index d186d4c..9351729 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -63,6 +63,7 @@ ppc64DataFree(virCPUppc64Data *data)
 if (!data)
 return;
 
+VIR_FREE(data->pvr);
 VIR_FREE(data);
 }
 
@@ -70,16 +71,27 @@ static virCPUppc64Data *
 ppc64DataCopy(virCPUppc64Data *data)
 {
 virCPUppc64Data *copy;
+size_t i;
 
 if (!data)
 return NULL;
 
 if (VIR_ALLOC(copy) < 0)
-return NULL;
+goto error;
+
+if (VIR_ALLOC_N(copy->pvr, data->len) < 0)
+goto error;
+
+copy->len = data->len;
 
-copy->pvr = data->pvr;
+for (i = 0; i < data->len; i++)
+copy->pvr[i].value = data->pvr[i].value;
 
 return copy;
+
+ error:
+ppc64DataFree(copy);
+return NULL;
 }
 
 static void
@@ -168,11 +180,13 @@ ppc64ModelFindPVR(const struct ppc64_map *map,
   uint32_t pvr)
 {
 struct ppc64_model *model;
+size_t i;
 
 model = map->models;
 while (model) {
-if (model->data->pvr == pvr)
-return model;
+for (i = 0; i < model->data->len; i++)
+if (model->data->pvr[i].value == pvr)
+return model;
 
 model = model->next;
 }
@@ -274,8 +288,12 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
struct ppc64_map *map)
 {
 struct ppc64_model *model;
+xmlNodePtr *nodes = NULL;
 char *vendor = NULL;
+char *prop = NULL;
 unsigned long pvr;
+size_t i;
+int n;
 
 if (VIR_ALLOC(model) < 0)
 return -1;
@@ -315,14 +333,38 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
 }
 }
 
-if (!virXPathBoolean("boolean(./pvr)", ctxt) ||
-virXPathULongHex("string(./pvr/@value)", ctxt, &pvr) < 0) {
+if ((n = virXPathNodeSet("./pvr", ctxt, &nodes)) <= 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Missing or invalid PVR value in CPU model %s"),
+   _("Missing PVR information for CPU model %s"),
model->name);
 goto ignore;
 }
-model->data->pvr = pvr;
+
+if (VIR_ALLOC_N(model->data->pvr, n) < 0)
+goto ignore;
+
+model->data->len = n;
+
+for (i = 0; i < n; i++) {
+
+if (!(prop = virXMLPropString(nodes[i], "value"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Missing PVR value in CPU model %s"),
+   model->name);
+goto ignore;
+}
+
+if (virStrToLong_ul(prop, NULL, 16, &pvr) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Invalid PVR value in CPU model %s"),
+   model->name);
+goto ignore;
+}
+
+model->data->pvr[i].value = pvr;
+
+VIR_FREE(prop);
+}
 
 if (!map->models) {
 map->models = model;
@@ -332,7 +374,9 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
 }
 
  cleanup:
+VIR_FREE(prop);
 VIR_FREE(vendor);
+VIR_FREE(nodes);
 return 0;
 
  ignore:
@@ -523,10 +567,10 @@ ppc64DriverDecode(virCPUDefPtr cpu,
 if (!data || !(map = ppc64LoadMap()))
 return -1;
 
-if (!(model = ppc64ModelFindPVR(map, data->data.ppc64->pvr))) {
+if (!(model = ppc64ModelFindPVR(map, data->data.ppc64->pvr[0].value))) {
 virReportError(VIR_ERR_OPERATION_FAILED,
_("Cannot find CPU model with PVR 0x%08x"),
-   data->data.ppc64->pvr);
+   data->data.ppc64->pvr[0].value);
 goto cleanup;
 }
 
@@ -571,16 +615,23 @@ ppc64DriverNodeData(virArch arch)
 if (VIR_ALLOC(cpuData->data.ppc64) < 0)
 goto error;
 
+if (VIR_ALLOC_N(cpuData->data.ppc64->pvr, 1) < 0)
+goto error;
+
+cpuData->data.ppc64->len = 1;
+
 cpuData->arch = arch;
 
 #if defined(__powerpc__) || defined(__powerpc64__)
 asm("mfpvr %0"
-: "=r" (cpuData->data.ppc64->pvr));
+: "=r" (cpuData->data.ppc64->pvr[0].value));
 #endif
 
 return cpuData;
 
  error:
+if (cpuData)
+ppc64DataFree(cpuData->data.ppc64);
 VIR_FREE(cpuData);
 return NULL;
 }
diff --git a/src/cpu/cpu_ppc64_data.h b/src/cpu/cpu_ppc64_data.h
index c18fc63..0d3cb0b 100644
--- a/src/cpu/cpu_ppc64_data.h
+++ b/src/cpu/cpu_ppc64_data.h
@@ -26,9 +26,15 @@
 
 # include 
 
+typedef struct _virCPUppc64PVR virCPUppc64PVR;
+struct _virCPUppc64PVR {
+uint32_t value;
+};
+
 typedef struct _virCPUppc64Data virCPUppc64Data;
 struct _virCPUppc64Data {
-uint32_t pvr;
+size_t len;
+virCPUppc64PVR *pvr;
 };
 
 #endi

[libvirt] [PATCH 08/18] tests: Known failing tests should never succeed

[Andrea Bolognani]

Fix a test case that was wrongly expected to fail as well.
---
 tests/cputest.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/tests/cputest.c b/tests/cputest.c
index 06b3f12..5b7de0f 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -273,13 +273,8 @@ cpuTestGuestData(const void *arg)
 guest->match = VIR_CPU_MATCH_EXACT;
 guest->fallback = cpu->fallback;
 if (cpuDecode(guest, guestData, data->models,
-  data->nmodels, data->preferred) < 0) {
-if (data->result < 0) {
-virResetLastError();
-ret = 0;
-}
+  data->nmodels, data->preferred) < 0)
 goto cleanup;
-}
 
 virBufferAsprintf(&buf, "%s+%s", data->host, data->name);
 if (data->nmodels)
@@ -302,6 +297,19 @@ cpuTestGuestData(const void *arg)
 virCPUDefFree(host);
 virCPUDefFree(cpu);
 virCPUDefFree(guest);
+
+if (data->result < 0) {
+virResetLastError();
+if (ret < 0) {
+ret = 0;
+} else {
+VIR_TEST_VERBOSE("\n%-70s... ",
+ "cpuGuestData/cpuDecode was expected "
+ "to fail but it succeeded");
+ret = -1;
+}
+}
+
 return ret;
 }
 
@@ -646,7 +654,7 @@ mymain(void)
   NULL, "Haswell-noTSX", 0);
 
 DO_TEST_GUESTDATA("ppc64", "host", "guest", ppc_models, NULL, 0);
-DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, 
"POWER7_v2.1", -1);
+DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, 
"POWER7_v2.1", 0);
 
 return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
-- 
2.4.3

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

[libvirt] [PATCH 06/18] cpu: Remove ISA information from CPU map XML

[Andrea Bolognani]

The information is not used anywhere in libvirt.

No functional changes.
---
 src/cpu/cpu_map.xml | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index b924bd3..6387ce4 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -1385,31 +1385,26 @@
 
 
   
-  
   
 
 
 
   
-  
   
 
 
 
   
-  
   
 
 
 
   
-  
   
 
 
 
   
-  
   
 
 
-- 
2.4.3

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

[libvirt] [PATCH 03/18] cpu: Add NULL check in ppc64ModelCopy()

[Andrea Bolognani]

---
 src/cpu/cpu_ppc64.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 05ff8f2..dd02a3f 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -115,6 +115,9 @@ ppc64ModelCopy(const struct ppc64_model *model)
 {
 struct ppc64_model *copy;
 
+if (!model)
+return NULL;
+
 if (VIR_ALLOC(copy) < 0 ||
 VIR_STRDUP(copy->name, model->name) < 0) {
 ppc64ModelFree(copy);
-- 
2.4.3

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

[libvirt] [PATCH 05/18] cpu: Reorder functions in the ppc64 driver

[Andrea Bolognani]

Having the functions grouped together this way will avoid further
shuffling around down the line.

No functional changes.
---
 src/cpu/cpu_ppc64.c | 135 +---
 1 file changed, 66 insertions(+), 69 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index c769221..20c68ca 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -57,6 +57,32 @@ struct ppc64_map {
 struct ppc64_model *models;
 };
 
+static void
+ppc64VendorFree(struct ppc64_vendor *vendor)
+{
+if (!vendor)
+return;
+
+VIR_FREE(vendor->name);
+VIR_FREE(vendor);
+}
+
+static struct ppc64_vendor *
+ppc64VendorFind(const struct ppc64_map *map,
+const char *name)
+{
+struct ppc64_vendor *vendor;
+
+vendor = map->vendors;
+while (vendor) {
+if (STREQ(vendor->name, name))
+return vendor;
+
+vendor = vendor->next;
+}
+
+return NULL;
+}
 
 static void
 ppc64ModelFree(struct ppc64_model *model)
@@ -69,6 +95,26 @@ ppc64ModelFree(struct ppc64_model *model)
 }
 
 static struct ppc64_model *
+ppc64ModelCopy(const struct ppc64_model *model)
+{
+struct ppc64_model *copy;
+
+if (!model)
+return NULL;
+
+if (VIR_ALLOC(copy) < 0 ||
+VIR_STRDUP(copy->name, model->name) < 0) {
+ppc64ModelFree(copy);
+return NULL;
+}
+
+copy->data.pvr = model->data.pvr;
+copy->vendor = model->vendor;
+
+return copy;
+}
+
+static struct ppc64_model *
 ppc64ModelFind(const struct ppc64_map *map,
const char *name)
 {
@@ -111,53 +157,6 @@ ppc64ModelFindPVR(const struct ppc64_map *map,
 }
 
 static struct ppc64_model *
-ppc64ModelCopy(const struct ppc64_model *model)
-{
-struct ppc64_model *copy;
-
-if (!model)
-return NULL;
-
-if (VIR_ALLOC(copy) < 0 ||
-VIR_STRDUP(copy->name, model->name) < 0) {
-ppc64ModelFree(copy);
-return NULL;
-}
-
-copy->data.pvr = model->data.pvr;
-copy->vendor = model->vendor;
-
-return copy;
-}
-
-static struct ppc64_vendor *
-ppc64VendorFind(const struct ppc64_map *map,
-const char *name)
-{
-struct ppc64_vendor *vendor;
-
-vendor = map->vendors;
-while (vendor) {
-if (STREQ(vendor->name, name))
-return vendor;
-
-vendor = vendor->next;
-}
-
-return NULL;
-}
-
-static void
-ppc64VendorFree(struct ppc64_vendor *vendor)
-{
-if (!vendor)
-return;
-
-VIR_FREE(vendor->name);
-VIR_FREE(vendor);
-}
-
-static struct ppc64_model *
 ppc64ModelFromCPU(const virCPUDef *cpu,
   const struct ppc64_map *map)
 {
@@ -180,6 +179,26 @@ ppc64ModelFromCPU(const virCPUDef *cpu,
 return NULL;
 }
 
+static void
+ppc64MapFree(struct ppc64_map *map)
+{
+if (!map)
+return;
+
+while (map->models) {
+struct ppc64_model *model = map->models;
+map->models = model->next;
+ppc64ModelFree(model);
+}
+
+while (map->vendors) {
+struct ppc64_vendor *vendor = map->vendors;
+map->vendors = vendor->next;
+ppc64VendorFree(vendor);
+}
+
+VIR_FREE(map);
+}
 
 static int
 ppc64VendorLoad(xmlXPathContextPtr ctxt,
@@ -304,27 +323,6 @@ ppc64MapLoadCallback(cpuMapElement element,
 return 0;
 }
 
-static void
-ppc64MapFree(struct ppc64_map *map)
-{
-if (!map)
-return;
-
-while (map->models) {
-struct ppc64_model *model = map->models;
-map->models = model->next;
-ppc64ModelFree(model);
-}
-
-while (map->vendors) {
-struct ppc64_vendor *vendor = map->vendors;
-map->vendors = vendor->next;
-ppc64VendorFree(vendor);
-}
-
-VIR_FREE(map);
-}
-
 static struct ppc64_map *
 ppc64LoadMap(void)
 {
@@ -511,7 +509,6 @@ ppc64DriverDecode(virCPUDefPtr cpu,
 return ret;
 }
 
-
 static void
 ppc64DriverFree(virCPUDataPtr data)
 {
-- 
2.4.3

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

[libvirt] [PATCH 02/18] cpu: Simplify NULL handling in ppc64 driver

[Andrea Bolognani]

Use briefer checks, eg. (!model) instead of (model == NULL), and
avoid initializing to NULL a pointer that would be assigned in
the first line of the function anyway.

Also remove a pointless NULL assignment.

No functional changes.
---
 src/cpu/cpu_ppc64.c | 33 -
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 5140297..05ff8f2 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -61,7 +61,7 @@ struct ppc64_map {
 static void
 ppc64ModelFree(struct ppc64_model *model)
 {
-if (model == NULL)
+if (!model)
 return;
 
 VIR_FREE(model->name);
@@ -75,7 +75,7 @@ ppc64ModelFind(const struct ppc64_map *map,
 struct ppc64_model *model;
 
 model = map->models;
-while (model != NULL) {
+while (model) {
 if (STREQ(model->name, name))
 return model;
 
@@ -92,7 +92,7 @@ ppc64ModelFindPVR(const struct ppc64_map *map,
 struct ppc64_model *model;
 
 model = map->models;
-while (model != NULL) {
+while (model) {
 if (model->data.pvr == pvr)
 return model;
 
@@ -158,15 +158,15 @@ static struct ppc64_model *
 ppc64ModelFromCPU(const virCPUDef *cpu,
   const struct ppc64_map *map)
 {
-struct ppc64_model *model = NULL;
+struct ppc64_model *model;
 
-if ((model = ppc64ModelFind(map, cpu->model)) == NULL) {
+if (!(model = ppc64ModelFind(map, cpu->model))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown CPU model %s"), cpu->model);
 goto error;
 }
 
-if ((model = ppc64ModelCopy(model)) == NULL)
+if (!(model = ppc64ModelCopy(model)))
 goto error;
 
 return model;
@@ -181,7 +181,7 @@ static int
 ppc64VendorLoad(xmlXPathContextPtr ctxt,
 struct ppc64_map *map)
 {
-struct ppc64_vendor *vendor = NULL;
+struct ppc64_vendor *vendor;
 
 if (VIR_ALLOC(vendor) < 0)
 return -1;
@@ -264,7 +264,7 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
 }
 model->data.pvr = pvr;
 
-if (map->models == NULL) {
+if (!map->models) {
 map->models = model;
 } else {
 model->next = map->models;
@@ -303,16 +303,16 @@ ppc64MapLoadCallback(cpuMapElement element,
 static void
 ppc64MapFree(struct ppc64_map *map)
 {
-if (map == NULL)
+if (!map)
 return;
 
-while (map->models != NULL) {
+while (map->models) {
 struct ppc64_model *model = map->models;
 map->models = model->next;
 ppc64ModelFree(model);
 }
 
-while (map->vendors != NULL) {
+while (map->vendors) {
 struct ppc64_vendor *vendor = map->vendors;
 map->vendors = vendor->next;
 ppc64VendorFree(vendor);
@@ -350,7 +350,6 @@ ppc64MakeCPUData(virArch arch,
 
 cpuData->arch = arch;
 cpuData->data.ppc64 = *data;
-data = NULL;
 
 return cpuData;
 }
@@ -417,7 +416,7 @@ ppc64Compute(virCPUDefPtr host,
 !(guest_model = ppc64ModelFromCPU(cpu, map)))
 goto cleanup;
 
-if (guestData != NULL) {
+if (guestData) {
 if (cpu->type == VIR_CPU_TYPE_GUEST &&
 cpu->match == VIR_CPU_MATCH_STRICT &&
 STRNEQ(guest_model->name, host_model->name)) {
@@ -478,7 +477,7 @@ ppc64DriverDecode(virCPUDefPtr cpu,
 
 virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1);
 
-if (data == NULL || (map = ppc64LoadMap()) == NULL)
+if (!data || !(map = ppc64LoadMap()))
 return -1;
 
 if (!(model = ppc64ModelFindPVR(map, data->data.ppc64.pvr))) {
@@ -512,7 +511,7 @@ ppc64DriverDecode(virCPUDefPtr cpu,
 static void
 ppc64DriverFree(virCPUDataPtr data)
 {
-if (data == NULL)
+if (!data)
 return;
 
 VIR_FREE(data);
@@ -575,7 +574,7 @@ ppc64DriverBaseline(virCPUDefPtr *cpus,
 unsigned int nmodels ATTRIBUTE_UNUSED,
 unsigned int flags)
 {
-struct ppc64_map *map = NULL;
+struct ppc64_map *map;
 const struct ppc64_model *model;
 const struct ppc64_vendor *vendor = NULL;
 virCPUDefPtr cpu = NULL;
@@ -667,7 +666,7 @@ ppc64DriverGetModels(char ***models)
 goto error;
 
 model = map->models;
-while (model != NULL) {
+while (model) {
 if (models) {
 if (VIR_STRDUP(name, model->name) < 0)
 goto error;
-- 
2.4.3

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

[libvirt] [PATCH 14/18] cpu: Simplify ppc64 part of CPU map XML

[Andrea Bolognani]

Use multiple PVRs per CPU model to reduce the number of models we
need to keep track of.

Remove specific CPU models (eg. POWER7+_v2.1): the corresponding
generic CPU model (eg. POWER7) should be used instead to ensure
the guest can be booted on any compatible host.

Get rid of all the entries that did not match any of the CPU
models supported by QEMU, like power8 and power8e.
---
 src/cpu/cpu_map.xml| 39 ++
 tests/cputest.c|  4 +--
 .../ppc64-baseline-incompatible-vendors.xml|  4 +--
 .../ppc64-baseline-no-vendor-result.xml|  2 +-
 tests/cputestdata/ppc64-baseline-no-vendor.xml |  2 +-
 tests/cputestdata/ppc64-exact.xml  |  2 +-
 tests/cputestdata/ppc64-guest-nofallback.xml   |  2 +-
 tests/cputestdata/ppc64-guest.xml  |  2 +-
 .../ppc64-host+guest,ppc_models-result.xml |  2 +-
 tests/cputestdata/ppc64-host.xml   |  2 +-
 tests/cputestdata/ppc64-strict.xml |  4 +--
 11 files changed, 16 insertions(+), 49 deletions(-)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 6387ce4..b3c4477 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -1358,53 +1358,20 @@
 
 
 
-
-  
-  
-
-
-
-  
-  
-
-
-
-  
-  
-
-
-
-  
-  
-
-
-
-  
-  
-
-
-
+
   
   
 
 
-
+
   
   
-
-
-
-  
   
 
 
-
+
   
   
-
-
-
-  
   
 
 
diff --git a/tests/cputest.c b/tests/cputest.c
index ec867a7..82999f8 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -493,7 +493,7 @@ static const char *model486[]   = { "486" };
 static const char *nomodel[]= { "nomodel" };
 static const char *models[] = { "qemu64", "core2duo", "Nehalem" };
 static const char *haswell[]= { "SandyBridge", "Haswell" };
-static const char *ppc_models[] = { "POWER7", "POWER7_v2.1", "POWER7_v2.3", 
"POWER8_v1.0"};
+static const char *ppc_models[] = { "POWER6", "POWER7", "POWER8" };
 
 static int
 mymain(void)
@@ -654,7 +654,7 @@ mymain(void)
   NULL, "Haswell-noTSX", 0);
 
 DO_TEST_GUESTDATA("ppc64", "host", "guest", ppc_models, NULL, 0);
-DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, 
"POWER7_v2.1", -1);
+DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, 
"POWER6", -1);
 
 return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
diff --git a/tests/cputestdata/ppc64-baseline-incompatible-vendors.xml 
b/tests/cputestdata/ppc64-baseline-incompatible-vendors.xml
index 97d3c9c..9e67e9d 100644
--- a/tests/cputestdata/ppc64-baseline-incompatible-vendors.xml
+++ b/tests/cputestdata/ppc64-baseline-incompatible-vendors.xml
@@ -1,13 +1,13 @@
 
 
   ppc64
-  POWER7+_v2.1
+  POWER7
   Intel
   
 
 
   ppc64
-  POWER8_v1.0
+  POWER8
   Intel
   
 
diff --git a/tests/cputestdata/ppc64-baseline-no-vendor-result.xml 
b/tests/cputestdata/ppc64-baseline-no-vendor-result.xml
index 36bae52..7fac4b7 100644
--- a/tests/cputestdata/ppc64-baseline-no-vendor-result.xml
+++ b/tests/cputestdata/ppc64-baseline-no-vendor-result.xml
@@ -1,3 +1,3 @@
 
-  POWER7_v2.3
+  POWER7
 
diff --git a/tests/cputestdata/ppc64-baseline-no-vendor.xml 
b/tests/cputestdata/ppc64-baseline-no-vendor.xml
index 5e69a62..6d8dd0d 100644
--- a/tests/cputestdata/ppc64-baseline-no-vendor.xml
+++ b/tests/cputestdata/ppc64-baseline-no-vendor.xml
@@ -1,7 +1,7 @@
 
 
   ppc64
-  POWER7_v2.3
+  POWER7
   
 
 
diff --git a/tests/cputestdata/ppc64-exact.xml 
b/tests/cputestdata/ppc64-exact.xml
index c84f16a..f416a59 100644
--- a/tests/cputestdata/ppc64-exact.xml
+++ b/tests/cputestdata/ppc64-exact.xml
@@ -1,3 +1,3 @@
 
-  POWER8_v1.0
+  POWER8
 
diff --git a/tests/cputestdata/ppc64-guest-nofallback.xml 
b/tests/cputestdata/ppc64-guest-nofallback.xml
index 42026b4..603a3ab 100644
--- a/tests/cputestdata/ppc64-guest-nofallback.xml
+++ b/tests/cputestdata/ppc64-guest-nofallback.xml
@@ -1,4 +1,4 @@
 
-  POWER7_v2.1
+  POWER6
   
 
diff --git a/tests/cputestdata/ppc64-guest.xml 
b/tests/cputestdata/ppc64-guest.xml
index 9e91501..210f96d 100644
--- a/tests/cputestdata/ppc64-guest.xml
+++ b/tests/cputestdata/ppc64-guest.xml
@@ -1,4 +1,4 @@
 
-  POWER7_v2.3
+  POWER7
   
 
diff --git a/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml 
b/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml
index 3e55f68..3548c0e 100644
--- a/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml
+++ b/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml
@@ -1,5 +1,5 @@
 
   ppc64
-  POWER7_v2.3
+  POWER7
   IBM
 
diff --git a/tests/cputestdata/ppc64-host.xml b/tests/cputestdata/ppc64-host.xml
index 39cb741..0ac5c4e 100644
--- a/tests/cputestdata/ppc64-host.xml
+++ b/tests/cputestdata/ppc64-host.xml
@@ -1,6 +1,6 @@
 
   ppc64
-  POWER7_v2.3
+  POWER7
   IBM
   
 
diff --git a/test

[libvirt] [PATCH 09/18] cpu: Don't skip CPU model name check in ppc64 driver

[Andrea Bolognani]

ppc64Compute(), called by cpuNodeData(), is used not only to retrieve
the driver-specific data associated to a guest CPU definition, but
also to check whether said guest CPU is compatible with the host CPU.

If the user is not interested in the CPU data, it's perfectly fine
to pass a NULL pointer instead of a return location, and the
compatibility data returned should not be affected by this. One of
the checks, specifically the one on CPU model name, was however
only performed if the return location was non-NULL.
---
 src/cpu/cpu_ppc64.c | 31 +++
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 20c68ca..633515f 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -418,26 +418,25 @@ ppc64Compute(virCPUDefPtr host,
 !(guest_model = ppc64ModelFromCPU(cpu, map)))
 goto cleanup;
 
-if (guestData) {
-if (cpu->type == VIR_CPU_TYPE_GUEST &&
-cpu->match == VIR_CPU_MATCH_STRICT &&
-STRNEQ(guest_model->name, host_model->name)) {
-VIR_DEBUG("host CPU model does not match required CPU model %s",
-  guest_model->name);
-if (message &&
-virAsprintf(message,
-_("host CPU model does not match required "
-"CPU model %s"),
-guest_model->name) < 0)
-goto cleanup;
-
-ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+if (cpu->type == VIR_CPU_TYPE_GUEST &&
+cpu->match == VIR_CPU_MATCH_STRICT &&
+STRNEQ(guest_model->name, host_model->name)) {
+VIR_DEBUG("host CPU model does not match required CPU model %s",
+  guest_model->name);
+if (message &&
+virAsprintf(message,
+_("host CPU model does not match required "
+"CPU model %s"),
+guest_model->name) < 0)
 goto cleanup;
-}
 
+ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+goto cleanup;
+}
+
+if (guestData)
 if (!(*guestData = ppc64MakeCPUData(arch, &guest_model->data)))
 goto cleanup;
-}
 
 ret = VIR_CPU_COMPARE_IDENTICAL;
 
-- 
2.4.3

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

[libvirt] [PATCH 12/18] cpu: Align ppc64 CPU data with x86

[Andrea Bolognani]

Use a typedef instead of the plain struct and heap allocation. This
will make it easier to extend the ppc64 specific CPU data later on.
---
 src/cpu/cpu.h|  2 +-
 src/cpu/cpu_ppc64.c  | 81 ++--
 src/cpu/cpu_ppc64_data.h |  3 +-
 3 files changed, 67 insertions(+), 19 deletions(-)

diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 49d4226..7375876 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -38,7 +38,7 @@ struct _virCPUData {
 virArch arch;
 union {
 virCPUx86Data *x86;
-struct cpuPPC64Data ppc64;
+virCPUppc64Data *ppc64;
 /* generic driver needs no data */
 } data;
 };
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 4428a55..d186d4c 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -48,7 +48,7 @@ struct ppc64_vendor {
 struct ppc64_model {
 char *name;
 const struct ppc64_vendor *vendor;
-struct cpuPPC64Data data;
+virCPUppc64Data *data;
 struct ppc64_model *next;
 };
 
@@ -58,6 +58,31 @@ struct ppc64_map {
 };
 
 static void
+ppc64DataFree(virCPUppc64Data *data)
+{
+if (!data)
+return;
+
+VIR_FREE(data);
+}
+
+static virCPUppc64Data *
+ppc64DataCopy(virCPUppc64Data *data)
+{
+virCPUppc64Data *copy;
+
+if (!data)
+return NULL;
+
+if (VIR_ALLOC(copy) < 0)
+return NULL;
+
+copy->pvr = data->pvr;
+
+return copy;
+}
+
+static void
 ppc64VendorFree(struct ppc64_vendor *vendor)
 {
 if (!vendor)
@@ -90,6 +115,7 @@ ppc64ModelFree(struct ppc64_model *model)
 if (!model)
 return;
 
+ppc64DataFree(model->data);
 VIR_FREE(model->name);
 VIR_FREE(model);
 }
@@ -102,16 +128,22 @@ ppc64ModelCopy(const struct ppc64_model *model)
 if (!model)
 return NULL;
 
-if (VIR_ALLOC(copy) < 0 ||
-VIR_STRDUP(copy->name, model->name) < 0) {
-ppc64ModelFree(copy);
-return NULL;
-}
+if (VIR_ALLOC(copy) < 0)
+goto error;
+
+if (VIR_STRDUP(copy->name, model->name) < 0)
+goto error;
+
+if (!(copy->data = ppc64DataCopy(model->data)))
+goto error;
 
-copy->data.pvr = model->data.pvr;
 copy->vendor = model->vendor;
 
 return copy;
+
+ error:
+ppc64ModelFree(copy);
+return NULL;
 }
 
 static struct ppc64_model *
@@ -139,7 +171,7 @@ ppc64ModelFindPVR(const struct ppc64_map *map,
 
 model = map->models;
 while (model) {
-if (model->data.pvr == pvr)
+if (model->data->pvr == pvr)
 return model;
 
 model = model->next;
@@ -248,6 +280,11 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
 if (VIR_ALLOC(model) < 0)
 return -1;
 
+if (VIR_ALLOC(model->data) < 0) {
+ppc64ModelFree(model);
+return -1;
+}
+
 model->name = virXPathString("string(@name)", ctxt);
 if (!model->name) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -285,7 +322,7 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
model->name);
 goto ignore;
 }
-model->data.pvr = pvr;
+model->data->pvr = pvr;
 
 if (!map->models) {
 map->models = model;
@@ -329,7 +366,7 @@ ppc64LoadMap(void)
 struct ppc64_map *map;
 
 if (VIR_ALLOC(map) < 0)
-return NULL;
+goto error;
 
 if (cpuMapLoad("ppc64", ppc64MapLoadCallback, map) < 0)
 goto error;
@@ -343,7 +380,7 @@ ppc64LoadMap(void)
 
 static virCPUDataPtr
 ppc64MakeCPUData(virArch arch,
- struct cpuPPC64Data *data)
+ virCPUppc64Data *data)
 {
 virCPUDataPtr cpuData;
 
@@ -351,7 +388,9 @@ ppc64MakeCPUData(virArch arch,
 return NULL;
 
 cpuData->arch = arch;
-cpuData->data.ppc64 = *data;
+
+if (!(cpuData->data.ppc64 = ppc64DataCopy(data)))
+VIR_FREE(cpuData);
 
 return cpuData;
 }
@@ -432,7 +471,7 @@ ppc64Compute(virCPUDefPtr host,
 }
 
 if (guestData)
-if (!(*guestData = ppc64MakeCPUData(arch, &guest_model->data)))
+if (!(*guestData = ppc64MakeCPUData(arch, guest_model->data)))
 goto cleanup;
 
 ret = VIR_CPU_COMPARE_IDENTICAL;
@@ -484,10 +523,10 @@ ppc64DriverDecode(virCPUDefPtr cpu,
 if (!data || !(map = ppc64LoadMap()))
 return -1;
 
-if (!(model = ppc64ModelFindPVR(map, data->data.ppc64.pvr))) {
+if (!(model = ppc64ModelFindPVR(map, data->data.ppc64->pvr))) {
 virReportError(VIR_ERR_OPERATION_FAILED,
_("Cannot find CPU model with PVR 0x%08x"),
-   data->data.ppc64.pvr);
+   data->data.ppc64->pvr);
 goto cleanup;
 }
 
@@ -517,6 +556,7 @@ ppc64DriverFree(virCPUDataPtr data)
 if (!data)
 return;
 
+ppc64DataFree(data->data.ppc64);
 VIR_FREE(data);
 }
 
@@ -526,16 +566,23 @@ ppc64DriverNodeData(virArch arch)
 virCPUDataPtr cpuData;
 
 if (VIR_ALLOC(cpuData) < 0)
-return NULL;
+goto error;
+
+if (VIR_ALLO

[libvirt] [PATCH 07/18] tests: Remove unused file

[Andrea Bolognani]

---
 tests/cputestdata/ppc64-baseline-1-result.xml | 3 ---
 1 file changed, 3 deletions(-)
 delete mode 100644 tests/cputestdata/ppc64-baseline-1-result.xml

diff --git a/tests/cputestdata/ppc64-baseline-1-result.xml 
b/tests/cputestdata/ppc64-baseline-1-result.xml
deleted file mode 100644
index cbdd9bc..000
--- a/tests/cputestdata/ppc64-baseline-1-result.xml
+++ /dev/null
@@ -1,3 +0,0 @@
-
-  POWER7+_v2.1
-
-- 
2.4.3

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

[libvirt] [PATCH 00/18] cpu: Fix and improve the ppc64 driver

[Andrea Bolognani]

This series depends on

  [PATCH 0/4] cpu: Rename {powerpc,ppc} => ppc64

which has been already ACKed[1] and is pending upload.

Patches 1-11 are fixes and cleanups that don't introduce
any new feature in the driver and just lay the groundwork
for the second part of the series.

Patches 12-18 rework the driver and add some test cases
to cover both existing an new features.

Cheers.


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

Andrea Bolognani (18):
  cpu: Mark driver functions in ppc64 driver
  cpu: Simplify NULL handling in ppc64 driver
  cpu: Add NULL check in ppc64ModelCopy()
  cpu: Use a different name for the copy in ppc64ModelFromCPU()
  cpu: Reorder functions in the ppc64 driver
  cpu: Remove ISA information from CPU map XML
  tests: Remove unused file
  tests: Known failing tests should never succeed
  cpu: Don't skip CPU model name check in ppc64 driver
  cpu: CPU model names have to match on ppc64
  cpu: Use ppc64Compute() to implement ppc64DriverCompare()
  cpu: Align ppc64 CPU data with x86
  cpu: Support multiple PVRs in the ppc64 driver
  cpu: Simplify ppc64 part of CPU map XML
  cpu: Add PVR mask to CPU map XML for ppc64 models
  cpu: Parse and use PVR masks in the ppc64 driver
  cpu: Add POWER8 NVL information to CPU map XML
  tests: Add a bunch of ppc64 cases to the cpu test

 src/cpu/cpu.h  |   2 +-
 src/cpu/cpu_map.xml|  59 +--
 src/cpu/cpu_ppc64.c| 432 +
 src/cpu/cpu_ppc64_data.h   |  12 +-
 tests/cputest.c|  34 +-
 tests/cputestdata/ppc64-baseline-1-result.xml  |   3 -
 .../ppc64-baseline-incompatible-models.xml |  14 +
 .../ppc64-baseline-incompatible-vendors.xml|   4 +-
 .../ppc64-baseline-no-vendor-result.xml|   2 +-
 tests/cputestdata/ppc64-baseline-no-vendor.xml |   2 +-
 .../ppc64-baseline-same-model-result.xml   |   3 +
 tests/cputestdata/ppc64-baseline-same-model.xml|  14 +
 tests/cputestdata/ppc64-exact.xml  |   2 +-
 tests/cputestdata/ppc64-guest-nofallback.xml   |   2 +-
 tests/cputestdata/ppc64-guest.xml  |   2 +-
 .../ppc64-host+guest,ppc_models-result.xml |   2 +-
 ...st-nofallback,ppc_models,POWER7_v2.1-result.xml |   5 -
 tests/cputestdata/ppc64-host-better.xml|   6 +
 tests/cputestdata/ppc64-host-incomp-arch.xml   |   6 +
 tests/cputestdata/ppc64-host-no-vendor.xml |   5 +
 tests/cputestdata/ppc64-host-worse.xml |   6 +
 tests/cputestdata/ppc64-host.xml   |   2 +-
 tests/cputestdata/ppc64-strict.xml |   4 +-
 23 files changed, 385 insertions(+), 238 deletions(-)
 delete mode 100644 tests/cputestdata/ppc64-baseline-1-result.xml
 create mode 100644 tests/cputestdata/ppc64-baseline-incompatible-models.xml
 create mode 100644 tests/cputestdata/ppc64-baseline-same-model-result.xml
 create mode 100644 tests/cputestdata/ppc64-baseline-same-model.xml
 delete mode 100644 
tests/cputestdata/ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml
 create mode 100644 tests/cputestdata/ppc64-host-better.xml
 create mode 100644 tests/cputestdata/ppc64-host-incomp-arch.xml
 create mode 100644 tests/cputestdata/ppc64-host-no-vendor.xml
 create mode 100644 tests/cputestdata/ppc64-host-worse.xml

-- 
2.4.3

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

Re: [libvirt] [PATCH 00/13] Move generic virsh data to a separate module vsh

[Martin Kletzander]

[getting back to this after *loong* time, sorry]

On Fri, Jul 17, 2015 at 02:30:20PM +0200, Erik Skultety wrote:

Actually, I found out this is easier to review as a one patch with
various diff options used for various parts of the patch.  Some
questions and suggestions below.

Why vshClientHooks and vshCmdGrp aren't in the vshControl struct?  If
we move the client helpers into a library (I think this stuff can be
in src/util/virshell.c for example), then it won't be thread-safe.
Moving those to the control structure will also be cleaner.



It would be nice, but since readline-defined user callbacks do not
contain any opaque argument in their signature, you can only rely on
statically declared data, therefore you can't do this with command
groups, although it does work for client hooks at least.



That's fair.  Let's keep it as a static global then.


Some things are still broken up, like readline stuff.  It should be
either completely hidden or completely exposed.  For example,
vshReadline{Init,Deinit} should be moved into vsh{Init,Deinit}.

Commands from former virshCmds (except connect) should be in vsh.c so
each client can use them in their cmdGroups definition without
re-implementing them.



Well, I thought off 2 possible approaches how to tackle this one. The
first one is (though not my favorite) to simply move handler
implementations and structures initialization to vsh.c. As we do specify
all the command groups and options in fixed arrays, we would have to
create a completely new group for the generic commands available to
every client. In virsh for example, that would leave us with 'connect'
command only which we should create a separate group to make it all
work. To be honest, I don't like this idea at all.



Me neither.


The other idea however, might be nicer, but much more complex and it's
for a debate if we really want that and if so, I think it should be a
separate follow-up patch, definitely not part of v2. So, the idea is to
implement command group register mechanism using linked lists. That way,
each client would have to register every group individually. And because
lists are easily extensible, we could just add the 'connect' command to
the 'virsh' group by implementing some internal APIs to command group
management (add/update/whatever).



That's nice, but I had a different thing in mind.  I though that vsh.c
would create and export common command definitions and it'd be up to
the client whether it uses them in one of its command groups or not.
Easier than the first version and more extensible then the second one.


vsh is not doing the argument parsing, but that's be fine.  I would,
at least, wrap some options in a function that can be called from
multiple clients, but that's one of the nice-to-have things that can
be done later.


There is a reason for this. In my opinion, each client should be
responsible for parsing their own command line arguments. It doesn't
really matter that all the clients most likely will implement usage,
help, connect arguments...Being generic in this case would require each
client to provide their callbacks for generic options, their specific
options list and callbacks to handle these specific options as well.
Maybe I just don't see it the way you see it :), but I still think, in
this specific case we shouldn't try to split the code even more.



I saw it the other way around.  I thought there could be a teeny tiny
function for parsing common arguments that all clients *could* call,
but as I said, that's not necessary and not thought through.  Just a
possible future idea.


vshInit() is declared with ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
but handles passed NULLs properly, that declaration should be fixed.


Fixed in v2.


Also there are some whitespace problems (e.g. with parameters of
virshLookupDomainBy), but considering how many of them are there
inside the files already, it's nothing compared to the size of this
refactor.


I tried to fix in v2 as many as I could find.


The exclude_file_name_regexp--sc_avoid_strcase should only contain
^tools/vsh\.h$$, not virsh.h.


Fixed.

Anyway, here's a list of things that should be changed (either from
virsh to vsh or vice versa) split into categories (feel free to
disagree with any):

Totally:
- virshCommandOptTimeoutToMs

   ^^ Something tells me I overlooked this one and pushed v2 to my
forked repo without it...Sigh, v3 it is..

- VIRSH_MAX_XML_FILE
- virshPrettyCapacity
- vshFindDisk
- vshSnapshotListCollect

Most likely:
- vshCatchInt
- virshPrintJobProgress

   ^^currently this is only used for migration and blockjobs and
so far, we haven't talked about some time consuming admin jobs,
so I'd say either we leave it for good or we can move it to the lib
some time later (possibly with virsh-edit stuff, see below)



We can, yes.  I'm just really afraid of these two clients having more
and more re-implemented in both of them.  Basically I don't want these
to end up as qemu and lxc drivers :)


- virsh

[libvirt] [PATCH] qemu: Forbid image pre-creation for non-shared storage migration

[Peter Krempa]

Libvirt doesn't reliably know the location of the backing chain when
pre-creating images for non-shared migration. This isn't a problem for
full copy, but incremental copy requires the information.

Forbid pre-creating the image in cases where incremental migration is
required. This limitation can perhaps be lifted once libvirt will fully
support loading of backing chain information from the XML.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1249587
---
 src/qemu/qemu_migration.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 458b269..ff89ab5 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1613,7 +1613,8 @@ qemuMigrationPrecreateStorage(virConnectPtr conn,
   virDomainObjPtr vm,
   qemuMigrationCookieNBDPtr nbd,
   size_t nmigrate_disks,
-  const char **migrate_disks)
+  const char **migrate_disks,
+  bool incremental)
 {
 int ret = -1;
 size_t i = 0;
@@ -1644,6 +1645,13 @@ qemuMigrationPrecreateStorage(virConnectPtr conn,
 continue;
 }

+if (incremental) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+   _("pre-creation of storage targets for incremental "
+ "storage migration is not supported"));
+goto cleanup;
+}
+
 VIR_DEBUG("Proceeding with disk source %s", NULLSTR(diskSrcPath));

 if (qemuMigrationPrecreateDisk(conn, disk, nbd->disks[i].capacity) < 0)
@@ -3339,7 +3347,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 }

 if (qemuMigrationPrecreateStorage(dconn, driver, vm, mig->nbd,
-  nmigrate_disks, migrate_disks) < 0)
+  nmigrate_disks, migrate_disks,
+  !!(flags & VIR_MIGRATE_NON_SHARED_INC)) 
< 0)
 goto cleanup;

 if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
-- 
2.4.5

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

Re: [libvirt] [PATCH] Allow vfio hotplug of a device to the domain which owns the iommu

[Shivaprasad bhat]

Ping!

Thanks,
Shivaprasad

On Tue, Jul 14, 2015 at 5:26 PM, Shivaprasad G Bhat
 wrote:
> The commit 7e72de4 didn't consider the hotplug scenarios. The patch addresses
> the hotplug case whereby if atleast one of the pci function is owned by a
> guest, the hotplug of other functions/devices in the same iommu group to the
> same guest goes through successfully.
>
> Signed-off-by: Shivaprasad G Bhat 
> ---
>  src/util/virhostdev.c |   34 +++---
>  1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index 809caed..529753c 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -54,23 +54,35 @@ static virClassPtr virHostdevManagerClass;
>  static void virHostdevManagerDispose(void *obj);
>  static virHostdevManagerPtr virHostdevManagerNew(void);
>
> +struct virHostdevIsPCINodeDeviceUsedData {
> +virHostdevManagerPtr hostdev_mgr;
> +const char *domainName;
> +const bool usesVfio;
> +};
> +
>  static int virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, 
> void *opaque)
>  {
>  virPCIDevicePtr other;
>  int ret = -1;
>  virPCIDevicePtr pci = NULL;
> -virHostdevManagerPtr hostdev_mgr = opaque;
> +struct virHostdevIsPCINodeDeviceUsedData *helperData = opaque;
>
>  if (!(pci = virPCIDeviceNew(devAddr->domain, devAddr->bus,
>  devAddr->slot, devAddr->function)))
>  goto cleanup;
>
> -other = virPCIDeviceListFind(hostdev_mgr->activePCIHostdevs, pci);
> +other = virPCIDeviceListFind(helperData->hostdev_mgr->activePCIHostdevs,
> + pci);
>  if (other) {
>  const char *other_drvname = NULL;
>  const char *other_domname = NULL;
>  virPCIDeviceGetUsedBy(other, &other_drvname, &other_domname);
>
> +if (helperData->usesVfio &&
> +(other_domname && helperData->domainName) &&
> +(STREQ(other_domname, helperData->domainName)))
> +goto iommu_owner;
> +
>  if (other_drvname && other_domname)
>  virReportError(VIR_ERR_OPERATION_INVALID,
> _("PCI device %s is in use by "
> @@ -83,6 +95,7 @@ static int 
> virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void *o
> virPCIDeviceGetName(pci));
>  goto cleanup;
>  }
> + iommu_owner:
>  ret = 0;
>   cleanup:
>  virPCIDeviceFree(pci);
> @@ -562,6 +575,9 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
> hostdev_mgr,
>  for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
>  virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
>  bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK);
> +bool usesVfio = STREQ(virPCIDeviceGetStubDriver(dev), "vfio-pci");
> +struct virHostdevIsPCINodeDeviceUsedData data = {hostdev_mgr, 
> dom_name,
> + usesVfio};
>
>  if (!virPCIDeviceIsAssignable(dev, strict_acs_check)) {
>  virReportError(VIR_ERR_OPERATION_INVALID,
> @@ -579,12 +595,12 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr 
> hostdev_mgr,
>   * belonging to same iommu group can't be shared
>   * across guests.
>   */
> -if (STREQ(virPCIDeviceGetStubDriver(dev), "vfio-pci")) {
> +if (usesVfio) {
>  if (virPCIDeviceAddressIOMMUGroupIterate(devAddr,
>   
> virHostdevIsPCINodeDeviceUsed,
> - hostdev_mgr) < 0)
> + &data) < 0)
>  goto cleanup;
> -} else if (virHostdevIsPCINodeDeviceUsed(devAddr, hostdev_mgr)) {
> +} else if (virHostdevIsPCINodeDeviceUsed(devAddr, &data)) {
>  goto cleanup;
>  }
>  }
> @@ -1544,6 +1560,8 @@ virHostdevPCINodeDeviceDetach(virHostdevManagerPtr 
> hostdev_mgr,
>virPCIDevicePtr pci)
>  {
>  virPCIDeviceAddressPtr devAddr = NULL;
> +struct virHostdevIsPCINodeDeviceUsedData data = { hostdev_mgr, NULL,
> + false };
>  int ret = -1;
>
>  virObjectLock(hostdev_mgr->activePCIHostdevs);
> @@ -1552,7 +1570,7 @@ virHostdevPCINodeDeviceDetach(virHostdevManagerPtr 
> hostdev_mgr,
>  if (!(devAddr = virPCIDeviceGetAddress(pci)))
>  goto out;
>
> -if (virHostdevIsPCINodeDeviceUsed(devAddr, hostdev_mgr))
> +if (virHostdevIsPCINodeDeviceUsed(devAddr, &data))
>  goto out;
>
>  if (virPCIDeviceDetach(pci, hostdev_mgr->activePCIHostdevs,
> @@ -1573,6 +1591,8 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr 
> hostdev_mgr,
>  virPCIDevicePtr pci)
>  {
>  virPCIDeviceAddressPtr devAddr = NULL;
> +struct virHostdevIsPCINode