[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]