[{"id":1758621,"web_url":"http://patchwork.ozlabs.org/comment/1758621/","msgid":"","list_archive_url":null,"date":"2017-08-28T14:51:43","subject":"Re: [Qemu-devel] [PATCH] qemu-iotests: Extend non-shared storage\n\tmigration test (194)","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 08/28/2017 06:29 AM, Kashyap Chamarthy wrote:\n> This is the follow-up patch that was discussed[*] as part of feedback to\n> qemu-iotest 194.\n> \n> Changes in this patch:\n> \n> - Supply 'job-id' parameter to `drive-mirror` invocation.\n> \n> - Issue `block-job-cancel` command on the source QEMU to gracefully\n> complete the mirroring operation.\n> \n> - Stop the NBD server on the destination QEMU.\n> \n> - Finally, exit once the event BLOCK_JOB_COMPLETED is emitted.\n> \n> With the above, the test will also be (almost) in sync with the\n> procedure outlined in the document live-block-operations.rst[+]\n> (section: \"QMP invocation for live storage migration with\n> ``drive-mirror`` + NBD\").\n> \n> [*] https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg04820.html\n> -- qemu-iotests: add 194 non-shared storage migration test\n> [+] https://git.qemu.org/gitweb.cgi?p=qemu.git;a=blob;f=docs/interop/live-block-operations.rst\n> \n> Signed-off-by: Kashyap Chamarthy \n> ---\n> I wonder:\n> - Is it worth printing the MIGRATION event state change?\n\nI think waiting for both the BLOCK_JOB_COMPLETED and MIGRATION events\nmake sense (in other words, let's check both events in the expected\norder, rather than just one or the other).\n\n> - Since we're not checking on the MIGRATION event anymore, can\n> the migration state change events related code (that is triggerred\n> by setting 'migrate-set-capabilities') be simply removed?\n\nIf we're going to mirror libvirt's non-shared storage migration\nsequence, I think we want to keep everything, rather than drop the\nmigration half.\n\n> ---\n> tests/qemu-iotests/194 | 17 ++++++++++++-----\n> tests/qemu-iotests/194.out | 14 ++++++++------\n> 2 files changed, 20 insertions(+), 11 deletions(-)\n> \n> diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194\n> index 8028111e21bed5cf4a2e8e32dc04aa5a9ea9caca..8d746be9d0033f478f11886ee93f95b0fa55bab0 100755\n> --- a/tests/qemu-iotests/194\n> +++ b/tests/qemu-iotests/194\n> @@ -46,16 +46,17 @@ iotests.log('Launching NBD server on destination...')\n> iotests.log(dest_vm.qmp('nbd-server-start', addr={'type': 'unix', 'data': {'path': nbd_sock_path}}))\n> iotests.log(dest_vm.qmp('nbd-server-add', device='drive0', writable=True))\n> \n> -iotests.log('Starting drive-mirror on source...')\n> +iotests.log('Starting `drive-mirror` on source...')\n> iotests.log(source_vm.qmp(\n> 'drive-mirror',\n> device='drive0',\n> target='nbd+unix:///drive0?socket={0}'.format(nbd_sock_path),\n> sync='full',\n> format='raw', # always raw, the server handles the format\n> - mode='existing'))\n> + mode='existing',\n> + job_id='mirror-job0'))\n> \n> -iotests.log('Waiting for drive-mirror to complete...')\n> +iotests.log('Waiting for `drive-mirror` to complete...')\n\nSo, up to here is okay,\n\n> iotests.log(source_vm.event_wait('BLOCK_JOB_READY'),\n> filters=[iotests.filter_qmp_event])\n> \n> @@ -66,8 +67,14 @@ dest_vm.qmp('migrate-set-capabilities',\n> capabilities=[{'capability': 'events', 'state': True}])\n> iotests.log(source_vm.qmp('migrate', uri='unix:{0}'.format(migration_sock_path)))\n> \n> +iotests.log('Gracefully ending the `drive-mirror` job on source...')\n> +iotests.log(source_vm.qmp('block-job-cancel', device='mirror-job0'))\n> +\n> +iotests.log('Stopping the NBD server on destination...')\n> +iotests.log(dest_vm.qmp('nbd-server-stop'))\n> +\n> while True:\n> - event = source_vm.event_wait('MIGRATION')\n> + event = source_vm.event_wait('BLOCK_JOB_COMPLETED')\n\nAnd this event makes sense for catching the block-job-cancel, but I\nthink you STILL want to keep a while loop for catching migration as well.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xgvsH5xTkz9s81\n\tfor ;\n\tTue, 29 Aug 2017 00:53:23 +1000 (AEST)","from localhost ([::1]:40154 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dmLPl-0003tK-NA\n\tfor incoming@patchwork.ozlabs.org; Mon, 28 Aug 2017 10:53:21 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:46805)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dmLOR-0003GP-AW\n\tfor qemu-devel@nongnu.org; Mon, 28 Aug 2017 10:52:01 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dmLOQ-0005XV-A3\n\tfor qemu-devel@nongnu.org; Mon, 28 Aug 2017 10:51:59 -0400","from mx1.redhat.com ([209.132.183.28]:52548)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dmLOL-0005Uw-9O; Mon, 28 Aug 2017 10:51:53 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 145357573E;\n\tMon, 28 Aug 2017 14:51:52 +0000 (UTC)","from [10.10.122.186] (ovpn-122-186.rdu2.redhat.com [10.10.122.186])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 0CC9F82A41;\n\tMon, 28 Aug 2017 14:51:44 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 145357573E","To":"Kashyap Chamarthy , qemu-devel@nongnu.org","References":"<20170828112952.22965-1-kchamart@redhat.com>","From":"Eric Blake ","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"","Date":"Mon, 28 Aug 2017 09:51:43 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170828112952.22965-1-kchamart@redhat.com>","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"sS1PJdlDWxTKdp9QgIo4rVSXp2VK145lb\"","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tMon, 28 Aug 2017 14:51:52 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [PATCH] qemu-iotests: Extend non-shared storage\n\tmigration test (194)","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, famz@redhat.com, stefanha@redhat.com,\n\tqemu-block@nongnu.org, dgilbert@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1758646,"web_url":"http://patchwork.ozlabs.org/comment/1758646/","msgid":"<20170828153525.d4wgjfcqfnpxavrt@eukaryote>","list_archive_url":null,"date":"2017-08-28T15:35:25","subject":"Re: [Qemu-devel] [PATCH] qemu-iotests: Extend non-shared storage\n\tmigration test (194)","submitter":{"id":65934,"url":"http://patchwork.ozlabs.org/api/people/65934/","name":"Kashyap Chamarthy","email":"kchamart@redhat.com"},"content":"On Mon, Aug 28, 2017 at 09:51:43AM -0500, Eric Blake wrote:\n> On 08/28/2017 06:29 AM, Kashyap Chamarthy wrote:\n> > This is the follow-up patch that was discussed[*] as part of feedback to\n> > qemu-iotest 194.\n> > \n> > Changes in this patch:\n> > \n> > - Supply 'job-id' parameter to `drive-mirror` invocation.\n> > \n> > - Issue `block-job-cancel` command on the source QEMU to gracefully\n> > complete the mirroring operation.\n> > \n> > - Stop the NBD server on the destination QEMU.\n> > \n> > - Finally, exit once the event BLOCK_JOB_COMPLETED is emitted.\n> > \n> > With the above, the test will also be (almost) in sync with the\n> > procedure outlined in the document live-block-operations.rst[+]\n> > (section: \"QMP invocation for live storage migration with\n> > ``drive-mirror`` + NBD\").\n> > \n> > [*] https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg04820.html\n> > -- qemu-iotests: add 194 non-shared storage migration test\n> > [+] https://git.qemu.org/gitweb.cgi?p=qemu.git;a=blob;f=docs/interop/live-block-operations.rst\n> > \n> > Signed-off-by: Kashyap Chamarthy \n> > ---\n> > I wonder:\n> > - Is it worth printing the MIGRATION event state change?\n> \n> I think waiting for both the BLOCK_JOB_COMPLETED and MIGRATION events\n> make sense (in other words, let's check both events in the expected\n> order, rather than just one or the other).\n\nThat sounds more robust, will do in the next iteration.\n\n> > - Since we're not checking on the MIGRATION event anymore, can\n> > the migration state change events related code (that is triggerred\n> > by setting 'migrate-set-capabilities') be simply removed?\n> \n> If we're going to mirror libvirt's non-shared storage migration\n> sequence, I think we want to keep everything, rather than drop the\n> migration half.\n\nYes, noted.\n\n[...]\n\n> > -iotests.log('Starting drive-mirror on source...')\n> > +iotests.log('Starting `drive-mirror` on source...')\n> > iotests.log(source_vm.qmp(\n> > 'drive-mirror',\n> > device='drive0',\n> > target='nbd+unix:///drive0?socket={0}'.format(nbd_sock_path),\n> > sync='full',\n> > format='raw', # always raw, the server handles the format\n> > - mode='existing'))\n> > + mode='existing',\n> > + job_id='mirror-job0'))\n> > \n> > -iotests.log('Waiting for drive-mirror to complete...')\n> > +iotests.log('Waiting for `drive-mirror` to complete...')\n> \n> So, up to here is okay,\n> \n> > iotests.log(source_vm.event_wait('BLOCK_JOB_READY'),\n> > filters=[iotests.filter_qmp_event])\n> > \n> > @@ -66,8 +67,14 @@ dest_vm.qmp('migrate-set-capabilities',\n> > capabilities=[{'capability': 'events', 'state': True}])\n> > iotests.log(source_vm.qmp('migrate', uri='unix:{0}'.format(migration_sock_path)))\n> > \n> > +iotests.log('Gracefully ending the `drive-mirror` job on source...')\n> > +iotests.log(source_vm.qmp('block-job-cancel', device='mirror-job0'))\n> > +\n> > +iotests.log('Stopping the NBD server on destination...')\n> > +iotests.log(dest_vm.qmp('nbd-server-stop'))\n> > +\n> > while True:\n> > - event = source_vm.event_wait('MIGRATION')\n> > + event = source_vm.event_wait('BLOCK_JOB_COMPLETED')\n> \n> And this event makes sense for catching the block-job-cancel, but I\n> think you STILL want to keep a while loop for catching migration as well.\n\nYes, will do.\n\nThanks for the quick feedback.\n\n[...]","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=kchamart@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xgwpk1k2Sz9sNn\n\tfor ;\n\tTue, 29 Aug 2017 01:36:14 +1000 (AEST)","from localhost ([::1]:40288 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dmM5E-0000Cd-5R\n\tfor incoming@patchwork.ozlabs.org; Mon, 28 Aug 2017 11:36:12 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:59179)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dmM4o-0000Ae-RD\n\tfor qemu-devel@nongnu.org; Mon, 28 Aug 2017 11:35:47 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dmM4n-0005xr-M0\n\tfor qemu-devel@nongnu.org; Mon, 28 Aug 2017 11:35:46 -0400","from mx1.redhat.com ([209.132.183.28]:45378)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dmM4g-0005u6-VQ; Mon, 28 Aug 2017 11:35:39 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id D121840431E;\n\tMon, 28 Aug 2017 15:35:37 +0000 (UTC)","from eukaryote.localdomain (ovpn-117-78.ams2.redhat.com\n\t[10.36.117.78])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 5CF4560C14;\n\tMon, 28 Aug 2017 15:35:27 +0000 (UTC)","by eukaryote.localdomain (Postfix, from userid 1000)\n\tid CAA7E240CEC; Mon, 28 Aug 2017 17:35:25 +0200 (CEST)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com D121840431E","Date":"Mon, 28 Aug 2017 17:35:25 +0200","From":"Kashyap Chamarthy ","To":"Eric Blake ","Message-ID":"<20170828153525.d4wgjfcqfnpxavrt@eukaryote>","References":"<20170828112952.22965-1-kchamart@redhat.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.6.0.1 (2016-04-01)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.38]);\n\tMon, 28 Aug 2017 15:35:37 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH] qemu-iotests: Extend non-shared storage\n\tmigration test (194)","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, famz@redhat.com, qemu-block@nongnu.org,\n\tqemu-devel@nongnu.org, dgilbert@redhat.com, stefanha@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]