From 96e02019a229296d11d8d07d0686637b883be2c5 Mon Sep 17 00:00:00 2001 From: mayank Date: Tue, 4 Feb 2020 19:03:08 +0530 Subject: [PATCH 1/6] feat(readonly): adding test cases for uzfs zpool readonly support Signed-off-by: mayank --- include/sys/fs/zfs.h | 1 + include/sys/spa_impl.h | 3 + module/zcommon/zpool_prop.c | 3 + module/zfs/spa.c | 53 ++++++++++ tests/cstor/gtest/test_zrepl_prot.cc | 152 +++++++++++++++++++++++++++ 5 files changed, 212 insertions(+) diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index b4be6e35a0c7..ff095d018ddb 100644 --- a/include/sys/fs/zfs.h +++ b/include/sys/fs/zfs.h @@ -229,6 +229,7 @@ typedef enum { ZPOOL_PROP_TNAME, ZPOOL_PROP_MAXDNODESIZE, ZPOOL_PROP_MULTIHOST, + ZPOOL_PROP_UZFS_READONLY, ZPOOL_NUM_PROPS } zpool_prop_t; diff --git a/include/sys/spa_impl.h b/include/sys/spa_impl.h index 9cfd23cc27cf..2d495479e520 100644 --- a/include/sys/spa_impl.h +++ b/include/sys/spa_impl.h @@ -297,6 +297,9 @@ struct spa { taskq_t *spa_prefetch_taskq; /* Taskq for prefetch threads */ uint64_t spa_multihost; /* multihost aware (mmp) */ mmp_thread_t spa_mmp; /* multihost mmp thread */ +#ifdef _UZFS + boolean_t readonly; /* pool is readonly or not */ +#endif /* * spa_refcount & spa_config_lock must be the last elements diff --git a/module/zcommon/zpool_prop.c b/module/zcommon/zpool_prop.c index fd21f31176a5..5d70bdd12cd1 100644 --- a/module/zcommon/zpool_prop.c +++ b/module/zcommon/zpool_prop.c @@ -73,6 +73,9 @@ zpool_prop_init(void) PROP_DEFAULT, ZFS_TYPE_POOL, " | none", "CACHEFILE"); zprop_register_string(ZPOOL_PROP_COMMENT, "comment", NULL, PROP_DEFAULT, ZFS_TYPE_POOL, "", "COMMENT"); + zprop_register_string(ZPOOL_PROP_UZFS_READONLY, "io.openebs:readonly", "", + PROP_DEFAULT, ZFS_TYPE_POOL, "on | off", "ZPOOL_READONLY"); + /* readonly number properties */ zprop_register_number(ZPOOL_PROP_SIZE, "size", 0, PROP_READONLY, diff --git a/module/zfs/spa.c b/module/zfs/spa.c index 6eb2e6fd4c31..dd3618e3dbd6 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -79,6 +79,7 @@ #include #ifdef _UZFS #include +#include #endif #ifdef _KERNEL #include @@ -635,7 +636,17 @@ spa_prop_validate(spa_t *spa, nvlist_t *props) intval != 0 && intval < ZIO_DEDUPDITTO_MIN) error = SET_ERROR(EINVAL); break; +#ifdef _UZFS + case ZPOOL_PROP_UZFS_READONLY: + if ((error = nvpair_value_string(elem, &strval)) != 0) + break; + if (strcmp(strval, "on") != 0 && + strcmp(strval, "off") != 0) { + error = SET_ERROR(EINVAL); + break; + } +#endif default: break; } @@ -688,6 +699,9 @@ spa_prop_set(spa_t *spa, nvlist_t *nvp) int error; nvpair_t *elem = NULL; boolean_t need_sync = B_FALSE; +#ifdef _UZFS + char *val; +#endif if ((error = spa_prop_validate(spa, nvp)) != 0) return (error); @@ -729,6 +743,20 @@ spa_prop_set(spa_t *spa, nvlist_t *nvp) continue; } +#ifdef _UZFS + if (prop == ZPOOL_PROP_UZFS_READONLY) { + VERIFY(nvpair_value_string(elem, &val) == 0); + if (strcmp(val, "on") == 0) { + spa->readonly = B_TRUE; + } else if (strcmp(val, "off") == 0) { + spa->readonly = B_FALSE; + } else { + return (EINVAL); + } + dmu_objset_find(spa->spa_name, uzfs_zpool_rdonly_cb, val, DS_FIND_CHILDREN); + } +#endif + need_sync = B_TRUE; break; } @@ -2141,6 +2169,28 @@ spa_load_verify(spa_t *spa) return (verify_ok ? 0 : EIO); } +#ifdef _UZFS +static void +uzfs_get_readonly_prop(spa_t *spa) +{ + char val[4]; // max strlen("off") + int err; + + err = zap_lookup(spa->spa_meta_objset, spa->spa_pool_props_object, + zpool_prop_to_name(ZPOOL_PROP_UZFS_READONLY), 1, 4, &val); + if (err) { + // ignore error if value doesn't exist + return; + } + + if (strcmp(val, "on") == 0) { + spa->readonly = B_TRUE; + } else if (strcmp(val, "off") == 0) { + spa->readonly = B_FALSE; + } +} +#endif + /* * Find a value in the pool props object. */ @@ -3094,6 +3144,9 @@ spa_load_impl(spa_t *spa, uint64_t pool_guid, nvlist_t *config, spa_prop_find(spa, ZPOOL_PROP_MULTIHOST, &spa->spa_multihost); spa_prop_find(spa, ZPOOL_PROP_DEDUPDITTO, &spa->spa_dedup_ditto); +#ifdef _UZFS + uzfs_get_readonly_prop(spa); +#endif spa->spa_autoreplace = (autoreplace != 0); } diff --git a/tests/cstor/gtest/test_zrepl_prot.cc b/tests/cstor/gtest/test_zrepl_prot.cc index 82a8dd2fa838..76f363fd81ed 100644 --- a/tests/cstor/gtest/test_zrepl_prot.cc +++ b/tests/cstor/gtest/test_zrepl_prot.cc @@ -1200,6 +1200,79 @@ TEST_F(ZreplDataTest, ZVolReadOnly) { sleep(5); } +/* + * if zpool is readonly then.. + * - write should fail + * - read should happen + */ +TEST_F(ZreplDataTest, ZpoolReadOnly) { + zvol_io_hdr_t hdr_in; + struct zvol_io_rw_hdr read_hdr; + struct zvol_io_rw_hdr write_hdr; + struct zrepl_status_ack status; + struct mgmt_ack mgmt_ack; + char buf[4096]; + int rc; + std::string output; + + execCmd("zpool", std::string("set io.openebs:readonly=on ") + m_pool1->m_name); + output = execCmd("zpool", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + m_pool1->m_name); + ASSERT_NE(output.find("on"), std::string::npos); + + // read should happen + read_data_start(m_datasock1.fd(), m_ioseq1, 0, sizeof (buf), &hdr_in, &read_hdr); + ASSERT_EQ(hdr_in.status, ZVOL_OP_STATUS_OK); + ASSERT_EQ(hdr_in.len, sizeof (read_hdr) + sizeof (buf)); + ASSERT_EQ(read_hdr.len, sizeof (buf)); + + rc = read(m_datasock1.fd(), buf, read_hdr.len); + ASSERT_ERRNO("read", rc >= 0); + ASSERT_EQ(rc, read_hdr.len); + + // write should fail + init_buf(buf, sizeof (buf), "cStor-data"); + write_data(m_datasock1.fd(), m_ioseq1, buf, 0, sizeof (buf), ++m_ioseq1); + rc = read(m_datasock1.fd(), &hdr_in, sizeof (hdr_in)); + ASSERT_ERRNO("read", rc >= 0); + ASSERT_EQ(rc, sizeof (hdr_in)); + EXPECT_EQ(hdr_in.opcode, ZVOL_OPCODE_WRITE); + EXPECT_EQ(hdr_in.status, ZVOL_OP_STATUS_FAILED); + EXPECT_EQ(hdr_in.io_seq, m_ioseq1); + + m_pool1->pExport(); + m_pool1->import(); + + // mgmt connection should not happen + m_control_fd1 = target1->accept(10); + ASSERT_EQ(m_control_fd1, -1); + + execCmd("zpool", std::string("set io.openebs:readonly=off ") + m_pool1->m_name); + output = execCmd("zpool", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + m_pool1->m_name); + ASSERT_NE(output.find("off"), std::string::npos); + + // mgmt connection should happen + m_control_fd1 = target1->accept(10); + ASSERT_GE(m_control_fd1, 0); + + do_handshake(m_zvol_name1, m_host1, m_port1, NULL, NULL, m_control_fd1, + ZVOL_OP_STATUS_OK); + + do_data_connection(m_datasock1.fd(), m_host1, m_port1, m_zvol_name1); + + // write should happen + write_data(m_datasock1.fd(), m_ioseq1, buf, 0, sizeof (buf), ++m_ioseq1); + rc = read(m_datasock1.fd(), &hdr_in, sizeof (hdr_in)); + ASSERT_ERRNO("read", rc >= 0); + ASSERT_EQ(rc, sizeof (hdr_in)); + EXPECT_EQ(hdr_in.opcode, ZVOL_OPCODE_WRITE); + EXPECT_EQ(hdr_in.status, ZVOL_OP_STATUS_OK); + EXPECT_EQ(hdr_in.io_seq, m_ioseq1); + + m_datasock1.graceful_close(); + m_datasock2.graceful_close(); + sleep(5); +} + TEST(ReplicaState, SingleReplicaQuorumOff) { Zrepl zrepl; TestPool pool("replicaState"); @@ -2048,6 +2121,85 @@ TEST(Snapshot, ZVolReadOnly) { sleep(3); } +TEST(Snapshot, ZpoolReadOnly) { + zvol_io_hdr_t hdr_in, hdr_out = {0}; + Zrepl zrepl; + Target target; + int rc, control_fd; + SocketFd datasock; + TestPool pool("snappool"); + char *buf; + std::string vol_name = pool.getZvolName("vol"); + std::string snap_name = pool.getZvolName("vol@snap"); + uint64_t ioseq; + std::string host; + uint16_t port; + struct zvol_snapshot_list *snaplist; + std::string output; + + zrepl.start(); + pool.create(); + pool.createZvol("vol", "-o io.openebs:targetip=127.0.0.1 -o io.openebs:zvol_replica_id=12345"); + + rc = target.listen(); + ASSERT_GE(rc, 0); + control_fd = target.accept(-1); + ASSERT_GE(control_fd, 0); + + do_handshake(vol_name, host, port, NULL, NULL, control_fd, ZVOL_OP_STATUS_OK); + do_data_connection(datasock.fd(), host, port, vol_name, 4096, 2); + + // create the snapshot + transition_zvol_to_online(ioseq, control_fd, vol_name); + sleep(5); + hdr_out.io_seq = 2; + hdr_out.len = snap_name.length() + 1; + + // make zvol readonly + execCmd("zpool", std::string("set io.openebs:readonly=on ") + pool.m_name); + output = execCmd("zpool", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + pool.m_name); + ASSERT_NE(output.find("on"), std::string::npos); + // snap prepare should fail + prep_snap(control_fd, snap_name, hdr_out.io_seq, ZVOL_OP_STATUS_FAILED); + + // disable zvol readonly + execCmd("zpool", std::string("set io.openebs:readonly=off ") + pool.m_name); + output = execCmd("zpool", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + pool.m_name); + ASSERT_NE(output.find("off"), std::string::npos); + // snap prepare should happen + prep_snap(control_fd, snap_name, hdr_out.io_seq, ZVOL_OP_STATUS_OK); + + + // set zvol readonly + execCmd("zpool", std::string("set io.openebs:readonly=on ") + pool.m_name); + output = execCmd("zpool", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + pool.m_name); + ASSERT_NE(output.find("on"), std::string::npos); + + hdr_out.version = REPLICA_VERSION; + hdr_out.opcode = ZVOL_OPCODE_SNAP_CREATE; + hdr_out.status = ZVOL_OP_STATUS_OK; + hdr_out.io_seq = 0; + hdr_out.len = snap_name.length() + 1; + rc = write(control_fd, &hdr_out, sizeof (hdr_out)); + ASSERT_EQ(rc, sizeof (hdr_out)); + rc = write(control_fd, snap_name.c_str(), hdr_out.len); + ASSERT_EQ(rc, hdr_out.len); + + // snap create should fail + rc = read(control_fd, &hdr_in, sizeof (hdr_in)); + ASSERT_EQ(rc, sizeof (hdr_in)); + EXPECT_EQ(hdr_in.version, REPLICA_VERSION); + EXPECT_EQ(hdr_in.opcode, ZVOL_OPCODE_SNAP_CREATE); + EXPECT_EQ(hdr_in.status, ZVOL_OP_STATUS_FAILED); + EXPECT_EQ(hdr_in.io_seq, 0); + ASSERT_EQ(hdr_in.len, 0); + + datasock.graceful_close(); + graceful_close(control_fd); + sleep(3); +} + + /* * resize the cloned volume when while making From aa206a1721c32f50b541a31caf819f5e82587db4 Mon Sep 17 00:00:00 2001 From: mayank Date: Mon, 10 Feb 2020 16:05:41 +0530 Subject: [PATCH 2/6] Adding 'readOnly' tag in `zfs stats` output Signed-off-by: mayank --- module/zcommon/zpool_prop.c | 4 +-- module/zfs/spa.c | 3 +- module/zfs/zvol.c | 2 ++ tests/cstor/gtest/gtest_utils.cc | 12 ++++++++ tests/cstor/gtest/test_zrepl_prot.cc | 46 ++++++++++++++++------------ 5 files changed, 44 insertions(+), 23 deletions(-) diff --git a/module/zcommon/zpool_prop.c b/module/zcommon/zpool_prop.c index 5d70bdd12cd1..4fb74c357ec5 100644 --- a/module/zcommon/zpool_prop.c +++ b/module/zcommon/zpool_prop.c @@ -73,8 +73,8 @@ zpool_prop_init(void) PROP_DEFAULT, ZFS_TYPE_POOL, " | none", "CACHEFILE"); zprop_register_string(ZPOOL_PROP_COMMENT, "comment", NULL, PROP_DEFAULT, ZFS_TYPE_POOL, "", "COMMENT"); - zprop_register_string(ZPOOL_PROP_UZFS_READONLY, "io.openebs:readonly", "", - PROP_DEFAULT, ZFS_TYPE_POOL, "on | off", "ZPOOL_READONLY"); + zprop_register_string(ZPOOL_PROP_UZFS_READONLY, "io.openebs:readonly", + "", PROP_DEFAULT, ZFS_TYPE_POOL, "on | off", "ZPOOL_READONLY"); /* readonly number properties */ diff --git a/module/zfs/spa.c b/module/zfs/spa.c index dd3618e3dbd6..43effdb64b6b 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -753,7 +753,8 @@ spa_prop_set(spa_t *spa, nvlist_t *nvp) } else { return (EINVAL); } - dmu_objset_find(spa->spa_name, uzfs_zpool_rdonly_cb, val, DS_FIND_CHILDREN); + dmu_objset_find(spa->spa_name, uzfs_zpool_rdonly_cb, + val, DS_FIND_CHILDREN); } #endif diff --git a/module/zfs/zvol.c b/module/zfs/zvol.c index 738410f66dca..4ec9036eb183 100644 --- a/module/zfs/zvol.c +++ b/module/zfs/zvol.c @@ -373,6 +373,8 @@ uzfs_ioc_stats(zfs_cmd_t *zc, nvlist_t *nvl) fnvlist_add_string(innvl, "status", status_to_str(zv)); + fnvlist_add_string(innvl, "readOnly", + IS_ZVOL_READONLY(zv->main_zv) ? "on" : "off"); fnvlist_add_string(innvl, "rebuildStatus", rebuild_status_to_str( diff --git a/tests/cstor/gtest/gtest_utils.cc b/tests/cstor/gtest/gtest_utils.cc index 889849852999..d247c6e55a5e 100644 --- a/tests/cstor/gtest/gtest_utils.cc +++ b/tests/cstor/gtest/gtest_utils.cc @@ -136,6 +136,18 @@ void GtestUtils::TestPool::pExport() { execCmd("zpool", std::string("export ") + m_name); } +bool GtestUtils::TestPool::isReadOnly() { + std::string output; + output = execCmd("zpool", std::string("get -Hp -ovalue io.openebs:readonly ") + m_name); + if (output.compare("on") == 0) + return true; + if (output.compare("off") == 0) + return false; + + throw std::system_error(EINVAL, std::system_category(), + "Invalid readonly value:" + output); +} + void GtestUtils::TestPool::createZvol(std::string name, std::string arg /*= ""*/) { execCmd("zfs", std::string("create -sV ") + std::to_string(ZVOL_SIZE) + diff --git a/tests/cstor/gtest/test_zrepl_prot.cc b/tests/cstor/gtest/test_zrepl_prot.cc index 76f363fd81ed..7af791c8320e 100644 --- a/tests/cstor/gtest/test_zrepl_prot.cc +++ b/tests/cstor/gtest/test_zrepl_prot.cc @@ -1127,6 +1127,22 @@ TEST_F(ZreplDataTest, ReadMetaDataFlag) { sleep(5); } +static bool is_zvol_readonly(std::string zvol) { + std::string output; + + output = execCmd("zfs", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + zvol); + if (output.compare("on") == 0) + return true; + + output = execCmd("zfs", std::string("stats ") + zvol); + std::size_t found = output.find("\"readOnly\":on"); + if (found != std::string::npos) { + return true; + } + + return false; +} + /* * if zvol is readonly then.. * - write should fail @@ -1143,8 +1159,7 @@ TEST_F(ZreplDataTest, ZVolReadOnly) { std::string output; execCmd("zfs", std::string("set io.openebs:readonly=on ") + m_zvol_name1); - output = execCmd("zfs", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + m_zvol_name1); - ASSERT_NE(output.find("on"), std::string::npos); + EXPECT_EQ(is_zvol_readonly(m_zvol_name1), true); // read should happen read_data_start(m_datasock1.fd(), m_ioseq1, 0, sizeof (buf), &hdr_in, &read_hdr); @@ -1174,8 +1189,7 @@ TEST_F(ZreplDataTest, ZVolReadOnly) { ASSERT_EQ(m_control_fd1, -1); execCmd("zfs", std::string("set io.openebs:readonly=off ") + m_zvol_name1); - output = execCmd("zfs", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + m_zvol_name1); - ASSERT_NE(output.find("off"), std::string::npos); + EXPECT_EQ(is_zvol_readonly(m_zvol_name1), false); // mgmt connection should happen m_control_fd1 = target1->accept(10); @@ -1216,8 +1230,7 @@ TEST_F(ZreplDataTest, ZpoolReadOnly) { std::string output; execCmd("zpool", std::string("set io.openebs:readonly=on ") + m_pool1->m_name); - output = execCmd("zpool", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + m_pool1->m_name); - ASSERT_NE(output.find("on"), std::string::npos); + EXPECT_EQ(m_pool1->isReadOnly(), true); // read should happen read_data_start(m_datasock1.fd(), m_ioseq1, 0, sizeof (buf), &hdr_in, &read_hdr); @@ -1247,8 +1260,7 @@ TEST_F(ZreplDataTest, ZpoolReadOnly) { ASSERT_EQ(m_control_fd1, -1); execCmd("zpool", std::string("set io.openebs:readonly=off ") + m_pool1->m_name); - output = execCmd("zpool", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + m_pool1->m_name); - ASSERT_NE(output.find("off"), std::string::npos); + EXPECT_EQ(m_pool1->isReadOnly(), false); // mgmt connection should happen m_control_fd1 = target1->accept(10); @@ -2079,23 +2091,20 @@ TEST(Snapshot, ZVolReadOnly) { // make zvol readonly execCmd("zfs", std::string("set io.openebs:readonly=on ") + vol_name); - output = execCmd("zfs", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + vol_name); - ASSERT_NE(output.find("on"), std::string::npos); + EXPECT_EQ(is_zvol_readonly(vol_name), true); // snap prepare should fail prep_snap(control_fd, snap_name, hdr_out.io_seq, ZVOL_OP_STATUS_FAILED); // disable zvol readonly execCmd("zfs", std::string("set io.openebs:readonly=off ") + vol_name); - output = execCmd("zfs", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + vol_name); - ASSERT_NE(output.find("off"), std::string::npos); + EXPECT_EQ(is_zvol_readonly(vol_name), false); // snap prepare should happen prep_snap(control_fd, snap_name, hdr_out.io_seq, ZVOL_OP_STATUS_OK); // set zvol readonly execCmd("zfs", std::string("set io.openebs:readonly=on ") + vol_name); - output = execCmd("zfs", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + vol_name); - ASSERT_NE(output.find("on"), std::string::npos); + EXPECT_EQ(is_zvol_readonly(vol_name), true); hdr_out.version = REPLICA_VERSION; hdr_out.opcode = ZVOL_OPCODE_SNAP_CREATE; @@ -2157,23 +2166,20 @@ TEST(Snapshot, ZpoolReadOnly) { // make zvol readonly execCmd("zpool", std::string("set io.openebs:readonly=on ") + pool.m_name); - output = execCmd("zpool", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + pool.m_name); - ASSERT_NE(output.find("on"), std::string::npos); + EXPECT_EQ(pool.isReadOnly(), true); // snap prepare should fail prep_snap(control_fd, snap_name, hdr_out.io_seq, ZVOL_OP_STATUS_FAILED); // disable zvol readonly execCmd("zpool", std::string("set io.openebs:readonly=off ") + pool.m_name); - output = execCmd("zpool", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + pool.m_name); - ASSERT_NE(output.find("off"), std::string::npos); + EXPECT_EQ(pool.isReadOnly(), false); // snap prepare should happen prep_snap(control_fd, snap_name, hdr_out.io_seq, ZVOL_OP_STATUS_OK); // set zvol readonly execCmd("zpool", std::string("set io.openebs:readonly=on ") + pool.m_name); - output = execCmd("zpool", std::string("get io.openebs:readonly ") + std::string(" -Hpo value ") + pool.m_name); - ASSERT_NE(output.find("on"), std::string::npos); + EXPECT_EQ(pool.isReadOnly(), true); hdr_out.version = REPLICA_VERSION; hdr_out.opcode = ZVOL_OPCODE_SNAP_CREATE; From 91e346e3de065b43a858be3681d401b9b4be3b14 Mon Sep 17 00:00:00 2001 From: mayank Date: Tue, 11 Feb 2020 15:19:50 +0530 Subject: [PATCH 3/6] Addressing review comments Signed-off-by: mayank --- include/sys/fs/zfs.h | 2 ++ module/zfs/spa.c | 2 ++ tests/cstor/gtest/test_zrepl_prot.cc | 32 ++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index ff095d018ddb..c0eed149ec84 100644 --- a/include/sys/fs/zfs.h +++ b/include/sys/fs/zfs.h @@ -229,7 +229,9 @@ typedef enum { ZPOOL_PROP_TNAME, ZPOOL_PROP_MAXDNODESIZE, ZPOOL_PROP_MULTIHOST, +#ifdef _UZFS ZPOOL_PROP_UZFS_READONLY, +#endif ZPOOL_NUM_PROPS } zpool_prop_t; diff --git a/module/zfs/spa.c b/module/zfs/spa.c index 43effdb64b6b..8ad3a28477e8 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -755,6 +755,8 @@ spa_prop_set(spa_t *spa, nvlist_t *nvp) } dmu_objset_find(spa->spa_name, uzfs_zpool_rdonly_cb, val, DS_FIND_CHILDREN); + need_sync = B_TRUE; + continue; } #endif diff --git a/tests/cstor/gtest/test_zrepl_prot.cc b/tests/cstor/gtest/test_zrepl_prot.cc index 7af791c8320e..bf74ca0a5d6d 100644 --- a/tests/cstor/gtest/test_zrepl_prot.cc +++ b/tests/cstor/gtest/test_zrepl_prot.cc @@ -1143,6 +1143,26 @@ static bool is_zvol_readonly(std::string zvol) { return false; } +static bool isIOAckSenderCreated(std::string zvol) { + std::string output; + + output = execCmd("zfs", std::string("stats ") + zvol); + if (output.find("\"isIOAckSenderCreated\":1") != std::string::npos) + return true; + + return false; +} + +static bool isIOReceiverCreated(std::string zvol) { + std::string output; + + output = execCmd("zfs", std::string("stats ") + zvol); + if (output.find("\"isIOReceiverCreated\":1") != std::string::npos) + return true; + + return false; +} + /* * if zvol is readonly then.. * - write should fail @@ -1160,6 +1180,8 @@ TEST_F(ZreplDataTest, ZVolReadOnly) { execCmd("zfs", std::string("set io.openebs:readonly=on ") + m_zvol_name1); EXPECT_EQ(is_zvol_readonly(m_zvol_name1), true); + EXPECT_EQ(isIOAckSenderCreated(m_zvol_name1), true); + EXPECT_EQ(isIOReceiverCreated(m_zvol_name1), true); // read should happen read_data_start(m_datasock1.fd(), m_ioseq1, 0, sizeof (buf), &hdr_in, &read_hdr); @@ -1187,6 +1209,8 @@ TEST_F(ZreplDataTest, ZVolReadOnly) { // mgmt connection should not happen m_control_fd1 = target1->accept(10); ASSERT_EQ(m_control_fd1, -1); + EXPECT_EQ(isIOAckSenderCreated(m_zvol_name1), false); + EXPECT_EQ(isIOReceiverCreated(m_zvol_name1), false); execCmd("zfs", std::string("set io.openebs:readonly=off ") + m_zvol_name1); EXPECT_EQ(is_zvol_readonly(m_zvol_name1), false); @@ -1199,6 +1223,8 @@ TEST_F(ZreplDataTest, ZVolReadOnly) { ZVOL_OP_STATUS_OK); do_data_connection(m_datasock1.fd(), m_host1, m_port1, m_zvol_name1); + EXPECT_EQ(isIOAckSenderCreated(m_zvol_name1), true); + EXPECT_EQ(isIOReceiverCreated(m_zvol_name1), true); // write should happen write_data(m_datasock1.fd(), m_ioseq1, buf, 0, sizeof (buf), ++m_ioseq1); @@ -1231,6 +1257,8 @@ TEST_F(ZreplDataTest, ZpoolReadOnly) { execCmd("zpool", std::string("set io.openebs:readonly=on ") + m_pool1->m_name); EXPECT_EQ(m_pool1->isReadOnly(), true); + EXPECT_EQ(isIOAckSenderCreated(m_zvol_name1), true); + EXPECT_EQ(isIOReceiverCreated(m_zvol_name1), true); // read should happen read_data_start(m_datasock1.fd(), m_ioseq1, 0, sizeof (buf), &hdr_in, &read_hdr); @@ -1258,6 +1286,8 @@ TEST_F(ZreplDataTest, ZpoolReadOnly) { // mgmt connection should not happen m_control_fd1 = target1->accept(10); ASSERT_EQ(m_control_fd1, -1); + EXPECT_EQ(isIOAckSenderCreated(m_zvol_name1), false); + EXPECT_EQ(isIOReceiverCreated(m_zvol_name1), false); execCmd("zpool", std::string("set io.openebs:readonly=off ") + m_pool1->m_name); EXPECT_EQ(m_pool1->isReadOnly(), false); @@ -1270,6 +1300,8 @@ TEST_F(ZreplDataTest, ZpoolReadOnly) { ZVOL_OP_STATUS_OK); do_data_connection(m_datasock1.fd(), m_host1, m_port1, m_zvol_name1); + EXPECT_EQ(isIOAckSenderCreated(m_zvol_name1), true); + EXPECT_EQ(isIOReceiverCreated(m_zvol_name1), true); // write should happen write_data(m_datasock1.fd(), m_ioseq1, buf, 0, sizeof (buf), ++m_ioseq1); From 5299b04063f42a65979d26f657fffd9edf6b9a8a Mon Sep 17 00:00:00 2001 From: mayank Date: Wed, 12 Feb 2020 12:17:29 +0530 Subject: [PATCH 4/6] Addressing review comments Signed-off-by: mayank --- module/zfs/spa.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/module/zfs/spa.c b/module/zfs/spa.c index 8ad3a28477e8..e42daa1f19b0 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -644,8 +644,8 @@ spa_prop_validate(spa_t *spa, nvlist_t *props) if (strcmp(strval, "on") != 0 && strcmp(strval, "off") != 0) { error = SET_ERROR(EINVAL); - break; } + break; #endif default: break; @@ -701,6 +701,7 @@ spa_prop_set(spa_t *spa, nvlist_t *nvp) boolean_t need_sync = B_FALSE; #ifdef _UZFS char *val; + boolean_t update_rdonly = B_FALSE; #endif if ((error = spa_prop_validate(spa, nvp)) != 0) @@ -747,15 +748,24 @@ spa_prop_set(spa_t *spa, nvlist_t *nvp) if (prop == ZPOOL_PROP_UZFS_READONLY) { VERIFY(nvpair_value_string(elem, &val) == 0); if (strcmp(val, "on") == 0) { - spa->readonly = B_TRUE; + if (!spa->readonly) { + update_rdonly = B_TRUE; + spa->readonly = B_TRUE; + } } else if (strcmp(val, "off") == 0) { - spa->readonly = B_FALSE; + if (spa->readonly) { + update_rdonly = B_TRUE; + spa->readonly = B_FALSE; + } } else { return (EINVAL); } - dmu_objset_find(spa->spa_name, uzfs_zpool_rdonly_cb, - val, DS_FIND_CHILDREN); - need_sync = B_TRUE; + if (update_rdonly) { + dmu_objset_find(spa->spa_name, + uzfs_zpool_rdonly_cb, val, + DS_FIND_CHILDREN); + need_sync = B_TRUE; + } continue; } #endif @@ -2180,7 +2190,8 @@ uzfs_get_readonly_prop(spa_t *spa) int err; err = zap_lookup(spa->spa_meta_objset, spa->spa_pool_props_object, - zpool_prop_to_name(ZPOOL_PROP_UZFS_READONLY), 1, 4, &val); + zpool_prop_to_name(ZPOOL_PROP_UZFS_READONLY), 1, + sizeof (val), &val); if (err) { // ignore error if value doesn't exist return; From 6c49caac875484c19f635dc05f341c049ee235b9 Mon Sep 17 00:00:00 2001 From: mayank Date: Wed, 12 Feb 2020 17:25:35 +0530 Subject: [PATCH 5/6] Adding logging Signed-off-by: mayank --- module/zfs/spa.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/module/zfs/spa.c b/module/zfs/spa.c index e42daa1f19b0..0e72a4e422e6 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -761,6 +761,8 @@ spa_prop_set(spa_t *spa, nvlist_t *nvp) return (EINVAL); } if (update_rdonly) { + fprintf(stderr, "Updating readonly for %s " + "to %s\n", spa->spa_name, val); dmu_objset_find(spa->spa_name, uzfs_zpool_rdonly_cb, val, DS_FIND_CHILDREN); @@ -2202,6 +2204,8 @@ uzfs_get_readonly_prop(spa_t *spa) } else if (strcmp(val, "off") == 0) { spa->readonly = B_FALSE; } + fprintf(stderr, "pool %s imported with readonly:%s\n", + spa->spa_name, val); } #endif From 4ba36f526ddbbc2a4ae318a38944490d6055fbef Mon Sep 17 00:00:00 2001 From: mayank Date: Thu, 13 Feb 2020 09:13:19 +0530 Subject: [PATCH 6/6] Fixing kernel build Signed-off-by: mayank --- module/zcommon/zpool_prop.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/module/zcommon/zpool_prop.c b/module/zcommon/zpool_prop.c index 4fb74c357ec5..b15ff0b4a5fa 100644 --- a/module/zcommon/zpool_prop.c +++ b/module/zcommon/zpool_prop.c @@ -73,8 +73,10 @@ zpool_prop_init(void) PROP_DEFAULT, ZFS_TYPE_POOL, " | none", "CACHEFILE"); zprop_register_string(ZPOOL_PROP_COMMENT, "comment", NULL, PROP_DEFAULT, ZFS_TYPE_POOL, "", "COMMENT"); +#ifdef _UZFS zprop_register_string(ZPOOL_PROP_UZFS_READONLY, "io.openebs:readonly", "", PROP_DEFAULT, ZFS_TYPE_POOL, "on | off", "ZPOOL_READONLY"); +#endif /* readonly number properties */