All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] block: handle "rechs" translation option
@ 2014-01-30 13:08 Paolo Bonzini
  2014-01-30 14:37 ` Markus Armbruster
  2014-01-30 14:39 ` Stefan Hajnoczi
  0 siblings, 2 replies; 4+ messages in thread
From: Paolo Bonzini @ 2014-01-30 13:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, stefanha

The rechs translation option is so obscure that we support it but do
not even attempt to parse it.  Archeologists will surely be puzzled
by this (assuming they care about QEMU and CHS translation), so fix it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 blockdev.c |  2 ++
 vl.c       | 18 ++++++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 36ceece..5946bbe 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -776,6 +776,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
             translation = BIOS_ATA_TRANSLATION_NONE;
         } else if (!strcmp(value, "lba")) {
             translation = BIOS_ATA_TRANSLATION_LBA;
+        } else if (!strcmp(value, "rechs")) {
+            translation = BIOS_ATA_TRANSLATION_RECHS;
         } else if (!strcmp(value, "auto")) {
             translation = BIOS_ATA_TRANSLATION_AUTO;
         } else {
diff --git a/vl.c b/vl.c
index 7f4fe0d..f161a727f 100644
--- a/vl.c
+++ b/vl.c
@@ -3053,14 +3053,17 @@ int main(int argc, char **argv, char **envp)
                         goto chs_fail;
                     if (*p == ',') {
                         p++;
-                        if (!strcmp(p, "none"))
+                        if (!strcmp(p, "none")) {
                             translation = BIOS_ATA_TRANSLATION_NONE;
-                        else if (!strcmp(p, "lba"))
+                        } else if (!strcmp(p, "lba")) {
                             translation = BIOS_ATA_TRANSLATION_LBA;
-                        else if (!strcmp(p, "auto"))
+                        } else if (!strcmp(p, "rechs")) {
+                            translation = BIOS_ATA_TRANSLATION_RECHS;
+                        } else if (!strcmp(p, "auto")) {
                             translation = BIOS_ATA_TRANSLATION_AUTO;
-                        else
+                        } else {
                             goto chs_fail;
+                        }
                     } else if (*p != '\0') {
                     chs_fail:
                         fprintf(stderr, "qemu: invalid physical CHS format\n");
@@ -3074,10 +3077,13 @@ int main(int argc, char **argv, char **envp)
                         qemu_opt_set(hda_opts, "heads", num);
                         snprintf(num, sizeof(num), "%d", secs);
                         qemu_opt_set(hda_opts, "secs", num);
-                        if (translation == BIOS_ATA_TRANSLATION_LBA)
+                        if (translation == BIOS_ATA_TRANSLATION_RECHS) {
+                            qemu_opt_set(hda_opts, "trans", "rechs");
+                        } else if (translation == BIOS_ATA_TRANSLATION_LBA) {
                             qemu_opt_set(hda_opts, "trans", "lba");
-                        if (translation == BIOS_ATA_TRANSLATION_NONE)
+                        } else if (translation == BIOS_ATA_TRANSLATION_NONE) {
                             qemu_opt_set(hda_opts, "trans", "none");
+                        }
                     }
                 }
                 break;
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] block: handle "rechs" translation option
  2014-01-30 13:08 [Qemu-devel] [PATCH] block: handle "rechs" translation option Paolo Bonzini
@ 2014-01-30 14:37 ` Markus Armbruster
  2014-01-30 14:44   ` Paolo Bonzini
  2014-01-30 14:39 ` Stefan Hajnoczi
  1 sibling, 1 reply; 4+ messages in thread
From: Markus Armbruster @ 2014-01-30 14:37 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kwolf, qemu-devel, stefanha

Paolo Bonzini <pbonzini@redhat.com> writes:

> The rechs translation option is so obscure that we support it but do

"Support" is a rather strong word:

    $ git-grep -i rechs
    include/hw/block/block.h:#define BIOS_ATA_TRANSLATION_RECHS  4

> not even attempt to parse it.  Archeologists will surely be puzzled
> by this (assuming they care about QEMU and CHS translation), so fix it.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  blockdev.c |  2 ++
>  vl.c       | 18 ++++++++++++------
>  2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 36ceece..5946bbe 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -776,6 +776,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
>              translation = BIOS_ATA_TRANSLATION_NONE;
>          } else if (!strcmp(value, "lba")) {
>              translation = BIOS_ATA_TRANSLATION_LBA;
> +        } else if (!strcmp(value, "rechs")) {
> +            translation = BIOS_ATA_TRANSLATION_RECHS;
>          } else if (!strcmp(value, "auto")) {
>              translation = BIOS_ATA_TRANSLATION_AUTO;
>          } else {

This is -drive parameter "trans", kept for backward compatibility.

> diff --git a/vl.c b/vl.c
> index 7f4fe0d..f161a727f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3053,14 +3053,17 @@ int main(int argc, char **argv, char **envp)
>                          goto chs_fail;
>                      if (*p == ',') {
>                          p++;
> -                        if (!strcmp(p, "none"))
> +                        if (!strcmp(p, "none")) {
>                              translation = BIOS_ATA_TRANSLATION_NONE;
> -                        else if (!strcmp(p, "lba"))
> +                        } else if (!strcmp(p, "lba")) {
>                              translation = BIOS_ATA_TRANSLATION_LBA;
> -                        else if (!strcmp(p, "auto"))
> +                        } else if (!strcmp(p, "rechs")) {
> +                            translation = BIOS_ATA_TRANSLATION_RECHS;
> +                        } else if (!strcmp(p, "auto")) {
>                              translation = BIOS_ATA_TRANSLATION_AUTO;
> -                        else
> +                        } else {
>                              goto chs_fail;
> +                        }
>                      } else if (*p != '\0') {
>                      chs_fail:
>                          fprintf(stderr, "qemu: invalid physical CHS format\n");
> @@ -3074,10 +3077,13 @@ int main(int argc, char **argv, char **envp)
>                          qemu_opt_set(hda_opts, "heads", num);
>                          snprintf(num, sizeof(num), "%d", secs);
>                          qemu_opt_set(hda_opts, "secs", num);
> -                        if (translation == BIOS_ATA_TRANSLATION_LBA)
> +                        if (translation == BIOS_ATA_TRANSLATION_RECHS) {
> +                            qemu_opt_set(hda_opts, "trans", "rechs");
> +                        } else if (translation == BIOS_ATA_TRANSLATION_LBA) {
>                              qemu_opt_set(hda_opts, "trans", "lba");
> -                        if (translation == BIOS_ATA_TRANSLATION_NONE)
> +                        } else if (translation == BIOS_ATA_TRANSLATION_NONE) {
>                              qemu_opt_set(hda_opts, "trans", "none");
> +                        }
>                      }
>                  }
>                  break;

This is -hdachs.  So crusty you should wear protective clothing when you
touch it.

The right way to control IDE geometry translation is ide-hd property
bios-chs-trans.  Unfortunately, you missed that one entirely.  Have a
gander at bios_chs_trans_table[] in hw/core/qdev-properties.c.

We also don't let the user ask for BIOS_ATA_TRANSLATION_LARGE.  Its only
source is hd_geometry_guess().  I have no idea whether any user would
want to set it.  But that applies to "rechs", too :)

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] block: handle "rechs" translation option
  2014-01-30 13:08 [Qemu-devel] [PATCH] block: handle "rechs" translation option Paolo Bonzini
  2014-01-30 14:37 ` Markus Armbruster
@ 2014-01-30 14:39 ` Stefan Hajnoczi
  1 sibling, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2014-01-30 14:39 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kwolf, qemu-devel, stefanha

On Thu, Jan 30, 2014 at 02:08:02PM +0100, Paolo Bonzini wrote:
> The rechs translation option is so obscure that we support it but do
> not even attempt to parse it.  Archeologists will surely be puzzled
> by this (assuming they care about QEMU and CHS translation), so fix it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  blockdev.c |  2 ++
>  vl.c       | 18 ++++++++++++------
>  2 files changed, 14 insertions(+), 6 deletions(-)

Is my understanding correct that QEMU actually does nothing with RECHS except
poke a magic number into CMOS memory when x86 guests start?

(I ask because we don't even use the constant:
$ git grep _RECHS
include/hw/block/block.h:#define BIOS_ATA_TRANSLATION_RECHS  4)

Please also add changes for:

blockdev.c:
QemuOptsList qemu_legacy_drive_opts = {
        ...
        },{
            .name = "trans",
            .type = QEMU_OPT_STRING,
            .help = "chs translation (auto, lba, none)",
        },{

qemu-options.hx:
Force hard disk 0 physical geometry (1 <= @var{c} <= 16383, 1 <=
@var{h} <= 16, 1 <= @var{s} <= 63) and optionally force the BIOS
translation mode (@var{t}=none, lba or auto).

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] block: handle "rechs" translation option
  2014-01-30 14:37 ` Markus Armbruster
@ 2014-01-30 14:44   ` Paolo Bonzini
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2014-01-30 14:44 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: kwolf, qemu-devel, stefanha

Il 30/01/2014 15:37, Markus Armbruster ha scritto:
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
>> The rechs translation option is so obscure that we support it but do
>
> "Support" is a rather strong word:
>
>     $ git-grep -i rechs
>     include/hw/block/block.h:#define BIOS_ATA_TRANSLATION_RECHS  4

As Stefan said, all we do is pass the info to the BIOS.

> The right way to control IDE geometry translation is ide-hd property
> bios-chs-trans.  Unfortunately, you missed that one entirely.  Have a
> gander at bios_chs_trans_table[] in hw/core/qdev-properties.c.

This one is handled by the other series I sent today.  That's how I 
found it.

> We also don't let the user ask for BIOS_ATA_TRANSLATION_LARGE.  Its only
> source is hd_geometry_guess().  I have no idea whether any user would
> want to set it.  But that applies to "rechs", too :)

Yeah, you are right that this patch should cover "large" as well.  Will 
send v2.

Paolo

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-01-30 14:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-30 13:08 [Qemu-devel] [PATCH] block: handle "rechs" translation option Paolo Bonzini
2014-01-30 14:37 ` Markus Armbruster
2014-01-30 14:44   ` Paolo Bonzini
2014-01-30 14:39 ` Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.