Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(readonly): Adding test cases for uzfs zvol readonly support #288

Merged
merged 5 commits into from
Feb 3, 2020
Merged

feat(readonly): Adding test cases for uzfs zvol readonly support #288

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
932121b
Adding test cases for uzfs zvol readonly support
Jan 31, 2020
ad3d2a5
Merge branch 'develop' into pool_crash
mynktl Jan 31, 2020
5bcdb0f
Addressing review comments
Feb 3, 2020
d0c59f4
Merge branch 'pool_crash' of github.com:mynktl/cstor into pool_crash
Feb 3, 2020
2b37d0c
Addressing review comment
Feb 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/sys/fs/zfs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ typedef enum {
ZFS_PROP_QUORUM,
ZFS_PROP_TARGETIP,
ZFS_PROP_REPLICA_ID,
ZFS_PROP_ZVOL_READONLY,
ZFS_PROP_WORKERS,
ZFS_NUM_PROPS
} zfs_prop_t;
Expand Down
2 changes: 2 additions & 0 deletions module/zcommon/zfs_prop.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,8 @@ zfs_prop_init(void)
PROP_DEFAULT, ZFS_TYPE_VOLUME, "<zvol worker>", "NWORKER");
zprop_register_string(ZFS_PROP_REPLICA_ID, "io.openebs:zvol_replica_id",
"", PROP_DEFAULT, ZFS_TYPE_VOLUME, "<replica id>", "REPLICA_ID");
zprop_register_string(ZFS_PROP_ZVOL_READONLY, "io.openebs:readonly", "",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vishnuitta @mynktl , io.openebs:readonly does not convey it's meaning, also it conflicts with the existing readony property. Can we rename it to io.openebs:suspend (or some better name)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both property have the same functionality but at different layer. To differentiate this i thought of giving a different name. One more point, this is completely limited to cstor API layer only, so better to group it with organization.

PROP_DEFAULT, ZFS_TYPE_VOLUME, "on | off", "ZVOL_READONLY");

/* readonly index (boolean) properties */
zprop_register_index(ZFS_PROP_MOUNTED, "mounted", 0, PROP_READONLY,
Expand Down
8 changes: 7 additions & 1 deletion module/zfs/zfs_ioctl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ static int get_nvlist(uint64_t nvl, uint64_t size, int iflag, nvlist_t **nvp);
#include <sys/dmu_impl.h>
#include <uzfs_mgmt.h>
#include <zrepl_mgmt.h>
#include <uzfs_prop.h>

#include "zfs_fletcher.h"
#include "zfs_namecheck.h"
Expand Down Expand Up @@ -2949,6 +2950,7 @@ zfs_ioc_set_prop(zfs_cmd_t *zc)
zprop_source_t source = (received ? ZPROP_SRC_RECEIVED :
ZPROP_SRC_LOCAL);
nvlist_t *errors;
char *val;
int error;

#ifdef _UZFS
Expand Down Expand Up @@ -2984,9 +2986,13 @@ zfs_ioc_set_prop(zfs_cmd_t *zc)
*/
curtargetip[0] = '\0';
(void) nvlist_lookup_string(nvl, ZFS_PROP_TARGET_IP, &targetip);
if (targetip != NULL)
if (targetip != NULL) {
error = zfs_set_targetip_prehook(zc->zc_name, source, targetip,
&curtargetip[0]);
} else if (error == 0 && nvlist_lookup_string(nvl,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets allow setting both (targetip and readonly) at once if we want to?

Copy link
Member Author

@mynktl mynktl Feb 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can do that, but that will complicate creation part i think from control plane, need to check. We can plan this in next release.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the problem if we remove 'else' which allows both the settings?

zfs_prop_to_name(ZFS_PROP_ZVOL_READONLY), &val) == 0) {
error = uzfs_zinfo_update_rdonly(zc->zc_name, val);
}
#endif

errors = fnvlist_alloc();
Expand Down
4 changes: 4 additions & 0 deletions tests/cstor/gtest/gtest_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ void GtestUtils::TestPool::import() {
execCmd("zpool", std::string("import ") + m_name + " -d /tmp");
}

void GtestUtils::TestPool::pExport() {
execCmd("zpool", std::string("export ") + m_name);
}

void GtestUtils::TestPool::createZvol(std::string name, std::string arg /*= ""*/) {
execCmd("zfs",
std::string("create -sV ") + std::to_string(ZVOL_SIZE) +
Expand Down
172 changes: 166 additions & 6 deletions tests/cstor/gtest/test_zrepl_prot.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,20 +700,21 @@ class ZreplDataTest : public testing::Test {
* the setup hook should be simplified to minimum again.
*/
static void SetUpTestCase() {
Target target1, target2;
m_pool1 = new TestPool("ihandshake");
m_pool2 = new TestPool("handshake");
m_zrepl = new Zrepl();
target1 = new Target();
target2 = new Target();
int rc;

m_zrepl->start();
m_pool1->create();
m_pool1->createZvol("ivol1", "-o io.openebs:targetip=127.0.0.1:6060 -o io.openebs:zvol_replica_id=12345");
m_zvol_name1 = m_pool1->getZvolName("ivol1");

rc = target1.listen();
rc = target1->listen();
ASSERT_GE(rc, 0);
m_control_fd1 = target1.accept(-1);
m_control_fd1 = target1->accept(-1);
ASSERT_GE(m_control_fd1, 0);

do_handshake(m_zvol_name1, m_host1, m_port1, NULL, NULL, m_control_fd1,
Expand All @@ -722,7 +723,7 @@ class ZreplDataTest : public testing::Test {

m_zrepl->start();
m_pool1->import();
m_control_fd1 = target1.accept(-1);
m_control_fd1 = target1->accept(-1);
ASSERT_GE(m_control_fd1, 0);

do_handshake(m_zvol_name1, m_host1, m_port1, NULL, NULL, m_control_fd1,
Expand All @@ -732,9 +733,9 @@ class ZreplDataTest : public testing::Test {
m_pool2->createZvol("vol1", "-o io.openebs:targetip=127.0.0.1:12345 -o io.openebs:zvol_replica_id=12345");
m_zvol_name2 = m_pool1->getZvolName("ivol1");

rc = target2.listen(12345);
rc = target2->listen(12345);
ASSERT_GE(rc, 0);
m_control_fd2 = target2.accept(-1);
m_control_fd2 = target2->accept(-1);
ASSERT_GE(m_control_fd2, 0);

do_handshake(m_zvol_name2, m_host2, m_port2, NULL, NULL, m_control_fd2,
Expand All @@ -755,6 +756,8 @@ class ZreplDataTest : public testing::Test {
if (m_control_fd2 >= 0)
close(m_control_fd2);
delete m_zrepl;
delete target1;
delete target2;
}

ZreplDataTest() {
Expand Down Expand Up @@ -786,6 +789,8 @@ class ZreplDataTest : public testing::Test {
static TestPool *m_pool2;
static std::string m_zvol_name1;
static std::string m_zvol_name2;
static Target *target1;
static Target *target2;

SocketFd m_datasock1;
SocketFd m_datasock2;
Expand All @@ -805,6 +810,9 @@ std::string ZreplDataTest::m_zvol_name2 = "";
TestPool *ZreplDataTest::m_pool2 = nullptr;
Zrepl *ZreplDataTest::m_zrepl = nullptr;

Target *ZreplDataTest::target1 = nullptr;
Target *ZreplDataTest::target2 = nullptr;

TEST_F(ZreplDataTest, WrongVersion) {
zvol_io_hdr_t hdr_in, hdr_out = {0};
// use unique ptr to implicitly dealloc mem when exiting from func
Expand Down Expand Up @@ -1119,6 +1127,79 @@ TEST_F(ZreplDataTest, ReadMetaDataFlag) {
sleep(5);
}

/*
* if zvol is readonly then..
* - write should fail
* - read should happen
*/
TEST_F(ZreplDataTest, ZVolReadOnly) {
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("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);

// 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("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);

// 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");
Expand Down Expand Up @@ -1889,6 +1970,85 @@ TEST(Snapshot, CreateAndDestroy) {
sleep(3);
}

TEST(Snapshot, ZVolReadOnly) {
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("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);
// 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);
// 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);

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
* the data conntection.
Expand Down