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

Re: [libvirt] [PATCH v4] Ignore backing file errors in FS storage pool

On 03/01/2011 08:48 AM, Philipp Hahn wrote:
> Currently a single storage volume with a broken backing file will disable the
> whole storage pool. This can happen when the backing file is on some
> unavailable network storage or if the backing volume is deleted, while the
> storage volumes using it remain.
> Since the storage pool can not be re-activated, re-creating the missing
> or deleting the now useless volumes using libvirt only is not possible.
> Fixing this is a little bit tricky:
> 1. virStorageBackendProbeTarget() only detects the missing backing file,
>    if the backing file format is not explicitly specified. If the
>    backing file is created using
> 	   kvm-img create -f qcow2 -o backing_fmt=qcow2,backing_file=... ...
>    no error is detected at this stage.
>    The new return code -3 signals that the backing file could not be
>    opened.
> 2. The backingStore.format must be >= 0, since values < 0 would break
>    virStorageVolTargetDefFormat() when dumping the XML data such as
>        <format type='...'/>
>    Because of this the format is faked as VIR_STORAGE_FILE_RAW.
> 3. virStorageBackendUpdateVolTargetInfo() always opens the backing file
>    and thus always detects a missing backing file.
>    Since it "only" updates the capacity, allocation, owner, group, mode
>    and SELinux label, just ignore errors at this stage, print an error
>    message and continue.
> 4. Using vol-dump on a broken volume still doesn't work, but at leas


>    vol-destroy and pool-refresh do work now.

Thanks for the ping, and yes, this version looks like it fixes the
feedback from earlier review.  Sorry for the delay review.

> To reproduce:
>   dir=$(mktemp -d)
>   virsh pool-create-as tmp dir '' '' '' '' "$dir"
>   virsh vol-create-as --format qcow2 tmp back 1G
>   virsh vol-create-as --format qcow2 --backing-vol-format qcow2 --backing-vol back tmp cow 1G
>   virsh vol-delete --pool tmp back
>   virsh pool-refresh tmp
> After the last step, the pool will be gone (because it was not persistent). As
> long as the now broken image stays in the directory, you will not be able to
> re-create or re-start the pool.

Even more awesome when you do this :)

> @@ -665,8 +680,15 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED,
>              if (virStorageBackendUpdateVolTargetInfo(&vol->backingStore,
>                                                       NULL,
>                                                       NULL) < 0) {
> -                VIR_FREE(vol->backingStore.path);
> -                goto cleanup;
> +                /* The backing file is currently unavailable, the capacity,
> +                 * allocation, owner, group and mode are unknown. Just log the
> +                 * error an continue.
> +                 * Unfortunately virStorageBackendProbeTarget() might already
> +                 * have logged a similar message for the same problem, but only
> +                 * if AUTO format detection was used. */
> +                virStorageReportError(VIR_ERR_INTERNAL_ERROR,
> +                                      _("cannot probe backing volume info: %s"),
> +                                      vol->backingStore);


  CC     libvirt_driver_storage_la-storage_backend_fs.lo
cc1: warnings being treated as errors
storage/storage_backend_fs.c: In function
storage/storage_backend_fs.c:688:17: error: format '%s' expects type
'char *', but argument 8 has type 'virStorageVolTarget' [-Wformat]

ACK with that fixed, so I squashed this and pushed:

diff --git i/src/storage/storage_backend_fs.c
index e629219..0a6b074 100644
--- i/src/storage/storage_backend_fs.c
+++ w/src/storage/storage_backend_fs.c
@@ -1,7 +1,7 @@
  * storage_backend_fs.c: storage backend for FS and directory handling
- * Copyright (C) 2007-2010 Red Hat, Inc.
+ * Copyright (C) 2007-2011 Red Hat, Inc.
  * Copyright (C) 2007-2008 Daniel P. Berrange
  * This library is free software; you can redistribute it and/or
@@ -687,7 +687,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr
                  * if AUTO format detection was used. */
                                       _("cannot probe backing volume
info: %s"),
-                                      vol->backingStore);
+                                      vol->backingStore.path);

Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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