If we use fake reboot then domain goes thru running->shutdown->running
state changes with shutdown state only for short period of time.  At
least this is implementation details leaking into API. And also there is
one real case when this is not convinient. I'm doing a backup with the
help of temporary block snapshot (with the help of qemu's API which is
used in the newly created libvirt's backup API). If guest is shutdowned
I want to continue to backup so I don't kill the process and domain is
in shutdown state. Later when backup is finished I want to destroy qemu
process. So I check if it is in shutdowned state and destroy it if it
is. Now if instead of shutdown domain got fake reboot then I can destroy
process in the middle of fake reboot process.

After shutdown event we also get stop event and now as domain state is
running it will be transitioned to paused state and back to running
later. Though this is not critical for the described case I guess it is
better not to leak these details to user too. So let's leave domain in
running state on stop event if fake reboot is in process.

Reconnection code handles this patch without modification. It detects
that qemu is not running due to shutdown and then calls qemuProcessShutdownOrReboot
which reboots as fake reboot flag is set.
Signed-off-by: NNikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

The 'cleanup' flag is doing no cleaup in this function. We can
remove it and return NULL on error or qemuBuildCommandLine().
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

The g_auto*() changes made by the previous patches made a lot
of 'cleanup' labels obsolete. Let's remove them.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Change all feasible pointers to use g_autoptr().
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Change all feasible strings and scalar pointers to use g_autofree.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Now, that we have everything prepared, we can generate command
line for NVMe disks.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

This reverts commit 7be5fe66.

This commit broke resctrl, because it missed the fact that the
virResctrlInfoGetCache() has side-effects causing it to actually
change the virResctrlInfo parameter, not merely get data from
it.

This code will need some refactoring before we can try separating
it from virCapabilities again.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

When QEMU uid/gid is set to non-root this is pointless as if we just
used a regular setuid/setgid call, the process will have all its
capabilities cleared anyway by the kernel.

When QEMU uid/gid is set to root, this is almost (always?) never
what people actually want. People make QEMU run as root in order
to access some privileged resource that libvirt doesn't support
yet and this often requires capabilities. As a result they have
to go find the qemu.conf param to turn this off. This is not
viable for libguestfs - they want to control everything via the
XML security label to request running as root regardless of the
qemu.conf settings for user/group.

Clearing capabilities was implemented originally because there
was a proposal in Fedora to change permissions such that root,
with no capabilities would not be able to compromise the system.
ie a locked down root account. This never went anywhere though,
and as a result clearing capabilities when running as root does
not really get us any security benefit AFAICT. The root user
can easily do something like create a cronjob, which will then
faithfully be run with full capabilities, trivially bypassing
the restriction we place.

IOW, our clearing of capabilities is both useless from a security
POV, and breaks valid use cases when people need to run as root.

This removes the clear_emulator_capabilities configuration
option from qemu.conf, and always runs QEMU with capabilities
when root.  The behaviour when non-root is unchanged.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

We will want to use the async job infrastructure along with all the APIs
and event for the backup job so add the backup job as a new async job
type.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

We always refresh the capabilities object when using virResctrlInfo
during process startup. This is undesirable overhead, because we can
just directly create a virResctrlInfo instead.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Avoid grabbing the whole virCapsPtr object when we only need the
host CPU information.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Avoid grabbing the whole virCapsPtr object when we only need the
NUMA information.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The NUMA cells are stored directly in the virCapsHostPtr
struct. This moves them into their own struct allowing
them to be stored independantly of the rest of the host
capabilities. The change is used as an excuse to switch
the representation to use a GPtrArray too.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Now that the domain XML APIs don't use virCapsPtr we can stop passing it
around many QEMU driver methods.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Our normal practice is for the object type to be the name prefix, and
the object instance be the first parameter passed in.

Rename these to virDomainObjSave and virDomainDefSave moving their
primary parameter to be the first one. Ensure that the xml options
are passed into both functions in prep for future work.

Finally enforce checking of the return type and mark all parameters
as non-NULL.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

As part of a goal to eliminate the need to use virCapsPtr for anything
other than the virConnectGetCapabilies() API impl, cache the host arch
against the QEMU driver struct and use that field directly.

In the tests we move virArchFromHost() globally in testutils.c so that
every test runs with a fixed default architecture reported.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

In v5.9.0-370-g8fa0374c I've tried to fix a bug by removing
some stale XATTRs in qemuProcessStop(). However, I forgot to
do nothing when the VIR_QEMU_PROCESS_STOP_NO_RELABEL flag was
specified.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

We clear some capabilities here so the lockouts need to be
re-evaluated.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

Do all post-processing of capabilities in qemuProcessPrepareQEMUCaps.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

Move the post-processing of the QEMU_CAPS_CHARDEV_FD_PASS flag to the
new function.

The clearing of the capability is based on the presence of
VIR_QEMU_PROCESS_START_STANDALONE so we must also pass in the process
start flags.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

Start aggregating all capability post-processing code in one place.

The comment was modified while moving it as it was mentioning floppies
which are no longer clearing the blockdev capability.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The function is now used only in qemu_process.c so move it there and
name it 'qemuProcessPrepareQEMUCaps' which is more appropriate to what
it's doing.

The reworded comment now mentions that it will also post-process the
caps for VM startup.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The redetection was originally added in 43c01d38 as a way to recover
from libvirtd upgrade from the time when we didn't persist the qemu
capabilities in the status XML. Also this the oldest supported qemu by
more than two years.

Even if somebody would have a running VM running at least qemu 1.5 with
such an old libvirt we certainly wouldn't do the right thing by
redetecting the capabilities and then trying to communicate with qemu.

For now it will be the best to just stop considering this scenario any
more and error out for such VM.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Commit c90fb5a8 added explicit use of the private copy of the qemu
capabilities to various places. The change to qemuProcessInit was bogus
though as at the point where we re-initiate the post parse callbacks
priv->qemuCaps is still NULL as we clear it after shutdown of the VM and
don't initiate it until a later point.

Using the value from priv->qemuCaps might mislead readers of the code
into thinking that something useful is being passed at that point so go
with an explicit NULL instead.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Block jobs may be members of async jobs so it makes more sense to
refresh block job state after we do steps for async job recovery.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

This also isn't required (due to the vportprofile being stored in the
NetDef as a pointer rather than being directly contained), but it
seemed dishonest to not mark it as const (and thus permit users to
modify its contents)
Signed-off-by: NLaine Stump <laine@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

If user starts a blockcommit or a blockcopy then we modify access
for qemu on both images and leave it like that until the job
terminates.  So far so good. Problem is, if user instead of
terminating the job (where we would modify the access again so
that the state before the job is restored) calls destroy on the
domain or if qemu dies whilst executing the block job.  In this
case we don't ever clear the access we granted at the beginning.
To fix this, maybe a bit harsh approach is used, but it works:
after all labels were restored (that is after
qemuSecurityRestoreAllLabel() was called), we iterate over each
disk in the domain and remove XATTRs from the whole backing chain
and also from any file the disk is being mirrored to.

This would have been done at the time of pivot, but it isn't
because user decided to kill the domain instead. If we don't do
this and leave some XATTRs behind the domain might be unable to
start.

Also, secdriver can't do this because it doesn't know if there is
any job running. It's outside of its scope - the hypervisor
driver is responsible for calling secdriver's APIs.

Moreover, this is safe to call because we don't remember labels
for any member of a backing chain except of the top layer. But
that one was restored in qemuSecurityRestoreAllLabel() call done
earlier. Therefore, not only we don't remember labels (and thus
this is basically a NOP for other images in the backing chain) it
is also safe to call this when no blockjob was started in the
first place, or if some parts of the backing chain are shared
with some other domains - this is NOP, unless a block job is
active at the time of domain destroy.

https://bugzilla.redhat.com/show_bug.cgi?id=1741456#c19Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

Install the convertor function which enables the internals that will use
-blockdev to make qemu open the firmware image and stop using -drive.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

The functions return virDomainCapsCPUModelsPtr and thus they should be
called *CPUModels for consistency. Functions called *CPUDefinitions will
work on qemuMonitorCPUDefsPtr.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The function would return a valid virDomainCapsCPUModelsPtr with empty
CPU models list if query-cpu-definitions exists in QEMU, but returns
GenericError meaning it's not in fact implemented. This behaviour is a
bit strange especially after such virDomainCapsCPUModels structure is
stored in capabilities XML and parsed back, which will result in NULL
virDomainCapsCPUModelsPtr rather than a structure containing nothing.

Let's just keep virDomainCapsCPUModelsPtr NULL if the QMP command is not
implemented and change the return value to int so that callers can
easily check for failure or success.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Some callers of virQEMUCapsGetCPUDefinitions will need to filter the
returned list of CPU models. Let's add the filtering parameters directly
to virQEMUCapsGetCPUDefinitions to avoid copying the CPU models list
twice.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Rather than returning a direct pointer the list stored in qemuCaps the
function now creates a new copy of the CPU models list.

The main purpose of this seemingly useless change is to update callers
to free the result returned by virQEMUCapsGetCPUDefinitions because the
internals of this function will change significantly in the following
patches.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The @def variable holds pointer to the domain defintion, but is
set only somewhere in the middle of the function. This is
suboptimal.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Commit <f136b831> reworked process
affinity setting but did not take cgroups into account which introduced
an issue when starting VM with custom cpuset.cpus for the whole machine
group.

If the machine group is limited to some pCPUs libvirt should not try to
set a VM to run on all pCPUs as it will result in permission denied when
writing to cpuset.cpus.

To fix this the affinity has to be set separately from cgroups cpuset.

Resolves: <https://bugzilla.redhat.com/show_bug.cgi?id=1746517>
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Prefer the GLib version to the one from gnulib.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

As suggested by Cole, this patch uses the domain capabilities to
validate the supported video model types. This allows us to remove the
model type validation from qemu_process.c and qemu_domain.c and
consolidates it all in a single place that will automatically adjust
when new domain capabilities are added.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NJonathon Jongsma <jjongsma@redhat.com>