From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Wolf Subject: Re: [PATCH] add support for protocol driver create_options Date: Tue, 25 May 2010 15:43:17 +0200 Message-ID: <4BFBD3F5.5020805@redhat.com> References: <1274333777-15415-1-git-send-email-morita.kazutaka@lab.ntt.co.jp> <4BF6BB80.1060504@redhat.com> <87bpc524gm.wl%morita.kazutaka@lab.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: sheepdog@lists.wpkg.org, hch@lst.de, kvm@vger.kernel.org, qemu-devel@nongnu.org To: MORITA Kazutaka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:37461 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752983Ab0EYNoA (ORCPT ); Tue, 25 May 2010 09:44:00 -0400 In-Reply-To: <87bpc524gm.wl%morita.kazutaka@lab.ntt.co.jp> Sender: kvm-owner@vger.kernel.org List-ID: Am 24.05.2010 08:34, schrieb MORITA Kazutaka: > At Fri, 21 May 2010 18:57:36 +0200, > Kevin Wolf wrote: >> >> Am 20.05.2010 07:36, schrieb MORITA Kazutaka: >>> + >>> +/* >>> + * Append an option list (list) to an option list (dest). >>> + * >>> + * If dest is NULL, a new copy of list is created. >>> + * >>> + * Returns a pointer to the first element of dest (or the newly allocated copy) >>> + */ >>> +QEMUOptionParameter *append_option_parameters(QEMUOptionParameter *dest, >>> + QEMUOptionParameter *list) >>> +{ >>> + size_t num_options, num_dest_options; >>> + >>> + num_options = count_option_parameters(dest); >>> + num_dest_options = num_options; >>> + >>> + num_options += count_option_parameters(list); >>> + >>> + dest = qemu_realloc(dest, (num_options + 1) * sizeof(QEMUOptionParameter)); >>> + >>> + while (list && list->name) { >>> + if (get_option_parameter(dest, list->name) == NULL) { >>> + dest[num_dest_options++] = *list; >> >> You need to add a dest[num_dest_options].name = NULL; here. Otherwise >> the next loop iteration works on uninitialized memory and possibly an >> unterminated list. I got a segfault for that reason. >> > > I forgot to add it, sorry. > Fixed version is below. > > Thanks, > > Kazutaka > > == > This patch enables protocol drivers to use their create options which > are not supported by the format. For example, protcol drivers can use > a backing_file option with raw format. > > Signed-off-by: MORITA Kazutaka $ ./qemu-img create -f qcow2 -o cluster_size=4k /tmp/test.qcow2 4G Unknown option 'cluster_size' qemu-img: Invalid options for file format 'qcow2'. I think you added another num_dest_options++ which shouldn't be there. Kevin From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=51889 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OGuUS-0005IC-2T for qemu-devel@nongnu.org; Tue, 25 May 2010 09:48:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OGuQK-0002Zc-Es for qemu-devel@nongnu.org; Tue, 25 May 2010 09:44:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:13179) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OGuQK-0002ZS-6t for qemu-devel@nongnu.org; Tue, 25 May 2010 09:44:00 -0400 Message-ID: <4BFBD3F5.5020805@redhat.com> Date: Tue, 25 May 2010 15:43:17 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1274333777-15415-1-git-send-email-morita.kazutaka@lab.ntt.co.jp> <4BF6BB80.1060504@redhat.com> <87bpc524gm.wl%morita.kazutaka@lab.ntt.co.jp> In-Reply-To: <87bpc524gm.wl%morita.kazutaka@lab.ntt.co.jp> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH] add support for protocol driver create_options List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: MORITA Kazutaka Cc: sheepdog@lists.wpkg.org, hch@lst.de, kvm@vger.kernel.org, qemu-devel@nongnu.org Am 24.05.2010 08:34, schrieb MORITA Kazutaka: > At Fri, 21 May 2010 18:57:36 +0200, > Kevin Wolf wrote: >> >> Am 20.05.2010 07:36, schrieb MORITA Kazutaka: >>> + >>> +/* >>> + * Append an option list (list) to an option list (dest). >>> + * >>> + * If dest is NULL, a new copy of list is created. >>> + * >>> + * Returns a pointer to the first element of dest (or the newly allocated copy) >>> + */ >>> +QEMUOptionParameter *append_option_parameters(QEMUOptionParameter *dest, >>> + QEMUOptionParameter *list) >>> +{ >>> + size_t num_options, num_dest_options; >>> + >>> + num_options = count_option_parameters(dest); >>> + num_dest_options = num_options; >>> + >>> + num_options += count_option_parameters(list); >>> + >>> + dest = qemu_realloc(dest, (num_options + 1) * sizeof(QEMUOptionParameter)); >>> + >>> + while (list && list->name) { >>> + if (get_option_parameter(dest, list->name) == NULL) { >>> + dest[num_dest_options++] = *list; >> >> You need to add a dest[num_dest_options].name = NULL; here. Otherwise >> the next loop iteration works on uninitialized memory and possibly an >> unterminated list. I got a segfault for that reason. >> > > I forgot to add it, sorry. > Fixed version is below. > > Thanks, > > Kazutaka > > == > This patch enables protocol drivers to use their create options which > are not supported by the format. For example, protcol drivers can use > a backing_file option with raw format. > > Signed-off-by: MORITA Kazutaka $ ./qemu-img create -f qcow2 -o cluster_size=4k /tmp/test.qcow2 4G Unknown option 'cluster_size' qemu-img: Invalid options for file format 'qcow2'. I think you added another num_dest_options++ which shouldn't be there. Kevin