From 41ebfb6c54ecbc22d5be609a76ef3663cc04f693 Mon Sep 17 00:00:00 2001 From: Kern Sibbald Date: Thu, 24 Feb 2011 12:56:22 +0100 Subject: [PATCH] Fix lock race conditions in bug #1675 --- bacula/src/stored/autochanger.c | 20 +++++++++++++++----- bacula/src/stored/stored_conf.c | 5 +++-- bacula/src/stored/stored_conf.h | 5 ++--- bacula/src/stored/vol_mgr.c | 11 +++++------ 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/bacula/src/stored/autochanger.c b/bacula/src/stored/autochanger.c index b01f4cfd52..b32656e8e7 100644 --- a/bacula/src/stored/autochanger.c +++ b/bacula/src/stored/autochanger.c @@ -1,7 +1,7 @@ /* Bacula® - The Network Backup Solution - Copyright (C) 2002-2010 Free Software Foundation Europe e.V. + Copyright (C) 2002-2011 Free Software Foundation Europe e.V. The main author of Bacula is Kern Sibbald, with contributions from many others, a complete list can be found in the file AUTHORS. @@ -321,8 +321,13 @@ static void lock_changer(DCR *dcr) { AUTOCHANGER *changer_res = dcr->device->changer_res; if (changer_res) { + int errstat; Dmsg1(200, "Locking changer %s\n", changer_res->hdr.name); - P(changer_res->changer_mutex); /* Lock changer script */ + if ((errstat=rwl_writelock(&changer_res->changer_lock)) != 0) { + berrno be; + Jmsg(dcr->jcr, M_ERROR_TERM, 0, _("Lock failure on autochanger. ERR=%s\n"), + be.bstrerror(errstat)); + } } } @@ -330,8 +335,13 @@ static void unlock_changer(DCR *dcr) { AUTOCHANGER *changer_res = dcr->device->changer_res; if (changer_res) { + int errstat; Dmsg1(200, "Unlocking changer %s\n", changer_res->hdr.name); - V(changer_res->changer_mutex); /* Unlock changer script */ + if ((errstat=rwl_writeunlock(&changer_res->changer_lock)) != 0) { + berrno be; + Jmsg(dcr->jcr, M_ERROR_TERM, 0, _("Unlock failure on autochanger. ERR=%s\n"), + be.bstrerror(errstat)); + } } } @@ -364,6 +374,7 @@ bool unload_autochanger(DCR *dcr, int loaded) return true; } + lock_changer(dcr); if (loaded < 0) { loaded = get_autochanger_loaded_slot(dcr); } @@ -371,7 +382,6 @@ bool unload_autochanger(DCR *dcr, int loaded) if (loaded > 0) { POOL_MEM results(PM_MESSAGE); POOLMEM *changer = get_pool_memory(PM_FNAME); - lock_changer(dcr); Jmsg(jcr, M_INFO, 0, _("3307 Issuing autochanger \"unload slot %d, drive %d\" command.\n"), loaded, dev->drive_index); @@ -394,11 +404,11 @@ bool unload_autochanger(DCR *dcr, int loaded) } else { dev->set_slot(0); /* nothing loaded */ } - unlock_changer(dcr); free_volume(dev); /* Free any volume associated with this drive */ free_pool_memory(changer); } + unlock_changer(dcr); if (ok) { dev->clear_unload(); } diff --git a/bacula/src/stored/stored_conf.c b/bacula/src/stored/stored_conf.c index 11abee2a46..a9fc44da5d 100644 --- a/bacula/src/stored/stored_conf.c +++ b/bacula/src/stored/stored_conf.c @@ -457,6 +457,7 @@ void free_resource(RES *sres, int type) if (res->res_changer.device) { delete res->res_changer.device; } + rwl_destroy(&res->res_changer.changer_lock); break; case R_STORAGE: if (res->res_store.sdaddrs) { @@ -629,9 +630,9 @@ void save_resource(int type, RES_ITEM *items, int pass) foreach_alist(dev, res->res_changer.device) { dev->changer_res = (AUTOCHANGER *)&res->res_changer; } - if ((errstat = pthread_mutex_init(&res->res_changer.changer_mutex, NULL)) != 0) { + if ((errstat = rwl_init(&res->res_changer.changer_lock)) != 0) { berrno be; - Jmsg1(NULL, M_ERROR_TERM, 0, _("Unable to init mutex: ERR=%s\n"), + Jmsg1(NULL, M_ERROR_TERM, 0, _("Unable to init lock: ERR=%s\n"), be.bstrerror(errstat)); } break; diff --git a/bacula/src/stored/stored_conf.h b/bacula/src/stored/stored_conf.h index 7597ff04f5..13d8c74bf5 100644 --- a/bacula/src/stored/stored_conf.h +++ b/bacula/src/stored/stored_conf.h @@ -1,7 +1,7 @@ /* Bacula® - The Network Backup Solution - Copyright (C) 2000-2009 Free Software Foundation Europe e.V. + Copyright (C) 2000-2011 Free Software Foundation Europe e.V. The main author of Bacula is Kern Sibbald, with contributions from many others, a complete list can be found in the file AUTHORS. @@ -28,7 +28,6 @@ /* * Resource codes -- they must be sequential for indexing * - * Version $Id$ */ enum { @@ -110,7 +109,7 @@ public: alist *device; /* List of DEVRES device pointers */ char *changer_name; /* Changer device name */ char *changer_command; /* Changer command -- external program */ - pthread_mutex_t changer_mutex; /* One changer operation at a time */ + brwlock_t changer_lock; /* One changer operation at a time */ }; /* Device specific definitions */ diff --git a/bacula/src/stored/vol_mgr.c b/bacula/src/stored/vol_mgr.c index 788d644d8e..e16e433471 100644 --- a/bacula/src/stored/vol_mgr.c +++ b/bacula/src/stored/vol_mgr.c @@ -1,7 +1,7 @@ /* Bacula® - The Network Backup Solution - Copyright (C) 2000-2009 Free Software Foundation Europe e.V. + Copyright (C) 2000-2011 Free Software Foundation Europe e.V. The main author of Bacula is Kern Sibbald, with contributions from many others, a complete list can be found in the file AUTHORS. @@ -32,8 +32,6 @@ * * Split from reserve.c October 2008 * - * Version $Id: reserve.c 7380 2008-07-14 10:42:59Z kerns $ - * */ #include "bacula.h" @@ -611,12 +609,13 @@ bool free_volume(DEVICE *dev) { VOLRES *vol; - if (dev->vol == NULL) { + lock_volumes(); + vol = dev->vol; + if (vol == NULL) { Dmsg1(dbglvl, "No vol on dev %s\n", dev->print_name()); + unlock_volumes(); return false; } - lock_volumes(); - vol = dev->vol; /* Don't free a volume while it is being swapped */ if (!vol->is_swapping()) { Dmsg1(dbglvl, "=== clear in_use vol=%s\n", vol->vol_name); -- 2.39.5