[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 07/12] qemu_process: move checks to qemuProcessStartValidate



On 03/15/2016 09:16 AM, Pavel Hrdina wrote:
> Move all code that checks host and domain.  Do not check host if we use
> VIR_QEMU_PROCESS_START_PRETEND flag.
> 
> Signed-off-by: Pavel Hrdina <phrdina redhat com>
> ---
>  src/qemu/qemu_driver.c    |   4 +-
>  src/qemu/qemu_migration.c |   2 +-
>  src/qemu/qemu_process.c   | 168 ++++++++++++++++++++++++----------------------
>  src/qemu/qemu_process.h   |   9 ++-
>  tests/qemuxml2argvtest.c  |   5 +-
>  5 files changed, 103 insertions(+), 85 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 24fa3ec..1862c60 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6985,7 +6985,9 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn,
>                                              VIR_DOMAIN_DEF_PARSE_ABI_UPDATE)))
>          goto cleanup;
>  
> -    if (qemuProcessStartValidate(driver, vm, qemuCaps, false, false) < 0)
> +    if (qemuProcessStartValidate(driver, vm, qemuCaps, false, false,
> +                                 VIR_QEMU_PROCESS_START_COLD |
> +                                 VIR_QEMU_PROCESS_START_PRETEND) < 0)
>          goto cleanup;
>  
>      /* Generate per-domain paths because we don't have the domain object */
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index f204855..8c7fd5c 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -3465,7 +3465,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>      }
>  
>      if (qemuProcessInit(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
> -                        true, false) < 0)
> +                        true, false, VIR_QEMU_PROCESS_START_AUTODESTROY) < 0)
>          goto stopjob;
>      stopProcess = true;
>  
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 32efafb..6265cf5 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4327,18 +4327,97 @@ qemuProcessMakeDir(virQEMUDriverPtr driver,
>   * start of a VM.
>   */
>  int
> -qemuProcessStartValidate(virDomainDefPtr def,
> +qemuProcessStartValidate(virQEMUDriverPtr driver,
> +                         virDomainObjPtr vm,
>                           virQEMUCapsPtr qemuCaps,
>                           bool migration,
> -                         bool snapshot)
> +                         bool snapshot,
> +                         unsigned int flags)
>  {
> -    if (qemuValidateCpuCount(def, qemuCaps) < 0)
> +    bool check_shmem = false;
> +    size_t i;
> +
> +    if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) {
> +        if (vm->def->virtType == VIR_DOMAIN_VIRT_KVM) {
> +            VIR_DEBUG("Checking for KVM availability");
> +            if (!virFileExists("/dev/kvm")) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("Domain requires KVM, but it is not available. "
> +                                 "Check that virtualization is enabled in the "
> +                                 "host BIOS, and host configuration is setup to "
> +                                 "load the kvm modules."));
> +                return -1;
> +            }
> +        }
> +
> +        if (qemuDomainCheckDiskPresence(driver, vm,
> +                                        flags & VIR_QEMU_PROCESS_START_COLD) < 0)
> +            return -1;
> +
> +        VIR_DEBUG("Checking domain and device security labels");
> +        if (virSecurityManagerCheckAllLabel(driver->securityManager, vm->def) < 0)
> +            return -1;
> +
> +    }

Can you add a comment before this block noting that anything which touches
host state needs to be added to this block? Maybe even mention that test suite
will break otherwise. Same could go for other uses of this flag in previous
patches.

ACK with that

- Cole


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]