If the storage device type is parsed as network our parser still allows
it to omit the <source> element. The empty drive check would not trigger
on such device as it expects that every network storage source is valid.

Use VIR_STORAGE_NET_PROTOCOL_NONE as a marker that the storage source is
empty.

The gluster volume name extraction code was copied from the XML parser
without changing the VIR_ERR_XML_ERROR error code. Use
VIR_ERR_CONFIG_UNSUPPORTED instead.

Similar to commit fdb80ed4 libvirtd
would crash if a gluster URI without path would be used in the backing
chain of a volume. The crash happens in the gluster specific part of the
parser that extracts the gluster volume name from the path.

Fix the crash by checking that the PATH is NULL.

This patch does not contain a test case as it's not possible to test it
with the current infrastructure as the test suite would attempt to
contact the gluster server in the URI. I'm working on the test suite
addition but that will be post-release material.

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

Do not use relPath, it has not been filled by virStorageFileMetadataNew.

If a storage file would be backed with a NBD device without path
(nbd://localhost) libvirt would crash when parsing the backing path for
the disk as the URI structure's path element is NULL in such case but
the NBD parser would access it shamelessly.

After the 8-byte size header, there are two one-byte headers
and two bytes of padding before the crypt_header field.

Our QCOW1_HDR_CRYPT constant did not skip the padding.
http://git.qemu.org/?p=qemu.git;a=blob;f=block/qcow.c;h=ece22697#l41

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

Remove the resize flag and use the same code path for all callers.
This flag was added by commit 18f03166 to allow virStorageFileResize
use 'safezero' while preserving the behavior.

Explicitly return -2 when a fallback to a different method should
be done, to make the code path more obvious.

Fail immediately when ftruncate fails in the mmap method,
as we did before commit 18f03166.

Right now, grabbing blockinfo always calls stat on the disk, then
opens the image to determine the capacity, using a throw-away
virStorageSourcePtr.  This has a couple of drawbacks:

1. We are calling stat and opening a file on every invocation of
the API.  However, there are cases where the stats should NOT be
changing between successive calls (if a domain is running, no
one should be changing the physical size of a block device or raw
image behind our backs; capacity of read-only files should not
be changing; and we are the gateway to the block-resize command
to know when the capacity of read-write files should be changing).
True, we still have to use stat in some cases (a sparse raw file
changes allocation if it is read-write and the amount of holes is
changing, and a read-write qcow2 image stored in a file changes
physical size if it was not fully pre-allocated).  But for
read-only images, even this should be something we can remember
from the previous time, rather than repeating every call.

2. We want to enhance the power of virDomainListGetStats, by
sharing code.  But we already have a virStorageSourcePtr for
each disk, and it would be easier to reuse the common structure
than to have to worry about the one-off virDomainBlockInfoPtr.

While this patch does not optimize reuse of information in point
1, it does get us closer to being able to do so; by updating a
structure that survives between consecutive calls.

* src/util/virstoragefile.h (_virStorageSource): Add physical, to
mirror virDomainBlockInfo; rearrange fields to match public struct.
(virStorageSourceCopy): Copy the new field.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Store into
storage source, then copy to block info.
Signed-off-by: NEric Blake <eblake@redhat.com>

Currently virStorageFileResize() function uses build conditionals to
choose either the posix_fallocate() or syscall(SYS_fallocate) with no
fallback in order to preallocate the space in the newly resized file.

Since the safezero code has a similar set of conditionals modify the
resize and safezero code in order to allow the resize logic to make use
of safezero to unify the look/feel of the code paths.

Add a new boolean (resize) to safezero() to make the optional decision
whether to try syscall(SYS_fallocate) if the posix_fallocate fails because
HAVE_POSIX_FALLOCATE is not defined (eg, return -1 and errno == 0).

Create a local safezero_sys_fallocate in order to handle the resize
code paths that support that.  If not present, the set errno = ENOSYS
in order to allow the caller to handle the failure scenarios.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

To be able to express some use cases of the RBD backing with libvirt, we
need to be able to specify a config file for the RBD client to qemu as
that is one of the commonly used options.

Some storage systems have internal support for snapshots. Libvirt should
be able to select a correct snapshot when starting a VM.

This patch adds a XML element to select a storage source snapshot for
the RBD protocol which supports this feature.

As we now have a common function to parse backing store string for RBD
backing store we can reuse it in the backing store walker so that we
don't fail on files backed by RBD storage.

This patch also adds a few tests to verify that the parsing works as
expected.

Split out the code so that the function looks homogenous after adding
more protocol specific parsers.

To allow reuse this non-trivial parser code in the backing store parser
this part of the command line parser needs to be split out into a
separate funciton.

If there are no hosts for a storage source virStorageSourceCopy and
virStorageSourceNewFromBackingRelative would try to copy them anyways.
As the success of virStorageNetHostDefCopy is determined by returning
a pointer and malloc of 0 elements might return NULL according to the
implementation, the result of the copy function may vary.

Fix this by copying the hosts array only if there are hosts defined.

When creating a disk image snapshot the libvirt code would blindly copy
the parents label to the newly created image. This runs into problems
when you start a VM from an image hosted on NFS (or other storage system
that doesn't support selinux labels) and the snapshot destination is on
a storage system that does support selinux labels. Libvirt's code in
that case generates a different security label for the image hosted on
NFS. This label is valid only for NFS images and doesn't allow access in
case of a locally stored image.

To fix this issue libvirt needs to refrain from copying security
information in cases where the default domain seclabel is a better
choice.

This patch repurposes the now unused @force argument of
virStorageSourceInitChainElement to denote whether a copy of the
security labelling stuff should be attempted or not. This allows to
fine-control the copy operation for cases where we need to keep the
label of the old disk vs. the cases where we need to keep the label
unset to use the default domain imagelabel.

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

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

The code that parses the schema from the URI touches the "hosts[0]"
member of the storage file source structure in case the URI contains a
schema. The hosts array was not yet allocated at the point in the code
where the transport protocol was parsed and set. This lead to a crash of
libvirtd.

Fix the code by allocating the "hosts" array upfront and add a test case
to verify this scenario. (Unfortunately this requires shuffling the test
case numbers too).

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

virStorageSourceInitChainElement initializes a new storage chain element
for use as a new disk source. If the new element doesn't contain the
driver name, copy it from the old source.

This fixes issue where a disk would forget the driver after a snapshot.

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

The backing store string location offset 0 determines that the file
isn't present. The string size shouldn't be then checked:

from qemu.git/docs/specs/qcow2.txt

== Header ==

The first cluster of a qcow2 image contains the file header:

Byte  0 -  3:   magic
                QCOW magic string ("QFI\xfb")

      4 -  7:   version
                Version number (valid values are 2 and 3)

      8 - 15:   backing_file_offset
                Offset into the image file at which the backing file name
                is stored (NB: The string is not null terminated). 0 if the
                image doesn't have a backing file.

     16 - 19:   backing_file_size
                Length of the backing file name in bytes. Must not be
                longer than 1023 bytes. Undefined if the image doesn't have
                a backing file.         ^^^^^^^^^

This patch intentionally leaves the backing file string size check in
place in case a malformatted file would be presented to libvirt. Also
according to the docs the string size is maximum 1023 bytes, thus this
patch adds a check to verify that.

I was also able to verify that the check was done the same way in the
legacy qcow fromat (in qemu's code).

To express empty drive we historically use storage source with empty
path. Unfortunately NBD disks may be declared without a path.

Add a helper to wrap this logic.

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

Valgrind caught a memory leak:

==2018== 9 bytes in 1 blocks are definitely lost in loss record 143 of 927
==2018==    at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==2018==    by 0x8C42369: strdup (strdup.c:42)
==2018==    by 0x50EACC9: virStrdup (virstring.c:676)
==2018==    by 0x50E79E5: virStorageSourceCopy (virstoragefile.c:1845)
==2018==    by 0x20A3FAA7: qemuDomainBlockCommit (qemu_driver.c:15620)
==2018==    by 0x51DC6B2: virDomainBlockCommit (libvirt.c:20092)

I traced it to the fact that blockcopy and blockcommit end up
reparsing a backing chain on pivot, but the chain parsing code
doesn't gracefully handle the case where the backing file is
already known.

I'm not exactly sure when this was introduced, but suspect that the
refactoring in commit 9944b710 and friends that moved towards probing
in-place rather than into a temporary structure are part of the cause.

* src/util/virstoragefile.c (virStorageFileGetMetadataInternal):
Don't leak any prior value.
Signed-off-by: NEric Blake <eblake@redhat.com>

The commit referenced above changed function arguments of
virStorageFileGetMetadataFromBuf() but didn't tweak the
ATTRIBUTE_NONNULL tied to them. This was caught by coverity as it
actually obeys them. We disabled them for GCC and thus it didn't show
up.

Additionally in commit 3ea661de I passed
NULL to the backingFormat argument which was also marked as nonnull. Use
a dummy int's address when the argument isn't supplied so that the code
doesn't need to change much.

Instead of just returning the parent path, return the complete parent
source structure.

Add a few checks and avoid resolving relative links on networked
storage.

When discovering a disk backing chain the parent disk's metadata need to
be populated into the guest images so that each piece of the backing
chain contains a copy of those. This will allow us to refactor the
security driver so that it will not need to carry around the original
disk definition.

We are going to modify storage source chains in place. Add a helper that
will copy relevant information such as security labels to the new
element if that doesn't contain it.

Now that we have pointers to store disk source information and thus can
easily exchange the structs behind we need a function to copy all the
data.

The qemu block info function relied on working with local storage. Break
this assumption by adding support for remote volumes. Unfortunately we
still need to take a hybrid approach as some of the operations require a
filedescriptor.

Previously you'd get:
 $ virsh domblkinfo gl vda
 error: cannot stat file '/img10': Bad file descriptor

Now you get some stats:
 $ virsh domblkinfo gl vda
 Capacity:       10485760
 Allocation:     197120
 Physical:       197120

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

To allow reusing this function in the qemu driver we need to allow
specifying the storage format. Also separate return of the backing store
path now isn't necessary.

There was just one callsite left. Integrate the body to the only calling
function.

There's a lot of places where we skip doing actions based on the
locality of given storage type. The usual pattern is to skip it if:

virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK

Add a simple helper to simplify the pattern to
virStorageSourceIsLocalStorage(src)

Replace the inline "auth" struct in virStorageSource with a pointer
to a virStorageAuthDefPtr and utilize between the domain_conf, qemu_conf,
and qemu_command sources for finding the auth data for a domain disk

Introduce virStorageAuthDef and friends.  Future patches will merge/utilize
their view of storage source/pool auth/secret definitions.

New API's include:
    virStorageAuthDefParse:  Parse the "<auth/>" XML data for either the
                             domain disk or storage pool returning a
                             virStorageAuthDefPtr
    virStorageAuthDefCopy:   Copy a virStorageAuthDefPtr - to be used by
                             the qemuTranslateDiskSourcePoolAuth when it
                             copies storage pool auth data into domain
                             disk auth data
    virStorageAuthDefFormat: Common output of the "<auth" in the domain
                             disk or storage pool XML
    virStorageAuthDefFree:   Free memory associated with virStorageAuthDef

Subsequent patches will utilize the new functions for the domain disk and
storage pools.

Future work in the hostdev pass through can then make use of common data
structures and code.

Replace:
if (virBufferError(&buf)) {
    virBufferFreeAndReset(&buf);
    virReportOOMError();
    ...
}

with:
if (virBufferCheckError(&buf) < 0)
    ...

This should not be a functional change (unless some callers
misused the virBuffer APIs - a different error would be reported
then)

Rename them to comply with the naming policy.

The parent directory doesn't necessarily need to be stored after we
don't mangle the path stored in the image. Remove it and tweak the code
to avoid using it.

Store backing chain paths as non-canonical. The canonicalization step
will be already taken. This will allow to avoid storing unnecessary
amounts of data.

Now that we store only relative names in virStorageSource's member
relPath the backingRelative member is obsolete. Remove it and adapt the
code to the removal.