On 06/05/2013 10:59 AM, Peter Krempa wrote:
> Setting of local variables in virStorageBackendCreateQemuImgCmd was
> unnecessarily cluttered with ternary operators and repeated testing of
> of conditions.
>
> This patch refactors the function to use if statements and optimizes the
> code flow resulting in a line count reduction.
> ---
> src/storage/storage_backend.c | 80 +++++++++++++++++++------------------------
> 1 file changed, 35 insertions(+), 45 deletions(-)
>
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index ace9cae..4846210 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -649,53 +649,56 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
...
> - if (preallocate && vol->target.format != VIR_STORAGE_FILE_QCOW2) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("metadata preallocation only available with qcow2"));
> - return NULL;
Why did you remove this error? It would be nice if you could add a test for it. :)
> +
> + if (inputvol) {
> + inputPath = inputvol->target.path;
> + inputType = virStorageFileFormatTypeToString(inputvol->type == VIR_STORAGE_VOL_BLOCK ?
> + VIR_STORAGE_FILE_RAW :
> + inputvol->target.format);
Before, inputType was only filled if inputvol->target.path was non-NULL. Do we
need to check for it explicitly, or is it not possible to call this function
with an inputvol that has no target?
> +
> + if (!inputType) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unknown storage vol type %d"),
> + inputvol->target.format);
> + return NULL;
> + }
> +
> + /* XXX: Not strictly required: qemu-img has an option a different
> + * backing store, not really sure what use it serves though, and it
> + * may cause issues with lvm. Untested essentially.
> + */
> + if (STRNEQ_NULLABLE(inputvol->backingStore.path, vol->backingStore.path)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("a different backing store cannot be specified."));
> + return NULL;
> + }
Before, this check was only done when vol->backingStore.path is not NULL.
Now, you won't be allowed to clone a volume with a backing store to a volume
without it.
Jan
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list