Moving project infrastructure / services to GitLab
by Daniel P. Berrangé
We've discussed the idea of moving to a GitForge before, with the debate
circling around the pros/cons of mailing list vs merge request workflows.
This mail is NOT focused on that particular debate, rather it is tackling the
much broader issue of libvirt project infrastructure consolidation, such that
we can eliminate time spent as OS sysadmins, while having long term sustainable
infra which facilitates collaboration for project participants & integration
between services.
Having said that, a move to a merge request workflow IS facilitated by this
and IS one of the items proposed. A full consideration of this item would make
this mail way to large. Thus I won't discuss it in any significant detail here,
merely mention it as a bullet point. I'll send a separate mail for this topic
later, as it isn't a pre-requisite for any of the other changes described in
this mail.
The short summary
=================
The mail is quite a long read, so lets get the key points out of the way in a
few lines.
- Libvirt uses a wide range of services, spread across many different
providers (redhat.com, quay.io, gitlab.com, travis-ci.org, openshift.com
and libvirt.org)
- The interaction/integration between the services we use is minimal or
non-existant.
- There is no consistency to whom has access / admin rights and multiple
logins are needed by people involved.
- Several services rely on individual people to do key manual tasks
- Several services are only managable by Red Hat employees (BZ, mailman,
openshift)
- The libvirt.org server is outdated, single point of failure, that is causing
frequent problems for the website build.
- Maintaining infrastucture is not a good use of libvirt contributors limited
time.
- GitLab can consolidate all the services that our *current* dev workflow
requires, except for the mailing list. This is beneficial even without
merge requests as a workflow.
- Libvirt project will be following commonly accepted best practices for open
source project development, lowering the barrier to entry / project specific
knowledge gap for contributors.
The key proposed changes
========================
The text below will outline each of the infrastructure changes to be performed
with key points for their rationale. Although the points below are numbered,
they should NOT be considered to be a strictly ordered sequence. Most of the
points are independent with few or no dependancies on other points.
1. Use gitlab.com as the location of the "master" git repositories
Current committers to libvirt.org would have create accounts on gitlab.com
and upload their SSH key.
No change in development workflow, merely update the "origin" URI.
Any current committer who has comitted in the last 12 months would get
access to the new git repos. This would cleanup any inactive people who
no longer contribute to libvirt.
Gives us the ablity to have per-GIT repo commit privileges
Partially eliminates DanB / DanV as points of failure on libvirt.org, as
the gitlab.com project admin privileges can be more flexibly granted, as
compared to multiple people having root on DanV's personal server.
Eliminates the libvirt.org physical server as a single point of failure
for SCM, which has no disaster recovery plan in place.
Improved reliability as libvirt.org anon-git breaks periodically
libvirt.org SCM would remain as a read-only mirror, to serve as a disaster
recovery option should we need it.
2. Use gitlab.com CI job as a way to generate the static website
Replaces the frequently breaking cronjob on libvirt.org which runs
configure+make.
Is more reliable and secure since it runs in a confined container
with known good distro package envionrment matching libvirt's min needs.
A new cron job on libvirt.org would download the published artifact
from the CI job, and deploy it to libvirt.org apache root
Partially elimintes DanB / DanV as points of failure, as the most
likely part to break is now in the CI job & fixable by any libvirt
contributor.
Still reliant on libvirt.org server for web presence in near term.
3. Use gitlab.com file hosting for release artifacts
Replaces the current use of libvirt.org/sources/ download archive.
gitlab is said to have a default size limit of 100 MB, but raised
to 1GB on gitlab.com. It is believed this is a per-file limit, but
it is unclear if there is also a cummulative limit across all files.
Limits must be confirmed before attempting this change.
libvirt.org has 4.5 GB of tar.xz files for libvirt, 7 GB of rpm files,
and 0.5 GB of other pieces (ie bindings)
The RPMs have been produced against a wide variety of distros over
time, from Fedora 12 to Fedora 30. The RPMs don't provide much obvious
value since downstream users have a variety of OS. Fedora Virt Preview
repo will provide the very same content after release in an easy to
consume YUM repo.
All historical tar.gz/xz files would be uploaded to gitlab.
No historical RPMs would be uploaded to gitlab.
A cronjob would sync newly uploaded files in gitlab, back to libvirt.org
to provide a disaster recovery option should we need it.
4. Use gitlab.com as the primary upstream issue tracker
This is to replace the current usage of bugzilla "Virtualization Tools"
product.
For the work we do with the upstream bugzilla product the GitLab issue
tracker is a good match, avoiding the complexity Bugzilla has grown for
dealing with RHEL process bureaucracy.
It is good for users, as they no longer need to register for an account
on BZ.
It is easier & more inclusive for maintainers as changes to the issue
tracker config are entirely self-service, instead of via a private Red
Hat issue tracker only available to Red Hat employees, or knowing the
right Red Hat admins personally.
Repo forks for major pieces of work can have their own issue tracker
for free, providing a collborative way to track the problems before
the code lands in upstream.
5. Use gitlab.com CI and container registry to build and host the CI images
This replaces our use of Quay.io
No longer any need for manually triggering container builds on Quay.io
Any libvirt maintainer can make changes to the CI/container setup and it
gets automatically processed when the changes hit git master.
libvirt-dockerfiles project would no longer be required, as dockerfiles
needed by each git repo would be added to that git repo. eg libvirt.git
would contain the its own docker files (generated from lcitool still)/
Eliminates the complexity or breakage when needing to deploy changes to
the container images in lockstep with changes to the CI control rules in
the project.
Eliminates the need to create yet another account login for Quay
6. Use gitlab.com CI as primary post-merge build and test platform
Red Hat has recently provided libvirt significant resource on both an
OpenStack and OpenShift, to serve as CI runners for libvirt, as we see
fit.
We can initially use the shared runners for all Linux testing and provide
our own docker containers as the environment.
If our CI throughput requires it, we can provide further private runners
for Linux via the Red Hat OpenShift resources we have access to.
For FreeBSD we would need to make lcitool install the gitlab agent in the
VM images. Can optionally do this for Linux images too if desired.
Choice of either carrying on using the physical host from CentOS CI
runners, but just connected to GitLab instead of Jenkins, or go straight
to new VMs deployed on Red Hat OpenStack resource we have access to.
Consolidates all our CI logic (except macOS) into one place in GitLab CI
yaml config. All the jenkins job builder logic in libvirt-jenkins-ci.git
is obsoleted, lcitool remains though potentially simplified.
Consistent with use of CI for generating website static content and
building container images.
Eliminates the need to use & manage CentOS CI, and eliminates need for
Travis CI except for macOS, so fewer accounts for contributors to create.
Any forks of the gitlab repo will automatically have a full set of CI
which is good for developers
7. Use gitlab.com as the project wiki
Replaces the current mediawiki install that is deployed via a Red Hat
hosted OpenShift instance
Would require a way to do an automated migration of the content from
the current mediawiki deployment that I manage for wiki.libvirt.org
Would requires a way to HTTP redirects from the old URLs
DanB is eliminated as a single point of failure for the wiki and no longer
has to waste time playing sysadmin for mediawiki / mysql.
8. Use gitlab.com pages for hosting virttools.org blog planet
Replaces the current hosting of planet tools in a Red Hat hosted
OpenShift instance
Setup the planet software as a periodic gitlab CI job publishing
artifacts to be server on gitlab pages.
DanB is eliminated as a single point of failure for the planet and no
longer has to waste time playing sysadmin
9. Use gitlab.com merge requests for non-core projects
This means every git repo that is not the core libvirt.org. All the
language bindings, the other object mappings (libvirt-dbus/snmp/etc)
and supporting tools (libvirt-jenkins-ci, etc)
All these projects would now benefit from pre-merge CI testing which
will catch build problems before they hit master. There is less
disruption to downstream consumers & no need for "build breaker fix"
rule to push changes without review.
The patch traffic for these repos is fairly low compared to libvirt.git
and the patches themselves tend to be simpler and thus easier to review.
Moving the non-core projects thus makes a smooth onramp for the libvirt
contributors to become more familiar with the merge request workflow,
and identify any likely places we might need to invest in tooling
Refer to separate mail to follow later for full consideration.
10. Use gitlab.com merge requests for core project
This is the final piece, removing the mailing list workflow for the main
libvirt.git repo.
Same points as for merger requests with non-core projects
Refer to separate mail for full consideration.
Background information
======================
As I was coming up with this email, I spent a bunch of time thinking about the
history of how libvirt project infra has grown, and what has happened to the
open source world in that time. What follows is stuff I wrote to help my own
understanding of why the GitForge model is so appealing & has grown so fast.
This was originally going to be the main part of the email, but I changed to
put all the concrete actions first. This is just background that motivated
the changes.
Project history
---------------
Since the very beginning of the project, libvirt has followed an email based
development workflow, using the libvir-list mailing list. At this time, email
based development was the standard model followed by all significant projects.
Prominent hosting options like sourceforge & its clones, offered a service
approx covering email, SCM and bug tracking.
Over time we've made some changes to the process for libvirt, but nothing
major. The most notable changes were switching from CVS to Git, and the use
of CentOS CI for post-merge testing & formalization of our platform support.
Less notable were mandating Signed-off-by, and partial usage of Reviewed-by
tags.
In the time since we switched to Git, the open source world has changed
massively with the rapid adoption of Git Forge services. An email based
workflow is no longer the norm, it is the rare outlier. This has gone hand in
hand with the increased recognition of the importance of CI automation into
the development workflow, and more recently importance of containers as a
deployment & distribution mechanism.
Libvirt.org management
----------------------
The libvirt.org server & domain registration is owned & managed by DanV. I
have sudo access to do administrative tasks too. It also hosts xmlsoft.org for
libxml / libxslt. This server is running RHEL-6 which has increasingly caused
us problems, since libvirt itself stopped supporting RHEL-6. This impacted
our ability to create nightly tarballs, and update the static website in
particular. It is also a major single point of failure both in terms of
hardware and administration access.
The key reason why the libvirt project exists on the libvirt.org server is
because in the early days of the project we considered it important that the
infrastructure used by the project was NOT under the control of Red Hat
corporate and IT. This was an attempt to promote the project as independently
run, as well as provide resilience for the long term, should Red Hat loose
interest in its development.
Since we manage libvirt.org we can in theory grant access to anyone we need
to who is involved in the project. Management of the server is very old school,
however, with no automation. We've been lucky to not have many serious outages
over the years. Access control is also crude as there's no two factor auth,
no fine grained repository commit permissions.
This leads to three key questions
- Is the use of the libvirt.org physical server neccessary for the
long term viability of the libvirt project
- Are we doing a good job at maintaining its services and ensuring we
are resilient to problems that occurrs
- Is working on sysadmin tasks a sensible and productive use of project
maintainer time
I'm pretty clear that the answer to all these questions is NO.
I still believe in the key point of avoiding a dependancy on Red Hat corporate
and IT, but my priorirty is changed. The original reasons are still valid, but
much more importantly than that, modern self-service infrastructure is a more
flexible approach, than infrastructure that depends on individual admins (like
myself and DanV for libvirt.org), or by opening tickets to get changes made
that take many days or weeks to be looked at (mailman, BZ).
Understanding the appeal of the Git Forge
-----------------------------------------
In the early days of open source, projects would start off in a single
maintainer mode, with code shared in an adhoc manner. Even if the maintainer
used SCM, typically no one would see it, so everyone else was a second class
citizenship in terms of participation.
If a project took off in popularity with multiple contributors actively
collaborating, it would have to organize its own infrastructure for something
like a CVS server, mailing list and a possibly a website. This was a burden
for whomever maintained the infra, but at least all active contributors were
now first class citizens. With an SCM like CVS though, infrequent contributors
were still second class citizens.
SourceForge was the first big service to offer commodity project hosting
infrastructure for free to open source projects. This reduced the burden on
project maintainers, no longer requiring them to spend time on sysadmin tasks.
SourceForge was briefly released as open source software, but went closed
source again, resulting in various forks, which you still see evidence of in
services like GNU Savannah. Use of this hosting was following a classic model
of needing to apply for a new project, get approval and wait for it to be
created on the backend.
In the classic world, forking a project had a very high cost because the fork
is cut off from any interaction with the origin project's infrastructure, even
if using a platform like sourceforge. A fork was thus a long term burden and
only desirable if there was compelling benefit to be had. If the fork withered,
it was often lost in the ether as its infrastructure went away.
The arrival of Git started something of a revolution for open source projects.
It democratized usage of the SCM, such that part time contributors are no
longer second class citizens vs the active maintainers. Everyone has accesss to
the same set of SCM tooling functionality.
The SCM tools are only a small part of the infra around a project, there is
still the web hosting, public SCM hosting, mailing list, bug trackers, CI
infrastructure, etc which are all an increasingly important part of a project
with multiple active contributors.
The compelling value of GitHub and GitLab is that they provide the way to
democratize all aspects of the project infra hosting to an extent that was not
achieved with the SourceForge like platforms. The keys differences are that
the new Git Forge platforms are completely self-service / instantaneous, while
also providing and encouraging a collaborative model between the project forks
and their services (linking issue trackers / merge requests across projects).
The projects' active maintainers are no longer in a tier above part time
contributors when it comes to project infrastructure. With one click to fork
the project, the user has access to the full range of infrastructure that the
original project had, SCM, bug tracking, web hosting, CI. Even more importantly
the fork is not cut off in a silo from the origin. The merge request model
makes it trivial to feed changes from a fork into the origin, or to reference
bugs betweeen projects. The notion of which project is "the origin" is now
fluid.
Forks no longer need to be thought of as a costly thing to avoid as much as
possible, rather they become a convenient tool for innovation, to develop code
for ideas which can't immediately be done as part of the origin project.
Successful ideas can then feed back into the origin.
It is no longer which project owns the infrastructure that matters most, but
rather which one has the biggest gravity amongst contributors, drawing in
their pull requests and bug reports. This may change over time, which is
especially beneficial for the lone-maintainer projects where the original
author looses interest, but a new person steps up to drive it forward. It is
also useful to multi-maintainer projects to have infra on a neutral third
party, so if the company sponsoring project developer changes focus and stop
funding infra, there's no longer a need to redeploy elsehwere. In this way the
forges help to keep compelling code alive over the long term and facilitate a
collaboration model that is stronger than that exhibited with the pre-GitForge
approach to project infrastructure.
Tangent: The fall from grace of SourceForge is a cautionary tale of risk of
relying on closed source hosted software for infrastructure. This lesson can be
extended to cover any hosted service in general, even if running open source
software, and taken to its logical conclusion, a project would end up hosting
everything itself. The latter has a huge burden in both cost and time, as well
as ease of collaboration. Thus there needs to be a risk/cost/reward tradeoff
made to decide where to draw the line. Relying on hosted services, but only
those based on open source software, is one pragmatic choice in where to place
the line that we've considered appropriate for libvirt, hence our use of Quay
in preference to DockerHub, and CentOS CI. This is the driver for picking
GitLab over GitHub, maximising the feature set available, but still using a
software platform that is open source with proven 3rd party deployments such as
those used by the GNOME and FreeDesktop projects. IOW we can use public hosted
gitlab.com to eliminate our sysadmin burden, while having confidence in our
ability to switch to self-hosted if circumstances change.
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 :|
4 years, 8 months
[libvirt PATCH 0/7] valgrind-inspired fixes
by Ján Tomko
First, clean up some valgrind noise.
Then, fix some reported leaks in the test suite.
Last, refactor some touched tests.
Ján Tomko (7):
tests: valgrind.supp: suppress g_type_register_static leaks
tests: valgrind: do not trace system binaries
qemumonitorjsontest: do not leak qapiData.schema
qemumonitorjsontest: use virCPUDefNew()
virsystemdtest: do not leak socket path
qemumonitorjsontest: GetCPUModelComparison: use g_auto
qemumonitorjsontest: GetCPUModelBaseline: use g_auto
tests/.valgrind.supp | 13 +++++++++++++
tests/Makefile.am | 2 +-
tests/qemumonitorjsontest.c | 29 ++++++++---------------------
tests/virsystemdtest.c | 5 ++++-
4 files changed, 26 insertions(+), 23 deletions(-)
--
2.24.1
4 years, 8 months
[libvirt PATCH v2] src: fix mixup of stack and heap allocated data in auth callback
by Daniel P. Berrangé
In the following recent change:
commit db72866310d1e520efa8ed2d4589bdb5e76a1c95
Author: Daniel P. Berrangé <berrange(a)redhat.com>
Date: Tue Jan 14 10:40:52 2020 +0000
util: add API for reading password from the console
the fact that "bufptr" pointer may point to either heap or stack
allocated data was overlooked. As a result, when the strdup was
removed, we ended up returning a pointer to the local stack to
the caller. When the caller referenced this stack pointer they
got out garbage which fairly quickly resulted in a crash.
We need to copy the stack buffer into heap memory in the username
case.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
Changed in v2:
- Keep use of fgets for mingw portability, but strdup the
static buffer
src/libvirt.c | 5 ++--
tests/Makefile.am | 2 ++
tests/virsh-auth | 57 ++++++++++++++++++++++++++++++++++++++++++++
tests/virsh-auth.xml | 5 ++++
4 files changed, 67 insertions(+), 2 deletions(-)
create mode 100755 tests/virsh-auth
create mode 100644 tests/virsh-auth.xml
diff --git a/src/libvirt.c b/src/libvirt.c
index a30eaa7590..76bf1fa677 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -111,7 +111,7 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr cred,
for (i = 0; i < ncred; i++) {
char buf[1024];
- char *bufptr = buf;
+ char *bufptr = NULL;
size_t len;
switch (cred[i].type) {
@@ -138,14 +138,15 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr cred,
if (!fgets(buf, sizeof(buf), stdin)) {
if (feof(stdin)) { /* Treat EOF as "" */
- buf[0] = '\0';
break;
}
return -1;
}
+
len = strlen(buf);
if (len != 0 && buf[len-1] == '\n')
buf[len-1] = '\0';
+ bufptr = g_strdup(buf);
break;
case VIR_CRED_PASSPHRASE:
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 83326dbaa9..3b5abcc12b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -164,6 +164,7 @@ EXTRA_DIST = \
xlconfigdata \
xmconfigdata \
xml2vmxdata \
+ virsh-auth.xml \
virstorageutildata \
virfilecachedata \
virresctrldata \
@@ -406,6 +407,7 @@ test_scripts =
libvirtd_test_scripts = \
libvirtd-fail \
libvirtd-pool \
+ virsh-auth \
virsh-cpuset \
virsh-define-dev-segfault \
virsh-int-overflow \
diff --git a/tests/virsh-auth b/tests/virsh-auth
new file mode 100755
index 0000000000..d548694190
--- /dev/null
+++ b/tests/virsh-auth
@@ -0,0 +1,57 @@
+#!/usr/bin/env python3
+# run virsh to validate interactive auth
+
+# Copyright (C) 2020 Red Hat, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see
+# <http://www.gnu.org/licenses/>.
+
+import os
+import os.path
+import sys
+import subprocess
+
+builddir = os.getenv("abs_top_builddir")
+if builddir is None:
+ builddir = os.path.join(os.getcwd(), "..")
+
+srcdir = os.getenv("abs_top_srcdir")
+if srcdir is None:
+ srcdir = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))
+
+uri = "test://" + os.path.join(srcdir, "tests", "virsh-auth.xml")
+
+virsh = os.path.join(builddir, "tools", "virsh")
+
+proc = subprocess.Popen([virsh, "-c", uri, "uri"],
+ universal_newlines=True,
+ start_new_session=True,
+ stdin=subprocess.PIPE,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+out, err = proc.communicate("astrochicken")
+
+if proc.returncode != 0:
+ print("virsh failed with code %d" % proc.returncode, file=sys.stderr)
+ if out != "":
+ print("stdout=%s" % out)
+ if err != "":
+ print("stderr=%s" % err)
+ sys.exit(1)
+
+if uri not in out:
+ print("Expected '%s' in '%s'" % (uri, out), file=sys.stderr)
+ sys.exit(1)
+
+sys.exit(0)
diff --git a/tests/virsh-auth.xml b/tests/virsh-auth.xml
new file mode 100644
index 0000000000..a045a0746e
--- /dev/null
+++ b/tests/virsh-auth.xml
@@ -0,0 +1,5 @@
+<node>
+ <auth>
+ <user>astrochicken</user>
+ </auth>
+</node>
--
2.24.1
4 years, 8 months
[PATCH] virthread: Free thread name only after worker has finished
by Michal Privoznik
When spawning a thread via our virThread APIs we let pthread
spawn this helper thread which sets couple of thread local
variables (e.g. thread job name or thread worker name) and as of
v6.1.0-40-gc85256b31b it also sets pthread name (which is then
visible in `ps' output for instance). Only after these steps the
intended function is called. However, just before calling it we
free the buffer that holds the thread name which results in
invalid memory reads:
==47027== Invalid read of size 1
==47027== at 0x48389C2: strlen (vg_replace_strmem.c:459)
==47027== by 0x58BB3D6: __vfprintf_internal (vfprintf-internal.c:1645)
==47027== by 0x58CE6E0: __vasprintf_internal (vasprintf.c:57)
==47027== by 0x574BA28: g_vasprintf (in /usr/lib64/libglib-2.0.so.0.6000.7)
==47027== by 0x57240CC: g_strdup_vprintf (in /usr/lib64/libglib-2.0.so.0.6000.7)
==47027== by 0x48E0EFA: vir_g_strdup_vprintf (glibcompat.c:209)
==47027== by 0x493AA05: virLogVMessage (virlog.c:573)
==47027== by 0x493A8FE: virLogMessage (virlog.c:513)
==47027== by 0x4992FC7: virThreadJobClear (virthreadjob.c:121)
==47027== by 0x4992844: virThreadHelper (virthread.c:237)
==47027== by 0x5817496: start_thread (pthread_create.c:486)
==47027== by 0x59563CE: clone (clone.S:95)
The problem is that neither virThreadJobSetWorker() nor
virThreadJobSet() create a copy of passed name. They just set a
thread local variable to point to the buffer which is then
freed. Moving the free towards the end of the wrapper function
solves the issue.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/virthread.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virthread.c b/src/util/virthread.c
index 37b2cdfbe9..64013b575c 100644
--- a/src/util/virthread.c
+++ b/src/util/virthread.c
@@ -217,7 +217,6 @@ static void *virThreadHelper(void *data)
} else {
thname = g_strdup(local.name);
}
- g_free(local.name);
#if defined(__linux__) || defined(WIN32)
pthread_setname_np(pthread_self(), thname);
@@ -236,6 +235,7 @@ static void *virThreadHelper(void *data)
if (!local.worker)
virThreadJobClear(0);
+ g_free(local.name);
return NULL;
}
--
2.24.1
4 years, 8 months
[libvirt PATCH v2 0/7] qemu: introduce a per-VM event loop thread
by Daniel P. Berrangé
This series changes the way we manage the QEMU monitor and
QEMU agent, such that all I/O is processed by a dedicated
event loop thread.
Many times in the past years people are reported issues
where long running monitor event callbacks block the main
libvirtd event loop for an unacceptably long period of
time. In the best case, this delays other work being
completed, but in bad cases it leads to mgmt app failures
when keepalive times trigger a client disconnect.
With this series, when we spawn QEMU, we also spawn a
dedicated thread running a GMainLoop instance. Then QEMU
monitor and QEMU agent UNIX sockets are switched to use
GMainContext for events instead of the traditional libvirt
event loop APIs. We kill off the event thread when we see
EOF on the QEMU monitor during shutdown.
The cost of this approach is one extra thread per VM,
which incurs a new OS process and a new stack allocation.
The QEMU driver already delegates some QMP event handling
to a thread pool for certain types of event. This was a
previous hack to mitigate the impact on the main event
loop. It is likely that we can remove this thread pool
from the QEMU driver & rely on the per-VM event threads
to do all the work. This will, however, require careful
analysis of each handler we pushed into the thread pool
to make sure its work doesn't have a dependency on the
event loop running in parallel.
This is one step towards eliminating the need to have the
libvirt event loop registered when using the embedded QEMU
driver. A further step is using a thread to dispatch the
lifecycle events, since that currently relies on a zero
second timer being registered with the event loop.
Changed in v2:
- Fixed race accessing free'd memory causing crash
- Fixed unused variables
- Merged first acked patches
Daniel P. Berrangé (7):
src: introduce an abstraction for running event loops
qemu: start/stop an event loop thread for domains
qemu: start/stop an event thread for QMP probing
tests: start/stop an event thread for QEMU monitor/agent tests
qemu: convert monitor to use the per-VM event loop
qemu: fix variable naming in agent code
qemu: convert agent to use the per-VM event loop
po/POTFILES.in | 1 +
src/libvirt_private.syms | 5 +
src/qemu/qemu_agent.c | 600 ++++++++++++++++++-----------------
src/qemu/qemu_agent.h | 1 +
src/qemu/qemu_domain.c | 33 ++
src/qemu/qemu_domain.h | 6 +
src/qemu/qemu_monitor.c | 145 ++++-----
src/qemu/qemu_monitor.h | 3 +-
src/qemu/qemu_process.c | 43 ++-
src/qemu/qemu_process.h | 2 +
src/util/Makefile.inc.am | 2 +
src/util/vireventthread.c | 190 +++++++++++
src/util/vireventthread.h | 31 ++
tests/qemumonitortestutils.c | 14 +
14 files changed, 700 insertions(+), 376 deletions(-)
create mode 100644 src/util/vireventthread.c
create mode 100644 src/util/vireventthread.h
--
2.24.1
4 years, 8 months
[libvirt PATCH] src: fix mixup of stack and heap allocated data in auth callback
by Daniel P. Berrangé
In the following recent change:
commit db72866310d1e520efa8ed2d4589bdb5e76a1c95
Author: Daniel P. Berrangé <berrange(a)redhat.com>
Date: Tue Jan 14 10:40:52 2020 +0000
util: add API for reading password from the console
the fact that "bufptr" pointer may point to either heap or stack
allocated data was overlooked. As a result, when the strdup was
removed, we ended up returning a pointer to the local stack to
the caller. When the caller referenced this stack pointer they
got out garbage which fairly quickly resulted in a crash.
Switching from fgets() to getline() lets to avoid the need for
the stack allocated buffer entirely, which makes both cases
of the switch consistent.
Test case addition is inspired by the libguestfs test which
caught this bug.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/libvirt.c | 30 +++++++++++------------
tests/Makefile.am | 2 ++
tests/virsh-auth | 57 ++++++++++++++++++++++++++++++++++++++++++++
tests/virsh-auth.xml | 5 ++++
4 files changed, 79 insertions(+), 15 deletions(-)
create mode 100755 tests/virsh-auth
create mode 100644 tests/virsh-auth.xml
diff --git a/src/libvirt.c b/src/libvirt.c
index a30eaa7590..cc9c2eb5a2 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -110,9 +110,8 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr cred,
size_t i;
for (i = 0; i < ncred; i++) {
- char buf[1024];
- char *bufptr = buf;
- size_t len;
+ char *buf = NULL;
+ size_t len = 0;
switch (cred[i].type) {
case VIR_CRED_EXTERNAL: {
@@ -136,16 +135,17 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr cred,
if (fflush(stdout) != 0)
return -1;
- if (!fgets(buf, sizeof(buf), stdin)) {
- if (feof(stdin)) { /* Treat EOF as "" */
- buf[0] = '\0';
- break;
+ if (getline(&buf, &len, stdin) < 0) {
+ if (!feof(stdin)) {
+ return -1;
}
- return -1;
+ /* Use creddefault on EOF */
+ buf = NULL;
+ } else {
+ len = strlen(buf);
+ if (len != 0 && buf[len-1] == '\n')
+ buf[len-1] = '\0';
}
- len = strlen(buf);
- if (len != 0 && buf[len-1] == '\n')
- buf[len-1] = '\0';
break;
case VIR_CRED_PASSPHRASE:
@@ -155,9 +155,9 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr cred,
if (fflush(stdout) != 0)
return -1;
- bufptr = virGetPassword();
- if (STREQ(bufptr, ""))
- VIR_FREE(bufptr);
+ buf = virGetPassword();
+ if (STREQ(buf, ""))
+ VIR_FREE(buf);
break;
default:
@@ -165,7 +165,7 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr cred,
}
if (cred[i].type != VIR_CRED_EXTERNAL) {
- cred[i].result = bufptr ? bufptr : g_strdup(cred[i].defresult ? cred[i].defresult : "");
+ cred[i].result = buf ? buf : g_strdup(cred[i].defresult ? cred[i].defresult : "");
cred[i].resultlen = strlen(cred[i].result);
}
}
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 83326dbaa9..3b5abcc12b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -164,6 +164,7 @@ EXTRA_DIST = \
xlconfigdata \
xmconfigdata \
xml2vmxdata \
+ virsh-auth.xml \
virstorageutildata \
virfilecachedata \
virresctrldata \
@@ -406,6 +407,7 @@ test_scripts =
libvirtd_test_scripts = \
libvirtd-fail \
libvirtd-pool \
+ virsh-auth \
virsh-cpuset \
virsh-define-dev-segfault \
virsh-int-overflow \
diff --git a/tests/virsh-auth b/tests/virsh-auth
new file mode 100755
index 0000000000..d548694190
--- /dev/null
+++ b/tests/virsh-auth
@@ -0,0 +1,57 @@
+#!/usr/bin/env python3
+# run virsh to validate interactive auth
+
+# Copyright (C) 2020 Red Hat, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see
+# <http://www.gnu.org/licenses/>.
+
+import os
+import os.path
+import sys
+import subprocess
+
+builddir = os.getenv("abs_top_builddir")
+if builddir is None:
+ builddir = os.path.join(os.getcwd(), "..")
+
+srcdir = os.getenv("abs_top_srcdir")
+if srcdir is None:
+ srcdir = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))
+
+uri = "test://" + os.path.join(srcdir, "tests", "virsh-auth.xml")
+
+virsh = os.path.join(builddir, "tools", "virsh")
+
+proc = subprocess.Popen([virsh, "-c", uri, "uri"],
+ universal_newlines=True,
+ start_new_session=True,
+ stdin=subprocess.PIPE,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+out, err = proc.communicate("astrochicken")
+
+if proc.returncode != 0:
+ print("virsh failed with code %d" % proc.returncode, file=sys.stderr)
+ if out != "":
+ print("stdout=%s" % out)
+ if err != "":
+ print("stderr=%s" % err)
+ sys.exit(1)
+
+if uri not in out:
+ print("Expected '%s' in '%s'" % (uri, out), file=sys.stderr)
+ sys.exit(1)
+
+sys.exit(0)
diff --git a/tests/virsh-auth.xml b/tests/virsh-auth.xml
new file mode 100644
index 0000000000..a045a0746e
--- /dev/null
+++ b/tests/virsh-auth.xml
@@ -0,0 +1,5 @@
+<node>
+ <auth>
+ <user>astrochicken</user>
+ </auth>
+</node>
--
2.24.1
4 years, 8 months
[PATCH 0/3] remove dimm auto-align on hotplug/unplug
by Daniel Henrique Barboza
This series fixes bug [1]. See patch 2 for details.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1780506
Daniel Henrique Barboza (3):
qemu_domain.c: make qemuDomainGetMemoryModuleSizeAlignment() public
qemu_hotplug.c: remove dimm auto-align on hotplug/unplug
qemu_domain.c: remove qemuDomainMemoryDeviceAlignSize()
src/qemu/qemu_domain.c | 23 +++--------------------
src/qemu/qemu_domain.h | 4 ++--
src/qemu/qemu_hotplug.c | 23 +++++++++++++++++++++--
3 files changed, 26 insertions(+), 24 deletions(-)
--
2.24.1
4 years, 8 months
[PATCH 0/4] NVDIMM suport for pSeries guests
by Daniel Henrique Barboza
Hi,
This patch series adds NVDIMM suport for ppc64 guests,
which consists on adding an extra 'uuid' element in
the nvdimm command line on the existing support for
x86.
These patches shouldn't change anything in the existing
x86 support, given that the new 'uuid' element is only
applicable for pSeries guests.
Daniel Henrique Barboza (4):
conf: Introduce optional 'uuid' element for NVDIMM memory
qemu_command.c: enable NVDIMM support for ppc64
formatdomain.html.in: document the new 'uuid' NVDIMM element
news.xml: document the new NVDIMM support for Pseries guest
docs/formatdomain.html.in | 13 +++++
docs/news.xml | 11 +++++
docs/schemas/domaincommon.rng | 5 ++
src/conf/domain_conf.c | 37 +++++++++++++--
src/conf/domain_conf.h | 3 ++
src/qemu/qemu_command.c | 7 +++
.../memory-hotplug-nvdimm-ppc64.args | 32 +++++++++++++
.../memory-hotplug-nvdimm-ppc64.xml | 47 +++++++++++++++++++
tests/qemuxml2argvtest.c | 4 ++
.../memory-hotplug-nvdimm-ppc64.xml | 47 +++++++++++++++++++
tests/qemuxml2xmltest.c | 2 +
11 files changed, 205 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.args
create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml
create mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml
--
2.24.1
4 years, 8 months
[PATCH v2 0/3] Tighten qemu-img rules on missing backing format
by Eric Blake
In v2:
- patch 3 changes to ALWAYS warn if -b provided without -F (rather
than being silent on raw or json:) [Peter]
- patch 3 changes to ONLY write implied format if probe read raw (all
other probes are still mentioned, but not implicitly written) [Peter]
- couple more tests converted in patch 1 [fallout from the above]
Eric Blake (3):
iotests: Specify explicit backing format where sensible
block: Add support to warn on backing file change without format
qemu-img: Deprecate use of -b without -F
qemu-deprecated.texi | 15 +++++++++++
include/block/block.h | 4 +--
block.c | 34 ++++++++++++++++++++++---
block/qcow2.c | 2 +-
block/stream.c | 2 +-
blockdev.c | 3 ++-
qemu-img.c | 10 ++++++--
tests/qemu-iotests/017 | 2 +-
tests/qemu-iotests/017.out | 2 +-
tests/qemu-iotests/018 | 2 +-
tests/qemu-iotests/018.out | 2 +-
tests/qemu-iotests/019 | 5 ++--
tests/qemu-iotests/019.out | 2 +-
tests/qemu-iotests/020 | 4 +--
tests/qemu-iotests/020.out | 4 +--
tests/qemu-iotests/024 | 8 +++---
tests/qemu-iotests/024.out | 5 ++--
tests/qemu-iotests/028 | 4 +--
tests/qemu-iotests/028.out | 2 +-
tests/qemu-iotests/030 | 26 +++++++++++++------
tests/qemu-iotests/034 | 2 +-
tests/qemu-iotests/034.out | 2 +-
tests/qemu-iotests/037 | 2 +-
tests/qemu-iotests/037.out | 2 +-
tests/qemu-iotests/038 | 2 +-
tests/qemu-iotests/038.out | 2 +-
tests/qemu-iotests/039 | 3 ++-
tests/qemu-iotests/039.out | 2 +-
tests/qemu-iotests/040 | 47 +++++++++++++++++++++++++----------
tests/qemu-iotests/041 | 37 ++++++++++++++++++---------
tests/qemu-iotests/042 | 4 +--
tests/qemu-iotests/043 | 18 +++++++-------
tests/qemu-iotests/043.out | 16 +++++++-----
tests/qemu-iotests/046 | 2 +-
tests/qemu-iotests/046.out | 2 +-
tests/qemu-iotests/050 | 4 +--
tests/qemu-iotests/050.out | 2 +-
tests/qemu-iotests/051 | 2 +-
tests/qemu-iotests/051.out | 2 +-
tests/qemu-iotests/051.pc.out | 2 +-
tests/qemu-iotests/056 | 3 ++-
tests/qemu-iotests/060 | 2 +-
tests/qemu-iotests/060.out | 2 +-
tests/qemu-iotests/061 | 10 ++++----
tests/qemu-iotests/061.out | 10 ++++----
tests/qemu-iotests/069 | 2 +-
tests/qemu-iotests/069.out | 2 +-
tests/qemu-iotests/073 | 2 +-
tests/qemu-iotests/073.out | 2 +-
tests/qemu-iotests/082 | 16 +++++++-----
tests/qemu-iotests/082.out | 16 ++++++------
tests/qemu-iotests/085 | 4 +--
tests/qemu-iotests/085.out | 6 ++---
tests/qemu-iotests/089 | 2 +-
tests/qemu-iotests/089.out | 2 +-
tests/qemu-iotests/095 | 4 +--
tests/qemu-iotests/095.out | 4 +--
tests/qemu-iotests/097 | 4 +--
tests/qemu-iotests/097.out | 16 ++++++------
tests/qemu-iotests/098 | 2 +-
tests/qemu-iotests/098.out | 8 +++---
tests/qemu-iotests/110 | 4 +--
tests/qemu-iotests/110.out | 4 +--
tests/qemu-iotests/114 | 4 +--
tests/qemu-iotests/114.out | 1 +
tests/qemu-iotests/122 | 27 ++++++++++++--------
tests/qemu-iotests/122.out | 8 +++---
tests/qemu-iotests/126 | 4 +--
tests/qemu-iotests/126.out | 4 +--
tests/qemu-iotests/127 | 4 +--
tests/qemu-iotests/127.out | 4 +--
tests/qemu-iotests/129 | 3 ++-
tests/qemu-iotests/133 | 2 +-
tests/qemu-iotests/133.out | 2 +-
tests/qemu-iotests/139 | 2 +-
tests/qemu-iotests/141 | 4 +--
tests/qemu-iotests/141.out | 4 +--
tests/qemu-iotests/142 | 2 +-
tests/qemu-iotests/142.out | 2 +-
tests/qemu-iotests/153 | 14 +++++------
tests/qemu-iotests/153.out | 35 ++++++++++++++------------
tests/qemu-iotests/154 | 42 +++++++++++++++----------------
tests/qemu-iotests/154.out | 42 +++++++++++++++----------------
tests/qemu-iotests/155 | 12 ++++++---
tests/qemu-iotests/156 | 9 ++++---
tests/qemu-iotests/156.out | 6 ++---
tests/qemu-iotests/158 | 2 +-
tests/qemu-iotests/158.out | 2 +-
tests/qemu-iotests/161 | 8 +++---
tests/qemu-iotests/161.out | 8 +++---
tests/qemu-iotests/176 | 4 +--
tests/qemu-iotests/176.out | 32 ++++++++++++------------
tests/qemu-iotests/177 | 2 +-
tests/qemu-iotests/177.out | 2 +-
tests/qemu-iotests/179 | 2 +-
tests/qemu-iotests/179.out | 2 +-
tests/qemu-iotests/189 | 2 +-
tests/qemu-iotests/189.out | 2 +-
tests/qemu-iotests/191 | 12 ++++-----
tests/qemu-iotests/191.out | 12 ++++-----
tests/qemu-iotests/195 | 6 ++---
tests/qemu-iotests/195.out | 6 ++---
tests/qemu-iotests/198 | 2 +-
tests/qemu-iotests/198.out | 3 ++-
tests/qemu-iotests/204 | 2 +-
tests/qemu-iotests/204.out | 2 +-
tests/qemu-iotests/216 | 2 +-
tests/qemu-iotests/224 | 4 +--
tests/qemu-iotests/228 | 5 ++--
tests/qemu-iotests/245 | 3 ++-
tests/qemu-iotests/249 | 4 +--
tests/qemu-iotests/249.out | 4 +--
tests/qemu-iotests/252 | 2 +-
tests/qemu-iotests/257 | 3 ++-
tests/qemu-iotests/267 | 4 +--
tests/qemu-iotests/267.out | 6 ++---
tests/qemu-iotests/270 | 2 +-
tests/qemu-iotests/270.out | 2 +-
tests/qemu-iotests/273 | 4 +--
tests/qemu-iotests/273.out | 4 +--
tests/qemu-iotests/279 | 4 +--
tests/qemu-iotests/279.out | 4 +--
122 files changed, 476 insertions(+), 351 deletions(-)
--
2.25.1
4 years, 8 months
[libvirt PATCH] logging: Use default timeout of 120 seconds for virtlogd
by Andrea Bolognani
This is the same timeout of all other daemons, and just like them
virtlogd is socket-activated, so it will automatically be started
on demand whenever that's necessary.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/logging/virtlogd.sysconf | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/logging/virtlogd.sysconf b/src/logging/virtlogd.sysconf
index 5886f35110..678af34faf 100644
--- a/src/logging/virtlogd.sysconf
+++ b/src/logging/virtlogd.sysconf
@@ -1,3 +1,3 @@
#
# Pass extra arguments to virtlogd
-#VIRTLOGD_ARGS=
+VIRTLOGD_ARGS="--timeout 120"
--
2.24.1
4 years, 8 months
