device: Fix basic async I/O error handling

diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index 7678290836d3a6e0fb701def8f2112bfb14e26fe..4022ea64c2bb9d3c00a2fae0178ffcc953df6a86 100644 (file)
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -1099,6 +1099,7 @@ next:
 }
 
 /* Track the number of outstanding label reads */
+/* FIXME Switch to struct and also track failed */
 static void _process_label_data(int failed, unsigned ioflags, void *context, const void *data)
 {
        int *nr_labels_outstanding = context;
@@ -1160,6 +1161,7 @@ int lvmcache_label_scan(struct cmd_context *cmd)
        _destroy_duplicate_device_list(&_found_duplicate_devs);
 
        while ((dev = dev_iter_get(iter))) {
+               log_debug_io("Scanning device %s", dev_name(dev));
                nr_labels_outstanding++;
                if (!label_read_callback(dev, UINT64_C(0), AIO_SUPPORTED_CODE_PATH, _process_label_data, &nr_labels_outstanding))
                        nr_labels_outstanding--;

diff --git a/lib/config/config.c b/lib/config/config.c
index c69ac69fa041bcd4d59e531f5db3829f0dfe8876..97c5db8a1a83708504d7d9c87049dace12e77119 100644 (file)
--- a/lib/config/config.c
+++ b/lib/config/config.c
@@ -609,8 +609,11 @@ int config_file_read_fd(struct dm_pool *mem, struct dm_config_tree *cft, struct
                                goto_out;
                        _process_config_file_buffer(0, ioflags, pcfp, buf);
                        dm_free((void *)buf);
-               } else if (!dev_read_callback(dev, (uint64_t) offset, size, reason, ioflags, _process_config_file_buffer, pcfp))
-                       goto_out;
+               } else {
+                       dev_read_callback(dev, (uint64_t) offset, size, reason, ioflags, _process_config_file_buffer, pcfp);
+                       if (config_file_read_fd_callback)
+                               return 1;
+               }
                r = pcfp->ret;
        }
 

diff --git a/lib/device/dev-io.c b/lib/device/dev-io.c
index 460c874c1c9c4f2ffe2ca625c9b84fceed33456c..31506e8828c4e043f14d87a7af9d9686552888cc 100644 (file)
--- a/lib/device/dev-io.c
+++ b/lib/device/dev-io.c
@@ -245,7 +245,8 @@ int dev_async_getevents(void)
                        _release_devbuf(devbuf);
                        if (dev_read_callback_fn)
                                dev_read_callback_fn(1, AIO_SUPPORTED_CODE_PATH, dev_read_callback_context, NULL);
-                       r = 0;
+                       else
+                               r = 0;
                }
        }
 
@@ -1080,24 +1081,28 @@ static void _dev_inc_error_count(struct device *dev)
 }
 
 /*
- * Data is returned (read-only) at DEV_DEVBUF_DATA(dev, reason)
+ * Data is returned (read-only) at DEV_DEVBUF_DATA(dev, reason).
+ * If dev_read_callback_fn is supplied, we always return 1 and take
+ * responsibility for calling it exactly once.  This might happen before the
+ * function returns (if there's an error or the I/O is synchronous) or after.
+ * Any error is passed to that function, which must track it if required.
  */
-int dev_read_callback(struct device *dev, uint64_t offset, size_t len, dev_io_reason_t reason,
-                     unsigned ioflags, lvm_callback_fn_t dev_read_callback_fn, void *callback_context)
+static int _dev_read_callback(struct device *dev, uint64_t offset, size_t len, dev_io_reason_t reason,
+                             unsigned ioflags, lvm_callback_fn_t dev_read_callback_fn, void *callback_context)
 {
        struct device_area where;
        struct device_buffer *devbuf;
        uint64_t buf_end;
        int cached = 0;
-       int ret = 1;
+       int ret = 0;
 
        if (!dev->open_count) {
                log_error(INTERNAL_ERROR "Attempt to access device %s while closed.", dev_name(dev));
-               return 0;
+               goto out;
        }
 
        if (!_dev_is_valid(dev))
-               return 0;
+               goto_out;
 
        /*
         * Can we satisfy this from data we stored last time we read?
@@ -1110,6 +1115,7 @@ int dev_read_callback(struct device *dev, uint64_t offset, size_t len, dev_io_re
                        devbuf->data_offset = offset - devbuf->where.start;
                        log_debug_io("Cached read for %" PRIu64 " bytes at %" PRIu64 " on %s (for %s)",
                                     (uint64_t) len, (uint64_t) offset, dev_name(dev), _reason_text(reason));
+                       ret = 1;
                        goto out;
                }
        }
@@ -1126,16 +1132,26 @@ int dev_read_callback(struct device *dev, uint64_t offset, size_t len, dev_io_re
 
 out:
        /* If we had an error or this was sync I/O, pass the result to any callback fn */
-       if ((!ret || !_aio_ctx || !aio_supported_code_path(ioflags) || cached) && dev_read_callback_fn)
+       if ((!ret || !_aio_ctx || !aio_supported_code_path(ioflags) || cached) && dev_read_callback_fn) {
                dev_read_callback_fn(!ret, ioflags, callback_context, DEV_DEVBUF_DATA(dev, reason));
+               return 1;
+       }
 
        return ret;
 }
 
+void dev_read_callback(struct device *dev, uint64_t offset, size_t len, dev_io_reason_t reason,
+                     unsigned ioflags, lvm_callback_fn_t dev_read_callback_fn, void *callback_context)
+{
+       /* Always returns 1 if callback fn is supplied */
+       if (!_dev_read_callback(dev, offset, len, reason, ioflags, dev_read_callback_fn, callback_context))
+               log_error(INTERNAL_ERROR "_dev_read_callback failed");
+}
+
 /* Returns pointer to read-only buffer. Caller does not free it.  */
 const char *dev_read(struct device *dev, uint64_t offset, size_t len, dev_io_reason_t reason)
 {
-       if (!dev_read_callback(dev, offset, len, reason, 0, NULL, NULL))
+       if (!_dev_read_callback(dev, offset, len, reason, 0, NULL, NULL))
                return_NULL;
 
        return DEV_DEVBUF_DATA(dev, reason);
@@ -1144,7 +1160,7 @@ const char *dev_read(struct device *dev, uint64_t offset, size_t len, dev_io_rea
 /* Read into supplied retbuf owned by the caller. */
 int dev_read_buf(struct device *dev, uint64_t offset, size_t len, dev_io_reason_t reason, void *retbuf)
 {
-       if (!dev_read_callback(dev, offset, len, reason, 0, NULL, NULL)) {
+       if (!_dev_read_callback(dev, offset, len, reason, 0, NULL, NULL)) {
                log_error("Read from %s failed", dev_name(dev));
                return 0;
        }

diff --git a/lib/device/device.h b/lib/device/device.h
index 90611399fb2ee9fe54e50703185b5a14a964e660..ea71d00a4cfac4c7a8fd9377c803d485103208a2 100644 (file)
--- a/lib/device/device.h
+++ b/lib/device/device.h
@@ -184,9 +184,9 @@ const char *dev_read(struct device *dev, uint64_t offset, size_t len, dev_io_rea
 const char *dev_read_circular(struct device *dev, uint64_t offset, size_t len,
                              uint64_t offset2, size_t len2, dev_io_reason_t reason);
 
-/* Passes the data to dev_read_callback_fn */
-int dev_read_callback(struct device *dev, uint64_t offset, size_t len, dev_io_reason_t reason,
-                     unsigned ioflags, lvm_callback_fn_t dev_read_callback_fn, void *callback_context);
+/* Passes the data (or error) to dev_read_callback_fn */
+void dev_read_callback(struct device *dev, uint64_t offset, size_t len, dev_io_reason_t reason,
+                      unsigned ioflags, lvm_callback_fn_t dev_read_callback_fn, void *callback_context);
 
 /* Read data and copy it into a supplied private buffer. */
 /* Only use for tiny reads or on unimportant code paths. */

diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 7f63ad62a2a142903e5a1a0bcc39513c3a303d2b..e1603e737eb1c18094aec81a800ea8e974f2feaa 100644 (file)
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -385,8 +385,8 @@ static void _process_raw_mda_header(int failed, unsigned ioflags, void *context,
 bad:
        prmp->ret = 0;
 out:
-       if (prmp->ret && prmp->mdah_callback_fn)
-               prmp->mdah_callback_fn(0, ioflags, prmp->mdah_callback_context, mdah);
+       if (prmp->mdah_callback_fn)
+               prmp->mdah_callback_fn(!prmp->ret, ioflags, prmp->mdah_callback_context, mdah);
 }
 
 static struct mda_header *_raw_read_mda_header(struct dm_pool *mem, struct device_area *dev_area, int primary_mda,
@@ -417,14 +417,15 @@ static struct mda_header *_raw_read_mda_header(struct dm_pool *mem, struct devic
        prmp->mdah_callback_context = mdah_callback_context;
        prmp->ret = 1;
 
-       if (!dev_read_callback(dev_area->dev, dev_area->start, MDA_HEADER_SIZE, MDA_HEADER_REASON(primary_mda),
-                              ioflags, _process_raw_mda_header, prmp))
-               stack;
+       dev_read_callback(dev_area->dev, dev_area->start, MDA_HEADER_SIZE, MDA_HEADER_REASON(primary_mda),
+                         ioflags, _process_raw_mda_header, prmp);
+       if (mdah_callback_fn)
+               return mdah;
 
        if (!prmp->ret)
                return_NULL;
-
-       return mdah;
+       else
+               return mdah;
 }
 
 struct mda_header *raw_read_mda_header(struct dm_pool *mem, struct device_area *dev_area, int primary_mda)
@@ -1404,8 +1405,7 @@ static void _vgname_from_mda_process(int failed, unsigned ioflags, void *context
        }
 
 out:
-       if (vfmp->ret)
-               vfmp->update_vgsummary_fn(0, ioflags, vfmp->update_vgsummary_context, vfmp->vgsummary);
+       vfmp->update_vgsummary_fn(!vfmp->ret, ioflags, vfmp->update_vgsummary_context, vfmp->vgsummary);
 }
 
 static void _vgname_from_mda_validate(int failed, unsigned ioflags, void *context, const void *data)
@@ -1469,7 +1469,8 @@ static void _vgname_from_mda_validate(int failed, unsigned ioflags, void *contex
        }
 
 out:
-       ;
+       if (!vfmp->ret && vfmp->update_vgsummary_fn)
+               vfmp->update_vgsummary_fn(1, ioflags, vfmp->update_vgsummary_context, vfmp->vgsummary);
 }
 
 int vgname_from_mda(const struct format_type *fmt,
@@ -1517,11 +1518,12 @@ int vgname_from_mda(const struct format_type *fmt,
 
        /* Do quick check for a vgname */
        /* We cannot read the full metadata here because the name has to be validated before we use the size field */
-       if (!dev_read_callback(dev_area->dev, dev_area->start + rlocn->offset, NAME_LEN, MDA_CONTENT_REASON(primary_mda),
-                              ioflags, _vgname_from_mda_validate, vfmp))
-               return_0;
-
-       return vfmp->ret;
+       dev_read_callback(dev_area->dev, dev_area->start + rlocn->offset, NAME_LEN, MDA_CONTENT_REASON(primary_mda),
+                              ioflags, _vgname_from_mda_validate, vfmp);
+       if (update_vgsummary_fn)
+               return 1;
+       else
+               return vfmp->ret;
 }
 
 static int _scan_raw(const struct format_type *fmt, const char *vgname __attribute__((unused)))

diff --git a/lib/format_text/import.c b/lib/format_text/import.c
index 69af925bce9c6e0e2c6cb837516327e1c19d2022..0138ddd8b001e88360d16c031825af12906af8aa 100644 (file)
--- a/lib/format_text/import.c
+++ b/lib/format_text/import.c
@@ -78,8 +78,8 @@ static void _import_vgsummary(int failed, unsigned ioflags, void *context, const
 out:
        config_destroy(ivsp->cft);
 
-       if (ivsp->ret && ivsp->process_vgsummary_fn)
-               ivsp->process_vgsummary_fn(0, ioflags, ivsp->process_vgsummary_context, NULL);
+       if (ivsp->process_vgsummary_fn)
+               ivsp->process_vgsummary_fn(!ivsp->ret, ioflags, ivsp->process_vgsummary_context, NULL);
 }
 
 /*

diff --git a/lib/format_text/text_label.c b/lib/format_text/text_label.c
index d3d92f0b6014b60f820556a5e85563dfe5a4c64a..45136e9c465c70100fe0508cbe517ab6cb319827 100644 (file)
--- a/lib/format_text/text_label.c
+++ b/lib/format_text/text_label.c
@@ -344,6 +344,7 @@ static void _process_vgsummary(int failed, unsigned ioflags, void *context, cons
 
        --pmp->umb->nr_outstanding_mdas;
 
+       /* FIXME Need to distinguish genuine errors here */
        if (failed)
                goto_out;
 
@@ -446,7 +447,10 @@ static int _update_mda(struct metadata_area *mda, void *baton)
                return 1;
        }
 
-       return pmp->ret;
+       if (umb->read_label_callback_fn)
+               return 1;
+       else
+               return pmp->ret;
 }
 
 static int _text_read(struct labeller *l, struct device *dev, void *buf, unsigned ioflags,

diff --git a/lib/label/label.c b/lib/label/label.c
index 0997b962c638f61c2f2d45fdfa0f5561b2466768..ae3a2c181374bf8fd46dd85e7e4d7dcd9779e9d5 100644 (file)
--- a/lib/label/label.c
+++ b/lib/label/label.c
@@ -155,8 +155,10 @@ out:
        if (!dev_close(flp->dev))
                stack;
 
-       if (flp->process_label_data_fn)
+       if (flp->process_label_data_fn) {
+               log_debug_io("Completed label reading for %s", dev_name(flp->dev));
                flp->process_label_data_fn(!flp->ret, ioflags, flp->process_label_data_context, NULL);
+       }
 }
 
 static void _find_labeller(int failed, unsigned ioflags, void *context, const void *data)
@@ -324,8 +326,10 @@ static int _label_read(struct device *dev, uint64_t scan_sector, struct label **
                log_debug_devs("Reading label from lvmcache for %s", dev_name(dev));
                if (result)
                        *result = lvmcache_get_label(info);
-               if (process_label_data_fn)
+               if (process_label_data_fn) {
+                       log_debug_io("Completed label reading for %s", dev_name(dev));
                        process_label_data_fn(0, ioflags, process_label_data_context, NULL);
+               }
                return 1;
        }
 
@@ -356,12 +360,7 @@ static int _label_read(struct device *dev, uint64_t scan_sector, struct label **
                return 0;
        }
 
-       if (!(dev_read_callback(dev, scan_sector << SECTOR_SHIFT, LABEL_SCAN_SIZE, DEV_IO_LABEL, ioflags, _find_labeller, flp))) {
-               log_debug_devs("%s: Failed to read label area", dev_name(dev));
-               _set_label_read_result(1, ioflags, flp, NULL);
-               return 0;
-       }
-
+       dev_read_callback(dev, scan_sector << SECTOR_SHIFT, LABEL_SCAN_SIZE, DEV_IO_LABEL, ioflags, _find_labeller, flp);
        if (process_label_data_fn)
                return 1;
        else