From mboxrd@z Thu Jan  1 00:00:00 1970
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Subject: Re: [RFC Patch v3 16/22] blktap2: move ramdisk related
	codes to block-replication.c
Date: Wed, 24 Sep 2014 11:44:33 -0700
Message-ID: <CAP8mzPOSkkzpYNWUJPb=0ATbC6BBgDb2s_8E24bg4O9GiuGopA@mail.gmail.com>
References: <1409909158-19243-1-git-send-email-wency@cn.fujitsu.com>
	<1409909158-19243-17-git-send-email-wency@cn.fujitsu.com>
Reply-To: rshriram@cs.ubc.ca
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="===============2624405282218255835=="
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1409909158-19243-17-git-send-email-wency@cn.fujitsu.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Wen Congyang <wency@cn.fujitsu.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>, Ian Jackson <Ian.Jackson@eu.citrix.com>, Jiang Yunhong <yunhong.jiang@intel.com>, Dong Eddie <eddie.dong@intel.com>, xen devel <xen-devel@lists.xen.org>, Yang Hongyang <yanghy@cn.fujitsu.com>, Ian Campbell <ian.campbell@citrix.com>
List-Id: xen-devel@lists.xenproject.org

--===============2624405282218255835==
Content-Type: multipart/alternative; boundary=047d7bdc0dd2349a420503d413f0

--047d7bdc0dd2349a420503d413f0
Content-Type: text/plain; charset=UTF-8

On Sep 5, 2014 5:32 AM, "Wen Congyang" <wency@cn.fujitsu.com> wrote:
>
>  COLO will reuse them
>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Cc: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> ---
>  tools/blktap2/drivers/block-remus.c       | 485
++----------------------------
>  tools/blktap2/drivers/block-replication.c | 452
++++++++++++++++++++++++++++
>  tools/blktap2/drivers/block-replication.h |  48 +++
>  3 files changed, 523 insertions(+), 462 deletions(-)
>
> diff --git a/tools/blktap2/drivers/block-remus.c
b/tools/blktap2/drivers/block-remus.c
> index 8b6f157..2713af1 100644
> --- a/tools/blktap2/drivers/block-remus.c
> +++ b/tools/blktap2/drivers/block-remus.c
...snip..
>
>  static int backup_start(td_driver_t *driver)
> @@ -1120,7 +686,9 @@ static void server_do_wreq(td_driver_t *driver)
>         if (mread(s->stream_fd.fd, buf, len) < 0)
>                 goto err;
>
> -       if (ramdisk_write(&s->ramdisk, *sector, *sectors, buf) < 0) {
> +       if (ramdisk_write_to_hashtable(s->h, *sector, *sectors,

Please rename to something else.
Eg, ramdisk flush to disk?

I am fine with the rest of the patch. Have you tested this setup with Remus
(not Colo) and ensured that you haven't reintroduced the write after write
issue, that was fixed a couple of years ago? Although I don't know how you
would test with 4.5 code base.

Acked-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
(subject to addressing the renaming comment)

--047d7bdc0dd2349a420503d413f0
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

<p dir=3D"ltr">On Sep 5, 2014 5:32 AM, &quot;Wen Congyang&quot; &lt;<a href=
=3D"mailto:wency@cn.fujitsu.com">wency@cn.fujitsu.com</a>&gt; wrote:<br>
&gt;<br>
&gt; =C2=A0COLO will reuse them<br>
&gt;<br>
&gt; Signed-off-by: Wen Congyang &lt;<a href=3D"mailto:wency@cn.fujitsu.com=
">wency@cn.fujitsu.com</a>&gt;<br>
&gt; Cc: Shriram Rajagopalan &lt;<a href=3D"mailto:rshriram@cs.ubc.ca">rshr=
iram@cs.ubc.ca</a>&gt;<br>
&gt; ---<br>
&gt; =C2=A0tools/blktap2/drivers/block-remus.c=C2=A0 =C2=A0 =C2=A0 =C2=A0| =
485 ++----------------------------<br>
&gt; =C2=A0tools/blktap2/drivers/block-replication.c | 452 ++++++++++++++++=
++++++++++++<br>
&gt; =C2=A0tools/blktap2/drivers/block-replication.h |=C2=A0 48 +++<br>
&gt; =C2=A03 files changed, 523 insertions(+), 462 deletions(-)<br>
&gt;<br>
&gt; diff --git a/tools/blktap2/drivers/block-remus.c b/tools/blktap2/drive=
rs/block-remus.c<br>
&gt; index 8b6f157..2713af1 100644<br>
&gt; --- a/tools/blktap2/drivers/block-remus.c<br>
&gt; +++ b/tools/blktap2/drivers/block-remus.c<br>
...snip..<br>
&gt;<br>
&gt; =C2=A0static int backup_start(td_driver_t *driver)<br>
&gt; @@ -1120,7 +686,9 @@ static void server_do_wreq(td_driver_t *driver)<b=
r>
&gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (mread(s-&gt;stream_fd.fd, buf, len) &l=
t; 0)<br>
&gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto err;<br>
&gt;<br>
&gt; -=C2=A0 =C2=A0 =C2=A0 =C2=A0if (ramdisk_write(&amp;s-&gt;ramdisk, *sec=
tor, *sectors, buf) &lt; 0) {<br>
&gt; +=C2=A0 =C2=A0 =C2=A0 =C2=A0if (ramdisk_write_to_hashtable(s-&gt;h, *s=
ector, *sectors,</p>
<p dir=3D"ltr">Please rename to something else.<br>
Eg, ramdisk flush to disk?</p>
<p dir=3D"ltr">I am fine with the rest of the patch. Have you tested this s=
etup with Remus (not Colo) and ensured that you haven&#39;t reintroduced th=
e write after write issue, that was fixed a couple of years ago? Although I=
 don&#39;t know how you would test with 4.5 code base.</p>
<p dir=3D"ltr">Acked-by: Shriram Rajagopalan &lt;<a href=3D"mailto:rshriram=
@cs.ubc.ca">rshriram@cs.ubc.ca</a>&gt;<br>
 (subject to addressing the renaming comment)</p>

--047d7bdc0dd2349a420503d413f0--


--===============2624405282218255835==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

--===============2624405282218255835==--