There seems to be no need to add the ignore_value wrapper or
caste with (void) to the unlink() calls, so let's just remove
them. I assume at one point in time Coverity complained. So,
let's just be consistent - those that care to check the return
status can and those that don't can just have the naked unlink.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1614283
Save the error from the refresh failure because the stopPool
processing may overwrite the error or even worse clear it
due to calling an external libvirt API that resets the last
error such as is the case with the SCSI pool which may call
virGetConnectNodeDev (see commit decaeb288) in order to
process deleting an NPIV vport.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Create a common pool refresh failure handling method as the
same code is repeated multiple times.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Rather than duplicate the error code, let's create an error
label to keep code common.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Alter the code path to remove the need to to go cleanup and thus
remove the label completely.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
If the virStoragePoolRefresh fails and we call stopPool, the
code neglected to clean up the state file leading to the next
libvirtd restart attempting to start the pool. For a transient
pool this could make it unexpectedly reappear.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
All of the ones being removed are pulled in by internal.h. The only
exception is sanlock which expects the application to include <stdint.h>
before sanlock's headers, because sanlock prototypes use fixed width
int, but they don't include stdint.h themselves, so we have to leave
that one in place.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
It doesn't really make sense for us to have stdlib.h and string.h but
not stdio.h in the internal.h header.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1613737
When processing the inputvol for encryption, we need to handle
the case where the inputvol is encrypted. This then allows for
the encrypted inputvol to be used either for an output encrypted
volume or an output volume of some XML provided type.
Add tests to show the various conversion options when either input
or output is encrypted. This includes when both are encrypted.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 39cef12a9 altered/fixed the inputvol processing to create
a multistep process when using an inputvol to create an encrypted
output volume; however, it unnecessarily assumed/restricted the
inputvol to be of 'raw' format only.
Modify the processing code to allow the inputvol format to be checked
and used in order to create the encrypted volume.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
There's really no need for it to be there since it's only ever
used inside virStorageBackendCreateQemuImgCmdFromVol
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
As advertised in previous commit, there are three APIs that might
run for quite some time (because they read/write data from/to a
volume) and these three are: downloadVol, uploadVol, wipeVol.
Release pool object lock and reacquire it later to allow more
concurrency.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
There are few operations in the storage driver that read/write
data onto volumes. Such operations can take very long time to
finish. During that time the storage pool object is locked which
has bad performance impacts (other threads can't fetch its XML
for instance). This commit prepares the storage driver for
releasing the lock during those operations (downloadVol,
uploadVol, wipeVol).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Just like a few commits earlier, checking for pool source
duplicates and unlocking pools list afterwards is a buggy
pattern. The check must go into virStoragePoolObjAssignDef.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The @conn argument is needed only to do some source matching in
case of iSCSI source. Anyway, it's used just for node device
driver and as such can be replaced with virGetConnectNodeDev().
At the same time, the @conn struct member is dropped from
_virStoragePoolObjFindDuplicateData.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Even though we do some checking it is not as thorough as it
should be. We already have virStoragePoolObjIsDuplicate but the
way we use it is a typical TOCTOU. Imagine two threads trying to
define two pools with the same name but different UUIDs. With the
current code neither of them finds a duplicate and thus proceed
to virStoragePoolObjAssignDef where only names are compared.
Therefore both threads succeed which is obviously wrong.
We should check for duplicates where we care for them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In two places the passed pool object argument is marked as
ATTRIBUTE_UNUSED even though it's used right away.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1613746
When shrinking the capacity of a qcow2 or luks volume using
the qemu-img program, the --shrink qualifier must be added.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The vol-dumpxml shows the volume target format type as raw for
encrypted volumes. The error message when attempting to resize
with prealloc is confusing here.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The virSecretGetSecretString() helper looks up a secret for given
pool and returns its value in @secret_value and its length in
@secret_value_size. However, the trailing '\0' is not included in
either of the variables. This is because usually the value of the
secret is passed to some encoder (usually base64 encoder) where
the trailing zero must not be accounted for.
However, in two places we actually want the string as we don't
process it any further.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code to set the connection and connect using libiscsi will always be
the same. Add function to avoid code duplication.
Signed-off-by: Clementine Hayat <clem@lse.epita.fr>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The size of blocks inside a volume and the number of blocks are needed
later. Having a function that return thoses information will avoid code
duplication.
Signed-off-by: Clementine Hayat <clem@lse.epita.fr>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Change the SetContext function to be able to take the session type in
argument.
Took the function findPoolSources of iscsi backend and wired it to my
function since the formatting is essentially the same.
Signed-off-by: Clementine Hayat <clem@lse.epita.fr>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We need here libiscsi for the storgae pool backend.
For the iscsi-direct storage pool, only checkPool and refreshPool should
be necessary for basic support.
The pool is state-less and just need the informations within the volume
to work.
Signed-off-by: Clementine Hayat <clem@lse.epita.fr>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Introducing the pool as a noop. Integration inside the build
system. Implementation will be in the following commits.
Signed-off-by: Clementine Hayat <clem@lse.epita.fr>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
After a new iSCSI interface is successfully set up, we issue a
sendtargets command. However, after 56057900dc we don't
update the host config which in turn makes login fail because
iscsiadm is unable to find any matching record for the interface.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When scanning for targets, iSCSI might give different results
depending on the interface used. This is basically just name of
config file under /etc/iscsi/ifaces to use. The file contains
initiator IQN thus different results claim.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
After my change to the original patch that resulted in commit
8ed874b39b it was brought to my attention that all three defines
are the same: FICLONE = BTRFS_IOC_CLONE = XFS_IOC_CLONE (as
documented in ioctl_ficlone(2)). Therefore we should prefer
generic FICLONE over 'specific' defines for btrfs/xfs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
When re-defining an active storage pool, due to a bug the config
file on disk is not changed. This is because we are passing old
definition instead of new one to virStoragePoolObjSaveDef.
This issue was introduced by bfcd8fc9,
Signed-off-by: Changkuo Shi <shi.changkuo@h3c.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This commit renames and adds other macros to support aother filesystems
when a reflink is performed. After that, XFS filesystems (and others)
with reflink support will be able to clone.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1565004
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Starting with QEMU 2.9, encryption convert processing requires
a multi-step process in order to generate an encrypted image from
some non encrypted raw image.
Processing requires to first create an encrypted image using the
sizing parameters from the input source and second to use the
--image-opts, -n, and --target-image-opts options along with inline
driver options to describe the input and output files, generating
two commands such as:
$ qemu-img create -f luks \
--object secret,id=demo.img_encrypt0,file=/path/to/secretFile \
-o key-secret=demo.img_encrypt0 \
demo.img 500K
Formatting 'demo.img', fmt=luks size=512000 key-secret=demo.img_encrypt0
$ qemu-img convert --image-opts -n --target-image-opts \
--object secret,id=demo.img_encrypt0,file=/path/to/secretFile \
driver=raw,file.filename=sparse.img \
driver=luks,file.filename=demo.img,key-secret=demo.img_encrypt0
$
This patch handles the convert processing by running the processing
in a do..while loop essentially reusing the existing create logic and
arguments to create the target vol from the inputvol and then converting
the inputvol using new arguments.
This then allows the following virsh command to work properly:
virsh vol-create-from default encrypt1-luks.xml data.img --inputpool default
where encrypt1-luks.xml would provided the path and secret for
the new image, while data.img would be the source image.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Since we no longer support creating qcow2 encryption format
volumes, we no longer have to possibly create some secret and
have no real need for the function, so move the remaining
functionality to build the secret path back into the caller
storageBackendCreateQemuImg.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Since we only generate the @encinfo when there's a secret object
and thus we need to reference it in the options,
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Remove the checks for qcow encryption since both callers (create
and resize) would have already disallowed usage.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1526382
Since commit c4eedd793 disallowed qcow2 encrypted images to be
used for domains, it no longer makes sense to allow a qcow2
encrypted volume to be created or resized.
Add a test that will exhibit the failure of creation as well
as the xml2xml validation of the format still being correct.
Update the documentation to note the removal of the capability
to create and use qcow/default encrypted volumes.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Change from @enc to @encinfo leaving @enc for the vol->target.encryption
in the storageBackendCreateQemuImgSetOptions code path.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Allowing a NULL @secretPath for virStorageBackendCreateQemuImgCmdFromVol
would result in a generated command line with a dangling "file=" output.
So let's make sure the @secretPath exists before processing.
This means we should pass a dummy path from the storage test.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Fix the case when creating a luks encrypted volume
via an xml file without 'secret' element.
libvirtd was receiving SIGSEGV, now proper error is reported for
the missing element.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1468422
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
Modify virStorageBackendLogicalLVCreate to ensure if encryption
is requested that only type LUKS is supported; otherwise, error.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1560946
Similar to the the Logical backend, use qemu-img on the created
disk partition device to set up for LUKS encryption. Secret mgmt
for the device can be complicated by a reboot possibly changing
the path to the device if the infrastructure changes.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1400475
In order to avoid a possible error as a result of kernel interactions
with the partition helper, let's use virWaitForDevices to force things
to settle down before attempting to open and read the partition. This
is related to https://bugzilla.redhat.com/show_bug.cgi?id=1264719.
Although perhaps overkill to have too many places to settle, since
we know that the act of reading the partitions via libvirt_parthelper
will cause udev activity/events - we just need to ensure udev has
been settled before proceding with usage of the device.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Extract out command line setup and run from storageBackendCreateQemuImg
as we'll need to run it twice soon.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Split up virStorageBackendCreateQemuImgCmdFromVol into two parts.
It's too long anyway and virStorageBackendCreateQemuImgCmdFromVol
should just handle the command line processing.
NB: Requires changing info.* into info->* references.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The only way preallocate could be set is if the info->format was
not RAW (see storageBackendCreateQemuImgSetBacking), so let's just
extract it from the if/else surrounding the application of the
encryption options.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The only way backing_fmts could be set is if the info->format was
not RAW (see storageBackendCreateQemuImgSetBacking), so let's just
extract it from the if/else surrounding the application of the
encryption options.
Signed-off-by: John Ferlan <jferlan@redhat.com>
