Re: [Qemu-devel] [PATCH v2 6/6] geometry: Target specific hook for s390x in geometry guessing

2014-11-28 Thread Markus Armbruster
Ekaterina Tumanova tuman...@linux.vnet.ibm.com writes:

 For target-s390x, the behaviour is chosen as follows:
 If no geo could be guessed from backing device, guess_disk_lchs (partition
 table guessing) is called.
 If this is not successful (e.g. image files) geometry is derived from the
 size of the disk (as before).
 We have no translation on s390, so we simply set in to NONE.

 Signed-off-by: Ekaterina Tumanova tuman...@linux.vnet.ibm.com
 ---
  hw/block/Makefile.objs |  6 +-
  hw/block/hd-geometry.c | 36 +++-
  2 files changed, 36 insertions(+), 6 deletions(-)

 diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
 index d4c3ab7..1f7da7a 100644
 --- a/hw/block/Makefile.objs
 +++ b/hw/block/Makefile.objs
 @@ -1,4 +1,4 @@
 -common-obj-y += block.o cdrom.o hd-geometry.o
 +common-obj-y += block.o cdrom.o
  common-obj-$(CONFIG_FDC) += fdc.o
  common-obj-$(CONFIG_SSI_M25P80) += m25p80.o
  common-obj-$(CONFIG_NAND) += nand.o
 @@ -13,3 +13,7 @@ obj-$(CONFIG_SH4) += tc58128.o
  
  obj-$(CONFIG_VIRTIO) += virtio-blk.o
  obj-$(CONFIG_VIRTIO) += dataplane/
 +
 +# geometry is target/architecture dependent and therefore needs to be built
 +# for every target platform
 +obj-y += hd-geometry.o
 diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
 index 4972114..b462225 100644
 --- a/hw/block/hd-geometry.c
 +++ b/hw/block/hd-geometry.c
 @@ -49,11 +49,12 @@ struct partition {
  
  /* try to guess the disk logical geometry from the MSDOS partition table.
 Return 0 if OK, -1 if could not guess */
 -static int guess_disk_lchs(BlockBackend *blk,
 -   int *pcylinders, int *pheads, int *psectors)
 +static int guess_disk_lchs(BlockBackend *blk, uint32_t *pcylinders,
 +   uint32_t *pheads, uint32_t *psectors)
  {
  uint8_t buf[BDRV_SECTOR_SIZE];
 -int i, heads, sectors, cylinders;
 +int i;
 +uint32_t heads, sectors, cylinders;
  struct partition *p;
  uint32_t nr_sects;
  uint64_t nb_sectors;
 @@ -116,11 +117,11 @@ static void guess_chs_for_size(BlockBackend *blk,
  *psecs = 63;
  }
  
 +#ifdef TARGET_S390X
  void hd_geometry_guess(BlockBackend *blk,
 uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
 int *ptrans)
  {
 -int cylinders, heads, secs, translation;
  struct ProbeGeometry geometry = blk_probe_geometry(blk);
  
  if (geometry.rc == 0) {
 @@ -129,7 +130,32 @@ void hd_geometry_guess(BlockBackend *blk,
  *pheads = geometry.geo.heads;
  goto done;
  }
 +if (guess_disk_lchs(blk, pcyls, pheads, psecs) == 0) {
 +goto done;
 +}
 +guess_chs_for_size(blk, pcyls, pheads, psecs);
 +done:
 +if (ptrans) {
 +*ptrans = BIOS_ATA_TRANSLATION_NONE;
 +}

The goto looks awkward...  What about

if (guess_disk_lchs(blk, pcyls, pheads, psecs)  0) {
guess_chs_for_size(blk, pcyls, pheads, psecs);
}

  
 +trace_hd_geometry_guess(blk, *pcyls, *pheads, *psecs,
 +BIOS_ATA_TRANSLATION_NONE);
 +}
 +#else
 +void hd_geometry_guess(BlockBackend *blk,
 +   uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
 +   int *ptrans)
 +{
 +int cylinders, heads, secs, translation;
 +struct ProbeGeometry geometry = blk_probe_geometry(blk);
 +
 +if (geometry.rc == 0) {
 +*pcyls = geometry.geo.cylinders;
 +*psecs = geometry.geo.sectors;
 +*pheads = geometry.geo.heads;
 +goto done;
 +}
  if (guess_disk_lchs(blk, cylinders, heads, secs)  0) {
  /* no LCHS guess: use a standard physical disk geometry  */
  guess_chs_for_size(blk, pcyls, pheads, psecs);
 @@ -157,7 +183,7 @@ done:
  }
  trace_hd_geometry_guess(blk, *pcyls, *pheads, *psecs, translation);
  }
 -
 +#endif

Please put the blank line after the #endif.

  int hd_bios_chs_auto_trans(uint32_t cyls, uint32_t heads, uint32_t secs)
  {
  return cyls = 1024  heads = 16  secs = 63

hd_geometry_guess() behavior before this series:

If we can't guess LCHS from MSDOS partition table
guess geometry on size
translation is NONE for very small disk, else LBA
Else if LCHS guess has heads  16
BIOS LBA translation must be active
guess geometry on size
translation is LARGE for sufficiently small disk, else LBA
Else
use LCHS guess
translation is NONE

Afterwards, targets other than s390x:

If backend has a geometry (currently only DASD has)
use backend geometry
translation is NONE
Else behave like before this series

Incompatible change.  Why do we want it?

Afterwards, target s390x:

If backend has a geometry (currently only DASD has)
use backend geometry
translation is NONE
Else if we can't guess LCHS from MSDOS partition table
guess geometry on size
Else
use LCHS guess

[Qemu-devel] [PATCH v2 6/6] geometry: Target specific hook for s390x in geometry guessing

2014-11-19 Thread Ekaterina Tumanova
For target-s390x, the behaviour is chosen as follows:
If no geo could be guessed from backing device, guess_disk_lchs (partition
table guessing) is called.
If this is not successful (e.g. image files) geometry is derived from the
size of the disk (as before).
We have no translation on s390, so we simply set in to NONE.

Signed-off-by: Ekaterina Tumanova tuman...@linux.vnet.ibm.com
---
 hw/block/Makefile.objs |  6 +-
 hw/block/hd-geometry.c | 36 +++-
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
index d4c3ab7..1f7da7a 100644
--- a/hw/block/Makefile.objs
+++ b/hw/block/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += block.o cdrom.o hd-geometry.o
+common-obj-y += block.o cdrom.o
 common-obj-$(CONFIG_FDC) += fdc.o
 common-obj-$(CONFIG_SSI_M25P80) += m25p80.o
 common-obj-$(CONFIG_NAND) += nand.o
@@ -13,3 +13,7 @@ obj-$(CONFIG_SH4) += tc58128.o
 
 obj-$(CONFIG_VIRTIO) += virtio-blk.o
 obj-$(CONFIG_VIRTIO) += dataplane/
+
+# geometry is target/architecture dependent and therefore needs to be built
+# for every target platform
+obj-y += hd-geometry.o
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 4972114..b462225 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -49,11 +49,12 @@ struct partition {
 
 /* try to guess the disk logical geometry from the MSDOS partition table.
Return 0 if OK, -1 if could not guess */
-static int guess_disk_lchs(BlockBackend *blk,
-   int *pcylinders, int *pheads, int *psectors)
+static int guess_disk_lchs(BlockBackend *blk, uint32_t *pcylinders,
+   uint32_t *pheads, uint32_t *psectors)
 {
 uint8_t buf[BDRV_SECTOR_SIZE];
-int i, heads, sectors, cylinders;
+int i;
+uint32_t heads, sectors, cylinders;
 struct partition *p;
 uint32_t nr_sects;
 uint64_t nb_sectors;
@@ -116,11 +117,11 @@ static void guess_chs_for_size(BlockBackend *blk,
 *psecs = 63;
 }
 
+#ifdef TARGET_S390X
 void hd_geometry_guess(BlockBackend *blk,
uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
int *ptrans)
 {
-int cylinders, heads, secs, translation;
 struct ProbeGeometry geometry = blk_probe_geometry(blk);
 
 if (geometry.rc == 0) {
@@ -129,7 +130,32 @@ void hd_geometry_guess(BlockBackend *blk,
 *pheads = geometry.geo.heads;
 goto done;
 }
+if (guess_disk_lchs(blk, pcyls, pheads, psecs) == 0) {
+goto done;
+}
+guess_chs_for_size(blk, pcyls, pheads, psecs);
+done:
+if (ptrans) {
+*ptrans = BIOS_ATA_TRANSLATION_NONE;
+}
 
+trace_hd_geometry_guess(blk, *pcyls, *pheads, *psecs,
+BIOS_ATA_TRANSLATION_NONE);
+}
+#else
+void hd_geometry_guess(BlockBackend *blk,
+   uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
+   int *ptrans)
+{
+int cylinders, heads, secs, translation;
+struct ProbeGeometry geometry = blk_probe_geometry(blk);
+
+if (geometry.rc == 0) {
+*pcyls = geometry.geo.cylinders;
+*psecs = geometry.geo.sectors;
+*pheads = geometry.geo.heads;
+goto done;
+}
 if (guess_disk_lchs(blk, cylinders, heads, secs)  0) {
 /* no LCHS guess: use a standard physical disk geometry  */
 guess_chs_for_size(blk, pcyls, pheads, psecs);
@@ -157,7 +183,7 @@ done:
 }
 trace_hd_geometry_guess(blk, *pcyls, *pheads, *psecs, translation);
 }
-
+#endif
 int hd_bios_chs_auto_trans(uint32_t cyls, uint32_t heads, uint32_t secs)
 {
 return cyls = 1024  heads = 16  secs = 63
-- 
1.8.5.5