[sheepdog] [PATCH 1/2] sheepdog: adopting protocol update for VDI locking

[Hitoshi Mitake]

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 <kwolf at redhat.com>
> > > > Cc: Stefan Hajnoczi <stefanha at redhat.com>
> > > > Cc: Liu Yuan <namei.unix at gmail.com>
> > > > Cc: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
> > > > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > > > ---
> > > >  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

Even we set the default lock type as 0, the old QEMU cannot issue a
correct locking request. 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).

Thanks,
Hitoshi