The virIdentity getters are unusual in that they return -1 to indicate
"not found" and don't report any error. Change them to return -1 for
real errors, 0 for not found, and 1 for success.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

It is simpler to remove this unused method than to rewrite it using
typed parameters in the next patch.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Only expose the type safe getters/setters to other code in preparation
for changing the internal storage of data.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

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

Remove the "UNIX" tag from the names for user name, group name,
process ID and process time, since these attributes are all usable
for non-UNIX platforms like Windows.

User ID and group ID are left with a "UNIX" tag, since there's no
equivalent on Windows. The closest equivalent concept on Windows,
SID, is a struct containing a number of integer fields, which is
commonly represented in string format instead. This would require
a separate attribute, and is left for a future exercise, since
the daemons are not currently built on Windows anyway.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

When using the fine grained access control mechanism for APIs, when a
client connects to libvirtd, the latter will fetch the uid, gid, selinux
info of the remote client on the UNIX domain socket. This is then used
as the identity when checking ACLs.

With the new split daemons things are a bit more complicated. The user
can connect to virtproxyd, which in turn connects to virtqemud. When
virtqemud requests the identity over the UNIX domain socket, it will
get the identity that virtproxyd is running as, not the identity of
the real end user/application.

virproxyd knows what the real identity is, and needs to be able to
forward this information to virtqemud. The virConnectSetIdentity API
provides a mechanism for doing this. Obviously virtqemud should not
accept such identity overrides from any client, it must only honour it
from a trusted client, aka one running as the same uid/gid as itself.

The typed parameters exposed in the API are the same as those currently
supported by the internal virIdentity class, with a few small name
changes.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Most code paths prevent starting a blockjob if we already have one but
the job registering function does not do this check. While this isn't a
problem for regular cases we had a bad test case where we registered two
jobs for a single disk which leaked one of the jobs. Prevent this in the
registering function until we allow having multiple jobs per disk.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
ACKed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>

There were accidentally two disks with 'vdc' target with corresponding
blockjobs which made libvirt leak some references as there are not
supposed to be two blockjobs for a single disk. Fix this mess by
renaming some of the disks.

In addition the block job names also didn't correspond to the naming
convetion which also includes the disk target. Fix it as well.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
ACKed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>

qemuDomainAttachNetDevice() (hotplug) previously had some of the
validation that is in qemuDomainValidateActualNetDef(), but it was
incomplete. qemuDomainChangeNet() had none of that validation, but it
is all appropriate in both cases.

This is the final piece of a previously partial resolution to
https://bugzilla.redhat.com/1502754Signed-off-by: NLaine Stump <laine@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The same validation should be done for both static network devices and
hotplugged devices, but they are currently inconsistent. Move all the
relevant validation from qemuBuildInterfaceCommandLine() into the new
function qemuDomainValidateActualNetDef() and call the latter from
the former.
Signed-off-by: NLaine Stump <laine@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

It needs to be used by a function that only has a const pointer to
virDomainNetDef.
Signed-off-by: NLaine Stump <laine@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

AppArmorGetSecurityProcessLabel copies the VM's profile name to the
label member of virSecurityLabel struct. If the profile is not loaded,
the name is set empty before calling virStrcpy to copy it. However,
virStrcpy will fail if src is empty (0 length), causing
AppArmorGetSecurityProcessLabel to needlessly fail. Simple operations
that report security driver information will subsequently fail

virsh dominfo test
Id:             248
Name:           test
...
Security model: apparmor
Security DOI:   0
error: internal error: error copying profile name

Avoid copying an empty profile name when the profile is not loaded.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

To aid in troubleshooting add some debug messages wrt
bandwidth settings and networks.
Reviewed-by: NLaine Stump <laine@laine.org>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

We previously allowed bandwidth settings when attaching NICs
to networks with forward mode=bridge:

  commit 42a92ee9
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Tue Nov 20 11:30:05 2018 +0000

    network: add missing bandwidth limits for bridge forward type

    In the case of a network with forward=bridge, which has a bridge device
    listed, we are capable of setting bandwidth limits but fail to call the
    function to register them.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Unfortunately the wrong version of this patch was posted and
reviewed and thus it lacked the code to actually apply the
bandwidth settings to the bridge itself.
Reviewed-by: NLaine Stump <laine@laine.org>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Since the introduction of the virNetworkPort object, the network driver
has a persistent record of ports that have been created against the
networks. Thus the hypervisor drivers no longer communicate to the
network driver during libvirtd restart.

This change, however, meant that the connection usage counts were
no longer re-initialized during a libvirtd restart. To deal with this we
must iterate over all virNetworkPortDefPtr objects we have and invoke
the notify callback to record the connection usage count.
Reviewed-by: NLaine Stump <laine@laine.org>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The virTestOOMActive method was deleted in

  commit 2c52ecd9
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Thu Aug 29 13:04:07 2019 +0100

    util: purge all code for testing OOM handling
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

This fixes bug in

  commit bbe2aa62
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Thu Jul 26 17:24:30 2018 +0100

    conf: simplify link from hostdev back to network device

    hostdevs have a link back to the original network device. This is fairly
    generic accepting any type of device, however, we don't intend to make
    use of this approach in future. It can thus be specialized to network
    devices.
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

which mistakenly deleted the assignment to the 'net' variable,
which meant we never invoked the network driver release callback
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The functions are left returning an "int" to avoid an immediate
big-bang cleanup. They'll simply never return anything other
than 0.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Only a few of the _QUIET allocation macros are used. Since we're no
longer reporting OOM as errors, we want to eliminate all the _QUIET
variants. This starts with the easy, unused, cases.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The functions are left returning an "int" to avoid an immediate
big-bang cleanup. They'll simply never return anything other
than 0, except for virInsertN which can still return an error
if the requested insertion index is out of range. Interestingly
in that case, the _QUIET function would none the less report
an error.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The OOM handling requires special build time options which we never
enable in our CI. Even once enabled the tests are incredibly slow and
typically require manual inspection of the results to weed out false
positives.

Since there was previous agreement to switch to abort on OOM in libvirt
code, there's no point continuing to keep the unused OOM testing code.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The virNetworkPortDef config stores the 'managed' attribute
as the virTristateBool type.

The virDomainDef config stores the 'managed' attribute as
the bool type.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

If the hypervisor driver has not yet created the network port, the
portid field will be "00000000-0000-0000-0000-000000000000".

If a failure occurs during early VM startup, the hypervisor driver may
none the less try to release the network port, resulting in an
undesirable warning:

2019-09-12 13:17:42.349+0000: 16544: error :
virNetworkObjLookupPort:1679 : network port not found: Network port with
UUID 00000000-0000-0000-0000-000000000000 does not exist

By checking if the portid UUID is valid, we can avoid polluting the logs
in this way.
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>

The pci_dev->physical_function is rewritten in
virPCIGetPhysicalFunction() to a newly allocated pointer.
Therefore, we must free the old one to avoid memleak.
Signed-off-by: NJiang kun <jiang.kun2@zte.com.cn>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The LIBVIRT_RESULT function takes two or three arguments. The
first one is the name of the result (aka CHECK_NAME). It is
printed before the colon character. The rest of the arguments is
printed after the character. To produce colourized output a
couple of changes needs to be made.

Firstly, we need to print the CHECK_NAME using "echo -n" so that
the new line is not appended at the end of the message. To
achieve this, AS_MESSAGE_N function is introduced. It's a
verbatim copy of AS_MESSAGE (which is just another alias to
AC_MSG_NOTICE) except it doesn't put '\n' at the EOL.

The alias is defined at /usr/share/autoconf-*/autoconf/general.m4
and the AS_MESSAGE is then defined at
/usr/share/autoconf-2.69/m4sugar/m4sh.m4.

Secondly, the rest of the arguments are printed colourized and to
achieve that and also keep printing them into the log file the
_AS_ECHO and COLORIZE_RESULT functions need to be called.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

If we're running from a TTY we can put some colors around 'yes',
'no' and other messages.

Shamelessly copied from Ruby source code and modified a bit to
comply with syntax-check.

https://github.com/ruby/ruby/commit/e4879592873abd4cd8aeed56f4cbaa360a3d3736Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Now that we have qemuFirmwareGetSupported() so that it also
returns a list of FW image paths, we can use it to report them in
domain capabilities instead of the old time default list.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1733940Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

There is one hack hidden here, but since this is in a test, it's
okay. In order to get a list of expected firmwares in
virFirmwarePtr form I'm using virFirmwareParseList(). But
usually, in real life scenario, this function is used only to
parse a list of UEFI images which have NVRAM split out. In other
words, this function expects ${FW}:${NVRAM} pairs. But in this
test, we also want to allow just a single path: ${FW} because
some reported firmwares are just a BIOS image really. To avoid
writing some parser function, let's just pass "NULL" as ${NVRAM}
and fix the result later.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

The qemuFirmwareGetSupported() function is called from qemu
driver to generate domain capabilities XML based on FW descriptor
files. However, the function currently reports only some features
from domcapabilities XML and not actual FW image paths. The paths
reported in the domcapabilities XML are still from pre-FW
descriptor era and therefore the XML might be a bit confusing.
For instance, it may say that secure boot is supported but
secboot enabled FW is not in the listed FW image paths.

To resolve this problem, change qemuFirmwareGetSupported() so
that it also returns a list of FW images (we have the list
anyway). Luckily, we already have a structure to represent a FW
image - virFirmware.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1733940Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

This function is going to get some new arguments. Document the
current ones for clarity.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

This function frees a _virFirmware struct. So far, it doesn't
need to be called from outside of the module, but this will
change shortly. In the light of recent VIR_DEFINE_AUTOPTR_FUNC()
additions, do the same to virFirmwareFree().
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

The times, when we had small CRTs are long gone. Now, in the era
of wide screens we can be more generous when it comes to aligning
the output of configure. The longest string before the colon is
'wireshark_dissector' which counts 19 characters.  Therefore,
align the strings at 20.

At the same time, drop the useless result alignment. It behaves
oddly - it puts a space at the end of each "no" because of the
%-3s format we use.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

One of the advantages is that LIBVIRT_RESULT aligns the resulting
message for us.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

Rewrite some parts for clarity, elaborate the meaning of some of the XML
attributes.  And where necessary, distinguish that we're dealing with
two different XML documents here:

  - the domainCapabilities XML, to detect the host "hypervisor"
    (QEMU/KVM) capabilities, and what libvirt knows about them.

  - the guest XML definition, i.e. what features a guest can use, based
    on the capabilities (of QEMU and libvirt and the host) reported in
    the domainCapabilities XML.
Signed-off-by: NKashyap Chamarthy <kchamart@redhat.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Currently the RPM spec doesn't add the 'secboot'-variant OVMF binaries
(an unintentional omission, checking with Cole on #virt, OFTC) for
'x86_64' and 'ia32'.  Add them.

This way, getDomainCapabilities() will report all the OVMF binaries that
are present on the system.  E.g. on Fedora 29, if you only have the
edk2-ovmf-20190308stable-1.fc29.noarch package installed, then running
`virsh domcapabilities` will enumerate _both_ the OVMF binaries (instead
of just the OVMF_CODE.fd):

  $> virsh getdomcapabilities
    ...
    <loader supported='yes'>
      <value>/usr/share/edk2/ovmf/OVMF_CODE.fd</value>
      <value>/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd</value>
    ...

(
Learnt this from a discussion with Michal Privoznik in this bug,
comment#2:

    https://bugzilla.redhat.com/show_bug.cgi?id=1733940 -- RFE: Report
    firmware (FW) paths in domainCapabilities based on FW descriptor
    files
)
Signed-off-by: NKashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The snapshot-create operation of running guests saves the live
XML and uses it to replace the active and inactive domain in
case of revert. So, the config XML is ignored by the snapshot
process. This commit changes it and adds the config XML in the
snapshot XML as the <inactiveDomain> entry.

In case of offline guest, the behavior remains the same and the
config XML is saved in the snapshot XML as <domain> entry. The
behavior of older snapshots of running guests, that don't have
the new <inactiveDomain>, remains the same too. The revert, in
this case, overrides both active and inactive domain with the
<domain> entry. So, the <inactiveDomain> in the snapshot XML is
not required to snapshot work, but it's useful to preserve the
config XML of running guests.
Signed-off-by: NMaxiwell S. Garcia <maxiwell@linux.ibm.com>
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>

The function virDomainDefFormatInternal() has the predefined root name
"domain" to format the XML. But to save both active and inactive domain
in the snapshot XML, the new root name "inactiveDomain" was created.
So, the new function virDomainDefFormatInternalSetRootName() allows to
choose the root name of XML. The former function became a tiny wrapper
to call the new function setting the correct parameters.
Signed-off-by: NMaxiwell S. Garcia <maxiwell@linux.ibm.com>
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NJiri Denemark <jdenemar@redhat.com>

Once we copy the domain definition from virDomainSnapshotDef, we either
need to assign it to the domain object or free it to avoid memory leaks.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>