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

* docs/news.xml: updated for the release
* po/*.po*: regenerated

So there are couple of issues here. Firstly, we never unref the
@pendingReply and thus it leaks.

==13279== 144 (72 direct, 72 indirect) bytes in 1 blocks are definitely lost in loss record 1,095 of 1,259
==13279==    at 0x4C2E080: calloc (vg_replace_malloc.c:711)
==13279==    by 0x781FA97: _dbus_pending_call_new_unlocked (in /usr/lib64/libdbus-1.so.3.14.11)
==13279==    by 0x7812A4C: dbus_connection_send_with_reply (in /usr/lib64/libdbus-1.so.3.14.11)
==13279==    by 0x56BEDF3: virNetDaemonCallInhibit (virnetdaemon.c:514)
==13279==    by 0x56BEF18: virNetDaemonAddShutdownInhibition (virnetdaemon.c:536)
==13279==    by 0x12473B: daemonInhibitCallback (libvirtd.c:742)
==13279==    by 0x1249BD: daemonRunStateInit (libvirtd.c:823)
==13279==    by 0x554FBCF: virThreadHelper (virthread.c:206)
==13279==    by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)
==13279==    by 0x928DE3C: clone (in /lib64/libc-2.23.so)

Secondly, while we send the message, we are suspended ('cos we're
talking to a UNIX socket).  However, until we are resumed back
again the reply might have came therefore subsequent
dbus_pending_call_set_notify() has no effect and in fact the
virNetDaemonGotInhibitReply() callback is never called. Thirdly,
the dbus_connection_send_with_reply() has really stupid policy
for return values. To cite the man page:

  Returns
      FALSE if no memory, TRUE otherwise.

Yes, that's right. If anything goes wrong and it's not case of
OOM then TRUE is returned, i.e. you're trying to pass FDs and
it's not supported, or you're not connected, or anything else.
Therefore, checking for return value of
dbus_connection_send_with_reply() is not enoguh. We also have to
check if @pendingReply is not NULL before proceeding any further.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

We are given a string in @machinename, we never allocate it, just
merely use it for reading. We should not free it otherwise it
leads to double free:

==32191== Thread 17:
==32191== Invalid free() / delete / delete[] / realloc()
==32191==    at 0x4C2D1A0: free (vg_replace_malloc.c:530)
==32191==    by 0x54BBB84: virFree (viralloc.c:582)
==32191==    by 0x2BC04499: qemuProcessStop (qemu_process.c:6313)
==32191==    by 0x2BC500FF: processMonitorEOFEvent (qemu_driver.c:4724)
==32191==    by 0x2BC502FC: qemuProcessEventHandler (qemu_driver.c:4769)
==32191==    by 0x5550640: virThreadPoolWorker (virthreadpool.c:167)
==32191==    by 0x554FBCF: virThreadHelper (virthread.c:206)
==32191==    by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)
==32191==    by 0x928DE3C: clone (in /lib64/libc-2.23.so)
==32191==  Address 0x31893d70 is 0 bytes inside a block of size 1,100 free'd
==32191==    at 0x4C2D1A0: free (vg_replace_malloc.c:530)
==32191==    by 0x54BBB84: virFree (viralloc.c:582)
==32191==    by 0x54C1936: virCgroupValidateMachineGroup (vircgroup.c:343)
==32191==    by 0x54C4B29: virCgroupNewDetectMachine (vircgroup.c:1550)
==32191==    by 0x2BBDDA29: qemuConnectCgroup (qemu_cgroup.c:972)
==32191==    by 0x2BC05DA7: qemuProcessReconnect (qemu_process.c:6822)
==32191==    by 0x554FBCF: virThreadHelper (virthread.c:206)
==32191==    by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)
==32191==    by 0x928DE3C: clone (in /lib64/libc-2.23.so)
==32191==  Block was alloc'd at
==32191==    at 0x4C2BE80: malloc (vg_replace_malloc.c:298)
==32191==    by 0x4C2E35F: realloc (vg_replace_malloc.c:785)
==32191==    by 0x54BB492: virReallocN (viralloc.c:245)
==32191==    by 0x54BEDF2: virBufferGrow (virbuffer.c:150)
==32191==    by 0x54BF3B9: virBufferVasprintf (virbuffer.c:408)
==32191==    by 0x54BF324: virBufferAsprintf (virbuffer.c:381)
==32191==    by 0x55BB271: virDomainGenerateMachineName (domain_conf.c:27078)
==32191==    by 0x2BBD5B8F: qemuDomainGetMachineName (qemu_domain.c:9595)
==32191==    by 0x2BBDD9B4: qemuConnectCgroup (qemu_cgroup.c:966)
==32191==    by 0x2BC05DA7: qemuProcessReconnect (qemu_process.c:6822)
==32191==    by 0x554FBCF: virThreadHelper (virthread.c:206)
==32191==    by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)

Moreover, make the @machinename 'const char *' to mark it
explicitly that we are not changing the passed string.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

I was volunteered to prepare the release notes.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

Prior to qemu 2.5 the node names would not be generated, thus would be
missing from 'query-blockstats' and 'query-named-block-nodes'. Test that
the code correctly detects nothing.

Additionally make sure that a VM without disks does not cause problems.

The test case change is necessary as our test file checker does not play
well with empty files.

Treat an NULL string equivalent to an empty string in
virTestCompareToFile so that callers don't need to add additional logic
in case when a test produces no output.

The test makes sense only when building the daemon, thus run it only in
such case.

'virt-aa-helper' is compiled when both WITH_LIBVIRTD and
WITH_SECDRIVER_APPARMOR are defined. The test was run only when
WITH_SECDRIVER_APPARMOR was defined thus causing a build failure when
building without the daemon.

I just forgot '-u' in a54c9622.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Commit @4cb719b2 moved the driver locks around since these have become
unnecessary at spots where the code handles now self-lockable object
list, but missed the possible double unlock if udevEnumerateDevices
fails, because at that point the driver lock had been already dropped.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

This reverts commit d3d422e0.

Already fixed by commit 5a30b817.

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

In commit 5e515b54 I've attempted to fix the inability to access
storage from the apparmor helper program by linking with the storage
driver. By linking with the .so the linker complains that it's not
portable. Fix this by loading the module dynamically as we are supposed
to do.
Reviewed-by: NDaniel P. Berrange <berrange@redhat.com>

Driver modules proved to be reliable for a long time. Since support for
not building modules complicates the code and makefiles drop it.
Reviewed-by: NDaniel P. Berrange <berrange@redhat.com>

Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

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

If a domain name contains a '=' and the unix socket path is
auto-generated or socket path provided by user contains '=' QEMU
is unable to properly parse the command line.  In order to make it
work we need to use the new command line syntax for VNC if it's
available, otherwise we can use the old syntax.

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

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

Test storage was created on a rhel/centos 7 node using targetcli.
Reviewed-by: NEric Blake <eblake@redhat.com>

Test file created by:

qemu-img create -f luks /var/lib/libvirt/images/luks 10M \
        -o key-secret=asdf --object secret,id=asdf,format=raw,data=asdf

Used in libvirt as:
 <disk type='file' device='disk'>
   <driver name='qemu' type='raw'/>
   <source file='/var/lib/libvirt/images/luks'/>
   <target dev='vda' bus='virtio'/>
   <encryption format='luks'>
     <secret type='passphrase' uuid='9b2c831a-fdb9-4c09-873c-1959580589e1'/>
   </encryption>
 </disk>
Reviewed-by: NEric Blake <eblake@redhat.com>

qemu 2.9 returns an extra layer in the backing data if a block job is
active. Add a test case to see whether our code properly detects and
ignores such layer.

The test data was prepared by creating a backing chain of qcow2 images
(with qemu-img and with libvirt's snapshot feature).

One of the layers was then merged back by doing a block-commit:

virsh blockcommit VM hda --top /var/lib/libvirt/images/b

and then a block-copy job was started and kept in synchronized phase:

virsh blockcopy VM hda /tmp/tgt.img --transient job
Reviewed-by: NEric Blake <eblake@redhat.com>

Add the blockstats data and fix the expected output.
Reviewed-by: NEric Blake <eblake@redhat.com>

Add the blockstats data and fix the expected output.

Test data was created as:

qemu-img create -f qcow2 img0 10M
qemu-img create -f qcow2 -o "backing_fmt=qcow2,backing_file=img0" img1
qemu-img create -f qcow2 -o "backing_fmt=qcow2,backing_file=img1" img2
...
Reviewed-by: NEric Blake <eblake@redhat.com>

With the new approach we are actually able to correctly detect node
names for the two instances of the same backing file.

Test images were created as:
qemu-img create -f qcow2 /var/lib/libvirt/images/base.qcow2 10M
qemu-img create -f qcow2 \
   -o "backing_fmt=qcow2,backing_file=/var/lib/libvirt/images/base.qcow2 \
   /var/lib/libvirt/images/a.qcow2
qemu-img create -f qcow2 \
   -o "backing_fmt=qcow2,backing_file=/var/lib/libvirt/images/base.qcow2 \
   /var/lib/libvirt/images/b.qcow2

and then used for two separate disks.
Reviewed-by: NEric Blake <eblake@redhat.com>

Store the 'drv' field both for the storage node and for the format node
and format them in the test case.
Reviewed-by: NEric Blake <eblake@redhat.com>

We can now iterate the hash table and print all detected backing chains.
This simplifies calling of the test cases.
Reviewed-by: NEric Blake <eblake@redhat.com>

virHashNew calls virRandomBits to initialize seed for the hashing
function. If a test uses iteration through the hash table to produce
results they may/will be non-deterministic. Extract the mock library
which was used for mac address mapping to be universal.
Reviewed-by: NEric Blake <eblake@redhat.com>

Remove the complex and unreliable code which inferred the node name
hierarchy only from data returned by 'query-named-block-nodes'. It turns
out that query-blockstats contain the full hierarchy of nodes as
perceived by qemu so the inference code is not necessary.

In query blockstats, the 'parent' object corresponds to the storage
behind a storage volume and 'backing' corresponds to the lower level of
backing chain. Since all have node names this data can be really easily
used to detect node names.

In addition to the code refactoring the one remaining test case needed
to be fixed along.
Reviewed-by: NEric Blake <eblake@redhat.com>

Make it less confusing by naming the field which refers to the storage
object as 'nodestorage'.
Reviewed-by: NEric Blake <eblake@redhat.com>

The same operation will become useful in other places so rename the
function to be more generic and move it to the top so that it can be
reused earlier in the file.
Reviewed-by: NEric Blake <eblake@redhat.com>

To simplify the refactoring patches disable the tests. This will allow
adding test data later.
Reviewed-by: NEric Blake <eblake@redhat.com>

Extract the test prefix path into a variable and reuse
virTestLoadFileJSON to load the sample json files rather than doing it
manually.
Reviewed-by: NEric Blake <eblake@redhat.com>

This new helper loads, parses and returns a JSON file from 'abs_srcdir'
By using variable arguments for the function, it's not necessary to
format the path separately in the test cases.
Reviewed-by: NEric Blake <eblake@redhat.com>

As a sample usage of the new helper convert the calls in qemuhelptest to
the new helper.
Reviewed-by: NEric Blake <eblake@redhat.com>

This new helper loads and returns a file from 'abs_srcdir'. By using
variable arguments for the function, it's not necessary to format the
path separately in the test cases.
Reviewed-by: NEric Blake <eblake@redhat.com>

Split out the worker loop into a separate function and export it.
Reviewed-by: NEric Blake <eblake@redhat.com>

The callers don't use it so don't waste a strlen(). Also fix the comment
for the function.
Reviewed-by: NEric Blake <eblake@redhat.com>

The node name and backing file name can be inferred from the hierarchy.
This will also help when converting to detect node names using
query-blockstats data.
Reviewed-by: NEric Blake <eblake@redhat.com>