Jeff,
The more appropriate fix is to use spa_writeable(spa) instead of
readonly. The readonly variable is just used to get the property and
that's it, the rest of the spa uses spa_writeable(spa) as a way to
determine if write activity is allowed.
Thanks,
George
On 11/8/14, 7:43 AM, Josef 'Jeff' Sipek via illumos-zfs wrote:
On Fri, Nov 07, 2014 at 01:50:29PM -0800, Matthew Ahrens wrote:
On Fri, Nov 7, 2014 at 10:05 AM, Josef 'Jeff' Sipek via illumos-zfs <
[email protected]> wrote:
I'm trying to figure out sort of a self-inflicted assertion in zdb. This
lead me to two questions for which I didn't find an obvious answer:
(1) why does zdb import pools read-write? E.g.,
libzpool.so.1`spa_import+0x48d(8072c430, 8072c440, 0, 4)
main+0x611(3, fffffd7fffdffd08)
_start+0x6c()
zdb does everything readonly:
kernel_init(FREAD);
so even if you don't pass readonly=true in the props, you won't be able to
write to the pool.
Aha! This is the cause for the source of confusion. The source of
confusion is the fact that spa_import has two local variables: readonly and
mode.
In a way, they both contain the same information - just in a different
format.
Suppose zdb invokes spa_import. After all the variables are declared, we
have:
mode = FREAD
readonly = B_FALSE
Then later, we try to lookup a zpool readonly prop. Since zdb doesn't
specify any props, readonly remains B_FALSE.
At this point, these two variable hold their final values. Note that in the
case of zdb, mode is set correctly but readonly is not. In the illumos-gate
code, the only use of these variables is spa_activate, which uses the mode
variable (which has the correct value).
In the code base I'm working on, others added a dsl_sync_task call to the
end of spa_import (after the namespace lock is dropped) and gated it with a
"if (!readonly)". This results in a sync task attempting to queue up on a
readonly pool as imported by zdb. While I could change the code to say
"if (mode != FREAD)", I'm thinking something like the following is better
since it is more future proof:
diff --git a/usr/src/uts/common/fs/zfs/spa.c b/usr/src/uts/common/fs/zfs/spa.c
index 00ae211..26c06d9 100644
--- a/usr/src/uts/common/fs/zfs/spa.c
+++ b/usr/src/uts/common/fs/zfs/spa.c
@@ -4101,7 +4101,7 @@ spa_import(const char *pool, nvlist_t *config, nvlist_t
*props, uint64_t flags)
spa_load_state_t state = SPA_LOAD_IMPORT;
zpool_rewind_policy_t policy;
uint64_t mode = spa_mode_global;
- uint64_t readonly = B_FALSE;
+ uint64_t readonly = (mode == FREAD);
int error;
nvlist_t *nvroot;
nvlist_t **spares, **l2cache;
Thoughts? Should I try to upstream this?
(2) at what point during/after import is it safe to issue sync tasks? (The
code I'm dealing with tries to at the end of spa_import.)
Not sure that is the best place to do it, but it should work. (Assuming
this is at the very end of spa_import(), after the namespace lock is
dropped.)
Yeah, location-wise it seems fishy. Anyway, see above for details.
Thanks!
Jeff.
P.S. I'm having trouble sending mail to [email protected]. My server
eventually gives up delivery attempts because of:
<[email protected]>: conversation with
mail.open-zfs.net[199.233.237.234] timed out while sending end of data
-- message may be sent more than once
_______________________________________________
developer mailing list
[email protected]
http://lists.open-zfs.org/mailman/listinfo/developer
