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>

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>

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.

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>

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

When implementing memory hotplug I've opted to recalculate the initial
memory size (contents of the <memory> element) as a sum of the sizes of
NUMA nodes when NUMA was enabled. This was based on an assumption that
qemu did not allow starting when the NUMA node size total didn't equal
to the initial memory size. Unfortunately the check was introduced to
qemu just lately.

This patch uses the new XML parser flag to decide whether it's safe to
update the memory size total from the NUMA cell sizes or not.

As an additional improvement we now report an error in case when the
size of hotplug memory would exceed the total memory size.

The rest of the changes assures that the function is called with correct
flags.

Add 'initial_memory' member to struct virDomainMemtune so that the
memory size can be pre-calculated once instead of inferring it always
again and again.

Separating of the fields will also allow finer granularity of decisions
in later patches where it will allow to keep the old initial memory
value in cases where we are handling incomming migration from older
versions that did not always update the size from NUMA as the code did
previously.

The change also requires modification of the qemu memory alignment
function since at the point where we are modifying the size of NUMA
nodes the total size needs to be recalculated too.

The refactoring done in this patch also fixes a crash in the hyperv
driver that did not properly initialize def->numa and thus
virDomainNumaGetMemorySize(def->numa) crashed.

In summary this patch should have no functional impact at this point.

The name of the variable was misleading. Rename it and it's setting
accessor before other fixes.

The post parse func is growing rather large. Since later patches will
introduce more logic in the memory post parse code, split it into a
separate handler.

Add a new parser flag that will mark code paths that parse XML files
wich will not be used with existing VM state so that post parse
callbacks can possibly do ABI incompatible changes if needed.

The flag was used only for formatting the XML and once the parser and
formatter flags were split in 0ecd6851
it doesn't make sense any more to have it.

Extract the size determination into a separate function and reuse it
across the memory device alignment functions. Since later we will need
to decide the alignment size according to architecture let's pass def to
the functions.

Add a simple helper so that the code doesn't have to rewrite the same
condition multiple times.

Commits 45697fe5 and f863ac80 used direct access to the variable instead
of the preferred accessor method.

Since test suite now correctly creates capabilities cache, the hack is not
needed any more.
Signed-off-by: NPavel Fedin <p.fedin@samsung.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

The main purpose of this patch is to introduce test mode to
virQEMUCapsCacheLookup(). This is done by adding a global variable, which
effectively overrides binary name. This variable is supposed to be set by
test suite.

The second addition is qemuTestCapsCacheInsert() function which allows the
test suite to actually populate the cache.
Signed-off-by: NPavel Fedin <p.fedin@samsung.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

 Invalid read of size 4
    at 0x945CA30: __pthread_mutex_unlock_full (in /lib64/libpthread-2.20.so)
    by 0x4F0404B: virMutexUnlock (virthread.c:94)
    by 0x4F7161B: virStoragePoolObjUnlock (storage_conf.c:2603)
    by 0x4FE0476: testStoragePoolUndefine (test_driver.c:4328)
    by 0x4FCF086: virStoragePoolUndefine (libvirt-storage.c:656)
    by 0x15A7F5: cmdPoolUndefine (virsh-pool.c:1721)
    by 0x12F48D: vshCommandRun (vsh.c:1212)
    by 0x132AA7: main (virsh.c:943)
  Address 0xfda56a0 is 16 bytes inside a block of size 104 free'd
    at 0x4C2BA6C: free (vg_replace_malloc.c:473)
    by 0x4EA5C96: virFree (viralloc.c:582)
    by 0x4F70B69: virStoragePoolObjFree (storage_conf.c:412)
    by 0x4F7167B: virStoragePoolObjRemove (storage_conf.c:437)
    by 0x4FE0468: testStoragePoolUndefine (test_driver.c:4323)
    by 0x4FCF086: virStoragePoolUndefine (libvirt-storage.c:656)
    by 0x15A7F5: cmdPoolUndefine (virsh-pool.c:1721)
    by 0x12F48D: vshCommandRun (vsh.c:1212)
    by 0x132AA7: main (virsh.c:943)

Rather than inlining the code, use the common API.

Rather than inlining the code, use the common API.

Rather than inlining the code, use the common API

Similar to commit id '35847860', it's possible to attempt to create
a 'netfs' directory in an NFS root-squash environment which will cause
the 'vol-delete' command to fail.  It's also possible error paths from
the 'vol-create' would result in an error to remove a created directory
if the permissions were incorrect (and disallowed root access).

Thus rename the virFileUnlink to be virFileRemove to match the C API
functionality, adjust the code to following using rmdir or unlink
depending on the path type, and then use/call it for the VIR_STORAGE_VOL_DIR

As far as not every call of prlsdkUUIDParse assume correct UUID
supplied, there is no use to complain about wrong format in it.
Otherwise our log is flooded with false error messages.
For instance, calling prlsdkUUIDParse from prlsdkEventsHandler
works as a filter and in case of uuid absence for event issuer,
we simply know that we shouldn't continue further processing.
Instead of error logging for all calls we should explicitly take
into accaunt where it is called from.
Signed-off-by: NMaxim Nestratov <mnestratov@virtuozzo.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1159219Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

So far this function was not kept in sync with changing
virDomainDiskDef. Fill in all the missing checks and reorganize
their order so it's easier to track which items are not being
checked for.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

I always felt like this function is qemu specific rather than
libvirt-wide. Other drivers may act differently on virDomainDef
change and in fact may require talking to underlying hypervisor
even if something else's than disk->src has changed.  I know that
the function is still incomplete, but lets break that into two
commits that are easier to review. This one is pure code
movement.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Firstly, our coding guidelines suggest using 'cleanup' label
instead of 'end'. Then, @ret should be set to value representing
success as the last statement before the 'cleanup' label.
And while I am at this function, lets enumerate all the possible
enum items (virDomainDiskDevice) and avoid using 'default' in
switch(). Pooh. Also, nothing bad happens if we look up the disk
to change in the domain upfront. In fact, it's going to be
helpful later when we want to keep some old values for performing
a rollback.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This new private API should return true iff sources of two disks
differs in sense that qemu should be instructed to change the
disk backend. For instance, ejecting a CDROM is such case, or
pointing disk into a different ISO location, and so on.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

While we currently only allow changing a media in a disk, this is
going to change in a while, so the function name would be
invalid. Moreover, the old name does not match the pattern laid
out by other update functions.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>