Re: [Qemu-devel] [PATCH 4/5] ATAPI pass through v2: bring the new ATAPI pass throug feature

[Bique Alexandre <alexandre.bique@citrix.com>]

On Thursday 09 July 2009 15:48:34 Bique Alexandre wrote:
> On Monday 06 July 2009 12:23:22 Bique Alexandre wrote:
> > On Sunday 05 July 2009 09:15:33 Christoph Hellwig wrote:
> > > On Wed, Jul 01, 2009 at 07:32:09PM +0100, Bique Alexandre wrote:
> > > > This patch brings the new ATAPI pass through feature.
> > >
> > > I really don't understand the need for the BDRV_TYPE_CDROM_PT block
> > > device type.  The use of BDRV_TYPE_* is mostly a hint for ide/scsi
> > > what kind of device where dealing with, and a passthrough CDROM should
> > > be different from a real one.  I think we should just make any drive
> > > that points to a hardware CDROM device use the passthrough code,
> > > similar to how you'll get a hardware passthrough SCSI CDROM if you
> > > point the -drive option to a /dev/sr* device for a host CDROM.
> >
> > In the beginning I was using bsg, so it was not a classical device on
> > which you read/write/etc... It's useful there:
> >
> > @@ -2856,8 +2921,16 @@ static void ide_init2(IDEState *ide_state,
> >
> >              if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
> >                  s->is_cdrom = 1;
> > +                s->atapi_cmd = ide_atapi_cmd;
> >  		bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
> >              }
> > +#if CONFIG_ATAPI_PT
> > +            else if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM_PT) {
> > +                s->is_cdrom = 1;
> > +                s->atapi_cmd = ide_atapi_pt_cmd;
> > +                bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
> > +            }
> > +#endif /* CONFIG_ATAPI_PT */
> >          }
> >          s->drive_serial = drive_serial++;
> >          strncpy(s->drive_serial_str, drive_get_serial(s->bs),
> >
> > So I need a flag somewhere to initialize IDEState as a CD-ROM pass
> > through. Even if the emulation code and the pass through code uses the
> > same device, they don't use it the same way:
> >  - read/write/seek/... (emulation)
> >  - ioctl (pass through)
> >
> > I think that it's good to keep a way to use the emulation code over the
> > CD-ROM device.
> >
> > I'll look at a better way to do this. If you have any suggestion ? :-)
>
> Hey Christoph,
>
> I spent some time some time on looking on a better solution, but it look
> like the current one is ok for me. Because when you use -cdrom on an iso
> file, this file will be be used as BDRV_TYPE_CDROM. So i can't rely on
> BDRV_TYPE_CDROM to know if it's a real device or not. So i guess i can
> still do a stat an find if it's a block device or a regular file, and then
> have a BDRV_TYPE_ISO and BDRV_TYPE_CDROM. But that's the same as
> BDRV_TYPE_CDROM{,_PT}.
>
> For the question of the firmware upgrade. I really feel that having a
> switch is the right solution.
>
> So tell me what you want exactly and if you can give me some hint on how
> you want me to do it, then I'll be happy to do it as soon as possible to
> see theses patches merged into QEMU.
>
> Thank you very much.

Hey,

Anything new ? 

Thanks.

-- 
Alexandre Bique