Just a cleanup I stumbled upon in one of my older branches I did when
browsing through some code and forgot to send it.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Add new function to manage adding the disk -drive options to the
command line removing that task from the mainline qemuBuildCommandLine.

Also since using const virDomainDef in new function, that means other
functions called needed to change their usage.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Add new function to manage adding the controller -device options to the
command line removing that task from the mainline qemuBuildCommandLine.

Also adjust to using const virDomainDef instead of virDomainDefPtr.
This causes collateral damage in order to modify called APIs to use
the const virDomainDef instead as well.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Extend the chardev source XML so that there is a new optional
<log/> element, which is applicable to all character device
backend types. For example, to log output of a TCP backed
serial port

    <serial type='tcp'>
      <source mode='connect' host='127.0.0.1' service='9999'/>
      <protocol type='raw'/>
      <log file='/var/log/libvirt/qemu/demo-serial0.log' append='on'/>
      <target port='0'/>
    </serial>

Not all hypervisors will support use of logfiles.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Cpus are indexed starting from '0' so the check was invalid.

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

Introduce VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN domain feature flag
whcih will allow to skip ignoring of the pinning information for
hypervisor drivers which will want to implement forward-pinning of
vcpus.

Introduce a helper to check supported device and domain config and move
the memory hotplug checks to it.

The advantage of this approach is that by default all new features are
considered unsupported by all hypervisors unless specifically changed
rather than the previous approach where every hypervisor would need to
declare that a given feature is unsupported.

To avoid having to forbid new features added to domain XML in post parse
callbacks for individual hypervisor drivers the feature flag mechanism
will allow to add a central check that will be disabled for the drivers
that will add support.

As a first example flag, the 'hasWideSCSIBus' is converted to the new
bitmap.

The API documentation states that the function is returning pinning for
all vCPUs, so we can actually do so if the user passes a large enough
array.

The implementation of the inner guts of the function is similar for all
drivers, so we can add a helper and not have to reimplement it three
times.

The VIR_DOMAIN_EVENT_ID_JOB_COMPLETED event will be triggered once a job
(such as migration) finishes and it will contain statistics for the job
as one would get by calling virDomainGetJobStats. Thanks to this event
it is now possible to get statistics of a completed migration of a
transient domain on the source host.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

While trying to build with -Os couple of compile errors showed
up.

conf/domain_conf.c: In function 'virDomainChrRemove':
conf/domain_conf.c:13666:24: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
     virDomainChrDefPtr ret, **arrPtr = NULL;
                        ^
Compiler fails to see that @ret is used only if set in the loop,
but whatever, there's no harm in initializing the variable.

In vboxAttachDrivesNew and _vboxAttachDrivesOld compiler thinks
that @rc may be used uninitialized. Well, not directly, but maybe
after some optimization. Yet again, no harm in initializing a
variable.

In file included from ./util/virthread.h:26:0,
                 from ./datatypes.h:28,
                 from vbox/vbox_tmpl.c:43,
                 from vbox/vbox_V3_1.c:37:
vbox/vbox_tmpl.c: In function '_vboxAttachDrivesOld':
./util/virerror.h:181:5: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
     virReportErrorHelper(VIR_FROM_THIS, code, __FILE__,              \
     ^
In file included from vbox/vbox_V3_1.c:37:0:
vbox/vbox_tmpl.c:1041:14: note: 'rc' was declared here
     nsresult rc;
              ^
Yet again, one uninitialized variable:

qemu/qemu_driver.c: In function 'qemuDomainBlockCommit':
qemu/qemu_driver.c:17194:9: error: 'baseSource' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         qemuDomainPrepareDiskChainElement(driver, vm, baseSource,
         ^

And another one:

storage/storage_backend_logical.c: In function 'virStorageBackendLogicalMatchPoolSource.isra.2':
storage/storage_backend_logical.c:618:33: error: 'thisSource' may be used uninitialized in this function [-Werror=maybe-uninitialized]
                       thisSource->devices[j].path))
                                 ^
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

While trying to build with -Os I've encountered some build
failures.

util/vircommand.c: In function 'virCommandAddEnvFormat':
util/vircommand.c:1257:1: error: inlining failed in call to 'virCommandAddEnv': call is unlikely and code size would grow [-Werror=inline]
 virCommandAddEnv(virCommandPtr cmd, char *env)
 ^
util/vircommand.c:1308:5: error: called from here [-Werror=inline]
     virCommandAddEnv(cmd, env);
     ^
This function is big enough for the compiler to be not inlined.
This is the error message I'm seeing:

Then virDomainNumatuneNodeSpecified is exported and called from
other places. It shouldn't be inlined then.

In file included from network/bridge_driver_platform.h:30:0,
                 from network/bridge_driver_platform.c:26:
network/bridge_driver_linux.c: In function 'networkRemoveRoutingFirewallRules':
./conf/network_conf.h:350:1: error: inlining failed in call to 'virNetworkDefForwardIf.constprop': call is unlikely and code size would grow [-Werror=inline]
 virNetworkDefForwardIf(const virNetworkDef *def, size_t n)
 ^
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Prior to commit id '3d021381' virDomainObjUpdateModificationImpact was
part of virDomainLiveConfigHelperMethod and the *flags if condition
VIR_DOMAIN_AFFECT_CONFIG checked the ->persistent boolean and made the
virDomainObjGetPersistentDef call.

Since the functions were split the ->persistent check is all that remained
and thus could be combined into one if statement.
Signed-off-by: NNikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

This parameter controls the maximum bandwidth to be used
within a period for whole domain.
Signed-off-by: NAlexander Burluka <aburluka@virtuozzo.com>

This parameter represents top level period cgroup
that limits whole domain enforcement period for a quota
Signed-off-by: NAlexander Burluka <aburluka@virtuozzo.com>

This attribute is used to extend secondary PCI bar and expose it to the
guest as 64bit memory.  It works like this: attribute vram is there to
set size of secondary PCI bar and guest sees it as 32bit memory,
attribute vram64 can extend this secondary PCI bar.  If both attributes
are used, guest sees two memory bars, both address the same memory, with
the difference that the 32bit bar can address only the first part of the
whole memory.

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

We always place primary video device at first place, to make it easier
to create a qemu command or format an xml, but we should also set the
primary boolean for primary video device to 'true'.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

Add Spice graphics gl attribute. qemu 2.6 should have -spice gl=on argument to
enable opengl rendering context (patches on the ML). This is necessary to
actually enable virgl rendering.

Add a qemuxml2argv test for virtio-gpu + spice with virgl.
Signed-off-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

While reviewing how storage driver used ObjListPtr's for reference
in some recent secret driver patches to use the same mechanism, I came
across an instance where the wrong API was called for error paths after
successfully allocating the storage pool pointer and inserting into
the driver pool list.

The path is after virStoragePoolObjAssignDef succeeds - the 'def' passed
in is assigned to pool->def (or newDef) so it shouldn't be the only thing
deleted. The pool is now part of driver->pools.objs, so it would need to
be removed (as happens in the storagePoolCreateXML error paths).

Rather than calling virStoragePoolDefFree to free the def which is now
assigned to the pool, call virStoragePoolObjRemove to ensure the pool
element is removed from the driver list and that anything stored in pool
is properly handled by virStoragePoolObjFree including the call to
virStoragePoolDefFree for the pool->{def|newDef} element.

Revert commit 55e6d8cd.

This fix for https://bugzilla.redhat.com/show_bug.cgi?id=1267256
unconditionally required a machine type for all machine types
even though qemu is the only emulator using them.

Revert it to fix persistent configs for drivers with no machine type:
https://www.redhat.com/archives/libvir-list/2016-February/msg01228.html

Checking whether x > 0 before looping over [0..x] items doesn't make
sense and multi-line body must have curly brackets around it.

Best viewed with '-w'.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

There's a check if a domain definition has any graphics card and
if so, we iterate over each one of them. This makes no sense,
because even if it has none we can still iterate over.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Seems like the natural fit, since we are already adding other XML bits
in the PostParse routine.

Previously AddImplicitControllers was only called at the end of XML
parsing, meaning code that builds a DomainDef by hand had to manually
call it. Now those PostParse callers get it for free.

There's some test churn here; xen xm and sexpr test suite bits weren't
calling this before, but now they are, so you'll see new IDE controllers.
I don't think this will cause problems in practice, since the code already
needs to handle these implicit controllers like in the case when a user
defines their own XML.

virDomainObjWait is designed to be called in a loop. Make sure we break
the loop in case the domain dies to avoid waiting for an event which
will never happen.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Commit f1a89a8b allowed parsing configs from /etc/libvirt
without validating the emulator capabilities.

Check for the presence of os->type.machine even if the
VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS flag is set,
otherwise the daemon can crash on carelessly crafted input
in the config directory.

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

Our existing virHashForEach method iterates through all items disregarding the
fact, that some of the iterators might have actually failed. Errors are usually
dispatched through an error element in opaque data which then causes the
original caller of virHashForEach to return -1. In that case, virHashForEach
could return as soon as one of the iterators fail. This patch changes the
iterator return type and adjusts all of its instances accordingly, so the
actual refactor of virHashForEach method can be dealt with later.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

Since no value in the virGICVersion enumeration is negative, a clever
enough compiler can report an error such as

  src/conf/domain_conf.c:15337:75: error: comparison of unsigned enum
  expression < 0 is always false [-Werror,-Wtautological-compare]
    if ((def->gic_version = virGICVersionTypeFromString(tmp)) < 0 ||
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~

virGICVersionTypeFromString() can, however, return a negative value if
the input string is not part of the enumeration, so we definitely need
that check.

Work around the problem by storing the return value in a temporary int
variable.

Instead of allowing any random positive number, restrict the possible
values to the ones that are part of the virGICVersion enumeration.

The snapshot name generator truncates the original file name after a '.'
and replaces the suffix with the snapshot name. If two disks source
images differ only in the suffix portion, the generated name will be
duplicate.

Since this is a corner case just error out stating that a duplicate name
was generated. The user can work around this situation by providing
the file names explicitly.

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

Get rid of one indentation level by negating condition and remove ugly
pointer arithmetic at the cost of one extra allocation.

Stick to the naming pattern.

Older gcc fails to see that the variable is set iff @hasPriority
== true in which case the former is set a value. Initialize the
value while declaring it to make the compiler shut up.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Similarly to previous commit change the way how iothread scheduler info
is stored and clean up a lot of unnecessary code.

Due to bad design the vcpu sched element is orthogonal to the way how
the data belongs to the corresponding objects. Now that vcpus are a
struct that allow to store other info too, let's convert the data to the
sane structure.

The helpers for the conversion are made universal so that they can be
reused for iothreads too.

This patch also resolves https://bugzilla.redhat.com/show_bug.cgi?id=1235180
since with the correct storage approach you can't have dangling data.

virDomainDefFormatInternal is growing rather large. Extract the cputune
formatter into a separate function.

Now that the pinning info is stored elsewhere we can delete all the
obsolete code.

Now with the new struct the data can be stored in a much saner place.