From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=33965 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OuTyL-0005C9-TD for qemu-devel@nongnu.org; Sat, 11 Sep 2010 13:34:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OuTyH-00052O-MS for qemu-devel@nongnu.org; Sat, 11 Sep 2010 13:34:41 -0400 Received: from mail-gy0-f173.google.com ([209.85.160.173]:55428) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OuTyH-00052H-HY for qemu-devel@nongnu.org; Sat, 11 Sep 2010 13:34:37 -0400 Received: by gya1 with SMTP id 1so1898347gya.4 for ; Sat, 11 Sep 2010 10:34:37 -0700 (PDT) Message-ID: <4C8BBDAA.5090507@codemonkey.ws> Date: Sat, 11 Sep 2010 12:34:34 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 3/3] disk: don't read from disk until the guest starts References: <1284213896-12705-1-git-send-email-aliguori@us.ibm.com> <1284213896-12705-4-git-send-email-aliguori@us.ibm.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi , Juan Quintela On 09/11/2010 12:24 PM, Stefan Hajnoczi wrote: > On Sat, Sep 11, 2010 at 3:04 PM, Anthony Liguori wrote: > >> This fixes a couple nasty problems relating to live migration. >> >> 1) When dealing with shared storage with weak coherence (i.e. NFS), even if >> we re-read, we may end up with undesired caching. By delaying any reads >> until we absolutely have to, we decrease the likelihood of any undesirable >> caching. >> >> 2) When dealing with copy-on-read, the local storage acts as a cache. We need >> to make sure to avoid any reads to avoid polluting the local cache. >> >> Signed-off-by: Anthony Liguori >> >> diff --git a/hw/ide/core.c b/hw/ide/core.c >> index 1e466d1..57d8db3 100644 >> --- a/hw/ide/core.c >> +++ b/hw/ide/core.c >> @@ -105,6 +132,8 @@ static void ide_identify(IDEState *s) >> return; >> } >> >> + guess_geometry(s); >> + >> > Does the same change need to be made in ide_cfata_identify()? > > I quickly checked the VMStateDescription and don't see cylinders, > heads, sectors being saved for migration. I am concerned that IDE > will break after migration if the following happens: > 1. Guest resumes and does not issue ATA IDENTIFY so cylinders, heads, > sectors are not initialized. > 2. Normal I/O is performed, invoking ide_get_sector() which uses > geometry information that has not been initialized. > > Did I miss something? > Nope, I just missed some places that need calls. I've renamed guess_geometry() to init_geometry() and added calls to ide_cfata_identify(), ide_get_sector(), and ide_set_sector(). That should cover it all. >> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c >> index bd6bbe6..0bf17ec 100644 >> --- a/hw/virtio-blk.c >> +++ b/hw/virtio-blk.c >> @@ -427,6 +427,10 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) >> >> bdrv_get_geometry(s->bs,&capacity); >> bdrv_get_geometry_hint(s->bs,&cylinders,&heads,&secs); >> + if (cylinders == 0) { >> + bdrv_guess_geometry(s->bs,&cylinders,&heads,&secs); >> + } >> + >> > bdrv_guess_geometry() can be called unconditionally. The call to > bdrv_get_geometry_hint() can be eliminated. bdrv_guess_geometry() > updates the geometry hint and does not probe the boot sector after the > first time. > Yeah, that's a fair point. Regards, Anthony Liguori > Stefan > >