[libvirt] [PATCH 3/3] qemu: process: Call disk startup policy check after cloning domain def

[John Ferlan]

On 06/02/2016 10:09 AM, Peter Krempa wrote:
> In commit 1e38ef72 the disk startup policy check was moved prior to the
> call to virDomainObjSetDefTransient which dropped the disk from the
> config rather than the def to be started which is a bug.
> 
> Additionally we'd not report the disk change event for this since the
> disk aliases were not set at that point.
> 
> Finally 'volume' based disks would not work with startup policy too.
> 
> Fix it by moving it back after the definition is copied, aliases are
> assigned and disk sources are translated.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1341415
> ---
>  src/qemu/qemu_process.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 

Does it make sense to check all disk labels for something that's going
to be dropped? (e.g. call to virSecurityManagerCheckAllLabel also goes
through ndisks)...

Your comment below makes me wonder if there's anything else that got
moved that may have assumed Translate had been run. Would make that
virDomainDiskDefValidate less interesting if the Translate could be done
during Init prior to Validate. My head/eyes hurt

ACK to what's here though - just was trying to think out loud...

John


> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index a5bb99e..ad04a96 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4348,10 +4348,6 @@ qemuProcessStartValidate(virQEMUDriverPtr driver,
>              }
>          }
> 
> -        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;
> @@ -4876,6 +4872,14 @@ qemuProcessPrepareDomain(virConnectPtr conn,
>              goto cleanup;
>      }
> 
> +    /* drop possibly missing disks from the definition. This needs to happen
> +     * after the def is copied, aliases are set and disk sources are translated */
> +    if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) {
> +        if (qemuDomainCheckDiskPresence(driver, vm,
> +                                        flags & VIR_QEMU_PROCESS_START_COLD) < 0)
> +            goto cleanup;
> +    }
> +
>      VIR_DEBUG("Create domain masterKey");
>      if (qemuDomainMasterKeyCreate(vm) < 0)
>          goto cleanup;
>