Prepare the function to integrate other protocols and start folding
other network protocols into a common place.

Automatically assign secret type from the disk source definition and
pull in adding of the comma. Then update callers to keep generated
output the same.

The snapshot code will need to use qemu-style formatted URIs of network
disks. Split out the code to avoid duplication.

Before this patch, the translation function still needs a second ugly
helper function to actually format the command line for qemu. But if we
do the right stuff in the translation function, we don't have to bother
with the second function any more.

This patch removes the messy qemuBuildVolumeString function and changes
qemuTranslateDiskSourcePool to set stuff up correctly so that the
regular code paths meant for volumes can be used to format the command
line correctly.

For this purpose a new helper "qemuDiskGetActualType()" is introduced to
return the type of the volume in a pool.

As a part of the refactor the qemuTranslateDiskSourcePool function is
fixed to do decisions based on the pool type instead of the volume type.
This allows to separate pool-type-specific stuff more clearly and will
ease addition of other pool types that will require certain other
operations to get the correct pool source.

The previously fixed tests should make sure that we don't break stuff
that was working before.

Though trying to destroy a physical HBA doesn't make sense at all,
it's still a bit misleading with saying "only works for HBA".
Signed-off-by: NOsier Yang <jyang@redhat.com>

The virDomainGetBlockJobInfo method did not zero out the
virDomainBlockJobInfo pointer arg, so when block jobs were
not active it would return garbage for the bandwidth/cur/end
fields.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

To avoid future pain, add placeholder functions to get the actual
snapshot disk type.

Add virDomainDiskAuthClear to help cleaning out the struct in other
places too.

When doing an internal snapshot on a VM with sheepdog or RBD disks we
would not set a flag to mark the domain is using internal snapshots and
might end up creating a mixed snapshot. Move the setting of the variable
to avoid this problem.

To simplify operations on virDomainDiskHostDef arrays we will need deep
copy and freeing functions. Add and properly export them.

Consider the following valid snapshot XML as the <driver> element is
allowed to be empty in the domainsnapshot.rng schema:

$ cat snap.xml
<domainsnapshot>
  <disks>
    <disk name='vda' snapshot='external'>
      <source file='/tmp/foo'/>
      <driver/>
    </disk>
  </disks>
</domainsnapshot>

produces the following error:

$ virsh snapshot-create domain snap.xml
error: internal error: unknown disk snapshot driver '(null)'

The driver type is parsed as NULL from the XML as the attribute is not
present and then directly used to produce the error message.

With this patch the attempt to parse the driver type is skipped if not
present to avoid changing the schema to forbid the empty driver element.

Disk source elements for snapshots were using separate code from our
config parser. As snapshots can be stored on more than just regular
files, we will need the universal parser to allow us to expose a variety
of snapshot disk targets. This patch reuses the config parsers and
formatters to do the job.

This initial support only changes the code without any visible XML
change.

This code will be reused in the snapshot disk definition parser.

The code is common for all the various disk types. Split it out to a
common function.

Avoid if statements when used with virBufferEscapeString which
automaticaly omits the whole string. Also add some line breaks to
visualy separate the code.

The <source> element formatting function was expecting a
virDomainDiskDefPtr to store the data. As snapshots are not using this
data structure to hold the data, we need to add an internal function
which splits out individual fields separately.

The original code ignored errors of virDomainHostdevDefAlloc,
however, we should properly do error return from the function
if it occurs.

The fix pulls out virDomainHostdevDefAlloc from the loop and
executes it all together before the loop. So we can easily
return on errors without the notion of other memory allocations
in the loop.

The deallocation code is separated from the allocation code
because it will be used by a further patch for fixing other error
handlings.
Reported-by: NLaine Stump <laine@laine.org>
Signed-off-by: NRyota Ozaki <ozaki.ryota@gmail.com>

The fixed loop used logical OR to combine two conditions, however,
it is apparently incorrect and logical AND is correct.

We can fix it by replacing OR with AND, but this patch instead
fixes the problem by getting rid of the first conditional
statement: USBFilterCount < def->nhostdevs. It isn't needed
because USBFilterCount will never be greater than or equal to
def->nhostdevs.

def->nhostdevs is calculated in the following code
above the loop in question like this:

    for (i = 0; i < deviceFilters.count; i++) {
        PRBool active = PR_FALSE;
        IUSBDeviceFilter *deviceFilter = deviceFilters.items[i];

        deviceFilter->vtbl->GetActive(deviceFilter, &active);
        if (active) {
            def->nhostdevs++;
        }
    }

And the loop is constructed as like this:

    for (i = 0; (USBFilterCount < def->nhostdevs) || (i < deviceFilters.count); i++) {
        PRBool active                  = PR_FALSE;
(snip)
        deviceFilter->vtbl->GetActive(deviceFilter, &active);
        if (!active)
            continue;
(snip)
        USBFilterCount++;
    }

So def->nhostdevs is the number of active device filters and
USBFilterCount is counted up only when a device filter is active.
Thus, we can remove USBFilterCount < def->nhostdevs safely.
Reported-by: NLaine Stump <laine@laine.org>
Signed-off-by: NRyota Ozaki <ozaki.ryota@gmail.com>

While running nwfilterxml2xmltest, it was found that valgrind pointed out the
following error...

==7466== 16 bytes in 1 blocks are definitely lost in loss record 26 of 90
==7466==    at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
    ==7466==    by 0x4C651AD: virAlloc (viralloc.c:142)
    ==7466==    by 0x4D0450D: virNWFilterDefParseNode (nwfilter_conf.c:2575)
    ==7466==    by 0x4D05D84: virNWFilterDefParse (nwfilter_conf.c:2647)
    ==7466==    by 0x401FDE: testCompareXMLToXMLHelper (nwfilterxml2xmltest.c:39)
    ==7466==    by 0x402DE1: virtTestRun (testutils.c:138)
    ==7466==    by 0x4018E9: mymain (nwfilterxml2xmltest.c:111)
    ==7466==    by 0x403482: virtTestMain (testutils.c:593)
    ==7466==    by 0x341F421A04: (below main) (libc-start.c:225)

...21 times, which are related to 21 tests in nwfilterxml2xmltest.c which sent
EXPECT_WARN = false. There were two scenarios in virNWFilterDefParseXML(),
when the variable 'entry' was malloc'ed, but not freed.

This patch fixes the memory leaks found while running qemuxml2argvtest

==8260== 3 bytes in 1 blocks are definitely lost in loss record 1 of
129
==8260==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==8260==    by 0x341F485E21: strdup (strdup.c:42)
==8260==    by 0x4CADCFF: virStrdup (virstring.c:554)
==8260==    by 0x4CBB839: virXPathString (virxml.c:90)
==8260==    by 0x4CE753A: virDomainDefParseXML (domain_conf.c:11478)
==8260==    by 0x4CEB4FE: virDomainDefParseNode (domain_conf.c:12742)
==8260==    by 0x4CEB675: virDomainDefParse (domain_conf.c:12684)
==8260==    by 0x425958: testCompareXMLToArgvHelper (qemuxml2argvtest.c:107)
==8260==    by 0x427111: virtTestRun (testutils.c:138)
==8260==    by 0x41D3FE: mymain (qemuxml2argvtest.c:452)
==8260==    by 0x4277B2: virtTestMain (testutils.c:593)
==8260==    by 0x341F421A04: (below main) (libc-start.c:225)
==8260==
==8260== 4 bytes in 1 blocks are definitely lost in loss record 5 of
129
==8260==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==8260==    by 0x341F485E21: strdup (strdup.c:42)
==8260==    by 0x4CADCFF: virStrdup (virstring.c:554)
==8260==    by 0x4CBB839: virXPathString (virxml.c:90)
==8260==    by 0x4CE753A: virDomainDefParseXML (domain_conf.c:11478)
==8260==    by 0x4CEB4FE: virDomainDefParseNode (domain_conf.c:12742)
==8260==    by 0x4CEB675: virDomainDefParse (domain_conf.c:12684)
==8260==    by 0x425958: testCompareXMLToArgvHelper (qemuxml2argvtest.c:107)
==8260==    by 0x427111: virtTestRun (testutils.c:138)
==8260==    by 0x41D39A: mymain (qemuxml2argvtest.c:451)
==8260==    by 0x4277B2: virtTestMain (testutils.c:593)
==8260==    by 0x341F421A04: (below main) (libc-start.c:225)
==8260==

When setting up filesystems backed by block devices or file
images, the SELinux mount options must be used to ensure the
correct context is set
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

This patch resolves:

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

The basic problem is that during a network update, the required
iptables rules sometimes change, and this was being handled by simply
removing and re-adding the rules. However, the removal of the old
rules was done based on the *new* state of the network, which would
mean that some of the rules would not match those currently in the
system, so the old rules wouldn't be removed.

This patch removes the old rules prior to updating the network
definition then adds the new rules as soon as the definition is
updated. Note that this could lead to a stray packet or two during the
interim, but that was already a problem before (the period of limbo is
now just slightly longer).

While moving the location for the rules, I added a few more sections
that should result in the iptables rules being redone:

DHCP_RANGE and DHCP_HOST - these are needed because adding/removing a dhcp
host entry could lead to the dhcp service being started/stopped, which
would require that the mangle rule that fixes up dhcp response
checksums sould need to be added/removed, and this wasn't being done.

The code for extracting sub-mounts would just do a STRPREFIX
check on the mount. This was flawed because if there were
the following mounts

 /etc/aliases
 /etc/aliases.db

and '/etc/aliases' was asked for, it would return both even
though the latter isn't a sub-mount.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Move the code for lxcContainerGetSubtree into the virfile
module creating 2 new functions

  int virFileGetMountSubtree(const char *mtabpath,
                             const char *prefix,
                             char ***mountsret,
                             size_t *nmountsret);
  int virFileGetMountReverseSubtree(const char *mtabpath,
                                    const char *prefix,
                                    char ***mountsret,
                                    size_t *nmountsret);

Add a new virfiletest.c test case to validate the new code.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Add virStringSortCompare and virStringSortRevCompare as
standard functions to use with qsort.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Export string conversion from and to the virStorageVolType enum.

When attempting to backport gluster pools to an older versoin
where there is no VIR_STRDUP, I got a crash from calling
strdup(,NULL). Rather than relying on the current else branch
safely doing nothing when there is no fd, it is easier to just
skip it. While at it, there's no need to explicitly set
perms.label to NULL after a VIR_FREE().

* src/storage/storage_backend.c
(virStorageBackendUpdateVolTargetInfoFD): Minor optimization.
Signed-off-by: NEric Blake <eblake@redhat.com>

The virsh command 'domxml-to-native' (virConnectDomainXMLToNative())
converts all network devices to "type='ethernet'" in order to make it
more likely that the generated command could be run directly from a
shell (other libvirt network device types end up referencing file
descriptors for tap devices assumed to have been created by libvirt,
which can't be done in this case).

During this conversion, all of the netdev parameters are cleared out,
then specific items are filled in after changing the type. The MAC
address was not one of these preserved items, and the result was that
mac addresses in the generated commandlines were always
00:00:00:00:00:00.

This patch saves the mac address before the conversion, then
repopulates it afterwards, so the proper mac addresses show up in the
commandline.
Signed-off-by: NBing Bu Cao <mars@linux.vnet.ibm.com>
Signed-off-by: NLaine Stump <laine@laine.org>

Commit 348b4e25 introduced a potential problem (thankfully not
in any release): we are attempting to use virFileReadHeaderFD()
on a file that was opened with O_NONBLOCK.  While this
shouldn't be a problem in practice (because O_NONBLOCK
typically doesn't affect regular or block files, and fifos and
sockets cannot be storage volumes), it's better to play it safe
to avoid races from opening an unexpected file type while also
avoiding problems with having to handle EAGAIN while read()ing.

Based on a report by Dan Berrange.

* src/storage/storage_backend.c
(virStorageBackendVolOpenCheckMode): Fix up fd after avoiding race.
Signed-off-by: NEric Blake <eblake@redhat.com>

Also after commit 5ff9d8a65ce80efb509ce4e8051394e9ed2cd942
vfs: Lock in place mounts from more privileged users,

unprivileged user has no rights to umount the mounts that
inherited from parent mountns.

right now, I have no good idea to fix this problem, we need
to do more research. this patch just skip unmounting these
mounts for shared root.

BTW, I think when libvirt lxc enables user namespace, the
configuation that shares root with host is very rara.
Signed-off-by: NGao feng <gaofeng@cn.fujitsu.com>

After kernel commit 5ff9d8a65ce80efb509ce4e8051394e9ed2cd942
vfs: Lock in place mounts from more privileged users,

unprivileged user has no rights to move the mounts that
inherited from parent mountns. we use this feature to move
the /stateDir/domain-name.{dev, devpts} to the /dev/ and
/dev/pts directroy of container. this commit breaks libvirt lxc.

this patch changes the behavior to bind these mounts when
user namespace is enabled and move these mounts when user
namespace is disabled.
Signed-off-by: NGao feng <gaofeng@cn.fujitsu.com>

Since the label is at the beginning of the loop, this has the same effect.

With some authentication mechanism (PLAIN for example), sasl_client_start()
can return SASL_OK, which translates to virNetSASLSessionClientStart()
returning VIR_NET_SASL_COMPLETE.
cyrus-sasl documentation is a bit vague as to what to do in such situation,
but upstream clarified this a bit in
http://asg.andrew.cmu.edu/archive/message.php?mailbox=archive.cyrus-sasl&msg=10104

When we got VIR_NET_SASL_COMPLETE after virNetSASLSessionClientStart() and
if the remote also tells us that authentication is complete, then we should
end the authentication procedure rather than forcing a call to
virNetSASLSessionClientStep(). Without this patch, when trying to use SASL
PLAIN, I get:
error :authentication failed : Failed to step SASL negotiation: -1
(SASL(-1): generic failure: Unable to find a callback: 32775)

This patch is based on a spice-gtk patch by Dietmar Maurer.

virNetSASLSessionClientStep logs the data that is going to be passed to
sasl_client_step as input data. However, it tries to log it as a string,
while there is no guarantee that this data is going to be nul-terminated.
This leads to this valgrind log:

==20938== Invalid read of size 1
==20938==    at 0x8BDB08F: vfprintf (vfprintf.c:1635)
==20938==    by 0x8C06DF2: vasprintf (vasprintf.c:62)
==20938==    by 0x4CCEDF9: virVasprintfInternal (virstring.c:337)
==20938==    by 0x4CA9516: virLogVMessage (virlog.c:842)
==20938==    by 0x4CA939A: virLogMessage (virlog.c:778)
==20938==    by 0x4E21E0D: virNetSASLSessionClientStep (virnetsaslcontext.c:458)
==20938==    by 0x4DE47B8: remoteAuthSASL (remote_driver.c:4136)
==20938==    by 0x4DE33AE: remoteAuthenticate (remote_driver.c:3635)
==20938==    by 0x4DDBFAA: doRemoteOpen (remote_driver.c:832)
==20938==    by 0x4DDC8BA: remoteConnectOpen (remote_driver.c:1027)
==20938==    by 0x4D8595F: do_open (libvirt.c:1239)
==20938==    by 0x4D863F3: virConnectOpenAuth (libvirt.c:1481)
==20938==    by 0x12762B: vshReconnect (virsh.c:337)
==20938==    by 0x12C9B0: vshInit (virsh.c:2470)
==20938==    by 0x12E9A5: main (virsh.c:3338)
==20938==  Address 0xe329ccd is 0 bytes after a block of size 141 alloc'd
==20938==    at 0x4A081D4: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==20938==    by 0x8CB91B4: xdr_array (xdr_array.c:94)
==20938==    by 0x4E039C2: xdr_remote_auth_sasl_start_ret (remote_protocol.c:3134)
==20938==    by 0x4E1F8AA: virNetMessageDecodePayload (virnetmessage.c:405)
==20938==    by 0x4E119F5: virNetClientProgramCall (virnetclientprogram.c:377)
==20938==    by 0x4DF8141: callFull (remote_driver.c:5794)
==20938==    by 0x4DF821A: call (remote_driver.c:5816)
==20938==    by 0x4DE46CF: remoteAuthSASL (remote_driver.c:4112)
==20938==    by 0x4DE33AE: remoteAuthenticate (remote_driver.c:3635)
==20938==    by 0x4DDBFAA: doRemoteOpen (remote_driver.c:832)
==20938==    by 0x4DDC8BA: remoteConnectOpen (remote_driver.c:1027)
==20938==    by 0x4D8595F: do_open (libvirt.c:1239)
==20938==    by 0x4D863F3: virConnectOpenAuth (libvirt.c:1481)
==20938==    by 0x12762B: vshReconnect (virsh.c:337)
==20938==    by 0x12C9B0: vshInit (virsh.c:2470)
==20938==    by 0x12E9A5: main (virsh.c:3338)

The array of sasl_callback_t callbacks which is passed to sasl_client_new()
must be kept alive as long as the created sasl_conn_t object is alive as
cyrus-sasl uses this structure internally for things like logging, so
the memory used for callbacks must only be freed after sasl_dispose() has
been called.

During testing of successful SASL logins with
virsh -c qemu+tls:///system list --all
I've been getting invalid read reports from valgrind

==9237== Invalid read of size 8
==9237==    at 0x6E93B6F: _sasl_getcallback (common.c:1745)
==9237==    by 0x6E95430: _sasl_log (common.c:1850)
==9237==    by 0x16593D87: digestmd5_client_mech_dispose (digestmd5.c:4580)
==9237==    by 0x6E91653: client_dispose (client.c:332)
==9237==    by 0x6E9476A: sasl_dispose (common.c:851)
==9237==    by 0x4E225A1: virNetSASLSessionDispose (virnetsaslcontext.c:678)
==9237==    by 0x4CBC551: virObjectUnref (virobject.c:262)
==9237==    by 0x4E254D1: virNetSocketDispose (virnetsocket.c:1042)
==9237==    by 0x4CBC551: virObjectUnref (virobject.c:262)
==9237==    by 0x4E2701C: virNetSocketEventFree (virnetsocket.c:1794)
==9237==    by 0x4C965D3: virEventPollCleanupHandles (vireventpoll.c:583)
==9237==    by 0x4C96987: virEventPollRunOnce (vireventpoll.c:652)
==9237==    by 0x4C94730: virEventRunDefaultImpl (virevent.c:274)
==9237==    by 0x12C7BA: vshEventLoop (virsh.c:2407)
==9237==    by 0x4CD3D04: virThreadHelper (virthreadpthread.c:161)
==9237==    by 0x7DAEF32: start_thread (pthread_create.c:309)
==9237==    by 0x8C86EAC: clone (clone.S:111)
==9237==  Address 0xe2d61b0 is 0 bytes inside a block of size 168 free'd
==9237==    at 0x4A07577: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==9237==    by 0x4C73827: virFree (viralloc.c:580)
==9237==    by 0x4DE4BC7: remoteAuthSASL (remote_driver.c:4219)
==9237==    by 0x4DE33D0: remoteAuthenticate (remote_driver.c:3639)
==9237==    by 0x4DDBFAA: doRemoteOpen (remote_driver.c:832)
==9237==    by 0x4DDC8DC: remoteConnectOpen (remote_driver.c:1031)
==9237==    by 0x4D8595F: do_open (libvirt.c:1239)
==9237==    by 0x4D863F3: virConnectOpenAuth (libvirt.c:1481)
==9237==    by 0x12762B: vshReconnect (virsh.c:337)
==9237==    by 0x12C9B0: vshInit (virsh.c:2470)
==9237==    by 0x12E9A5: main (virsh.c:3338)

This commit changes virNetSASLSessionNewClient() to take ownership of the SASL
callbacks. Then we can free them in virNetSASLSessionDispose() after the corresponding
sasl_conn_t has been freed.

When testing SASL authentication over TLS with
virsh -c qemu+tls:///system list --all
I got this valgrind trace after entering wrong credentials:

==30540== 26,903 (88 direct, 26,815 indirect) bytes in 1 blocks are definitely lost in loss record 289 of 293
==30540==    at 0x4A081D4: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==30540==    by 0x4C7379A: virAllocVar (viralloc.c:558)
==30540==    by 0x4CBC178: virObjectNew (virobject.c:190)
==30540==    by 0x4CBC329: virObjectLockableNew (virobject.c:216)
==30540==    by 0x4E2D003: virNetTLSContextNew (virnettlscontext.c:719)
==30540==    by 0x4E2DC3F: virNetTLSContextNewPath (virnettlscontext.c:930)
==30540==    by 0x4E2DD5B: virNetTLSContextNewClientPath (virnettlscontext.c:957)
==30540==    by 0x4DDB618: doRemoteOpen (remote_driver.c:627)
==30540==    by 0x4DDC8BA: remoteConnectOpen (remote_driver.c:1031)
==30540==    by 0x4D8595F: do_open (libvirt.c:1239)
==30540==    by 0x4D863F3: virConnectOpenAuth (libvirt.c:1481)
==30540==    by 0x12762B: vshReconnect (virsh.c:337)
==30540==    by 0x12C9B0: vshInit (virsh.c:2470)
==30540==    by 0x12E9A5: main (virsh.c:3338)

You'd think I'd learn to actually COMMIT my working tree
between testing that a last-minute fix compiles and pushing.

* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshVol): Typo fix.
Signed-off-by: NEric Blake <eblake@redhat.com>

Putting together pieces from previous patches, it is now possible
for 'virsh vol-dumpxml --pool gluster volname' to report metadata
about a qcow2 file stored on gluster.  The backing file is still
treated as raw; to fix that, more patches are needed to make the
storage backing chain analysis recursive rather than halting at
a network protocol name, but that work will not need any further
calls into libgfapi so much as just reusing this code, and that
should be the only code outside of the storage driver that needs
any help from libgfapi.  Any additional use of libgfapi within
libvirt should only be needed for implementing storage pool APIs
such as volume creation or resizing, where backing chain analysis
should be unaffected.

* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterReadHeader): New helper function.
(virStorageBackendGlusterRefreshVol): Probe non-raw files.
Signed-off-by: NEric Blake <eblake@redhat.com>