1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/7] qemu: refactor disk startup handling (blockdev-add saga)
  4. View patch

[libvirt] [PATCH 7/7] qemu: process: move disk presence checking to host setup function

Peter Krempa posted 7 patches 8 years, 4 months ago
[libvirt] [PATCH 7/7] qemu: process: move disk presence checking to host setup function
Posted by Peter Krempa 8 years, 4 months ago 
Checking of disk presence accesses storage on the host so it should be
done from the host setup function. Move the code to new function called
qemuProcessPrepareHostStorage and remove qemuDomainCheckDiskPresence.
---
 src/qemu/qemu_domain.c  | 41 -----------------------------------------
 src/qemu/qemu_process.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 42 insertions(+), 45 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d3d5dbac6..a8c718f62 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5681,47 +5681,6 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
 }


-int
-qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
-                            virDomainObjPtr vm,
-                            unsigned int flags)
-{
-    size_t i;
-    bool pretend = flags & VIR_QEMU_PROCESS_START_PRETEND;
-    bool cold_boot = flags & VIR_QEMU_PROCESS_START_COLD;
-
-    VIR_DEBUG("Checking for disk presence");
-    for (i = vm->def->ndisks; i > 0; i--) {
-        size_t idx = i - 1;
-        virDomainDiskDefPtr disk = vm->def->disks[idx];
-        virStorageFileFormat format = virDomainDiskGetFormat(disk);
-
-        if (pretend)
-            continue;
-
-        if (virStorageSourceIsEmpty(disk->src))
-            continue;
-
-        /* There is no need to check the backing chain for disks
-         * without backing support, the fact that the file exists is
-         * more than enough */
-        if (virStorageSourceIsLocalStorage(disk->src) &&
-            format > VIR_STORAGE_FILE_NONE &&
-            format < VIR_STORAGE_FILE_BACKING &&
-            virFileExists(virDomainDiskGetSource(disk)))
-            continue;
-
-        if (qemuDomainDetermineDiskChain(driver, vm, disk, true, true) >= 0)
-            continue;
-
-        if (qemuDomainCheckDiskStartupPolicy(driver, vm, idx, cold_boot) >= 0)
-            continue;
-
-        return -1;
-    }
-
-    return 0;
-}

 /*
  * The vm must be locked when any of the following cleanup functions is
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 84792c2a7..6bebfe4f4 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5385,10 +5385,6 @@ qemuProcessPrepareDomain(virConnectPtr conn,
     if (qemuProcessPrepareDomainStorage(conn, driver, vm, cfg, flags) < 0)
         goto cleanup;

-    /* Drop possibly missing disks from the definition. */
-    if (qemuDomainCheckDiskPresence(driver, vm, flags) < 0)
-        goto cleanup;
-
     VIR_DEBUG("Create domain masterKey");
     if (qemuDomainMasterKeyCreate(vm) < 0)
         goto cleanup;
@@ -5435,6 +5431,44 @@ qemuProcessPrepareDomain(virConnectPtr conn,
 }


+static int
+qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
+                              virDomainObjPtr vm,
+                              unsigned int flags)
+{
+    size_t i;
+    bool cold_boot = flags & VIR_QEMU_PROCESS_START_COLD;
+
+    for (i = vm->def->ndisks; i > 0; i--) {
+        size_t idx = i - 1;
+        virDomainDiskDefPtr disk = vm->def->disks[idx];
+        virStorageFileFormat format = virDomainDiskGetFormat(disk);
+
+        if (virStorageSourceIsEmpty(disk->src))
+            continue;
+
+        /* There is no need to check the backing chain for disks
+         * without backing support, the fact that the file exists is
+         * more than enough */
+        if (virStorageSourceIsLocalStorage(disk->src) &&
+            format > VIR_STORAGE_FILE_NONE &&
+            format < VIR_STORAGE_FILE_BACKING &&
+            virFileExists(virDomainDiskGetSource(disk)))
+            continue;
+
+        if (qemuDomainDetermineDiskChain(driver, vm, disk, true, true) >= 0)
+            continue;
+
+        if (qemuDomainCheckDiskStartupPolicy(driver, vm, idx, cold_boot) >= 0)
+            continue;
+
+        return -1;
+    }
+
+    return 0;
+}
+
+
 /**
  * qemuProcessPrepareHost:
  * @driver: qemu driver
@@ -5527,6 +5561,10 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
     if (qemuDomainWriteMasterKeyFile(driver, vm) < 0)
         goto cleanup;

+    VIR_DEBUG("Preparing disks (host)");
+    if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0)
+        goto cleanup;
+
     ret = 0;
  cleanup:
     virObjectUnref(cfg);
-- 
2.14.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/7] qemu: process: move disk presence checking to host setup function
Posted by John Ferlan 8 years, 4 months ago 

On 10/04/2017 07:42 AM, Peter Krempa wrote:
> Checking of disk presence accesses storage on the host so it should be
> done from the host setup function. Move the code to new function called
> qemuProcessPrepareHostStorage and remove qemuDomainCheckDiskPresence.
> ---
>  src/qemu/qemu_domain.c  | 41 -----------------------------------------
>  src/qemu/qemu_process.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 42 insertions(+), 45 deletions(-)
> 

[...]

> 
> +static int
> +qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
> +                              virDomainObjPtr vm,
> +                              unsigned int flags)
> +{
> +    size_t i;
> +    bool cold_boot = flags & VIR_QEMU_PROCESS_START_COLD;
> +

Consider my note from patch 5/7.  Previously this loop essentially
wasn't run for the pretend environment.  Now it will be, although since
@flags is passed it conceivably could be checked.  My make check passes
either way, so it may not matter, but figured to point it out for purely
testing purposes it seems.


Reviewed-by: John Ferlan <jferlan@redhat.com>

John

[...]


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/7] qemu: process: move disk presence checking to host setup function
Posted by Peter Krempa 8 years, 4 months ago 
On Wed, Oct 04, 2017 at 12:09:04 -0400, John Ferlan wrote:
> 
> 
> On 10/04/2017 07:42 AM, Peter Krempa wrote:
> > Checking of disk presence accesses storage on the host so it should be
> > done from the host setup function. Move the code to new function called
> > qemuProcessPrepareHostStorage and remove qemuDomainCheckDiskPresence.
> > ---
> >  src/qemu/qemu_domain.c  | 41 -----------------------------------------
> >  src/qemu/qemu_process.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
> >  2 files changed, 42 insertions(+), 45 deletions(-)
> > 
> 
> [...]
> 
> > 
> > +static int
> > +qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
> > +                              virDomainObjPtr vm,
> > +                              unsigned int flags)
> > +{
> > +    size_t i;
> > +    bool cold_boot = flags & VIR_QEMU_PROCESS_START_COLD;
> > +
> 
> Consider my note from patch 5/7.  Previously this loop essentially
> wasn't run for the pretend environment.  Now it will be, although since
> @flags is passed it conceivably could be checked.  My make check passes
> either way, so it may not matter, but figured to point it out for purely
> testing purposes it seems.

It won't. Previously it was called from the domain prepare function
which is called in qemuProcessCreatePretendCmd which is the only place
that sets VIR_QEMU_PROCESS_START_PRETEND.

By moving the code to the "host prepare" function we gain that by
default.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/7] qemu: process: move disk presence checking to host setup function
Posted by Peter Krempa 8 years, 4 months ago 
On Wed, Oct 04, 2017 at 18:21:24 +0200, Peter Krempa wrote:
> On Wed, Oct 04, 2017 at 12:09:04 -0400, John Ferlan wrote:
> > 
> > 
> > On 10/04/2017 07:42 AM, Peter Krempa wrote:
> > > Checking of disk presence accesses storage on the host so it should be
> > > done from the host setup function. Move the code to new function called
> > > qemuProcessPrepareHostStorage and remove qemuDomainCheckDiskPresence.
> > > ---
> > >  src/qemu/qemu_domain.c  | 41 -----------------------------------------
> > >  src/qemu/qemu_process.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
> > >  2 files changed, 42 insertions(+), 45 deletions(-)
> > > 
> > 
> > [...]
> > 
> > > 
> > > +static int
> > > +qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
> > > +                              virDomainObjPtr vm,
> > > +                              unsigned int flags)
> > > +{
> > > +    size_t i;
> > > +    bool cold_boot = flags & VIR_QEMU_PROCESS_START_COLD;
> > > +
> > 
> > Consider my note from patch 5/7.  Previously this loop essentially
> > wasn't run for the pretend environment.  Now it will be, although since
> > @flags is passed it conceivably could be checked.  My make check passes
> > either way, so it may not matter, but figured to point it out for purely
> > testing purposes it seems.
> 
> It won't. Previously it was called from the domain prepare function
> which is called in qemuProcessCreatePretendCmd which is the only place
> that sets VIR_QEMU_PROCESS_START_PRETEND.
> 
> By moving the code to the "host prepare" function we gain that by
> default.

Forgot to finish thought: We get that by default, since the 'host
prepare' function is not called from qemuProcessCreatePretendCmd on
purpose. The host prepare function agregates stuff that depends on the
host state and thus is invalid to call in the test suite.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.