From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1IZLwN-0006f8-8Z for qemu-devel@nongnu.org; Sun, 23 Sep 2007 03:31:43 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1IZLwL-0006eR-Ui for qemu-devel@nongnu.org; Sun, 23 Sep 2007 03:31:42 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1IZLwL-0006eM-SY for qemu-devel@nongnu.org; Sun, 23 Sep 2007 03:31:41 -0400 Received: from main.gmane.org ([80.91.229.2] helo=ciao.gmane.org) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1IZLwK-0002Kv-V8 for qemu-devel@nongnu.org; Sun, 23 Sep 2007 03:31:41 -0400 Received: from list by ciao.gmane.org with local (Exim 4.43) id 1IZLwC-0007ZB-2y for qemu-devel@nongnu.org; Sun, 23 Sep 2007 07:31:32 +0000 Received: from 83.189.192.132 ([83.189.192.132]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Sun, 23 Sep 2007 07:31:32 +0000 Received: from lorenzo.campedelli by 83.189.192.132 with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Sun, 23 Sep 2007 07:31:32 +0000 From: Lorenzo Campedelli Date: Sun, 23 Sep 2007 09:31:20 +0200 Message-ID: References: <219e947f0709221443l494e4c46w6efafce989dbb867@mail.gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------080605030105010802020000" In-Reply-To: Sender: news Subject: [Qemu-devel] Re: [PATCH] vvfat mbr fixes Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org This is a multi-part message in MIME format. --------------080605030105010802020000 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Johannes Schindelin wrote: > Hi, > > On Sun, 23 Sep 2007, Ivan Kalvachev wrote: > >> I've been having problems using vvfat virtual block device. Even linux >> fdisk was able to find problems with it. The reason turned out to be >> simple, MBR have bogus parameters. > > Thanks for doing this; I did not find any time for that. > > Overall, I like what you did, but here are some comments (if you would > have inlined the patch, I would have commented with references): > > - I like the convert_sector2CHS() function, although I would have named it > sector2CHS() for brevity (although the pretty magic -- or unintuitive > -- detection if lba is needed would have to be done differently, which > I maintain would be better), > > - you write the NT-ID byte-per-byte, whereas I would have used strcpy() > for clarity, > > - I'd have introduced a member nt_id instead of hardcoding an offset into > the "ignored" part, and > > - fat_type == 12 and lba does not make sense, or does it? > > Thanks, > Dscho While you are working on vvfat issues, could you give a look to the attached small patch? It tries to fix the problem with using vvfat together with -snapshot. The patch is not complete (it doesn't fix additional options such as :rw:, :floppy: :32: etc,) nor clean (I guess this specific code should be in block-vvfat.c, not in block.c, but the backing file handling is done there...) but I needed a quick fix for my needs. If anybody has suggestions on how to make a better patch to be integrated in CVS I'd be glad :) While I'm here I'd add a related question: wouldn't it be useful to be able to specify a per-device "-snapshot" option? Is it doable? Thanks, Lorenzo --------------080605030105010802020000 Content-Type: text/x-patch; name="qemu-0.9.0.20070921-block.c.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="qemu-0.9.0.20070921-block.c.patch" --- qemu-0.9.0.20070921/block.c.orig 2007-09-21 21:10:53.000000000 +0200 +++ qemu-0.9.0.20070921/block.c 2007-09-22 10:09:32.000000000 +0200 @@ -331,6 +331,7 @@ if (flags & BDRV_O_SNAPSHOT) { BlockDriverState *bs1; + BlockDriver *drv1; int64_t total_size; /* if snapshot, we create a temporary backing file and open it @@ -346,10 +347,22 @@ return -1; } total_size = bdrv_getlength(bs1) >> SECTOR_BITS; + drv1 = bs1->drv; bdrv_delete(bs1); get_tmp_filename(tmp_filename, sizeof(tmp_filename)); - realpath(filename, backing_filename); + /* + * for vvfat protocol the string "fat::" should remain + * the prefix of the filename even after realpath() call ... + */ + if (drv1 == &bdrv_vvfat) { + int i = strrchr(filename, ':') - filename + 1; + + strncpy(backing_filename, filename, i); + realpath(filename + i, backing_filename + i); + } else { + realpath(filename, backing_filename); + } if (bdrv_create(&bdrv_qcow2, tmp_filename, total_size, backing_filename, 0) < 0) { return -1; --------------080605030105010802020000--