Since qemu 2.9 via 9103f1ce "file-posix: Consider max_segments for
BlockLimits.max_transfer" this is a new access that is denied by the
qemu profile.
It is non fatal, but prevents the fix mentioned to actually work.
It should be safe to allow reading from that path.
Since qemu opens a symlink path we need to translate that for apparmor from
"/sys/dev/block/*/queue/max_segments" to
"/sys/devices/**/block/*/queue/max_segments"
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Peter Krempa [Fri, 3 Nov 2017 14:20:55 +0000 (15:20 +0100)]
tests: Add testing of storage backend JSON props formatter
Add a new test program called 'qemublocktest' to test the block layer
related stuff and test storage source to JSON generator by comparing it
to the JSON parser.
Peter Krempa [Mon, 23 Oct 2017 14:39:49 +0000 (16:39 +0200)]
storage: Don't store leading '/' in image name when splitting out volume
Libvirt historically stores storage source path including the volume as
one string in the XML, but that is not really flexible enough when
dealing with the fields in the code. Previously we'd store the slash
separating the two as part of the image name. This was fine for gluster
but it's not necessary and does not scale well when converting other
protocols.
Don't store the slash as part of the path. The resulting change from
absolute to relative path within the gluster driver should be okay,
as the root directory is the default when accessing gluster.
Peter Krempa [Thu, 13 Jul 2017 13:38:50 +0000 (15:38 +0200)]
qemu: process: Split out useful parts from qemuBuildNetworkDriveURI
Extract the part formatting the basic URI part so that it can be reused
to format JSON backing definitions. Parts specific to the command line
format will remain in qemuBuildNetworkDriveURI. The new function is
called qemuBlockStorageSourceGetURI.
Peter Krempa [Mon, 23 Oct 2017 16:02:28 +0000 (18:02 +0200)]
qemu: block: Use proper type for servers for VxHS disks
Original implementation used 'SocketAddress' equivalent from qemu for
the disk server field, while qemu documentation specifies
'InetSocketAddress'. The backing store parser uses the correct parsing
function but the formatter used the incorrect one (and also with the
legacy mode enabled which was wrong).
Peter Krempa [Wed, 1 Nov 2017 10:17:20 +0000 (11:17 +0100)]
qemu: command: Refactor qemuBuildDriveStrValidate to make qemuCaps optional
To allow merging this with other disk type checks we need to check
qemuCaps only when available, since some of the checks are executed on
disk cold-plug and thus capabilities should not be checked.
Make the checks optional by making them conditional on qemuCaps not
being NULL.
Peter Krempa [Wed, 1 Nov 2017 10:02:41 +0000 (11:02 +0100)]
qemu: command: Directly report bus type in qemuBuildDriveStrValidate
All of the error message are already in a conditional block with known
bus type. Inline the bus type rather than formatting it from a separate
variable.
Peter Krempa [Wed, 1 Nov 2017 09:41:55 +0000 (10:41 +0100)]
qemu: command: Move disk index validation closer to usage
The disk index validation is used only in very specific cases and does
not need to be performed otherwise. Move it out of the global check into
the usage place.
Peter Krempa [Wed, 1 Nov 2017 09:33:24 +0000 (10:33 +0100)]
qemu: command: Remove dead code when formatting -drive
busid and unitid are ever used only if the device is an SD card due to
the check in qemuDiskBusNeedsDeviceArg. Since the SD card does not have
an bus or unit number, most of the code and command line formatter can
be removed since it will never be used.
Michal Privoznik [Mon, 23 Oct 2017 09:33:06 +0000 (11:33 +0200)]
qemu: Move memPath generation from memoryBackingDir to a separate function
In near future we will need more than just a plain VIR_STRDUP().
Better implement that in a separate function and in
qemuBuildMemoryBackendStr() which is complicated enough already.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
rpc,lockd: Add missing netserver refcount increment on reload
After the virNetDaemonAddServerPostExec call in virtlogd we should have
netserver refcount set to 2. One goes to netdaemon servers hashtable
and one goes to virt{logd,lock} own reference to netserver. Let's add
the missing increment in virNetDaemonAddServerPostExec itself while
holding the daemon lock.
Since lockd defers management of the @srv object by the presence
in the hash table, virLockDaemonNewPostExecRestart must Unref the
alloc'd Ref on the @srv object done as part of virNetDaemonAddServerPostExec
and virNetServerNewPostExecRestart processing. The virNetDaemonGetServer
in lock_daemon main will also take a reference which is Unref'd during
main cleanup.
John Ferlan [Sat, 28 Oct 2017 21:03:20 +0000 (17:03 -0400)]
lockd: Need to Unref @srv when done with it.
Commit id '252610f7d' used a hash table to store the @srv, but
didn't handle the virObjectUnref if virNetDaemonNew failed nor
did it use virObjectUnref once successfully placed into the table
which will now be managing it's lifetime (and would cause the
virObjectRef if successfully inserted into the table).
Jiri Denemark [Thu, 2 Nov 2017 18:58:00 +0000 (19:58 +0100)]
conf: Don't inline virDomainNetTypeSharesHostView
When coverage build is enabled, gcc complains about it:
In file included from qemu/qemu_agent.h:29:0,
from qemu/qemu_driver.c:47:
qemu/qemu_driver.c: In function 'qemuDomainSetInterfaceParameters':
./conf/domain_conf.h:3397:1: error: inlining failed in call to
'virDomainNetTypeSharesHostView': call is unlikely and code size would
grow [-Werror=inline]
virDomainNetTypeSharesHostView(const virDomainNetDef *net)
^
Michal Privoznik [Tue, 31 Oct 2017 10:47:36 +0000 (11:47 +0100)]
virsh: Define multi line macros properly
In some cases there's dangling backward slash at the end of multi
line macros. While technically the code works, it will stop if
some empty lines are removed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Dawid Zamirski [Tue, 24 Oct 2017 19:35:31 +0000 (15:35 -0400)]
vbox: Add more IStorageController API mappings
This patch exposes additional methods of the native VBOX API to the
libvirt 'unified' vbox API to deal with IStorageController. The exposed
methods are:
Dawid Zamirski [Tue, 24 Oct 2017 19:35:30 +0000 (15:35 -0400)]
vbox: Support empty removable drives.
Original code was checking for non empty disk source before proceeding
to actually attach disk device to VM. This prevented from creating
empty removable devices like DVD or floppy. Therefore, this patch
re-organizes the loop work-flow to allow such configurations as well as
makes the code follow better libvirt practices. Additionally, adjusted
debug logs to be more helpful - removed old ones and added new which
give more valuable info for troubleshooting.
Dawid Zamirski [Tue, 24 Oct 2017 19:35:29 +0000 (15:35 -0400)]
vbox: Errors in vboxAttachDrives are now critical
Previously, if one tried to define a VBOX VM and the API failed to
perform the requested actions for some reason, it would just log the
error and move on to process remaining disk definitions. This is not
desired as it could result in incorrectly defined VM without the caller
even knowing about it. So now all the code paths that call
virReportError are now treated as hard failures as they should have
been.
Dawid Zamirski [Fri, 3 Nov 2017 11:49:32 +0000 (07:49 -0400)]
vbox: Remove unused mediumEmpty
Remove the setting since it's unused as of commit 34364df3 which should
have never copied it in from the old code which ended up getting removed
as part of commit c7c286c6.
Dawid Zamirski [Tue, 24 Oct 2017 19:35:28 +0000 (15:35 -0400)]
vbox: Cleanup vboxAttachDrives implementation
This commit primes vboxAttachDrives for further changes so when they
are made, the diff is less noisy:
* move variable declarations to the top of the function
* add disk variable to replace all the def->disks[i] instances
* add cleanup at the end of the loop body, so it's all in one place
rather than scattered through the loop body. It's purposefully
called 'cleanup' rather than 'skip' or 'continue' because future
commit will treat errors as hard-failures.
Dawid Zamirski [Tue, 24 Oct 2017 19:35:27 +0000 (15:35 -0400)]
vbox: vboxAttachDrives now relies on address info
Previously, the driver was computing VBOX's devicePort/deviceSlot values
based on device name and max port/slot values. While this worked, it
completely ignored <address> values. Additionally, libvirt's built-in
virDomainDiskDefAssignAddress already does a good job setting default
values on virDomainDeviceDriveAddress struct which we can use to set
devicePort and deviceSlot and accomplish the same result while allowing
the customizing those via XML. Also, this allows to remove some code
which will make further patches smaller.
Dawid Zamirski [Tue, 24 Oct 2017 19:35:25 +0000 (15:35 -0400)]
vbox: Close media when undefining domains
When registering a VM we call OpenMedium on each disk image which adds it
to vbox's global media registry. Therefore, we should make sure to call
Close when unregistering VM so we cleanup the media registry entries
after ourselves - this does not remove disk image files. This follows
the behaviour of the VBoxManage unregistervm command.
Right-aligning backslashes when defining macros or using complex
commands in Makefiles looks cute, but as soon as any changes is
required to the code you end up with either distractingly broken
alignment or unnecessarily big diffs where most of the changes
are just pushing all backslashes a few characters to one side.
Generated using
$ git grep -El '[[:blank:]][[:blank:]]\\$' | \
grep -E '*\.([chx]|am|mk)$$' | \
while read f; do \
sed -Ei 's/[[:blank:]]*[[:blank:]]\\$/ \\/g' "$f"; \
done
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Peter Krempa [Fri, 29 Sep 2017 10:02:29 +0000 (12:02 +0200)]
qemu: domain: skip chain detection to end of backing chain
When a user provides the backing chain, we will not need to re-detect
all the backing stores again, but should move to the end of the user
specified chain. Additionally if a user provides a full terminated chain
we should not attempt any further detection.
Peter Krempa [Fri, 20 Oct 2017 11:50:23 +0000 (13:50 +0200)]
qemu: domain: Extract setup for disk source secrets
Separate it so that it deals only with single virStorageSource, so that
it can later be reused for full backing chain support.
Two aliases are passed since authentication is more relevant to the
'storage backend' whereas encryption is more relevant to the protocol
layer. When using node names, the aliases will be different.
Peter Krempa [Mon, 16 Oct 2017 12:10:09 +0000 (14:10 +0200)]
qemu: domain: Simplify using DAC permissions of top of backing chain
qemuDomainGetImageIds and qemuDomainStorageFileInit are helpful when
trying to access a virStorageSource from the qemu driver since they
figure out the correct uid and gid for the image.
When accessing members of a backing chain the permissions for the top
level would be used. To allow using specific permissions per backing
chain level but still allow inheritance from the parent of the chain we
need to add a new parameter to the image ID APIs.
Peter Krempa [Tue, 17 Oct 2017 06:03:42 +0000 (08:03 +0200)]
security: selinux: Take parent security label into account
Until now we ignored user-provided backing chains and while detecting
the code inherited labels of the parent device. With user provided
chains we should keep this functionality, so label of the parent image
in the backing chain will be applied if an image-specific label is not
present.
Peter Krempa [Tue, 17 Oct 2017 06:03:42 +0000 (08:03 +0200)]
security: dac: Take parent security label into account
Until now we ignored user-provided backing chains and while detecting
the code inherited labels of the parent device. With user provided
chains we should keep this functionality, so label of the parent image
in the backing chain will be applied if an image-specific label is not
present.
Peter Krempa [Tue, 17 Oct 2017 05:25:51 +0000 (07:25 +0200)]
security: selinux: Pass parent storage source into image labeling helper
virSecuritySELinuxSetImageLabelInternal assigns different labels to
backing chain members than to the parent image. This was done via the
'first' flag. Convert it to passing in pointer to the parent
virStorageSource. This will allow us to use the parent virStorageSource
in further changes.
When the user provides backing chain, we don't need the full support for
traversing the backing chain. This patch adds a feature check for the
virStorageSourceAccess API.
Peter Krempa [Mon, 16 Oct 2017 11:23:51 +0000 (13:23 +0200)]
storage: Extract common code to retrieve driver backend for support check
The 'file access' module of the storage driver has few feature checks to
determine whether libvirt supports given storage driver method. The code
to retrieve the driver struct needed for the check is the same so it can
be extracted.
Jiri Denemark [Thu, 26 Oct 2017 19:51:37 +0000 (21:51 +0200)]
qemu: Add support for block-incremental migration parameter
We handle incremental storage migration in a different way. The support
for this new (as of QEMU 2.10) parameter is only needed for full
coverage of migration parameters used by QEMU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Jiri Denemark [Thu, 26 Oct 2017 18:47:16 +0000 (20:47 +0200)]
qemu: Add support for max-bandwidth migration parameter
We already support several ways of setting migration bandwidth and this
is not adding another one. With this patch we are able to read and write
this parameter using query-migrate-parameters and migrate-set-parameters
in one call with all other parameters.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Jiri Denemark [Thu, 26 Oct 2017 18:11:47 +0000 (20:11 +0200)]
qemu: Rename TLS related migration parameters
The parameters used "migrate" prefix which is pretty redundant and
qemuMonitorMigrationParams structure is our internal representation of
QEMU migration parameters and it is supposed to use names which match
QEMU names.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Jiri Denemark [Mon, 23 Oct 2017 14:55:02 +0000 (16:55 +0200)]
qemu: Add support for setting downtime-limit migration parameter
We already support setting the maximum downtime with a dedicated
virDomainMigrateSetMaxDowntime API. This patch does not implement
another way of setting the downtime by adding a new public migration
parameter. It just makes sure any parameter we are able to get from a
QEMU monitor by query-migrate-parameters can be passed back to QEMU via
migrate-set-parameters.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Jiri Denemark [Thu, 26 Oct 2017 18:50:35 +0000 (20:50 +0200)]
qemumonitorjsontest: Rename 1st CHECK macro in migration params test
The first CHECK macro in the test is used for checking integer values.
Let's make it a bit more generic to be usable for any numeric type and
use it for a new CHECK_INT macro.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Jiri Denemark [Wed, 1 Nov 2017 17:35:57 +0000 (18:35 +0100)]
cputest: Skip tests requiring JSON_MODELS if QEMU is disabled
Some tests require JSON_MODELS to be parsed into qemuCaps and applied
when computing CPU models and such test cannot succeed if QEMU driver is
disabled. Let's mark the tests with JSON_MODELS_REQUIRED and skip the
appropriate parts if building without QEMU.
On the other hand, CPU tests with JSON_MODELS should succeed even if
model definitions from QEMU are not parsed and applied. Let's explicitly
test this by repeating the tests without JSON_MODELS set.
This fixes the build with QEMU driver disabled, e.g., on some
architectures on RHEL/CentOS.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Jiri Denemark [Mon, 23 Oct 2017 11:06:44 +0000 (13:06 +0200)]
spec: Restart libvirtd in posttrans
When upgrading libvirt packages, there's no strict ordering for the
installation or removal of the individual libvirt sub packages. Thus
libvirt-daemon may be upgraded (and its %postun scriptlet) started
before all sub packages with driver libraries are upgraded. When
libvirt-daemon's %postun scriptlet restarts the daemon old drivers may
still be laying around and the daemon may crash when it tries to use
them.
Let's restart the daemon in %posttrans to make sure libvirtd is
restarted only after all sub packages are at the same version.
Michal Privoznik [Wed, 25 Oct 2017 10:42:41 +0000 (12:42 +0200)]
qemuDomainSetInterfaceParameters: Explicitly reject unsupported net types
For instance, NET_TYPE_MCAST doesn't support setting QoS. Instead
of claiming success and doing nothing, we should be explicit
about that and report an error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Use virStorageBackendCreateVolUsingQemuImg to apply the LUKS information
to the logical volume just created. As part of the processing of the
lvcreate command add 2MB to the capacity to account for the LUKS header
when it's determined that the volume desires to use encryption.
John Ferlan [Fri, 6 Oct 2017 16:45:01 +0000 (12:45 -0400)]
storage: Extract out the LVCREATE
Refactor to extract out the LVCREATE command. This also removes the
need for the local @created since the error path can now only be reached
after the creation of the logical volume.
Turns out the virStorageBackendVolResizeLocal did not differentiate
whether the target volume was a LUKS volume or not and just blindly
did the ftruncate() on the target volume.
Follow the volume creation logic (in general) and create a qemu-img
resize command to resize the target volume for LUKS ensuring that
the --object secret is provided as well as the '--image-opts' used
by the qemu-img resize logic to describe the path and secret ensuring
that it's using the luks driver on the volume of course.
John Ferlan [Fri, 6 Oct 2017 15:06:13 +0000 (11:06 -0400)]
storage: Alter storageBackendCreateQemuImgSecretObject args
Since all that was really needed was a couple of fields and building
the object can be more generic, let's alter the args a bit. This will
be useful shortly for adding the secret object for a volume resize
operation on a luks volume that will need a secret object.
Marc Hartmayer [Fri, 27 Oct 2017 07:53:03 +0000 (09:53 +0200)]
qemu: Fix memory leak in virQEMUCapsLoadCPUModels
Don't leak @blockNodes in the loop.
==226576== 7,120 bytes in 60 blocks are definitely lost in loss record 122 of 125
==226576== at 0x4835214: calloc (vg_replace_malloc.c:711)
==226576== by 0x4950D7B: virAllocN (viralloc.c:191)
==226576== by 0x49EB5BB: virXPathNodeSet (virxml.c:676)
==226576== by 0x104DB67: virQEMUCapsLoadCPUModels (qemu_capabilities.c:3738)
==226576== by 0x105510D: virQEMUCapsLoadCache (qemu_capabilities.c:3929)
==226576== by 0x104459F: qemuTestParseCapabilities (testutilsqemu.c:498)
==226576== by 0x1040DC9: testQemuCapsCopy (qemucapabilitiestest.c:105)
==226576== by 0x1041F07: virTestRun (testutils.c:180)
==226576== by 0x1040B45: mymain (qemucapabilitiestest.c:181)
==226576== by 0x104320F: virTestMain (testutils.c:1119)
==226576== by 0x1041149: main (qemucapabilitiestest.c:193)
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Jim Fehlig [Thu, 26 Oct 2017 17:23:07 +0000 (11:23 -0600)]
daemon: remove minsize directive from hypervisor logrotate files
Commit 6c43149c removed the minsize directive from the qemu logrotate
file but missed other hypervisors. Remove minsize from the libxl, lxc,
and uml logrotate files as well.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Jim Fehlig [Wed, 25 Oct 2017 21:09:20 +0000 (15:09 -0600)]
qemu: logrotate: drop minsize directive
On a cloud host it is possible to create 100's of unique instances
per day, each leaving behind a /var/log/libvirt/qemu/instance-name.log
file that is < 100k. With the current 'minsize 100k' directive, these
files are never rotated and hence never removed. Over months of time,
tens of thousands of these files can accumulate on the host.
Dropping 'minsize 100k' allows rotating small files, which will
increase the number of log files, but 'rotate 4' ensures they will
be removed after a month.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
virt-aa-helper: fix libusb access to udev usb descriptions
In bf3a4140 "virt-aa-helper: fix libusb access to udev usb data" the
libusb access to properly detect the device/bus ids was fixed.
The path /run/udev/data/+usb* contains a subset of that information we
already allow to be read and are currently not needed for the function
qemu needs libusb for. But on the init of libusb all those files are
still read so a lot of apparmor denials can be seen when using usb host
devices, like:
apparmor="DENIED" operation="open" name="/run/udev/data/+usb:2-1.2:1.0"
comm="qemu-system-x86" requested_mask="r" denied_mask="r"
Today we could silence the warnings with a deny rule without breaking
current use cases. But since the data in there is only a subset of those
it can read already it is no additional information exposure. And on the
other hand a future udev/libusb/qemu combination might need it so allow
the access in the default apparmor profile.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Hot-adding disks does not parse the full XML to generate apparmor rules.
Instead it uses -f <PATH> to append a generic rule for that file path.
580cdaa7: "virt-aa-helper: locking disk files for qemu 2.10" implemented
the qemu 2.10 requirement to allow locking on disks images that are part of
the domain xml.
But on attach-device a user will still trigger an apparmor deny by going
through virt-aa-helper -f, to fix that add the lock "k" permission to the
append file case of virt-aa-helper.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
The initial assumption was ~2 files per guest, but some common setups
like Openstack drive up to 4 files per guest.
E.g. on Arm where the following XML leads to 4 file handles:
<serial type='file'>
<source path='/var/lib/nova/instances/7c0dcd78-.../console.log'/>
<target port='0'/>
<alias name='serial0'/>
</serial>
<console type='file'>
<source path='/var/lib/nova/instances/7c0dcd78-.../console.log'/>
<target type='serial' port='0'/>
<alias name='serial0'/>
</console>
With that in mind and the target to support 4k guests by default we
should raise the limit to 16k.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>