[libvirt] [PATCH v3 3/3] qemu: check presence of each disk and its backing file as well

[Martin Kletzander]

On 07/30/2013 08:26 AM, Guannan Ren wrote:
> For disk with startupPolicy support, such as cdrom and floppy
> when its chain is broken, the startup policy will apply,
> otherwise, report an error.
> ---
>  src/qemu/qemu_domain.c  | 31 +++++++++++++------------------
>  src/qemu/qemu_process.c |  6 ------
>  2 files changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index be77991..1e75adb 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2043,19 +2043,11 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
>              break;
>  
>          case VIR_DOMAIN_STARTUP_POLICY_MANDATORY:
> -            virReportSystemError(errno,
> -                                 _("cannot access file '%s'"),
> -                                 disk->src);
>              goto error;
> -            break;
>  
>          case VIR_DOMAIN_STARTUP_POLICY_REQUISITE:
> -            if (cold_boot) {
> -                virReportSystemError(errno,
> -                                     _("cannot access file '%s'"),
> -                                     disk->src);
> +            if (cold_boot)
>                  goto error;
> -            }
>              break;
>  
>          case VIR_DOMAIN_STARTUP_POLICY_DEFAULT:
> @@ -2064,6 +2056,7 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
>              break;
>      }
>  
> +    virResetLastError();

Even though it makes perfect sense to have it here, I'd move it one
function up, because it seems more readable to me.

>      VIR_DEBUG("Dropping disk '%s' on domain '%s' (UUID '%s') "
>                "due to inaccessible source '%s'",
>                disk->dst, vm->def->name, uuid, disk->src);
> @@ -2091,22 +2084,24 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
>      virDomainDiskDefPtr disk;
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>  
> +    VIR_DEBUG("Checking for disk presence");
>      for (i = 0; i < vm->def->ndisks; i++) {
>          disk = vm->def->disks[i];
>  
> -        if (!disk->startupPolicy || !disk->src)
> +        if (!disk->src)
>              continue;
>  
> -        if (virFileAccessibleAs(disk->src, F_OK,
> -                                cfg->user,
> -                                cfg->group) >= 0) {
> -            /* disk accessible */
> -            continue;
> +        if (qemuDomainDetermineDiskChain(driver, disk, false) >= 0 &&
> +            qemuDiskChainCheckBroken(disk) >= 0)
> +                continue;
> +
> +        if (disk->startupPolicy) {
> +            if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk,
> +                                                 cold_boot) >= 0)

And rewrite this to one condition.

So basically ACK with this squashed in:

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1e75adb..c54f9f6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2056,7 +2056,6 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr
driver,
             break;
     }

-    virResetLastError();
     VIR_DEBUG("Dropping disk '%s' on domain '%s' (UUID '%s') "
               "due to inaccessible source '%s'",
               disk->dst, vm->def->name, uuid, disk->src);
@@ -2095,10 +2094,11 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
             qemuDiskChainCheckBroken(disk) >= 0)
                 continue;

-        if (disk->startupPolicy) {
-            if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk,
-                                                 cold_boot) >= 0)
-                continue;
+        if (disk->startupPolicy &&
+            qemuDomainCheckDiskStartupPolicy(driver, vm, disk,
+                                             cold_boot) >= 0) {
+            virResetLastError();
+            continue;
         }

         goto cleanup;
--

Martin