From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41827) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UfrEc-0003BQ-M2 for qemu-devel@nongnu.org; Fri, 24 May 2013 08:36:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UfrEW-0003zA-7P for qemu-devel@nongnu.org; Fri, 24 May 2013 08:36:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:30435) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UfrEW-0003yy-0F for qemu-devel@nongnu.org; Fri, 24 May 2013 08:36:32 -0400 Date: Fri, 24 May 2013 14:36:25 +0200 From: Stefan Hajnoczi Message-ID: <20130524123625.GA24111@stefanha-thinkpad.redhat.com> References: <1369331087-22345-1-git-send-email-coreyb@linux.vnet.ibm.com> <20130524095938.GJ21639@stefanha-thinkpad.redhat.com> <519F5967.70801@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <519F5967.70801@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH 0/7] VNVRAM persistent storage List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Berger Cc: kwolf@redhat.com, aliguori@us.ibm.com, Stefan Hajnoczi , Corey Bryant , qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, jschopp@linux.vnet.ibm.com, lcapitulino@redhat.com On Fri, May 24, 2013 at 08:13:27AM -0400, Stefan Berger wrote: > On 05/24/2013 05:59 AM, Stefan Hajnoczi wrote: > >On Thu, May 23, 2013 at 01:44:40PM -0400, Corey Bryant wrote: > >>This patch series provides VNVRAM persistent storage support that > >>QEMU can use internally. The initial target user will be a software > >>vTPM 1.2 backend that needs to store keys in VNVRAM and be able to > >>reboot/migrate and retain the keys. > >> > >>This support uses QEMU's block driver to provide persistent storage > >>by reading/writing VNVRAM data from/to a drive image. The VNVRAM > >>drive image is provided with the -drive command line option just like > >>any other drive image and the vnvram_create() API will find it. > >> > >>The APIs allow for VNVRAM entries to be registered, one at a time, > >>each with a maximum blob size. Entry blobs can then be read/written > >>from/to an entry on the drive. Here's an example of usage: > >> > >>VNVRAM *vnvram; > >>int errcode > >>const VNVRAMEntryName entry_name; > >>const char *blob_w = "blob data"; > >>char *blob_r; > >>uint32_t blob_r_size; > >> > >>vnvram = vnvram_create("drive-ide0-0-0", false, &errcode); > >>strcpy((char *)entry_name, "first-entry"); > >VNVRAMEntryName is very prone to buffer overflow. I hope real code > >doesn't use strcpy(). The cast is ugly, please don't hide the type. > > > >>vnvram_register_entry(vnvram, &entry_name, 1024); > >>vnvram_write_entry(vnvram, &entry_name, (char *)blob_w, strlen(blob_w)+1); > >>vnvram_read_entry(vnvram, &entry_name, &blob_r, &blob_r_size); > >These are synchronous functions. If I/O is involved then this is a > >problem: QEMU will be blocked waiting for host I/O to complete and the > >big QEMU lock is held. This can cause poor guest interactivity and poor > >scalability because vcpus cannot make progress, neither can the QEMU > >monitor respond. > > The vTPM is going to run as a thread and will have to write state > blobs into a bdrv. The above functions will typically be called from > this thead. When I originally wrote the code, the vTPM thread could > not write the blobs into bdrv directly, so I had to resort to > sending a message to the main QEMU thread to write the data to the > bdrv. How else could we do this? How else: use asynchronous APIs like bdrv_aio_writev() or the coroutine versions (which eliminate the need for callbacks) like bdrv_co_writev(). I'm preparing patches that allow the QEMU block layer to be used safely outside the QEMU global mutex. Once this is possible it would be okay to use synchronous methods. Stefan