Commit Graph

821 Commits

Author SHA1 Message Date
Ján Tomko
4bccdf0ceb storage: move buffer allocation inside virStorageBackendWipeExtentLocal
We do not need to pass a zero-filled buffer as an argument,
the function can allocate its own.
2015-12-17 12:44:35 +01:00
Ján Tomko
09cbfc0481 storage: fix return values of virStorageBackendWipeExtentLocal
Return -1:
* on all failures of fdatasync. Instead of propagating -errno
  all the way up to the virStorageVolWipe API, which is documented
  to return 0 or -1.
* after a partial wipe. If safewrite failed, we would re-use the
  non-negative return value of lseek (which should be 0 in this case,
  because that's the only offset we seek to).
2015-12-17 12:44:02 +01:00
John Ferlan
71b803ac9a storage: Add helper to compare logical pool def against pvs output
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.
2015-12-15 14:33:05 -05:00
John Ferlan
ae5519f7f8 storage: Create helper for virStorageBackendLogicalFindPoolSources
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.
2015-12-15 14:33:04 -05:00
John Ferlan
dae7007d6e storage: Check FS pool source during virStorageBackendFileSystemIsMounted
https://bugzilla.redhat.com/show_bug.cgi?id=1025230

When determining whether a FS pool is mounted, rather than assuming that
the FS pool is mounted just because the target.path is in the mount list,
let's make sure that the FS pool source matches what is mounted
2015-12-15 14:33:04 -05:00
John Ferlan
61c29fe56f storage: Refactor virStorageBackendFileSystemGetPoolSource
Refactor code to use standard return functioning with respect to setting
a ret value and going to cleanup.
2015-12-15 14:33:04 -05:00
John Ferlan
1d1330f37e storage: Create helper to generate FS pool source value
Refactor the code that builds the pool source string during the FS
storage pool mount to be a separate helper.

A future patch will use the helper in order to validate the mounted
FS matches the pool's expectation during poolCheck processing
2015-12-15 14:33:00 -05:00
Eric Blake
034e47c338 CVE-2015-5313: storage: don't allow '/' in filesystem volume names
The libvirt file system storage driver determines what file to
act on by concatenating the pool location with the volume name.
If a user is able to pick names like "../../../etc/passwd", then
they can escape the bounds of the pool.  For that matter,
virStoragePoolListVolumes() doesn't descend into subdirectories,
so a user really shouldn't use a name with a slash.

Normally, only privileged users can coerce libvirt into creating
or opening existing files using the virStorageVol APIs; and such
users already have full privilege to create any domain XML (so it
is not an escalation of privilege).  But in the case of
fine-grained ACLs, it is feasible that a user can be granted
storage_vol:create but not domain:write, and it violates
assumptions if such a user can abuse libvirt to access files
outside of the storage pool.

Therefore, prevent all use of volume names that contain "/",
whether or not such a name is actually attempting to escape the
pool.

This changes things from:

$ virsh vol-create-as default ../../../../../../etc/haha --capacity 128
Vol ../../../../../../etc/haha created
$ rm /etc/haha

to:

$ virsh vol-create-as default ../../../../../../etc/haha --capacity 128
error: Failed to create vol ../../../../../../etc/haha
error: Requested operation is not valid: volume name '../../../../../../etc/haha' cannot contain '/'

Signed-off-by: Eric Blake <eblake@redhat.com>
2015-12-11 16:34:53 -07:00
John Ferlan
a523770c32 storage: Ignore block devices that fail format detection
https://bugzilla.redhat.com/show_bug.cgi?id=1276198

Prior to commit id '98322052' failure to saferead the block device would
cause an error to be logged and the device to be skipped while attempting
to discover/create a stable target path for a new LUN (NPIV).

This was because virStorageBackendSCSIFindLUs ignored errors from
processLU and virStorageBackendSCSINewLun.

Ignoring the failure allowed a multipath device with an "active" and
"ghost" to be present on the host with the "ghost" block device being
ignored. This patch will return a -2 to the caller indicating the desire
to ignore the block device since it cannot be used directly rather than
fail the pool startup.
2015-12-09 16:31:15 -05:00
John Ferlan
b3df72c4dd storage: Add debug message
I found this useful while processing a volume that wouldn't end up
showing up in the resulting list of block volumes. In this case, the
partition type wasn't found in the disk_types table.
2015-12-09 16:31:14 -05:00
John Ferlan
1bc84b0a08 storage: Handle readflags errors
Similar to the openflags VIR_STORAGE_VOL_OPEN_NOERROR processing, if some
read processing operation fails, check the readflags for the corresponding
error flag being set. If so, rather then causing an error - use VIR_WARN
to flag the error, but return -2 which some callers can use to perform
specific actions. Use a new VIR_STORAGE_VOL_READ_NOERROR flag in a new
VolReadErrorMode enum.
2015-12-09 16:31:14 -05:00
John Ferlan
1edfce9b18 storage: Set ret = -1 on failures in virStorageBackendUpdateVolTargetInfo
While processing the volume for lseek, virFileReadHeaderFD, and
virStorageFileGetMetadataFromBuf - failure would cause an error,
but ret would not be set. That would result in an error message being
sent, but successful status being returned.
2015-12-09 16:31:14 -05:00
John Ferlan
af4028dccd storage: Add comments for backend APIs
Just so it's clearer what to expect upon input and what types of return
values could be generated.  These were loosely copied from existing
virStorageBackendUpdateVolTargetInfoFD.
2015-12-09 16:31:14 -05:00
John Ferlan
22346003dc storage: Add readflags for backend error processing
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.
2015-12-09 16:31:14 -05:00
John Ferlan
5e3ad0b775 storage: Change virStorageBackendVolOpen to use virFileOpenAs
https://bugzilla.redhat.com/show_bug.cgi?id=1282288

Rather than using just open on the path, allow for the possibility that
the path to be opened resides on an NFS root-squash target and was created
under a different uid/gid.

Without using virFileOpenAs an attempt to get the volume size data may fail
if the current user doesn't have permissions to read the volume, such as
would be the case if mode wasn't supplied in the volume XML and the default
VIR_STORAGE_DEFAULT_VOL_PERM_MODE (e.g. 0600) was used. Under this scenario
the owner/group is not root:root, thus this path run under root would fail
to open/read the volume.

NB: The virFileOpenAs code using OPEN_FORK will only work when the failure
is not EACESS/EPERM and the path resolves to a shared file system.
2015-11-20 17:07:13 -05:00
John Ferlan
ce6506b0bc storage: Really fix setting mode for backend exec in NFS root-squash env
https://bugzilla.redhat.com/show_bug.cgi?id=1282288

Although commit id '77346f27' resolves part of the problem regarding creating
a qemu-img image in an NFS root-squash environment, it really didn't fix the
entire problem. Unfortunately it only masked the problem. It seems qemu-img
must open/create the image using 0644, which if used by target.perms would
result in the chmod not being called since the mode desired and set match.

Although qemu-img could conceivably ignore the mode when creating, libvirt
has more knowledge of the environment and can make the adjustment to the
mode far more easily by using virFileOpenAs with VIR_FILE_OPEN_FORCE_MODE.
If that's successful, then we know on return the file will have the right
owner and mode, so we can declare success
2015-11-20 17:07:13 -05:00
John Ferlan
d3fa510a75 storage: Don't assume storage pool exists for FC/SCSI refresh thread
https://bugzilla.redhat.com/show_bug.cgi?id=1277781

The virStoragePoolFCRefreshThread had passed a pointer to the pool obj
in the virStoragePoolFCRefreshInfoPtr; however, we cannot assume that
the pool exists still since we don't keep the pool lock throughout
the duration of the thread.

Therefore, instead of passing the pool obj pointer, pass the UUID of
the pool and perform a lookup.  If found, then we can perform the
refresh using the locked pool obj pointer; otherwise, we just exit
the thread since the pool is now gone.
2015-11-12 06:30:33 -05:00
John Ferlan
c3afa6a9a3 storage: Introduce virStoragePoolObjFindPoolByUUID
Add a new API to search the currently defined pool list for a pool with
a matching UUID and return the locked pool object pointer.
2015-11-12 06:30:32 -05:00
John Ferlan
27432ba70c storage: Change cbdata scsi refresh thread field name
Change the field name from 'name' to 'fchost_name' to better id it.
2015-11-12 06:30:32 -05:00
John Ferlan
0c7a9b994c storage: Make active boolean
Since we treat it like a boolean, let's store it that way. At least one
path had already treated as true/false anyway.
2015-11-12 06:30:32 -05:00
John Ferlan
4cd7d220c9 storage: On 'buildVol' failure don't delete the volume
https://bugzilla.redhat.com/show_bug.cgi?id=1233003

Commit id 'fdda3760' only managed a symptom where it was possible to
create a file in a pool without libvirt's knowledge, so it was reverted.

The real fix is to have all the createVol API's which actually create
a volume (disk, logical, zfs) and the buildVol API's which handle the
real creation of some volume file (fs, rbd, sheepdog) manage deleting
any volume which they create when there is some sort of error in
processing the volume.

This way the onus isn't left up to the storage_driver to determine whether
the buildVol failure was due to some failure as a result of adjustments
made to the volume after creation such as getting sizes, changing ownership,
changing volume protections, etc. or simple a failure in creation.

Without needing to consider that the volume has to be removed, the
buildVol failure path only needs to remove the volume from the pool.
This way if a creation failed due to duplicate name, libvirt wouldn't
remove a volume that it didn't create in the pool target.
2015-11-04 07:21:11 -05:00
John Ferlan
0a6e709c95 Revert "storage: Prior to creating a volume, refresh the pool"
This reverts commit fdda37608a.

This commit only manages a symptom of finding a buildRet failure
where a volume was not listed in the pool, but someone created the
volume outside of libvirt in the pool being managed by libvirt.
2015-11-04 07:21:11 -05:00
John Ferlan
a1703557fd storage: Pull volume removal from pool in storageVolDeleteInternal
Create a helper function to remove volume from the pool.
2015-11-04 07:21:11 -05:00
John Ferlan
2265e7dd14 storage: Cleanup failures in virStorageBackendCreateRaw
After successfully returning from virFileOpenAs, if subsequent calls fail,
then we need to remove the file since our caller expects that failures after
creation will remove the created file.
2015-11-04 07:21:11 -05:00
John Ferlan
9345c2bfcf storage: Cleanup failures virStorageBackendCreateExecCommand
After a successful qemu-img/qcow-create of the backing file, if we
fail to stat the file, change it owner/group, or mode, then the
cleanup path should remove the file.
2015-11-04 07:21:11 -05:00
John Ferlan
77346f2787 storage: Fix setting mode in virStorageBackendCreateExecCommand
Currently the code does not handle the NFS root squash environment
properly since if the file gets created, then the subsequent chmod
will fail in a root squash environment where we're creating a file
in the pool with qemu tools, such as seen via:

   $ virsh vol-create-from $pool $file.xml file.img --inputpool $pool

assuming $file.xml is creating a file of "<format type='qcow2'"> from
an existing file.img in the pool of "<format type='raw'>".

This patch will utilize the virCommandSetUmask when creating the file
in the NETFS pool. The virCommandSetUmask API was added in commit id
'0e1a1a8c4', which was after the original code was developed in commit
id 'e1f27784' to attempt to handle the root squash environment.

Also, rather than blindly attempting to chmod, check to see if the
st_mode bits from the stat match what we're trying to set and only
make the chmod if they don't.

Also, a slight adjustment to the fallback algorithm to move the
virCommandSetUID/virCommandSetGID inside the if (!filecreated) since
they're only useful if we need to attempt to create the file again.
2015-11-04 07:21:11 -05:00
Wido den Hollander
3c7590e0a4 rbd: Remove snapshots if the DELETE_WITH_SNAPSHOTS flag has been provided
When a RBD volume has snapshots it can not be removed.

This patch introduces a new flag to force volume removal,
VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS.

With this flag any existing snapshots will be removed prior to
removing the volume.

No existing mechanism in libvirt allowed us to pass such information,
so that's why a new flag was introduced.

Signed-off-by: Wido den Hollander <wido@widodh.nl>
2015-10-27 16:12:12 -04:00
Ishmanpreet Kaur Khera
32cee5b2f0 Avoid using !STREQ and !STRNEQ
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: Ishmanpreet Kaur Khera <khera.ishman@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-10-21 15:03:35 +02:00
John Ferlan
1059c48180 storage: Rework error paths for virStorageBackendCreateExecCommand
Rework the code in order to use the "ret = -1;" and goto cleanup;
coding style.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2015-10-13 18:03:55 -04:00
John Ferlan
c4dd2a1faf storage: Track successful creation of LV for removal
https://bugzilla.redhat.com/show_bug.cgi?id=1233003

Track when the logical volume was successfully created in order to
properly handle the call to virStorageBackendLogicalDeleteVol. It's
possible that the failure to create was because someone created an
LV in the pool outside of libvirt's knowledge. In this case, we don't
want to delete that LV.  A subsequent or future refresh of the pool
will find the volume and cause an earlier failure

Signed-off-by: John Ferlan <jferlan@redhat.com>
2015-10-13 18:03:55 -04:00
John Ferlan
27d2d99fe7 storage: Fix a resource leak in storageVolCreateXML
Commit id '1b5685da' refactored the code to move buildvoldef inside
the buildVol conditional; however, the VIR_FREE of the memory was
left only when 'buildret' failed, thus we're leaking memory.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2015-10-13 18:03:55 -04:00
John Ferlan
6ab98a68b7 storage: Remove duplicitous refreshVol in Sheepdog buildVol
As of commit id '155ca616' a 'refreshVol' is called after a buildVol
succeeds in storageVolCreateXML, thus a volStorageBackendSheepdogRefreshVolInfo
call in virStorageBackendSheepdogBuildVol is no longer necessary.

Additionally, the 'conn' parameter becomes unused.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2015-10-13 18:03:55 -04:00
John Ferlan
9cb36def82 storage: Remove duplicitous refreshVol in RBD buildVol
As of commit id '155ca616' a 'refreshVol' is called after the buildVol
succeeds in storageVolCreateXML, thus the volStorageBackendRBDRefreshVolInfo
call in virStorageBackendRBDBuildVol is no longer necessary.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2015-10-13 18:03:55 -04:00
John Ferlan
5275c0f4a1 storage: Fix incorrect format for <disk> <auth> XML
https://bugzilla.redhat.com/show_bug.cgi?id=1256999

After creating a copy of the 'authdef' in a pool -> disk translation,
unconditionally clear the 'authType' in the resulting disk auth def
structure since that's used for a storage pool and not a disk.  This
ensures virStorageAuthDefFormat will properly format the <auth> XML
for a <disk> (e.g. it won't have a <auth type='%s'.../>).
2015-10-12 09:46:59 -04:00
John Ferlan
4ee3d4c285 storage: Perform some cleanup of calls
Cleanup calls to virStorageBackendCopyToFD a bit.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2015-10-08 07:06:51 -04:00
John Ferlan
1895b42114 storage: Adjust calculation of alloc/capacity for disk
https://bugzilla.redhat.com/show_bug.cgi?id=1247987

Calculation of the extended and logical partition values for the disk
pool is complex. As the bz points out an extended partition should have
it's allocation initialized to 0 (zero) and keep the capacity as the size
dictated by the extents read.  Then for each logical partition found,
adjust the allocation of the extended partition.

Finally, previous logic tried to avoid recalculating things if a logical
partition was deleted; however, since we now have special logic to handle
the allocation of the extended partition, just make life easier by reading
the partition table again - rather than doing the reverse adjustment.
2015-10-05 08:14:44 -04:00
John Ferlan
657f3bea8d storage: Introduce virStorageBackendDiskStartPool
https://bugzilla.redhat.com/show_bug.cgi?id=1251461

When 'starting' up a disk pool, we need to make sure the label on the
device is valid; otherwise, the followup refreshPool will assume the
disk has been properly formatted for use. If we don't find the valid
label, then refuse the start and give a proper reason.
2015-10-05 08:14:44 -04:00
John Ferlan
fba2076f43 storage: Add additional errors/checks for disk label
Let's check to ensure we can find the Partition Table in the label
and that libvirt actually recognizes that type; otherwise, when we
go to read the partitions during a refresh operation we may not be
reading what we expect.

This will expand upon the types of errors or reason that a build
would fail, so we can create more direct error messages.
2015-10-05 08:14:44 -04:00
John Ferlan
05c46f5c22 storage: Add param to check whether we can write a disk label
Modify virStorageBackendDiskValidLabel to add a 'writelabel' parameter.
While initially for the purpose of determining whether the label should
be written during DiskBuild, a future use during DiskStart could determine
whether the pool should be started using the label found. Augment the
error messages also to give a hint as to what someone may need to do
or why the command failed.
2015-10-05 08:14:44 -04:00
John Ferlan
2f177c5a41 storage: Refactor disk label checking
Create a new function virStorageBackendDiskValidLabel to handle checking
whether there is a label on the device and whether it's valid or not.
While initially for the purpose of determining whether the label can be
overwritten during DiskBuild, a future use during DiskStart could determine
whether the pool should be started using the label found.
2015-10-05 08:14:44 -04:00
John Ferlan
fdda37608a storage: Prior to creating a volume, refresh the pool
https://bugzilla.redhat.com/show_bug.cgi?id=1233003

Although perhaps bordering on a don't do that type scenario, if
someone creates a volume in a pool outside of libvirt, then uses that
same name to create a volume in the pool via libvirt, then the creation
will fail and in some cases cause the same name volume to be deleted.

This patch will refresh the pool just prior to checking whether the
named volume exists prior to creating the volume in the pool. While
it's still possible to have a timing window to create a file after the
check - at least we tried.  At that point, someone is being malicious.
2015-10-05 08:14:44 -04:00
Ján Tomko
1b5685dada Create a shallow copy for volume building only if supported
Since the previous commit, the shallow copy is only used inside
the if (backend->buildVol) if.
2015-09-29 10:45:01 +02:00
Ján Tomko
56a4e9cb61 Update pool allocation with new values on volume creation
Since commit e0139e3, we update the pool allocation with
the user-provided allocation values.

For qcow2, the allocation is ignored for volume building,
but we still subtracted it from pool's allocation.
This can result in interesting values if the user-provided
allocation is large enough:

Capacity:       104.71 GiB
Allocation:     109.13 GiB
Available:      16.00 EiB

We already do a VolRefresh on volume creation. Also refresh
the volume after creating and use the new value to update the pool.

https://bugzilla.redhat.com/show_bug.cgi?id=1163091
2015-09-29 10:45:01 +02:00
John Ferlan
1b046a6837 virfile: Rename virFileUnlink to virFileRemove
Similar to commit id '35847860', it's possible to attempt to create
a 'netfs' directory in an NFS root-squash environment which will cause
the 'vol-delete' command to fail.  It's also possible error paths from
the 'vol-create' would result in an error to remove a created directory
if the permissions were incorrect (and disallowed root access).

Thus rename the virFileUnlink to be virFileRemove to match the C API
functionality, adjust the code to following using rmdir or unlink
depending on the path type, and then use/call it for the VIR_STORAGE_VOL_DIR
2015-09-21 08:24:16 -04:00
John Ferlan
db9277a39b storage: Handle failure from refreshVol
Commit id '155ca616' added the 'refreshVol' API. In an NFS root-squash
environment it was possible that if the just created volume from XML wasn't
properly created with the right uid/gid and/or mode, then the followup
refreshVol will fail to open the volume in order to get the allocation/
capacity values. This would leave the volume still on the server and
cause a libvirtd crash because 'voldef' would be in the pool list, but
the cleanup code would free it.
2015-09-02 08:59:53 -04:00
John Ferlan
691dd388ae storage: Correct the 'mode' check
Commit id '7c2d65dde2' changed the default value of mode to be -1 if not
supplied in the XML, which should cause creation of the volume using the
default mode of VIR_STORAGE_DEFAULT_VOL_PERM_MODE; however, the check
made was whether mode was '0' or not to use default or provided value.

This patch fixes the issue to check if the 'mode' was provided in the XML
and use that value.
2015-09-02 08:59:53 -04:00
John Ferlan
35847860f6 virfile: Introduce virFileUnlink
In an NFS root-squashed environment the 'vol-delete' command will fail to
'unlink' the target volume since it was created under a different uid:gid.

This code continues the concepts introduced in virFileOpenForked and
virDirCreate[NoFork] with respect to running the unlink command under
the uid/gid of the child. Unlike the other two, don't retry on EACCES
(that's why we're here doing this now).
2015-09-02 08:59:53 -04:00
Guido Günther
269d39afe5 storage: only run safezero if allocation is > 0
While a zero allocation in safezero should be fine it isn't when we use
posix_fallocate which returns EINVAL on a zero allocation.

While we could skip the zero allocation in safezero_posix_fallocate it's
an optimization to do it for all allocations.

This fixes vm installation via virtinst for me which otherwise aborts
like:

   Starting install...
   Retrieving file linux...               | 5.9 MB     00:01 ...
   Retrieving file initrd.gz...           |  29 MB     00:07 ...
   ERROR    Couldn't create storage volume 'virtinst-linux.sBgds4': 'cannot fill file '/var/lib/libvirt/boot/virtinst-linux.sBgds4': Invalid argument'

The error was introduced by e30297b0 as spotted by Chunyan Liu
2015-08-24 13:51:44 +02:00
Chris J Arges
c6eea54008 storage: allow zero capacity with non-backing file to be created
In commit 155ca616e, a change was introduced that no longer allowed defining
volumes via XML with a capacity of '0'. Because we check for info.size_arg
to be non-zero, this use-case fails. This patch allows info.size_arg to be
zero if no backing store is specified.

Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
2015-07-24 11:22:20 -04:00
Christophe Fergeau
60d5ed8c52 storage: Fix pool building when directory already exists
Currently, when trying to virsh pool-define/virsh pool-build a new
'dir' pool, if the target directory already exists, virsh
pool-build/virStoragePoolBuild will error out. This is a change of
behaviour compared to eg libvirt 1.2.13

This is caused by the wrong type being used for the dir_create_flags
variable in virStorageBackendFileSystemBuild , it's defined as a bool
but is used as a flag bit field so should be unsigned int (this matches
the type virDirCreate expects for this variable).

This should fix https://bugzilla.gnome.org/show_bug.cgi?id=752417 (GNOME
Boxes) and https://bugzilla.redhat.com/show_bug.cgi?id=1244080
(downstream virt-manager).
2015-07-17 15:24:18 +02:00
John Ferlan
279238fea3 rbd: Return error from rbd_create for message processing
Resolving an error reporting bug introduced by commit id '761491e' which
just took the return of virStorageBackendRBDCreateImage and used it as
the basis for the message generated. This would generate EPERM regardless
of error seen.
2015-07-16 12:31:25 -04:00
Wido den Hollander
045cac32fd rbd: Use RBD format 2 by default when creating images.
We used to look at the librbd code version and depending on that
we would invoke rbd_create3() or rbd_create().

Since librbd version 0.67.9 we can however tell RBD that it should
create rbd format 2 images even if we invoke rbd_create().

The less options we pass to librbd, the more we can lean on the sane
defaults it uses.

For rbd_create3() we had things like the stripe count and unit hardcoded
in libvirt and that might cause problems down the road.

Signed-off-by: Wido den Hollander <wido@widodh.nl>
2015-07-16 12:31:20 -04:00
Prerna Saxena
dd519a294b Fix cloning of raw, sparse volumes
When virsh vol-clone is attempted on a raw file where capacity > allocation,
the resulting cloned volume has a size that matches the virtual-size of
the parent; in place of matching its actual, disk size.
This patch fixes the cloned disk to have same _allocated_size_ as
the parent file from which it was cloned.

Ref: http://www.redhat.com/archives/libvir-list/2015-May/msg00050.html

Also fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1130739

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2015-07-10 08:54:10 +02:00
Ján Tomko
e30297b096 Rewrite allocation tracking when cloning volumes
Instead of storing the remaining bytes, store the position of the first
unallocated byte. This will allow changing the amount of bytes copied
by virStorageBackendCopyToFD without changing the safezero call.

No functional impact.
2015-07-10 08:53:26 +02:00
Erik Skultety
b563787192 storage: Revert volume obj list updating after volume creation (4749d82a)
This patch reverts commit 4749d82a which tried to tweak the logic in
volume creation. We did realloc and update our object list before we executed
volume building within a specific storage backend. If that failed, we
had to update (again) our object list to the original state as it was before the
build and delete the volume from the pool (even though it didn't exist - this
truly depends on the backend).
I misunderstood the base idea to be able to poll the status of the volume
creation using vol-info. After commit 4749d82a this wasn't possible
anymore, although no BZ has been reported yet.

Commit 4749d82a also claimed to fix
https://bugzilla.redhat.com/show_bug.cgi?id=1223177, but commit c8be606b of the
same series as 4749d82ad (which was more of a refactor than a fix)
fixes the same issue so the revert should be pretty straightforward.
Further more, BZ https://bugzilla.redhat.com/show_bug.cgi?id=1241454 can be
fixed with this revert.
2015-07-09 13:23:27 +02:00
Erik Skultety
f92f31213a storage: Fix regression in storagePoolUpdateAllState
Commit 2a31c5f0 introduced support for storage pool state XMLs, however
it also introduced a regression:

if (!virstoragePoolObjIsActive(pool)) {
    virStoragePoolObjUnlock(pool);
    continue;
}

The idea behind this was that since we've got state XMLs and the pool
wasn't marked as active by autostart routine (if the autostart flag had been
set earlier), the pool is inactive and we can leave it be and continue with
other pools. However, filesystem type pools like fs,dir, possibly netfs are
supposed to be active if the filesystem is mounted on the host. And this is
exactly where the regression occurs, e.g. pool type 'dir' which has been
previously destroyed and marked as !autostart gets filtered out
by the condition above.
The resolution should be simply to remove the condition completely,
all pools will get their 'active' flag updated by check callback and if
they do not support such callback, the logic doesn't change and such
pools will be inactive by default (e.g. RBD, even if a state XML exists).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1238610
2015-07-08 12:21:25 +02:00
John Ferlan
dbad001899 mpath: Update path in CheckPool function
https://bugzilla.redhat.com/show_bug.cgi?id=1230664

Per the devmapper docs, use "/dev/mapper" or "/dev/dm-n" in order to
determine if a device is under control of DM Multipath.

So add "/dev/mapper" to the virFileExists, leaving the "/dev/mpath"
as a "legacy" option since it appears for a while it was the preferred
mechanism, but is no longer maintained
2015-06-30 08:47:02 -04:00
Prerna Saxena
7e7dee4389 Storage: Introduce shadow vol for refresh while the main vol builds.
Libvirt periodically refreshes all volumes in a storage pool, including
the volumes being cloned.
While cloning a storage volume from parent, we drop pool locks. Subsequent
volume refresh sometimes changes allocation for an ongoing copy, and leads
to corrupt images.
Fix: Introduce a shadow volume that isolates the volume object under refresh
from the base which has a copy ongoing.

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2015-06-30 14:29:38 +02:00
John Ferlan
e66a4c0b53 storage: Set correct vol->type at VolCreate
https://bugzilla.redhat.com/show_bug.cgi?id=1227664

If the requested format type for the new entry in the file system pool
is a 'dir', then be sure to set the vol->type correctly as would be done
when the pool is refreshed.
2015-06-30 06:49:49 -04:00
John Ferlan
1b695be173 scsi: Force error for SCSI pools on virStorageBackendSCSIFindLUs failure
Related to :

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

Rather than ignore the return status from virStorageBackendSCSIFindLUs,
cause a failure to start the pool if a -1 is returned. Issue was noted
during testing of the bz for iscsi that 'scsi' and 'fc' pools don't fail.
2015-06-24 09:18:59 -04:00
John Ferlan
31d3af6fea storage: Force setting of disk format type
Commit id '832a9256' adjusted the code to recognize when the default
type of "unknown" was provided as the format type and to use "dos" if
found. Since the pool is built with "dos" and it could cause some
confusion when formatting the XML after building by seeing "unknown"
in the output, let's just adjust the pool's setting to "dos" so that
subsequent formats will see the value.
2015-06-23 09:25:24 -04:00
Vasiliy Tolstov
ee4d2908dd update sheepdog client] update sheepdog client path
Nnever sheepdog versions have dog client binary
while old have collie. Check them both.

Signed-off-by: Vasiliy Tolstov <v.tolstov@selfip.ru>
2015-06-19 16:05:04 +02:00
John Ferlan
5c9fc1c11f scsi: Adjust return status from getBlockDevice
https://bugzilla.redhat.com/show_bug.cgi?id=1224233

Currently it's not possible to determine the difference between a
fatal memory allocation or failure to open/read the directory error
with a perhaps less fatal, I didn't find the "block" device in the
directory (which may be a disk entry without a block device).

In the case of the latter, we shouldn't cause failure to continue
searching in the caller (virStorageBackendSCSIFindLUs), rather we
should allow trying reading the next directory entry.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-06-18 17:14:00 +02:00
John Ferlan
29230951f1 storage: Generate correct parameters for CIFS
https://bugzilla.redhat.com/show_bug.cgi?id=1186969

When generating the path to the dir for a CIFS/Samba driver, the code
would generate a source path for the mount using "%s:%s" while the
mount.cifs expects to see "//%s/%s". So check for the cifsfs and
format the source path appropriately.

Additionally, since there is no means to authenticate, the mount
needs a "-o guest" on the command line in order to anonymously mount
the Samba directory.
2015-06-15 17:25:47 -04:00
John Ferlan
257250f764 storage: Adjust command arglist for gluster
In order for the glusterfs boolean to be set, the pool->def->type must be
VIR_STORAGE_POOL_NETFS, thus the check within virCommandNewArgList whether
pool->def->type is VIR_STORAGE_POOL_FS will never be true, so remove it
2015-06-15 17:25:47 -04:00
Michal Privoznik
c4bdfafcbb getOldStyleBlockDevice: Adjust formatting
Instead of initializing return value to zero (success) and overwriting
it on every failure just before the control jumps onto 'out' label,
let's initialize to an error value and set to zero only when we are
sure about the success. Just follow the pattern we have in the rest of
the code.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-06-15 14:13:39 +02:00
Michal Privoznik
71b66bec2b getNewStyleBlockDevice: Adjust formatting
Instead of initializing return value to zero (success) and overwriting
it on every failure just before the control jumps onto 'out' label,
let's initialize to an error value and set to zero only when we are
sure about the success. Just follow the pattern we have in the rest of
the code.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-06-15 14:13:34 +02:00
John Ferlan
84020f9a39 storage: Disallow wiping an extended disk partition
https://bugzilla.redhat.com/show_bug.cgi?id=1225694

Check if the disk partition to be wiped is the extended partition, if
so then disallow it. Do this via changing the wipeVol backend to check
the volume before passing to the common virStorageBackendVolWipeLocal
2015-06-15 07:45:06 -04:00
John Ferlan
1feaccf000 storage: Need to set secrettype for direct iscsi disk volume
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.
2015-06-15 07:14:40 -04:00
John Ferlan
c178d38b8f logical: Fix typo in error message 2015-06-09 18:21:57 -04:00
John Ferlan
94a1579b0a storage: Add check for valid FS types in checkPool callback
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.
2015-06-05 06:25:43 -04:00
John Ferlan
fcf0fd52cb storage: FS backend adjust error message on error path
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.
2015-06-05 06:25:19 -04:00
John Ferlan
5a8c98dbd9 storage: Refactor storage pool type checks
Refactor the code for both startPool (*Mount) and stopPool (*Unmount) code
paths by introducing virStorageBackendFileSystemIsValid.
2015-06-05 06:24:59 -04:00
John Ferlan
325a8134f9 storage: Remove extraneous @conn from function comments
Over time the parameters changed, but the comment wasn't updated
2015-06-05 06:07:50 -04:00
Erik Skultety
c8be606bae storage: RBD: do not return error when deleting non-existent volume
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.
2015-06-02 15:02:10 +02:00
Erik Skultety
4749d82a8b storage: Don't update volume objs list before we successfully create one
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
2015-06-02 15:02:02 +02:00
John Ferlan
6839b08ba1 storage: Fix problem with disk backend pool allocation calculation
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.
2015-05-28 13:32:16 -04:00
John Ferlan
48809204d1 storage: Don't adjust pool alloc/avail values for disk backend
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.
2015-05-28 13:32:16 -04:00
John Ferlan
6727bfd728 Revert "storage: Don't duplicate efforts of backend driver"
This reverts commit 2ac0e647bd.
2015-05-28 13:32:16 -04:00
Ján Tomko
8b316fe5da Fix shrinking volumes with the delta flag
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 055bbf4.

When the SHRINK flag was finally implemented for the first backend
in 1.2.13 (commit aa9aa6a), 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
2015-05-28 14:10:32 +02:00
Ján Tomko
7211f66ad7 Simplify allocation check in storageVolResize
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
2015-05-28 14:10:09 +02:00
Martin Kletzander
7d0481cb93 storage_fs: Create directory with UID if needed
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: Martin Kletzander <mkletzan@redhat.com>
2015-05-27 16:59:25 +02:00
Cole Robinson
db1140f117 storage: fs: Only force directory permissions if required
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.
2015-05-25 20:52:57 -04:00
Cole Robinson
7c2d65dde2 storage: conf: Don't set any default <mode> in the XML
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.
2015-05-25 20:52:55 -04:00
John Ferlan
2d0243f4d6 storage: Resolve Coverity FORWARD_NULL
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: John Ferlan <jferlan@redhat.com>
2015-05-24 07:01:48 -04:00
Cole Robinson
9ce409561a virfile: virDirCreate: Drop redundant FORCE_PERMS flag
The only two virDirCreate callers already use it
2015-05-19 19:29:39 -04:00
Cole Robinson
d6f8b35db5 storage: fs: Fill in permissions on pool refresh
This means pool XML actually reports accurate user/group/mode/label.

This uses UpdateVolTargetInfoFD in a bit of a hackish way, but it works
2015-05-04 12:56:38 -04:00
Cole Robinson
27a4c492f5 storage: fs: Don't overwrite virDirCreate error
virDirCreate will give us fine grained details about what actually failed.
2015-05-04 12:56:38 -04:00
Pavel Hrdina
ff3f93bcc2 use new macro helpers to check exclusive flags
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2015-05-04 09:20:00 +02:00
Ján Tomko
a41b1f196c iscsi: do not fail to stop a stopped pool
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
2015-04-30 13:05:10 +02:00
Cole Robinson
56476f6a2d storage: fs: Ignore volumes that fail to open with EACCESS/EPERM
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
2015-04-28 09:42:19 -04:00
Cole Robinson
65fc824666 storage: If driver startup state syncing fails, delete statefile
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.
2015-04-28 09:37:58 -04:00
Cole Robinson
af9dc75c1f storage: Break out storageDriverLoadPoolState
Will simplify a future patch
2015-04-28 09:37:57 -04:00
Cole Robinson
c180a3dcf7 storage: Don't leave stale state file if pool startup fails
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.
2015-04-28 09:37:57 -04:00
Cole Robinson
b29aff322f storage: Fix autostart dir for qemu:///session 2015-04-28 09:37:57 -04:00
John Ferlan
0259426060 storage: Resolve Coverity UNINIT
commit id '1e13eff4' didn't init found when changed from a bool to
an int in virStoragePoolFCRefreshThread and Coverity...
2015-04-27 06:57:51 -04:00
John Ferlan
9832205263 scsi: Adjust return values from processLU
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.
2015-04-21 10:20:17 -04:00
John Ferlan
1e13eff435 scsi: Change return values for virStorageBackendSCSIFindLUs
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.
2015-04-21 10:20:17 -04:00
John Ferlan
adb182fa2f scsi: Adjust return value for virStorageBackendSCSINewLun
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
2015-04-21 10:20:13 -04:00
John Ferlan
9e5e1ca144 storage: Fix check for stable path check
Fix the if (!STRPREFIX(path, "/dev")) to be if (!STRPREFIX(path, "/dev/"))
to ensure a path such as "/device" isn't declared stable.
2015-04-21 06:08:56 -04:00
John Ferlan
9126161d0b storage: Split out the stable path check
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
2015-04-21 06:08:05 -04:00
John Ferlan
0cf87b51d4 storage: Refactor virStorageBackendSCSINewLun
Invert the logical of retval and clean up code paths, especially goto's

Signed-off-by: John Ferlan <jferlan@redhat.com>
2015-04-16 12:28:04 -04:00
Ján Tomko
60db2bc80f Ignore storage volumes with control codes in their names
To prevent generating invalid XML.

https://bugzilla.redhat.com/show_bug.cgi?id=1066564
2015-04-15 18:41:20 +02:00
Ján Tomko
9146fd7aa9 Remove overengineered loop
Do not loop over enum with one value.
2015-04-13 15:59:20 +02:00
Ján Tomko
fbcf7da95b Introduce struct _virStorageBackendQemuImgInfo
This will contain the data required for creating the qemu-img
command line without having access to the volume definition.
2015-04-13 15:59:20 +02:00
Ján Tomko
a832735df9 Rename virStorageBackendCreateQemuImgCmd
Add FromVol at the end. This function will create the qemu-img
command line from volume definitions and check them.
2015-04-13 15:59:20 +02:00
John Ferlan
2ac0e647bd storage: Don't duplicate efforts of backend driver
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.
2015-04-09 19:04:18 -04:00
John Ferlan
1ffd82bb89 storage: Need to update freeExtent at delete primary partition
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
2015-04-09 19:04:18 -04:00
John Ferlan
1095230dee storage: Fix issues in storageVolResize
https://bugzilla.redhat.com/show_bug.cgi?id=1073305

When creating a volume in a pool, the creation allows the 'capacity'
value to be larger than the available space in the pool. As long as
the 'allocation' value will fit in the space, the volume will be created.

However, resizing the volume checks were made with the new absolute
capacity value against existing capacity + the available space without
regard for whether the new absolute capacity was actually allocating
space or not.  For example, a pool with 75G of available space creates
a volume of 10G using a capacity of 100G and allocation of 10G will succeed;
however, if the allocation used a capacity of 10G instead and then tried
to resize the allocation to 100G the code would fail to allow the backend
to try the resize.

Furthermore, when updating the pool "available" and "allocation" values,
the resize code would just "blindly" adjust them regardless of whether
space was "allocated" or just "capacity" was being adjusted.  This left
a scenario whereby a resize to 100G would fail; however, a resize to 50G
followed by one to 100G would both succeed.  Again, neither was adjusting
the allocation value, just the "capacity" value.

This patch adds more logic to the resize code to understand whether the
new capacity value is actually "allocating" space as well and whether it
shrinking or expanding. Since unsigned arithmatic is involved, the possibility
that we adjust the pool size values incorrectly is probable.

This patch also ensures that updates to the pool values only occur if we
actually performed the allocation.

NB: The storageVolDelete, storageVolCreateXML, and storageVolCreateXMLFrom
each only updates the pool allocation/availability values by the target
volume allocation value.
2015-04-09 19:04:18 -04:00
Erik Skultety
2a31c5f030 storage: Introduce storagePoolUpdateAllState function
The 'checkPool' callback was originally part of the storageDriverAutostart function,
but the pools need to be checked earlier during initialization phase,
otherwise we can't start a domain which mounts a volume after the
libvirtd daemon restarted. This is because qemuProcessReconnect is called
earlier than storageDriverAutostart. Therefore the 'checkPool' logic has been
moved to storagePoolUpdateAllState which is called inside storageDriverInitialize.

We also need a valid 'conn' reference to be able to execute 'refreshPool'
during initialization phase. Though it isn't available until storageDriverAutostart
all of our storage backends do ignore 'conn' pointer, except for RBD,
but RBD doesn't support 'checkPool' callback, so it's safe to pass
conn = NULL in this case.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
2015-04-07 16:22:40 +02:00
Erik Skultety
a9700771f5 conf: Introduce virStoragePoolLoadAllState && virStoragePoolLoadState
These functions operate exactly the same as their network equivalents
virNetworkLoadAllState, virNetworkLoadState.
2015-04-07 16:22:40 +02:00
Erik Skultety
723143a19c storage: Add support for storage pool state XML
This patch introduces new virStorageDriverState element stateDir.
Also adds necessary changes to storageStateInitialize, so that
directories initialization becomes more generic.
2015-04-07 16:22:40 +02:00
John Ferlan
f51fbdd19d scsi: Remove unused 'type_path' in processLU
Seems to be a remnant that was never cleaned up from original submit...

Signed-off-by: John Ferlan <jferlan@redhat.com>
2015-04-02 08:46:30 -04:00
John Ferlan
f9efcd9218 iscsi: Fix exit path for virStorageBackendISCSIFindLUs failure
If the call to virStorageBackendISCSIGetHostNumber failed, we set
retval = -1, but yet still called virStorageBackendSCSIFindLUs.
Need to add a goto cleanup - while at it, adjust the logic to
initialize retval to -1 and only changed to 0 (zero) on success.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2015-04-02 08:46:26 -04:00
John Ferlan
d9ece06526 iscsi: Use error message from virStorageBackendSCSIFindLUs
Don't supercede the error message virStorageBackendSCSIFindLUs as the
message such as "error: Failed to find LUs on host 60: ..." is not overly
clear as to what the real problem might be.

Signed-off-by: John Ferlan <jferlan@redhat.com>
2015-04-02 08:46:23 -04:00
Erik Skultety
cf7392a0d2 storage: Remove unused attribute conn from 'checkPool' callback
In order to be able to use 'checkPool' inside functions which do not
have any connection reference, 'conn' attribute needs to be discarded
from the checkPool's signature, since it's not used by any storage backend
anyway.
2015-04-02 11:57:07 +02:00
Ján Tomko
22fd3ac38f Introduce virBitmapIsBitSet
A helper that never returns an error and treats bits out of bitmap range
as false.

Use it everywhere we use ignore_value on virBitmapGetBit, or loop over
the bitmap size.
2015-03-13 15:31:33 +01:00
John Ferlan
30f69ae86b iscsi: Adjust error message for findStorageSources backend
The virStorageBackendISCSIFindPoolSources API only needs the 'host' name
in order to discover iSCSI pools, it returns the various device paths.
On input, it's also possible to further restrict a search by providing the
port attribute for the host element and the (undocumented) initiator element.

For example:

$  virsh find-storage-pool-sources-as iscsi
error: Failed to find any iscsi pool sources
error: invalid argument: hostname and device path must be specified for iscsi sources

$ virsh find-storage-pool-sources-as iscsi 192.168.122.1
<sources>
  <source>
    <host name='192.168.122.1' port='3260'/>
    <device path='iqn.2013-12.com.example:iscsi-chap-lclpool'/>
  </source>
</sources>
2015-03-02 22:57:27 -05:00
John Ferlan
832a9256b2 disk: Provide a default storage source format type.
https://bugzilla.redhat.com/show_bug.cgi?id=1181062

According to the formatstorage.html description for <source> element
and "format" attribute: "All drivers are required to have a default
value for this, so it is optional."

As it turns out the disk backend did not choose a default value, so I
added a default of "msdos" if the source type is "unknown" as well as
updating the storage.html backend disk volume driver documentation to
indicate the default format is dos.
2015-03-02 22:42:25 -05:00
Peter Krempa
1fcb9351d7 storage: sheepdog: Avoid skipping variable initialization
Commit 155ca616eb added a error message
that skips initialization of the 'cmd' variable. Fortunately it was not
released.
2015-03-02 10:09:49 +01:00
Ján Tomko
155ca616eb Allow creating volumes with a backing store but no capacity
The tool creating the image can get the capacity from the backing
storage. Just refresh the volume afterwards.

https://bugzilla.redhat.com/show_bug.cgi?id=958510
2015-03-02 08:07:11 +01:00
Ján Tomko
d3452a3f73 Revert "Restore skipping of setting capacity"
This reverts commit f1856eb622.

Now that we can update capacity from image metadata,
we don't need to skip the update.
2015-03-02 08:07:11 +01:00
Ján Tomko
a760ba3a7f Probe for capacity in virStorageBackendUpdateVolTargetInfo
Instead of just looking at the output of fstat, call
virStorageFileGetMetadata to get the full capacity from
image headers.

Note that the capacity is probed unconditionally. The updateCapacity
bool parameter is ignored and will be removed in the following commit.
2015-03-02 08:07:11 +01:00
Ján Tomko
e3f1d2a820 Allow cloning volumes with no capacity specified
In virStorageVolCreateXML, add VIR_VOL_XML_PARSE_NO_CAPACITY
to the call parsing the XML of the new volume to make the capacity
optional.

If the capacity is omitted, use the capacity of the old volume.
We already do that for values that are less than the original
volume capacity.
2015-03-02 08:07:11 +01:00
Ján Tomko
cbd788eba6 Add flags argument to virStorageVolDefParse*
Allow the callers to pass down libvirt-internal flags.
2015-03-02 08:07:11 +01:00
Jiri Denemark
bc6e206322 Search for schemas and cpu_map.xml in source tree
Not all files we want to find using virFileFindResource{,Full} are
generated when libvirt is built, some of them (such as RNG schemas) are
distributed with sources. The current API was not able to find source
files if libvirt was built in VPATH.

Both RNG schemas and cpu_map.xml are distributed in source tarball.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2015-02-19 15:25:04 +01:00
Daniel P. Berrange
aa9aa6a975 Allow shrinking of file based volumes
While the main storage driver code allows the flag
VIR_STORAGE_VOL_RESIZE_SHRINK to be set, none of the backend
drivers are supporting it. At the very least this can work
for plain file based volumes since we just ftruncate() them
to the new size. It does not work with qcow2 volumes, but we
can arguably delegate to qemu-img for error reporting for that
instead of second guessing this for ourselves:

$ virsh vol-resize --shrink /home/berrange/VirtualMachines/demo.qcow2 2G
error: Failed to change size of volume 'demo.qcow2' to 2G

error: internal error: Child process (/usr/bin/qemu-img resize /home/berrange/VirtualMachines/demo.qcow2 2147483648) unexpected exit status 1: qemu-img: qcow2 doesn't support shrinking images yet
qemu-img: This image does not support resize

See also https://bugzilla.redhat.com/show_bug.cgi?id=1021802
2015-02-12 11:11:52 +00:00
John Ferlan
1d2e4d8ca2 storage: Need to clear pool prior to refreshPool during Autostart
https://bugzilla.redhat.com/show_bug.cgi?id=1176510

When storageDriverAutostart is called path virStateReload via a 'service
libvirtd reload', then because the volume list in the pool wasn't cleared
prior to the call, each volume would be listed multiple times (as many
times as we reload). I believe the issue would be introduced by commit
id '9e093f0b' at least for the libvirtd reload path, although I suppose
the introduction of virStateReload (commit id '70da0494') could be a
different cause.

Thus like other places prior to calling refreshPool, we need to call
virStoragePoolObjClearVols
2015-01-31 07:56:15 -05:00
John Ferlan
9bbbb91216 storage: Check the partition name against provided name
https://bugzilla.redhat.com/show_bug.cgi?id=1138516

If the provided volume name doesn't match what parted generated as the
partition name, then return a failure.

Update virsh.pod and formatstorage.html.in to describe the 'name' restriction
for disk pools as well as the usage of the <target>'s <format type='value'>.
2015-01-28 17:28:03 -05:00
John Ferlan
471e1c4e2a storage: When delete extended partition, need to refresh pool
When removing a volume that is the extended partition, all the logical
volume partitions that exist within the extended partition will also be
removed, so we need to refresh the pool to have the updated list
2015-01-28 17:28:03 -05:00
John Ferlan
bce671b731 storage: Adjust how to refresh extended partition disk data
During virStorageBackendDiskMakeDataVol processing, if we find an extended
partition, then handle it specially when updating the capacity/allocation
rather than calling virStorageBackendUpdateVolInfo.

As it turns out, once a logical partition exists, any attempt to refresh
the pool or after libvirtd restart/reload will result in a failure to open
the extended partition device resulting in the inability to start the pool.
The downside to this is we will lose the <permissions> and <timestamps> for
the extended partition upon subsequent restart, refresh, reload since the
stat() in virStorageBackendUpdateVolTargetInfoFD will not be called. However,
since it's really only a container and shouldn't directly be used for
storage that seems reasonable.

Therefore, only use the existing code that already had a comment about
getting the allocation wrong for extended partitions for just the setting
of the extended partition data.
2015-01-28 17:28:03 -05:00
John Ferlan
a0d88ed4e7 storage: Fix check for partition type for disk backing volumes
While checking the existing partitions in virStorageBackendDiskPartFormat,
the code would erroneously compare the volume target format type (eg, the
virStoragePartedFsType) rather than the source partition type (eg, the
virStorageVolTypeDisk) which is set during virStorageBackendDiskReadPartitions.
2015-01-28 17:28:03 -05:00
John Ferlan
290ffcfbbc storage: Attempt error recovery in virStorageBackendDiskCreateVol
During virStorageBackendDiskCreateVol if virStorageBackendDiskReadPartitions
fails, then we were leaving with an error and a partition on the disk for
which there was no corresponding volume and used space on the disk which
could be reclaimable through direct parted activity. On a subsequent restart,
reload, or refresh the volume may magically appear too.
2015-01-28 17:28:03 -05:00
John Ferlan
1e79ad6d35 storage: Move virStorageBackendDiskDeleteVol
Move the API to before virStorageBackendDiskCreateVol in order to be
able to call the DeleteVol API when virStorageBackendDiskReadPartitions
fails so that we don't by chance leave a partition on the disk.
2015-01-28 17:28:03 -05:00
Chen Hanxiao
95da191376 storage: add a flag to clone files on btrfs
When creating a RAW file, we don't take advantage
of clone of btrfs.

Add a VIR_STORAGE_VOL_CREATE_REFLINK flag to request
a reflink copy.

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2015-01-27 13:41:14 +01:00
Chen Hanxiao
466b29c8c3 storage: introduce btrfsCloneFile() for COW copy
Add a wrapper for BTRFS_IOC_CLONE ioctl.

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2015-01-27 13:24:10 +01:00
Daniel P. Berrange
55ea7be7d9 Removing probing of secondary drivers
For stateless, client side drivers, it is never correct to
probe for secondary drivers. It is only ever appropriate to
use the secondary driver that is associated with the
hypervisor in question. As a result the ESX & HyperV drivers
have both been forced to do hacks where they register no-op
drivers for the ones they don't implement.

For stateful, server side drivers, we always just want to
use the same built-in shared driver. The exception is
virtualbox which is really a stateless driver and so wants
to use its own server side secondary drivers. To deal with
this virtualbox has to be built as 3 separate loadable
modules to allow registration to work in the right order.

This can all be simplified by introducing a new struct
recording the precise set of secondary drivers each
hypervisor driver wants

struct _virConnectDriver {
    virHypervisorDriverPtr hypervisorDriver;
    virInterfaceDriverPtr interfaceDriver;
    virNetworkDriverPtr networkDriver;
    virNodeDeviceDriverPtr nodeDeviceDriver;
    virNWFilterDriverPtr nwfilterDriver;
    virSecretDriverPtr secretDriver;
    virStorageDriverPtr storageDriver;
};

Instead of registering the hypervisor driver, we now
just register a virConnectDriver instead. This allows
us to remove all probing of secondary drivers. Once we
have chosen the primary driver, we immediately know the
correct secondary drivers to use.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2015-01-27 12:02:04 +00:00
Ján Tomko
1390c26847 safezero: fall back to writing zeroes even when resizing
Remove the resize flag and use the same code path for all callers.
This flag was added by commit 18f0316 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 18f0316.
2015-01-09 13:48:23 +01:00
John Ferlan
cafb934db8 logical: Add "--type snapshot" to lvcreate command
A recent lvm change has resulted in a change for the "default" type of
logical volume created when the "--virtualsize" or "--V" is supplied on
the command line (e.g. when the allocation and capacity values of a to
be created volume differ). It seems that at the very least the following
change adjusts the default type:

https://git.fedorahosted.org/cgit/lvm2.git/commit/?id=e0164f21

and the following may also have some impact.

https://git.fedorahosted.org/cgit/lvm2.git/commit/?id=87fc3b71

When using the virsh vol-create-as or vol-create xmlfile commands, the
result is that libvirt will now create a "thin logical volume" and a
"thin logical volume pool" rather than just a "thin snapshot logical
volume". For example the following sequence:

  # lvcreate --name test -L 2M -V 5M lvm_test
    Rounding up size to full physical extent 4.00 MiB
    Rounding up size to full physical extent 8.00 MiB
    Logical volume "test" created.
  # lvs lvm_test
    LV    VG       Attr       LSize Pool  Origin Data%  Meta%  Move Log Cpy%Sync Convert
    lvol1 lvm_test twi-a-tz-- 4.00m              0.00   0.98
    test  lvm_test Vwi-a-tz-- 8.00m lvol1        0.00

compared to the former code which had the following:

    LV   VG       Attr       LSize  Pool Origin         Data%  Move Log Cpy%Sync Convert
    test LVM_Test swi-a-s---  4.00m      [test_vorigin]   0.00

Since libvirt doesn't know how to parse the thin logical volume
and pool, it will fail to find the newly created volume and pool
even though it exists in the volume group.

It cannot find since the command used to find/parse returns a thin volume
'test' with no associated device, for example the output is:

  lvol1##UgUwkp-fTFP-C0rc-ufue-xrYh-dkPr-FGPFPx#lvol1_tdata(0)#thin-pool#1#4194304#4194304#4194304#twi-a-tz--
  test##NcaIoH-4YWJ-QKu3-sJc3-EOcS-goff-cThLIL##thin#0#8388608#4194304#8388608#Vwi-a-tz--

as compared to the former which had the following:

      test#[test_vorigin]#Dt5Of3-4WE6-buvw-CWJ4-XOiz-ywOU-YULYw6#/dev/sda3(1300)#linear#1#4194304#4194304#4194304#swi-a-s---

While it's possible to generate code to handle the new thin lv and pool, this
patch will add a "--type snapshot" onto the lvcreate command libvirt uses
in order to "for now" be able to continue to utilize the thin snapshots
2014-12-17 06:14:21 -05:00
John Ferlan
18f03166fd virstoragefile: Have virStorageFileResize use safezero
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: John Ferlan <jferlan@redhat.com>
2014-12-16 13:11:35 -05:00
Hao Liu
9788007892 storage: Check stderr when matching parted output
In old version of parted like parted-2.1-25, error message is shown in
stdout when printing a disk info without disk label.

    Error: /dev/sda: unrecognised disk label

This line has been moved to stderr in newer version of parted. So we
should check both stdout and stderr when locating this message.

This should fix bug:
    https://bugzilla.redhat.com/show_bug.cgi?id=1172468

Signed-off-by: Hao Liu <hliu@redhat.com>
2014-12-10 10:55:23 +01:00
Peter Krempa
8ef4f598f1 storage: Fix printing/casting of uid_t/gid_t
Other parts of libvirt use "%u" for formatting uid/gid and typecast to
unsigned int. Storage driver used the signed variant.
2014-12-08 11:36:29 +01:00
Peter Krempa
3b31cbc558 storage: backend: Log uid/gid when initializing storage file backend
To ease debugging permission problems add uid/gid values to the debug
message when initializing a storage file backend.
2014-12-05 10:07:17 +01:00
Luyao Huang
87b9437f89 storage: fix crash caused by no check return before set close
https://bugzilla.redhat.com/show_bug.cgi?id=1087104#c5

When trying to use an invalid offset to virStorageVolUpload(), libvirt
fails in virFDStreamOpenFileInternal(), although it seems libvirt does
not check the return in storageVolUpload(), and calls
virFDStreamSetInternalCloseCb() right after.  But stream doesn't have a
privateData (is NULL) yet, and the daemon crashes then.

0  0x00007f09429a9c10 in pthread_mutex_lock () from /lib64/libpthread.so.0
1  0x00007f094514dbf5 in virMutexLock (m=<optimized out>) at util/virthread.c:88
2  0x00007f09451cb211 in virFDStreamSetInternalCloseCb at fdstream.c:795
3  0x00007f092ff2c9eb in storageVolUpload at storage/storage_driver.c:2098
4  0x00007f09451f46e0 in virStorageVolUpload at libvirt.c:14000
5  0x00007f0945c78fa1 in remoteDispatchStorageVolUpload at remote_dispatch.h:14339
6  remoteDispatchStorageVolUploadHelper at remote_dispatch.h:14309
7  0x00007f094524a192 in virNetServerProgramDispatchCall at rpc/virnetserverprogram.c:437

Signed-off-by: Luyao Huang <lhuang@redhat.com>
2014-12-03 17:36:07 +01:00
Michal Privoznik
5ab746b83a storage: Introduce storagePoolLookupByTargetPath
While this could be exposed as a public API, it's not done yet as
there's no demand for that yet. Anyway, this is just preparing
the environment for easier volume creation on the destination.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-12-02 17:51:57 +01:00
John Ferlan
a0b13d35e7 Replace virSecretFree with virObjectUnref
Since virSecretFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
2014-12-02 11:03:41 -05:00
John Ferlan
adbbff5fb7 Replace virStoragePoolFree with virObjectUnref
Since virStoragePoolFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
2014-12-02 11:03:40 -05:00
John Ferlan
d1219054e3 Replace virStorageVolFree with virObjectUnref
Since virStorageVolFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
2014-12-02 11:03:40 -05:00
John Ferlan
b09ff13848 storage: Add mixed fc_host/scsi_host duplicate adapter source checks
https://bugzilla.redhat.com/show_bug.cgi?id=1159180

The virStoragePoolSourceFindDuplicate only checks the incoming definition
against the same type of pool as the def; however, for "scsi_host" and
"fc_host" adapter pools, it's possible that either some pool "scsi_host"
adapter definition is already using the scsi_hostN that the "fc_host"
adapter definition wants to use or some "fc_host" pool adapter definition
is using a vHBA scsi_hostN or parent scsi_hostN that an incoming "scsi_host"
definition is trying to use.

This patch adds the mismatched type checks and adds extraneous comments
to describe what each check is determining.

This patch also modifies the documentation to be describe what scsi_hostN
devices a "scsi_host" source adapter should use and which to avoid. It also
updates the parent definition to specifically call out that for mixed
environments it's better to define which parent to use so that the duplicate
pool checks can be done properly.
2014-12-01 10:04:25 -05:00
John Ferlan
7b4cdb6eaa storage: Move and rename getVhbaSCSIHostParent
https://bugzilla.redhat.com/show_bug.cgi?id=1159180

Move the API from the backend to storage_conf and rename it to
virStoragePoolGetVhbaSCSIHostParent.  A future patch will need to
use this functionality from storage_conf
2014-12-01 10:04:19 -05:00