From a74be32bf646dacb3a2930621cf53987cd3d8371 Mon Sep 17 00:00:00 2001 From: Alasdair Kergon Date: Mon, 3 Aug 2009 18:01:45 +0000 Subject: [PATCH] Manage without dm_udev_cleanup? --- lib/activate/dev_manager.c | 30 ++++++++++++++---------------- libdm/.exported_symbols | 1 - libdm/ioctl/libdm-iface.c | 11 ++++++++++- libdm/ioctl/libdm-targets.h | 1 + libdm/libdevmapper.h | 1 - libdm/libdm-common.c | 28 +++++++--------------------- libdm/libdm-deptree.c | 8 -------- tools/dmsetup.c | 28 ++++++++-------------------- 8 files changed, 40 insertions(+), 68 deletions(-) diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c index 0b9bb56dd..4398ba515 100644 --- a/lib/activate/dev_manager.c +++ b/lib/activate/dev_manager.c @@ -1114,6 +1114,7 @@ static int _clean_tree(struct dev_manager *dm, struct dm_tree_node *root) struct dm_tree_node *child; char *vgname, *lvname, *layer; const char *name, *uuid; + int r; while ((child = dm_tree_next_child(&handle, root, 0))) { if (!(name = dm_tree_node_get_name(child))) @@ -1132,12 +1133,12 @@ static int _clean_tree(struct dev_manager *dm, struct dm_tree_node *root) continue; dm_tree_set_cookie(root, 0); - if (!dm_tree_deactivate_children(root, uuid, strlen(uuid))) { - (void) dm_udev_cleanup(dm_tree_get_cookie(root)); - return_0; - } + r = dm_tree_deactivate_children(root, uuid, strlen(uuid)); if (!dm_udev_wait(dm_tree_get_cookie(root))) stack; + + if (!r) + return_0; } return 1; @@ -1171,12 +1172,11 @@ static int _tree_action(struct dev_manager *dm, struct logical_volume *lv, actio case DEACTIVATE: /* Deactivate LV and all devices it references that nothing else has open. */ dm_tree_set_cookie(root, 0); - if (!dm_tree_deactivate_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1)) { - (void) dm_udev_cleanup(dm_tree_get_cookie(root)); - goto_out; - } + r = dm_tree_deactivate_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1); if (!dm_udev_wait(dm_tree_get_cookie(root))) stack; + if (!r) + goto_out; if (!_remove_lv_symlinks(dm, root)) log_error("Failed to remove all device symlinks associated with %s.", lv->name); break; @@ -1196,24 +1196,22 @@ static int _tree_action(struct dev_manager *dm, struct logical_volume *lv, actio /* Preload any devices required before any suspensions */ dm_tree_set_cookie(root, 0); - if (!dm_tree_preload_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1)) { - (void) dm_udev_cleanup(dm_tree_get_cookie(root)); - goto_out; - } + r = dm_tree_preload_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1); if (!dm_udev_wait(dm_tree_get_cookie(root))) stack; + if (!r) + goto_out; if (dm_tree_node_size_changed(root)) dm->flush_required = 1; if (action == ACTIVATE) { dm_tree_set_cookie(root, 0); - if (!dm_tree_activate_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1)) { - (void) dm_udev_cleanup(dm_tree_get_cookie(root)); - goto_out; - } + r = dm_tree_activate_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1); if (!dm_udev_wait(dm_tree_get_cookie(root))) stack; + if (!r) + goto_out; } if (!_create_lv_symlinks(dm, root)) { diff --git a/libdm/.exported_symbols b/libdm/.exported_symbols index 085e5c99d..0e59ebb09 100644 --- a/libdm/.exported_symbols +++ b/libdm/.exported_symbols @@ -161,4 +161,3 @@ dm_udev_set_sync_support dm_udev_get_sync_support dm_udev_complete dm_udev_wait -dm_udev_cleanup diff --git a/libdm/ioctl/libdm-iface.c b/libdm/ioctl/libdm-iface.c index bd9bce026..fae01626e 100644 --- a/libdm/ioctl/libdm-iface.c +++ b/libdm/ioctl/libdm-iface.c @@ -1742,9 +1742,18 @@ int dm_task_run(struct dm_task *dmt) if (!_open_control()) return 0; + /* FIXME Detect and warn if cookie set but should not be. */ repeat_ioctl: - if (!(dmi = _do_dm_ioctl(dmt, command, _ioctl_buffer_double_factor))) + if (!(dmi = _do_dm_ioctl(dmt, command, _ioctl_buffer_double_factor))) { + /* + * If an operation that uses a cookie fails, decrement the + * semaphore instead of udev. + * FIXME Review error paths: found one where uevent fired too. + */ + if (dmt->cookie_set) + dm_udev_complete(dmt->event_nr); return 0; + } if (dmi->flags & DM_BUFFER_FULL_FLAG) { switch (dmt->type) { diff --git a/libdm/ioctl/libdm-targets.h b/libdm/ioctl/libdm-targets.h index 267bb8d5f..5975f9853 100644 --- a/libdm/ioctl/libdm-targets.h +++ b/libdm/ioctl/libdm-targets.h @@ -60,6 +60,7 @@ struct dm_task { int skip_lockfs; int suppress_identical_reload; uint64_t existing_table_size; + int cookie_set; char *uuid; }; diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h index 12f75a73b..b4d8cad9c 100644 --- a/libdm/libdevmapper.h +++ b/libdm/libdevmapper.h @@ -1023,6 +1023,5 @@ void dm_udev_set_sync_support(int sync_with_udev); int dm_udev_get_sync_support(void); int dm_udev_complete(uint32_t cookie); int dm_udev_wait(uint32_t cookie); -int dm_udev_cleanup(uint32_t cookie); #endif /* LIB_DEVICE_MAPPER_H */ diff --git a/libdm/libdm-common.c b/libdm/libdm-common.c index 134487bc4..257f43973 100644 --- a/libdm/libdm-common.c +++ b/libdm/libdm-common.c @@ -174,6 +174,7 @@ struct dm_task *dm_task_create(int type) dmt->no_open_count = 0; dmt->read_ahead = DM_READ_AHEAD_AUTO; dmt->read_ahead_flags = 0; + dmt->cookie_set = 0; return dmt; } @@ -799,11 +800,6 @@ int dm_udev_wait(uint32_t cookie) return 1; } -int dm_udev_cleanup(uint32_t cookie) -{ - return 1; -} - #else /* UDEV_SYNC_SUPPORT */ void dm_udev_set_sync_support(int sync_with_udev) @@ -821,7 +817,8 @@ static int _get_cookie_sem(uint32_t cookie, int *semid) if (!(cookie >> 16 & COOKIE_MAGIC)) { log_error("Could not continue to access notification " "semaphore identified by cookie value %" - PRIu32 " (0x%x). Incorrect cookie prefix."); + PRIu32 " (0x%x). Incorrect cookie prefix.", + cookie, cookie); return 0; } @@ -952,7 +949,7 @@ static int _udev_notify_sem_create(uint32_t *cookie, int *semid) log_error("semid %d: semctl failed: %s", gen_semid, strerror(errno)); /* We have to destroy just created semaphore * so it won't stay in the system. */ - _udev_notify_sem_destroy(gen_semid, gen_cookie); + (void) _udev_notify_sem_destroy(gen_semid, gen_cookie); goto bad; } @@ -996,6 +993,7 @@ int dm_task_set_cookie(struct dm_task *dmt, uint32_t *cookie) } dmt->event_nr = *cookie; + dmt->cookie_set = 1; return 1; bad: @@ -1039,7 +1037,7 @@ int dm_udev_wait(uint32_t cookie) "semaphore identified by cookie value %" PRIu32 " (0x%x) " "to initialize waiting for incoming notifications.", cookie, cookie); - _udev_notify_sem_destroy(semid, cookie); + (void) _udev_notify_sem_destroy(semid, cookie); return 0; } @@ -1050,23 +1048,11 @@ repeat_wait: log_error("Could not set wait state for notification semaphore " "identified by cookie value %" PRIu32 " (0x%x): %s", cookie, cookie, strerror(errno)); - _udev_notify_sem_destroy(semid, cookie); + (void) _udev_notify_sem_destroy(semid, cookie); return 0; } return _udev_notify_sem_destroy(semid, cookie); } -int dm_udev_cleanup(uint32_t cookie) -{ - int semid; - - if (!cookie || !dm_udev_get_sync_support() || !dm_cookie_supported()) - return 1; - - if (!_get_cookie_sem(cookie, &semid)) - return_0; - - return _udev_notify_sem_destroy(semid, cookie); -} #endif /* UDEV_SYNC_SUPPORT */ diff --git a/libdm/libdm-deptree.c b/libdm/libdm-deptree.c index 47b5e63bb..8f103b2df 100644 --- a/libdm/libdm-deptree.c +++ b/libdm/libdm-deptree.c @@ -845,9 +845,6 @@ static int _deactivate_node(const char *name, uint32_t major, uint32_t minor, ui r = dm_task_run(dmt); - if (!r) - (void) dm_udev_complete(*cookie); - /* FIXME Until kernel returns actual name so dm-ioctl.c can handle it */ rm_dev_node(name); @@ -888,9 +885,6 @@ static int _rename_node(const char *old_name, const char *new_name, uint32_t maj r = dm_task_run(dmt); - if (!r) - (void) dm_udev_complete(*cookie); - out: dm_task_destroy(dmt); @@ -934,8 +928,6 @@ static int _resume_node(const char *name, uint32_t major, uint32_t minor, if ((r = dm_task_run(dmt))) r = dm_task_get_info(dmt, newinfo); - else - (void) dm_udev_complete(*cookie); out: dm_task_destroy(dmt); diff --git a/tools/dmsetup.c b/tools/dmsetup.c index d0da19068..bc0d15e11 100644 --- a/tools/dmsetup.c +++ b/tools/dmsetup.c @@ -588,12 +588,8 @@ static int _create(int argc, char **argv, void *data __attribute((unused))) dm_udev_set_sync_support(0); if (!dm_task_set_cookie(dmt, &cookie) || - !dm_task_run(dmt)) { - (void) dm_udev_cleanup(cookie); + !dm_task_run(dmt)) goto out; - } - - (void) dm_udev_wait(cookie); r = 1; @@ -601,6 +597,7 @@ static int _create(int argc, char **argv, void *data __attribute((unused))) r = _display_info(dmt); out: + (void) dm_udev_wait(cookie); dm_task_destroy(dmt); return r; @@ -626,16 +623,13 @@ static int _rename(int argc, char **argv, void *data __attribute((unused))) goto out; if (!dm_task_set_cookie(dmt, &cookie) || - !dm_task_run(dmt)) { - (void) dm_udev_cleanup(cookie); + !dm_task_run(dmt)) goto out; - } - - (void) dm_udev_wait(cookie); r = 1; out: + (void) dm_udev_wait(cookie); dm_task_destroy(dmt); return r; @@ -816,24 +810,18 @@ static int _simple(int task, const char *name, uint32_t event_nr, int display) _read_ahead_flags)) goto out; - if (udev_wait_flag && !dm_task_set_cookie(dmt, &cookie)) { - (void) dm_udev_cleanup(cookie); + if (udev_wait_flag && !dm_task_set_cookie(dmt, &cookie)) goto out; - } r = dm_task_run(dmt); - if (udev_wait_flag) { - if (r) - (void) dm_udev_wait(cookie); - else - (void) dm_udev_cleanup(cookie); - } - if (r && display && _switches[VERBOSE_ARG]) r = _display_info(dmt); out: + if (udev_wait_flag) + (void) dm_udev_wait(cookie); + dm_task_destroy(dmt); return r; }