On 07/04/2018 05:23 AM, Michal Privoznik wrote:
Extend this existing test so that a case when IQN is provided is tested too. Since a special iSCSI interface is created and its name is randomly generated at runtime we need to link with virrandommock to have predictable names.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/viriscsitest.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 2 deletions(-)
Should testIscsiadmCbData initialization get { false, false } now (and I should have mention in patch 9 that : + struct testIscsiadmCbData cbData = { 0 }; should be + struct testIscsiadmCbData cbData = { false }; I know that 0 and false are synonymous, but syntactical correctness is in play here ;-) I also think you're doing two things here: 1. Generating a dryRun output for "existing" test without initiator.iqn 2. Adding tests to test initiator.iqn
diff --git a/tests/viriscsitest.c b/tests/viriscsitest.c index c6e0e3b8ff..55889d04a3 100644 --- a/tests/viriscsitest.c +++ b/tests/viriscsitest.c @@ -60,8 +60,19 @@ const char *iscsiadmSendtargetsOutput = "10.20.30.40:3260,1 iqn.2008-04.example:example1:iscsi.bar\n" "10.20.30.40:3260,1 iqn.2009-04.example:example1:iscsi.seven\n";
+const char *iscsiadmIfaceDefaultOutput = + "default tcp,<empty>,<empty>,<empty>,<empty>\n" + "iser iser,<empty>,<empty>,<empty>,<empty>\n"; + +const char *iscsiadmIfaceIfaceOutput = + "default tcp,<empty>,<empty>,<empty>,<empty>\n" + "iser iser,<empty>,<empty>,<empty>,<empty>\n" + "libvirt-iface-03020100 tcp,<empty>,<empty>,<empty>,iqn.2004-06.example:example1:initiator\n"; + + struct testIscsiadmCbData { bool output_version; + bool iface_created; };
static void testIscsiadmCb(const char *const*args, @@ -103,6 +114,62 @@ static void testIscsiadmCb(const char *const*args, args[7] && STREQ(args[7], "--login") && args[8] == NULL) { /* nada */
Is this "nada"
+ } else if (args[0] && STREQ(args[0], ISCSIADM) && + args[1] && STREQ(args[1], "--mode") && + args[2] && STREQ(args[2], "iface") && + args[3] == NULL) { + if (data->iface_created)
How would iface_created be set by this point if...
+ ignore_value(VIR_STRDUP(*output, iscsiadmIfaceIfaceOutput)); + else + ignore_value(VIR_STRDUP(*output, iscsiadmIfaceDefaultOutput)); + } else if (args[0] && STREQ(args[0], ISCSIADM) && + args[1] && STREQ(args[1], "--mode") && + args[2] && STREQ(args[2], "iface") && + args[3] && STREQ(args[3], "--interface") && + args[4] && STREQ(args[4], "libvirt-iface-03020100") && + args[5] && STREQ(args[5], "--op") && + args[6] && STREQ(args[6], "new") && + args[7] == NULL) { + data->iface_created = true;
... it's being set unconditionally here? See note below, shouldn't the caller be doing this?
+ } else if (args[0] && STREQ(args[0], ISCSIADM) && + args[1] && STREQ(args[1], "--mode") && + args[2] && STREQ(args[2], "iface") && + args[3] && STREQ(args[3], "--interface") && + args[4] && STREQ(args[4], "libvirt-iface-03020100") && + args[5] && STREQ(args[5], "--op") && + args[6] && STREQ(args[6], "update") && + args[7] && STREQ(args[7], "--name") && + args[8] && STREQ(args[8], "iface.initiatorname") && + args[9] && STREQ(args[9], "--value") && + args[10] && STREQ(args[10], "iqn.2004-06.example:example1:initiator") && + args[11] == NULL && + data->iface_created) { + /* nada */
?? Why nada (now you know where my 7/10 requery came from)
+ } else if (args[0] && STREQ(args[0], ISCSIADM) && + args[1] && STREQ(args[1], "--mode") && + args[2] && STREQ(args[2], "discovery") && + args[3] && STREQ(args[3], "--type") && + args[4] && STREQ(args[4], "sendtargets") && + args[5] && STREQ(args[5], "--portal") && + args[6] && STREQ(args[6], "10.20.30.40:3260,1") && + args[7] && STREQ(args[7], "--interface") && + args[8] && STREQ(args[8], "libvirt-iface-03020100") && + args[9] == NULL && + data->iface_created) { + ignore_value(VIR_STRDUP(*output, iscsiadmSendtargetsOutput)); + } else if (args[0] && STREQ(args[0], ISCSIADM) && + args[1] && STREQ(args[1], "--mode") && + args[2] && STREQ(args[2], "node") && + args[3] && STREQ(args[3], "--portal") && + args[4] && STREQ(args[4], "10.20.30.40:3260,1") && + args[5] && STREQ(args[5], "--targetname") && + args[6] && STREQ(args[6], "iqn.2004-06.example:example1:iscsi.test") && + args[7] && STREQ(args[7], "--login") && + args[8] && STREQ(args[8], "--interface") && + args[9] && STREQ(args[9], "libvirt-iface-03020100") && + args[10] == NULL && + data->iface_created) { + /* nada */
Similar, why nada?
} else { *status = -1; } @@ -204,9 +271,10 @@ static int testISCSIConnectionLogin(const void *data) { const struct testConnectionInfoLogin *info = data; + struct testIscsiadmCbData cbData = { 0 };
s/0/false, false/
int ret = -1;
Why wouldn't initialization be: cbData.iface_create = info->initiatoriqn != NULL; John
- virCommandSetDryRun(NULL, testIscsiadmCb, NULL); + virCommandSetDryRun(NULL, testIscsiadmCb, &cbData);
if (virISCSIConnectionLogin(info->portal, info->initiatoriqn, info->target) < 0) goto cleanup; @@ -265,11 +333,14 @@ mymain(void) } while (0)
DO_LOGIN_TEST("10.20.30.40:3260,1", NULL, "iqn.2004-06.example:example1:iscsi.test"); + DO_LOGIN_TEST("10.20.30.40:3260,1", "iqn.2004-06.example:example1:initiator", + "iqn.2004-06.example:example1:iscsi.test");
if (rv < 0) return EXIT_FAILURE; return EXIT_SUCCESS; }
-VIR_TEST_MAIN(mymain) +VIR_TEST_MAIN_PRELOAD(mymain, + abs_builddir "/.libs/virrandommock.so") #endif /* WIN32 */