[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
Re: [Qemu-devel] [PATCH 1/3] Add args to -cdrom to define where is connected the cdrom
Thu, 08 Nov 2007 10:02:57 +0100
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.
I agree, I'll use MAX_IDE_BUS only for PC.
But how can I compute MAX_DISKS if the number of IDE disks vary with
For the moment it is the sum of all available MAX_* .
Should I take an explicit value like "32" or something like ?
> - 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 ?
I agree, and a drive can be something else like a flash or a tape (in
> - 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.
I'm not sure I understand correctly this one.
For the moment "-disk ," allows to declare a drive without media (an
empty CDROM drive, for instance).
Do you mean we must use "-drive file=a.img" instead of "-disk a.img" and
"-drive file=" instead of "-disk ,"
> - 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.
Can I use bdrv_set_geometry_hint() and bdrv_set_translation_hint()
before the bdrv_open() ?
> - 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.
Well, I like to write a lot of little patches ;-)
It is easier to review them, isn't it ?
But it is as you want.
Thank you for your comments,
Description: Ceci est une partie de message numériquement signée