From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60019 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Oq2Fk-0003er-VM for qemu-devel@nongnu.org; Mon, 30 Aug 2010 07:10:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OpzmX-0003gT-Np for qemu-devel@nongnu.org; Mon, 30 Aug 2010 04:31:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60322) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OpzmX-0003g8-GU for qemu-devel@nongnu.org; Mon, 30 Aug 2010 04:31:57 -0400 Message-ID: <4C7B6C87.80305@redhat.com> Date: Mon, 30 Aug 2010 10:32:07 +0200 From: Kevin Wolf MIME-Version: 1.0 Subject: Re: [Qemu-devel] [Patch][Bug 618533]OpenSolaris guest fails to see the Solaris partitions of a physical disk References: <661696.47524.qm@web31708.mail.mud.yahoo.com> In-Reply-To: <661696.47524.qm@web31708.mail.mud.yahoo.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: devsk Cc: qemu-devel@nongnu.org Am 28.08.2010 09:17, schrieb devsk: > Attached is the patch for fixing the issue reported in bug 618533. > > The patch applies clean against 0.12.5 as well as git version. > > -devsk > PS: I am not on the list. Please CC me. > > Signed Off by: devsk > ============================================= Please include a patch description that can be used as a commit message. If you use git format-patch, you make it easiest for maintainers to apply your patch. Also, the tag is spelled "Signed-off-by:" and you should use your real name there. > --- hw/ide/core.c.orig 2010-08-16 20:16:11.168843003 -0700 > +++ hw/ide/core.c 2010-08-16 20:37:48.979843000 -0700 > @@ -109,7 +109,6 @@ > put_le16(p + 0, 0x0040); > put_le16(p + 1, s->cylinders); > put_le16(p + 3, s->heads); > - put_le16(p + 4, 512 * s->sectors); /* XXX: retired, remove ? */ Just curious, is removing this field actually required? > put_le16(p + 5, 512); /* XXX: retired, remove ? */ > put_le16(p + 6, s->sectors); > padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */ > @@ -134,8 +133,16 @@ > put_le16(p + 58, oldsize >> 16); > if (s->mult_sectors) > put_le16(p + 59, 0x100 | s->mult_sectors); > - put_le16(p + 60, s->nb_sectors); > - put_le16(p + 61, s->nb_sectors >> 16); > + if (s->nb_sectors <= (1 << 28) - 1) > + { You may write the same condition a bit simpler as (s->nb_sectors < (1 << 28)). Please put the brace on the same line as the if, see the CODING_STYLE file in the source. Also, indentation should be four spaces. > + put_le16(p + 60, s->nb_sectors); > + put_le16(p + 61, s->nb_sectors >> 16); > + } > + else > + { > + put_le16(p + 60, ((1 << 28) - 1) & 0xffff); > + put_le16(p + 61, ((1 << 28) - 1) >> 16); Hm, I guess it's more readable like this: .... } else { put_le16(p + 60, 0xffff); put_le16(p + 61, 0xffff); } > + } > put_le16(p + 62, 0x07); /* single word dma0-2 supported */ > put_le16(p + 63, 0x07); /* mdma0-2 supported */ > put_le16(p + 65, 120); > @@ -156,7 +163,7 @@ > else > put_le16(p + 85, (1 << 14) | 1); > /* 13=flush_cache_ext,12=flush_cache,10=lba48 */ > - put_le16(p + 86, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10)); > + put_le16(p + 86, (1 << 13) | (1 <<12) | (1 << 10)); > /* 14=set to 1, 1=smart self test, 0=smart error logging */ > put_le16(p + 87, (1 << 14) | 0); > put_le16(p + 88, 0x3f | (1 << 13)); /* udma5 set and supported */ The logic looks correct. Please CC me when you submit v2 of the patch. Kevin