On Mon, 23 May 2011 20:57:52 +0200, Christian Brunner <chb@xxxxxx>
wrote:
> Hi Josh,
>
> I don't know if you followed this thread on the qemu list. I would
> sugest, that you resend the current patchset. I will review it and
> send an ack afterwards.
>
> Please feel free to add your name to the header and the copyright.
>
> Regards,
> Christian
Hi Christian,
Thanks for pointing this out. I'll resend later today.
Josh
> ---------- Forwarded message ----------
> From: Kevin Wolf <kwolf@xxxxxxxxxx>
> Date: 2011/5/23
> Subject: Re: [Qemu-devel] [PATCH] block/rbd: Remove unused local variable
> To: chb@xxxxxx
> Cc: QEMU Developers <qemu-devel@xxxxxxxxxx>
>
>
> Am 23.05.2011 11:01, schrieb Christian Brunner:
>> 2011/5/22 Stefan Weil <weil@xxxxxxxxxxxxxxx>:
>>> 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@xxxxxx>
>>>> Cc: Kevin Wolf<kwolf@xxxxxxxxxx>
>>>> Signed-off-by: Stefan Weil<weil@xxxxxxxxxxxxxxx>
>>>> ---
>>>> Â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@xxxxxx>
>>
>> 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
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
