[libvirt] [PATCH] util: return generic error in virCopyLastError if error is not set
by Nikolay Shirokovskiy
virCopyLastError is intended to be used after last error is set.
However due to virLastErrorObject failures (very unlikely through
as thread local error is allocated on first use) we can have zero
fields in a copy as a result. In particular code field can be set
to VIR_ERR_OK.
In some places (qemu monitor, qemu agent and qemu migaration code
for example) we use copy result as a flag and this leads to bugs.
Let's set copy result to generic error ("cause unknown") in this case.
Approach is based on John Ferlan comments in [1] and [2] to initial
attempt which fixes issue in much less generic way.
[1] https://www.redhat.com/archives/libvir-list/2018-April/msg02700.html
[2] https://www.redhat.com/archives/libvir-list/2018-May/msg00124.html
---
src/util/virerror.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c
index c000b00..9f158af 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -343,7 +343,7 @@ virCopyLastError(virErrorPtr to)
if (err)
virCopyError(err, to);
else
- virResetError(to);
+ virErrorGenericFailure(err);
return to->code;
}
--
1.8.3.1
6 years, 4 months
[libvirt] [PATCH v3 0/3] Implement the HTM pSeries feature
by Andrea Bolognani
Applies cleanly on top of aa51063927a0d48768ff88b11f12075ad5efc89f.
Changes from [v2]:
* rebase on top of master; the series is much smaller now.
Changes from [v1]:
* drop qom-list-properties machinery, as equivalent functionality
has been merged in the meantime;
* don't introduce scaffolding for uniform machine option handling
as a requirement for implementing this specific feature: we can
do ad-hoc processing for now, per the status quo, then clean up
everything (including existing features) with a separate series
later.
Changes from [RFC v3]:
* can be considered for merging, since the QEMU part already has;
* fix compatibility issues with QEMU <2.12 spotted by Shivaprasad;
* drop all features except for HTM, at least for the time being;
* add documentation.
Changes from [RFC v2]:
* use qom-list-properties to probe availability;
* test all features with a single XML file.
Changes from [RFC v1]:
* don't nest features inside a <pseries/> element;
* implement all optional features.
[v2] https://www.redhat.com/archives/libvir-list/2018-June/msg01374.html
[v1] https://www.redhat.com/archives/libvir-list/2018-March/msg00474.html
[RFC v3] https://www.redhat.com/archives/libvir-list/2018-March/msg00042.html
[RFC v2] https://www.redhat.com/archives/libvir-list/2018-February/msg00310.html
[RFC v1] https://www.redhat.com/archives/libvir-list/2018-January/msg00779.html
Andrea Bolognani (3):
qemu: Add capability for the HTM pSeries feature
qemu: Implement the HTM pSeries feature
news: Update for the HTM pSeries feature
docs/formatdomain.html.in | 8 ++++++++
docs/news.xml | 9 +++++++++
docs/schemas/domaincommon.rng | 5 +++++
src/conf/domain_conf.c | 19 ++++++++++++++++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 20 +++++++++++++++++++
src/qemu/qemu_domain.c | 13 ++++++++++++
.../caps_2.12.0.ppc64.xml | 1 +
.../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 +
tests/qemuxml2argvdata/pseries-features.args | 2 +-
tests/qemuxml2argvdata/pseries-features.xml | 1 +
tests/qemuxml2argvtest.c | 1 +
tests/qemuxml2xmloutdata/pseries-features.xml | 1 +
tests/qemuxml2xmltest.c | 1 +
16 files changed, 85 insertions(+), 1 deletion(-)
--
2.17.1
6 years, 4 months
[libvirt] [dbus PATCH 00/15] Build system fixes, cleanups and tweaks
by Andrea Bolognani
It all started when I got
tests/Makefile.am:17: warning: source file '$(top_srcdir)/src/util.c' is in a subdirectory,
tests/Makefile.am:17: but option 'subdir-objects' is disabled
automake: warning: possible forward-incompatibility.
automake: At least a source file is in a subdirectory, but the 'subdir-objects'
automake: automake option hasn't been enabled. For now, the corresponding output
automake: object file(s) will be placed in the top-level directory. However,
automake: this behaviour will change in future Automake versions: they will
automake: unconditionally cause object files to be placed in the same subdirectory
automake: of the corresponding sources.
automake: You are advised to start using 'subdir-objects' option throughout your
automake: project, to avoid future incompatibilities.
when running autogen.sh and decided to investigate; before
I realized it, I was already deep down the rabbit hole and
falling towards Wonderland at an increasingly concerning
speed.
Andrea Bolognani (15):
gitignore: Fix man page pattern
gitignore: Fix aclocal.m4 pattern
tests: Don't distribute compiled binaries
src: Fix typo in PIE_LDFLAGS
src: Remove empty CLEANFILES
src: Don't list source files in EXTRA_DIST
tests: Move includes to AM_CPPFLAGS
src: Make CFLAGS and LDFLAGS global
configure: Enable libtool
src: Build libutil.la separately
configure: Enable subdir-objects
configure: Enable -Wno-obsolete and tar-pax
configure: Use xz for release archives
data: Rename some variables to *_in_files
autotools: Use consistent style
.gitignore | 15 ++++++++--
Makefile.am | 12 ++++++--
configure.ac | 3 +-
data/Makefile.am | 40 +++++++++++++++++--------
docs/Makefile.am | 16 +++++++---
libvirt-dbus.spec.in | 2 +-
src/Makefile.am | 70 ++++++++++++++++++++++++++------------------
tests/Makefile.am | 36 ++++++++++++++++-------
8 files changed, 130 insertions(+), 64 deletions(-)
--
2.17.1
6 years, 4 months
[libvirt] [PATCH] qemuDomainDeviceDefValidateNetwork: Check for range only if IP prefix set
by Michal Privoznik
The @prefix attribute to <ip/> element for interface type user is
optional. Therefore, if left out it has value of zero in which
case we should not check whether it falls into <4, 27> range.
Otherwise we fail parsing domain XML for no good reason.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index afd572fc5e..c1798edf41 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4373,7 +4373,8 @@ qemuDomainDeviceDefValidateNetwork(const virDomainNetDef *net)
}
hasIPv4 = true;
- if (ip->prefix < 4 || ip->prefix > 27) {
+ if (ip->prefix > 0 &&
+ (ip->prefix < 4 || ip->prefix > 27)) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("invalid prefix, must be in range of 4-27"));
return -1;
--
2.16.4
6 years, 4 months
[libvirt] [PATCH] virsh: Provide completer for detach-device-alias
by Michal Privoznik
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-completer.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
tools/virsh-completer.h | 4 ++++
tools/virsh-domain.c | 1 +
3 files changed, 53 insertions(+)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index d3effe59ea..ae579a5bdb 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -811,3 +811,51 @@ virshCellnoCompleter(vshControl *ctl,
VIR_FREE(ret);
goto cleanup;
}
+
+
+char **
+virshDomainDeviceAliasCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags)
+{
+ virshControlPtr priv = ctl->privData;
+ xmlDocPtr xmldoc = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ int naliases;
+ xmlNodePtr *aliases = NULL;
+ size_t i;
+ unsigned int domainXMLFlags = 0;
+ char **ret = NULL;
+ char **tmp = NULL;
+
+ virCheckFlags(0, NULL);
+
+ if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+ return NULL;
+
+ if (vshCommandOptBool(cmd, "config"))
+ domainXMLFlags = VIR_DOMAIN_XML_INACTIVE;
+
+ if (virshDomainGetXML(ctl, cmd, domainXMLFlags, &xmldoc, &ctxt) < 0)
+ goto cleanup;
+
+ naliases = virXPathNodeSet("./devices//alias/@name", ctxt, &aliases);
+ if (naliases < 0)
+ goto cleanup;
+
+ if (VIR_ALLOC_N(tmp, naliases + 1) < 0)
+ goto cleanup;
+
+ for (i = 0; i < naliases; i++) {
+ if (!(tmp[i] = virXMLNodeContentString(aliases[i])))
+ goto cleanup;
+ }
+
+ VIR_STEAL_PTR(ret, tmp);
+ cleanup:
+ VIR_FREE(aliases);
+ xmlFreeDoc(xmldoc);
+ xmlXPathFreeContext(ctxt);
+ virStringListFree(tmp);
+ return ret;
+}
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index ee7eec68c5..50dc068d73 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -94,6 +94,10 @@ char ** virshNodedevEventNameCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+char ** virshDomainDeviceAliasCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
+
char ** virshCellnoCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6aa79f11b9..e9b88f0013 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11684,6 +11684,7 @@ static const vshCmdOptDef opts_detach_device_alias[] = {
{.name = "alias",
.type = VSH_OT_DATA,
.flags = VSH_OFLAG_REQ,
+ .completer = virshDomainDeviceAliasCompleter,
.help = N_("device alias")
},
VIRSH_COMMON_OPT_DOMAIN_CONFIG,
--
2.16.4
6 years, 4 months
[libvirt] CPU model versioning separate from machine type versioning ?
by Daniel P. Berrangé
This post is to raise question about helping use of named CPU models with
KVM ie any case not using -cpu host.
In the old days (ie before 2018), the world was innocent and we had a nice
set of named CPU models that corresponded to different Intel/AMD physical
CPU families/generations (lets temporarily ignore the -noTSX fiasco).
An application could query libvirt to determine what the host CPU model
was/is and use that model name in the guest XML and be fairly happy. If
they wanted to, they could explicitly include the extra features listed
by capabilities XML, or just rely on the host-model.
Then Spectre happened, and QEMU took the decision to almost double the
number of x86 models, adding in -IBRS / -IBPB variants for most CPU model,
so that applications could get the spec_ctrl / ibpb flags set without
having to manually list them.
In retrospect this was somewhat pointless, at least at the QEMU level,
because there is little difference in complexity between the two approaches:
-cpu Westmere,+spec-ctrl
-cpu Westmere-IBRS
At a higher level the extra named CPU models were slightly useful in so much
as many application developers had taken a lazy approach and not provided
users any way to explicitly turn on extra flags. This affected oVirt,
OpenStack and virt-manager, and probably more. Though OpenStack since added
ability to turn on arbitrary flags in response to the Spectre flaw, others
have not.
Then a recently along came the Speculative Store Bypass hardware vulnerability
requiring addition of yet another CPU flag to guest configs. This required use
of 'ssbd' on Intel and 'virt-ssbd' on AMD. While QEMU could have now added yet
more CPU models, eg Westmere-SSBD, this does not feel like a winning strategy
long term. Looking at the models how would a user have any clue whether the
-IBRS or -SSBD or -NEXT-FLAW or -YET-ANOTHER-FLAW suffix is "better" ? So QEMU
and libvirt took the joint decision to stop adding new named CPU models when
CPU vulnerabilities are discovered from this point forwards. Applications /
users would be expected to turn on CPU features explicitly as needed and are
considered broken if they don't provide this functionality.
As briefly mentioned above though, even before Spectre we had the pain of
dealing with the -noTSX CPU models working around brokenness in the Intel TSX
impl where they had to delete a CPU feature during microcode updates. This was
rather painful to roll out at the time.
An alternative to adding CPU models is to change meaning of existing CPU
models. QEMU has a way todo this by tieing the change to machine types, and
it has in fact been used to correct mistakes in the specification of CPU
models in the past, when those mistakes have not had dependancies on microcode
changes. This is not a particularly attractice way to deal with the errata.
Short life distros tend to stick with upstream QEMU machine types and won't
want to diverge by adding their own machine types. This gates them on having
upstream define the extra machine types which is tricky under embargo. Long
life distros do typically take on the burden of defining custom machine types,
but usually only add them when doing major updates.
The pain point with machine types is that the testing matrix grows at O(n^2)
Using machine types for CPU security errata would significant increase the
number of machine types and thus the testing matrix. eg if a security fix
is needed in rhel-7.3, 7.4, 7.5 we can't just add a pc-rhel-7.5.1 machine
with the fix, as it would not be possible to implement that in 7.3. So we
would need would need pc-rhel-7.3.1, pc-rhel-7.4.1, pc-rhel-7.5.1, machine
types, with 7.5 gaining all three. Finally CPU model changes have host
hardware dependancies and machine types need to be independant of the host,
since they are decided statically are build time. The only nice thing about
machine type is that it is reasonably obvious what the "best" machine type
is as they include a version number in the name, and users automatically get
the best if they use an unversioned name.
What if we can borrow the concept of versioning from machine types and apply
it to CPU models directly. For example, considering the history of "Haswell"
in QEMU, if we had versioned things, we would by now have:
Haswell-1.3.0 - first version (37507094f350b75c62dc059f998e7185de3ab60a)
Haswell-2.2.0 - added 'rdrand' (78a611f1936b3eac8ed78a2be2146a742a85212c_
Haswell-2.3.0 - removed 'hle' & 'rtm' (a356850b80b3d13b2ef737dad2acb05e6da03753)
Haswell-2.5.0 - added 'abm' (becb66673ec30cb604926d247ab9449a60ad8b11
Haswell-2.12.0 - added 'spec-ctrl' (ac96c41354b7e4c70b756342d9b686e31ab87458)
Haswell-3.0.0 - added 'ssbd' (never done)
If we followed the machine type approach, then a bare "Haswell" would
statically resolve at build time to the most recent Haswell-X.X.X version
associated with the QEMU release. This is unhelpful as we have a direct
dependancy on the host hardware features. Better would be for a bare
"Haswell" to be dynamically resolved at runtime, picking the most recent
version that is capable of launching given the current hardware, KVM/TCG impl
and QEMU version.
ie -cpu Haswell
should use Haswell-2.5.0 if on silicon with the TSX errata applied,
but use Haswell-2.12.0 if the Spectre errata is applied in microcode,
and use Haswell-3.0.0 once Intel finally releases SSBD microcode errata.
Versioning of CPU models as opposed to using arbitrary string suffixes
(-noTSX, -IBRS) has a number of usability improvements that we would
gain with versioned machine types, while avoiding exploding the machine
type matrix. With versioned CPU models we can
- Automatically tailor the best model based on hardware support
- Users always get the best model if they use the bare CPU name
- It is obvious to users which is the "best" / "newest" CPU model
- Avoid combinatorial expansion of machines since same CPU model
version can be added to all releases without adding machine types.
- Users can still force a specific downgraded model by using the
fully versioned name.
Such versioning of CPU models would largely "just work" with existing
libvirt versions, but to libvirt would really want to expand the bare
CPU name to a versioned CPU name when recording new guest XML, so the
ABI is preserved long term.
An application like virt-manager which wants a simple UI can forever be
happy simply giving users a list of bare CPU model names, and allowing
libvirt / QEMU to automatically expand to the best versioned model for
their host.
An application like oVirt/OpenStack which wants direct control can allow
the admin to choice if a bare name, or explicitly picking a versioned name
if they need to cope with possibility of outdated hosts.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
6 years, 4 months
[libvirt] [dbus PATCH] configure: Use HTTPS URL for libvirt.org
by Andrea Bolognani
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
Pushed as trivial.
configure.ac | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 1a0a457..d7820dd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,4 +1,4 @@
-AC_INIT([libvirt-dbus], [1.1.0], [libvir-list(a)redhat.com], [], [http://libvirt.org])
+AC_INIT([libvirt-dbus], [1.1.0], [libvir-list(a)redhat.com], [], [https://libvirt.org])
AC_CONFIG_SRCDIR(src/main.c)
AC_CONFIG_AUX_DIR([build-aux])
--
2.17.1
6 years, 4 months
