Re: [libvirt] [PATCH v5 2/4] qemu: Add PCI-Express root to ARM virt machine
On Mon, Aug 10, 2015 at 07:23:07PM -0400, Cole Robinson wrote:
On 08/10/2015 11:09 AM, Daniel P. Berrange wrote:
On Thu, Aug 06, 2015 at 07:46:58PM +0200, Martin Kletzander wrote:
On Thu, Aug 06, 2015 at 06:37:41PM +0200, Martin Kletzander wrote:
On Fri, Jul 17, 2015 at 02:27:45PM +0300, Pavel Fedin wrote:
Here we assume that if qemu supports generic PCI host controller,
it is a part of virt machine and can be used for adding PCI devices.
In qemu this is actually a PCIe bus, so we also declare multibus
capability so that 0'th bus is specified to qemu correctly as 'pcie.0'
Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
src/qemu/qemu_capabilities.c | 8
src/qemu/qemu_domain.c | 17 +
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d570fdd..f3486c7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2138,6 +2138,14 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps,
return false;
}
+if (ARCH_IS_ARM(def-os.arch)) {
+/* If 'virt' supports PCI, it supports multibus.
+ * No extra conditions here for simplicity.
+ */
So every ARM qemu with the virt machine type supports both PCI and
multiqueue? How about those virt-* for which you check below. That
might not be related, I'm just curious.
+if (STREQ(def-os.machine, virt))
+return true;
+}
+
return false;
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8b050a0..c7d14e4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -981,7 +981,7 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = {
static int
qemuDomainDefPostParse(virDomainDefPtr def,
virCapsPtr caps,
- void *opaque ATTRIBUTE_UNUSED)
+ void *opaque)
{
bool addDefaultUSB = true;
bool addImplicitSATA = false;
@@ -1030,12 +1030,21 @@ qemuDomainDefPostParse(virDomainDefPtr def,
break;
case VIR_ARCH_ARMV7L:
- addDefaultUSB = false;
- addDefaultMemballoon = false;
- break;
case VIR_ARCH_AARCH64:
addDefaultUSB = false;
addDefaultMemballoon = false;
+ if (STREQ(def-os.machine, virt) ||
+ STRPREFIX(def-os.machine, virt-)) {
+ virQEMUDriverPtr driver = opaque;
+
+ /* This condition is actually a (temporary) hack for test suite
which
+* does not create capabilities cache */
Few questions here. a) how temporary is this since you're not
removing it in this series? b) for what tests you need this hack and
what part of the below is the hack?
Moreover, you cannot use capabilities when defining an XML. The
emulator can change between the domain is defined and started, so you
cannot know with what emulator this will be started.
I see Michal (Cc'd) just pushed this, I probably just missed the mail
Of course I forgot, Cc'ing now.
I agree with your core statement that we should not be using the QEMU
capabilities when defining the XML. With all existing scenarios we have
been able to determine whether to add the implicit PCI controller based
on the machine type name only, because with every other QEMU arch when
doing such a major change as adding a PCI bus, they have created a new
machine type. The problem is that arm 'virt' machine type is not stable,
it is being changed arbitrarily in new QEMU releases :-(
So AFAIK, that leaves us with 3 choices
- Never add PCI controller at time the XML is defined, on the basis
that we have to be conservative in what we add to cope with old QEMU
- Always add PCI controller at time the XML is defined, on the basis
that most people will have new enough QEMU because ARM 'virt' machine
type is very much still in development, so no one will serously stick
with the older QEMU versions which lack PCI.
- Use the capabilities in XML post-parse to conditionally add the
PCI controller. This is what was currently merged
I don't think option 1 makes much sense as it'll harm ARM arch forever
more, to cope with QEMU versions that will almost never be used in
practice.
I'd be inclined to go with option 2, and then if any PCI devices are
actually used with the guest, check the capability at start time when
we are doing auto-address assignment.
Another option: add versioned 'virt' machine types to the next qemu release
(like virt-2.5 etc.), and key libvirt enabling pci off of that.
_Eventually_ we are going to need versioned 'virt' machine types for migration
compatibility like we already do with x86 -M pc-*. Might as well make the
change early so libvirt can actually use it.
I was also thinking about a middle ground between choices 1 and 2 from
Dan in case the machine types are not versioned any time soon. We
would, by default add no pci controller when defining unless we think
it's needed. That would be determined by any of the following:
a)
Re: [libvirt] Buffer overflow error when starting libvirtd
On Mon, Aug 10, 2015 at 11:21:47PM -0400, Willard Dennis wrote: Compiled libvirt 1.2.18 on Ubuntu 12.04.5 (installed over 12.04 libvirt-bin 0.9.9 package files); when I try to start libvirtd, I get the following error / backtrace: $ sudo /usr/sbin/libvirtd *** stack smashing detected ***: /usr/sbin/libvirtd terminated === Backtrace: = /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x37)[0x7f02fe6d3e57] /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x0)[0x7f02fe6d3e20] /usr/lib/libvirt.so.0(+0xaa359)[0x7f02fec4f359] /usr/lib/libvirt/connection-driver/libvirt_driver_nodedev.so(+0x99fe)[0x7f02f1fc59fe] /usr/lib/libvirt/connection-driver/libvirt_driver_nodedev.so(+0xa672)[0x7f02f1fc6672] /usr/lib/libvirt.so.0(virStateInitialize+0xaf)[0x7f02fed1cd2f] /usr/sbin/libvirtd(+0x17d80)[0x7f02ff98dd80] /usr/lib/libvirt.so.0(+0xd25d2)[0x7f02fec775d2] /lib/x86_64-linux-gnu/libpthread.so.0(+0x7e9a)[0x7f02fe98fe9a] /lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)[0x7f02fe6bd38d] Not sure if this may be a bug with 1.2.18, or a fault with my configure/compile/install process; please advise... This has been reported last week: https://www.redhat.com/archives/libvir-list/2015-August/msg00163.html And here is the patch proposed to fix it, waiting on review: https://www.redhat.com/archives/libvir-list/2015-August/msg00263.html Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 04/20] cpu: Reorder functions in the ppc64 driver
On Mon, Aug 10, 2015 at 10:55:46 +0200, Andrea Bolognani wrote: Having the functions grouped together this way will avoid further shuffling around down the line. No functional changes. Still ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH v2 05/19] Image: Refactor create function
On Tue, Aug 04, 2015 at 08:11:11PM +, Eren Yagdiran wrote:
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/virt-sandbox-image.py
b/virt-sandbox-image/virt-sandbox-image.py
index 2573c29..6f65445 100755
--- a/virt-sandbox-image/virt-sandbox-image.py
+++ b/virt-sandbox-image/virt-sandbox-image.py
@@ -116,59 +116,6 @@ def delete_template(name, destdir):
parent = None
imagetagid = parent
-
-def get_image_list(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 create_template(name, imagepath, format, destdir):
-if not format in [qcow2]:
-raise ValueError([Unsupported image format %s % format])
-
-imagelist = 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)
-debug(Run %s\n % .join(cmd))
-subprocess.call(cmd)
-parentImage = templateImage
-
def download(args):
try:
dynamic_source_loader(args.source).download_template(name=args.name,
@@ -186,8 +133,13 @@ def delete(args):
delete_template(args.name, default_template_dir)
def create(args):
-info(Creating %s from %s in format %s\n % (args.imagepath, args.name,
args.format))
-create_template(args.name, args.imagepath, args.format,
default_template_dir)
+try:
+dynamic_source_loader(args.source).create_template(name=args.name,
+
connect=args.connect,
+
imagepath=args.imagepath,
+
format=args.format)
+except Exception,e:
+print Create Error %s % str(e)
def requires_name(parser):
parser.add_argument(name,
@@ -198,6 +150,10 @@ def requires_source(parser):
default=docker,
help=_(name of the template))
+def requires_connect(parser):
+parser.add_argument(-c,--connect,
+help=_(Connect string for libvirt))
+
def requires_auth_conn(parser):
parser.add_argument(-r,--registry,
help=_(Url of the custom registry))
@@ -226,10 +182,12 @@ def gen_create_args(subparser):
parser = subparser.add_parser(create,
help=_(Create image from template data))
requires_name(parser)
+requires_source(parser)
+requires_connect(parser)
parser.add_argument(imagepath,
help=_(path for image))
parser.add_argument(format,
-help=_(format))
+help=_(format format for image))
For the previous 'download' command we called the directory 'templatedir'
but for 'create' we are calling it 'imagepath'. We should be consistent
about what we do and so call it 'templatedir' for all commands, and
also make sure it uses '--templatedir' and not a positional argument.
The same applies to the 'run' command.
Also we should really use '--format=qcow2' too, so people don't have
to pass it by default
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|:
Re: [libvirt] [sandbox PATCH v2 04/19] Image: Add download function
On Mon, Aug 10, 2015 at 05:30:52PM +0100, Daniel P. Berrange wrote: On Tue, Aug 04, 2015 at 08:11:10PM +, Eren Yagdiran wrote: 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 erenyagdi...@gmail.com +* +''' +#!/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 These are class level variables +def __init__(self,server=index.docker.io,destdir=/var/lib/libvirt/templates): +self.default_index_server = server +self.default_template_dir = destdir And these are object level variables with the same name. Generally object level variables should have a leading _ to indicate that they are private. I'd suggest we remove the default directories from this bit of code though. def download(args): -info(Downloading %s from %s to %s\n % (args.name, default_index_server, default_template_dir)) -download_template(args.name, default_index_server, default_template_dir) +try: + dynamic_source_loader(args.source).download_template(name=args.name, + registry=args.registry, + username=args.username, + password=args.password, + templatedir=args.template_dir) +except IOError,e: +print Source %s cannot be found in given path %args.source +except Exception,e: +print Download Error %s % str(e) def delete(args): info(Deleting %s from %s\n % (args.name, default_template_dir)) @@ -355,10 +193,27 @@ def requires_name(parser): parser.add_argument(name, help=_(name of the template)) +def requires_source(parser): +parser.add_argument(-s,--source, +default=docker, +help=_(name of the template)) + +def requires_auth_conn(parser): +parser.add_argument(-r,--registry, +help=_(Url of the custom registry)) +parser.add_argument(-u,--username, +help=_(Username for the custom registry)) +
Re: [libvirt] [sandbox PATCH v2 13/19] Image: Add Volume Support
On Tue, Aug 04, 2015 at 08:11:19PM +, Eren Yagdiran wrote: Volumes let user to map host-paths into guest. Docker containers need volumes because its filesystem read-only by default. Hmm, it seems our filesystem is not read-only. Normally we would have / as rread-only, but because we remappe it using -m host-image:/=/path/to/template we end up with / being read-write. We should add option to --mount to request a read-only mount, so we can force / to be read-only againt. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH v2 11/19] Image: Add run function
On Mon, Aug 10, 2015 at 05:16:19PM +0100, Daniel P. Berrange wrote: On Tue, Aug 04, 2015 at 08:11:17PM +, Eren Yagdiran wrote: 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) This code actually ends up launching a sandbox using the template file as the root disk image. This is not good, because we need to be able to run multiple instances of the sandbox, all using the same template file. As such we need to be able to create a new temporary disk image for each sandbox instance, that is an overlay on the main template, an then delete this temporary disk at shutdown. Or if we make sure / is read-only that would avoid the problem I mention here. With normal docker toolchain is / read-only by default, or do they make it writable with a throw-away snapshot ? Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 3/4] qemu: Build correct command line for PCI NICs on ARM
On 07/17/2015 07:27 AM, Pavel Fedin wrote: Legacy -net option works correctly only with embedded device models, which do not require any bus specification. Therefore, we should use -device for PCI hardware This function is starting to look a bit like a hack on top of a hack on top of a hack. Are the original reasons for using -net instead of -device on certain arches still applicable? Signed-off-by: Pavel Fedin p.fe...@samsung.com --- src/qemu/qemu_command.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 42906a8..5569be6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -457,7 +457,8 @@ qemuDomainSupportsNicdev(virDomainDefPtr def, /* non-virtio ARM nics require legacy -net nic */ if (((def-os.arch == VIR_ARCH_ARMV7L) || (def-os.arch == VIR_ARCH_AARCH64)) -net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) +net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO +net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) return false; return true; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/2] Add support for virtio-scsi controllers with ioeventfd
On Mon, Aug 10, 2015 at 10:23:47AM +0200, Martin Kletzander wrote: Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1150484 v2: - Changed one occurence virTristateBool to virTristateSwitch as it should be - wrapped one line to be = 80 characters long - Rebased on top of current master (with laine's patches pushed) Martin Kletzander (2): conf: Add ioeventfd option for controllers qemu: Enable ioeventfd usage for virtio-scsi controllers docs/formatdomain.html.in | 8 ++ docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 20 +-- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c| 7 ++ .../qemuxml2argv-disk-virtio-scsi-ioeventfd.args | 9 +++ .../qemuxml2argv-disk-virtio-scsi-ioeventfd.xml| 29 ++ tests/qemuxml2argvtest.c | 3 +++ tests/qemuxml2xmltest.c| 1 + 9 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-ioeventfd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-ioeventfd.xml ACK 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] Drive hot-unplug: reliable parsing of HMP results
Hot-unplugging a disk from a guest that supports hot-unplugging generates an
error
in the libvirt log when running QEMU with the -msg timestamp=on flag.
2015-08-06 10:48:59.945+: 11662: error : qemuMonitorTextDriveDel:2594 :
operation failed: deleting drive-virtio-disk4 drive failed:
2015-08-06T10:48:59.945058Z Device 'drive-virtio-disk4' not found
This error is caused because the HMP results are getting prefixed with a
timestamp.
Parsing the output is not reliable with STRPREFIX as the results can be
prefixed with a timestamp.
Using strstr ensures that parsing the output works whether the results are
prefixed or not.
Cc: Stefan Hajnoczi stefa...@redhat.com
Cc: Daniel P. Berrange berra...@redhat.com
Signed-off-by: Frank Schreuder fschreu...@transip.nl
---
src/qemu/qemu_monitor_text.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 2aa0460..d5ef089 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -2586,7 +2586,7 @@ int qemuMonitorTextDriveDel(qemuMonitorPtr mon,
/* (qemu) drive_del wark
* Device 'wark' not found */
-} else if (STRPREFIX(reply, Device ') (strstr(reply, not found))) {
+} else if (strstr(reply, Device ') strstr(reply, not found)) {
/* NB: device not found errors mean the drive was auto-deleted and we
* ignore the error */
} else if (STRNEQ(reply, )) {
--
2.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 2/4] qemu: Add PCI-Express root to ARM virt machine
On Mon, Aug 10, 2015 at 05:12:26PM +0300, Pavel Fedin wrote: Hello! And that's the exact problem. I'm not saying capabilities are not made available when defining, but rather there are no capabilities when defining. Stop stop stop... If we don't mess up with terminology... OK, sorry for that, I'll be clear from now on. The test case contains some hardcoded capabilities, needed by the test, right? Ok, this means that Let's say we're speaking about qemuxml2xmltest, just to make it as easy as possible. That test ultimately checks parsing and formatting and parsing is what we're talking about here. That test has *no* hardcoded capabilities. And the reason why it has no hardcoded capabilities is what I'm trying to express here. actually we have this information, and we need to make up capabilities cache from these caps, and there will be no problem. With this change the test will exactly mimic real life behavior, where we have the cache, and this cache was obtained from the qemu binary. I don't know whether to explain what's wrong here just so we don't get confusing again. But I must try. In libvirt, we are not using capabilities when parsing/formatting XML because it [parsing/formatting] must a) work without any qemu binary and b) produce exactly same results no matter what is installed in the system. Is that any clearer now by any chance? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt master] interface type: add udp socket support
On Fri, Aug 07, 2015 at 03:14:36PM -0700, Jonathan Toppins wrote: Adds a new interface type using UDP sockets, this seems only applicable to QEMU but have edited tree-wide to support the new interface type. The interface type required the addition of a destaddr (destination address), this then maps into the following xml and qemu call. interface type='udp' mac address='52:54:00:5c:67:56'/ source address='127.0.0.1' port='2'/ model type='virtio'/ dest address=127.0.0.1' port='2'/ address type='pci' domain='0x' bus='0x00' slot='0x07' function='0x0'/ ... QEMU call: -netdev socket,udp=127.0.0.1:2,localaddr=127.0.0.1:2 Notice the xml source entry becomes the localaddr for the qemu call. reference: http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg00629.html Signed-off-by: Jonathan Toppins jtopp...@cumulusnetworks.com --- docs/formatdomain.html.in| 17 src/conf/domain_conf.c | 56 +--- src/conf/domain_conf.h | 3 +++ src/conf/netdev_bandwidth_conf.h | 1 + src/libxl/libxl_conf.c | 1 + src/lxc/lxc_controller.c | 1 + src/lxc/lxc_process.c| 1 + src/qemu/qemu_command.c | 12 + src/qemu/qemu_hotplug.c | 1 + src/qemu/qemu_interface.c| 2 ++ src/uml/uml_conf.c | 5 src/xenconfig/xen_sxpr.c | 1 + tools/virsh-domain.c | 1 + 13 files changed, 99 insertions(+), 3 deletions(-) Missing test cases for tests/qemuxml2argvtest.c and tests/qemuxml2xmltest.c. diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c0a265a..95f7f5d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4165,6 +4165,23 @@ lt;/devicesgt; .../pre +h5a name=elementsNICSUDPUDP unicast tunnel/a/h5 + +p +A UDP unicast architecture provides a virtual network which enables +connections between Qemu instances using Qemu's UDP infrastructure./p + It would be nice to document what the addresses mean, and mention that this is supported span class=sinceSince 1.2.19/span. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 2/4] qemu: Add PCI-Express root to ARM virt machine
On Thu, Aug 06, 2015 at 07:46:58PM +0200, Martin Kletzander wrote:
On Thu, Aug 06, 2015 at 06:37:41PM +0200, Martin Kletzander wrote:
On Fri, Jul 17, 2015 at 02:27:45PM +0300, Pavel Fedin wrote:
Here we assume that if qemu supports generic PCI host controller,
it is a part of virt machine and can be used for adding PCI devices.
In qemu this is actually a PCIe bus, so we also declare multibus
capability so that 0'th bus is specified to qemu correctly as 'pcie.0'
Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
src/qemu/qemu_capabilities.c | 8
src/qemu/qemu_domain.c | 17 +
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d570fdd..f3486c7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2138,6 +2138,14 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr
qemuCaps,
return false;
}
+if (ARCH_IS_ARM(def-os.arch)) {
+/* If 'virt' supports PCI, it supports multibus.
+ * No extra conditions here for simplicity.
+ */
So every ARM qemu with the virt machine type supports both PCI and
multiqueue? How about those virt-* for which you check below. That
might not be related, I'm just curious.
+if (STREQ(def-os.machine, virt))
+return true;
+}
+
return false;
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8b050a0..c7d14e4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -981,7 +981,7 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace =
{
static int
qemuDomainDefPostParse(virDomainDefPtr def,
virCapsPtr caps,
- void *opaque ATTRIBUTE_UNUSED)
+ void *opaque)
{
bool addDefaultUSB = true;
bool addImplicitSATA = false;
@@ -1030,12 +1030,21 @@ qemuDomainDefPostParse(virDomainDefPtr def,
break;
case VIR_ARCH_ARMV7L:
- addDefaultUSB = false;
- addDefaultMemballoon = false;
- break;
case VIR_ARCH_AARCH64:
addDefaultUSB = false;
addDefaultMemballoon = false;
+ if (STREQ(def-os.machine, virt) ||
+ STRPREFIX(def-os.machine, virt-)) {
+ virQEMUDriverPtr driver = opaque;
+
+ /* This condition is actually a (temporary) hack for test suite
which
+* does not create capabilities cache */
Few questions here. a) how temporary is this since you're not
removing it in this series? b) for what tests you need this hack and
what part of the below is the hack?
Moreover, you cannot use capabilities when defining an XML. The
emulator can change between the domain is defined and started, so you
cannot know with what emulator this will be started.
I see Michal (Cc'd) just pushed this, I probably just missed the mail
Of course I forgot, Cc'ing now.
I agree with your core statement that we should not be using the QEMU
capabilities when defining the XML. With all existing scenarios we have
been able to determine whether to add the implicit PCI controller based
on the machine type name only, because with every other QEMU arch when
doing such a major change as adding a PCI bus, they have created a new
machine type. The problem is that arm 'virt' machine type is not stable,
it is being changed arbitrarily in new QEMU releases :-(
So AFAIK, that leaves us with 3 choices
- Never add PCI controller at time the XML is defined, on the basis
that we have to be conservative in what we add to cope with old QEMU
- Always add PCI controller at time the XML is defined, on the basis
that most people will have new enough QEMU because ARM 'virt' machine
type is very much still in development, so no one will serously stick
with the older QEMU versions which lack PCI.
- Use the capabilities in XML post-parse to conditionally add the
PCI controller. This is what was currently merged
I don't think option 1 makes much sense as it'll harm ARM arch forever
more, to cope with QEMU versions that will almost never be used in
practice.
I'd be inclined to go with option 2, and then if any PCI devices are
actually used with the guest, check the capability at start time when
we are doing auto-address assignment.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Drive hot-unplug: reliable parsing of HMP results
On Mon, Aug 10, 2015 at 4:37 PM, Frank Schreuder fschreu...@transip.nl wrote:
Hot-unplugging a disk from a guest that supports hot-unplugging generates an
error
in the libvirt log when running QEMU with the -msg timestamp=on flag.
2015-08-06 10:48:59.945+: 11662: error : qemuMonitorTextDriveDel:2594 :
operation failed: deleting drive-virtio-disk4 drive failed:
2015-08-06T10:48:59.945058Z Device 'drive-virtio-disk4' not found
This error is caused because the HMP results are getting prefixed with a
timestamp.
Parsing the output is not reliable with STRPREFIX as the results can be
prefixed with a timestamp.
Using strstr ensures that parsing the output works whether the results are
prefixed or not.
Cc: Stefan Hajnoczi stefa...@redhat.com
Cc: Daniel P. Berrange berra...@redhat.com
Signed-off-by: Frank Schreuder fschreu...@transip.nl
---
src/qemu/qemu_monitor_text.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 2aa0460..d5ef089 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -2586,7 +2586,7 @@ int qemuMonitorTextDriveDel(qemuMonitorPtr mon,
/* (qemu) drive_del wark
* Device 'wark' not found */
-} else if (STRPREFIX(reply, Device ') (strstr(reply, not found)))
{
+} else if (strstr(reply, Device ') strstr(reply, not found)) {
/* NB: device not found errors mean the drive was auto-deleted and we
* ignore the error */
} else if (STRNEQ(reply, )) {
I'm not very familiar with the libvirt codebase, but perhaps the
timestamps on error messages should be stripped in the QEMU monitor
processIO handler functions?
That way any remaining bugs like this will be resolved too.
Stefan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Drive hot-unplug: reliable parsing of HMP results
On Mon, Aug 10, 2015 at 04:49:03PM +0100, Stefan Hajnoczi wrote:
On Mon, Aug 10, 2015 at 4:37 PM, Frank Schreuder fschreu...@transip.nl
wrote:
Hot-unplugging a disk from a guest that supports hot-unplugging generates
an error
in the libvirt log when running QEMU with the -msg timestamp=on flag.
2015-08-06 10:48:59.945+: 11662: error : qemuMonitorTextDriveDel:2594 :
operation failed: deleting drive-virtio-disk4 drive failed:
2015-08-06T10:48:59.945058Z Device 'drive-virtio-disk4' not found
This error is caused because the HMP results are getting prefixed with a
timestamp.
Parsing the output is not reliable with STRPREFIX as the results can be
prefixed with a timestamp.
Using strstr ensures that parsing the output works whether the results are
prefixed or not.
Cc: Stefan Hajnoczi stefa...@redhat.com
Cc: Daniel P. Berrange berra...@redhat.com
Signed-off-by: Frank Schreuder fschreu...@transip.nl
---
src/qemu/qemu_monitor_text.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 2aa0460..d5ef089 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -2586,7 +2586,7 @@ int qemuMonitorTextDriveDel(qemuMonitorPtr mon,
/* (qemu) drive_del wark
* Device 'wark' not found */
-} else if (STRPREFIX(reply, Device ') (strstr(reply, not
found))) {
+} else if (strstr(reply, Device ') strstr(reply, not found)) {
/* NB: device not found errors mean the drive was auto-deleted and
we
* ignore the error */
} else if (STRNEQ(reply, )) {
I'm not very familiar with the libvirt codebase, but perhaps the
timestamps on error messages should be stripped in the QEMU monitor
processIO handler functions?
That way any remaining bugs like this will be resolved too.
That would be right froma strict correctness POV, but looking at the
existing monitor handling code it all uses strstr too, so this is
matching existing practice. The text monitor code is scary enough
that it is probably wise not to try changing it at this stage in
its life.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/5] Introduce virDomainRename API
On 06.08.2015 12:21, Tomas Meszaros wrote:
Also, among with this new API new ACL that restricts rename capability
is invented too.
Signed-off-by: Tomas Meszaros e...@tty.sk
---
include/libvirt/libvirt-domain.h | 2 ++
src/access/viraccessperm.c | 3 ++-
src/access/viraccessperm.h | 6 ++
src/driver-hypervisor.h | 5 +
src/libvirt-domain.c | 31 +++
src/libvirt_public.syms | 5 +
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 17 -
src/remote_protocol-structs | 8
9 files changed, 76 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h
b/include/libvirt/libvirt-domain.h
index e8202cf..2ddc47d 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3837,4 +3837,6 @@ int virDomainSetUserPassword(virDomainPtr dom,
const char *password,
unsigned int flags);
+int virDomainRename(virDomainPtr dom, const char *new_name);
+
#endif /* __VIR_LIBVIRT_DOMAIN_H__ */
diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c
index 0f58290..bdc7f60 100644
--- a/src/access/viraccessperm.c
+++ b/src/access/viraccessperm.c
@@ -43,7 +43,8 @@ VIR_ENUM_IMPL(virAccessPermDomain,
fs_trim, fs_freeze,
block_read, block_write, mem_read,
open_graphics, open_device, screenshot,
- open_namespace, set_time, set_password);
+ open_namespace, set_time, set_password,
+ rename);
This ^^ ..
VIR_ENUM_IMPL(virAccessPermInterface,
VIR_ACCESS_PERM_INTERFACE_LAST,
diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h
index 1817da7..6ae4ee7 100644
--- a/src/access/viraccessperm.h
+++ b/src/access/viraccessperm.h
@@ -306,6 +306,12 @@ typedef enum {
*/
VIR_ACCESS_PERM_DOMAIN_SET_PASSWORD,
+/**
+ * @desc: Rename domain
+ * @message: Renaming the domain requires authorization
+ */
+VIR_ACCESS_PERM_DOMAIN_RENAME,
+
.. and this change ^^ are not required since the new perm is not used
anywhere.
VIR_ACCESS_PERM_DOMAIN_LAST,
} virAccessPermDomain;
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 3275343..e8c8c2a 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -650,6 +650,10 @@ typedef int
(*virDrvDomainIsActive)(virDomainPtr dom);
typedef int
+(*virDrvDomainRename)(virDomainPtr dom,
+ const char *new_name);
+
+typedef int
(*virDrvDomainIsPersistent)(virDomainPtr dom);
typedef int
@@ -1347,6 +1351,7 @@ struct _virHypervisorDriver {
virDrvConnectIsEncrypted connectIsEncrypted;
virDrvConnectIsSecure connectIsSecure;
virDrvDomainIsActive domainIsActive;
+virDrvDomainRename domainRename;
virDrvDomainIsPersistent domainIsPersistent;
virDrvDomainIsUpdated domainIsUpdated;
virDrvConnectCompareCPU connectCompareCPU;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 837933f..c200965 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -8774,6 +8774,37 @@ virDomainIsPersistent(virDomainPtr dom)
return -1;
}
+/**
+ * virDomainRename:
+ * @dom: pointer to the domain object
+ * @new_name: new domain name
+ *
+ * Rename an inactive domain. New domain name is specified in the second
+ * argument.
I wouldn't state here that only an inactive domain can be renamed. It's
just a limitation of current implementation which may change in the
future. If you want to be more verbose (and I guess it's desirable here)
you may say that depending on each driver implementation it may be
required that domain is in a specific state.
+ *
+ * Returns 0 if renamed, -1 on error
+ */
+int
+virDomainRename(virDomainPtr dom, const char *new_name)
We really, really need to have 'unsigned int flags' here. Even though it
is not currently needed, it may be in the future. Every new API should
have it.
+{
+VIR_DEBUG(dom=%p, new_name=%s, dom, NULLSTR(new_name));
+
+virResetLastError();
+virCheckDomainReturn(dom, -1);
+virCheckNonNullArgGoto(new_name, error);
+
+if (dom-conn-driver-domainRename) {
+int ret = dom-conn-driver-domainRename(dom, new_name);
+if (ret 0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+ error:
+virDispatchError(dom-conn);
+return -1;
+}
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/5] domainRename API implementation
On 06.08.2015 12:21, Tomas Meszaros wrote: s is an effort to implement domain rename API. Presented patch series consists of the following: virDomainRename API implementation for qemu, implementation of the virsh command domrename and the additional support code. The idea behind this endeavor is to provide convenient and safe way to rename a domain. Instead of the: virsh dumpxml domain domain.xml (change domain name in domain.xml) virsh undefine domain virsh define domain.xml user can simply type: virsh domrename foo bar or call virDomainRename() API and domain foo will be renamed to bar. We currently support only renaming inactive domains without snapshots. Renaming procedure takes care of domain log, config, guest agent path and should be able to recover in case of failure. I've been working on this functionality in collaboration with Michal Privoznik who is my mentor during the GSoC 2015. If you have any questions, ideas or criticism feel free to join the discussion. v2: - removed guest agent path rename code - removed rename permission - added code for emitting undefined+renamed event for the old domain Tomas Meszaros (5): Introduce virDomainRename API virsh: Implement domrename command domain_conf: Introducde virDomainObjListRenameAddNew() virDomainObjListRenameRemove() Introduce new VIR_DOMAIN_EVENT_DEFINED_RENAMED event qemu: Implement virDomainRename examples/object-events/event-test.c | 4 + include/libvirt/libvirt-domain.h| 4 + src/access/viraccessperm.c | 3 +- src/access/viraccessperm.h | 6 ++ src/conf/domain_conf.c | 35 + src/conf/domain_conf.h | 5 ++ src/driver-hypervisor.h | 5 ++ src/libvirt-domain.c| 31 src/libvirt_private.syms| 2 + src/libvirt_public.syms | 5 ++ src/qemu/qemu_driver.c | 144 src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x| 17 - src/remote_protocol-structs | 8 ++ tools/virsh-domain.c| 63 +++- tools/virsh.pod | 7 ++ 16 files changed, 336 insertions(+), 4 deletions(-) I think this looks okay. Well, I have some findings in 1/5 but the rest looks okay to me. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH v2 19/19] Image: Add custom environment support
On Tue, Aug 04, 2015 at 08:11:25PM +, Eren Yagdiran wrote: 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): Here you call it 'get_env' but in the DockerSource you call it 'get_environment'. This causes python to immediately exit saying that DockerSource doens't implement the API defined by Source. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH v2 04/19] Image: Add download function
On Tue, Aug 04, 2015 at 08:11:10PM +, Eren Yagdiran wrote: 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 erenyagdi...@gmail.com +* +''' +#!/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 These are class level variables +def __init__(self,server=index.docker.io,destdir=/var/lib/libvirt/templates): +self.default_index_server = server +self.default_template_dir = destdir And these are object level variables with the same name. Generally object level variables should have a leading _ to indicate that they are private. I'd suggest we remove the default directories from this bit of code though. def download(args): -info(Downloading %s from %s to %s\n % (args.name, default_index_server, default_template_dir)) -download_template(args.name, default_index_server, default_template_dir) +try: +dynamic_source_loader(args.source).download_template(name=args.name, + registry=args.registry, + username=args.username, + password=args.password, + templatedir=args.template_dir) +except IOError,e: +print Source %s cannot be found in given path %args.source +except Exception,e: +print Download Error %s % str(e) def delete(args): info(Deleting %s from %s\n % (args.name, default_template_dir)) @@ -355,10 +193,27 @@ def requires_name(parser): parser.add_argument(name, help=_(name of the template)) +def requires_source(parser): +parser.add_argument(-s,--source, +default=docker, +help=_(name of the template)) + +def requires_auth_conn(parser): +parser.add_argument(-r,--registry, +help=_(Url of the custom registry)) +parser.add_argument(-u,--username, +help=_(Username for the custom registry)) +parser.add_argument(-p,--password, +help=_(Password for the custom registry)) +parser.add_argument(-t,--template-dir, +help=_(Template directory for saving
Re: [libvirt] [sandbox PATCH v2 04/19] Image: Add download function
On Tue, Aug 04, 2015 at 08:11:10PM +, Eren Yagdiran wrote: 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/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 erenyagdi...@gmail.com +* +''' +#!/usr/bin/python Same points here about comment syntax, etc + +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 Should print this to stderr, rather than stdout. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 06/20] tests: Remove unused file
On Mon, Aug 10, 2015 at 10:55:48 +0200, Andrea Bolognani wrote: No functional changes. --- 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 @@ -cpu mode='custom' match='exact' - model fallback='allow'POWER7+_v2.1/model -/cpu ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 05/20] cpu: Remove ISA information from CPU map XML
On Mon, Aug 10, 2015 at 10:55:47 +0200, Andrea Bolognani wrote: The information is not used anywhere in libvirt. No functional changes. ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/9] Add missing QoS implementation
On 03.08.2015 08:39, Michal Privoznik wrote: The first patch is unrelated to the rest, but I have it on the same branch. The patches 2..5 fix a daemon crasher (since domain_write ACL is required, I'm not going through libvirt-security). Then, since after patch 5 we have nearly everything prepared, patches 6..9 implement new feature: setting @floor live on domain interfaces. Michal Privoznik (9): virNetDevParseMcast: Avoid magic constant virNetDevBandwidthUpdateRate: turn class_id into integer bridge_driver: Introduce networkBandwidthChangeAllowed bridge_driver: Introduce networkBandwidthUpdate qemuDomainSetInterfaceParameters: Use new functions to update bandwidth virsh: Rework parseRateStr Introduce VIR_DOMAIN_BANDWIDTH_IN_FLOOR virsh: Implement VIR_DOMAIN_BANDWIDTH_IN_FLOOR qemu: Implement VIR_DOMAIN_BANDWIDTH_IN_FLOOR include/libvirt/libvirt-domain.h | 7 ++ src/network/bridge_driver.c | 234 +-- src/network/bridge_driver.h | 22 src/qemu/qemu_driver.c | 30 - src/util/virnetdev.c | 2 +- src/util/virnetdevbandwidth.c| 10 +- src/util/virnetdevbandwidth.h| 2 +- tools/virsh-domain.c | 99 +++-- tools/virsh.pod | 12 +- 9 files changed, 334 insertions(+), 84 deletions(-) Ping? This fixes a really bad crasher. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Revert LXC: show used memory as 0 when domain is not active
This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e,
which introduced a significant semantic change to the
virDomainGetInfo() API. Additionally, the change was only
made to 2 of the 15 virt drivers.
Conflicts:
src/qemu/qemu_driver.c
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/lxc/lxc_driver.c | 2 +-
src/qemu/qemu_driver.c | 12 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index d8d5119..5b0a757 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -597,7 +597,7 @@ static int lxcDomainGetInfo(virDomainPtr dom,
if (!virDomainObjIsActive(vm)) {
info-cpuTime = 0;
-info-memory = 0;
+info-memory = vm-def-mem.cur_balloon;
} else {
if (virCgroupGetCpuacctUsage(priv-cgroup, (info-cpuTime)) 0) {
virReportError(VIR_ERR_OPERATION_FAILED,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6998e12..48cc534 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2641,13 +2641,13 @@ qemuDomainGetInfo(virDomainPtr dom,
goto cleanup;
}
-if (virDomainObjIsActive(vm)) {
-if (VIR_ASSIGN_IS_OVERFLOW(info-memory, vm-def-mem.cur_balloon)) {
-virReportError(VIR_ERR_OVERFLOW, %s,
- _(Current memory size too large));
-goto cleanup;
-}
+if (VIR_ASSIGN_IS_OVERFLOW(info-memory, vm-def-mem.cur_balloon)) {
+virReportError(VIR_ERR_OVERFLOW, %s,
+ _(Current memory size too large));
+goto cleanup;
+}
+if (virDomainObjIsActive(vm)) {
if (qemuGetProcessInfo((info-cpuTime), NULL, NULL, vm-pid, 0) 0) {
virReportError(VIR_ERR_OPERATION_FAILED, %s,
_(cannot read cputime for domain));
--
2.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 1/2] network: added waiting for DAD to finish for bridge address.
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.
---
Difference to v2: Moved to virnetdev.
src/libvirt_private.syms| 1 +
src/network/bridge_driver.c | 35 +-
src/util/virnetdev.c| 160
src/util/virnetdev.h| 2 +
4 files changed, 197 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0517c24..fa9e1c1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1776,6 +1776,7 @@ virNetDevSetRcvMulti;
virNetDevSetupControl;
virNetDevSysfsFile;
virNetDevValidateConfig;
+virNetDevWaitDadFinish;
# util/virnetdevbandwidth.h
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 3d6721b..2172a3d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2026,6 +2026,33 @@ networkAddRouteToBridge(virNetworkObjPtr network,
}
static int
+networkWaitDadFinish(virNetworkObjPtr network)
+{
+virNetworkIpDefPtr ipdef;
+virSocketAddrPtr *addrs = NULL;
+size_t i;
+int ret;
+for (i = 0;
+ (ipdef = virNetworkDefGetIpByIndex(network-def, AF_INET6, i));
+ i++) {}
+
+if (i == 0)
+return 0;
+if (VIR_ALLOC_N(addrs, i))
+return -1;
+
+for (i = 0;
+ (ipdef = virNetworkDefGetIpByIndex(network-def, AF_INET6, i));
+ i++) {
+addrs[i] = ipdef-address;
+}
+
+ret = virNetDevWaitDadFinish(addrs, i);
+VIR_FREE(addrs);
+return ret;
+}
+
+static int
networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
virNetworkObjPtr network)
{
@@ -2159,7 +2186,13 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr
driver,
if (v6present networkStartRadvd(driver, network) 0)
goto err4;
-/* DAD has happened (dnsmasq waits for it), dnsmasq is now bound to the
+/* dnsmasq main process does not wait for DAD to complete,
+ * so we need to wait for it ourselves.
+ */
+if (v6present networkWaitDadFinish(network) 0)
+goto err4;
+
+/* DAD has happened, dnsmasq is now bound to the
* bridge's IPv6 address, so we can now set the dummy tun down.
*/
if (tapfd = 0) {
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 1e20789..c81342a 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -96,6 +96,7 @@ VIR_LOG_INIT(util.netdev);
# define FEATURE_BIT_IS_SET(blocks, index, field)\
(FEATURE_WORD(blocks, index, field) FEATURE_FIELD_FLAG(index))
#endif
+# define IP_BUF_SIZE 4096
typedef enum {
VIR_MCAST_TYPE_INDEX_TOKEN,
@@ -1219,6 +1220,103 @@ int virNetDevClearIPAddress(const char *ifname,
return ret;
}
+/* return whether there is a known address with 'tentative' flag set */
+static int
+virNetDevParseDadStatus(struct nlmsghdr *nlh, int len,
+virSocketAddrPtr *addrs, size_t count)
+{
+struct ifaddrmsg *ifaddrmsg_ptr;
+unsigned int ifaddrmsg_len;
+struct rtattr *rtattr_ptr;
+size_t i;
+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. */
+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;
+}
+
+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;
+}
+
+/* We check only known addresses. */
+for (i = 0; i count; i++) {
+addr = addrs[i]-data.inet6.sin6_addr;
+if (!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 */
+int
+virNetDevWaitDadFinish(virSocketAddrPtr *addrs, size_t count)
+{
+struct nl_msg *nlmsg = NULL;
+struct ifaddrmsg ifa;
+struct nlmsghdr *resp = NULL;
+unsigned int recvbuflen;
+int ret = -1, dad = 1;
+
+if (!(nlmsg =
[libvirt] [PATCHv3 0/2] Added waiting for DAD to finish for bridge address.
This is a fix for commit db488c79173b240459c7754f38c3c6af9b432970 dnsmasq main process which is relied on when waiting for DAD to complete exits without actually waiting for DAD. This is dnsmasq daemon's task. It seems to be a race that DAD finished before dnsmasq main process exited. The above commit needs the execution to block until DAD finishes for bridge IPv6 address because then it closes dummy tap device. Thus we need to ensure this ourselves. 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. We use netlink to dump information about existing IPv6 addresses. Netlink's response is a multi-part message. Unfortunately, the current implementation of virNetlink treats such messages as faulty and throws an error. So the patch 2/2 adds multi-part nelink response support. Update v2: fixed syntax. Update v3: moved to virnetdev. Maxim Perevedentsev (2): network: added waiting for DAD to finish for bridge address. Add support for multi-part netlink messages. src/libvirt_private.syms| 1 + src/network/bridge_driver.c | 35 +- src/util/virnetdev.c| 160 src/util/virnetdev.h| 2 + src/util/virnetlink.c | 4 +- 5 files changed, 200 insertions(+), 2 deletions(-) -- Sincerely, Maxim Perevedentsev -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 2/2] Add support for multi-part netlink messages.
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. --- 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..f02bb59 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
Re: [libvirt] [PATCH v3 02/20] cpu: Simplify NULL handling in ppc64 driver
On Mon, Aug 10, 2015 at 10:55:44 +0200, Andrea Bolognani wrote: 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. ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 01/20] cpu: Mark driver functions in ppc64 driver
On Mon, Aug 10, 2015 at 10:55:43 +0200, Andrea Bolognani wrote: 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. ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH v2 05/19] Image: Refactor create function
On Tue, Aug 04, 2015 at 08:11:11PM +, Eren Yagdiran wrote: 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): +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) We need to include the '-p' argument, because if running with qemu:///session you'll be unprivileged by default, so need -p to become root in the sandbox. +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', Could probably leave out the 'v' flag, since I don't think we need to display a list of every file. Or make it conditional on some --debug flag to virt-sandbox-image. + '%s' %tarfile, + '-C', + '/mnt'] +cmd = cmd + params +subprocess.call(cmd) Same here about -p flag needed Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 2/4] qemu: Add PCI-Express root to ARM virt machine
On 08/10/2015 11:09 AM, Daniel P. Berrange wrote:
On Thu, Aug 06, 2015 at 07:46:58PM +0200, Martin Kletzander wrote:
On Thu, Aug 06, 2015 at 06:37:41PM +0200, Martin Kletzander wrote:
On Fri, Jul 17, 2015 at 02:27:45PM +0300, Pavel Fedin wrote:
Here we assume that if qemu supports generic PCI host controller,
it is a part of virt machine and can be used for adding PCI devices.
In qemu this is actually a PCIe bus, so we also declare multibus
capability so that 0'th bus is specified to qemu correctly as 'pcie.0'
Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
src/qemu/qemu_capabilities.c | 8
src/qemu/qemu_domain.c | 17 +
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d570fdd..f3486c7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2138,6 +2138,14 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr
qemuCaps,
return false;
}
+if (ARCH_IS_ARM(def-os.arch)) {
+/* If 'virt' supports PCI, it supports multibus.
+ * No extra conditions here for simplicity.
+ */
So every ARM qemu with the virt machine type supports both PCI and
multiqueue? How about those virt-* for which you check below. That
might not be related, I'm just curious.
+if (STREQ(def-os.machine, virt))
+return true;
+}
+
return false;
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8b050a0..c7d14e4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -981,7 +981,7 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace
= {
static int
qemuDomainDefPostParse(virDomainDefPtr def,
virCapsPtr caps,
- void *opaque ATTRIBUTE_UNUSED)
+ void *opaque)
{
bool addDefaultUSB = true;
bool addImplicitSATA = false;
@@ -1030,12 +1030,21 @@ qemuDomainDefPostParse(virDomainDefPtr def,
break;
case VIR_ARCH_ARMV7L:
- addDefaultUSB = false;
- addDefaultMemballoon = false;
- break;
case VIR_ARCH_AARCH64:
addDefaultUSB = false;
addDefaultMemballoon = false;
+ if (STREQ(def-os.machine, virt) ||
+ STRPREFIX(def-os.machine, virt-)) {
+ virQEMUDriverPtr driver = opaque;
+
+ /* This condition is actually a (temporary) hack for test
suite which
+* does not create capabilities cache */
Few questions here. a) how temporary is this since you're not
removing it in this series? b) for what tests you need this hack and
what part of the below is the hack?
Moreover, you cannot use capabilities when defining an XML. The
emulator can change between the domain is defined and started, so you
cannot know with what emulator this will be started.
I see Michal (Cc'd) just pushed this, I probably just missed the mail
Of course I forgot, Cc'ing now.
I agree with your core statement that we should not be using the QEMU
capabilities when defining the XML. With all existing scenarios we have
been able to determine whether to add the implicit PCI controller based
on the machine type name only, because with every other QEMU arch when
doing such a major change as adding a PCI bus, they have created a new
machine type. The problem is that arm 'virt' machine type is not stable,
it is being changed arbitrarily in new QEMU releases :-(
So AFAIK, that leaves us with 3 choices
- Never add PCI controller at time the XML is defined, on the basis
that we have to be conservative in what we add to cope with old QEMU
- Always add PCI controller at time the XML is defined, on the basis
that most people will have new enough QEMU because ARM 'virt' machine
type is very much still in development, so no one will serously stick
with the older QEMU versions which lack PCI.
- Use the capabilities in XML post-parse to conditionally add the
PCI controller. This is what was currently merged
I don't think option 1 makes much sense as it'll harm ARM arch forever
more, to cope with QEMU versions that will almost never be used in
practice.
I'd be inclined to go with option 2, and then if any PCI devices are
actually used with the guest, check the capability at start time when
we are doing auto-address assignment.
Except that auto-address assignment happens after the parse, but before
we write the config to disk (i.e. we don't wait until the domain is
started. I'm guessing that was originally done because we didn't want to
be potentially re-writing the config to disk every time the domain was
started.) Also, we can't assume that no PCI devices used at start
means no PCI devices will ever be desired because hotplug.
At any rate, a machinetype that sometimes has an implied controller
device and sometimes doesn't is just a completely broken idea -
different instances of the same machinetype are assumed to
Re: [libvirt] [sandbox PATCH v2 11/19] Image: Add run function
On Tue, Aug 04, 2015 at 08:11:17PM +, Eren Yagdiran wrote: 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) This code actually ends up launching a sandbox using the template file as the root disk image. This is not good, because we need to be able to run multiple instances of the sandbox, all using the same template file. As such we need to be able to create a new temporary disk image for each sandbox instance, that is an overlay on the main template, an then delete this temporary disk at shutdown. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH v2 03/19] Image: Add Hooking Mechanism
On Tue, Aug 04, 2015 at 08:11:09PM +, Eren Yagdiran wrote: 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/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 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +* +* Author: Eren Yagdiran erenyagdi...@gmail.com +* +''' Python comment syntax is '# ' What you have done here is declare a string. Also since the copyright line has accents in it, you need to add #coding=utf-8 as the 2nd line in the file, otherwise python refuses to load the code at all saying it doens't know the character encoding. +#!/usr/bin/python THis should be the first line + +from abc import ABCMeta, abstractmethod + +class Source(): +__metaclass__ = ABCMeta +def __init__(self): +pass diff --git a/virt-sandbox-image/sources/__init__.py b/virt-sandbox-image/sources/__init__.py new file mode 100644 index 000..4830c7a --- /dev/null +++ b/virt-sandbox-image/sources/__init__.py @@ -0,0 +1,29 @@ +''' +* +* 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 erenyagdi...@gmail.com +* +''' +#!/usr/bin/python Same here about comment syntax, encoding and #!/usr being the first line. + +import os +import glob +modules = glob.glob(os.path.dirname(__file__)+/*.py) +__all__ = [ os.path.basename(f)[:-3] for f in modules] I don't think any of this stuff is needed, since we are directly importing the module we want based on its name diff --git a/virt-sandbox-image/virt-sandbox-image.py b/virt-sandbox-image/virt-sandbox-image.py old mode 100644 new mode 100755 index a9cb0ff..b20b240 --- a/virt-sandbox-image/virt-sandbox-image.py +++ b/virt-sandbox-image/virt-sandbox-image.py @@ -1,5 +1,5 @@ #!/usr/bin/python -Es -# +# -*- coding: utf-8 -*- # Authors: Daniel P. Berrange berra...@redhat.com # Eren Yagdiran erenyagdi...@gmail.com # @@ -38,6 +38,17 @@ default_template_dir = /var/lib/libvirt/templates debug = True verbose = True +sys.dont_write_bytecode = True This kind of change doesn't belong in the code. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |:
[libvirt] [PATCH v3 5/5] qemu: Implement virDomainRename
Currently supports only renaming inactive domains without snapshots.
Signed-off-by: Tomas Meszaros e...@tty.sk
---
src/qemu/qemu_driver.c | 145 +
1 file changed, 145 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6998e12..41e5fc4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19853,6 +19853,150 @@ qemuDomainSetUserPassword(virDomainPtr dom,
}
+static int qemuDomainRename(virDomainPtr dom,
+const char *new_name,
+unsigned int flags ATTRIBUTE_UNUSED)
+{
+virQEMUDriverPtr driver = dom-conn-privateData;
+virQEMUDriverConfigPtr cfg = NULL;
+virDomainObjPtr vm;
+virObjectEventPtr event_new = NULL;
+virObjectEventPtr event_old = NULL;
+int ret = -1;
+int logfile = -1;
+char ebuf[1024];
+char *timestamp;
+char *rename_log_msg = NULL;
+char *new_dom_name = NULL;
+char *old_dom_name = NULL;
+char *old_dom_cfg_file = NULL;
+
+if (VIR_STRDUP(new_dom_name, new_name) 0)
+goto cleanup;
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainRenameEnsureACL(dom-conn, vm-def) 0)
+goto cleanup;
+
+cfg = virQEMUDriverGetConfig(driver);
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
+goto cleanup;
+
+if (virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(cannot rename active domain));
+goto endjob;
+}
+
+if (!vm-persistent) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(cannot rename a transient domain));
+goto endjob;
+}
+
+if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_SHUTOFF) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(domain has to be shutoff before renaming));
+goto endjob;
+}
+
+if (virDomainSnapshotObjListNum(vm-snapshots, NULL, 0) 0) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(cannot rename domain with snapshots));
+goto endjob;
+}
+
+if (virAsprintf(rename_log_msg, : domain %s has been renamed to %s\n,
+vm-def-name, new_name) 0) {
+goto endjob;
+}
+
+if (!(old_dom_cfg_file = virDomainConfigFile(cfg-configDir,
+ vm-def-name))) {
+goto endjob;
+}
+
+if (virDomainObjListRenameAddNew(driver-domains, vm, new_name) 0)
+goto endjob;
+
+if ((logfile = qemuDomainCreateLog(driver, vm, true)) 0)
+goto rollback;
+
+event_old = virDomainEventLifecycleNewFromObj(vm,
+VIR_DOMAIN_EVENT_UNDEFINED,
+
VIR_DOMAIN_EVENT_UNDEFINED_RENAMED);
+
+/* Switch name in domain definition. */
+old_dom_name = vm-def-name;
+vm-def-name = new_dom_name;
+new_dom_name = NULL;
+
+if (virDomainSaveConfig(cfg-configDir, vm-def) 0)
+goto rollback;
+
+if (virFileExists(old_dom_cfg_file)
+unlink(old_dom_cfg_file) 0) {
+virReportSystemError(errno,
+ _(cannot remove old domain config file %s),
+ old_dom_cfg_file);
+goto rollback;
+}
+
+/* Remove old domain name from table. */
+virDomainObjListRenameRemove(driver-domains, old_dom_name);
+
+event_new = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_DEFINED,
+
VIR_DOMAIN_EVENT_DEFINED_RENAMED);
+
+/* Write message to the log. */
+if ((timestamp = virTimeStringNow()) != NULL) {
+if (safewrite(logfile, timestamp, strlen(timestamp)) 0 ||
+safewrite(logfile, rename_log_msg,
+ strlen(rename_log_msg)) 0) {
+VIR_WARN(Unable to write timestamp to logfile: %s,
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+}
+VIR_FREE(timestamp);
+}
+
+/* Success, domain has been renamed. */
+ret = 0;
+
+ endjob:
+qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
+if (VIR_CLOSE(logfile) 0) {
+VIR_WARN(Unable to close logfile: %s,
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+}
+virDomainObjEndAPI(vm);
+VIR_FREE(old_dom_cfg_file);
+VIR_FREE(old_dom_name);
+VIR_FREE(new_dom_name);
+VIR_FREE(rename_log_msg);
+if (event_old)
+qemuDomainEventQueue(driver, event_old);
+if (event_new)
+qemuDomainEventQueue(driver, event_new);
+virObjectUnref(cfg);
+return ret;
+
+ rollback:
+if (old_dom_name) {
+new_dom_name = vm-def-name;
+vm-def-name = old_dom_name;
+old_dom_name = NULL;
+}
+
+virDomainObjListRenameRemove(driver-domains, new_name);
+goto
[libvirt] [PATCH v3 1/5] Introduce virDomainRename API
Also, among with this new API new ACL that restricts rename capability
is invented too.
Signed-off-by: Tomas Meszaros e...@tty.sk
---
include/libvirt/libvirt-domain.h | 4
src/driver-hypervisor.h | 6 ++
src/libvirt-domain.c | 34 ++
src/libvirt_public.syms | 5 +
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 18 +-
src/remote_protocol-structs | 8
7 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index e8202cf..82f86bb 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3837,4 +3837,8 @@ int virDomainSetUserPassword(virDomainPtr dom,
const char *password,
unsigned int flags);
+int virDomainRename(virDomainPtr dom,
+const char *new_name,
+unsigned int flags);
+
#endif /* __VIR_LIBVIRT_DOMAIN_H__ */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 3275343..ae2ec4d 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -650,6 +650,11 @@ typedef int
(*virDrvDomainIsActive)(virDomainPtr dom);
typedef int
+(*virDrvDomainRename)(virDomainPtr dom,
+ const char *new_name,
+ unsigned int flags);
+
+typedef int
(*virDrvDomainIsPersistent)(virDomainPtr dom);
typedef int
@@ -1347,6 +1352,7 @@ struct _virHypervisorDriver {
virDrvConnectIsEncrypted connectIsEncrypted;
virDrvConnectIsSecure connectIsSecure;
virDrvDomainIsActive domainIsActive;
+virDrvDomainRename domainRename;
virDrvDomainIsPersistent domainIsPersistent;
virDrvDomainIsUpdated domainIsUpdated;
virDrvConnectCompareCPU connectCompareCPU;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 837933f..b8618d8 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -8774,6 +8774,40 @@ virDomainIsPersistent(virDomainPtr dom)
return -1;
}
+/**
+ * virDomainRename:
+ * @dom: pointer to the domain object
+ * @new_name: new domain name
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Rename a domain. New domain name is specified in the second
+ * argument. Depending on each driver implementation it may be
+ * required that domain is in a specific state.
+ *
+ * Returns 0 if renamed, -1 on error
+ */
+int
+virDomainRename(virDomainPtr dom, const char *new_name,
+unsigned int flags ATTRIBUTE_UNUSED)
+{
+VIR_DEBUG(dom=%p, new_name=%s, dom, NULLSTR(new_name));
+
+virResetLastError();
+virCheckDomainReturn(dom, -1);
+virCheckNonNullArgGoto(new_name, error);
+
+if (dom-conn-driver-domainRename) {
+int ret = dom-conn-driver-domainRename(dom, new_name, flags);
+if (ret 0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+ error:
+virDispatchError(dom-conn);
+return -1;
+}
/**
* virDomainIsUpdated:
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 2c653f2..dd94191 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -720,4 +720,9 @@ LIBVIRT_1.2.17 {
virTypedParamsAddStringList;
} LIBVIRT_1.2.16;
+LIBVIRT_1.2.19 {
+global:
+virDomainRename;
+} LIBVIRT_1.2.17;
+
# define new API here using predicted next version number
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 5c4cf7c..ec26ebe 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8391,6 +8391,7 @@ static virHypervisorDriver hypervisor_driver = {
.domainGetFSInfo = remoteDomainGetFSInfo, /* 1.2.11 */
.domainInterfaceAddresses = remoteDomainInterfaceAddresses, /* 1.2.14 */
.domainSetUserPassword = remoteDomainSetUserPassword, /* 1.2.16 */
+.domainRename = remoteDomainRename, /* 1.2.19 */
};
static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 9f1be6b..770aa72 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -3230,6 +3230,15 @@ struct remote_domain_set_user_password_args {
unsigned int flags;
};
+struct remote_domain_rename_args {
+remote_nonnull_domain dom;
+remote_string new_name;
+unsigned int flags;
+};
+
+struct remote_domain_rename_ret {
+int rename;
+};
/*- Protocol. -*/
@@ -5696,5 +5705,12 @@ enum remote_procedure {
* @generate:both
* @acl: domain:set_password
*/
-REMOTE_PROC_DOMAIN_SET_USER_PASSWORD = 357
+REMOTE_PROC_DOMAIN_SET_USER_PASSWORD = 357,
+
+/**
+ * @generate: both
+ * @acl: domain:write
+ * @acl: domain:save
+ */
+REMOTE_PROC_DOMAIN_RENAME = 358
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 48c3bd8..1fdaa55 100644
---
[libvirt] [PATCH v3 0/5] domainRename API implementation
This is an effort to implement domain rename API. Presented patch series consists of the following: virDomainRename API implementation for qemu, implementation of the virsh command domrename and the additional support code. The idea behind this endeavor is to provide convenient and safe way to rename a domain. Instead of the: virsh dumpxml domain domain.xml (change domain name in domain.xml) virsh undefine domain virsh define domain.xml user can simply type: virsh domrename foo bar or call virDomainRename() API and domain foo will be renamed to bar. We currently support only renaming inactive domains without snapshots. Renaming procedure takes care of domain log, config, guest agent path and should be able to recover in case of failure. I've been working on this functionality in collaboration with Michal Privoznik who is my mentor during the GSoC 2015. If you have any questions, ideas or criticism feel free to join the discussion. v2: - removed guest agent path rename code - removed rename permission - added code for emitting undefined+renamed event for the old domain v3: - removed domain rename permission - fixed virDomainRename doc comment - added @flags parameter to the virDomainRename API Tomas Meszaros (5): Introduce virDomainRename API virsh: Implement domrename command domain_conf: Introducde virDomainObjListRenameAddNew() virDomainObjListRenameRemove() Introduce new VIR_DOMAIN_EVENT_DEFINED_RENAMED event qemu: Implement virDomainRename examples/object-events/event-test.c | 4 + include/libvirt/libvirt-domain.h| 6 ++ src/conf/domain_conf.c | 35 + src/conf/domain_conf.h | 5 ++ src/driver-hypervisor.h | 6 ++ src/libvirt-domain.c| 34 + src/libvirt_private.syms| 2 + src/libvirt_public.syms | 5 ++ src/qemu/qemu_driver.c | 145 src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x| 18 - src/remote_protocol-structs | 8 ++ tools/virsh-domain.c| 63 +++- tools/virsh.pod | 7 ++ 14 files changed, 336 insertions(+), 3 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/5] domainRename API implementation
On 08/10/2015 01:59 PM, Tomas Meszaros wrote: This is an effort to implement domain rename API. Presented patch series consists of the following: virDomainRename API implementation for qemu, implementation of the virsh command domrename and the additional support code. The idea behind this endeavor is to provide convenient and safe way to rename a domain. Instead of the: virsh dumpxml domain domain.xml (change domain name in domain.xml) virsh undefine domain virsh define domain.xml user can simply type: virsh domrename foo bar or call virDomainRename() API and domain foo will be renamed to bar. Cool! Although I haven't reviewed it closely, this series is looking good. But I do have one small question: Do you have the time and energy to provide an implementation for the Xen libxl driver? :-). Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 1/2] network: added waiting for DAD to finish for bridge address.
On 08/10/2015 01:08 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.
Since the comments in our code imply that dnsmasq should be waiting for
DAD to complete prior to daemonizing, before pushing a fix like this I'd
like to find out from the dnsmasq folks if we are erroneously relying on
nonexistent dnsmasq behavior, or if maybe there is a bug in some version
of dnsmasq.
Simon (or other dnsmasq people) - when dnsmasq is run with enable-ra,
does it make sure it completes DAD prior to daemonizing? Or does libvirt
need to do this extra polling to assure that DAD has completed? (or
maybe there's some other config parameter we need to add?)
---
Difference to v2: Moved to virnetdev.
src/libvirt_private.syms| 1 +
src/network/bridge_driver.c | 35 +-
src/util/virnetdev.c| 160
src/util/virnetdev.h| 2 +
4 files changed, 197 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0517c24..fa9e1c1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1776,6 +1776,7 @@ virNetDevSetRcvMulti;
virNetDevSetupControl;
virNetDevSysfsFile;
virNetDevValidateConfig;
+virNetDevWaitDadFinish;
# util/virnetdevbandwidth.h
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 3d6721b..2172a3d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2026,6 +2026,33 @@ networkAddRouteToBridge(virNetworkObjPtr network,
}
static int
+networkWaitDadFinish(virNetworkObjPtr network)
+{
+virNetworkIpDefPtr ipdef;
+virSocketAddrPtr *addrs = NULL;
+size_t i;
+int ret;
+for (i = 0;
+ (ipdef = virNetworkDefGetIpByIndex(network-def, AF_INET6, i));
+ i++) {}
+
+if (i == 0)
+return 0;
+if (VIR_ALLOC_N(addrs, i))
+return -1;
+
+for (i = 0;
+ (ipdef = virNetworkDefGetIpByIndex(network-def, AF_INET6, i));
+ i++) {
+addrs[i] = ipdef-address;
+}
+
+ret = virNetDevWaitDadFinish(addrs, i);
+VIR_FREE(addrs);
+return ret;
+}
+
+static int
networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
virNetworkObjPtr network)
{
@@ -2159,7 +2186,13 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr
driver,
if (v6present networkStartRadvd(driver, network) 0)
goto err4;
-/* DAD has happened (dnsmasq waits for it), dnsmasq is now bound to the
+/* dnsmasq main process does not wait for DAD to complete,
+ * so we need to wait for it ourselves.
+ */
+if (v6present networkWaitDadFinish(network) 0)
+goto err4;
+
+/* DAD has happened, dnsmasq is now bound to the
* bridge's IPv6 address, so we can now set the dummy tun down.
*/
if (tapfd = 0) {
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 1e20789..c81342a 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -96,6 +96,7 @@ VIR_LOG_INIT(util.netdev);
# define FEATURE_BIT_IS_SET(blocks, index, field)\
(FEATURE_WORD(blocks, index, field) FEATURE_FIELD_FLAG(index))
#endif
+# define IP_BUF_SIZE 4096
typedef enum {
VIR_MCAST_TYPE_INDEX_TOKEN,
@@ -1219,6 +1220,103 @@ int virNetDevClearIPAddress(const char *ifname,
return ret;
}
+/* return whether there is a known address with 'tentative' flag set */
+static int
+virNetDevParseDadStatus(struct nlmsghdr *nlh, int len,
+virSocketAddrPtr *addrs, size_t count)
+{
+struct ifaddrmsg *ifaddrmsg_ptr;
+unsigned int ifaddrmsg_len;
+struct rtattr *rtattr_ptr;
+size_t i;
+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. */
+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;
+}
+
+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;
+}
Re: [libvirt] [PATCHv3 2/2] Add support for multi-part netlink messages.
On 08/10/2015 01:08 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. --- 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..f02bb59 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; 1) It's interesting that they don't seem to define what type the message will be in these cases (it's not NLMSG_DONE or NLMSG_ERROR, and the only other standard types are NLMSG_NOOP and NLMSG_OVERRUN.) So what *is* the type in the case of a multipart message. 2) Doesn't the presence of the NLM_F_MULTI flag indicate that we need to look for another packet, rather than just returning? It's been a long time since I looked at the details of the netlink message handling, but won't this constipate the socket? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/9] bridge_driver: Introduce networkBandwidthChangeAllowed
On 08/03/2015 02:39 AM, Michal Privoznik wrote:
When a domain vNIC's bandwidth is to be changed (at runtime) it is
possible that guaranteed minimal bandwidth (@floor) will change too.
Well, so far it is, because we still don't have an implementation that
allows setting it dynamically, so it's effectively erased on:
#virsh domiftune $dom vnet0 --inbound 0
However, that's slightly unfortunate. We do some checks on domain
startup to see if @floor can be guaranteed. We ought do the same if
QoS is changed at runtime.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/network/bridge_driver.c | 72
+++--
src/network/bridge_driver.h | 12
2 files changed, 82 insertions(+), 2 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 17fc430..fa60ba4 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4686,9 +4686,18 @@ networkGetNetworkAddress(const char *netname, char
**netaddr)
* networkCheckBandwidth:
* @net: network QoS
* @ifaceBand: interface QoS (may be NULL if no QoS)
+ * @oldBandwidth: new interface QoS (may be NULL if no QoS)
* @ifaceMac: interface MAC (used in error messages for identification)
* @new_rate: new rate for non guaranteed class
*
+ * Function checks if @ifaceBand can be satisfied on @net. However,
sometimes it
+ * may happen that the interface that @ifaceBand corresponds to is already
+ * plugged into the @net and the bandwidth is to be updated. In that case we
+ * need to check if new bandwidth can be satisfied. If that's the case
+ * @ifaceBand should point to new bandwidth settings and @oldBandwidth to
+ * current ones. If you want to suppress this functionality just pass
+ * @oldBandwidth == NULL.
+ *
* Returns: -1 if plugging would overcommit network QoS
* 0 if plugging is safe (@new_rate updated)
* 1 if no QoS is set (@new_rate untouched)
new_rate is unchanged if passed as NULL too
Part of me says - who cares - just set it and let the caller decide -
both paths will always have that extra if check now... and it only
ever matters if plugging is safe.
I'm OK keeping as is - so I'll ACK and let you decide. At the very least
point out that new_rate can be NULL in the comments. I'm guessing that
if the caller that has it NULL had a value, then there'd be a warning
about an unused variable...
John
@@ -4696,6 +4705,7 @@ networkGetNetworkAddress(const char *netname, char
**netaddr)
static int
networkCheckBandwidth(virNetworkObjPtr net,
virNetDevBandwidthPtr ifaceBand,
+ virNetDevBandwidthPtr oldBandwidth,
virMacAddr ifaceMac,
unsigned long long *new_rate)
{
@@ -4723,6 +4733,8 @@ networkCheckBandwidth(virNetworkObjPtr net,
}
tmp_new_rate = netBand-in-average;
+if (oldBandwidth oldBandwidth-in)
+tmp_floor_sum -= oldBandwidth-in-floor;
tmp_floor_sum += ifaceBand-in-floor;
/* check against peak */
@@ -4749,7 +4761,8 @@ networkCheckBandwidth(virNetworkObjPtr net,
goto cleanup;
}
-*new_rate = tmp_new_rate;
+if (new_rate)
+*new_rate = tmp_new_rate;
ret = 0;
cleanup:
@@ -4791,7 +4804,7 @@ networkPlugBandwidth(virNetworkObjPtr net,
char ifmac[VIR_MAC_STRING_BUFLEN];
virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);
-if ((plug_ret = networkCheckBandwidth(net, ifaceBand,
+if ((plug_ret = networkCheckBandwidth(net, ifaceBand, NULL,
iface-mac, new_rate)) 0) {
/* helper reported error */
goto cleanup;
@@ -4917,3 +4930,58 @@ networkNetworkObjTaint(virNetworkObjPtr net,
virNetworkTaintTypeToString(taint));
}
}
+
+
+static bool
+networkBandwidthGenericChecks(virDomainNetDefPtr iface,
+ virNetDevBandwidthPtr newBandwidth)
+{
+virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);
+unsigned long long old_floor, new_floor;
+
+if (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_NETWORK) {
+/* This is not an interface that's plugged into a network.
+ * We don't care. Thus from our POV bandwidth change is allowed. */
+return false;
+}
+
+old_floor = new_floor = 0;
+
+if (ifaceBand ifaceBand-in)
+old_floor = ifaceBand-in-floor;
+if (newBandwidth newBandwidth-in)
+new_floor = newBandwidth-in-floor;
+
+return new_floor != old_floor;
+}
+
+
+bool
+networkBandwidthChangeAllowed(virDomainNetDefPtr iface,
+ virNetDevBandwidthPtr newBandwidth)
+{
+virNetworkDriverStatePtr driver = networkGetDriver();
+virNetworkObjPtr network = NULL;
+virNetDevBandwidthPtr ifaceBand =
[libvirt] [PATCH] domain: Fix crash if trying to live update disk serial
If you pass diskserial XML to UpdateDevice, and the original device
didn't have a serial block, libvirtd crashes trying to read the original
NULL serial string.
Use _NULLABLE string comparisons to avoid the crash. A couple other
properties needed the change too.
---
src/conf/domain_conf.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fd0450f..f1e02e3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5871,28 +5871,28 @@ virDomainDiskDiffersSourceOnly(virDomainDiskDefPtr disk,
CHECK_EQ(transient, transient, true);
-if (disk-serial STRNEQ(disk-serial, orig_disk-serial)) {
+if (disk-serial STRNEQ_NULLABLE(disk-serial, orig_disk-serial)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_(cannot modify field '%s' of the disk),
serial);
return false;
}
-if (disk-wwn STRNEQ(disk-wwn, orig_disk-wwn)) {
+if (disk-wwn STRNEQ_NULLABLE(disk-wwn, orig_disk-wwn)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_(cannot modify field '%s' of the disk),
wwn);
return false;
}
-if (disk-vendor STRNEQ(disk-vendor, orig_disk-vendor)) {
+if (disk-vendor STRNEQ_NULLABLE(disk-vendor, orig_disk-vendor)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_(cannot modify field '%s' of the disk),
vendor);
return false;
}
-if (disk-product STRNEQ(disk-product, orig_disk-product)) {
+if (disk-product STRNEQ_NULLABLE(disk-product, orig_disk-product)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_(cannot modify field '%s' of the disk),
product);
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 2/4] qemu: Add PCI-Express root to ARM virt machine
On 08/10/2015 11:09 AM, Daniel P. Berrange wrote:
On Thu, Aug 06, 2015 at 07:46:58PM +0200, Martin Kletzander wrote:
On Thu, Aug 06, 2015 at 06:37:41PM +0200, Martin Kletzander wrote:
On Fri, Jul 17, 2015 at 02:27:45PM +0300, Pavel Fedin wrote:
Here we assume that if qemu supports generic PCI host controller,
it is a part of virt machine and can be used for adding PCI devices.
In qemu this is actually a PCIe bus, so we also declare multibus
capability so that 0'th bus is specified to qemu correctly as 'pcie.0'
Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
src/qemu/qemu_capabilities.c | 8
src/qemu/qemu_domain.c | 17 +
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d570fdd..f3486c7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2138,6 +2138,14 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr
qemuCaps,
return false;
}
+if (ARCH_IS_ARM(def-os.arch)) {
+/* If 'virt' supports PCI, it supports multibus.
+ * No extra conditions here for simplicity.
+ */
So every ARM qemu with the virt machine type supports both PCI and
multiqueue? How about those virt-* for which you check below. That
might not be related, I'm just curious.
+if (STREQ(def-os.machine, virt))
+return true;
+}
+
return false;
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8b050a0..c7d14e4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -981,7 +981,7 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace
= {
static int
qemuDomainDefPostParse(virDomainDefPtr def,
virCapsPtr caps,
- void *opaque ATTRIBUTE_UNUSED)
+ void *opaque)
{
bool addDefaultUSB = true;
bool addImplicitSATA = false;
@@ -1030,12 +1030,21 @@ qemuDomainDefPostParse(virDomainDefPtr def,
break;
case VIR_ARCH_ARMV7L:
- addDefaultUSB = false;
- addDefaultMemballoon = false;
- break;
case VIR_ARCH_AARCH64:
addDefaultUSB = false;
addDefaultMemballoon = false;
+ if (STREQ(def-os.machine, virt) ||
+ STRPREFIX(def-os.machine, virt-)) {
+ virQEMUDriverPtr driver = opaque;
+
+ /* This condition is actually a (temporary) hack for test
suite which
+* does not create capabilities cache */
Few questions here. a) how temporary is this since you're not
removing it in this series? b) for what tests you need this hack and
what part of the below is the hack?
Moreover, you cannot use capabilities when defining an XML. The
emulator can change between the domain is defined and started, so you
cannot know with what emulator this will be started.
I see Michal (Cc'd) just pushed this, I probably just missed the mail
Of course I forgot, Cc'ing now.
I agree with your core statement that we should not be using the QEMU
capabilities when defining the XML. With all existing scenarios we have
been able to determine whether to add the implicit PCI controller based
on the machine type name only, because with every other QEMU arch when
doing such a major change as adding a PCI bus, they have created a new
machine type. The problem is that arm 'virt' machine type is not stable,
it is being changed arbitrarily in new QEMU releases :-(
So AFAIK, that leaves us with 3 choices
- Never add PCI controller at time the XML is defined, on the basis
that we have to be conservative in what we add to cope with old QEMU
- Always add PCI controller at time the XML is defined, on the basis
that most people will have new enough QEMU because ARM 'virt' machine
type is very much still in development, so no one will serously stick
with the older QEMU versions which lack PCI.
- Use the capabilities in XML post-parse to conditionally add the
PCI controller. This is what was currently merged
I don't think option 1 makes much sense as it'll harm ARM arch forever
more, to cope with QEMU versions that will almost never be used in
practice.
I'd be inclined to go with option 2, and then if any PCI devices are
actually used with the guest, check the capability at start time when
we are doing auto-address assignment.
Another option: add versioned 'virt' machine types to the next qemu release
(like virt-2.5 etc.), and key libvirt enabling pci off of that.
_Eventually_ we are going to need versioned 'virt' machine types for migration
compatibility like we already do with x86 -M pc-*. Might as well make the
change early so libvirt can actually use it.
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 3/4] qemu: Build correct command line for PCI NICs on ARM
On 08/10/2015 12:29 PM, Laine Stump wrote: On 07/17/2015 07:27 AM, Pavel Fedin wrote: Legacy -net option works correctly only with embedded device models, which do not require any bus specification. Therefore, we should use -device for PCI hardware This function is starting to look a bit like a hack on top of a hack on top of a hack. Are the original reasons for using -net instead of -device on certain arches still applicable? Yes, the old logic is still needed for all non-virtio uses of arm and aarch64, and potentially a ton of other qemu architectures if we ever wanted to properly support them. - Cole Signed-off-by: Pavel Fedin p.fe...@samsung.com --- src/qemu/qemu_command.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 42906a8..5569be6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -457,7 +457,8 @@ qemuDomainSupportsNicdev(virDomainDefPtr def, /* non-virtio ARM nics require legacy -net nic */ if (((def-os.arch == VIR_ARCH_ARMV7L) || (def-os.arch == VIR_ARCH_AARCH64)) -net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) +net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO +net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) return false; return true; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/9] virNetDevBandwidthUpdateRate: turn class_id into integer
On 08/03/2015 02:39 AM, Michal Privoznik wrote:
This is no functional change. It's just that later in the series we
will need to pass class_id as an integer.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/network/bridge_driver.c | 4 ++--
src/util/virnetdevbandwidth.c | 10 +++---
src/util/virnetdevbandwidth.h | 2 +-
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 3d6721b..17fc430 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4840,7 +4840,7 @@ networkPlugBandwidth(virNetworkObjPtr net,
}
/* update rate for non guaranteed NICs */
new_rate -= net-floor_sum;
-if (virNetDevBandwidthUpdateRate(net-def-bridge, 1:2,
+if (virNetDevBandwidthUpdateRate(net-def-bridge, 2,
net-def-bandwidth, new_rate) 0)
VIR_WARN(Unable to update rate for 1:2 class on %s bridge,
net-def-bridge);
@@ -4891,7 +4891,7 @@ networkUnplugBandwidth(virNetworkObjPtr net,
}
/* update rate for non guaranteed NICs */
new_rate -= net-floor_sum;
-if (virNetDevBandwidthUpdateRate(net-def-bridge, 1:2,
+if (virNetDevBandwidthUpdateRate(net-def-bridge, 2,
net-def-bandwidth, new_rate) 0)
VIR_WARN(Unable to update rate for 1:2 class on %s bridge,
net-def-bridge);
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 6ae0877..91201ae 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -638,7 +638,8 @@ virNetDevBandwidthUnplug(const char *brname,
/**
* virNetDevBandwidthUpdateRate:
* @ifname: interface name
- * @classid: ID of class to update
+ * @id: unique identifier (MUST be greater than 2)
+ * @bandwidth: used to derive 'ceil' of class with @id
* @new_rate: new rate
*
* This function updates the 'rate' attribute of HTB class.
@@ -650,16 +651,18 @@ virNetDevBandwidthUnplug(const char *brname,
*/
int
virNetDevBandwidthUpdateRate(const char *ifname,
- const char *class_id,
+ unsigned int id,
virNetDevBandwidthPtr bandwidth,
unsigned long long new_rate)
{
int ret = -1;
virCommandPtr cmd = NULL;
+char *class_id = NULL;
char *rate = NULL;
char *ceil = NULL;
-if (virAsprintf(rate, %llukbps, new_rate) 0 ||
+if (virAsprintf(class_id, 1:%x, id) 0 ||
+virAsprintf(rate, %llukbps, new_rate) 0 ||
virAsprintf(ceil, %llukbps, bandwidth-in-peak ?
bandwidth-in-peak :
bandwidth-in-average) 0)
@@ -677,6 +680,7 @@ virNetDevBandwidthUpdateRate(const char *ifname,
cleanup:
virCommandFree(cmd);
+VIR_FREE(class_id);
VIR_FREE(rate);
VIR_FREE(ceil);
return ret;
diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
index 9b1d2a6..f42094c 100644
--- a/src/util/virnetdevbandwidth.h
+++ b/src/util/virnetdevbandwidth.h
@@ -68,7 +68,7 @@ int virNetDevBandwidthUnplug(const char *brname,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
int virNetDevBandwidthUpdateRate(const char *ifname,
- const char *class_id,
+ unsigned int id,
virNetDevBandwidthPtr bandwidth,
unsigned long long new_rate)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
You'll need to remove the ATTRIBUTE_NONNULL(2) to make Coverity builds
happy... You can move the ATTRIBUTE_RETURN_CHECK to the same line if
you want also.
ACK with that adjustment,
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 0/4] qemu: Allow PCI virtio on ARM virt machine
On 08/06/2015 09:38 AM, Michal Privoznik wrote: On 06.08.2015 15:06, Pavel Fedin wrote: Hello! ACKed all the patches, squashed in the 2/4 amendment and pushed. Sorry for the delay. Thank you very much. By the way, just curious, why is there a restriction to plug in most of devices only into PCI, not PCIe? On machines with PCIe host this seems to do nothing good, only prevents them from using MSI-X. The original patches to support pcie-root severely restricted what could plug into what because in real hardware you can't plug a PCI device into a PCIe slot (physically it doesn't work), and although it did happen to work with qemu, I had no information to tell me that, e.g. plugging a PCI device into a PCIe slot, or PCIe device into a PCI slot would continue to work in the future. I've since received assurances from qemu developers that this will continue to work, so the restriction has been loosened (see below). For example virtio-scsi still suffers from this problem. I ignore it for now because we currently don't have vhost-scsi support in qemu. Comments say that PCIe is not hotpluggable. You cannot hotplug directly into pcie-root. This is true in the real world, and either is or should be true in qemu. hotplug with pcie is supported by other pcie controllers that are connected into pcie-root at guest startup time. But why? because that's the way it was originally implemented in hardware? And why do we mandate hotplug? We don't (any longer) mandate it. But in the absence of any information to the contrary, we assume that the user might want it, so we try not to make a setup that will surprise and disappoint them. In the past, the rules about insisting on a hotpluggable port (and only plugging PCI devices into true PCI slots rather than PCIe) were strictly enforced by libvirt all the time, but there was an open BZ about that which I resolved a couple months ago: https://bugzilla.redhat.com/show_bug.cgi?id=1238338 The behavior now is that if libvirt is auto-assigning a slot for a device, it will put it into a hotpluggable true PCI slot, but if you manually assign it to a slot that is non-hotpluggable and/or PCIe, it will be allowed. Beyond that, just last night I pushed a series of patches supporting three new PCIe controller types: pcie-root-port, pcie-switch-upstream-port, and pcie-switch-downstream-port. With those implemented, we can now implement pretty much any standard PCIe bus topology, so you can now hotplug into a true PCIe port. (Note that there are a few restrictions about which type of controllers can be plugged into which other controllers and these *are* strictly enforced, because not following those rules leads to non-working configurations; for example, a pcie-switch-upstream-port cannot be plugged directly into pcie-root; it can *only* be plugged into a pcie-root-port or a pcie-switch-downstream-port.) Yeah, that's a good question. In a real world, PCIe is perfectly hot pluggable. Maybe the limitation comes from qemu? That is a very broad statement, and isn't true in all cases. For example, in the real world you can't hotplug/unplug directly to the pcie-root bus (nor can you hotplug/unplug to the dmi-to-pci-bridge (aka i8280111-bridge). But you *can* (at guest startup time) attach a pcie-root-port to the pcie-root, or a pcie-switch-upstream-port+pcie-switch-downstream-ports to pcie-root or to a dmi-to-pci-bridge, then hotplug/unplug to a pcie-root-port or a pcie-switch-downstream-port (do a git pull of master to get the new controller types I explained above and you'll see what I'm talking about). BTW, I'm still wondering if the arm machinetype really does support the supposedly Interl-x86-specific i82801b11-bridge device (and the new controller devices - ioh3420 (pcie-root-port), x3130-upstream (pcie-switch-upstream-port), and xio3130-downstream (pcie-switch-downstream-port). In particular, the i82801b11-bridge is important, since it is automatically added by libvirt when pcie-root is present. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Buffer overflow error when starting libvirtd
Compiled libvirt 1.2.18 on Ubuntu 12.04.5 (installed over 12.04 libvirt-bin 0.9.9 package files); when I try to start libvirtd, I get the following error / backtrace: $ sudo /usr/sbin/libvirtd *** stack smashing detected ***: /usr/sbin/libvirtd terminated === Backtrace: = /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x37)[0x7f02fe6d3e57] /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x0)[0x7f02fe6d3e20] /usr/lib/libvirt.so.0(+0xaa359)[0x7f02fec4f359] /usr/lib/libvirt/connection-driver/libvirt_driver_nodedev.so(+0x99fe)[0x7f02f1fc59fe] /usr/lib/libvirt/connection-driver/libvirt_driver_nodedev.so(+0xa672)[0x7f02f1fc6672] /usr/lib/libvirt.so.0(virStateInitialize+0xaf)[0x7f02fed1cd2f] /usr/sbin/libvirtd(+0x17d80)[0x7f02ff98dd80] /usr/lib/libvirt.so.0(+0xd25d2)[0x7f02fec775d2] /lib/x86_64-linux-gnu/libpthread.so.0(+0x7e9a)[0x7f02fe98fe9a] /lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)[0x7f02fe6bd38d] Not sure if this may be a bug with 1.2.18, or a fault with my configure/compile/install process; please advise... Thanks, Will -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: fix virNetDevSendEthtoolIoctl() and its callers
This started out as a fix for a crash reported in IRC and on libvir-list:
https://www.redhat.com/archives/libvir-list/2015-August/msg00162.html
but as I examined the existing code I found so many nits to pick that
I just did them all.
The most important fix here is that virNetDevGetFeatures was creating
a struct ethtool_gfeatures object as a local on the stack and,
although the struct is defined with 0 elements in its features array,
we were telling the ethtool ioctl that we had made space for 2
elements. This led to a crash, as outlined in the email above. The fix
for this is to allocate the memory for the ethtool_gfeatures object
using VIR_ALLOC_N to a char*, giving it the length:
sizeof(ethtool_gfeatures) + (2 * sizeof(ethtool_get_features_block)
because VIR_ALLOC_N is a macro and fails when you try to typecast a
pointer to char* within the invocation, I made a union that has both a
char* and an ethtool_gfeatures*, and used the char* of the union for
VIR_ALLOC_N, and the other for everything else.
Beyond that crash fixer, the following fixups were made to the
hierarchy of functions between virNetDevGetFeatures() and
virNetDevSendEthtoolIoctl():
* macros used to examine the gfeatures bits were renamed from
FEATURE_* to GFEATURE_*
virNetDevSentEthtoolIoctl():
* no need to initialize sock to -1, since it is always set at the top
of the function.
* remove VIR_DEBUG log (and subsequent *skipping* of error log!) when
errno is EPERM, EINVAL or EOPNOTSUPP. If one of those errors were
ever encountered, this would have been *very* problematic, as it
would have led to one of those situations where virsh reports an
error was encountered but the cause is unknown (or whatever the
message is when we have an error but no log message).
* always call VIR_FORCE_CLOSE(sock) since we know that sock is either
a valid fd, or else -1 (which VIR_FORCE_CLOSE() will skip).
virNetDevFeatureAvailable()
* simplify it - no need for ret.
* follow libvirt convention of checking for bobLobLaw(lawblog) 0
instead of !bobLobLaw(lawblog).
virNetDevGFeatureAvailable()
* eliminate this function, as it was ill-conceived (it really was only
checking for one gfeature (TX_UDP_TNL), not *any* gfeature.
virNetDevGetFeatures()
* move all data tables (struct elem) out of the function so that they
will be statics instead of taking up space on the stack.
* remove pointless/incorrect initialization of i = -1.
* remove unnecessary static initialization of struct ethtool_value cmd
* put struct ethtool_gfeatures into a union with a char* as described above
* use libvirt convention of checking return from
virNetDevFeatureAvailable() 0, instead of
!virNetDevFeatureAvailable(), and immediately return to caller
with an error when virNetDevFeatureAvailable() returns an error
(previously it was ignored).
* remove superfluous size_t j, and just re-use i instead.
* runtime allocation/free of proper size object for ethtoolfeatures as
described above.
* directly call virNetDevSentEthtoolIoctl() instead of now defunct
virNetDevGFeatureAvailable().
===
NB: This patch does *not* attempt to determine the proper number of
elements for the gfeature array at runtime, as proposed in this patch:
https://www.redhat.com/archives/libvir-list/2015-August/msg00263.html
since that wasn't the cause of the crash. I'll leave it up to Moshe to
repost that patch rebased around this one (or whatever ends up being
pushed) if he thinks there is value to it.
Also - as I mentioned in earlier mail in response to the crash, I
noticed when looking into the gfeatures ethtool code that it looks to
me like TX_UDP_TNL should actually be 26 rather than 25, but I may be
missing something. Moshe - can you either confirm or deny that? Where
did you get the value 25 from?
---
src/util/virnetdev.c | 182 ---
1 file changed, 84 insertions(+), 98 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 1e20789..7f0837d 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -91,10 +91,10 @@ VIR_LOG_INIT(util.netdev);
#if HAVE_DECL_ETHTOOL_GFEATURES
# define TX_UDP_TNL 25
# define GFEATURES_SIZE 2
-# define FEATURE_WORD(blocks, index, field) ((blocks)[(index) / 32U].field)
-# define FEATURE_FIELD_FLAG(index) (1U (index) % 32U)
-# define FEATURE_BIT_IS_SET(blocks, index, field)\
-(FEATURE_WORD(blocks, index, field) FEATURE_FIELD_FLAG(index))
+# define GFEATURE_WORD(blocks, index, field) ((blocks)[(index) / 32U].field)
+# define GFEATURE_FIELD_FLAG(index) (1U (index) % 32U)
+# define GFEATURE_BIT_IS_SET(blocks, index, field)\
+(GFEATURE_WORD(blocks, index, field) GFEATURE_FIELD_FLAG(index))
#endif
typedef enum {
@@ -3032,11 +3032,10 @@ static int
virNetDevSendEthtoolIoctl(const char *ifname, void *cmd)
{
int ret = -1;
-int sock = -1;
+int sock;
virIfreq ifr;
-sock = socket(AF_LOCAL,
[libvirt] [PATCH v3 4/5] Introduce new VIR_DOMAIN_EVENT_DEFINED_RENAMED event
This should be emitted whenever a domain is renamed.
Signed-off-by: Tomas Meszaros e...@tty.sk
---
examples/object-events/event-test.c | 4
include/libvirt/libvirt-domain.h| 2 ++
tools/virsh-domain.c| 6 --
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/examples/object-events/event-test.c
b/examples/object-events/event-test.c
index 4f17273..dcae981 100644
--- a/examples/object-events/event-test.c
+++ b/examples/object-events/event-test.c
@@ -108,10 +108,14 @@ static const char *eventDetailToString(int event, int
detail) {
ret = Added;
else if (detail == VIR_DOMAIN_EVENT_DEFINED_UPDATED)
ret = Updated;
+else if (detail == VIR_DOMAIN_EVENT_DEFINED_RENAMED)
+ret = Renamed;
break;
case VIR_DOMAIN_EVENT_UNDEFINED:
if (detail == VIR_DOMAIN_EVENT_UNDEFINED_REMOVED)
ret = Removed;
+else if (detail == VIR_DOMAIN_EVENT_UNDEFINED_RENAMED)
+ret = Renamed;
break;
case VIR_DOMAIN_EVENT_STARTED:
switch ((virDomainEventStartedDetailType) detail) {
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 82f86bb..77304f0 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2322,6 +2322,7 @@ typedef enum {
typedef enum {
VIR_DOMAIN_EVENT_DEFINED_ADDED = 0, /* Newly created config file */
VIR_DOMAIN_EVENT_DEFINED_UPDATED = 1, /* Changed config file */
+VIR_DOMAIN_EVENT_DEFINED_RENAMED = 2, /* Domain was renamed */
# ifdef VIR_ENUM_SENTINELS
VIR_DOMAIN_EVENT_DEFINED_LAST
@@ -2335,6 +2336,7 @@ typedef enum {
*/
typedef enum {
VIR_DOMAIN_EVENT_UNDEFINED_REMOVED = 0, /* Deleted the config file */
+VIR_DOMAIN_EVENT_UNDEFINED_RENAMED = 1, /* Domain was renamed */
# ifdef VIR_ENUM_SENTINELS
VIR_DOMAIN_EVENT_UNDEFINED_LAST
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 128cfac..2325360 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11763,12 +11763,14 @@ VIR_ENUM_DECL(vshDomainEventDefined)
VIR_ENUM_IMPL(vshDomainEventDefined,
VIR_DOMAIN_EVENT_DEFINED_LAST,
N_(Added),
- N_(Updated))
+ N_(Updated),
+ N_(Renamed))
VIR_ENUM_DECL(vshDomainEventUndefined)
VIR_ENUM_IMPL(vshDomainEventUndefined,
VIR_DOMAIN_EVENT_UNDEFINED_LAST,
- N_(Removed))
+ N_(Removed),
+ N_(Renamed))
VIR_ENUM_DECL(vshDomainEventStarted)
VIR_ENUM_IMPL(vshDomainEventStarted,
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/5] virsh: Implement domrename command
This patch implements new virsh command, domrename.
Using domrename, it will be possible to rename domain from the virsh shell by
calling virRenameDomain API.
It takes two arguments, current domain name and new domain name.
Example:
virsh # list --all
IdName State
- barshut off
virsh # domrename bar foo
Domain successfully renamed
virsh # list --all
IdName State
- fooshut off
virsh #
Signed-off-by: Tomas Meszaros e...@tty.sk
---
tools/virsh-domain.c | 57
tools/virsh.pod | 7 +++
2 files changed, 64 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4988ba2..128cfac 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9724,6 +9724,57 @@ cmdDomname(vshControl *ctl, const vshCmd *cmd)
}
/*
+ * domrename command
+ */
+static const vshCmdInfo info_domrename[] = {
+{.name = help,
+ .data = N_(rename a domain)
+},
+{.name = desc,
+ .data = Rename an inactive domain.
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_domrename[] = {
+{.name = domain,
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_(domain name, id or uuid)
+},
+{.name = new-name,
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_(new domain name)
+},
+{.name = NULL}
+};
+
+static bool
+cmdDomrename(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom;
+const char *new_name = NULL;
+bool ret = false;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+return ret;
+
+if (vshCommandOptStringReq(ctl, cmd, new-name, new_name) 0)
+goto cleanup;
+
+if (virDomainRename(dom, new_name, 0) 0)
+goto cleanup;
+
+vshPrint(ctl, Domain successfully renamed\n);
+ret = true;
+
+ cleanup:
+virDomainFree(dom);
+return false;
+}
+
+/*
* domid command
*/
static const vshCmdInfo info_domid[] = {
@@ -13080,6 +13131,12 @@ const vshCmdDef domManagementCmds[] = {
.info = info_domname,
.flags = 0
},
+{.name = domrename,
+ .handler = cmdDomrename,
+ .opts = opts_domrename,
+ .info = info_domrename,
+ .flags = 0
+},
{.name = dompmsuspend,
.handler = cmdDomPMSuspend,
.opts = opts_dom_pm_suspend,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 5ee9a96..c049f8f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1258,6 +1258,13 @@ on both of them).
Convert a domain Id (or UUID) to domain name
+=item Bdomrename Idomain Inew-name
+
+Rename a domain. This command changes current domain name to the new name
+specified in the second argument.
+
+BNote: Domain must be inactive and without snapshots.
+
=item Bdomstate Idomain [I--reason]
Returns state about a domain. I--reason tells virsh to also print
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 3/5] domain_conf: Introducde virDomainObjListRenameAddNew() virDomainObjListRenameRemove()
We just need to update the entry in the second hash table. Since commit 8728a56
we have two hash tables for the domain list so that we can do O(1) lookup
regardless of looking up by UUID or name. Since with renaming a domain UUID does
not change, we only need to update the second hash table, where domains are
referenced by their name.
We will call both functions from the qemuDomainRename().
Signed-off-by: Tomas Meszaros e...@tty.sk
---
src/conf/domain_conf.c | 35 +++
src/conf/domain_conf.h | 5 +
src/libvirt_private.syms | 2 ++
3 files changed, 42 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e4114f8..4edf37d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2848,6 +2848,41 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr
doms,
return ret;
}
+
+int
+virDomainObjListRenameAddNew(virDomainObjListPtr doms,
+ virDomainObjPtr vm,
+ const char *name)
+{
+int ret = -1;
+virObjectLock(doms);
+
+/* Add new name into the hash table of domain names. */
+if (virHashAddEntry(doms-objsName, name, vm) 0)
+goto cleanup;
+
+/* Okay, this is crazy. virHashAddEntry() does not increment
+ * the refcounter of @vm, but virHashRemoveEntry() does
+ * decrement it. We need to work around it. */
+virObjectRef(vm);
+
+ret = 0;
+ cleanup:
+virObjectUnlock(doms);
+return ret;
+}
+
+
+int
+virDomainObjListRenameRemove(virDomainObjListPtr doms, const char *name)
+{
+virObjectLock(doms);
+virHashRemoveEntry(doms-objsName, name);
+virObjectUnlock(doms);
+return 0;
+}
+
+
/*
* Mark the running VM config as transient. Ensures transient hotplug
* operations do not persist past shutdown.
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 698a4d2..9e6f7b8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2545,6 +2545,11 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr
doms,
virDomainXMLOptionPtr xmlopt,
unsigned int flags,
virDomainDefPtr *oldDef);
+int virDomainObjListRenameAddNew(virDomainObjListPtr doms,
+ virDomainObjPtr vm,
+ const char *name);
+int virDomainObjListRenameRemove(virDomainObjListPtr doms,
+ const char *name);
void virDomainObjAssignDef(virDomainObjPtr domain,
virDomainDefPtr def,
bool live,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e5d8437..3a206d3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -408,6 +408,8 @@ virDomainObjListNew;
virDomainObjListNumOfDomains;
virDomainObjListRemove;
virDomainObjListRemoveLocked;
+virDomainObjListRenameAddNew;
+virDomainObjListRenameRemove;
virDomainObjNew;
virDomainObjParseNode;
virDomainObjSetDefTransient;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/9] bridge_driver: Introduce networkBandwidthUpdate
On 08/03/2015 02:39 AM, Michal Privoznik wrote:
So, if a domain vNIC's bandwidth has been successfully set, it's
possible that because @floor is set on network's bridge, this
part may need updating too. And that's exactly what this function
does. While the previous commit introduced a function to check if
@floor can be satisfied, this does all the hard work. In general,
there may be three, well four possibilities:
1) No change in @floor value (either it remain unset, or its
value hasn't changed)
2) The @floor value has changed from a non-zero to a non-zero
value
3) New @floor is to be set
4) Old @floor must be cleared out
The difference between 2), 3) and 4) is, that while in 2) the QoS
tree on the network's bridge already has a special class for the
vNIC, in 3) the class must be created from scratch. In 4) it must
be removed. Fortunately, we have helpers for all three
interesting cases.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/network/bridge_driver.c | 160
src/network/bridge_driver.h | 10 +++
2 files changed, 142 insertions(+), 28 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index fa60ba4..5fad015 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4792,38 +4792,17 @@ networkNextClassID(virNetworkObjPtr net)
return ret;
}
+
static int
-networkPlugBandwidth(virNetworkObjPtr net,
- virDomainNetDefPtr iface)
+networkPlugBandwidthImpl(virNetworkObjPtr net,
+ virDomainNetDefPtr iface,
+ virNetDevBandwidthPtr ifaceBand,
+ unsigned long long new_rate)
{
virNetworkDriverStatePtr driver = networkGetDriver();
-int ret = -1;
-int plug_ret;
-unsigned long long new_rate = 0;
ssize_t class_id = 0;
-char ifmac[VIR_MAC_STRING_BUFLEN];
-virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);
-
-if ((plug_ret = networkCheckBandwidth(net, ifaceBand, NULL,
- iface-mac, new_rate)) 0) {
-/* helper reported error */
-goto cleanup;
-}
-
-if (plug_ret 0) {
-/* no QoS needs to be set; claim success */
-ret = 0;
-goto cleanup;
-}
-
-virMacAddrFormat(iface-mac, ifmac);
-if (iface-type != VIR_DOMAIN_NET_TYPE_NETWORK ||
-!iface-data.network.actual) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Cannot set bandwidth on interface '%s' of type
%d),
- ifmac, iface-type);
-goto cleanup;
-}
+int plug_ret;
+int ret = -1;
/* generate new class_id */
if ((class_id = networkNextClassID(net)) 0) {
@@ -4859,6 +4838,46 @@ networkPlugBandwidth(virNetworkObjPtr net,
net-def-bridge);
ret = 0;
+ cleanup:
+return ret;
+}
+
+
+static int
+networkPlugBandwidth(virNetworkObjPtr net,
+ virDomainNetDefPtr iface)
+{
+int ret = -1;
+int plug_ret;
+unsigned long long new_rate = 0;
+char ifmac[VIR_MAC_STRING_BUFLEN];
+virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);
+
+if ((plug_ret = networkCheckBandwidth(net, ifaceBand, NULL,
+ iface-mac, new_rate)) 0) {
+/* helper reported error */
+goto cleanup;
+}
+
+if (plug_ret 0) {
+/* no QoS needs to be set; claim success */
+ret = 0;
+goto cleanup;
+}
+
+virMacAddrFormat(iface-mac, ifmac);
+if (iface-type != VIR_DOMAIN_NET_TYPE_NETWORK ||
+!iface-data.network.actual) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Cannot set bandwidth on interface '%s' of type
%d),
+ ifmac, iface-type);
+goto cleanup;
+}
+
+if (networkPlugBandwidthImpl(net, iface, ifaceBand, new_rate) 0)
+goto cleanup;
+
+ret = 0;
cleanup:
return ret;
@@ -4939,6 +4958,9 @@ networkBandwidthGenericChecks(virDomainNetDefPtr iface,
virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);
^^
'iface' is used here... but !iface checked below...
unsigned long long old_floor, new_floor;
+if (!iface !newBandwidth)
+return false;
+
Adding this !iface check causes Coverity issues for the following usage
of 'iface' (especially if 'newBandwidth' is non NULL)...
Checking the two (eventual) callers - networkBandwidthChangeAllowed
already has ATTRIBUTE_NONNULL(1) (2) checks and it seems logically at
least that networkBandwidthUpdate should have it.
Thus I don't think it's necessary (although I may have read things wrong
too ;-))
if (virDomainNetGetActualType(iface) !=
Re: [libvirt] [PATCH 5/9] qemuDomainSetInterfaceParameters: Use new functions to update bandwidth
On 08/03/2015 02:39 AM, Michal Privoznik wrote:
As sketched in previous commits, imagine the following scenario:
virsh # domiftune gentoo vnet0
inbound.average: 100
inbound.peak : 0
inbound.burst : 0
outbound.average: 100
outbound.peak : 0
outbound.burst : 0
virsh # domiftune gentoo vnet0 --inbound 0
virsh # shutdown gentoo
Domain gentoo is being shutdown
virsh # list --all
error: Failed to list domains
error: Cannot recv data: Connection reset by peer
Program received signal SIGSEGV, Segmentation fault.
0x7fffe80ea221 in networkUnplugBandwidth (net=0x7fff9400c1a0,
iface=0x7fff940ea3e0) at network/bridge_driver.c:4881
4881net-floor_sum -= ifaceBand-in-floor;
This is rather unfortunate. We should not SIGSEGV here. The
problem is, that while in the second step the inbound QoS was
cleared out, the network part of it was not updated (moreover, we
don't report that vnet0 had inbound.floor set). Internal
structure therefore still had some fragments left (e.g.
class_id). So when qemuProcessStop() started to clean up the
environment it got to networkUnplugBandwidth(). Here, class_id is
set therefore function assumes that there is an inbound QoS. This
actually is a fair assumption to make, there's no need for a
special QoS box in network's QoS when there's no QoS to set.
Anyway, the problem is not the networkUnplugBandwidth() rather
than qemuDomainSetInterfaceParameters() which completely forgot
about QoS being disperse (some parts are set directly on
interface itself, some on bridge the interface is plugged into).
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_driver.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b9278f8..171b58f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -100,6 +100,7 @@
#include vircgroup.h
#include virnuma.h
#include dirname.h
+#include network/bridge_driver.h
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -11221,7 +11222,11 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
sizeof(*newBandwidth-out));
}
-if (virNetDevBandwidthSet(net-ifname, newBandwidth, false) 0) {
+if (!networkBandwidthChangeAllowed(net, newBandwidth))
+goto endjob;
+
+if (virNetDevBandwidthSet(net-ifname, newBandwidth, false) 0 ||
+networkBandwidthUpdate(net, newBandwidth) 0) {
Is this the only caller of virNetDevBandwidthSet that needs the
ChangeAllowed check first? I think the answer is no based on your
explanation in the commit message, but figured I'd ask... Tough to keep
track of the multiple callers and what they're checking before calling
the Set and/or if they need to Update as well (it's been a long day ;-),
but I figured I'd at least look...
ACK for the change in any case.
John
ignore_value(virNetDevBandwidthSet(net-ifname,
net-bandwidth,
false));
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/9] Introduce VIR_DOMAIN_BANDWIDTH_IN_FLOOR
On 08/03/2015 02:39 AM, Michal Privoznik wrote: This macro represents the single missing field we don't expose yet within QoS: inbound.floor. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- include/libvirt/libvirt-domain.h | 7 +++ 1 file changed, 7 insertions(+) ACK, John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/9] virNetDevBandwidthUpdateRate: turn class_id into integer
On 08/03/2015 02:39 AM, Michal Privoznik wrote:
This is no functional change. It's just that later in the series we
will need to pass class_id as an integer.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/network/bridge_driver.c | 4 ++--
src/util/virnetdevbandwidth.c | 10 +++---
src/util/virnetdevbandwidth.h | 2 +-
3 files changed, 10 insertions(+), 6 deletions(-)
something that dawned on me reading later patches...
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 6ae0877..91201ae 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -638,7 +638,8 @@ virNetDevBandwidthUnplug(const char *brname,
/**
* virNetDevBandwidthUpdateRate:
* @ifname: interface name
- * @classid: ID of class to update
+ * @id: unique identifier (MUST be greater than 2)
^^^ Comment says MUST be 2, but there's no check/error... Also I note
2 is passed often...
John
+ * @bandwidth: used to derive 'ceil' of class with @id
* @new_rate: new rate
*
* This function updates the 'rate' attribute of HTB class.
@@ -650,16 +651,18 @@ virNetDevBandwidthUnplug(const char *brname,
*/
int
virNetDevBandwidthUpdateRate(const char *ifname,
- const char *class_id,
+ unsigned int id,
virNetDevBandwidthPtr bandwidth,
unsigned long long new_rate)
{
int ret = -1;
virCommandPtr cmd = NULL;
+char *class_id = NULL;
char *rate = NULL;
char *ceil = NULL;
-if (virAsprintf(rate, %llukbps, new_rate) 0 ||
+if (virAsprintf(class_id, 1:%x, id) 0 ||
+virAsprintf(rate, %llukbps, new_rate) 0 ||
virAsprintf(ceil, %llukbps, bandwidth-in-peak ?
bandwidth-in-peak :
bandwidth-in-average) 0)
@@ -677,6 +680,7 @@ virNetDevBandwidthUpdateRate(const char *ifname,
cleanup:
virCommandFree(cmd);
+VIR_FREE(class_id);
VIR_FREE(rate);
VIR_FREE(ceil);
return ret;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/9] virsh: Rework parseRateStr
On 08/03/2015 02:39 AM, Michal Privoznik wrote:
The function is used to parse a tuple delimited by commas into
virNetDevBandwidth structure. So far only three out of fore
fields are supported: average, peak and burst. The single missing
field is floor. Well, the parsing works, but I think we can do
better. Especially when we will need to parse floor too in very
close future.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tools/virsh-domain.c | 80
++--
1 file changed, 47 insertions(+), 33 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a61656f..bb40ddd 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -865,36 +865,58 @@ static const vshCmdOptDef opts_attach_interface[] = {
/* parse inbound and outbound which are in the format of
* 'average,peak,burst', in which peak and burst are optional,
* thus 'average,,burst' and 'average,peak' are also legal. */
-static int parseRateStr(const char *rateStr, virNetDevBandwidthRatePtr rate)
+static int parseRateStr(vshControl *ctl,
+const char *rateStr,
+virNetDevBandwidthRatePtr rate)
{
-const char *average = NULL;
-char *peak = NULL, *burst = NULL;
+char *token;
+char *next;
+char *saveptr;
My compiler complained about uninitialized value here especially when
used with strtok_r (setting = NULL pacifies compiler).
ACK with the adjustment.
John
+enum {
+AVERAGE, PEAK, BURST
+} state;
+int ret = -1;
-average = rateStr;
-if (!average)
-return -1;
-if (virStrToLong_ull(average, peak, 10, rate-average) 0)
+if (!rateStr)
return -1;
-/* peak will be updated to point to the end of rateStr in case
- * of 'average' */
-if (peak *peak != '\0') {
-burst = strchr(peak + 1, ',');
-if (!(burst (burst - peak == 1))) {
-if (virStrToLong_ull(peak + 1, burst, 10, rate-peak) 0)
-return -1;
+next = vshStrdup(ctl, rateStr);
+
+for (state = AVERAGE; state = BURST; state++) {
+unsigned long long *tmp;
+const char *field_name;
+
+if (!(token = strtok_r(next, ,, saveptr)))
+break;
+next = NULL;
+
+switch (state) {
+case AVERAGE:
+tmp = rate-average;
+field_name = average;
+break;
+
+case PEAK:
+tmp = rate-peak;
+field_name = peak;
+break;
+
+case BURST:
+tmp = rate-burst;
+field_name = burst;
+break;
}
-/* burst will be updated to point to the end of rateStr in case
- * of 'average,peak' */
-if (burst *burst != '\0') {
-if (virStrToLong_ull(burst + 1, NULL, 10, rate-burst) 0)
-return -1;
+if (virStrToLong_ullp(token, NULL, 10, tmp) 0) {
+vshError(ctl, _(malformed %s field), field_name);
+goto cleanup;
}
}
-
-return 0;
+ret = 0;
+ cleanup:
+VIR_FREE(next);
+return ret;
}
static bool
@@ -952,10 +974,8 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
if (inboundStr) {
memset(inbound, 0, sizeof(inbound));
-if (parseRateStr(inboundStr, inbound) 0) {
-vshError(ctl, _(inbound format is incorrect));
+if (parseRateStr(ctl, inboundStr, inbound) 0)
goto cleanup;
-}
if (inbound.average == 0) {
vshError(ctl, _(inbound average is mandatory));
goto cleanup;
@@ -963,10 +983,8 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
}
if (outboundStr) {
memset(outbound, 0, sizeof(outbound));
-if (parseRateStr(outboundStr, outbound) 0) {
-vshError(ctl, _(outbound format is incorrect));
+if (parseRateStr(ctl, outboundStr, outbound) 0)
goto cleanup;
-}
if (outbound.average == 0) {
vshError(ctl, _(outbound average is mandatory));
goto cleanup;
@@ -3280,10 +3298,8 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd)
memset(outbound, 0, sizeof(outbound));
if (inboundStr) {
-if (parseRateStr(inboundStr, inbound) 0) {
-vshError(ctl, _(inbound format is incorrect));
+if (parseRateStr(ctl, inboundStr, inbound) 0)
goto cleanup;
-}
/* we parse the rate as unsigned long long, but the API
* only accepts UINT */
if (inbound.average UINT_MAX || inbound.peak UINT_MAX ||
@@ -3316,10 +3332,8 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd)
}
if (outboundStr) {
-if (parseRateStr(outboundStr, outbound) 0) {
-vshError(ctl, _(outbound format
Re: [libvirt] [PATCH 8/9] virsh: Implement VIR_DOMAIN_BANDWIDTH_IN_FLOOR
On 08/03/2015 02:39 AM, Michal Privoznik wrote:
We have a function parseRateStr() that parses --inbound and
--outbound arguments to both attach-interface and domiftune.
Now that we have all virTypedParams macros needed for QoS,
lets parse even floor attribute. The extended format for the
arguments looks like this then:
--inbound average[,peak[,burst[,floor]]]
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tools/virsh-domain.c | 23 ++-
tools/virsh.pod | 12 ++--
2 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index bb40ddd..5b7e623 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
Some comments above here need adjustment to describe the new field and
possible options...
Does it matter if someone provides floor on outbound (it's a testing
question ;-))
@@ -873,7 +873,7 @@ static int parseRateStr(vshControl *ctl,
char *next;
char *saveptr;
enum {
-AVERAGE, PEAK, BURST
+AVERAGE, PEAK, BURST, FLOOR
} state;
int ret = -1;
@@ -882,7 +882,7 @@ static int parseRateStr(vshControl *ctl,
next = vshStrdup(ctl, rateStr);
-for (state = AVERAGE; state = BURST; state++) {
+for (state = AVERAGE; state = FLOOR; state++) {
unsigned long long *tmp;
const char *field_name;
@@ -905,6 +905,11 @@ static int parseRateStr(vshControl *ctl,
tmp = rate-burst;
field_name = burst;
break;
+
+case FLOOR:
+tmp = rate-floor;
+field_name = floor;
+break;
}
if (virStrToLong_ullp(token, NULL, 10, tmp) 0) {
@@ -976,7 +981,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
memset(inbound, 0, sizeof(inbound));
if (parseRateStr(ctl, inboundStr, inbound) 0)
goto cleanup;
-if (inbound.average == 0) {
+if (!inbound.average !inbound.floor) {
vshError(ctl, _(inbound average is mandatory));
Why checking !inbound.floor?
What if someone does ,,,floor value
goto cleanup;
}
Also should we check below here for outboundStr having floor? (improperly)
@@ -3308,8 +3313,10 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd)
UINT_MAX);
goto cleanup;
}
-if (inbound.average == 0 (inbound.burst || inbound.peak)) {
-vshError(ctl, _(inbound average is mandatory));
+
+if ((!inbound.average (inbound.burst || inbound.peak))
+!inbound.floor) {
+vshError(ctl, _(either inbound average or floor is mandatory));
goto cleanup;
}
@@ -3329,6 +3336,12 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd)
VIR_DOMAIN_BANDWIDTH_IN_BURST,
inbound.burst) 0)
goto save_error;
+
+if (inbound.floor
+virTypedParamsAddUInt(params, nparams, maxparams,
+ VIR_DOMAIN_BANDWIDTH_IN_FLOOR,
+ inbound.floor) 0)
+goto save_error;
}
if (outboundStr) {
...
should we check here if someone provides outbound.floor value and fail?
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 5ee9a96..a6148d3 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -770,7 +770,7 @@ Iinterface-device can be the interface's target name or
the MAC address.
=item Bdomiftune Idomain Iinterface-device
[[I--config] [I--live] | [I--current]]
-[I--inbound average,peak,burst]
+[I--inbound average,peak,burst,floor]
[I--outbound average,peak,burst]
Set or query the domain's network interface's bandwidth parameters.
@@ -779,9 +779,9 @@ or the MAC address.
If no I--inbound or I--outbound is specified, this command will
query and show the bandwidth settings. Otherwise, it will set the
-inbound or outbound bandwidth. Iaverage,peak,burst is the same as
-in command Iattach-interface. Values for Iaverage and Ipeak are
-expressed in kilobytes per second, while Iburst is expressed in kilobytes
+inbound or outbound bandwidth. Iaverage,peak,burst,floor is the same as
+in command Iattach-interface. Values for Iaverage, Ipeak and Ifloor
+are expressed in kilobytes per second, while Iburst is expressed in
kilobytes
in a single burst at -Ipeak speed as described in the Network XML
documentation at Lhttp://libvirt.org/formatnetwork.html#elementQoS.
@@ -2499,7 +2499,7 @@ Likewise, I--shareable is an alias for I--mode
shareable.
=item Battach-interface Idomain Itype Isource
[[[I--live] [I--config] | [I--current]] | [I--persistent]]
[I--target target] [I--mac mac] [I--script script] [I--model model]
-[I--inbound average,peak,burst] [I--outbound average,peak,burst]
+[I--inbound
Re: [libvirt] [PATCH 9/9] qemu: Implement VIR_DOMAIN_BANDWIDTH_IN_FLOOR
On 08/03/2015 02:39 AM, Michal Privoznik wrote:
Well, there are just two places that needs adjustment:
qemuDomainGetInterfaceParameters - to report the @floor
qemuDomainSetInterfaceParameters - now that the function has been
fixed, we can allow updating @floor too.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_driver.c | 23 ++-
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 171b58f..7123083 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -138,7 +138,7 @@ VIR_LOG_INIT(qemu.qemu_driver);
#define QEMU_NB_BLKIO_PARAM 6
-#define QEMU_NB_BANDWIDTH_PARAM 6
+#define QEMU_NB_BANDWIDTH_PARAM 7
static void processWatchdogEvent(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -11126,6 +11126,8 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
VIR_TYPED_PARAM_UINT,
VIR_DOMAIN_BANDWIDTH_IN_BURST,
VIR_TYPED_PARAM_UINT,
+ VIR_DOMAIN_BANDWIDTH_IN_FLOOR,
+ VIR_TYPED_PARAM_UINT,
VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE,
VIR_TYPED_PARAM_UINT,
VIR_DOMAIN_BANDWIDTH_OUT_PEAK,
@@ -11178,6 +11180,9 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
bandwidth-in-peak = params[i].value.ui;
} else if (STREQ(param-field, VIR_DOMAIN_BANDWIDTH_IN_BURST)) {
bandwidth-in-burst = params[i].value.ui;
+} else if (STREQ(param-field, VIR_DOMAIN_BANDWIDTH_IN_FLOOR)) {
+bandwidth-in-floor = params[i].value.ui;
+inboundSpecified = true;
If 'average' is required, then can one really provide just floor?
} else if (STREQ(param-field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE)) {
bandwidth-out-average = params[i].value.ui;
outboundSpecified = true;
@@ -11191,7 +11196,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
/* average is mandatory, peak and burst are optional. So if no
* average is given, we free inbound/outbound here which causes
* inbound/outbound to not be set. */
Adjust comment to account for floor...
-if (!bandwidth-in-average)
+if (!bandwidth-in-average !bandwidth-in-floor)
Again, I thought average was required ... sorry it's just late. Maybe
the updated comment will answer my query...
ACK in principle
John
VIR_FREE(bandwidth-in);
if (!bandwidth-out-average)
VIR_FREE(bandwidth-out);
@@ -11355,7 +11360,15 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom,
if (net-bandwidth net-bandwidth-in)
params[i].value.ui = net-bandwidth-in-burst;
break;
-case 3: /* outbound.average */
+case 3: /* inbound.floor */
+if (virTypedParameterAssign(params[i],
+VIR_DOMAIN_BANDWIDTH_IN_FLOOR,
+VIR_TYPED_PARAM_UINT, 0) 0)
+goto cleanup;
+if (net-bandwidth net-bandwidth-in)
+params[i].value.ui = net-bandwidth-in-floor;
+break;
+case 4: /* outbound.average */
if (virTypedParameterAssign(params[i],
VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE,
VIR_TYPED_PARAM_UINT, 0) 0)
@@ -11363,7 +11376,7 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom,
if (net-bandwidth net-bandwidth-out)
params[i].value.ui = net-bandwidth-out-average;
break;
-case 4: /* outbound.peak */
+case 5: /* outbound.peak */
if (virTypedParameterAssign(params[i],
VIR_DOMAIN_BANDWIDTH_OUT_PEAK,
VIR_TYPED_PARAM_UINT, 0) 0)
@@ -11371,7 +11384,7 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom,
if (net-bandwidth net-bandwidth-out)
params[i].value.ui = net-bandwidth-out-peak;
break;
-case 5: /* outbound.burst */
+case 6: /* outbound.burst */
if (virTypedParameterAssign(params[i],
VIR_DOMAIN_BANDWIDTH_OUT_BURST,
VIR_TYPED_PARAM_UINT, 0) 0)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/9] virNetDevParseMcast: Avoid magic constant
On 08/03/2015 02:39 AM, Michal Privoznik wrote: There is no guarantee that an enum start it mapped onto a value of zero. However, we are guaranteed that enum items are consecutive integers. Moreover, it's a pity to define an enum to avoid using magical constants but then using them anyway. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/virnetdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [frankschreu...@outlook.com: Fwd: Reproducible live disk detach bug]
On Fri, Aug 07, 2015 at 10:18:59AM +0200, Frank Schreuder wrote:
Hi Laine,
I'm sorry for starting 3 threads with the same topic, seems like my mail was
heavily delayed yesterday.
[ removing personal mail ]
Op 8/7/2015 om 12:24 AM schreef Laine Stump:
Subject: Reproducible live disk detach bug
Hey guys,
I am currently running into a reproducible libvirt bug when I detach a
disk from a running VM using libvirt 1.2.18 and qemu 2.3.0.
I can attach a disk to a running VM without any problems.
During the attach operation I don't create a disk alias name, which
results in libvirt generating an unique alias.
Using virsh dumpxml I get the following output(snippet):
disk type='file' device='disk'
driver name='qemu' type='raw' cache='none'/
source file='/var/local/mnt/test-storage/test-storage.raw'/
backingStore/
target dev='vde' bus='virtio'/
iotune
read_bytes_sec94371840/read_bytes_sec
write_bytes_sec31457280/write_bytes_sec
read_iops_sec1000/read_iops_sec
write_iops_sec200/write_iops_sec
/iotune
alias name='virtio-disk4'/
address type='pci' domain='0x' bus='0x00' slot='0x09'
function='0x0'/
/disk
As you can see, libvirt generates 'virtio-disk4' as an alias. But as
soon as I try to detach the disk from the running VM, I will receive the
following two errors in my libvirt log:
2015-08-06 10:48:59.943+: 11657: warning :
qemuMonitorJSONHandleDeviceDeleted:939 : missing device in device
deleted event
2015-08-06 10:48:59.945+: 11662: error :
qemuMonitorTextDriveDel:2594 : operation failed: deleting
drive-virtio-disk4 drive failed: 2015-08-06T10:48:59.945058Z Device
'drive-virtio-disk4' not found
Apparently libvirt tries to detach the disk with a drive- prefix, which
doesn't exist?
I can't tell you right off why the failure message is there, but the
alias with the added drive- is easily explained.
Each disk element in the libvirt config corresponds to two entities in
the qemu virtual machine - a device component (what is seen by the
guest) and a drive component (what resources are used on the host to
back up that device). Each of these corresponds to a separate qemu
commandline argument (named -device and -drive), and each of those
must have a different, unique id option. For example, this is what is
added to my qemu commandline when I add a virtio disk named
rawhide.qcow2 to my virtual machine:
-drive
file=/var/lib/libvirt/images/rawhide.qcow2,if=none,id=drive-virtio-disk0,format=qcow2
-device
virtio-blk-pci,scsi=off,bus=pci.2,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
If you look at the status XML of the domain while it is running, you'll
see that the alias for the disk element is virtio-disk0, and that's
what is given to the -device half of the disk as its qemu id. The -drive
half uses drive-$alias as its qemu id though (you'll notice that the
-device half references that id with its drive option - that's how the
two halves are tied together by qemu).
So, there is nothing unusual about qemu wanting to delete something
called drive-virtio-disk4 when you have a disk with
alias='virtio-disk4'. It obviously isn't right that an error message is
being logged. Something sounds familiar about the missing device in
device deleted event message, but I can't recall details. Does anyone
else have a better memory? Frank - would it be possible for you to try
this same sequence of events with the same libvirt and an older qemu
(and/or vice-versa)?
I have some more interesting debug information.
I'm -not- able to reproduce it on libvirt 1.2.11 - qemu 2.3.0 and libvirt
1.2.11 - qemu 1.7.1 using a FreeBSD VM.
I'm able to reproduce the errors with libvirt 1.2.11 - qemu 2.3.0 and
libvirt 1.2.18 - qemu 2.3.0 using a Debian VM.
Seems like the error triggers depending on the guest OS?
During hot-unplugging of network interfaces libvirt also triggers the same
error on a Debian VM:
2015-08-06 10:48:59.945+: 11662: error :
qemuMonitorJSONHandleDeviceDeleted:939 : missing device in device deleted
event
I gathered some logging information with info level logging enabled while
detaching a disk:
http://pastebin.com/raw.php?i=vVLuavG2
The interesting this is this line:
2015-08-06 21:14:56.847+: 8037: info : qemuMonitorIOProcess:452 :
QEMU_MONITOR_IO_PROCESS: mon=0x7f9477cc0fc0 buf={return:
2015-08-06T21:14:56.847359Z Device 'drive-virtio-disk4' not found\r\n, id:
libvirt-12}
Notice how the data read from the monitor command has a timestamp
prefixed on it. This is very bad. We absolutely do not expect to
see a timestamp on HMP monitor replies, so whatever is doing that
in QEMU needs fixing.
AFAIK, QEMU is only supposed to add timestamps to stuff it prints
on stderr, when -msg timestamp=on. It should never timestamp HMP
Regards,
Daniel
--
|: http://berrange.com -o-
Re: [libvirt] [PATCH v2] virt-shell: v2 diff patch to original series
On Fri, Jul 17, 2015 at 02:34:20PM +0200, Erik Skultety wrote:
V1 introduced quite a big refactor, so to be able to better keep track
of changes performed from v1 to v2, only this v2 diff patch is presented
and eventually will be squashed to the previous series with a different
commit message.
---
You can still checkout the forked repo
https://github.com/eskultety/libvirt/tree/virt-shell
cfg.mk | 2 +-
tools/virsh-domain-monitor.c | 3 +-
tools/virsh-domain.c | 668 ++-
tools/virsh-domain.h | 3 +-
tools/virsh-edit.c | 8 +-
tools/virsh-interface.c | 42 +--
tools/virsh-interface.h | 2 +-
tools/virsh-network.c| 76 ++---
tools/virsh-network.h| 2 +-
tools/virsh-nodedev.c| 38 +--
tools/virsh-nwfilter.c | 34 +--
tools/virsh-nwfilter.h | 2 +-
tools/virsh-pool.c | 66 ++---
tools/virsh-pool.h | 2 +-
tools/virsh-secret.c | 28 +-
tools/virsh-snapshot.c | 94 +++---
tools/virsh-volume.c | 52 ++--
tools/virsh-volume.h | 2 +-
tools/virsh.c| 339 +-
tools/virsh.h| 25 +-
tools/vsh.c | 559 ++--
tools/vsh.h | 32 ++-
22 files changed, 1062 insertions(+), 1017 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index f26191f..e436434 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1086,7 +1086,7 @@ $(srcdir)/src/admin/admin_client.h:
$(srcdir)/src/admin/admin_protocol.x
$(MAKE) -C src admin/admin_client.h
# List all syntax-check exemptions:
-exclude_file_name_regexp--sc_avoid_strcase = ^tools/(virsh|vsh)\.h$$
+exclude_file_name_regexp--sc_avoid_strcase = ^tools/vsh\.h$$
_src1=libvirt-stream|fdstream|qemu/qemu_monitor|util/(vircommand|virfile)|xen/xend_internal|rpc/virnetsocket|lxc/lxc_controller|locking/lock_daemon
_test1=shunloadtest|virnettlscontexttest|virnettlssessiontest|vircgroupmock
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 0762d6e..340a8e2 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -2165,7 +2165,8 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd)
while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
if (!(dom = virshLookupDomainBy(ctl, opt-data,
-VSH_BYID | VSH_BYUUID |
VSH_BYNAME)))
+VIRSH_BYID |
+VIRSH_BYUUID | VIRSH_BYNAME)))
goto cleanup;
if (VIR_INSERT_ELEMENT(domlist, ndoms - 1, ndoms, dom) 0)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index df423c7..ff026fb 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -70,11 +70,11 @@ virshLookupDomainInternal(vshControl *ctl,
{
virDomainPtr dom = NULL;
int id;
-virCheckFlags(VSH_BYID | VSH_BYUUID | VSH_BYNAME, NULL);
+virCheckFlags(VIRSH_BYID | VIRSH_BYUUID | VIRSH_BYNAME, NULL);
virshControlPtr priv = ctl-privData;
/* try it by ID */
-if (flags VSH_BYID) {
+if (flags VIRSH_BYID) {
if (virStrToLong_i(name, NULL, 10, id) == 0 id = 0) {
vshDebug(ctl, VSH_ERR_DEBUG, %s: domain looks like ID\n,
cmdname);
@@ -83,7 +83,7 @@ virshLookupDomainInternal(vshControl *ctl,
}
/* try it by UUID */
-if (!dom (flags VSH_BYUUID)
+if (!dom (flags VIRSH_BYUUID)
strlen(name) == VIR_UUID_STRING_BUFLEN-1) {
vshDebug(ctl, VSH_ERR_DEBUG, %s: domain trying as domain UUID\n,
cmdname);
@@ -91,7 +91,7 @@ virshLookupDomainInternal(vshControl *ctl,
}
/* try it by NAME */
-if (!dom (flags VSH_BYNAME)) {
+if (!dom (flags VIRSH_BYNAME)) {
vshDebug(ctl, VSH_ERR_DEBUG, %s: domain trying as domain NAME\n,
cmdname);
dom = virDomainLookupByName(priv-conn, name);
@@ -106,8 +106,8 @@ virshLookupDomainInternal(vshControl *ctl,
virDomainPtr
virshLookupDomainBy(vshControl *ctl,
- const char *name,
- unsigned int flags)
+const char *name,
+unsigned int flags)
{
return virshLookupDomainInternal(ctl, unknown, name, flags);
}
@@ -265,7 +265,7 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
virDomainIsActive(dom) == 1)
flags |= VIR_DOMAIN_AFFECT_LIVE;
-if (virFileReadAll(from, VIRSH_MAX_XML_FILE, buffer) 0) {
+if (virFileReadAll(from, VSH_MAX_XML_FILE, buffer) 0) {
vshReportError(ctl);
goto cleanup;
}
@@ -902,8 +902,8 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
const char *mac = NULL, *target = NULL, *script = NULL,
-*type = NULL, *source = NULL, *model = NULL,
-*inboundStr = NULL, *outboundStr = NULL;
+ *type = NULL,
Re: [libvirt] [PATCH] virt drivers: show used memory as 0 when domain is inactive
On Fri, Aug 07, 2015 at 02:08:47PM -0600, Jim Fehlig wrote: Commit 1ce7c1d2 changed domainGetInfo to set virDomainInfoPtr-memory to 0 when domain is inactive, but only did this for the LXC and QEMU drivers. Make a similar change to the other virt drivers so they all behave the same. I'm not really convinced that this is right in general. I think that when a guest is shutoff, we should just report the balloon level as the same as the max memory level. Changing it to report 0 is quite a significant semantic change in our APIs too. IOW, I think we should revert that change you quote. 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 v3 01/20] cpu: Mark driver functions in ppc64 driver
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 v3 00/20] cpu: Fix and improve the ppc64 driver
Patches 01-07 are cleanups, 08-10 are bug fixes and 11-20
improve the driver.
Changes in v3:
* Fix a bug spotted by John thanks to Coverity
Changes in v2:
* Implement compatibility with guests defined on older
libvirt versions
* Always use fallback='forbid' when emitting CPU definitions
* Update all tests in one go instead of changing them
several times
* Handle test failure and system failure differently in
cpuTestGuestData()
* Simplify ppc64DriverNodeData()
* Simplify XML parsing code
* Remove unnecessary NULL checks
* Add even more test cases
* Other small changes such as shuffling chunks between
commits to make the history more readable
Cheers.
Andrea Bolognani (20):
cpu: Mark driver functions in ppc64 driver
cpu: Simplify NULL handling in ppc64 driver
cpu: Simplify ppc64ModelFromCPU()
cpu: Reorder functions in the ppc64 driver
cpu: Remove ISA information from CPU map XML
tests: Remove unused file
tests: Improve result handling in cpuTestGuestData()
cpu: Never skip CPU model name check in ppc64 driver
cpu: CPU model names have to match on ppc64
cpu: Use ppc64Compute() to implement ppc64DriverCompare()
tests: Temporarily disable ppc64 cpu tests
cpu: Align ppc64 CPU data with x86
cpu: Support multiple PVRs in the ppc64 driver
cpu: Simplify ppc64 part of CPU map XML
cpu: Parse and use PVR masks in the ppc64 driver
cpu: Add POWER8NVL information to CPU map XML
cpu: Implement backwards compatibility in the ppc64 driver
cpu: Forbid model fallback in the ppc64 driver
tests: Re-enable ppc64 cpu tests
tests: Add a bunch of cpu test case for ppc64
src/cpu/cpu.h | 2 +-
src/cpu/cpu_map.xml| 59 +--
src/cpu/cpu_ppc64.c| 451 +
src/cpu/cpu_ppc64_data.h | 12 +-
tests/cputest.c| 55 ++-
tests/cputestdata/ppc64-baseline-1-result.xml | 3 -
.../ppc64-baseline-incompatible-models.xml | 14 +
.../ppc64-baseline-incompatible-vendors.xml| 4 +-
tests/cputestdata/ppc64-baseline-legacy.xml| 14 +
.../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 | 3 -
tests/cputestdata/ppc64-guest-exact.xml| 3 +
.../ppc64-guest-legacy-incompatible.xml| 3 +
tests/cputestdata/ppc64-guest-legacy-invalid.xml | 3 +
tests/cputestdata/ppc64-guest-legacy.xml | 3 +
tests/cputestdata/ppc64-guest-nofallback.xml | 3 +-
tests/cputestdata/ppc64-guest-strict.xml | 3 +
tests/cputestdata/ppc64-guest.xml | 3 +-
.../ppc64-host+guest,ppc_models-result.xml | 2 +-
... ppc64-host+guest-legacy,ppc_models-result.xml} | 2 +-
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 | 3 -
29 files changed, 444 insertions(+), 247 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-legacy.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-exact.xml
create mode 100644 tests/cputestdata/ppc64-guest-exact.xml
create mode 100644 tests/cputestdata/ppc64-guest-legacy-incompatible.xml
create mode 100644 tests/cputestdata/ppc64-guest-legacy-invalid.xml
create mode 100644 tests/cputestdata/ppc64-guest-legacy.xml
create mode 100644 tests/cputestdata/ppc64-guest-strict.xml
rename
tests/cputestdata/{ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml
= ppc64-host+guest-legacy,ppc_models-result.xml} (64%)
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
delete mode 100644 tests/cputestdata/ppc64-strict.xml
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 20/20] tests: Add a bunch of cpu test case for ppc64
The test cases cover the cpuCompare(), cpuBaseline() and cpuNodeData() implementation. --- tests/cputest.c | 16 tests/cputestdata/ppc64-baseline-incompatible-models.xml | 14 ++ tests/cputestdata/ppc64-baseline-legacy.xml | 14 ++ tests/cputestdata/ppc64-baseline-same-model-result.xml | 3 +++ tests/cputestdata/ppc64-baseline-same-model.xml | 14 ++ tests/cputestdata/ppc64-guest-legacy-incompatible.xml| 3 +++ tests/cputestdata/ppc64-guest-legacy-invalid.xml | 3 +++ tests/cputestdata/ppc64-guest-legacy.xml | 3 +++ .../ppc64-host+guest-legacy,ppc_models-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 ++ 13 files changed, 98 insertions(+) create mode 100644 tests/cputestdata/ppc64-baseline-incompatible-models.xml create mode 100644 tests/cputestdata/ppc64-baseline-legacy.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-guest-legacy-incompatible.xml create mode 100644 tests/cputestdata/ppc64-guest-legacy-invalid.xml create mode 100644 tests/cputestdata/ppc64-guest-legacy.xml create mode 100644 tests/cputestdata/ppc64-host+guest-legacy,ppc_models-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 diff --git a/tests/cputest.c b/tests/cputest.c index 09f690a..5992dd0 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -571,6 +571,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); @@ -597,6 +604,9 @@ mymain(void) DO_TEST_COMPARE(ppc64, host, guest-strict, VIR_CPU_COMPARE_IDENTICAL); DO_TEST_COMPARE(ppc64, host, guest-exact, VIR_CPU_COMPARE_INCOMPATIBLE); +DO_TEST_COMPARE(ppc64, host, guest-legacy, VIR_CPU_COMPARE_IDENTICAL); +DO_TEST_COMPARE(ppc64, host, guest-legacy-incompatible, VIR_CPU_COMPARE_INCOMPATIBLE); +DO_TEST_COMPARE(ppc64, host, guest-legacy-invalid, VIR_CPU_COMPARE_ERROR); /* guest updates for migration * automatically compares host CPU with the result */ @@ -627,6 +637,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); +DO_TEST_BASELINE(ppc64, legacy, 0, -1); /* CPU features */ DO_TEST_HASFEATURE(x86, host, vmx, YES); @@ -664,6 +677,9 @@ mymain(void) DO_TEST_GUESTDATA(ppc64, host, guest, ppc_models, NULL, 0); DO_TEST_GUESTDATA(ppc64, host, guest-nofallback, ppc_models, POWER8, -1); +DO_TEST_GUESTDATA(ppc64, host, guest-legacy, ppc_models, NULL, 0); +DO_TEST_GUESTDATA(ppc64, host, guest-legacy-incompatible, ppc_models, NULL, -1); +DO_TEST_GUESTDATA(ppc64, host, guest-legacy-invalid, ppc_models, NULL, -1); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } 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 @@ +cpuTest +cpu + archppc64/arch + modelPOWER7/model + vendorIBM/vendor + topology sockets='2' cores='4' threads='1'/ +/cpu +cpu + archppc64/arch + modelPOWER8/model + vendorIBM/vendor + topology sockets='1' cores='1' threads='1'/ +/cpu +/cpuTest diff --git a/tests/cputestdata/ppc64-baseline-legacy.xml b/tests/cputestdata/ppc64-baseline-legacy.xml new file mode 100644 index 000..b652497 --- /dev/null +++ b/tests/cputestdata/ppc64-baseline-legacy.xml @@ -0,0 +1,14 @@ +cpuTest +cpu +
[libvirt] [PATCH v3 18/20] cpu: Forbid model fallback in the ppc64 driver
Unlike what happens on x86, on ppc64 you can't mix and match CPU features to obtain the guest CPU you want regardless of the host CPU, so the concept of model fallback doesn't apply. Make sure CPU definitions emitted by the driver, eg. as output of the cpuBaseline() and cpuUpdate() calls, reflect this fact. --- src/cpu/cpu_ppc64.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index d87e0d1..0f193cd 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -672,6 +672,7 @@ ppc64DriverUpdate(virCPUDefPtr guest, case VIR_CPU_MODE_HOST_MODEL: case VIR_CPU_MODE_HOST_PASSTHROUGH: guest-match = VIR_CPU_MATCH_EXACT; +guest-fallback = VIR_CPU_FALLBACK_FORBID; virCPUDefFreeModel(guest); return virCPUDefCopyModel(guest, host, true); @@ -759,6 +760,7 @@ ppc64DriverBaseline(virCPUDefPtr *cpus, cpu-type = VIR_CPU_TYPE_GUEST; cpu-match = VIR_CPU_MATCH_EXACT; +cpu-fallback = VIR_CPU_FALLBACK_FORBID; cleanup: ppc64MapFree(map); -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 19/20] tests: Re-enable ppc64 cpu tests
Now that all the changes have been implemented we can run the
test cases once again, after updating them to reflect the new
behaviour.
---
tests/cputest.c | 16
.../cputestdata/ppc64-baseline-incompatible-vendors.xml | 4 ++--
tests/cputestdata/ppc64-baseline-no-vendor-result.xml| 2 +-
tests/cputestdata/ppc64-baseline-no-vendor.xml | 2 +-
tests/cputestdata/ppc64-exact.xml| 3 ---
tests/cputestdata/ppc64-guest-exact.xml | 3 +++
tests/cputestdata/ppc64-guest-nofallback.xml | 3 +--
tests/cputestdata/ppc64-guest-strict.xml | 3 +++
tests/cputestdata/ppc64-guest.xml| 3 +--
tests/cputestdata/ppc64-host+guest,ppc_models-result.xml | 2 +-
tests/cputestdata/ppc64-host.xml | 2 +-
tests/cputestdata/ppc64-strict.xml | 3 ---
12 files changed, 18 insertions(+), 28 deletions(-)
delete mode 100644 tests/cputestdata/ppc64-exact.xml
create mode 100644 tests/cputestdata/ppc64-guest-exact.xml
create mode 100644 tests/cputestdata/ppc64-guest-strict.xml
delete mode 100644 tests/cputestdata/ppc64-strict.xml
diff --git a/tests/cputest.c b/tests/cputest.c
index 4dbccfd..09f690a 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -501,9 +501,7 @@ static const char *model486[] = { 486 };
static const char *nomodel[]= { nomodel };
static const char *models[] = { qemu64, core2duo, Nehalem };
static const char *haswell[]= { SandyBridge, Haswell };
-/* XXX temporarily disabled
-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)
@@ -597,10 +595,8 @@ mymain(void)
DO_TEST_COMPARE(x86, host-worse, nehalem-force,
VIR_CPU_COMPARE_IDENTICAL);
DO_TEST_COMPARE(x86, host-SandyBridge, exact-force-Haswell,
VIR_CPU_COMPARE_IDENTICAL);
-/* XXX temporarily disabled
-DO_TEST_COMPARE(ppc64, host, strict, VIR_CPU_COMPARE_IDENTICAL);
-DO_TEST_COMPARE(ppc64, host, exact, VIR_CPU_COMPARE_INCOMPATIBLE);
-*/
+DO_TEST_COMPARE(ppc64, host, guest-strict,
VIR_CPU_COMPARE_IDENTICAL);
+DO_TEST_COMPARE(ppc64, host, guest-exact,
VIR_CPU_COMPARE_INCOMPATIBLE);
/* guest updates for migration
* automatically compares host CPU with the result */
@@ -629,10 +625,8 @@ mymain(void)
DO_TEST_BASELINE(x86, 7, 0, 0);
DO_TEST_BASELINE(x86, 8, 0, 0);
-/* XXX temporarily disabled
DO_TEST_BASELINE(ppc64, incompatible-vendors, 0, -1);
DO_TEST_BASELINE(ppc64, no-vendor, 0, 0);
-*/
/* CPU features */
DO_TEST_HASFEATURE(x86, host, vmx, YES);
@@ -668,10 +662,8 @@ mymain(void)
DO_TEST_GUESTDATA(x86, host-Haswell-noTSX, Haswell-noTSX,
NULL, Haswell-noTSX, 0);
-/* XXX temporarily disabled
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,
POWER8, -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 @@
cpuTest
cpu
archppc64/arch
- modelPOWER7+_v2.1/model
+ modelPOWER7/model
vendorIntel/vendor
topology sockets='2' cores='4' threads='1'/
/cpu
cpu
archppc64/arch
- modelPOWER8_v1.0/model
+ modelPOWER8/model
vendorIntel/vendor
topology sockets='1' cores='1' threads='1'/
/cpu
diff --git a/tests/cputestdata/ppc64-baseline-no-vendor-result.xml
b/tests/cputestdata/ppc64-baseline-no-vendor-result.xml
index 36bae52..758099c 100644
--- a/tests/cputestdata/ppc64-baseline-no-vendor-result.xml
+++ b/tests/cputestdata/ppc64-baseline-no-vendor-result.xml
@@ -1,3 +1,3 @@
cpu mode='custom' match='exact'
- model fallback='allow'POWER7_v2.3/model
+ model fallback='forbid'POWER7/model
/cpu
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 @@
cpuTest
cpu
archppc64/arch
- modelPOWER7_v2.3/model
+ modelPOWER7/model
topology sockets='2' cores='4' threads='1'/
/cpu
/cpuTest
diff --git a/tests/cputestdata/ppc64-exact.xml
b/tests/cputestdata/ppc64-exact.xml
deleted file mode 100644
index c84f16a..000
--- a/tests/cputestdata/ppc64-exact.xml
+++ /dev/null
@@ -1,3 +0,0 @@
-cpu match='exact'
- modelPOWER8_v1.0/model
-/cpu
diff --git a/tests/cputestdata/ppc64-guest-exact.xml
[libvirt] [PATCH v3 17/20] cpu: Implement backwards compatibility in the ppc64 driver
All previously recognized CPU models (POWER7_v2.1, POWER7_v2.3,
POWER7+_v2.1 and POWER8_v1.0) are internally converted to the
corrisponding generation name so that existing guests don't stop
working.
---
src/cpu/cpu_ppc64.c | 36 ++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index bf8f4da..d87e0d1 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -57,6 +57,33 @@ struct ppc64_map {
struct ppc64_model *models;
};
+/* Convert a legacy CPU definition by transforming
+ * model names to generation names:
+ * POWER7_v2.1 = POWER7
+ * POWER7_v2.3 = POWER7
+ * POWER7+_v2.1 = POWER7
+ * POWER8_v1.0 = POWER8 */
+static virCPUDefPtr
+ppc64LegacyConvertCPUDef(const virCPUDef *legacy)
+{
+virCPUDefPtr cpu;
+
+if (!(cpu = virCPUDefCopy(legacy)))
+goto out;
+
+if (!(STREQ(cpu-model, POWER7_v2.1) ||
+ STREQ(cpu-model, POWER7_v2.3) ||
+ STREQ(cpu-model, POWER7+_v2.1) ||
+ STREQ(cpu-model, POWER8_v1.0))) {
+goto out;
+}
+
+cpu-model[strlen(POWERx)] = 0;
+
+ out:
+return cpu;
+}
+
static void
ppc64DataFree(virCPUppc64Data *data)
{
@@ -426,18 +453,22 @@ ppc64MakeCPUData(virArch arch,
static virCPUCompareResult
ppc64Compute(virCPUDefPtr host,
- const virCPUDef *cpu,
+ const virCPUDef *other,
virCPUDataPtr *guestData,
char **message)
{
struct ppc64_map *map = NULL;
struct ppc64_model *host_model = NULL;
struct ppc64_model *guest_model = NULL;
-
+virCPUDefPtr cpu = NULL;
virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR;
virArch arch;
size_t i;
+/* Ensure existing configurations are handled correctly */
+if (!(cpu = ppc64LegacyConvertCPUDef(other)))
+goto cleanup;
+
if (cpu-arch != VIR_ARCH_NONE) {
bool found = false;
@@ -506,6 +537,7 @@ ppc64Compute(virCPUDefPtr host,
ret = VIR_CPU_COMPARE_IDENTICAL;
cleanup:
+virCPUDefFree(cpu);
ppc64MapFree(map);
ppc64ModelFree(host_model);
ppc64ModelFree(guest_model);
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 13/20] cpu: Support multiple PVRs in the ppc64 driver
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 | 72
src/cpu/cpu_ppc64_data.h | 8 +-
2 files changed, 67 insertions(+), 13 deletions(-)
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 6c78ab8..db25f27 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -60,6 +60,10 @@ struct ppc64_map {
static void
ppc64DataFree(virCPUppc64Data *data)
{
+if (!data)
+return;
+
+VIR_FREE(data-pvr);
VIR_FREE(data);
}
@@ -67,13 +71,24 @@ static virCPUppc64Data *
ppc64DataCopy(const virCPUppc64Data *data)
{
virCPUppc64Data *copy;
+size_t i;
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
@@ -159,12 +174,14 @@ 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;
}
@@ -257,8 +274,16 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
struct ppc64_map *map)
{
struct ppc64_model *model;
+xmlNodePtr *nodes = NULL;
+xmlNodePtr bookmark;
char *vendor = NULL;
unsigned long pvr;
+size_t i;
+int n;
+
+/* Save the node the context was pointing to, as we're going
+ * to change it later. It's going to be restored on exit */
+bookmark = ctxt-node;
if (VIR_ALLOC(model) 0)
return -1;
@@ -298,14 +323,30 @@ 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++) {
+ctxt-node = nodes[i];
+
+if (!virXPathBoolean(boolean(./@value), ctxt) ||
+virXPathULongHex(string(./@value), ctxt, pvr) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Missing or invalid PVR value in CPU model
%s),
+ model-name);
+goto ignore;
+}
+model-data-pvr[i].value = pvr;
+}
if (!map-models) {
map-models = model;
@@ -315,7 +356,9 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
}
cleanup:
+ctxt-node = bookmark;
VIR_FREE(vendor);
+VIR_FREE(nodes);
return 0;
ignore:
@@ -506,10 +549,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;
}
@@ -557,9 +600,14 @@ ppc64DriverNodeData(virArch arch)
data = nodeData-data.ppc64;
+if (VIR_ALLOC_N(data-pvr, 1) 0)
+goto error;
+
+data-len = 1;
+
#if defined(__powerpc__) || defined(__powerpc64__)
asm(mfpvr %0
-: =r (data-pvr));
+: =r (data-pvr[0].value));
#endif
nodeData-arch = arch;
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 stdint.h
+typedef struct _virCPUppc64PVR virCPUppc64PVR;
+struct _virCPUppc64PVR {
+uint32_t value;
+};
+
typedef struct _virCPUppc64Data virCPUppc64Data;
struct _virCPUppc64Data {
-uint32_t pvr;
+size_t len;
+virCPUppc64PVR *pvr;
};
#endif /* __VIR_CPU_PPC64_DATA_H__ */
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 15/20] cpu: Parse and use PVR masks in the ppc64 driver
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.
---
src/cpu/cpu_map.xml | 14 +++---
src/cpu/cpu_ppc64.c | 24 ++--
src/cpu/cpu_ppc64_data.h | 1 +
3 files changed, 22 insertions(+), 17 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 @@
!-- IBM-based CPU models --
model name='POWER6'
vendor name='IBM'/
- pvr value='0x003e'/
+ pvr value='0x003e' mask='0x'/
/model
model name='POWER7'
vendor name='IBM'/
- pvr value='0x003f'/
- pvr value='0x004a'/
+ pvr value='0x003f' mask='0x'/
+ pvr value='0x004a' mask='0x'/
/model
model name='POWER8'
vendor name='IBM'/
- pvr value='0x004b'/
- pvr value='0x004d'/
+ pvr value='0x004b' mask='0x'/
+ pvr value='0x004d' mask='0x'/
/model
!-- Freescale-based CPU models --
model name='POWERPC_e5500'
vendor name='Freescale'/
- pvr value='0x8024'/
+ pvr value='0x8024' mask='0x'/
/model
model name='POWERPC_e6500'
vendor name='Freescale'/
- pvr value='0x8040'/
+ pvr value='0x8040' mask='0x'/
/model
/arch
/cpus
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index db25f27..bf8f4da 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -81,8 +81,10 @@ ppc64DataCopy(const 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;
@@ -179,20 +181,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;
}
@@ -346,6 +340,15 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
goto ignore;
}
model-data-pvr[i].value = pvr;
+
+if (!virXPathBoolean(boolean(./@mask), ctxt) ||
+virXPathULongHex(string(./@mask), ctxt, pvr) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Missing or invalid PVR mask in CPU model
%s),
+ model-name);
+goto ignore;
+}
+model-data-pvr[i].mask = pvr;
}
if (!map-models) {
@@ -609,6 +612,7 @@ ppc64DriverNodeData(virArch arch)
asm(mfpvr %0
: =r (data-pvr[0].value));
#endif
+data-pvr[0].mask = 0xul;
nodeData-arch = arch;
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 v3 09/20] cpu: CPU model names have to match on ppc64
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.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1250977
---
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 53d8fb0..0769956 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -350,7 +350,6 @@ ppc64Compute(virCPUDefPtr host,
const virCPUDef *cpu,
virCPUDataPtr *guestData,
char **message)
-
{
struct ppc64_map *map = NULL;
struct ppc64_model *host_model = NULL;
@@ -407,9 +406,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 93f9d2e..1e84fd3 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -501,7 +501,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)
@@ -662,7 +662,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 @@
cpu match='exact'
- modelPOWER8_v1.0/model
+ modelPOWER7_v2.3/model
topology sockets='2' cores='4' threads='1'/
/cpu
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 @@
cpu mode='custom' match='exact'
archppc64/arch
- model fallback='allow'POWER8_v1.0/model
+ model fallback='allow'POWER7_v2.3/model
vendorIBM/vendor
/cpu
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 @@
-cpu mode='custom' match='exact'
- archppc64/arch
- model fallback='forbid'POWER7_v2.1/model
- vendorIBM/vendor
-/cpu
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 02/13] qemu: implement model subelement to controller
On Sun, Aug 09, 2015 at 01:11:27AM -0400, Laine Stump wrote:
On 08/03/2015 06:05 AM, Martin Kletzander wrote:
[I reduced the Cc list so I don't need to hear jtomko's whining again]
On Sat, Jul 25, 2015 at 03:58:26PM -0400, Laine Stump wrote:
This patch provides qemu support for the contents of model in
controller for the two existing PCI controller types that need it
(i.e. the two controller types that are backed by a device that must
be specified on the qemu commandline):
1) pci-bridge - sets model name attribute default as pci-bridge
2) dmi-to-pci-bridge - sets model name attribute default as
i82801b11-bridge.
These both match current hardcoded practice.
The defaults are set at the end of qemuDomainAssignPCIAddresses(), It
can't be done earlier because some of the options that will be
autogenerated need full PCI address info for the controller and
because qemuDomainAssignPCIAddresses() might create extra controllers
which would need default settings added, and that hasn't been done at
the time the PostParse callbacks are being
run. qemuDomainAssignPCIAddresses() is still prior to the XML being
written to disk, though, so the autogenerated defaults are persistent.
qemu capabilities bits aren't checked until the commandline is
actually created (so the domain can possibly be defined on a host that
doesn't yet have support for the give n device, or a host different
from the one where it will eventually be run). At that time we compare
the type strings to known qemu device names and check for the
capabilities bit for that device.
---
Changes from V2: use enum instead of string model name.
src/qemu/qemu_command.c | 81
++---
1 file changed, 77 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 09f30c4..6a19d15 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2251,11 +2251,33 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr
def,
virDomainControllerDefPtr cont = def-controllers[i];
int idx = cont-idx;
virDevicePCIAddressPtr addr;
+virDomainPCIControllerOptsPtr options;
if (cont-type != VIR_DOMAIN_CONTROLLER_TYPE_PCI)
continue;
addr = cont-info.addr.pci;
+options = cont-opts.pciopts;
+
+/* set defaults for any other auto-generated config
+ * options for this controller that haven't been
+ * specified in config.
+ */
+switch ((virDomainControllerModelPCI)cont-model) {
+case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+if (options-modelName ==
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE)
+options-modelName =
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE;
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+if (options-modelName ==
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE)
+options-modelName =
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE;
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+break;
+}
+
/* check if every PCI bridge controller's ID is
greater than
* the bus it is placed onto
*/
@@ -4505,6 +4527,7 @@ qemuBuildControllerDevStr(virDomainDefPtr
domainDef,
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
int model = def-model;
+const char *modelName = NULL;
if (def-type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
if ((qemuSetSCSIControllerModel(domainDef, qemuCaps, model))
0)
@@ -4626,17 +4649,67 @@ qemuBuildControllerDevStr(virDomainDefPtr
domainDef,
}
switch (def-model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
-virBufferAsprintf(buf, pci-bridge,chassis_nr=%d,id=%s,
- def-idx, def-info.alias);
+if (def-opts.pciopts.modelName
+== VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(autogenerated pci-bridge options
not set));
+goto error;
+}
+
+modelName =
virDomainControllerPCIModelNameTypeToString(def-opts.pciopts.modelName);
+if (!modelName) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(unknown pci-bridge model name
value %d),
+ def-opts.pciopts.modelName);
+goto error;
+}
+if (def-opts.pciopts.modelName
+!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) {
+
[libvirt] [PATCH v2 1/2] conf: Add ioeventfd option for controllers
This will be used with a virtio-scsi controller later on.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 8 ++
docs/schemas/domaincommon.rng | 3 +++
src/conf/domain_conf.c | 20 +--
src/conf/domain_conf.h | 1 +
.../qemuxml2argv-disk-virtio-scsi-ioeventfd.xml| 29 ++
tests/qemuxml2xmltest.c| 1 +
6 files changed, 60 insertions(+), 2 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-ioeventfd.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 7eb21a8897dc..c3b495ac22de 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3000,6 +3000,14 @@
a sector is 512 bytes.
span class=sinceSince 1.2.7 (QEMU and KVM only)/span
/dd
+ dtcodeioeventfd/code/dt
+ dd
+The optional codeioeventfd/code attribute specifies
+whether the controller should use
+a href='https://patchwork.kernel.org/patch/43390/'
+I/O asynchronous handling/a or not. Accepted values are
+on and off. span class=sinceSince 1.2.18/span
+ /dd
/dl
p
USB companion controllers have an optional
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index ab02d14f8ee7..043c9757d268 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1832,6 +1832,9 @@
ref name=unsignedInt/
/attribute
/optional
+optional
+ ref name=ioeventfd/
+/optional
/element
/optional
/interleave
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8790aabe057a..fd0450f09b37 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7824,6 +7824,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
char *chassisNr = NULL;
char *chassis = NULL;
char *port = NULL;
+char *ioeventfd = NULL;
xmlNodePtr saved = ctxt-node;
int rc;
@@ -7867,6 +7868,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
queues = virXMLPropString(cur, queues);
cmd_per_lun = virXMLPropString(cur, cmd_per_lun);
max_sectors = virXMLPropString(cur, max_sectors);
+ioeventfd = virXMLPropString(cur, ioeventfd);
} else if (xmlStrEqual(cur-name, BAD_CAST model)) {
if (processedModel) {
virReportError(VIR_ERR_XML_ERROR, %s,
@@ -7910,6 +7912,13 @@ virDomainControllerDefParseXML(xmlNodePtr node,
goto error;
}
+if (ioeventfd
+(def-ioeventfd = virTristateSwitchTypeFromString(ioeventfd)) 0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Malformed 'ioeventfd' value %s'), max_sectors);
+goto error;
+}
+
if (def-type == VIR_DOMAIN_CONTROLLER_TYPE_USB
def-model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
VIR_DEBUG(Ignoring device address for none model usb controller);
@@ -8083,6 +8092,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
VIR_FREE(chassisNr);
VIR_FREE(chassis);
VIR_FREE(port);
+VIR_FREE(ioeventfd);
return def;
@@ -19148,7 +19158,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
}
if (pciModel || pciTarget ||
-def-queues || def-cmd_per_lun || def-max_sectors ||
+def-queues || def-cmd_per_lun || def-max_sectors || def-ioeventfd
||
virDomainDeviceInfoNeedsFormat(def-info, flags) || pcihole64) {
virBufferAddLit(buf, \n);
virBufferAdjustIndent(buf, 2);
@@ -19178,7 +19188,8 @@ virDomainControllerDefFormat(virBufferPtr buf,
virBufferAddLit(buf, /\n);
}
-if (def-queues || def-cmd_per_lun || def-max_sectors) {
+if (def-queues || def-cmd_per_lun ||
+def-max_sectors || def-ioeventfd) {
virBufferAddLit(buf, driver);
if (def-queues)
virBufferAsprintf(buf, queues='%u', def-queues);
@@ -19188,6 +19199,11 @@ virDomainControllerDefFormat(virBufferPtr buf,
if (def-max_sectors)
virBufferAsprintf(buf, max_sectors='%u', def-max_sectors);
+
+if (def-ioeventfd) {
+virBufferAsprintf(buf, ioeventfd='%s',
+
virTristateSwitchTypeToString(def-ioeventfd));
+}
virBufferAddLit(buf, /\n);
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 86430efb1c7f..9762c4f27698 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -840,6 +840,7 @@ struct _virDomainControllerDef {
unsigned int queues;
unsigned int cmd_per_lun;
unsigned int max_sectors;
+int ioeventfd; /* enum virTristateSwitch */
union {
Re: [libvirt] [PATCH v5 2/4] qemu: Add PCI-Express root to ARM virt machine
Hello! Of course, because there are no capabilities when defining XML, And now we need them, because depending on them we now may want to add PCIe root complex description. Therefore i think we have to add this to qemu test suite. I will take a look at it after some time. It actually shouldn't be that difficult. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/2] Add support for virtio-scsi controllers with ioeventfd
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1150484 v2: - Changed one occurence virTristateBool to virTristateSwitch as it should be - wrapped one line to be = 80 characters long - Rebased on top of current master (with laine's patches pushed) Martin Kletzander (2): conf: Add ioeventfd option for controllers qemu: Enable ioeventfd usage for virtio-scsi controllers docs/formatdomain.html.in | 8 ++ docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 20 +-- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c| 7 ++ .../qemuxml2argv-disk-virtio-scsi-ioeventfd.args | 9 +++ .../qemuxml2argv-disk-virtio-scsi-ioeventfd.xml| 29 ++ tests/qemuxml2argvtest.c | 3 +++ tests/qemuxml2xmltest.c| 1 + 9 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-ioeventfd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-ioeventfd.xml -- 2.5.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/2] qemu: Enable ioeventfd usage for virtio-scsi controllers
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1150484
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_command.c | 7 +++
.../qemuxml2argv-disk-virtio-scsi-ioeventfd.args | 9 +
tests/qemuxml2argvtest.c | 3 +++
3 files changed, 19 insertions(+)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-ioeventfd.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e12278c7b4fe..87468ca26fdb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4611,6 +4611,11 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
_('max_sectors' is only supported by virtio-scsi
controller));
return NULL;
}
+if (def-ioeventfd) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _('ioeventfd' is only supported by virtio-scsi
controller));
+return NULL;
+}
}
switch (def-type) {
@@ -4909,6 +4914,8 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
if (def-max_sectors)
virBufferAsprintf(buf, ,max_sectors=%u, def-max_sectors);
+qemuBuildIoEventFdStr(buf, def-ioeventfd, qemuCaps);
+
if (qemuBuildDeviceAddressStr(buf, domainDef, def-info, qemuCaps) 0)
goto error;
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-ioeventfd.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-ioeventfd.args
new file mode 100644
index ..1c5318935512
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-ioeventfd.args
@@ -0,0 +1,9 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 214 -smp 8 -nographic -nodefconfig -nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
+-device virtio-scsi-pci,id=scsi0,ioeventfd=on,bus=pci.0,addr=0x3 \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-scsi0-0-0-0 \
+-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\
+drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 7c37e114f34a..c2482e612448 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -878,6 +878,9 @@ mymain(void)
DO_TEST(disk-virtio-scsi-max_sectors,
QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
QEMU_CAPS_VIRTIO_SCSI);
+DO_TEST(disk-virtio-scsi-ioeventfd,
+QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
+QEMU_CAPS_VIRTIO_IOEVENTFD, QEMU_CAPS_VIRTIO_SCSI);
DO_TEST(disk-scsi-megasas,
QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
QEMU_CAPS_SCSI_MEGASAS);
--
2.5.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fail on attempts to use filterref for non-tap network connections
On Mon, Aug 10, 2015 at 02:09:42AM -0400, Laine Stump wrote: nwfilter uses iptables and ebtables, which only work properly on tap-based network connections (*not* on macvtap, for example), but we just ignore any filterref elements for other types of networks, potentially giving users a false sense of security. This patch checks the network type and fails/logs an error if any domain interface has a filterref when the connection isn't using a tap device. This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1180011 ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 04/20] cpu: Reorder functions in the ppc64 driver
Having the functions grouped together this way will avoid further
shuffling around down the line.
No functional changes.
---
src/cpu/cpu_ppc64.c | 127 +---
1 file changed, 62 insertions(+), 65 deletions(-)
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 18dbf86..5921263 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,23 @@ ppc64ModelFree(struct ppc64_model *model)
}
static struct ppc64_model *
+ppc64ModelCopy(const struct ppc64_model *model)
+{
+struct ppc64_model *copy;
+
+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,65 +154,41 @@ ppc64ModelFindPVR(const struct ppc64_map *map,
}
static struct ppc64_model *
-ppc64ModelCopy(const struct ppc64_model *model)
+ppc64ModelFromCPU(const virCPUDef *cpu,
+ const struct ppc64_map *map)
{
-struct ppc64_model *copy;
+struct ppc64_model *model;
-if (VIR_ALLOC(copy) 0 ||
-VIR_STRDUP(copy-name, model-name) 0) {
-ppc64ModelFree(copy);
+if (!(model = ppc64ModelFind(map, cpu-model))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Unknown CPU model %s), cpu-model);
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;
+return ppc64ModelCopy(model);
}
static void
-ppc64VendorFree(struct ppc64_vendor *vendor)
+ppc64MapFree(struct ppc64_map *map)
{
-if (!vendor)
+if (!map)
return;
-VIR_FREE(vendor-name);
-VIR_FREE(vendor);
-}
-
-static struct ppc64_model *
-ppc64ModelFromCPU(const virCPUDef *cpu,
- const struct ppc64_map *map)
-{
-struct ppc64_model *model;
+while (map-models) {
+struct ppc64_model *model = map-models;
+map-models = model-next;
+ppc64ModelFree(model);
+}
-if (!(model = ppc64ModelFind(map, cpu-model))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Unknown CPU model %s), cpu-model);
-return NULL;
+while (map-vendors) {
+struct ppc64_vendor *vendor = map-vendors;
+map-vendors = vendor-next;
+ppc64VendorFree(vendor);
}
-return ppc64ModelCopy(model);
+VIR_FREE(map);
}
-
static int
ppc64VendorLoad(xmlXPathContextPtr ctxt,
struct ppc64_map *map)
@@ -293,27 +312,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)
{
@@ -500,7 +498,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 v3 02/20] cpu: Simplify NULL handling in ppc64 driver
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 v3 06/20] tests: Remove unused file
No functional changes. --- 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 @@ -cpu mode='custom' match='exact' - model fallback='allow'POWER7+_v2.1/model -/cpu -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 07/20] tests: Improve result handling in cpuTestGuestData()
A test is considered successful if the obtained result matches
the expected result: if that's not the case, whether because a
test that was expected to succeed failed or because a test that
was supposed to fail succeeded, then something's not right and
we want the user to know about this.
On the other hand, if a failure that's unrelated to the bits
we're testing occurs, then the user should be notified even if
the test was expected to fail.
Use different values to tell these two situations apart.
Fix a test case that was wrongly expected to fail as well.
---
tests/cputest.c | 32
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/tests/cputest.c b/tests/cputest.c
index 06b3f12..93f9d2e 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -247,7 +247,7 @@ static int
cpuTestGuestData(const void *arg)
{
const struct data *data = arg;
-int ret = -1;
+int ret = -2;
virCPUDefPtr host = NULL;
virCPUDefPtr cpu = NULL;
virCPUDefPtr guest = NULL;
@@ -262,8 +262,10 @@ cpuTestGuestData(const void *arg)
cmpResult = cpuGuestData(host, cpu, guestData, NULL);
if (cmpResult == VIR_CPU_COMPARE_ERROR ||
-cmpResult == VIR_CPU_COMPARE_INCOMPATIBLE)
+cmpResult == VIR_CPU_COMPARE_INCOMPATIBLE) {
+ret = -1;
goto cleanup;
+}
if (VIR_ALLOC(guest) 0)
goto cleanup;
@@ -274,10 +276,7 @@ cpuTestGuestData(const void *arg)
guest-fallback = cpu-fallback;
if (cpuDecode(guest, guestData, data-models,
data-nmodels, data-preferred) 0) {
-if (data-result 0) {
-virResetLastError();
-ret = 0;
-}
+ret = -1;
goto cleanup;
}
@@ -294,7 +293,10 @@ cpuTestGuestData(const void *arg)
}
result = virBufferContentAndReset(buf);
-ret = cpuTestCompareXML(data-arch, guest, result, false);
+if (cpuTestCompareXML(data-arch, guest, result, false) 0)
+goto cleanup;
+
+ret = 0;
cleanup:
VIR_FREE(result);
@@ -302,6 +304,20 @@ cpuTestGuestData(const void *arg)
virCPUDefFree(host);
virCPUDefFree(cpu);
virCPUDefFree(guest);
+
+if (ret == data-result) {
+/* We got the result we expected, whether it was
+ * a success or a failure */
+virResetLastError();
+ret = 0;
+} else {
+VIR_TEST_VERBOSE(\nExpected result %d, got %d\n,
+ data-result, ret);
+/* Pad to line up with test name ... in virTestRun */
+VIR_TEST_VERBOSE(%74s, ... );
+ret = -1;
+}
+
return ret;
}
@@ -646,7 +662,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 v3 03/20] cpu: Simplify ppc64ModelFromCPU()
---
src/cpu/cpu_ppc64.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 05ff8f2..18dbf86 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -163,17 +163,10 @@ ppc64ModelFromCPU(const virCPUDef *cpu,
if (!(model = ppc64ModelFind(map, cpu-model))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Unknown CPU model %s), cpu-model);
-goto error;
+return NULL;
}
-if (!(model = ppc64ModelCopy(model)))
-goto error;
-
-return model;
-
- error:
-ppc64ModelFree(model);
-return NULL;
+return ppc64ModelCopy(model);
}
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 05/20] cpu: Remove ISA information from CPU map XML
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 @@ model name='power6' vendor name='IBM'/ - compat isa='2.05'/ pvr value='0x003e'/ /model model name='power7' vendor name='IBM'/ - compat isa='2.06'/ pvr value='0x003f'/ /model model name='power7+' vendor name='IBM'/ - compat isa='2.06B'/ pvr value='0x004a'/ /model model name='power8e' vendor name='IBM'/ - compat isa='2.07'/ pvr value='0x004b'/ /model model name='power8' vendor name='IBM'/ - compat isa='2.07'/ pvr value='0x004d'/ /model -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 12/20] cpu: Align ppc64 CPU data with x86
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 | 86
src/cpu/cpu_ppc64_data.h | 3 +-
3 files changed, 68 insertions(+), 23 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 efac739..6c78ab8 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,25 @@ struct ppc64_map {
};
static void
+ppc64DataFree(virCPUppc64Data *data)
+{
+VIR_FREE(data);
+}
+
+static virCPUppc64Data *
+ppc64DataCopy(const virCPUppc64Data *data)
+{
+virCPUppc64Data *copy;
+
+if (VIR_ALLOC(copy) 0)
+return NULL;
+
+copy-pvr = data-pvr;
+
+return copy;
+}
+
+static void
ppc64VendorFree(struct ppc64_vendor *vendor)
{
if (!vendor)
@@ -90,6 +109,7 @@ ppc64ModelFree(struct ppc64_model *model)
if (!model)
return;
+ppc64DataFree(model-data);
VIR_FREE(model-name);
VIR_FREE(model);
}
@@ -99,16 +119,22 @@ ppc64ModelCopy(const struct ppc64_model *model)
{
struct ppc64_model *copy;
-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 *
@@ -136,7 +162,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;
@@ -237,6 +263,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,
@@ -274,7 +305,7 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
model-name);
goto ignore;
}
-model-data.pvr = pvr;
+model-data-pvr = pvr;
if (!map-models) {
map-models = model;
@@ -318,7 +349,7 @@ ppc64LoadMap(void)
struct ppc64_map *map;
if (VIR_ALLOC(map) 0)
-return NULL;
+goto error;
if (cpuMapLoad(ppc64, ppc64MapLoadCallback, map) 0)
goto error;
@@ -332,7 +363,7 @@ ppc64LoadMap(void)
static virCPUDataPtr
ppc64MakeCPUData(virArch arch,
- struct cpuPPC64Data *data)
+ virCPUppc64Data *data)
{
virCPUDataPtr cpuData;
@@ -340,7 +371,9 @@ ppc64MakeCPUData(virArch arch,
return NULL;
cpuData-arch = arch;
-cpuData-data.ppc64 = *data;
+
+if (!(cpuData-data.ppc64 = ppc64DataCopy(data)))
+VIR_FREE(cpuData);
return cpuData;
}
@@ -421,7 +454,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;
@@ -473,10 +506,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;
}
@@ -506,25 +539,36 @@ ppc64DriverFree(virCPUDataPtr data)
if (!data)
return;
+ppc64DataFree(data-data.ppc64);
VIR_FREE(data);
}
static virCPUDataPtr
ppc64DriverNodeData(virArch arch)
{
-virCPUDataPtr cpuData;
+virCPUDataPtr nodeData;
+virCPUppc64Data *data;
-if (VIR_ALLOC(cpuData) 0)
-return NULL;
+if (VIR_ALLOC(nodeData) 0)
+goto error;
-cpuData-arch = arch;
+if (VIR_ALLOC(data)
[libvirt] [PATCH] lib: fix no zero arg check for iothread_id
https://bugzilla.redhat.com/show_bug.cgi?id=1251886 We do not allow delete an iothread which id is 0 in virDomainDelIOThread, but allow it in virDomainAddIOThread, Also we will output an error when parse an iothread which id is 0. Add a check for iothread_id in virDomainAddIOThread to fix it. Signed-off-by: Luyao Huang lhu...@redhat.com --- Is that okay to fix it in this place ? src/libvirt-domain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 837933f..8506942 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -7987,6 +7987,7 @@ virDomainAddIOThread(virDomainPtr domain, virCheckDomainReturn(domain, -1); virCheckReadOnlyGoto(domain-conn-flags, error); +virCheckNonZeroArgGoto(iothread_id, error); conn = domain-conn; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] lib: fix no zero arg check for iothread_id
On Mon, Aug 10, 2015 at 17:06:31 +0800, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1251886 We do not allow delete an iothread which id is 0 in virDomainDelIOThread, but allow it in virDomainAddIOThread, Also we will output an error when parse an iothread which id is 0. Add a check for iothread_id in virDomainAddIOThread to fix it. The limitation that iothread id shall not be 0 comes from the qemu implementation so I think that we could possibly want to have iothread id 0 in the future. I think the check should be done in the qemu driver. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] lib: fix no zero arg check for iothread_id
On 08/10/2015 05:23 PM, Peter Krempa wrote: On Mon, Aug 10, 2015 at 17:06:31 +0800, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1251886 We do not allow delete an iothread which id is 0 in virDomainDelIOThread, but allow it in virDomainAddIOThread, Also we will output an error when parse an iothread which id is 0. Add a check for iothread_id in virDomainAddIOThread to fix it. The limitation that iothread id shall not be 0 comes from the qemu implementation so I think that we could possibly want to have iothread id 0 in the future. I think the check should be done in the qemu driver. Make sense, i will move the check in qemu driver, also i will change the code for iothread pin/delete Thanks for your quick review. Peter Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 0/9] support multi-thread compress migration.
Hi, Eric and Jiri Can you have a look at this patch and give some comments at your leisure? -Original Message- From: Feng, Shaohe Sent: Thursday, July 9, 2015 9:02 PM To: libvir-list@redhat.com Cc: Jiri Denemark; Eric Blake; Li, Liang Z; Feng, Shaohe Subject: [PATCH V2 0/9] support multi-thread compress migration. These series patches support multi-thread compress during live migration. Eli Qiao (4): Add test cases for qemuMonitorJSONGetMigrationParameter remote: Add support for set and get multil thread migration parameters qemu_driver: Add support to set/get migration parameters. virsh: Add set and get multi-thread migration parameters commands ShaoHe Feng (5): qemu_migration: Add support for mutil-thread compressed migration enable qemu: Add monitor API for get/set migration parameters set multi-thread compress params for Migrate3 during live migration virsh: add multi-thread migration option for live migrate command Implement the public APIs for multi-thread compress parameters. .gnulib | 2 +- daemon/remote.c | 62 +++ include/libvirt/libvirt-domain.h | 31 ++ src/driver-hypervisor.h | 14 +++ src/libvirt-domain.c | 110 +++ src/libvirt_public.syms | 5 + src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 186 src/qemu/qemu_migration.c| 105 ++ src/qemu/qemu_migration.h| 32 -- src/qemu/qemu_monitor.c | 40 ++- src/qemu/qemu_monitor.h | 11 ++ src/qemu/qemu_monitor_json.c | 93 src/qemu/qemu_monitor_json.h | 9 ++ src/qemu/qemu_monitor_text.c | 95 + src/qemu/qemu_monitor_text.h | 10 ++ src/remote/remote_driver.c | 54 ++ src/remote/remote_protocol.x | 30 +- src/remote_protocol-structs | 26 + tests/qemumonitorjsontest.c | 53 ++ tools/virsh-domain.c | 223 ++- tools/virsh.pod | 37 +-- 22 files changed, 1212 insertions(+), 19 deletions(-) -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 11/20] tests: Temporarily disable ppc64 cpu tests
The upcoming commits will make heavy modifications to the ppc64
driver, split so that it's easier to review the changes.
Instead of updating the test cases so that they pass, possibly
only to update them again with the following commit, disable them
for the time being.
Another commit will update them all in one go once all required
changes are in place.
---
tests/cputest.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/tests/cputest.c b/tests/cputest.c
index 1e84fd3..4dbccfd 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -501,7 +501,9 @@ static const char *model486[] = { 486 };
static const char *nomodel[]= { nomodel };
static const char *models[] = { qemu64, core2duo, Nehalem };
static const char *haswell[]= { SandyBridge, Haswell };
+/* XXX temporarily disabled
static const char *ppc_models[] = { POWER7, POWER7_v2.1, POWER7_v2.3,
POWER8_v1.0};
+*/
static int
mymain(void)
@@ -595,8 +597,10 @@ mymain(void)
DO_TEST_COMPARE(x86, host-worse, nehalem-force,
VIR_CPU_COMPARE_IDENTICAL);
DO_TEST_COMPARE(x86, host-SandyBridge, exact-force-Haswell,
VIR_CPU_COMPARE_IDENTICAL);
+/* XXX temporarily disabled
DO_TEST_COMPARE(ppc64, host, strict, VIR_CPU_COMPARE_IDENTICAL);
DO_TEST_COMPARE(ppc64, host, exact, VIR_CPU_COMPARE_INCOMPATIBLE);
+*/
/* guest updates for migration
* automatically compares host CPU with the result */
@@ -625,8 +629,11 @@ mymain(void)
DO_TEST_BASELINE(x86, 7, 0, 0);
DO_TEST_BASELINE(x86, 8, 0, 0);
+/* XXX temporarily disabled
DO_TEST_BASELINE(ppc64, incompatible-vendors, 0, -1);
DO_TEST_BASELINE(ppc64, no-vendor, 0, 0);
+*/
+
/* CPU features */
DO_TEST_HASFEATURE(x86, host, vmx, YES);
DO_TEST_HASFEATURE(x86, host, lm, YES);
@@ -661,8 +668,10 @@ mymain(void)
DO_TEST_GUESTDATA(x86, host-Haswell-noTSX, Haswell-noTSX,
NULL, Haswell-noTSX, 0);
+/* XXX temporarily disabled
DO_TEST_GUESTDATA(ppc64, host, guest, ppc_models, NULL, 0);
DO_TEST_GUESTDATA(ppc64, host, guest-nofallback, ppc_models,
POWER7_v2.1, -1);
+*/
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 v3 08/20] cpu: Never skip CPU model name check in ppc64 driver
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 5921263..53d8fb0 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -407,26 +407,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 v3 16/20] cpu: Add POWER8NVL information to CPU map XML
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 @@ model name='POWER8' vendor name='IBM'/ pvr value='0x004b' mask='0x'/ + pvr value='0x004c' mask='0x'/ pvr value='0x004d' mask='0x'/ /model -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 14/20] cpu: Simplify ppc64 part of CPU map XML
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. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1250977 --- src/cpu/cpu_map.xml | 39 +++ 1 file changed, 3 insertions(+), 36 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 @@ vendor name='Freescale'/ !-- IBM-based CPU models -- -model name='POWER7' - vendor name='IBM'/ - pvr value='0x003f0200'/ -/model - -model name='POWER7_v2.1' - vendor name='IBM'/ - pvr value='0x003f0201'/ -/model - -model name='POWER7_v2.3' - vendor name='IBM'/ - pvr value='0x003f0203'/ -/model - -model name='POWER7+_v2.1' - vendor name='IBM'/ - pvr value='0x004a0201'/ -/model - -model name='POWER8_v1.0' - vendor name='IBM'/ - pvr value='0x004b0100'/ -/model - -model name='power6' +model name='POWER6' vendor name='IBM'/ pvr value='0x003e'/ /model -model name='power7' +model name='POWER7' vendor name='IBM'/ pvr value='0x003f'/ -/model - -model name='power7+' - vendor name='IBM'/ pvr value='0x004a'/ /model -model name='power8e' +model name='POWER8' vendor name='IBM'/ pvr value='0x004b'/ -/model - -model name='power8' - vendor name='IBM'/ pvr value='0x004d'/ /model -- 2.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 10/20] cpu: Use ppc64Compute() to implement ppc64DriverCompare()
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 0769956..efac739 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -438,16 +438,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
Re: [libvirt] [PATCH 1/2] network: added waiting for DAD to finish for bridge address.
On 08/04/2015 06:18 PM, Dmitry Guryanov wrote: On 07/31/2015 07:35 PM, Maxim Perevedentsev wrote: +/* 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 Then we should create list of sockaddr_in6 to pass addresses and netmasks to virnetlink.c since it should not include virNetwork* structures. This will be double work I think. +//Parse response. +dad = networkParseDadStatus(resp, recvbuflen, network); +if (dad) +usleep(1000 * 10); What if an interface will remain in transient state forever? I think this is the responsibility of kernel to update DAD state to 'success' or 'failed' to make interface leave transient state. In addition, this is what was expected from dnsmasq in above commit. -- Your sincerely, Maxim Perevedentsev -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] fix typo in comments
Signed-off-by: Cao jin caoj.f...@cn.fujitsu.com
---
src/datatypes.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/datatypes.h b/src/datatypes.h
index c498cb0..be108fe 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -383,7 +383,7 @@ struct _virConnect {
/* Per-connection error. */
virError err; /* the last error */
-virErrorFunc handler; /* associated handlet */
+virErrorFunc handler; /* associated handler */
void *userData; /* the user data */
/* Per-connection close callback */
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fail on attempts to use filterref for non-tap network connections
nwfilter uses iptables and ebtables, which only work properly on
tap-based network connections (*not* on macvtap, for example), but we
just ignore any filterref elements for other types of networks,
potentially giving users a false sense of security.
This patch checks the network type and fails/logs an error if any
domain interface has a filterref when the connection isn't using a
tap device.
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1180011
---
src/qemu/qemu_command.c | 11 +++
src/qemu/qemu_hotplug.c | 11 +++
2 files changed, 22 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e12278c..829f1dc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8581,6 +8581,17 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
return -1;
}
+/* and only TAP devices support nwfilter rules */
+if (net-filter
+!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
+ actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(filterref is not supported for
+ network interfaces of type %s),
+ virDomainNetTypeToString(actualType));
+return -1;
+}
+
if (net-backend.tap
!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1ea397f..aabdb78 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -910,6 +910,17 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
return -1;
}
+/* and only TAP devices support nwfilter rules */
+if (net-filter
+!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
+ actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(filterref is not supported for
+ network interfaces of type %s),
+ virDomainNetTypeToString(actualType));
+return -1;
+}
+
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
tapfdSize = vhostfdSize = net-driver.virtio.queues;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 12/20] cpu: Align ppc64 CPU data with x86
On Fri, 2015-08-07 at 16:45 -0400, John Ferlan wrote:
static virCPUDataPtr
ppc64DriverNodeData(virArch arch)
{
-virCPUDataPtr cpuData;
+virCPUDataPtr nodeData;
+virCPUppc64Data *data;
-if (VIR_ALLOC(cpuData) 0)
-return NULL;
+if (VIR_ALLOC(nodeData) 0)
+goto error;
-cpuData-arch = arch;
+data = nodeData-data.ppc64;
+
+if (VIR_ALLOC(data) 0)
+goto error;
Coverity complains that 'data' isn't free'd (or stored to be
free'd)
anywhere from here...
if you change the code as follows, the Coverity issue goes away (as
does
the follow-up patch (13) where data-pvr is allocated. That one is
fine...
if (VIR_ALLOC(nodeData-data.ppc64) 0)
goto error;
data = nodeData-data.ppc64;
The issue is 'data' is a local, the VIR_ALLOC will overwrite the
initial
setting.
Yup, that's definitely a bug. Thanks for pointing it out.
v3 is on the way :)
Cheers.
--
Andrea Bolognani
Software Engineer - Virtualization Team
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virDomainCoreDumpWithFormat: Mention enum for @dumpformat
So the API takes @dumpformat argument. This is what makes it special when compared to virDomainCoreDump. The argument is there so that users can choose the format of resulting core dump file. And to ease them the choosing process we even have an enum with supported values across all the hypervisors. But we don't mention the enum in the function description anywhere. Fix it! Signed-off-by: Michal Privoznik mpriv...@redhat.com --- Pushed as trivial. src/libvirt-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 837933f..dc89bab 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -1281,7 +1281,7 @@ virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags) * virDomainCoreDumpWithFormat: * @domain: a domain object * @to: path for the core file - * @dumpformat: format of domain memory's dump + * @dumpformat: format of domain memory's dump (one of virDomainCoreDumpFormat enum) * @flags: bitwise-OR of virDomainCoreDumpFlags * * This method will dump the core of a domain on a given file for analysis. -- 2.4.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix typo in comments
On Mon, Aug 10, 2015 at 03:31:45PM +0800, Cao jin wrote: Signed-off-by: Cao jin caoj.f...@cn.fujitsu.com --- src/datatypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK and pushed. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virNetDevBandwidthParseRate: Reject negative values
On Fri, Aug 07, 2015 at 05:12:12PM +0200, Michal Privoznik wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1022292 The following XML really does not make any sense: inbound average=-1 burst=-2 peak=-3 floor=-4/ There can't be a negative packet rate. Well, so far we haven't assigned any meaning to it. So reject it unless users harm themselves, because otherwise we turn the negative numbers into really big values. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/netdev_bandwidth_conf.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) ACK, would be nicer with a test case. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] rpc: Remove keepalive_required option
On 03.08.2015 13:19, Martin Kletzander wrote: Since its introduction in 2011 (particularly in commit f4324e329275), the option doesn't work. It just effectively disables all incoming connections. That's because the client private data that contain the 'keepalive_supported' boolean, are initialized to zeroes so the bool is false and the only other place where the bool is used is when checking whether the client supports keepalive. Thus, according to the server, no client supports keepalive. Removing this instead of fixing it is better because a) apparently nobody ever tried it since 2011 (4 years without one month) and b) we cannot know whether the client supports keepalive until we get a ping or pong keepalive packet. And that won't happen untile after we dispatched the ConnectOpen call. s/untile/until/ Another two reasons would be c) the keepalive_required was tracked on the server level, but keepalive_supported was in private data of the client as well as the check that was made in the remote layer, thus making all other instances of virNetServer miss this feature unless they all implemented it for themselves and d) we can always add it back in case there is a request and a use-case for it. Signed-off-by: Martin Kletzander mklet...@redhat.com --- v3: - Kept the config option in place the same way we did with log_buffer_size - Moved the deprecated options together v2: - Added a new daemontest file with current input/output. daemon/libvirtd-config.c | 4 - daemon/libvirtd-config.h | 2 - daemon/libvirtd.c | 2 - daemon/libvirtd.conf | 9 +- daemon/libvirtd.h | 1 - daemon/remote.c| 8 +- daemon/test_libvirtd.aug.in| 2 +- src/libvirt_remote.syms| 1 - src/locking/lock_daemon.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 25 + src/rpc/virnetserver.h | 3 - tests/libvirtdconftest.c | 4 +- .../input-data-no-keepalive-required.json | 124 + .../virnetdaemondata/output-data-admin-nomdns.json | 2 - .../virnetdaemondata/output-data-anon-clients.json | 1 - .../output-data-initial-nomdns.json| 1 - tests/virnetdaemondata/output-data-initial.json| 1 - .../output-data-no-keepalive-required.json | 124 + tests/virnetdaemontest.c | 2 +- 20 files changed, 262 insertions(+), 58 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-no-keepalive-required.json create mode 100644 tests/virnetdaemondata/output-data-no-keepalive-required.json ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
