Since we don't have syntax-check for this, it has to be checked
manually.  Let's hope this is the only place it happened.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This only affected the servers that re-exec themselves, which is only
virtlockd and it didn't do any mess, so this is mostly a clenaup.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1218577

Treat pinning an IOThread via API as if someone added an IOThread to
ensure the iothreadid doesn't cause the guest to disappear

Since 'autofill'd iothreadid entries are not written during XML format
processing, it is possible that if an iothreadid in the middle of an
autofilled list would then change it's id on a subsequent restart.

Thus during the iothreadid deletion, if we determine the delete is not
the "last" thread, then clear the autofill bit for all iothreadid's
following the one being deleted (either the first or one in the middle).
This way, iothreadid's will be printed/saved.

When parsing a cpulist, the virBitmapParse is used. On an invalid
bitmap an error is reported, but the error gets cleared
immediately by subsequent public APIs call, e.g. virDomainFree().
Moreover, we don't check whether bitmap fits into maximal CPU ID
on the host. Therefore the following examples failed without any
error:

 # virsh vcpupin test3 1 aaa

 # virsh vcpupin test3 1 1000
Signed-off-by: NLuyao Huang <lhuang@redhat.com>

We have a lot of passing arguments code just to pass connection
object cause it holds jobTimeout. Taking into account that
right now this value is defined at compile time let's just
get rid of it and make arguments list more clear in many
places.

In case we later need some runtime configurable timeout
value we can provide this value through arguments
function already operate such as a parallels domain
object etc as this timeouts are operation( and thus
object) specific in practice.
Signed-off-by: NNikolay Shirokovskiy <nshirokovskiy@parallels.com>

As of eeb008db the variable is not used anymore. Drop it.
Signed-off-by: NWang Yufei <james.wangyufei@huawei.com>
Signed-off-by: NZhang Bo <oscar.zhangbo@huawei.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This removes some noise when you're working on the repository
and also have a bunch of source files open in Vim in another
terminal.

In order not to bring in any link dependencies, bridge driver doesn't
use the usual stubs as other conditionally-built code does.  However,
having the function as a macro imposes a problem with possibly unused
variables if just defined as "0".  This was worked around by using
(dom=dom, iface=iface, 0) which should act like a 0 if used in a
condition.  However, gcc still bugs about that, so I came up with
another way how to fix that.

Using static inline functions in the header won't collide with anything,
it fixes the bug and does one thing that the macro didn't do.  It checks
whenther passed variables are pointers of compatible type.  It has only
one downside, and that is that we need to either a) define it with
ATTRIBUTE_UNUSED, which needs an exception in cfg.mk or b) do something
like ignore_value(variable); in the function body.  I went with the
first variant.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Otherwise we might allow coldplugging a device
that uses an address that is already occupied, creating
an unstartable domain.

https://bugzilla.redhat.com/show_bug.cgi?id=1220195

In the XML we have the vnc port number, but QEMU takes on command line
a vnc screen number, it's port-5900.  We should fail with error message
that only ports in range [5900,65535] are valid.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1164966Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

VIR_STRDUP plays nicely with NULLs. Theres no need to guard its
call with check for non-NULL.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

While implementing support for SPICE, I noticed VNC passwd was
never copied to libxl_device_vfb's vnc.passwd field.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1171984
https://bugzilla.redhat.com/show_bug.cgi?id=1188463

Remove the check for the source host name for iSCSI source XML processing
declaring duplicate sources when the source device path and if present the
initiator of a proposed storage pool matches an existing storage pool.

The backend iSCSI storage driver uses 'iscsiadm --mode session' to query
available iscsid target sessions. The output displayed is the IP address
and the IQN (target path) of known targets. The displayed IP address
is a resolved address based on the session --login. Additionally, iscsid
keeps track of the various ways to define the host name (IPv4 Address,
IPv6 Address, /etc/hosts, etc.) for that IQN (see output of an 'iscsiadm
--mode node'). If an incoming IQN matches and the host name provided by
libvirt is resolved to the existing IQN, then iscsid will "reuse" the
session. Although libvirt could do the same name resolution, if there
is a difference, iscsid could still declare two seemingly different sources
to be the same and not create a new session which means libvirt now has
two storage pools looking at the same source. Thus to avoid any strange
host name resolution issues, just rely on iscsid for that and do not
allow multiple pools on the same host to use the same device path (IQN).

Only perform the port number check if the incoming definition actually
provides it. Since the port number is optional we could erroneously pass
a duplicate source host check since some storage pool backends which fill
in the default port number (e.g., iSCSI and sheepdog) for the started pool.

https://bugzilla.redhat.com/show_bug.cgi?id=1220809

When cold-plugging an RNG device but something fails in
qemuDomainAssignAddresses, we will double free the RNG device.
Once a device is plugged into the domain, we should set the
device pointer to NULL to fix this issue.

...
5  0x00007fb7d180ac8a in virFree at util/viralloc.c:582
6  0x00007fb7d1895cdd in virDomainRNGDefFree at conf/domain_conf.c:19786
7  0x00007fb7d1895d99 in virDomainDeviceDefFree at conf/domain_conf.c:2022
8  0x00007fb7b92b8baf in qemuDomainAttachDeviceFlags at qemu/qemu_driver.c:8785
9  0x00007fb7d190c5d7 in virDomainAttachDeviceFlags at libvirt-domain.c:8488
10 0x00007fb7d23af9d2 in remoteDispatchDomainAttachDeviceFlags at remote_dispatch.h:2842
...
Signed-off-by: NLuyao Huang <lhuang@redhat.com>

Similarly to other error codes that notify the user that the object does
not exist lower the priority of VIR_ERR_NO_DOMAIN_METADATA to
VIR_LOG_DEBUG when writing the log entry.

Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

There is a lot of places, were it's pretty easy for user to enter some
characters that we need to escape to create a valid XML description.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1197580Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

On the s/390x architecture, libvirt may already return 0 in the
node_info->mhz field (see src/nodeinfo.c:linuxNodeInfoCPUPopulate).

We may also want to return this on aarch64 in future, because
calculating the proper value requires SMBIOS, which is not available
on non-server-class systems (specifically on systems which don't
adhere to the SBSA standard).

Therefore this change documents the existing behaviour and provides a
valid path for aarch64.
Signed-off-by: NRichard W.M. Jones <rjones@redhat.com>
Bug-URL: https://bugzilla.redhat.com/1206353

https://bugzilla.redhat.com/show_bug.cgi?id=1191227

Since 0fa15b19 we have this variable SYNC_TIME which allows users to
synchronize time on domain resume. However, despite what documentation
says, it's by default on because it's never initialized. Fix this by
setting it to zero at the beginning of the libvirt-guests script.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

The code to add device type to the commandline was identical for lsi
and other models of SCSI controllers, but was duplicated (with the
exception of a minor ordering difference of the if-else clauses) for
the two cases. This patch replaces those two with a single instance of
the code just before the if().

This patch makes qemuValideDevicePCISlotsChipsets() more consistent in
appearance by replacing several clauses of an if with the equivalent
call to qemuDomainMachineIsI440FX. The if was checking exactly the
same items, just in a slightly different order.

GCC installed from FreeBSD ports doesn't support building PIE executables
and fails with:

/usr/local/bin/ld: /usr/lib/crt1.o: relocation R_X86_64_32 against
`_DYNAMIC' can not be used when making a shared object; recompile with
-fPIC
/usr/lib/crt1.o: error adding symbols: Bad value
collect2: error: ld returned 1 exit status

However, the configure check for '-fPIC -DPIC' doesn't catch that. In
order to catch this case, add '-pie' to CFLAGS in m4/virt-compile-pie.m4
so it could detect lack of PIE support on configure time and don't fail
the build.

gcc5 reports an error like this:

bhyvexml2argvtest.c: In function 'testCompareXMLToArgvFiles':
bhyvexml2argvtest.c:24:18: error: variable 'vm' set but not used
[-Werror=unused-but-set-variable]
     virDomainObj vm;
                  ^
cc1: all warnings being treated as errors

Fix by dropping this variable.

https://bugzilla.redhat.com/show_bug.cgi?id=1220265

Passing the return value to an enum directly is not safe.  Fix this by
comparing the true integer result of virTristateSwitchTypeFromString().
Signed-off-by: NLuyao Huang <lhuang@redhat.com>

For some reason, we allow a bridge name with %d in it, which we replace
with an unsigned integer to form a bridge name that does not yet exist
on the host.

Do not blindly pass it to virAsprintf if it's not the only conversion,
to prevent crashing on input like:

<network>
  <name>test</name>
  <forward mode='none'/>
  <bridge name='virbr%d%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s'/>
</network>

Ignore any template strings that do not have exactly one %d conversion,
like we do in various drivers before calling virNetDevTapCreateInBridgePort.

Since libvirt doesn't call to update the new balloon size in qemu add
code that will handle tweaking of the size of the current balloon
statistic until qemu reports the new size using the event.

To avoid having the ballooned memory size larger than the actual
physical memory size, truncate the ballooned size if it overflows.

Specifying a balloon size more than the memory size of a guest isn't
something that should be rejected when parsing the XML. Truncate the
size to the maximum memory size.

Use the new domain list collection helpers to avoid going through
virDomainPtrs.

This additionally implements filter capability when called through the
api that accepts domain list filters.

Add virDomainObjListConvert that will take a list of virDomains, apply
filters and return a list of virDomainObjs.

Until now the virDomainListAllDomains API would lock the domain list and
then every single domain object to access and filter it. This would
potentially allow a unresponsive VM to block the whole daemon if a
*listAllDomains call would get stuck.

To avoid this problem this patch collects a list of referenced domain
objects first from the list and then unlocks it right away. The
expensive operation requiring locking of the domain object is executed
after the list lock is dropped. While a single blocked domain will still
lock up a listAllDomains call, the domain list won't be held locked and
thus other APIs won't be blocked.

Additionally this patch also fixes the lookup code, where we'd ignore
the vm->removing flag and thus potentially return domain objects that
would be deleted very soon so calling any API wouldn't make sense.

As other clients also could benefit from operating on a list of domain
objects rather than the public domain descriptors a new intermediate
API - virDomainObjListCollect - is introduced by this patch.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1181074

The passed function is meant to filter domains according to ACL match.

Separate the code to simplify future refactors.

Extend it to a universal helper used for clearing lists of any objects.
Note that the argument type is specifically void * to allow implicit
typecasting.

Additionally add a helper that works on non-NULL terminated arrays once
we know the length.

My commit 747761a7 (v1.2.15 only) dropped this bit of logic when filling
in a default arch in the XML:

-    /* First try to find one matching host arch */
-    for (i = 0; i < caps->nguests; i++) {
-        if (caps->guests[i]->ostype == ostype) {
-            for (j = 0; j < caps->guests[i]->arch.ndomains; j++) {
-                if (caps->guests[i]->arch.domains[j]->type == domain &&
-                    caps->guests[i]->arch.id == caps->host.arch)
-                    return caps->guests[i]->arch.id;
-            }
-        }
-    }

That attempt to match host.arch is important, otherwise we end up
defaulting to i686 on x86_64 host for KVM, which is not intended.
Duplicate it in the centralized CapsLookup function.

Additionally add some testcases that would have caught this.

https://bugzilla.redhat.com/show_bug.cgi?id=1219191

My commit 7b9de914 added some aarch64 CPU test cases. I wanted to test
two different code paths but inadvertently added two of the same test
cases.

The second code path (using <cpu><model>host</model</cpu>) isn't easily
exercised via the qemu tests anyways, I'll need to look elsewhere.

Regardless, remove the redundant tests for now

https://bugzilla.redhat.com/show_bug.cgi?id=890648

So, imagine you've issued an API that involves guest agent. For
instance, you want to query guest's IP addresses. So the API acquires
QUERY_JOB, locks the guest agent and issues the agent command.
However, for some reason, guest agent replies to initial ping
correctly, but then crashes tragically while executing real command
(in this case guest-network-get-interfaces). Since initial ping went
well, libvirt thinks guest agent is accessible and awaits reply to the
real command. But it will never come. What will is a monitor event.
Our handler (processSerialChangedEvent) will try to acquire
MODIFY_JOB, which will fail obviously because the other thread that's
executing the API already holds a job. So the event handler exits
early, and the QUERY_JOB is never released nor ended.

The way how to solve this is to put flag somewhere in the monitor
internals. The flag is called @running and agent commands are issued
iff the flag is set. The flag itself is set when we connect to the
agent socket. And unset whenever we see DISCONNECT event from the
agent. Moreover, we must wake up all the threads waiting for the
agent. This is done by signalizing the condition they're waiting on.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Running shutdown with mode agent on a shutoff domain gives cryptic
error message:

    virsh # shutdown --mode agent gentoo
    error: Failed to shutdown domain gentoo
    error: Guest agent is not responding: QEMU guest agent is not connected

After this patch, the error is more clear:

    virsh # shutdown --mode agent gentoo
    error: Failed to shutdown domain gentoo
    error: Requested operation is not valid: domain is not running
Reported-by: NMartin Kletzander <mkletzan@redhat.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>