From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=45294 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OJ6QK-00050R-3P for qemu-devel@nongnu.org; Mon, 31 May 2010 10:57:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OJ6QJ-0003Xd-1b for qemu-devel@nongnu.org; Mon, 31 May 2010 10:57:03 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59563) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OJ6QI-0003XU-RZ for qemu-devel@nongnu.org; Mon, 31 May 2010 10:57:03 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o4VEv1Qf017896 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 31 May 2010 10:57:02 -0400 Message-ID: <4C03CE2B.3010404@redhat.com> Date: Mon, 31 May 2010 16:56:43 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1275053935-10627-1-git-send-email-armbru@redhat.com> <1275053935-10627-6-git-send-email-armbru@redhat.com> In-Reply-To: <1275053935-10627-6-git-send-email-armbru@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 05/14] ide: Remove redundant IDEState member conf List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, kraxel@redhat.com Am 28.05.2010 15:38, schrieb Markus Armbruster: > Commit 428c149b added IDEState member conf to let commit 0009baf1 find > the BlockConf from there. It exists only for qdev drives, created via > ide_drive_initfn(), not for drives created via ide_init2(). > > But for a qdev drive, we can just as well reach its IDEDevice, which > contains the BlockConf. Do that, and revert the parts of commit > 428c149b that add IDEState member conf. > > Signed-off-by: Markus Armbruster > --- > hw/ide/core.c | 16 +++++++--------- > hw/ide/internal.h | 4 +--- > hw/ide/qdev.c | 3 +-- > 3 files changed, 9 insertions(+), 14 deletions(-) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index 066fecb..c3334b1 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -98,6 +98,7 @@ static void ide_identify(IDEState *s) > { > uint16_t *p; > unsigned int oldsize; > + IDEDevice *dev; > > if (s->identify_set) { > memcpy(s->io_buffer, s->identify_data, sizeof(s->identify_data)); > @@ -165,8 +166,9 @@ static void ide_identify(IDEState *s) > put_le16(p + 101, s->nb_sectors >> 16); > put_le16(p + 102, s->nb_sectors >> 32); > put_le16(p + 103, s->nb_sectors >> 48); > - if (s->conf && s->conf->physical_block_size) > - put_le16(p + 106, 0x6000 | get_physical_block_exp(s->conf)); > + dev = s->unit ? s->bus->slave : s->bus->master; > + if (dev && dev->conf.physical_block_size) If this is meant to be the same condition it would be dev->conf && ... I can't really tell right now if it can happen that either dev or dev->conf can be NULL here. In a quick attempt I couldn't get either one. So, which version is correct, or should the first part be dropped completely? Kevin