From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39785) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QlKTB-00066C-Lf for qemu-devel@nongnu.org; Mon, 25 Jul 2011 08:41:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QlKTA-0001tT-IM for qemu-devel@nongnu.org; Mon, 25 Jul 2011 08:41:13 -0400 Received: from e23smtp03.au.ibm.com ([202.81.31.145]:55621) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QlKT9-0001sf-SH for qemu-devel@nongnu.org; Mon, 25 Jul 2011 08:41:12 -0400 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [202.81.31.246]) by e23smtp03.au.ibm.com (8.14.4/8.13.1) with ESMTP id p6PCZd3A018421 for ; Mon, 25 Jul 2011 22:35:39 +1000 Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p6PCeFpn372798 for ; Mon, 25 Jul 2011 22:40:15 +1000 Received: from d23av01.au.ibm.com (loopback [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p6PCepgs001197 for ; Mon, 25 Jul 2011 22:40:52 +1000 Message-ID: <4E2D6718.3060300@in.ibm.com> Date: Mon, 25 Jul 2011 18:22:40 +0530 From: Supriya Kannery MIME-Version: 1.0 References: <20110704104237.28170.97021.sendpatchset@skannery> <20110704104320.28170.29248.sendpatchset@skannery> <4E12ED9A.7070805@in.ibm.com> <4E1D9879.8060009@in.ibm.com> <20110725063015.GA12854@stefanha-thinkpad.localdomain> In-Reply-To: <20110725063015.GA12854@stefanha-thinkpad.localdomain> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [V4 Patch 3/4 - Updated]Qemu: Command "block_set" for dynamic block params change List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , qemu-devel@nongnu.org, Christoph Hellwig On 07/25/2011 12:00 PM, Stefan Hajnoczi wrote: > On Wed, Jul 13, 2011 at 06:37:05PM +0530, Supriya Kannery wrote: >> + ret = bdrv_open(bs, bs->filename, bdrv_flags, drv); >> + if (ret< 0) { >> + /* Reopen failed. Try to open with original flags */ >> + error_report("Opening file with changed flags..."); >> + qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename); > > If the next open fails too then there will be two qerror_report() calls. > This causes a warning and the new qerror is dropped. Please consolidate > the error reporting so there is only one error before returning from > this function. > ok >> + if (bdrv_is_inserted(bs)) { >> + /* Reopen file with changed set of flags */ >> + return bdrv_reopen(bs, bdrv_flags); >> + } else { >> + /* Save hostcache change for future use */ >> + bs->open_flags = bdrv_flags; > > Can you explain the scenario where this works? > > Looking at do_change_block() the flags will be clobbered so saving them > away does not help. Intention here is to use the changed hostcache setting during device insertion and there after. To apply this to 'change' command (during insertion of a device), will need to make the following code changes in do_change_block. + + bdrv_flags = bs->open_flags; + bdrv_flags |= bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR; bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0; if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) { qerror_report(QERR_OPEN_FILE_FAILED, filename); Need to track bs->open_flags a bit more to see what other changes needed to ensure usage of changed hostcache setting until user initiates next hostcache change explicitly for that drive. Please suggest... should we be saving the hostcache change for future use or just inhibit user from changing hostcache setting for an empty drive? >> + /* If input not in "param=value" format, display error */ >> + driver = qemu_opt_get(opts, "driver"); >> + if (driver != NULL) { >> + error_report("Invalid parameter %s", driver); > > error_report() only works for HMP. Please use qerror_report() so both > HMP and QMP see the error. Same issue further down. > ok..will change error_report to qerror_report. Will make further code changes into patchset version 5