From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58072) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XGgWM-0001ba-K6 for qemu-devel@nongnu.org; Sun, 10 Aug 2014 23:43:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XGgWH-0007qn-I3 for qemu-devel@nongnu.org; Sun, 10 Aug 2014 23:43:42 -0400 Received: from mail-pd0-x22e.google.com ([2607:f8b0:400e:c02::22e]:44251) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XGgWH-0007qg-63 for qemu-devel@nongnu.org; Sun, 10 Aug 2014 23:43:37 -0400 Received: by mail-pd0-f174.google.com with SMTP id fp1so10096749pdb.33 for ; Sun, 10 Aug 2014 20:43:35 -0700 (PDT) Date: Mon, 11 Aug 2014 11:43:43 +0800 From: Liu Yuan Message-ID: <20140811034343.GP12057@ubuntu-trusty> References: <1407396520-2720-1-git-send-email-mitake.hitoshi@lab.ntt.co.jp> <1407396520-2720-2-git-send-email-mitake.hitoshi@lab.ntt.co.jp> <20140808052038.GD12057@ubuntu-trusty> <87vbq3zf0e.wl%mitake.hitoshi@lab.ntt.co.jp> <20140808074937.GI12057@ubuntu-trusty> <87y4uvrcqq.wl%mitake.hitoshi@lab.ntt.co.jp> <20140811033456.GO12057@ubuntu-trusty> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140811033456.GO12057@ubuntu-trusty> Subject: Re: [Qemu-devel] [PATCH 1/2] sheepdog: adopting protocol update for VDI locking List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hitoshi Mitake Cc: Kevin Wolf , sheepdog@lists.wpkg.org, mitake.hitoshi@gmail.com, MORITA Kazutaka , qemu-devel@nongnu.org, Stefan Hajnoczi On Mon, Aug 11, 2014 at 11:34:56AM +0800, Liu Yuan wrote: > On Mon, Aug 11, 2014 at 11:17:33AM +0900, Hitoshi Mitake wrote: > > At Fri, 8 Aug 2014 15:49:37 +0800, > > Liu Yuan wrote: > > > > > > On Fri, Aug 08, 2014 at 03:12:17PM +0900, Hitoshi Mitake wrote: > > > > At Fri, 8 Aug 2014 13:20:39 +0800, > > > > Liu Yuan wrote: > > > > > > > > > > On Thu, Aug 07, 2014 at 04:28:39PM +0900, Hitoshi Mitake wrote: > > > > > > The update is required for supporting iSCSI multipath. It doesn't > > > > > > affect behavior of QEMU driver but adding a new field to vdi request > > > > > > struct is required. > > > > > > > > > > > > Cc: Kevin Wolf > > > > > > Cc: Stefan Hajnoczi > > > > > > Cc: Liu Yuan > > > > > > Cc: MORITA Kazutaka > > > > > > Signed-off-by: Hitoshi Mitake > > > > > > --- > > > > > > block/sheepdog.c | 8 +++++++- > > > > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/block/sheepdog.c b/block/sheepdog.c > > > > > > index 8d9350c..36f76f0 100644 > > > > > > --- a/block/sheepdog.c > > > > > > +++ b/block/sheepdog.c > > > > > > @@ -103,6 +103,9 @@ > > > > > > #define SD_INODE_SIZE (sizeof(SheepdogInode)) > > > > > > #define CURRENT_VDI_ID 0 > > > > > > > > > > > > +#define LOCK_TYPE_NORMAL 1 > > > > > > +#define LOCK_TYPE_SHARED 2 /* for iSCSI multipath */ > > > > > > > > > > How about > > > > > > > > > > #define LOCK_TYPE_NORMAL 0 > > > > > #define LOCK_TYPE_SHARED 1 > > > > > > > > > > Then we don't need this patch. Since qemu won't make use of multipath for the > > > > > near future, we should avoid adding stuff related to multipath to qemu driver. > > > > > > > > (Cc-ing current Kazutaka-san's address) > > > > > > > > I think this isn't a good idea. Because it means that sheep has an > > > > assumption about padding field of the request data struct. This sort > > > > of workaround can cause hard to find problems in the future. > > > > > > > > > > The point is, how to keep backward compatibilty? E.g, old QEMU with present > > > sheep daemon with lock features. Then QEMU will send 0 instead of 1 to the sheep > > > daemon and based on how you handle the invalid value, these might cause problems. > > > > > > Suppose we have 1 old QEMU and 1 present QEMU who try to start the same image A. > > > Old QEMU will send invalid 0 to sheep daemon. We shouldn't deny it because it > > > can run with old sheep and should run on new sheep too. Then this image A isn't > > > locked due to invalid 0 value. Then present QEMU send correct LOCK signal and > > > will wrongly set up the image. > > > > > > Start with 0 instead of 1, in my option, is reasonable to keep backward > > > compatibility. > > > > I don't think so. Because the backward compatibility of the locking > > functionality is already broken in the far past. > > > > As Meng repoted in the sheepdog mailing list, old QEMU can issue > > locking request to sheep with vid == 0: > > http://lists.wpkg.org/pipermail/sheepdog/2014-August/015438.html > > I don't really understand why we can't start with 0 and can't keep backward > compatibility. By the way, I think the link has nothing to do with qemu, it is > a bug in sheep. > > locking has two state, one is lock and the other unlock. > > We choose 0 to mean 'lock' the vdi and 1 to 'unlock' the vdi. > > So both old and new QEMU issue 0 to lock the image and 'release' request to > unlock the image. What is in the way? > > > > > Even we set the default lock type as 0, the old QEMU cannot issue a > > correct locking request. > > why? > > > I'll post a patch for incrementing protocol > > version number later. But before doing that, I also want to clean > > DISCARD request. Because this request cannot work with snapshot (not > > only with the new GC algorithm. The old naive algorithm cannot work > > with the DISCARD request). > > If you remove discard, what if users run new qemu with old sheep, which make > use of old algorithm and people expect discard to work? Okay, you mean discard can't work with snapshots, but it should work with non-snapshots, so for the users of non-snapshots, people can expect it to work. As stated in my last email, you can handle this problem transparently without modification of protocol. QEMU --> discard[offset, lenght] --> sheep sheep: if req on snapshot return success; else return sheep_handle_discard(). Thanks Yuan