[libvirt] [PATCH v2] storage: Adjust expected format for Disk startup processing

John Ferlan posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170911212551.26741-1-jferlan@redhat.com
src/storage/storage_backend_disk.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[libvirt] [PATCH v2] storage: Adjust expected format for Disk startup processing
Posted by John Ferlan 6 years, 6 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1464313

If a Disk pool was defined/created using XML that either didn't
specify a specific format or specified format type='unknown', then
restarting a pool after an initial disk backend build with overwrite
would fail after a libvirtd restart for a non-autostarted pool.

This is because the persistent pool data is not updated during pool
build w/ overwrite processing to have the VIR_STORAGE_POOL_DISK_DOS
default format.

So in addition to the alteration done during disk build processing,
alter the default expectation for disk startup to be DOS if nothing
has been defined yet. That will either succeed if the pool had been
successfully built previously using the default DOS format or fail
with a message indicating the format is something else that does not
match the expect format 'dos'.

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

 v1: https://www.redhat.com/archives/libvir-list/2017-September/msg00124.html

 Changes since v1... 

   * Use a totally different methodology. Rather than updating the
     persistent config file, alter the start processing to handle the
     unknown setting in the persistent file properly.

 src/storage/storage_backend_disk.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index e8f67bb00..bce3b4e2a 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -446,8 +446,7 @@ static int
 virStorageBackendDiskStartPool(virConnectPtr conn ATTRIBUTE_UNUSED,
                                virStoragePoolObjPtr pool)
 {
-    const char *format =
-        virStoragePoolFormatDiskTypeToString(pool->def->source.format);
+    const char *format;
     const char *path = pool->def->source.devices[0].path;
 
     virWaitForDevices();
@@ -458,6 +457,9 @@ virStorageBackendDiskStartPool(virConnectPtr conn ATTRIBUTE_UNUSED,
         return -1;
     }
 
+    if (pool->def->source.format == VIR_STORAGE_POOL_DISK_UNKNOWN)
+        pool->def->source.format = VIR_STORAGE_POOL_DISK_DOS;
+    format = virStoragePoolFormatDiskTypeToString(pool->def->source.format);
     if (!virStorageBackendDeviceIsEmpty(path, format, false))
         return -1;
 
-- 
2.13.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] storage: Adjust expected format for Disk startup processing
Posted by Peter Krempa 6 years, 6 months ago
On Mon, Sep 11, 2017 at 17:25:51 -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1464313
> 
> If a Disk pool was defined/created using XML that either didn't
> specify a specific format or specified format type='unknown', then
> restarting a pool after an initial disk backend build with overwrite
> would fail after a libvirtd restart for a non-autostarted pool.
> 
> This is because the persistent pool data is not updated during pool
> build w/ overwrite processing to have the VIR_STORAGE_POOL_DISK_DOS
> default format.
> 
> So in addition to the alteration done during disk build processing,
> alter the default expectation for disk startup to be DOS if nothing
> has been defined yet. That will either succeed if the pool had been
> successfully built previously using the default DOS format or fail
> with a message indicating the format is something else that does not
> match the expect format 'dos'.

An alternative would be to fill the default value in a similar way how
we do in the post parse callbacks for VMs

> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> 
>  v1: https://www.redhat.com/archives/libvir-list/2017-September/msg00124.html
> 
>  Changes since v1... 
> 
>    * Use a totally different methodology. Rather than updating the
>      persistent config file, alter the start processing to handle the
>      unknown setting in the persistent file properly.
> 
>  src/storage/storage_backend_disk.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

ACK, this actually makes sense. We do the same transformation when
building and when starting thus the default is honored.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list