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

[Christian Brunner <chb@muc.de>]

2011/5/27 Stefan Weil <weil@mail.berlios.de>:
> Am 23.05.2011 12:26, schrieb Kevin Wolf:
>>
>> Am 23.05.2011 11:01, schrieb Christian Brunner:
>>>
>>> 2011/5/22 Stefan Weil <weil@mail.berlios.de>:
>>>>
>>>> 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<chb@muc.de>
>>>>> Cc: Kevin Wolf<kwolf@redhat.com>
>>>>> Signed-off-by: Stefan Weil<weil@mail.berlios.de>
>>>>> ---
>>>>> 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 <chb@muc.de>
>>>
>>> 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.

Kevin chose to merge josh's patches. This includes your patch.

Christian