Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Now that we can open connections to the secondary drivers on demand,
there is no need to pass a virConnectPtr into all the backend
functions.
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Instead of passing around a virConnectPtr object, just open a connection
to the secret driver at time of use. Opening connections on demand will
be beneficial when the secret driver is in a separate daemon. It also
solves the problem that a number of callers just pass in a NULL
connection today which prevents secret lookup working at all.
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

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

Use virStorageBackendCreateVolUsingQemuImg to apply the LUKS information
to the logical volume just created.  As part of the processing of the
lvcreate command add 2MB to the capacity to account for the LUKS header
when it's determined that the volume desires to use encryption.

Refactor to extract out the LVCREATE command.  This also removes the
need for the local @created since the error path can now only be reached
after the creation of the logical volume.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Add helpers that will simplify checking if a backing file is valid or
whether it has backing store. The helper virStorageSourceIsBacking
returns true if the given virStorageSource is a valid backing store
member. virStorageSourceHasBacking returns true if the virStorageSource
has a backing store child.

Adding these functions creates a central points for further refactors.

Storage driver uses virStorageSource only partially to store it's
configuration but fully when parsing backing files of storage volumes.
This patch sets the 'type' field to a value other than
VIR_STORAGE_TYPE_NONE so that further patches can add a terminator
element to backing chains without breaking iteration.

In preparation for privatizing the object, use the accessor.

Create/use virStoragePoolObjAddVol in order to add volumes onto list.

Create/use virStoragePoolObjRemoveVol in order to remove volumes from list.

Create/use virStoragePoolObjGetVolumesCount to get count of volumes on list.

For the storage driver, the logic alters when the volumes.obj list grows
to after we've fetched the volobj. This is an optimization of sorts, but
also doesn't "needlessly" grow the volumes.objs list and then just decr
the count if the virGetStorageVol fails.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Use the < 0 rather than == -1 (consistently) for virAsprintf errors.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Since a sector size may be larger than 512 bytes, let's just increase
the size to wipe to 1MB rather than 2KB

Rather than open code it, use the new function which uses the wipe algorithm
in order to zero the front and tail of the partition.

off_t is signed and it's size is the same as long only on 64b archs.
Thus it cannot be formatted as %lu.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

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

As it turns out some file headers (e.g. ext4) may be larger/longer than
the 512 bytes of zeros being written prior to a pvcreate, so let's write
out 2048 bytes similar to how the pvcreate sources would peek at the first
4 sectors of the device.

Make sure there is at enough bytes on the device to clear before doing
doing the clear - just to be sure.

The function is actually in virutil.c, but prototyped in virfile.h.
This patch fixes that by renaming the function to virWaitForDevices,
adding the prototype in virutil.h and libvirt_private.syms, and then
changing the callers to use the new name.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Add APIs that allow to dynamically register driver backends so that the
list of available drivers does not need to be known during compile time.

This will allow us to modularize the storage driver on runtime.

The file became a garbage dump for all kinds of utility functions over
time. Move them to a separate file so that the files can become a clean
interface for the storage backends.

The check is pointless since LVM is capable to detect it's own members
and the check is flawed as it would fail if neither libblkid nor parted
is installed.

We don't really need to babysit LVM in this way.

This reverts commit cb38b6cb.

At startup time, rather than blindly trusting the target devices are
still properly formatted, let's check to make sure the pool's target
devices are all properly formatted before attempting to start the pool.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

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

Add support and documentation for the [NO_]OVERWRITE flags for the
logical backend.

Update virsh.pod with a description of the process for usage of
the flags and building of the pool's volume group.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Make the remaining code a bit cleaner.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

If the build fails, then we need to ensure that we've run pvremove
on any devices which we've run pvcreate on; otherwise, a subsequent
build could fail since running pvcreate twice on a device requires
special force arguments.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Rather than have virCommandRun just spit out the error, allow callers
to decide to pass the exitstatus so the caller can make intelligent
decisions based on the error.

This reverts commit 6682d621.

The "if (vol->target.format != VIR_STORAGE_FILE_RAW)" check in the
createVol backend. This check is bogus because virStorageVolDefParseXML()
in conf/storage_conf.c sets target.format only if volOptions in
virStoragePoolTypeInfo has formatFromString set, and that's not the
case the logical backend.

So the check always fails and breaks volume creation.

This improves the code consistency around freeing vol->target.path in
createVol implementations.

While trying to build with -Os couple of compile errors showed
up.

conf/domain_conf.c: In function 'virDomainChrRemove':
conf/domain_conf.c:13666:24: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
     virDomainChrDefPtr ret, **arrPtr = NULL;
                        ^
Compiler fails to see that @ret is used only if set in the loop,
but whatever, there's no harm in initializing the variable.

In vboxAttachDrivesNew and _vboxAttachDrivesOld compiler thinks
that @rc may be used uninitialized. Well, not directly, but maybe
after some optimization. Yet again, no harm in initializing a
variable.

In file included from ./util/virthread.h:26:0,
                 from ./datatypes.h:28,
                 from vbox/vbox_tmpl.c:43,
                 from vbox/vbox_V3_1.c:37:
vbox/vbox_tmpl.c: In function '_vboxAttachDrivesOld':
./util/virerror.h:181:5: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
     virReportErrorHelper(VIR_FROM_THIS, code, __FILE__,              \
     ^
In file included from vbox/vbox_V3_1.c:37:0:
vbox/vbox_tmpl.c:1041:14: note: 'rc' was declared here
     nsresult rc;
              ^
Yet again, one uninitialized variable:

qemu/qemu_driver.c: In function 'qemuDomainBlockCommit':
qemu/qemu_driver.c:17194:9: error: 'baseSource' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         qemuDomainPrepareDiskChainElement(driver, vm, baseSource,
         ^

And another one:

storage/storage_backend_logical.c: In function 'virStorageBackendLogicalMatchPoolSource.isra.2':
storage/storage_backend_logical.c:618:33: error: 'thisSource' may be used uninitialized in this function [-Werror=maybe-uninitialized]
                       thisSource->devices[j].path))
                                 ^
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Generates a false positive for Coverity, but it turns out there's no need
to check ret == -1 since if VIR_APPEND_ELEMENT is successful, the local
vol pointer is cleared anyway.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Using strcat directly is more readable than passing strlen
of the copied string to strncat.

Rather than have a unwieldy regex string - split it up into its components
each having it's own #define and then combine in a different #define
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Rather than a loop reallocating space to build the regex, just allocate
it once up front, then if there's more than 1 nextent, append a comma and
another regex_unit string.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

The 'stripes' value is described as the "Number of stripes or mirrors in
a logical volume". So add "mirror" and anything that starts with "raid"
to the list of segtypes that can have an 'nextents' value greater than one.
Use of raid segtypes (raid1, raid4, raid5*, raid6*, and raid10) is favored
over mirror in more recent lvm code.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Rather than preallocating a set number of elements, then walking through
the extents and adjusting the specific element in place, use the APPEND
macros to handle that chore.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Create a helper routine in order to parse any extents information
including the extent size, length, and the device string contained
within the generated 'lvs' output string.

A future patch would then be able to avoid the code more cleanly
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

When commit id '82c1740a' made changes to the output format (changing from
using a ',' separator to '#'), the examples in the lvs output from the
comments weren't changed.

Additionally, the two new fields added ('segtype' and 'stripes') were
not included in the output, leaving it well confusing.

This patch fixes the sample output, adds a 'striped' example, and makes
other comment related adjustments for long line and spacing between followup
'NB' remarks (while I'm there).
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Commit id '71b803ac' assumed that the storage pool source device path
was required for a 'logical' pool. This resulted in a failure to start
a pool without any device path defined.

So, adjust the virStorageBackendLogicalMatchPoolSource logic to
return success if at least the pool name matches the vgs output
when no pool source device path is/are provided.

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

Add a new helper virStorageBackendLogicalMatchPoolSource to compare the
pool's source name against the output from a 'pvs' command to list all
volume group physical volume data on the host.  In addition, compare the
pool's source device list against the particular volume group's device
list to ensure the source device(s) listed for the pool match what the
was listed for the volume group.

Then for pool startup or check API's we need to call this new API in
order to ensure that the pool we're about to start or declare active
during checkPool has a valid definition vs. the running host.

Rework virStorageBackendLogicalFindPoolSources a bit to create a
helper virStorageBackendLogicalGetPoolSources that will make the
pvs call in order to generate a list of associated pv_name and vg_name's.

A future patch will make use of this for start/check processing to
ensure the storage pool source definition matches expectations.

Similar to the openflags which allow VIR_STORAGE_VOL_OPEN_NOERROR to be
passed to avoid open errors, add a 'readflags' variable so that in the
future read failures could also be ignored.

We have macros for both positive and negative string matching.
Therefore there is no need to use !STREQ or !STRNEQ. At the same
time as we are dropping this, new syntax-check rule is
introduced to make sure we won't introduce it again.
Signed-off-by: NIshmanpreet Kaur Khera <khera.ishman@gmail.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>