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

Commit id '1b4eaa61' added the ability to have a mode='direct' for
an iscsi disk volume.  It relied on virStorageTranslateDiskSourcePool
in order to copy any disk source pool authentication information to
the direct disk volume, but it neglected to also copy the 'secrettype'
field which ends up being used in the domain volume formatting code.
Adding a secrettype for this case will allow for proper formatting later
and allow disk snapshotting to work properly

Additionally libvirtd restart processing would fail to find the domain
since the translation processing code is run after domain xml processing,
so handle the the case where the authdef could have an empty secrettype
field when processing the auth and additionally ignore performing the
actual and expected auth secret type checks for a DISK_VOLUME since that
data will be reassembled later during translation processing of the
running domain.

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

The virStorageBackendFileSystemIsMounted is called from three source paths
checkPool, startPool, and stopPool. Both start and stop validate the FS
fields before calling *IsMounted; however the check path there is no call.
This could lead the code into returning a true in "isActive" if for some
reason the target path for the pool was mounted. The assumption being
that if it was mounted, then we believe we started/mounted it.

It's also of note that commit id '81165294' added an error message for
the start/mount path regarding that the target is already mounted so
fail the start. That check was adjusted by commit id '13fde7ce' to
only message if actually mounted.

At one time this led to the libvirtd restart autostart code to declare
that the pool was active even though the startPool would inhibit startup
and the stopPool would inhibit shutdown. The autostart path changed as
of commit id '2a31c5f0' as part of the keep storage pools started between
libvirtd restarts.

This patch adds the same check made prior to start/mount and stop/unmount
to ensure we have a valid configuration before attempting to see if the
target is already mounted to declare "isActive" or not. Finding an improper
configuration will now cause an error at checkPool, which should make it
so we can no longer be left in a situation where the pool was started and
we have no way to stop it.

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

Currently the assumption on the error message is that there are
no source device paths defined when the number of devices check
fails, but in reality the XML could have had none or it could have
had more than the value supported. Adjust the error message accordingly
to make it clearer what the error really is.

Refactor the code for both startPool (*Mount) and stopPool (*Unmount) code
paths by introducing virStorageBackendFileSystemIsValid.

Over time the parameters changed, but the comment wasn't updated

RBD API returns negative value of errno, in that case we can silently
ignore if RBD tries to delete a non-existent volume, just like FS
backend does.

We do update pool volume object list before we actually create any
volume. If buildVol fails, we then try to delete the volume in the
storage as well as remove it from our structures. The problem is, that
any backend that supports both buildVol and deleteVol would fail in this
case which is completely unnecessary. This patch causes the update to
take place after we know a volume has been created successfully, thus no
removal in case of a buildVol failure is necessary.

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

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

The disk pool recalculates the pool allocation, capacity, and available
values each time through processing a newly created disk partition. This
created an issue with the allocation setting since the code used is shared
with the refresh path. Each path calls virStorageBackendDiskReadPartitions
which initializes the pool values and then processes the partition table
from the 'libvirt_parthelper' utility output with the only difference being
create passes a specific volume to be processed while refresh pass a NULL
indicating to process all volumes. That passed volume is check during the
virStorageBackendDiskMakeVol call to see if the current partition described
by the volume key already exists. If it exists, then no adjustments are
made to the allocation and the next entry in the output is checked.

For the create path this resulted in only the most recently created
partition size would be accounted for in the 'allocation' setting. This
patch thus checks whether the incoming volume is NULL before clearing
the pool allocation value.

Commit id '2ac0e647' for https://bugzilla.redhat.com/show_bug.cgi?id=1206521
was meant to be a generic check for the CreateVol, CreateVolFrom, and
DeleteVol paths to check if the storage backend's changed the pool's view
of allocation or available values.

Unfortunately as it turns out this caused a side effect when the disk backend
created an extended partition there would be no actual storage removed from
the pool, thus the changes would not find any change in allocation or
available and incorrectly update the pool values using the size of the
extended partition. A subsequent refresh of the pool would reset the
values appropriately.

This patch modifies those checks in order to specifically not update the
pool allocation and available for only the disk backend rather than be
generic before and after checks.

This reverts commit 2ac0e647.

This never worked.

In 0.9.10 when this API was introduced, it was intended that
the SHRINK flag combined with DELTA would shrink the volume by
the specified capacity (to avoid passing negative numbers).
See commit 055bbf45.

When the SHRINK flag was finally implemented for the first backend
in 1.2.13 (commit aa9aa6a9), it was only implemented for the absolute
values and with the delta flag the volume is always extended,
regardless of the SHRINK flag.

Treat the SHRINK flag as a minus sign when used together with DELTA,
to allow shrinking volumes as was documented in the API since 0.9.10.

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

Since shrinking a volume below existing allocation is not allowed,
it is not possible for a successful resize with VOL_RESIZE_ALLOCATE
to increase the pool's available value.

Even with the SHRINK flag it is possible to extend the current
allocation or even the capacity. Remove the overflow when
computing delta with this flag and do the check even if the
flag was specified.

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

The code already exists there, it just modified different flags.  I just
noticed this when looking at the code.  This patch is better to view
with bigger context or '-W'.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Only set directory permissions at pool build time, if:

- User explicitly requested a mode via the XML
- The directory needs to be created
- We need to do the crazy NFS root-squash workaround

This allows qemu:///session to call build on an existing directory
like /tmp.

The XML parser sets a default <mode> if none is explicitly passed in.
This is then used at pool/vol creation time, and unconditionally reported
in the XML.

The problem with this approach is that it's impossible for other code
to determine if the user explicitly requested a storage mode. There
are some cases where we want to make this distinction, but we currently
can't.

Handle <mode> parsing like we handle <owner>/<group>: if no value is
passed in, set it to -1, and adjust the internal consumers to handle
it.

Coverity points out it's possible for one of the virCommand{Output|Error}*
API's to have not allocated 'output' and/or 'error' in which case the
strstr comparison will cause a NULL deref
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

The only two virDirCreate callers already use it

This means pool XML actually reports accurate user/group/mode/label.

This uses UpdateVolTargetInfoFD in a bit of a hackish way, but it works

virDirCreate will give us fine grained details about what actually failed.

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

Just as we allow stopping filesystem pools when they were unmounted
externally, do not fail to stop an iscsi pool when someone else
closed the session externally.

Reported at:
https://bugzilla.redhat.com/show_bug.cgi?id=1171984

Trying to use qemu:///session to create a storage pool pointing at
/tmp will usually fail with something like:

$ virsh pool-start tmp
error: Failed to start pool tmp
error: cannot open volume '/tmp/systemd-private-c38cf0418d7a4734a66a8175996c384f-colord.service-kEyiTA': Permission denied

If any volume in an FS pool can't be opened by the daemon, the refresh
fails, and the pool can't be used.

This causes pain for virt-install/virt-manager though. Imaging a user
downloads a disk image to /tmp. virt-manager wants to import /tmp as
a storage pool, so we can detect what disk format it is, and set the
XML correctly. However this case will likely fail as explained above.

Change the logic here to skip volumes that fail to open. This could
conceivably cause user complaints along the lines of 'why doesn't
libvirt show $ROOT-OWNED-VOLUME-FOO', but figuring that currently
the pool won't even startup, I don't think there are any current
users that care about that case.

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

If you end up with a state file for a pool that no longer starts up
or refreshes correctly, the state file is never removed and adds
noise to the logs everytime libvirtd is started.

If the initial state syncing fails, delete the statefile.

Will simplify a future patch

After pool startup we call refreshPool(). If that fails, we leave
a stale pool state file hanging around.

Hit this trying to create a pool with qemu:///session containing
root owned files.

commit id '1e13eff4' didn't init found when changed from a bool to
an int in virStoragePoolFCRefreshThread and Coverity...

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

Adjust the processLU error returns to be a bit more logical. Currently,
the calling code cannot determine the difference between a non disk/lun
volume and a processed/found disk/lun. It can also not differentiate
between perhaps real/fatal error and one that won't necessarily stop
the code from finding other volumes.

After this patch virStorageBackendSCSIFindLUsInternal will stop processing
as soon as a "fatal" message occurs rather than continuting processing
for no apparent reason. It will also only set the *found value when
at least one of the processLU's was successful.

With the failed return, if the reason for the stop was that the pool
target path did not exist, was /dev, was /dev/, or did not start with
/dev, then iSCSI pool startup and refresh will fail.

Rather than passing/returning a pointer to a boolean to indicate that
perhaps we should try again - adjust the return of the call to return
the count of LU's found during processing, then let the caller decide
what to do with that value.

Use virStorageBackendPoolUseDevPath API to determine whether creation of
stable target path is possible for the volume.

This will differentiate a failed virStorageBackendStablePath which won't
need to be fatal. Thus, we'll add a -2 return value to differentiate that
the failure was a result of either the inability to find the symlink for
the device or failure to open the target path directory

Fix the if (!STRPREFIX(path, "/dev")) to be if (!STRPREFIX(path, "/dev/"))
to ensure a path such as "/device" isn't declared stable.

For virStorageBackendStablePath, in order to make decisions in other code
split out the checks regarding whether the pool's target is empty, using /dev,
using /dev/, or doesn't start with /dev

Invert the logical of retval and clean up code paths, especially goto's
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

To prevent generating invalid XML.

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

Do not loop over enum with one value.

This will contain the data required for creating the qemu-img
command line without having access to the volume definition.

Add FromVol at the end. This function will create the qemu-img
command line from volume definitions and check them.

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

If the backend driver updates the pool available and/or allocation values,
then the storage_driver VolCreateXML, VolCreateXMLFrom, and VolDelete APIs
should not change the value; otherwise, it will appear as if the values
were "doubled" for each change.  Additionally since unsigned arithmetic will
be used depending on the size and operation, either or both values could be
appear to be much larger than they should be (in the EiB range).

Currently only the disk pool updates the values, but other pools could.
Assume a "fresh" disk pool of 500 MiB using /dev/sde:

$ virsh pool-info disk-pool
...
Capacity:       509.88 MiB
Allocation:     0.00 B
Available:      509.84 MiB

$ virsh vol-create-as disk-pool sde1 --capacity 300M

$ virsh pool-info disk-pool
...
Capacity:       509.88 MiB
Allocation:     600.47 MiB
Available:      16.00 EiB

Following assumes disk backend updated to refresh the disk pool at deletion
of primary partition as well as extended partition:

$ virsh vol-delete --pool disk-pool sde1
Vol sde1 deleted

$ virsh pool-info disk-pool
...
Capacity:       509.88 MiB
Allocation:     9.73 EiB
Available:      6.27 EiB

This patch will check if the backend updated the pool values and honor that
update.

Commit id '471e1c4e' only considered updating the pool if the extended
partition was removed. As it turns out removing a primary partition
would also need to update the freeExtent list otherwise the following
sequence would fail (assuming a "fresh" disk pool for /dev/sde of 500M):

$  virsh pool-info disk-pool
...
Capacity:       509.88 MiB
Allocation:     0.00 B
Available:      509.84 MiB

$ virsh vol-create-as disk-pool sde1 --capacity 300M
$ virsh vol-delete --pool disk-pool sde1
$ virsh vol-create-as disk-pool sde1 --capacity 300M
error: Failed to create vol sde1
error: internal error: no large enough free extent

$

This patch will refresh the pool, rereading the partitions, and
return