[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] Add args to -cdrom to define where is conne
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] Add args to -cdrom to define where is connected the cdrom |
Date: |
Sat, 10 Nov 2007 01:02:55 +0100 |
Hi,
you can find attached a patch trying to manage your comments.
This patch adds parameter "-drive" and manages all drives type supported
by qemu (ide, scsi, sd, mtd, floppy, pflash). It adds a DriveInfo
structure to do that.
It applies on top of patch I posted 2 days ago
(http://article.gmane.org/gmane.comp.emulators.qemu/21011 )
All comments are welcome,
Laurent
Le jeudi 08 novembre 2007 à 00:32 +0100, Fabrice Bellard a écrit :
> Laurent Vivier wrote:
> > Thiemo Seufer a écrit :
> >> Laurent Vivier wrote:
> >>> Daniel P. Berrange a écrit :
> >>>> On Sun, Oct 28, 2007 at 11:43:33PM +0100, address@hidden
> >>>> wrote:
> >>>>> From: Laurent Vivier <address@hidden(none)>
> >>>>>
> >>>>> This patch allows to define where is connected the CDROM device (bus,
> >>>>> unit).
> >>>>> It extends the "-cdrom" syntax to add these paramaters:
> >>>>>
> >>>>> -cdrom file[,if=type][,bus=n][,unit=m]
> >>>>>
> >>>>> where "type" defines the interface (by default, "ide")
> >>>>> "n" defines the bus number (by default 1)
> >>>>> "m" defines the unit number (by default 0)
> >>>> Having a separately named arg just for CDROMs was always rather
> >>>> odd/unhelpful.
> >>>> I'd suggest that we leave all the -hda,hdb,hdc,-cdrom,-fda,-fdb etc
> >>>> unchanged
> >>>> and use the -disk for setting up all types of disks, floppys,
> >>>> cdroms, etc. It
> >>>> would just require one extra field for the -disk arg:
> >>>> -disk file[,if=type][,bus=n][,unit=m][,mode=mode]
> >>>> where "type" defines the interface. [ide,scsi,fd] (by default,
> >>>> "ide")
> >>>> "n" defines the bus number (by default 1)
> >>>> "m" defines the unit number (by default 0)
> >>>> "mode" defines one of [disk,floppy,cdrom]
> >>>> If we ever up able to emulate other types of SCSI / IDE devices
> >>>> (tape drives,
> >>>> cdr, dvd perhaps) then the 'mode' can easily be extended to cover them.
> >>> I agree with that. And if everyone agrees I can modify patches to do
> >>> that...
> >>
> >> Please go ahead. :-)
> >
> > Well, it is done... is there someone that can comment them ?
> > Or if they are perfect (as usual ;-) ) perhaps it could be included in
> > CVS ?
>
> I had rejected a similar patch in the past, but I agree that a -disk
> command line option is needed.
>
> A few remarks:
>
> - Add a function in vl.c to add a disk so that all option handlers can
> call it instead of doing pstrcpy(); nb_disk++.
>
> - You replaced the constant "2" in many machines by MAX_IDE_BUS. It is
> dangerous because each machine has its own constraints.
>
> - Maybe the real option name could be "-drive" to insist on the fact
> that a drive can be created without a disk in it ! Any more comments ?
>
> - disk_init() should not modify its argument str. Moreover, maybe having
> an explicit "file=" argument would be clearer in the case there is no
> disk in the drive.
>
> - While modifying the machine init function, you can suppress the
> snapshot parameter.
>
> - In disk_init(), you should factorize the bdrv_new() and bdrv_open() as
> it is the same for all types.
>
> - It could be simpler to export an array of structs containg a bdrv,
> each one tagged with if, index and bus. Then each machine could call a
> function to get the bdrv from the parameters "if", "index" and "bus".
> For example, look at the NICInfo structure and do the same with a
> structure DriveInfo...
>
> If you prefer, you can resubmit a big patch with all the changes.
>
> Regards,
>
> Fabrice.
--
------------- address@hidden --------------
"In short: just say NO TO DRUGS and maybe you won't
end up like the Hurd people." -- Linus Torvald
drive-arg.patch
Description: Text Data
signature.asc
Description: Ceci est une partie de message numériquement signée