Re: [Libvir] [PATCH] Add bus attribute to disk target definition
On Tue, Apr 29, 2008 at 04:10:40PM +0200, Soren Hansen wrote:
On Tue, Apr 29, 2008 at 02:27:00PM +0100, Daniel P. Berrange wrote:
>> + if (!bus)
>> + disk->bus = QEMUD_DISK_BUS_IDE;
>> + else if (!strcmp((const char *)bus, "ide"))
>> + disk->bus = QEMUD_DISK_BUS_IDE;
>> + else if (!strcmp((const char *)bus, "scsi"))
>> + disk->bus = QEMUD_DISK_BUS_SCSI;
>> + else if (!strcmp((const char *)bus, "virtio"))
>> + disk->bus = QEMUD_DISK_BUS_VIRTIO;
> Can you use the STREQ macro here instead of strcmp.
Erm... I *could*.. I'm curious, though, why e.g. the similar code right
above it doesn't use STREQ if that's the preferred way to do it?
We've been slowly updating code to match these new standards when doing
patches.
> You can run 'make syntax-check' for check for such
issues.
Yes, in theory :) In the real world, however, "make syntax-check" fails
horribly here. I'll be fixing that up next.
If you send details to the list, Jim will no doubt be able to point you
in the right direction on this...
> Even if the -drive parameter is supported, it should still pass
the
> -boot a/c/d/n parameter in.
Why? And how would you boot from a virtio device this way?
It is needed for PXE boot at least, and IMHO, QEMU should treat 'boot c'
as if 'boot=on' were set for the first -drive parameter for back compat.
> There is nothing in the -drive parameter handling, AFAICT, that
> requires the boot drive to be listed first on the command line. So
> this first loop is not needed, and this second loop is sufficient,
> simply turn on the boot= flag on the first match drive type when
> iterating over the list.
If you want to specify more than one boot device, the only way to
determine the priority is by specifying them ordered by priority. At
least, that's how it *ought* to be, but now I see that extboot only
actually supports one boot device. :/ I could have sworn I had seen it
work with both hard drive and cdrom.
The QEMU code only allows a single boot device & will abort if > 1 has it
if (extboot_drive != -1) {
fprintf(stderr, "qemu: two bootable drives specified\n");
return -1;
}
Regards,
Dan.
--
|: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|