From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:34464) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QQ3vx-0007xp-Sv for qemu-devel@nongnu.org; Fri, 27 May 2011 16:47:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QQ3vw-0007Gh-4M for qemu-devel@nongnu.org; Fri, 27 May 2011 16:47:01 -0400 Received: from mail-vx0-f173.google.com ([209.85.220.173]:33169) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QQ3vw-0007Gd-23 for qemu-devel@nongnu.org; Fri, 27 May 2011 16:47:00 -0400 Received: by vxb37 with SMTP id 37so1849232vxb.4 for ; Fri, 27 May 2011 13:46:59 -0700 (PDT) MIME-Version: 1.0 Sender: c.m.brunner@gmail.com In-Reply-To: <4DDFEE25.60502@mail.berlios.de> References: <1304799357-19281-1-git-send-email-weil@mail.berlios.de> <4DD8FC9D.3090200@mail.berlios.de> <4DDA3646.404@redhat.com> <4DDFEE25.60502@mail.berlios.de> Date: Fri, 27 May 2011 22:46:58 +0200 Message-ID: From: Christian Brunner Content-Type: text/plain; charset=ISO-8859-1 Subject: Re: [Qemu-devel] [PATCH] block/rbd: Remove unused local variable Reply-To: chb@muc.de List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: Kevin Wolf , QEMU Developers 2011/5/27 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 : >>>> >>>> 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 >>>>> Cc: Kevin Wolf >>>>> Signed-off-by: Stefan Weil >>>>> --- >>>>> 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 >>> >>> 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