[libvirt] [PATCH 12/24] qemu: Separate raw IO code from qemuProcessStart

[Peter Krempa]

On Thu, Nov 12, 2015 at 19:37:14 +0100, Jiri Denemark wrote:
> qemuProcessStart is so big that any nontrivial code should be moved to
> dedicated functions to make the code easier to read and maintain.
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  src/qemu/qemu_process.c | 105 ++++++++++++++++++++++++++++--------------------
>  1 file changed, 62 insertions(+), 43 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 4b871a8..d5f6750 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4333,6 +4333,65 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver,
>  }
>  
>  
> +static int
> +qemuProcessSetupRawIO(virQEMUDriverPtr driver,
> +                      virDomainObjPtr vm,
> +                      virCommandPtr cmd ATTRIBUTE_UNUSED)
> +{
> +    bool rawio = false;
> +    size_t i;
> +    int ret = -1;
> +
> +    /* in case a certain disk is desirous of CAP_SYS_RAWIO, add this */
> +    for (i = 0; i < vm->def->ndisks; i++) {
> +        virDomainDeviceDef dev;
> +        virDomainDiskDefPtr disk = vm->def->disks[i];
> +
> +        if (disk->rawio == VIR_TRISTATE_BOOL_YES) {
> +            rawio = true;
> +#ifndef CAP_SYS_RAWIO

Yuck!, but pre-existing ...

> +            break;
> +#endif
> +        }
> +
> +        dev.type = VIR_DOMAIN_DEVICE_DISK;
> +        dev.data.disk = disk;
> +        if (qemuAddSharedDevice(driver, &dev, vm->def->name) < 0)
> +            goto cleanup;
> +
> +        if (qemuSetUnprivSGIO(&dev) < 0)
> +            goto cleanup;
> +    }
> +
> +    /* If rawio not already set, check hostdevs as well */
> +    if (!rawio) {
> +        for (i = 0; i < vm->def->nhostdevs; i++) {
> +            virDomainHostdevSubsysSCSIPtr scsisrc =
> +                &vm->def->hostdevs[i]->source.subsys.u.scsi;
> +            if (scsisrc->rawio == VIR_TRISTATE_BOOL_YES) {
> +                rawio = true;
> +                break;
> +            }
> +        }
> +    }
> +
> +    ret = 0;
> +
> + cleanup:
> +    if (rawio) {
> +#ifdef CAP_SYS_RAWIO
> +        if (ret == 0)
> +            virCommandAllowCap(cmd, CAP_SYS_RAWIO);
> +#else
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("Raw I/O is not supported on this platform"));
> +        ret = -1;
> +#endif
> +    }
> +    return ret;
> +}
> +
> +
>  int qemuProcessStart(virConnectPtr conn,
>                       virQEMUDriverPtr driver,
>                       virDomainObjPtr vm,
> @@ -4353,7 +4412,6 @@ int qemuProcessStart(virConnectPtr conn,
>      virCommandPtr cmd = NULL;
>      struct qemuProcessHookData hookData;
>      size_t i;
> -    bool rawio_set = false;
>      char *nodeset = NULL;
>      unsigned int stop_flags;
>      virQEMUDriverConfigPtr cfg;
> @@ -4689,48 +4747,9 @@ int qemuProcessStart(virConnectPtr conn,
>      if (cfg->clearEmulatorCapabilities)
>          virCommandClearCaps(cmd);
>  
> -    /* in case a certain disk is desirous of CAP_SYS_RAWIO, add this */
> -    for (i = 0; i < vm->def->ndisks; i++) {
> -        virDomainDeviceDef dev;
> -        virDomainDiskDefPtr disk = vm->def->disks[i];
> -
> -        if (vm->def->disks[i]->rawio == VIR_TRISTATE_BOOL_YES) {
> -#ifdef CAP_SYS_RAWIO
> -            virCommandAllowCap(cmd, CAP_SYS_RAWIO);
> -            rawio_set = true;
> -#else
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                           _("Raw I/O is not supported on this platform"));
> -            goto error;
> -#endif

... but the new code is possibly less-yuck than this.

> -        }
> -
> -        dev.type = VIR_DOMAIN_DEVICE_DISK;
> -        dev.data.disk = disk;
> -        if (qemuAddSharedDevice(driver, &dev, vm->def->name) < 0)
> -            goto error;
> -
> -        if (qemuSetUnprivSGIO(&dev) < 0)
> -            goto error;
> -    }
> -
> -    /* If rawio not already set, check hostdevs as well */
> -    if (!rawio_set) {
> -        for (i = 0; i < vm->def->nhostdevs; i++) {
> -            virDomainHostdevSubsysSCSIPtr scsisrc =
> -                &vm->def->hostdevs[i]->source.subsys.u.scsi;
> -            if (scsisrc->rawio == VIR_TRISTATE_BOOL_YES) {
> -#ifdef CAP_SYS_RAWIO
> -                virCommandAllowCap(cmd, CAP_SYS_RAWIO);
> -                break;
> -#else
> -                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                               _("Raw I/O is not supported on this platform"));
> -                goto error;
> -#endif
> -            }
> -        }
> -    }
> +    VIR_DEBUG("Setting up raw IO");
> +    if (qemuProcessSetupRawIO(driver, vm, cmd) < 0)
> +        goto error;
>  
>      virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData);
>      virCommandSetMaxProcesses(cmd, cfg->maxProcesses);

We probably should add a function that will be conditionally compiled
and determinign whether rawio works or not so that the code can be
cleaner.

This is a improvement compared to the old code though, so ...

ACK.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151113/65814d45/attachment-0001.sig>