Re: [Qemu-devel] [PATCH 1/7] xen: groundwork for xen support

[Gerd Hoffmann]

>> +/* xen_machine.c */
>> +extern QEMUMachine xenpv_machine;
>> +extern QEMUMachine xenfv_machine;
> 
> Why does xenfv need its own machine type?

This is how xen's qemu-dm handles it at the moment and I've decided to
do it the same way for simplicity.  I think qemu could also figure it
using a hypercall.

>> -    /* XXX: this should not be: some embedded targets just have flash */
>> +    /* need a disk for this machine to boot ? */
>> +    /* XXX: add embedded targets which just have flash */
>> +    nodisk_ok = 0;
>> +    if (0 == strcmp(machine->name, "xenpv"))
>> +    nodisk_ok = 1;
>>      if (!linux_boot && net_boot == 0 &&
>> -        nb_drives_opt == 0)
>> +        !nodisk_ok && nb_drives_opt == 0)
>>          help(1);
> 
> This patch is pretty clean with the exception of this bit.  I think a
> cleaner way to do this would be to let the machine types specify whether
> a disk is needed or not.

Agreed, I'll make it a machine_type field.

thanks,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/