From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:38822) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QlKcN-0003pw-Ab for qemu-devel@nongnu.org; Mon, 25 Jul 2011 08:50:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QlKcM-0003VT-BJ for qemu-devel@nongnu.org; Mon, 25 Jul 2011 08:50:43 -0400 Received: from mail-gx0-f173.google.com ([209.85.161.173]:60529) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QlKcM-0003VP-7B for qemu-devel@nongnu.org; Mon, 25 Jul 2011 08:50:42 -0400 Received: by gxk26 with SMTP id 26so2604372gxk.4 for ; Mon, 25 Jul 2011 05:50:41 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4E2D6718.3060300@in.ibm.com> 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> <4E2D6718.3060300@in.ibm.com> Date: Mon, 25 Jul 2011 13:50:41 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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: Kevin Wolf Cc: Supriya Kannery , qemu-devel@nongnu.org, Christoph Hellwig On Mon, Jul 25, 2011 at 1:52 PM, Supriya Kannery wrot= e: > On 07/25/2011 12:00 PM, Stefan Hajnoczi wrote: >> >> On Wed, Jul 13, 2011 at 06:37:05PM +0530, Supriya Kannery wrote: >>> + =A0 =A0if (bdrv_is_inserted(bs)) { >>> + =A0 =A0 =A0 =A0/* Reopen file with changed set of flags */ >>> + =A0 =A0 =A0 =A0return bdrv_reopen(bs, bdrv_flags); >>> + =A0 =A0} else { >>> + =A0 =A0 =A0 =A0/* Save hostcache change for future use */ >>> + =A0 =A0 =A0 =A0bs->open_flags =3D 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. > > + > + =A0 =A0bdrv_flags =3D bs->open_flags; > + =A0 =A0bdrv_flags |=3D bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR; > =A0 =A0 bdrv_flags |=3D bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0; > =A0 =A0 if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) { > =A0 =A0 =A0 =A0 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? The 'change' command does not support any cache=3D options today. It always opens the new image with cache=3Dwritethrough semantics. This seems like a bug in 'change' to me. We should preserve cache=3D settings across change or at least provide a way to specify them as arguments. Perhaps your existing code is fine. When 'change' is fixed or replaced then 'block_set' will work across 'change' too. Kevin: Thoughts? Stefan