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

Even after my rework of startupPolicy handling, one command
slipped my attention. The change-media command has a very unique
approach to constructing disk XML. However, it will not preserve
startupPolicy attribute.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Since the previous commit, the shallow copy is only used inside
the if (backend->buildVol) if.

Since commit e0139e30, we update the pool allocation with
the user-provided allocation values.

For qcow2, the allocation is ignored for volume building,
but we still subtracted it from pool's allocation.
This can result in interesting values if the user-provided
allocation is large enough:

Capacity:       104.71 GiB
Allocation:     109.13 GiB
Available:      16.00 EiB

We already do a VolRefresh on volume creation. Also refresh
the volume after creating and use the new value to update the pool.

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

Commit id '7383b8cc' changed virDomainDef 'virtType' to an enum, that
caused a build failure on some archs due to comparing an unsigned value
to < 0.  Adjust the fetch of 'type' to be into temporary 'int virtType'
and then assign that virtType to the def->virtType

Earlier virtType was of type int. After, introducing the enum VIR_DOMAIN_VIRT_NONE,
the type of virtType is modified to virDomainVirtType.

Introduce VIR_DOMAIN_VIRT_NONE to give domaintype the default value of zero.
This is specially helpful in constructing better error messages
when we don't want to look up the default emulator by virtType.

The test data in vircapstest.c is also modified to reflect this change.

otherwise messages end up in /var/log/kern.log if journald forwards to
syslog.

Reference: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=799633

So, our mingw build is broken. It's because while libvirt_shell
library is using some of our internal APIs, e.g. virStrndup, and
readline API but it's not being linked with nor libvirt.la nor
libreadline.  Only subsequent users of the library, like virsh,
do link to the needed libraries. In fact, I'm surprised Linux
linker doesn't care, because how can it make a static library
with missing symbols is mystery to me.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

As of commit 69929941, we set graphics/@listen attribute according to the
first listen child element even if that element is of type='network'.
This was done for backward compatibility with applications which only
support the original listen attribute. However, by doing so we broke
migration to older libvirt which tried to check that the listen
attribute matches one of the listen child elements but which did not
take type='network' elements into account.

We are not concerned about compatibility with old applications when
formatting domain XML for migration for two reasons. The XML is consumed
only by libvirtd and the IP address associated with type='network'
listen address on the source host is just useless on the destination
host. Thus, we can safely avoid propagating the type='network' IP
address to graphics/@listen attribute when creating migratable XML.

https://bugzilla.redhat.com/show_bug.cgi?id=1265111Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Instead of referencing vsh sources in all relevant client targets,
create a library that the client can link against.

This seemed to be more of a false positive as for some reason Coverity
was missing the "ret < 0" goto error condition and somehow believing that
event could be overwritten.  At first I thought it was just the ret != 0
condition difference, but it wasn't.

In any case, make use of the recent change to qemuDomainEventQueue to
check event == NULL and just pass it as a parameter directly in the
error path. That avoids the error.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

As it turns out, ret can only be 0 or -1, so rather than have some comparisons
be "ret < 0" and others be "ret != 0", make them all "ret < 0".

Coverity complains that return from virHookCall is not checked in
one place in qemuProcessStop.  Since the comment notes that we cannot
stop the operation even it if fails, just added the ignore_value.

Use 'dead_error_condition' instead of 'dead_error_begin'

The cleanup path did not clear the reference for sk1 and sk2
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

In the error path need to unref the 'caps' as well
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

So while working on my previous patches, I've noticed that
virDomainRestore implementation in qemu and test drivers has the
same problem as I am fixing.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

So far we have the following pattern occurring over and over
again:

  if (!vm->persistent)
      qemuDomainRemoveInactive(driver, vm);

It's safe to put the check into the function and save some LoC.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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

So, you want to create a domain from XML. The domain already
exists in libvirt's database of domains. It's okay, because name
and UUID matches. However, on domain startup, internal
representation of the domain is overwritten with your XML even
though we claim that the XML you've provided is a transient one.
The bug is to be found across nearly all the drivers.
Le sigh.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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

Okay, so we allow users to 'virsh create' an already existing
domain, providing completely different XML than the one stored in
Libvirt. Well, as long as name and UUID matches. However, in some
drivers the code that handles errors unconditionally removes the
domain that failed to start even though the domain might have
been persistent. Fortunately, the domain is removed just from the
internal list of domains and the config file is kept around.

Steps to reproduce:

1) virsh dumpxml $dom > /tmp/dom.xml
2) change XML so that it is still parse-able but won't boot, e.g.
change guest agent path to /foo/bar
3) virsh create /tmp/dom.xml
4) virsh dumpxml $dom
5) Observe "No such domain" error
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

As announced on the list [1], Cherrypop is a management
application based on libvirt. It's a decentralized cloud software
with nice scaling ability.

1: https://www.redhat.com/archives/libvir-list/2015-September/msg00670.htmlSigned-off-by: NMichal Privoznik <mprivozn@redhat.com>

I initially added this in order to keep the code more error-prone to
following additions, but it seems it's still frowned upon.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Now that virQEMUDriverCreateXMLConf is never called with NULL
(after 086f37e9) we can safely drop useless check in
qemuDomainDeviceDefPostParse as we are guaranteed to be always
called with the driver initialized. Therefore checking if driver
is NULL makes no sense. Moreover, if we mix it with direct driver
dereference. And after that, we are sure that nor @cfg will be
NULL, therefore we can drop checks for that too.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Although 0 length block jobs aren't entirely useful, the output of virsh
blockjob is empty due to the condition that suppresses the output for
migration jobs that did not start. Since the only place that actually
uses the condition that suppresses the output is in migration, let's
move the check there and thus add support for 0 of 0 equaling to 100%.

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

Qemu unfortunately doesn't update internal state right after migration
and so the actual balloon size as returned by 'query-balloon' are
invalid for a while after the CPUs are started after migration. If we'd
refresh our internal state at this point we would report invalid current
memory size until the next balloon event would arrive.

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

My original implementation was based on a qemu version that still did
not have all the checks in place. Using sizes that would align to odd
megabyte increments will produce the following error:

qemu-kvm: -device pc-dimm,node=0,memdev=memdimm0,id=dimm0: backend memory size must be multiple of 0x200000
qemu-kvm: -device pc-dimm,node=0,memdev=memdimm0,id=dimm0: Device 'pc-dimm' could not be initialized

Introduce an alignment retrieval function for memory devices and use it
to align the devices separately and modify a test case to verify it.

After my "client rpc: Report proper error for keepalive disconnections"
patch, virsh would no long print a warning when it closes a connection
to a daemon after a keepalive timeout. Although the warning

    virsh # 2015-09-15 10:59:26.729+0000: 642080: info :
    libvirt version: 1.2.19
    2015-09-15 10:59:26.729+0000: 642080: warning :
    virKeepAliveTimerInternal:143 : No response from client
    0x7efdc0a46730 after 1 keepalive messages in 2 seconds

was pretty ugly, it was still useful. This patch brings the useful part
back while making it much nicer:

virsh # error: Disconnected from qemu:///system due to keepalive timeout
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Even though we hit an error in client's IO loop, we still want to
process any pending data. So instead of reporting the error right away,
we can finish the current iteration and report the error once we're done
with it. Note that the error is stored in client->error by
virNetClientMarkClose so we don't need to worry about it being reset or
rewritten by any API we call in the meantime.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Whenever a connection was closed due to keepalive timeout, we would log
a warning but the interrupted API would return rather useless generic
error:

    internal error: received hangup / error event on socket

Let's report a proper keepalive timeout error and make sure it is
propagated to all pending APIs. The error should be better now:

    internal error: connection closed due to keepalive timeout

Based on an old patch from Martin Kletzander.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Just like e92e5ba1, this attribute was missed.
Signed-off-by: NLuyao Huang <lhuang@redhat.com>

Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Build fail and error like this:

  CC       qemu/libvirt_driver_qemu_impl_la-qemu_command.lo
qemu/qemu_capabilities.c:46:27: fatal error: qemu_capspriv.h: No such file or directory
 #include "qemu_capspriv.h"

Add qemu_capspriv.h to source.
Signed-off-by: NLuyao Huang <lhuang@redhat.com>

$ rpmbuild -ba libvirt.spec
warning: Macro expanded in comment on line 5: # If neither fedora nor rhel was defined, try to guess them from %{dist}

warning: Macro %enable_autotools defined but not used within scope
warning: Macro %client_only defined but not used within scope
...

We use the function to create a virDomainXMLOption object that is
required for some functions. However, we don't pass the driver
pointer to the object anywhere - rather than pass NULL. This
causes trouble later when parsing a domain XML and calling post
parse callbacks:

  Program received signal SIGSEGV, Segmentation fault.
  0x000000000043fa3e in qemuDomainDefPostParse (def=0x7d36c0, caps=0x7caf10, opaque=0x0) at qemu/qemu_domain.c:1043
  1043        qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);
  (gdb) bt
  #0  0x000000000043fa3e in qemuDomainDefPostParse (def=0x7d36c0, caps=0x7caf10, opaque=0x0) at qemu/qemu_domain.c:1043
  #1  0x00007ffff2928bf9 in virDomainDefPostParse (def=0x7d36c0, caps=0x7caf10, xmlopt=0x7c82c0) at conf/domain_conf.c:4269
  #2  0x00007ffff294de04 in virDomainDefParseXML (xml=0x7da8c0, root=0x7dab80, ctxt=0x7da980, caps=0x7caf10, xmlopt=0x7c82c0, flags=0) at conf/domain_conf.c:16400
  #3  0x00007ffff294e5b5 in virDomainDefParseNode (xml=0x7da8c0, root=0x7dab80, caps=0x7caf10, xmlopt=0x7c82c0, flags=0) at conf/domain_conf.c:16582
  #4  0x00007ffff294e424 in virDomainDefParse (xmlStr=0x0, filename=0x7c7ef0 "/home/zippy/work/libvirt/libvirt.git/tests/securityselinuxlabeldata/disks.xml", caps=0x7caf10, xmlopt=0x7c82c0, flags=0) at conf/domain_conf.c:16529
  #5  0x00007ffff294e4b2 in virDomainDefParseFile (filename=0x7c7ef0 "/home/zippy/work/libvirt/libvirt.git/tests/securityselinuxlabeldata/disks.xml", caps=0x7caf10, xmlopt=0x7c82c0, flags=0) at conf/domain_conf.c:16553
  #6  0x00000000004303ca in testSELinuxLoadDef (testname=0x53c929 "disks") at securityselinuxlabeltest.c:192
  #7  0x00000000004309e8 in testSELinuxLabeling (opaque=0x53c929) at securityselinuxlabeltest.c:313
  #8  0x0000000000431207 in virtTestRun (title=0x53c92f "Labelling \"disks\"", body=0x430964 <testSELinuxLabeling>, data=0x53c929) at testutils.c:211
  #9  0x0000000000430c5d in mymain () at securityselinuxlabeltest.c:373
  #10 0x00000000004325c2 in virtTestMain (argc=1, argv=0x7fffffffd7e8, func=0x430b4a <mymain>) at testutils.c:863
  #11 0x0000000000430deb in main (argc=1, argv=0x7fffffffd7e8) at securityselinuxlabeltest.c:381
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Even though usage of the lock is limited to a very few cases,
it's still needed. Therefore we should initialize it too.
Otherwise we may get some random test failures:

==1204== Conditional jump or move depends on uninitialised value(s)
==1204==    at 0xEF7F7CF: pthread_mutex_lock (in /lib64/libpthread-2.20.so)
==1204==    by 0x9CA89A5: virMutexLock (virthread.c:89)
==1204==    by 0x450B2A: qemuDriverLock (qemu_conf.c:83)
==1204==    by 0x45549C: virQEMUDriverGetConfig (qemu_conf.c:869)
==1204==    by 0x448E29: qemuDomainDeviceDefPostParse (qemu_domain.c:1240)
==1204==    by 0x9CC9B13: virDomainDeviceDefPostParse (domain_conf.c:4224)
==1204==    by 0x9CC9B91: virDomainDefPostParseDeviceIterator (domain_conf.c:4251)
==1204==    by 0x9CC7843: virDomainDeviceInfoIterateInternal (domain_conf.c:3440)
==1204==    by 0x9CC9C25: virDomainDefPostParse (domain_conf.c:4276)
==1204==    by 0x9CEEE03: virDomainDefParseXML (domain_conf.c:16400)
==1204==    by 0x9CEF5B4: virDomainDefParseNode (domain_conf.c:16582)
==1204==    by 0x9CEF423: virDomainDefParse (domain_conf.c:16529)
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This reverts commit 69b850fe.

This change broke the ability to "clear" or reset unfiltered back
to filtered.

For some machine types ppc64 machines now require that memory sizes are
aligned to 256MiB increments (due to the dynamically reconfigurable
memory). As now we treat existing configs reasonably in regards to
migration, we can round all the sizes unconditionally. The only drawback
will be that the memory size of a VM can potentially increase by
(256MiB - 1byte) * number_of_NUMA_nodes.

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

When we are starting a qemu process for an incomming migration or
snapshot reloading we should not modify the memory sizes in the domain
since we could potentially change the guest ABI that was tediously
checked before. Additionally the function now updates the initial memory
size according to the NUMA node size, which should not happen if we are
restoring state.

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