qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Cc: Fam Zheng <fam@euphon.net>,
	qemu-block@nongnu.org, qemu-trivial@nongnu.org,
	Michael Tokarev <mjt@tls.msk.ru>,
	qemu-devel@nongnu.org, Laurent Vivier <laurent@vivier.eu>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Max Reitz <mreitz@redhat.com>, John Snow <jsnow@redhat.com>
Subject: Re: [PATCH 3/7] hw/ide/core: Replace magic '512' value by BDRV_SECTOR_SIZE
Date: Mon, 17 Aug 2020 13:17:45 +0200	[thread overview]
Message-ID: <20200817111745.GG11402@linux.fritz.box> (raw)
In-Reply-To: <20200814082841.27000-4-f4bug@amsat.org>

Am 14.08.2020 um 10:28 hat Philippe Mathieu-Daudé geschrieben:
> Use self-explicit definitions instead of magic '512' value.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

BDRV_SECTOR_SIZE is the arbitrary unit in which some block layer
functions and variables work (such as bs->total_sectors). It happens to
be 512.

IDE disks have a sector size, too. Actually, two of them, a physical and
a logical one. The more important one is the logical one. We happen to
emulate only IDE devices for which the logical block size is 512 bytes
(ide_dev_initfn() errors out otherwise).

But just because two constants both happen to be 512 in practice, they
are not the same thing.

So if we want to replace all magic 512 values, we should probably
introduce a new IDE_SECTOR_SIZE and then decide case by case whether
IDE_SECTOR_SIZE or BDRV_SECTOR_SIZE is meant. I think most (if not all)
of the places you converted in this patch need to be IDE_SECTOR_SIZE.

Kevin



  parent reply	other threads:[~2020-08-17 11:18 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-14  8:28 [PATCH 0/7] block: Use definitions instead of magic values Philippe Mathieu-Daudé
2020-08-14  8:28 ` [PATCH 1/7] block/null: Make more explicit the driver default size is 1GiB Philippe Mathieu-Daudé
2020-08-14 16:58   ` Richard Henderson
2020-08-15  3:15   ` Li Qiang
2020-08-17 11:07   ` Kevin Wolf
2020-08-14  8:28 ` [PATCH 2/7] hw/ide/core: Trivial typo fix Philippe Mathieu-Daudé
2020-08-14 16:58   ` Richard Henderson
2020-08-15  3:18   ` Li Qiang
2020-08-17 11:10   ` Kevin Wolf
2020-08-14  8:28 ` [PATCH 3/7] hw/ide/core: Replace magic '512' value by BDRV_SECTOR_SIZE Philippe Mathieu-Daudé
2020-08-14 16:58   ` Richard Henderson
2020-08-15  3:19   ` Li Qiang
2020-08-17 11:17   ` Kevin Wolf [this message]
2020-09-23 14:53     ` John Snow
2020-09-23 16:57       ` Philippe Mathieu-Daudé
2020-08-14  8:28 ` [PATCH 4/7] hw/ide/ahci: " Philippe Mathieu-Daudé
2020-08-14 16:58   ` Richard Henderson
2020-08-15  3:21   ` Li Qiang
2020-08-14  8:28 ` [PATCH 5/7] hw/ide/atapi: " Philippe Mathieu-Daudé
2020-08-14 17:00   ` Richard Henderson
2020-08-15  3:22   ` Li Qiang
2020-08-14  8:28 ` [PATCH 6/7] hw/ide/pci: " Philippe Mathieu-Daudé
2020-08-14 17:00   ` Richard Henderson
2020-08-15  3:23   ` Li Qiang
2020-08-14  8:28 ` [PATCH 7/7] hw/scsi/scsi-disk: " Philippe Mathieu-Daudé
2020-08-14 17:01   ` Richard Henderson
2020-08-15  3:24   ` Li Qiang
2020-08-17 11:21   ` Kevin Wolf
2020-08-17  9:03 ` [PATCH 0/7] block: Use definitions instead of magic values Stefano Garzarella
2020-09-01  9:27 ` Laurent Vivier
2020-09-01 13:55   ` Philippe Mathieu-Daudé

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=20200817111745.GG11402@linux.fritz.box \
    --to=kwolf@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=fam@euphon.net \
    --cc=jsnow@redhat.com \
    --cc=laurent@vivier.eu \
    --cc=mjt@tls.msk.ru \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).