If the virJSONValueNewObject() fails, then rather than going to error
and getting a Coverity false positive since it doesn't seem to understand
the relationship between nkeywords, keywords, and values and seems to
believe calling qemuFreeKeywords will cause a NULL deref - just return NULL
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Coverity points out that if 'dom' isn't returned from virDomainQemuAttach,
then the code already jumps to cleanup, so there was no need for the
subsequent if (dom != NULL) check.

I moved the error message about failure into the goto cleanup on failure
and then removed the if (dom != NULL)
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Coverity complains that the various checks for autoincrement and changed
variables are DEADCODE - seems to me to be a false positive - so mark it.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Add another 'dead_code_begin' - victims of our own coding practices
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Coverity points out that by using EMPTYSTR(type) we are guarding against
the possibility that it could be NULL; however, based on how 'type' was
initialized to NULL, then using nested ternary if-then-else's (?:?:)
setting either "ipv4", "ipv6", or "" - there is no way it could be NULL.
Since "-" is supposed to mean something empty in a field - modify the
nested ternary to an easier to read/process if-then-else leaving the
initialization to NULL to mean "-" in the formatted output.

Also changed the name from 'type' to 'typestr'.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Adjust the parentheses in/for the waitpid loops; otherwise, Coverity
points out:

(1) Event assignment:   Assigning: "waitret" = "waitpid(pid, &status, 0) == -1"
(2) Event between:      At condition "waitret == -1", the value of "waitret"
                        must be between 0 and 1.
(3) Event dead_error_condition:     The condition "waitret == -1" cannot
                        be true.
(4) Event dead_error_begin:     Execution cannot reach this statement:
                        "ret = -*__errno_location();".
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Since 0766783a

Coverity complains that the EDIT_FREE definition results in DEADCODE.

As it turns out with the change to use the EDIT_FREE macro the call to
vir*Free() wouldn't be necessary nor would it happen...

Prior code to above commitid would :

  vir*Ptr foo = NULL;
  ...
  foo = vir*GetXMLDesc()
  ...
  vir*Free(foo);
  foo = vir*DefineXML()
  ...

And thus the free was needed.  With the change to use EDIT_FREE the
same code changed to:

  vir*Ptr foo = NULL;
  vir*Ptr foo_edited = NULL;
  ...
  foo = vir*GetXMLDesc()
  ...
  if (foo_edited)
      vir*Free(foo_edited);
  foo_edited = vir*DefineXML()
  ...

However, foo_edited could never be set in the code path - even with
all the goto's since the only way for it to be set is if vir*DefineXML()
succeeds in which case the code to allow a retry (and thus all the goto's)
never leaves foo_edited set

All error paths lead to "cleanup:" which causes both foo and foo_edited
to call the respective vir*Free() routines if set.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Coverity complains that when multiplying to 32 bit values that eventually
will be stored in a 64 bit value that it's possible the math could
overflow unless one of the values being multiplied is type cast to
the proper size.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Coverity complains that checking for !domlist after setting doms = domlist
and making a deref of doms just above

It seems the call in question was intended to me made in the case that
'doms' was passed in and not when the virDomainObjListExport() call
allocated domlist and already called virConnectGetAllDomainStatsCheckACL().

Thus rather than check for !domlist - check that "doms != domlist" in
order to avoid the Coverity message.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Handle a few places where Coverity complains about the value being
unused. For two of them (Close cases) - the comments above the close
indicate there is no harm to ignore the error - so added an ignore_value.
For the other condition, added an rc check like other callers.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Since cd4d5475

Coverity notes that setting 'ret = -3' prior to the unconditional
setting of 'ret = 0' will cause the value to be UNUSED.

Since the comment indicates that it is expect to allow the code
to continue, just remove the ret = -3 setting.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

In qemuDomainSetBlkioParameters(), Coverity points out that the calls
to qemuDomainParseBlkioDeviceStr() are slightly different and points
out there may be a cut-n-paste error.

In the first call (AFFECT_LIVE), the second parameter is "param->field";
however, for the second call (AFFECT_CONFIG), the second parameter is
"params->field".  It seems the "param->field" is correct especially since
each path as a setting of "param" to "&params[i]".  Furthermore, there
were a few more instances of using "params[i]" instead of "param->"
which I cleaned up.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

After a4431931 the TAP FDs ale labeled with image label instead
of the process label. On the other hand, the commit was
incomplete as a few lines above, there's still old check for the
process label presence while it should be check for the image
label instead.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

As of commit 5d29ca06:
qemu: switch PCI address set from hash table to an array

There is no error to be reset.

Tweak the messages so that they mention "title" rather than
"description" when operating in title mode. Also fixes one missing "%s"
before non-formatted gettext message.

Before:
 $ virsh desc --title dom
 No description for domain: dom

After:
 $ virsh desc --title dom
 No title for domain: dom

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

To allow the compiler to track future additions of disk types, convert
the function to use a switch statement with the correct type.

From time to time weird bugreports occur on the list, e.g [1].
Even though the kernel supports setns syscall, there's an older
glibc in the system that misses a wrapper over the syscall.
Hence, after the configure phase we think there's no setns
support in the system, which is obviously wrong. On the other
hand, we can't rely on linux distributions to provide newer glibc
soon. Therefore we need to introduce the wrapper on or own.

1: https://www.redhat.com/archives/libvir-list/2014-September/msg00492.htmlSigned-off-by: NStephan Sachse <ste.sachse@gmail.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Check if the VM is alive after we possibly called into monitor to reset
the guest.

Also drop a comment with obvious content.

virProcessTranslateStatus is used on error paths that should not spoil
the returned error. As the errors are ignored, use the quiet versions of
virAsprintf to create the message.

When using split UEFI image, it may come handy if libvirt manages per
domain _VARS file automatically. While the _CODE file is RO and can be
shared among multiple domains, you certainly don't want to do that on
the _VARS file. This latter one needs to be per domain. So at the
domain startup process, if it's determined that domain needs _VARS
file it's copied from this master _VARS file. The location of the
master file is configurable in qemu.conf.

Temporary, on per domain basis the location of master NVRAM file can
be overridden by this @template attribute I'm inventing to the
<nvram/> element. All it does is holding path to the master NVRAM file
from which local copy is created. If that's the case, the map in
qemu.conf is not consulted.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Acked-by: NLaszlo Ersek <lersek@redhat.com>

QEMU now supports UEFI with the following command line:

  -drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \
  -drive file=/usr/share/OVMF/OVMF_VARS.fd,if=pflash,format=raw,unit=1 \

where the first line reflects <loader> and the second one <nvram>.
Moreover, these two lines obsolete the -bios argument.

Note that UEFI is unusable without ACPI. This is handled properly now.
Among with this extension, the variable file is expected to be
writable and hence we need security drivers to label it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Acked-by: NLaszlo Ersek <lersek@redhat.com>

Up to now, users can configure BIOS via the <loader/> element. With
the upcoming implementation of UEFI this is not enough as BIOS and
UEFI are conceptually different. For instance, while BIOS is ROM, UEFI
is programmable flash (although all writes to code section are
denied). Therefore we need new attribute @type which will
differentiate the two. Then, new attribute @readonly is introduced to
reflect the fact that some images are RO.

Moreover, the OVMF (which is going to be used mostly), works in two
modes:
1) Code and UEFI variable store is mixed in one file.
2) Code and UEFI variable store is separated in two files

The latter has advantage of updating the UEFI code without losing the
configuration. However, in order to represent the latter case we need
yet another XML element: <nvram/>. Currently, it has no additional
attributes, it's just a bare element containing path to the variable
store file.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Acked-by: NLaszlo Ersek <lersek@redhat.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

After the previous commit, migration statistics on the source and
destination hosts are not equal because the destination updated time
statistics. Let's send the result back so that the same data can be
queried on both sides of the migration.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Total time of a migration and total downtime transfered from a source to
a destination host do not count with the transfer time to the
destination host and with the time elapsed before guest CPUs are
resumed. Thus, source libvirtd remembers when migration started and when
guest CPUs were paused. Both timestamps are transferred to destination
libvirtd which uses them to compute total migration time and total
downtime. Obviously, this requires the time to be synchronized between
the two hosts. The reported times are useless otherwise but they would
be equally useless if we didn't do this recomputation so don't lose
anything by doing it.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

When migrating a transient domain or with VIR_MIGRATE_UNDEFINE_SOURCE
flag, the domain may disappear from source host. And so will migration
statistics associated with the domain. We need to transfer the
statistics at the end of a migration so that they can be queried at the
destination host.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

New --completed flag for virsh domjobinfo command.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

virDomainGetJobStats gains new VIR_DOMAIN_JOB_STATS_COMPLETED flag that
can be used to fetch statistics of a completed job rather than a
currently running job.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Job statistics data were tracked in several structures and variables.
Let's make a new qemuDomainJobInfo structure which can be used as a
single source of statistics data as a preparation for storing data about
completed a job.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

The correct attribute name is 'format', not 'type'.

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

The parser accepts P and E, so the formatter should too.

* tools/virsh.c (vshPrettyCapacity): Handle larger units.
Signed-off-by: NEric Blake <eblake@redhat.com>

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

The new blockcopy API wants to reuse only a subset of the disk
hotplug parser - namely, we only care about the embedded
virStorageSourcePtr inside a <disk> XML.  Strange as it may
seem, it was easier to just parse an entire disk definition,
then throw away everything but the embedded source, than it
was to disentangle the source parsing code from the rest of
the overall disk parsing function.  All that I needed was a
couple of tweaks and a new internal flag that determines
whether the normally-mandatory target element can be
gracefully skipped, since everything else was already optional.

* src/conf/domain_conf.h (virDomainDiskSourceParse): New
prototype.
* src/conf/domain_conf.c (VIR_DOMAIN_XML_INTERNAL_DISK_SOURCE):
New flag.
(virDomainDiskDefParseXML): Honor flag to make target optional.
(virDomainDiskSourceParse): New function.
Signed-off-by: NEric Blake <eblake@redhat.com>

Commit c1d75dee caused this warning on 32-bit platforms (fatal when
-Werror is enabled):

virsh-domain.c: In function 'cmdBlockCopy':
virsh-domain.c:2003:17: error: comparison is always false due to limited range of data type [-Werror=type-limits]

Forcing the left side of the < to be ull instead of ul shuts up
the 32-bit compiler while still protecting 64-bit code from overflow.

* tools/virsh-domain.c (cmdBlockCopy): Add type coercion.
Signed-off-by: NEric Blake <eblake@redhat.com>

qemu now checks for invalid address type for a panic device, which is
currently implemented only to use ISA address type, thus rejecting
any other options, except for leaving XML attributes blank, in that case,
defaults are used (this behaviour remains the same from earlier verions).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138125Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

When QEMU fails during incoming migration after we successfully started
it (i.e., during Perform or Finish phase), we report a rather unhelpful
message

    Unable to read from monitor: Connection reset by peer

We already have a code that takes error messages from QEMU's error
output but we disable it once QEMU successfully starts. This patch
postpones this until the end of Finish phase during incoming migration
so that we can report a much better error message:

    internal error: early end of file from monitor: possible problem:
    Unknown savevm section or instance '0000:00:05.0/virtio-balloon' 0
    load of migration failed

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

Return failure right away when the domain object can't be looked up
instead of jumping to cleanup. This allows to remove the condition
before unlocking the domain object.