Re: [Qemu-devel] [PATCH] block/rbd: Remove unused local variable

[Stefan Weil]

Am 23.05.2011 12:26, schrieb Kevin Wolf:
> Am 23.05.2011 11:01, schrieb Christian Brunner:
> > 2011/5/22 Stefan Weil <address@hidden>:
> > > Am 07.05.2011 22:15, schrieb Stefan Weil:
> > > > cppcheck report:
> > > > rbd.c:246: style: Variable 'snap' is assigned a value that is never 
> > > > used
> > > >
> > > > Remove snap and the related code.
> > > >
> > > > Cc: Christian Brunner<address@hidden>
> > > > Cc: Kevin Wolf<address@hidden>
> > > > Signed-off-by: Stefan Weil<address@hidden>
> > > > ---
> > > > block/rbd.c | 4 ----
> > > > 1 files changed, 0 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/block/rbd.c b/block/rbd.c
> > > > index 249a590..5c7d44e 100644
> > > > --- a/block/rbd.c
> > > > +++ b/block/rbd.c
> > > > @@ -524,7 +524,6 @@ static int rbd_open(BlockDriverState *bs, const 
> > > > char
> > > > *filename, int flags)
> > > > RbdHeader1 *header;
> > > > char pool[RBD_MAX_SEG_NAME_SIZE];
> > > > char snap_buf[RBD_MAX_SEG_NAME_SIZE];
> > > > - char *snap = NULL;
> > > > char *hbuf = NULL;
> > > > int r;
> > > >
> > > > @@ -533,9 +532,6 @@ static int rbd_open(BlockDriverState *bs, const 
> > > > char
> > > > *filename, int flags)
> > > > s->name, sizeof(s->name))< 0) {
> > > > return -EINVAL;
> > > > }
> > > > - if (snap_buf[0] != '\0') {
> > > > - snap = snap_buf;
> > > > - }
> > > >
> > > > if ((r = rados_initialize(0, NULL))< 0) {
> > > > error_report("error initializing");
> > >
> > > What about this patch? Can it be applied to the block branch?
> > >
> > > Regards,
> > > Stefan W.
> >
> > No objections on my side. You can add:
> >
> > Reviewed-by: Christian Brunner <address@hidden>
> >
> > The questions is how we continue with the rbd driver. Recent ceph
> > versions had some changes in librados that are incompatible with the
> > current driver. We have to options now:
> >
> > 1. Change the function calls for new librados versions (I could
> > provide a patch for this).
> > 2. Use librbd (see Josh's patches).
> >
> > Using librbd simplifies the qemu driver a lot and gives us consistency
> > with the kernel driver. - I would prefer this. (Please note that there
> > is a race condition in the current librbd versions, that crashes qemu
> > under high i/o load, but I'm fairly confident, that Josh will have
> > sorted this out by the time 0.15 is released).
>
> The problem with Josh's patches (or basically anything related to the
> rbd driver) is that it hasn't received any review. I'm not familiar with
> librados and librbd, so reviewing rbd is even harder than other patches
> of the same size for me. Additionally, it's not a test environment that
> I have set up.
>
> So going forward with it, I think we need a separate rbd maintainer. So
> Christian, I think it would be helpful if you at least reviewed any rbd
> patch and either comment on it or send an Acked-by, which basically
> tells me to commit it without any further checks. Or maybe we should
> consider that you send pull requests yourself if the patches touch only
> rbd code.
>
> Kevin

This patch was reviewed by Christian, but is still in my queue of open 
patches.
Kevin, could you please take it into the block queue?

Thanks,
Stefan W.