It should be impossible to lack an alias in the domain definition. Other
disk types don't generate it so remove it here as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add test data for nested backing chains with/without indexes (used in
status XMLs) which will excercise blockdev and the related work.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The blockdev support will change existing approach to add disks to VMs
so all tests using the DO_TEST_CAPS_LATEST approach which have any disks
need to be forked so that the changes can be applied.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'device' field reported by 'query-block' is empty when -blockdev is
used. Add an argument which will allow matching disk by using the qdev
id so we can use this code with -blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'device' argument matches only the legacy drive alias. For blockdev
we need to set the throttling for a QOM id and thus we'll need to use
the 'id' field.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The wrapper executes the command and does error detection so there's no
need to open-code all of those things.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the preparation steps from qemuDomainAttachDiskGeneric up into
qemuDomainAttachDeviceDiskLive so that also media changing can use the
prepared file.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use qemuDomainAttachDeviceDiskLive to change the media in
qemuDomainChangeDiskLive as the former function already does all the
necessary steps to prepare the new medium.
This also allows us to turn qemuDomainChangeEjectableMedia static.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Turns out that 'query-nodes' is not what we want and the
'query-blockstats' command was in fact buggy. Revert the new field since
it's not needed.
This reverts commit 50edca1331.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We don't use it for anything useful so it does not make much sense to
extract it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU supports 'block_resize' since 0.14 so we don't need to do explicit
checking. Additionally the caller did not use the different value at
all.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the pointless "empty path" check and use a better error message
if the disk was not found.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Print the differences in case when the expected data does not match.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently we'd report the alias of the drive which is backing the cdrom
rather than the device itself:
$ virsh event ds tray-change --loop
event 'tray-change' for domain ds disk drive-ide0-0-1: opened
event 'tray-change' for domain ds disk drive-ide0-0-1: closed
Report the disk device alias as we document in the API docs:
https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectDomainEventTrayChangeCallback
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1610072
Due to historical reasons we were not parsing device info on
guestfwd channel. Sure, it doesn't make much sense to parse
<address/> but it surely makes sense to parse its alias (which
might be an user alias).
This reverts commit 47a3dd46ea
which fixed https://bugzilla.redhat.com/show_bug.cgi?id=1172526.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Since we're not saving the platform-specific data into a cache, we're
not going to populate the structure, which in turn will cause a crash
upon calling virNodeGetSEVInfo because of a NULL pointer dereference.
Ultimately, we should start caching this data along with host-specific
capabilities like NUMA and SELinux stuff into a separate cache, but for
the time being, this is a semi-proper fix for a potential crash.
Backtrace (requires libvirtd restart to load qemu caps from cache):
#0 qemuGetSEVInfoToParams
#1 qemuNodeGetSEVInfo
#2 virNodeGetSEVInfo
#3 remoteDispatchNodeGetSevInfo
#4 remoteDispatchNodeGetSevInfoHelper
#5 virNetServerProgramDispatchCall
#6 virNetServerProgramDispatch
#7 virNetServerProcessMsg
#8 virNetServerHandleJob
#9 virThreadPoolWorker
#10 virThreadHelper
https: //bugzilla.redhat.com/show_bug.cgi?id=1612009
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
So the procedure to detect SEV support works like this:
1) we detect that sev-guest is among the QOM types and set the cap flag
2) we probe the monitor for SEV support
- this is tricky, because QEMU with compiled SEV support will always
report -object sev-guest and query-sev-capabilities command, that
however doesn't mean SEV is supported
3) depending on what the monitor returned, we either keep or clear the
capability flag for SEV
Commit a349c6c21c added an explicit check for "GenericError" in the
monitor reply to prevent libvirtd to spam logs about missing
'query-sev-capabilities' command. At the same time though, it returned
success in this case which means that we didn't clear the capability
flag afterwards and happily formatted SEV into qemuCaps. Therefore,
adjust all the relevant callers to handle -1 on errors, 0 on SEV being
unsupported and 1 on SEV being supported.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
Keep with the recent effort of replacing as many explicit *Free
functions with their automatic equivalents.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
In order to test SEV we need real QEMU capabilities. Ideally, this would
be tested with -latest capabilities, however, our capabilities are
currently tied to Intel HW, even the 2.12.0 containing SEV were edited by
hand, so we can only use that one for now, as splitting the capabilities
according to the vendor is a refactor for another day. The need for real
capabilities comes from the extended SEV platform data (PDH, cbitpos,
etc.) we'll need to cache/parse.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
commit 5c81c342a7 forgot to skip the detaching of the shmem backend
when async unplug is requested which meant that we've tried to unplug
the backend prior to delivery of the DEVICE_DELETED event.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1618622
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Qemu-3.0 supports Hyper-V-style PV TLB flush, Windows guests can benefit
from this feature as KVM knows which vCPUs are not currently scheduled (and
thus don't require any immediate action).
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Qemu-3.0 supports so-called 'Reenlightenment' notifications and this (in
conjunction with 'hv-frequencies') can be used make Hyper-V on KVM pass
stable TSC page clocksource to L2 guests.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Qemu-2.12 gained 'hv-frequencies' cpu flag to enable Hyper-V frequency
MSRs. These MSRs are required (but not sufficient) to make Hyper-V on
KVM pass stable TSC page clocksource to L2 guests.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
There are some path where the buffer is not passed to
virCommandAddArgBuffer and therefore the buffer might leak.
==191201== 1,010 bytes in 1 blocks are definitely lost in loss record 826 of 836
==191201== at 0x4C2CE3F: malloc (vg_replace_malloc.c:298)
==191201== by 0x4C2F1BF: realloc (vg_replace_malloc.c:785)
==191201== by 0x5D39E82: virReallocN (viralloc.c:245)
==191201== by 0x5D3E8F2: virBufferGrow (virbuffer.c:150)
==191201== by 0x5D3E9C8: virBufferAdd (virbuffer.c:185)
==191201== by 0x56EAC98: qemuBuildFloppyCommandLineControllerOptions (qemu_command.c:2162)
==191201== by 0x56EB3E1: qemuBuildDisksCommandLine (qemu_command.c:2370)
==191201== by 0x570055E: qemuBuildCommandLine (qemu_command.c:10315)
==191201== by 0x575EA7F: qemuProcessCreatePretendCmd (qemu_process.c:6777)
==191201== by 0x113DAB: testCompareXMLToArgv (qemuxml2argvtest.c:598)
==191201== by 0x13A75B: virTestRun (testutils.c:180)
==191201== by 0x138BE8: mymain (qemuxml2argvtest.c:2975)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Libvirt now tries to preserve all mounts under /dev in qemu namespaces.
The old rules only listed a set of known paths but those are no more enough.
I found some due to containers like /dev/.lxc/* and such but also /dev/console
and /dev/net/tun.
Libvirt is correct to do so, but we can no more predict the names properly, so
we modify the rule to allow a wildcard based pattern matching what libvirt does.
Acked-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Several cases were found needing /tmp, for example ceph will try to list /tmp
This is a compromise of security and usability:
- we only allow generally enumerating the base dir
- enumerating anything deeper in the dir is at least guarded by the
"owner" restriction, but while that protects files of other services
it won't protect qemu instances against each other as they usually run
with the same user.
- even with the owner restriction we only allow read for the wildcard
path
Acked-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
If a guest runs unconfined <seclabel type='none'>, but libvirtd is
confined then the peer for signal can only be detected as
'unconfined'. That triggers issues like:
apparmor="DENIED" operation="signal"
profile="/usr/sbin/libvirtd" pid=22395 comm="libvirtd"
requested_mask="send" denied_mask="send" signal=term peer="unconfined"
To fix this add unconfined as an allowed peer for those operations.
I discussed with the apparmor folks, right now there is no better
separation to be made in this case. But there might be further down the
road with "policy namespaces with scope and view control + stacking"
This is more a use-case addition than a fix to the following two changes:
- 3b1d19e6 AppArmor: add rules needed with additional mediation features
- b482925c apparmor: support ptrace checks
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Acked-by: intrigeri <intrigeri+libvirt@boum.org>
virt-manager's UI connection will need socket access for openGraphicsFD
to work - otherwise users will face a failed connection error when
opening the UI view.
Depending on the exact versions of libvirt and qemu involved this needs
either a rule from qemu to libvirt or vice versa.
Acked-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Now that the virAuthGet*Path API's generate all the error messages
we can remove them from the callers.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Now that the virAuthGet*Path API's generate all the error messages
we can remove them from the callers.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Now that the virAuthGet*Path API's generate all the error messages
we can remove them from the callers. This means that we will no
longer overwrite the error from the API.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Now that the virAuthGet*Path API's generate all the error messages
we can remove them from the callers. This means that we will no
longer overwrite the error from the API.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Now that the virAuthGet*Path API's generate all the error messages
we can remove them from the callers. This means that we will no
longer overwrite the error from the API.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Now that the virAuthGet*Path API's generate all the error messages
we can remove them from the callers. This means that we will no
longer overwrite the error from the API.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Rather than forcing the caller to generate an error, let's
generate the Username or Password error message failure if
the auth->cb fails. This is the last error path that needs
a specific message for various callers.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
If we never find the valid credtype in the list, then we'd return
NULL without an error signaled forcing the caller to generate one
that will probably be incorrect. Let's be specific.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Now that the virAuthGet*Path helpers make the checks, we can remove
them from here.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Before trying to call @auth->cb, let's ensure it exists.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Before trying to dereference @auth, let's ensure it's valid.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
The virSecretGetSecretString() helper looks up a secret for given
pool and returns its value in @secret_value and its length in
@secret_value_size. However, the trailing '\0' is not included in
either of the variables. This is because usually the value of the
secret is passed to some encoder (usually base64 encoder) where
the trailing zero must not be accounted for.
However, in two places we actually want the string as we don't
process it any further.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As advertised in the previous commit, we need the list of
accessed files to also contain action that caused the $path to
appear on the list. Not only this enables us to fine tune our
white list rules it also helps us to see why $path is reported.
For instance:
/run/user/1000/libvirt/libvirt-sock: connect: qemuxml2argvtest: QEMU XML-2-ARGV net-vhostuser-multiq
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The check-file-access.pl script is used to match access list
generated by virtestmock against whitelisted rules stored in
file_access_whitelist.txt. So far the rules are in form:
$path: $progname: $testname
This is not sufficient because the rule does not take into
account 'action' that caused $path to appear in the list of
accessed files. After this commit the rule can be in new form:
$path: $action: $progname: $testname
where $action is one from ("open", "fopen", "access", "stat",
"lstat", "connect"). This way the white list can be fine tuned to
allow say access() but not connect().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
So far we are setting only fake secret and storage drivers.
Therefore if the code wants to call a public NWFilter API (like
qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() are
doing) the virGetConnectNWFilter() function will try to actually
spawn session daemon because there's no connection object set to
handle NWFilter driver.
Even though I haven't experienced the same problem with the rest
of the drivers (interface, network and node dev), the reasoning
above can be applied to them as well.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
