All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@suse.de>
To: Kevin Wolf <kwolf@suse.de>
Cc: xen-devel@lists.xensource.com,
	Keir Fraser <keir.fraser@eu.citrix.com>,
	Otavio Salvador <otavio@ossystems.com.br>
Subject: [PATCH] tapdisk: Fix L1 table endianess of qcow images
Date: Thu, 27 Mar 2008 10:55:55 +0100	[thread overview]
Message-ID: <47EB6F2B.6080302@suse.de> (raw)
In-Reply-To: <47EA559E.6000309@suse.de>

[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]

Kevin Wolf schrieb:
> What should we do with the tapdisk implementation? Leave it broken and
> hope that it will disappear soon, add support for big endian L1 tables
> or do a conversion the other way round? The latter doesn't feel right
> (in fact it would be intentionally breaking a correct image), but adding
> support for big endian is much more critical because we end up with
> "mixed endian" if we miss one conversion...

And another one for tapdisk. I'm taking the same approach as for ioemu
here, i.e. converting the endianess when the image is opened and
rewriting the tapdisk code to use big endian.

To avoid the mentioned "mixed endian" issue and thus data corruption,
please double check the patch before you check it in. I successfully
installed a VM with this patch, though, so I'm confident that it is correct.

Kevin


tapdisk: Fix L1 table endianess of qcow images

Fix tapdisk to use big endian L1 tables as used by qemu/ioemu. Old
tapdisk images with native endianess are automagically converted to big
endian when the image file is opened for the first time.

Signed-off-by: Kevin Wolf <kwolf@suse.de>

[-- Attachment #2: tapdisk-qcow-endianess.patch --]
[-- Type: text/x-patch, Size: 3200 bytes --]

diff -r 81147306041a tools/blktap/drivers/block-qcow.c
--- a/tools/blktap/drivers/block-qcow.c	Tue Mar 25 14:37:43 2008
+++ b/tools/blktap/drivers/block-qcow.c	Thu Mar 27 11:45:28 2008
@@ -76,6 +76,7 @@
 
 #define QCOW_OFLAG_COMPRESSED (1LL << 63)
 #define SPARSE_FILE 0x01
+#define EXTHDR_L1_BIG_ENDIAN 0x02
 
 #ifndef O_BINARY
 #define O_BINARY 0
@@ -147,19 +148,30 @@
 
 static uint32_t gen_cksum(char *ptr, int len)
 {
+	int i;
 	unsigned char *md;
 	uint32_t ret;
 
 	md = malloc(MD5_DIGEST_LENGTH);
 
 	if(!md) return 0;
-
-	if (MD5((unsigned char *)ptr, len, md) != md) {
-		free(md);
-		return 0;
-	}
-
-	memcpy(&ret, md, sizeof(uint32_t));
+	
+	/* Convert L1 table to big endian */
+	for(i = 0; i < len / sizeof(uint64_t); i++) {
+		cpu_to_be64s(&((uint64_t*) ptr)[i]);
+	}
+
+	/* Generate checksum */
+	if (MD5((unsigned char *)ptr, len, md) != md)
+		ret = 0;
+	else
+		memcpy(&ret, md, sizeof(uint32_t));
+
+	/* Convert L1 table back to native endianess */
+	for(i = 0; i < len / sizeof(uint64_t); i++) {
+		be64_to_cpus(&((uint64_t*) ptr)[i]);
+	}
+
 	free(md);
 	return ret;
 }
@@ -354,7 +366,8 @@
                                    int n_start, int n_end)
 {
 	int min_index, i, j, l1_index, l2_index, l2_sector, l1_sector;
-	char *tmp_ptr, *tmp_ptr2, *l2_ptr, *l1_ptr;
+	char *tmp_ptr2, *l2_ptr, *l1_ptr;
+	uint64_t *tmp_ptr;
 	uint64_t l2_offset, *l2_table, cluster_offset, tmp;
 	uint32_t min_count;
 	int new_l2_table;
@@ -400,6 +413,11 @@
 			DPRINTF("ERROR allocating memory for L1 table\n");
 		}
 		memcpy(tmp_ptr, l1_ptr, 4096);
+
+		/* Convert block to write to big endian */
+		for(i = 0; i < 4096 / sizeof(uint64_t); i++) {
+			cpu_to_be64s(&tmp_ptr[i]);
+		}
 
 		/*
 		 * Issue non-asynchronous L1 write.
@@ -777,7 +795,7 @@
 		goto fail;
 
 	for(i = 0; i < s->l1_size; i++) {
-		//be64_to_cpus(&s->l1_table[i]);
+		be64_to_cpus(&s->l1_table[i]);
 		//DPRINTF("L1[%d] => %llu\n", i, s->l1_table[i]);
 		if (s->l1_table[i] > final_cluster)
 			final_cluster = s->l1_table[i];
@@ -810,6 +828,38 @@
 		be32_to_cpus(&exthdr->xmagic);
 		if(exthdr->xmagic != XEN_MAGIC) 
 			goto end_xenhdr;
+    
+		/* Try to detect old tapdisk images. They have to be fixed because 
+		 * they don't use big endian but native endianess for the L1 table */
+		if ((exthdr->flags & EXTHDR_L1_BIG_ENDIAN) == 0) {
+
+			/* 
+			   The image is broken. Fix it. The L1 table has already been 
+			   byte-swapped, so we can write it to the image file as it is
+			   currently in memory. Then swap it back to native endianess
+			   for operation.
+			 */
+
+			DPRINTF("qcow: Converting image to big endian L1 table\n");
+
+			lseek(fd, s->l1_table_offset, SEEK_SET);
+			if (write(fd, s->l1_table, l1_table_size) != l1_table_size) {
+				DPRINTF("qcow: Failed to write new L1 table\n");
+				goto fail;
+			}
+
+			for(i = 0;i < s->l1_size; i++) {
+				cpu_to_be64s(&s->l1_table[i]);
+			}
+
+			/* Write the big endian flag to the extended header */
+			exthdr->flags |= EXTHDR_L1_BIG_ENDIAN;
+
+			if (write(fd, buf, 512) != 512) {
+				DPRINTF("qcow: Failed to write extended header\n");
+				goto fail;
+			}
+		}
 
 		/*Finally check the L1 table cksum*/
 		be32_to_cpus(&exthdr->cksum);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

  parent reply	other threads:[~2008-03-27  9:55 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-25 17:25 The two image formats called qcow Kevin Wolf
2008-03-25 18:58 ` Otavio Salvador
2008-03-26  8:10   ` Keir Fraser
2008-03-26  8:50     ` Kevin Wolf
2008-03-26 13:03       ` Otavio Salvador
2008-03-26 13:54         ` Kevin Wolf
2008-03-26 14:02           ` Keir Fraser
2008-03-26 14:07             ` Kevin Wolf
2008-03-26 14:15               ` Keir Fraser
2008-03-26 14:22                 ` Kevin Wolf
2008-03-27  9:55           ` Kevin Wolf [this message]
2008-03-26 13:23       ` Daniel P. Berrange

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47EB6F2B.6080302@suse.de \
    --to=kwolf@suse.de \
    --cc=keir.fraser@eu.citrix.com \
    --cc=otavio@ossystems.com.br \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.