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

[Hitoshi Mitake]

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.

Thanks,
Hitoshi

> 
> Thanks
> Yuan
> 
> > +
> >  typedef struct SheepdogReq {
> >      uint8_t proto_ver;
> >      uint8_t opcode;
> > @@ -166,7 +169,8 @@ typedef struct SheepdogVdiReq {
> >      uint8_t copy_policy;
> >      uint8_t reserved[2];
> >      uint32_t snapid;
> > -    uint32_t pad[3];
> > +    uint32_t type;
> > +    uint32_t pad[2];
> >  } SheepdogVdiReq;
> >  
> >  typedef struct SheepdogVdiRsp {
> > @@ -1090,6 +1094,7 @@ static int find_vdi_name(BDRVSheepdogState *s, const char *filename,
> >      memset(&hdr, 0, sizeof(hdr));
> >      if (lock) {
> >          hdr.opcode = SD_OP_LOCK_VDI;
> > +        hdr.type = LOCK_TYPE_NORMAL;
> >      } else {
> >          hdr.opcode = SD_OP_GET_VDI_INFO;
> >      }
> > @@ -1793,6 +1798,7 @@ static void sd_close(BlockDriverState *bs)
> >      memset(&hdr, 0, sizeof(hdr));
> >  
> >      hdr.opcode = SD_OP_RELEASE_VDI;
> > +    hdr.type = LOCK_TYPE_NORMAL;
> >      hdr.base_vdi_id = s->inode.vdi_id;
> >      wlen = strlen(s->name) + 1;
> >      hdr.data_length = wlen;
> > -- 
> > 1.8.3.2
> >