Re: [Qemu-devel] [PATCH v2 22/45] ide/atapi: Avoid physical/virtual tray state mismatch

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 03.08.2011 15:08, schrieb Markus Armbruster:
>> When ide-cd is backed by a physical drive, we want the physical tray
>> match the virtual one.  To that end, we call bdrv_eject() on guest's
>> load/eject, and bdrv_lock_medium() on guest's prevent/allow removal.
>> But we don't set the initial state on device model init.  Fix that.
>> 
>> While there, also unlock on device model exit.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>  hw/ide/core.c |    4 ++++
>>  hw/ide/qdev.c |   18 ++++++++++++++++++
>>  2 files changed, 22 insertions(+), 0 deletions(-)
>> 
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index 5bcc857..d8b1d43 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -1839,6 +1839,10 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, 
>> IDEDriveKind kind,
>>  
>>      ide_reset(s);
>>      bdrv_set_removable(bs, s->drive_kind == IDE_CD);
>> +    if (s->drive_kind == IDE_CD) {
>> +        bdrv_lock_medium(bs, s->tray_locked);
>> +        bdrv_eject(bs, s->tray_open);
>> +    }
>>      return 0;
>>  }
>>  
>> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
>> index 3b7b306..bc2f426 100644
>> --- a/hw/ide/qdev.c
>> +++ b/hw/ide/qdev.c
>> @@ -182,6 +182,12 @@ static int ide_cd_initfn(IDEDevice *dev)
>>      return ide_dev_initfn(dev, IDE_CD);
>>  }
>>  
>> +static int ide_cd_exitfn(IDEDevice *dev)
>> +{
>> +    bdrv_lock_medium(dev->conf.bs, 0);
>> +    return 0;
>> +}
>> +
>>  static int ide_drive_initfn(IDEDevice *dev)
>>  {
>>      DriveInfo *dinfo = drive_get_by_blockdev(dev->conf.bs);
>> @@ -189,6 +195,16 @@ static int ide_drive_initfn(IDEDevice *dev)
>>      return ide_dev_initfn(dev, dinfo->media_cd ? IDE_CD : IDE_HD);
>>  }
>>  
>> +static int ide_drive_exitfn(IDEDevice *dev)
>> +{
>> +    DriveInfo *dinfo = drive_get_by_blockdev(dev->conf.bs);
>> +
>> +    if (dinfo->media_cd) {
>> +        return ide_cd_exitfn(dev);
>> +    }
>
> Is dinfo->media_cd guaranteed to be the same as s->drive_kind? I
> wouldn't have expected this to be true at least since the introduction
> of ide-hd/cd.

I'm afraid you're right.  Now I need to find a way from IDEDevice to
IDEState.  These IDE data structures are sick...