Re: [Qemu-devel] [PATCH v3 5/5] BlockConf: Call backend functions to detect geometry and blocksizes

[Thomas Huth]

This patch looks basically good to me, I just got some cosmetic
requests:

On Fri,  5 Dec 2014 18:56:21 +0100
Ekaterina Tumanova <address@hidden> wrote:
...
> diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
> index 6fcf74d..fbd602d 100644
> --- a/hw/block/hd-geometry.c
> +++ b/hw/block/hd-geometry.c
> @@ -121,7 +121,17 @@ void hd_geometry_guess(BlockBackend *blk,
>                         int *ptrans)
>  {
>      int cylinders, heads, secs, translation;
> +    hdGeometry geo;
> 
> +    /* Try to probe the backing device geometry, otherwise fallback
> +       to the old logic. (as of 12/2014 probing only succeeds on DASDs) */
> +    if (blk_probe_geometry(blk, &geo) == 0) {
> +        *pcyls = geo.cylinders;
> +        *psecs = geo.sectors;
> +        *pheads = geo.heads;
> +        translation = BIOS_ATA_TRANSLATION_NONE;
> +        goto done;
> +    }
>      if (guess_disk_lchs(blk, &cylinders, &heads, &secs) < 0) {

Can you get rid of the "goto" above by placing an "else" in front of
this if-statement?

>          /* no LCHS guess: use a standard physical disk geometry  */
>          guess_chs_for_size(blk, pcyls, pheads, psecs);
> @@ -142,7 +152,9 @@ void hd_geometry_guess(BlockBackend *blk,
>          /* disable any translation to be in sync with
>             the logical geometry */
>          translation = BIOS_ATA_TRANSLATION_NONE;
> +

Unnecessary empty line, please remove this hunk.

>      }
> +done:
>      if (ptrans) {
>          *ptrans = translation;
>      }

 Thomas