Re: [Qemu-devel] [PATCH v2] sheepdog: add reopen support

[Liu Yuan <namei.unix@gmail.com>]

On Thu, Aug 27, 2015 at 03:00:32PM -0400, Jeff Cody wrote:
> On Thu, Aug 27, 2015 at 10:54:01AM +0800, Liu Yuan wrote:
> > From: Liu Yuan <liuyuan@cmss.chinamobile.com>
> > 
> > With reopen supported, block-commit (and offline commit) is now supported for
> > image files whose base image uses the Sheepdog protocol driver.
> > 
> > Cc: qemu-devel@nongnu.org
> > Cc: Jeff Cody <jcody@redhat.com>
> > Cc: Kevin Wolf <kwolf@redhat.com>
> > Cc: Stefan Hajnoczi <stefanha@redhat.com>
> > Signed-off-by: Liu Yuan <liuyuan@cmss.chinamobile.com>
> > ---
> > v2:
> >  - free AioHandler [Jeff Cody]
> > 
> >  block/sheepdog.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 75 insertions(+)
> > 
> > diff --git a/block/sheepdog.c b/block/sheepdog.c
> > index 9585beb..e54add4 100644
> > --- a/block/sheepdog.c
> > +++ b/block/sheepdog.c
> > @@ -377,6 +377,11 @@ typedef struct BDRVSheepdogState {
> >      QLIST_HEAD(inflight_aiocb_head, SheepdogAIOCB) inflight_aiocb_head;
> >  } BDRVSheepdogState;
> >  
> > +typedef struct BDRVSheepdogReopenState {
> > +    int fd;
> > +    int cache_flags;
> > +} BDRVSheepdogReopenState;
> > +
> >  static const char * sd_strerror(int err)
> >  {
> >      int i;
> > @@ -1486,6 +1491,67 @@ out:
> >      return ret;
> >  }
> >  
> > +static int sd_reopen_prepare(BDRVReopenState *state, BlockReopenQueue *queue,
> > +                             Error **errp)
> > +{
> > +    BDRVSheepdogState *s = state->bs->opaque;
> > +    BDRVSheepdogReopenState *re_s;
> > +    int ret = 0;
> > +
> > +    re_s = state->opaque = g_new0(BDRVSheepdogReopenState, 1);
> > +
> > +    if (state->flags & BDRV_O_NOCACHE) {
> > +        re_s->cache_flags = SD_FLAG_CMD_DIRECT;
> > +    }
> 
> In sd_open(), the s->cache_flag is set to SD_FLAG_CMD_CACHE by
> default, and then overwritten if BDRV_O_NOCACHE is set.  Do we want
> that same behavior here?
> 
> (Sorry I didn't ask this the first time around)

As far as I understood, sd_open() was called before sd_reopen, so I thought
s->cache_flags = SD_FLAG_CMD_CACHE was duplicated in sd_reopen. But for a second
thought, if the reopen indicates a complete flags reparsing, I'd agree with you.

I'll prepare the third version of the patch.

Thanks,
Yuan