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, [EMAIL PROTECTED] >>>> wrote: >>>>> From: Laurent Vivier <[EMAIL PROTECTED](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.
