| Message ID | 20180705080017.31123-3-bala24@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 05, 2018 at 01:30:17PM +0530, Balamuruhan S wrote: > This patch adds test for multifd migration feature with fd protocol. > > Signed-off-by: Balamuruhan S <bala24@linux.vnet.ibm.com> > --- > tests/migration-test.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 57 insertions(+) > > diff --git a/tests/migration-test.c b/tests/migration-test.c > index e2434e70ea..29ede3810d 100644 > --- a/tests/migration-test.c > +++ b/tests/migration-test.c > @@ -10,6 +10,7 @@ > * > */ > > +#include <unistd.h> > #include "qemu/osdep.h" > > #include "libqtest.h" > @@ -202,6 +203,17 @@ static void read_blocktime(QTestState *who) > qobject_unref(rsp); > } > > +static void getfd(QTestState *who, const char *fdname, int fd) > +{ > + QDict *rsp; > + gchar *cmd = g_strdup_printf("{ 'execute': 'getfd'," > + "'arguments': { '%s': '%d'} }", fdname, fd); > + rsp = wait_command(who, cmd); AFAIU Libvirt passes fds to QEMU via the QMP channel using SCM_RIGHTS. Here we directly specified the fd number. Could you help explain a bit on how it worked? > + g_assert(qdict_haskey(rsp, "return")); > + g_free(cmd); > + qobject_unref(rsp); > +} > + > static void wait_for_migration_complete(QTestState *who) > { > while (true) { > @@ -619,6 +631,50 @@ static void test_precopy_unix(void) > g_free(uri); > } > > +static void test_multifd_fd(void) > +{ > + int fd[2]; > + > + /* create pipe */ > + if (pipe(fd) == -1) > + g_test_message("Skipping test: Unable to create pipe"); > + return; > + > + /* set uri in target with read fd */ > + char *uri = g_strdup_printf("fd:%d", fd[0]); > + QTestState *from, *to; > + test_migrate_start(&from, &to, uri, false); > + > + /* Receive a write file descriptor for source using getfd */ > + getfd(from, "srcfd", fd[1]); > + > + /* set multifd capability on source and target */ > + migrate_set_capability(from, "x-multifd", "true"); > + migrate_set_capability(to, "x-multifd", "true"); > + > + /* Wait for the first serial output from the source */ > + wait_for_serial("src_serial"); > + > + /* perform migration */ > + migrate(from, uri); I'm not sure I understand correctly here, but I feel like we should use the fd[1] or anything that was bound to fd[1] on source side to do the migration rather than fd[0]? Please feel free to correct me.. Regards, > + > + wait_for_migration_pass(from); > + > + if (!got_stop) { > + qtest_qmp_eventwait(from, "STOP"); > + } > + qtest_qmp_eventwait(to, "RESUME"); > + > + wait_for_serial("dest_serial"); > + wait_for_migration_complete(from); > + test_migrate_end(from, to, true); > + g_free(uri); > + > + /* close the fds */ > + close(fd[0]); > + close(fd[1]); > +} > + > static void test_multifd_unix(void) > { > char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); > @@ -673,6 +729,7 @@ int main(int argc, char **argv) > qtest_add_func("/migration/bad_dest", test_baddest); > qtest_add_func("/migration/precopy/unix", test_precopy_unix); > qtest_add_func("/migration/multifd/unix", test_multifd_unix); > + qtest_add_func("/migration/multifd/fd", test_multifd_fd); > > ret = g_test_run(); > > -- > 2.14.3 >
On Thu, Jul 05, 2018 at 05:26:09PM +0800, Peter Xu wrote: > On Thu, Jul 05, 2018 at 01:30:17PM +0530, Balamuruhan S wrote: > > This patch adds test for multifd migration feature with fd protocol. > > > > Signed-off-by: Balamuruhan S <bala24@linux.vnet.ibm.com> > > --- > > tests/migration-test.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 57 insertions(+) > > > > diff --git a/tests/migration-test.c b/tests/migration-test.c > > index e2434e70ea..29ede3810d 100644 > > --- a/tests/migration-test.c > > +++ b/tests/migration-test.c > > @@ -10,6 +10,7 @@ > > * > > */ > > > > +#include <unistd.h> > > #include "qemu/osdep.h" > > > > #include "libqtest.h" > > @@ -202,6 +203,17 @@ static void read_blocktime(QTestState *who) > > qobject_unref(rsp); > > } > > > > +static void getfd(QTestState *who, const char *fdname, int fd) > > +{ > > + QDict *rsp; > > + gchar *cmd = g_strdup_printf("{ 'execute': 'getfd'," > > + "'arguments': { '%s': '%d'} }", fdname, fd); > > + rsp = wait_command(who, cmd); > > AFAIU Libvirt passes fds to QEMU via the QMP channel using SCM_RIGHTS. > Here we directly specified the fd number. Could you help explain a bit > on how it worked? Thank you Peter for reviewing the patch. so I explored on pipe() system call, that could create channel for inter process communication. It gives us 2 fds one for read (I thought we can use it for target qemu process) and one for write (we can use it for source qemu process). I tried the example snippet from man page so I thought we can use it in our migration Qtest. Please correct my work if it doesn't make sense. > > > + g_assert(qdict_haskey(rsp, "return")); > > + g_free(cmd); > > + qobject_unref(rsp); > > +} > > + > > static void wait_for_migration_complete(QTestState *who) > > { > > while (true) { > > @@ -619,6 +631,50 @@ static void test_precopy_unix(void) > > g_free(uri); > > } > > > > +static void test_multifd_fd(void) > > +{ > > + int fd[2]; > > + > > + /* create pipe */ > > + if (pipe(fd) == -1) > > + g_test_message("Skipping test: Unable to create pipe"); > > + return; > > + > > + /* set uri in target with read fd */ > > + char *uri = g_strdup_printf("fd:%d", fd[0]); > > + QTestState *from, *to; > > + test_migrate_start(&from, &to, uri, false); > > + > > + /* Receive a write file descriptor for source using getfd */ > > + getfd(from, "srcfd", fd[1]); > > + > > + /* set multifd capability on source and target */ > > + migrate_set_capability(from, "x-multifd", "true"); > > + migrate_set_capability(to, "x-multifd", "true"); > > + > > + /* Wait for the first serial output from the source */ > > + wait_for_serial("src_serial"); > > + > > + /* perform migration */ > > + migrate(from, uri); > > I'm not sure I understand correctly here, but I feel like we should > use the fd[1] or anything that was bound to fd[1] on source side to do > the migration rather than fd[0]? Please feel free to correct me.. > fd[0] - read channel so we should use it for target (-incoming fd:fd[0]) fd[1] - write channel so we shouls use it for source to transfer/write and for it we need to assign using `getfd` monitor command on source. > Regards, > > > + > > + wait_for_migration_pass(from); > > + > > + if (!got_stop) { > > + qtest_qmp_eventwait(from, "STOP"); > > + } > > + qtest_qmp_eventwait(to, "RESUME"); > > + > > + wait_for_serial("dest_serial"); > > + wait_for_migration_complete(from); > > + test_migrate_end(from, to, true); > > + g_free(uri); > > + > > + /* close the fds */ > > + close(fd[0]); > > + close(fd[1]); After studying manual example program, we should close the fds before we start migration like we use for O_CLOSEXEC on fd. so I will work on resend the patch and fixing style issue caught by patchew (sorry for it). -- Bala > > +} > > + > > static void test_multifd_unix(void) > > { > > char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); > > @@ -673,6 +729,7 @@ int main(int argc, char **argv) > > qtest_add_func("/migration/bad_dest", test_baddest); > > qtest_add_func("/migration/precopy/unix", test_precopy_unix); > > qtest_add_func("/migration/multifd/unix", test_multifd_unix); > > + qtest_add_func("/migration/multifd/fd", test_multifd_fd); > > > > ret = g_test_run(); > > > > -- > > 2.14.3 > > > > -- > Peter Xu >
On Thu, Jul 05, 2018 at 06:22:31PM +0530, Balamuruhan S wrote: > On Thu, Jul 05, 2018 at 05:26:09PM +0800, Peter Xu wrote: > > On Thu, Jul 05, 2018 at 01:30:17PM +0530, Balamuruhan S wrote: > > > This patch adds test for multifd migration feature with fd protocol. > > > > > > Signed-off-by: Balamuruhan S <bala24@linux.vnet.ibm.com> > > > --- > > > tests/migration-test.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 57 insertions(+) > > > > > > diff --git a/tests/migration-test.c b/tests/migration-test.c > > > index e2434e70ea..29ede3810d 100644 > > > --- a/tests/migration-test.c > > > +++ b/tests/migration-test.c > > > @@ -10,6 +10,7 @@ > > > * > > > */ > > > > > > +#include <unistd.h> > > > #include "qemu/osdep.h" > > > > > > #include "libqtest.h" > > > @@ -202,6 +203,17 @@ static void read_blocktime(QTestState *who) > > > qobject_unref(rsp); > > > } > > > > > > +static void getfd(QTestState *who, const char *fdname, int fd) > > > +{ > > > + QDict *rsp; > > > + gchar *cmd = g_strdup_printf("{ 'execute': 'getfd'," > > > + "'arguments': { '%s': '%d'} }", fdname, fd); > > > + rsp = wait_command(who, cmd); > > > > AFAIU Libvirt passes fds to QEMU via the QMP channel using SCM_RIGHTS. > > Here we directly specified the fd number. Could you help explain a bit > > on how it worked? > > Thank you Peter for reviewing the patch. > > so I explored on pipe() system call, that could create channel for inter > process communication. It gives us 2 fds one for read (I thought > we can use it for target qemu process) and one for write (we can use it > for source qemu process). I tried the example snippet from man page so > I thought we can use it in our migration Qtest. > > Please correct my work if it doesn't make sense. Yeah the pipe() usage should be fine - though it's the way to pass it to QEMU that I'm not sure about. Ok I gave it a shot with your tests on my host and I got this actually: $ QTEST_QEMU_BINARY=./x86_64-softmmu/qemu-system-x86_64 ./tests/migration-test -p /x86_64/migration/multifd/fd /x86_64/migration/multifd/fd: ** ERROR:/home/peterx/git/qemu/tests/migration-test.c:212:getfd: assertion failed: (qdict_haskey(rsp, "return")) Aborted (core dumped) $ QTEST_QEMU_BINARY=./x86_64-softmmu/qemu-system-x86_64 QTEST_LOG=1 ./tests/migration-test -p /x86_64/migration/multifd/fd /x86_64/migration/multifd/fd: [I 1530851360.669526] OPENED [R +0.037850] endianness [S +0.037870] OK little {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2}, "package": "v2.12.0-2164-g20aa87a802"}, "capabilities": []}}{"execute": "qmp_capabilities"} {"return": {}}[I 1530851360.754972] OPENED [R +0.055719] endianness [S +0.055737] OK little {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2}, "package": "v2.12.0-2164-g20aa87a802"}, "capabilities": []}}{"execute": "qmp_capabilities"} {"return": {}}{"execute": "getfd", "arguments": {"srcfd": "5"}} {"error": {"class": "GenericError", "desc": "Parameter 'fdname' is missing"}}** ERROR:/home/peterx/git/qemu/tests/migration-test.c:212:getfd: assertion failed: (qdict_haskey(rsp, "return")) [I +0.058378] CLOSED [I +0.156930] CLOSED Aborted (core dumped) So it seems that the "getfd" command will only take one parameter "fdname", then the fd should be passed in by UNIX socket magic. That should match with what I understand it. I'm not sure why it worked on your host. > > > > > > + g_assert(qdict_haskey(rsp, "return")); > > > + g_free(cmd); > > > + qobject_unref(rsp); > > > +} > > > + > > > static void wait_for_migration_complete(QTestState *who) > > > { > > > while (true) { > > > @@ -619,6 +631,50 @@ static void test_precopy_unix(void) > > > g_free(uri); > > > } > > > > > > +static void test_multifd_fd(void) > > > +{ > > > + int fd[2]; > > > + > > > + /* create pipe */ > > > + if (pipe(fd) == -1) > > > + g_test_message("Skipping test: Unable to create pipe"); > > > + return; > > > + > > > + /* set uri in target with read fd */ > > > + char *uri = g_strdup_printf("fd:%d", fd[0]); > > > + QTestState *from, *to; > > > + test_migrate_start(&from, &to, uri, false); > > > + > > > + /* Receive a write file descriptor for source using getfd */ > > > + getfd(from, "srcfd", fd[1]); > > > + > > > + /* set multifd capability on source and target */ > > > + migrate_set_capability(from, "x-multifd", "true"); > > > + migrate_set_capability(to, "x-multifd", "true"); > > > + > > > + /* Wait for the first serial output from the source */ > > > + wait_for_serial("src_serial"); > > > + > > > + /* perform migration */ > > > + migrate(from, uri); > > > > I'm not sure I understand correctly here, but I feel like we should > > use the fd[1] or anything that was bound to fd[1] on source side to do > > the migration rather than fd[0]? Please feel free to correct me.. > > > > fd[0] - read channel so we should use it for target (-incoming fd:fd[0]) > fd[1] - write channel so we shouls use it for source to transfer/write > and for it we need to assign using `getfd` monitor command on source. Yeah again the logic should be fine, though we might need to implement the SCM_RIGHTS magic here, like what we did in, e.g., qio_channel_socket_writev() in QEMU. Regards,
On Fri, Jul 06, 2018 at 12:37:40PM +0800, Peter Xu wrote: > On Thu, Jul 05, 2018 at 06:22:31PM +0530, Balamuruhan S wrote: > > On Thu, Jul 05, 2018 at 05:26:09PM +0800, Peter Xu wrote: > > > On Thu, Jul 05, 2018 at 01:30:17PM +0530, Balamuruhan S wrote: > > > > This patch adds test for multifd migration feature with fd protocol. > > > > > > > > Signed-off-by: Balamuruhan S <bala24@linux.vnet.ibm.com> > > > > --- > > > > tests/migration-test.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 57 insertions(+) > > > > > > > > diff --git a/tests/migration-test.c b/tests/migration-test.c > > > > index e2434e70ea..29ede3810d 100644 > > > > --- a/tests/migration-test.c > > > > +++ b/tests/migration-test.c > > > > @@ -10,6 +10,7 @@ > > > > * > > > > */ > > > > > > > > +#include <unistd.h> > > > > #include "qemu/osdep.h" > > > > > > > > #include "libqtest.h" > > > > @@ -202,6 +203,17 @@ static void read_blocktime(QTestState *who) > > > > qobject_unref(rsp); > > > > } > > > > > > > > +static void getfd(QTestState *who, const char *fdname, int fd) > > > > +{ > > > > + QDict *rsp; > > > > + gchar *cmd = g_strdup_printf("{ 'execute': 'getfd'," > > > > + "'arguments': { '%s': '%d'} }", fdname, fd); > > > > + rsp = wait_command(who, cmd); > > > > > > AFAIU Libvirt passes fds to QEMU via the QMP channel using SCM_RIGHTS. > > > Here we directly specified the fd number. Could you help explain a bit > > > on how it worked? > > > > Thank you Peter for reviewing the patch. > > > > so I explored on pipe() system call, that could create channel for inter > > process communication. It gives us 2 fds one for read (I thought > > we can use it for target qemu process) and one for write (we can use it > > for source qemu process). I tried the example snippet from man page so > > I thought we can use it in our migration Qtest. > > > > Please correct my work if it doesn't make sense. > > Yeah the pipe() usage should be fine - though it's the way to pass it > to QEMU that I'm not sure about. > > Ok I gave it a shot with your tests on my host and I got this > actually: > > $ QTEST_QEMU_BINARY=./x86_64-softmmu/qemu-system-x86_64 ./tests/migration-test -p /x86_64/migration/multifd/fd > /x86_64/migration/multifd/fd: ** > ERROR:/home/peterx/git/qemu/tests/migration-test.c:212:getfd: assertion failed: (qdict_haskey(rsp, "return")) > Aborted (core dumped) > $ QTEST_QEMU_BINARY=./x86_64-softmmu/qemu-system-x86_64 QTEST_LOG=1 ./tests/migration-test -p /x86_64/migration/multifd/fd > /x86_64/migration/multifd/fd: [I 1530851360.669526] OPENED > [R +0.037850] endianness > [S +0.037870] OK little > {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2}, "package": "v2.12.0-2164-g20aa87a802"}, "capabilities": []}}{"execute": "qmp_capabilities"} > > {"return": {}}[I 1530851360.754972] OPENED > [R +0.055719] endianness > [S +0.055737] OK little > {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2}, "package": "v2.12.0-2164-g20aa87a802"}, "capabilities": []}}{"execute": "qmp_capabilities"} > > {"return": {}}{"execute": "getfd", "arguments": {"srcfd": "5"}} > > {"error": {"class": "GenericError", "desc": "Parameter 'fdname' is missing"}}** > ERROR:/home/peterx/git/qemu/tests/migration-test.c:212:getfd: assertion failed: (qdict_haskey(rsp, "return")) > [I +0.058378] CLOSED > [I +0.156930] CLOSED > Aborted (core dumped) Okay, I think this is what I am facing when I run it in tp-qemu which I mentioned in cover letter, I will work on it to make it work in right way. > > So it seems that the "getfd" command will only take one parameter > "fdname", then the fd should be passed in by UNIX socket magic. That > should match with what I understand it. I'm not sure why it worked on > your host. fine, I will use the getfd correctly as per your suggestion, I couldn't hit error, it was silent false pass. > > > > > > > > > > + g_assert(qdict_haskey(rsp, "return")); > > > > + g_free(cmd); > > > > + qobject_unref(rsp); > > > > +} > > > > + > > > > static void wait_for_migration_complete(QTestState *who) > > > > { > > > > while (true) { > > > > @@ -619,6 +631,50 @@ static void test_precopy_unix(void) > > > > g_free(uri); > > > > } > > > > > > > > +static void test_multifd_fd(void) > > > > +{ > > > > + int fd[2]; > > > > + > > > > + /* create pipe */ > > > > + if (pipe(fd) == -1) > > > > + g_test_message("Skipping test: Unable to create pipe"); > > > > + return; > > > > + > > > > + /* set uri in target with read fd */ > > > > + char *uri = g_strdup_printf("fd:%d", fd[0]); > > > > + QTestState *from, *to; > > > > + test_migrate_start(&from, &to, uri, false); > > > > + > > > > + /* Receive a write file descriptor for source using getfd */ > > > > + getfd(from, "srcfd", fd[1]); > > > > + > > > > + /* set multifd capability on source and target */ > > > > + migrate_set_capability(from, "x-multifd", "true"); > > > > + migrate_set_capability(to, "x-multifd", "true"); > > > > + > > > > + /* Wait for the first serial output from the source */ > > > > + wait_for_serial("src_serial"); > > > > + > > > > + /* perform migration */ > > > > + migrate(from, uri); > > > > > > I'm not sure I understand correctly here, but I feel like we should > > > use the fd[1] or anything that was bound to fd[1] on source side to do > > > the migration rather than fd[0]? Please feel free to correct me.. > > > > > > > fd[0] - read channel so we should use it for target (-incoming fd:fd[0]) > > fd[1] - write channel so we shouls use it for source to transfer/write > > and for it we need to assign using `getfd` monitor command on source. > > Yeah again the logic should be fine, though we might need to implement > the SCM_RIGHTS magic here, like what we did in, e.g., > qio_channel_socket_writev() in QEMU. :+1: sure, I will work on it, Thank you Peter for your time and help. -- Bala > > Regards, > > -- > Peter Xu >
diff --git a/tests/migration-test.c b/tests/migration-test.c index e2434e70ea..29ede3810d 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -10,6 +10,7 @@ * */ +#include <unistd.h> #include "qemu/osdep.h" #include "libqtest.h" @@ -202,6 +203,17 @@ static void read_blocktime(QTestState *who) qobject_unref(rsp); } +static void getfd(QTestState *who, const char *fdname, int fd) +{ + QDict *rsp; + gchar *cmd = g_strdup_printf("{ 'execute': 'getfd'," + "'arguments': { '%s': '%d'} }", fdname, fd); + rsp = wait_command(who, cmd); + g_assert(qdict_haskey(rsp, "return")); + g_free(cmd); + qobject_unref(rsp); +} + static void wait_for_migration_complete(QTestState *who) { while (true) { @@ -619,6 +631,50 @@ static void test_precopy_unix(void) g_free(uri); } +static void test_multifd_fd(void) +{ + int fd[2]; + + /* create pipe */ + if (pipe(fd) == -1) + g_test_message("Skipping test: Unable to create pipe"); + return; + + /* set uri in target with read fd */ + char *uri = g_strdup_printf("fd:%d", fd[0]); + QTestState *from, *to; + test_migrate_start(&from, &to, uri, false); + + /* Receive a write file descriptor for source using getfd */ + getfd(from, "srcfd", fd[1]); + + /* set multifd capability on source and target */ + migrate_set_capability(from, "x-multifd", "true"); + migrate_set_capability(to, "x-multifd", "true"); + + /* Wait for the first serial output from the source */ + wait_for_serial("src_serial"); + + /* perform migration */ + migrate(from, uri); + + wait_for_migration_pass(from); + + if (!got_stop) { + qtest_qmp_eventwait(from, "STOP"); + } + qtest_qmp_eventwait(to, "RESUME"); + + wait_for_serial("dest_serial"); + wait_for_migration_complete(from); + test_migrate_end(from, to, true); + g_free(uri); + + /* close the fds */ + close(fd[0]); + close(fd[1]); +} + static void test_multifd_unix(void) { char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); @@ -673,6 +729,7 @@ int main(int argc, char **argv) qtest_add_func("/migration/bad_dest", test_baddest); qtest_add_func("/migration/precopy/unix", test_precopy_unix); qtest_add_func("/migration/multifd/unix", test_multifd_unix); + qtest_add_func("/migration/multifd/fd", test_multifd_fd); ret = g_test_run();
This patch adds test for multifd migration feature with fd protocol. Signed-off-by: Balamuruhan S <bala24@linux.vnet.ibm.com> --- tests/migration-test.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)