[v3,17/27] ide/atapi: Preserve tray state on migration

Submitted by Markus Armbruster on Sept. 6, 2011, 4:58 p.m.

Details

Message ID 1315328340-6192-18-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Sept. 6, 2011, 4:58 p.m.
Use a subsection, so that migration to older version still works,
provided the tray is closed and unlocked.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/core.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

Comments

Paolo Bonzini Sept. 7, 2011, 7:14 a.m.
On 09/06/2011 06:58 PM, Markus Armbruster wrote:
> Use a subsection, so that migration to older version still works,
> provided the tray is closed and unlocked.
>
> Signed-off-by: Markus Armbruster<armbru@redhat.com>
> ---
>   hw/ide/core.c |   32 ++++++++++++++++++++++++++++++++
>   1 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index b1a73ee..30cb7de 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -2058,6 +2058,22 @@ static bool ide_drive_pio_state_needed(void *opaque)
>           || (s->bus->error_status&  BM_STATUS_PIO_RETRY);
>   }
>
> +static int ide_tray_state_post_load(void *opaque, int version_id)
> +{
> +    IDEState *s = opaque;
> +
> +    bdrv_eject(s->bs, s->tray_open);
> +    bdrv_lock_medium(s->bs, s->tray_locked);
> +    return 0;
> +}
> +
> +static bool ide_tray_state_needed(void *opaque)
> +{
> +    IDEState *s = opaque;
> +
> +    return s->tray_open || s->tray_locked;

I wonder if the most common case is this, or rather "tray closed and 
locked".  Perhaps it depends (for Windows it is likely unlocked, for 
Linux locked).  In any case there's time before 1.0 to fix this, so

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo
Kevin Wolf Sept. 7, 2011, 7:35 a.m.
Am 07.09.2011 09:14, schrieb Paolo Bonzini:
> On 09/06/2011 06:58 PM, Markus Armbruster wrote:
>> Use a subsection, so that migration to older version still works,
>> provided the tray is closed and unlocked.
>>
>> Signed-off-by: Markus Armbruster<armbru@redhat.com>
>> ---
>>   hw/ide/core.c |   32 ++++++++++++++++++++++++++++++++
>>   1 files changed, 32 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index b1a73ee..30cb7de 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -2058,6 +2058,22 @@ static bool ide_drive_pio_state_needed(void *opaque)
>>           || (s->bus->error_status&  BM_STATUS_PIO_RETRY);
>>   }
>>
>> +static int ide_tray_state_post_load(void *opaque, int version_id)
>> +{
>> +    IDEState *s = opaque;
>> +
>> +    bdrv_eject(s->bs, s->tray_open);
>> +    bdrv_lock_medium(s->bs, s->tray_locked);
>> +    return 0;
>> +}
>> +
>> +static bool ide_tray_state_needed(void *opaque)
>> +{
>> +    IDEState *s = opaque;
>> +
>> +    return s->tray_open || s->tray_locked;
> 
> I wonder if the most common case is this, or rather "tray closed and 
> locked".  Perhaps it depends (for Windows it is likely unlocked, for 
> Linux locked).  In any case there's time before 1.0 to fix this, so

I would argue that the common case even for Linux is that you don't have
a CD mounted (probably the drive is empty anyway).

Kevin
Paolo Bonzini Sept. 7, 2011, 8 a.m.
On 09/07/2011 09:35 AM, Kevin Wolf wrote:
> >  I wonder if the most common case is this, or rather "tray closed and
> >  locked".  Perhaps it depends (for Windows it is likely unlocked, for
> >  Linux locked).  In any case there's time before 1.0 to fix this, so
>
> I would argue that the common case even for Linux is that you don't have
> a CD mounted (probably the drive is empty anyway).

Indeed---I was thinking about automounting by a GUI, but that's not the 
typical server case.

Paolo

Patch hide | download patch | download mbox

diff --git a/hw/ide/core.c b/hw/ide/core.c
index b1a73ee..30cb7de 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2058,6 +2058,22 @@  static bool ide_drive_pio_state_needed(void *opaque)
         || (s->bus->error_status & BM_STATUS_PIO_RETRY);
 }
 
+static int ide_tray_state_post_load(void *opaque, int version_id)
+{
+    IDEState *s = opaque;
+
+    bdrv_eject(s->bs, s->tray_open);
+    bdrv_lock_medium(s->bs, s->tray_locked);
+    return 0;
+}
+
+static bool ide_tray_state_needed(void *opaque)
+{
+    IDEState *s = opaque;
+
+    return s->tray_open || s->tray_locked;
+}
+
 static bool ide_atapi_gesn_needed(void *opaque)
 {
     IDEState *s = opaque;
@@ -2085,6 +2101,19 @@  static const VMStateDescription vmstate_ide_atapi_gesn_state = {
     }
 };
 
+static const VMStateDescription vmstate_ide_tray_state = {
+    .name = "ide_drive/tray_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .post_load = ide_tray_state_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_BOOL(tray_open, IDEState),
+        VMSTATE_BOOL(tray_locked, IDEState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_ide_drive_pio_state = {
     .name = "ide_drive/pio_state",
     .version_id = 1,
@@ -2139,6 +2168,9 @@  const VMStateDescription vmstate_ide_drive = {
             .vmsd = &vmstate_ide_drive_pio_state,
             .needed = ide_drive_pio_state_needed,
         }, {
+            .vmsd = &vmstate_ide_tray_state,
+            .needed = ide_tray_state_needed,
+        }, {
             .vmsd = &vmstate_ide_atapi_gesn_state,
             .needed = ide_atapi_gesn_needed,
         }, {