[libvirt] Re: [Libvir] [PATCH] Add bus attribute to disk target definition

[Soren Hansen]

On Tue, May 06, 2008 at 10:02:21PM +0100, Daniel P. Berrange wrote:
> > sorry for the delay. Here's the newest version of the patch which
> > should address all the issues that have been raised so far.
> Yes & no. It didn't address the redundant re-ordering of -drive
> parameters when using boot=on, 

The reasoning here was (I mentioned this in a previous mail, too) that
when qemu/kvm some day grows the ability to have more than one boot
device specified with boot=on (using extboot or whatever), we're going
to have to change things *anyway*.  Ordering the devices according to
boot priority seems like a reasonable guess as to what would be required
to do, so I figured I'd leave it as is.

> nor re-add the -boot param to make PXE work again. 

Yes and no. In the latest patch I provided I only set use_extboot if
there's only one boot device defined, and it's a virtio device, so PXE
booting would use the old-style "-boot n" syntax. I literallly woke up
this morning and instantly smacked my forehead due to another problem
this introduced, so I'm happy you changed it. :)

> One further complication is that QEMU 0.9.1 has -drive support but not
> the extboot support, so boot=on can't be used there.  It rather
> annoyingly complains 
> 
>   qemu: unknowm parameter 'boot' in 'file=/home/berrange/boot.iso,media=cdrom,boot=on'

Ah, figures.

>> +    if (!bus)
>> +        disk->bus = QEMUD_DISK_BUS_IDE;
> This was giving floppy disks a bus of 'ide' which isn't technically
> correct - they're really their own bus type - I reckon we should call
> it 'fdc'.

Ah. Yes, I must admit that floppy disks were completely off my radar.

> This double loop is redundant - there's no need to pull bootable
> drives to the front of the list - this is why there's an explicit flag
> rather than relying on ordering.

I did this for two reasons:

a) I wanted to avoid the bootDisk, bootFloppy, bootCD variables approach
you used. It just didn't appeal to me. *shrug*

b) As I mentioned further up, this was also done in an attempt to match
what would be needed when it becomes possible to specify ",boot=on" for
more than one device, but we can revisit this when that day comes.

Your patch looks fine to me.

Oh, and thanks for doing all the test cases as well. I didn't want to
get started on those until we had agreed on the logic that should be
applied.

-- 
Soren Hansen               | 
Virtualisation specialist  | Ubuntu Server Team
Canonical Ltd.             | http://www.ubuntu.com/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20080507/01112771/attachment-0001.sig>