qemu: command: Allow UEFI for non-x86

Message ID 0f41bb2d5732fbdb44a4eb900343a04df260e785.1413272536.git.crobinso@redhat.com
State New
Headers show

Commit Message

Cole Robinson Oct. 14, 2014, 7:42 a.m.
It's supported on aarch64 and armv7l as well, so just drop the restriction
entirely since it doesn't add much.
---
 src/qemu/qemu_command.c | 8 --------
 1 file changed, 8 deletions(-)

Comments

Michal Prívozník Oct. 15, 2014, 1:55 p.m. | #1
On 14.10.2014 09:42, Cole Robinson wrote:
> It's supported on aarch64 and armv7l as well, so just drop the restriction
> entirely since it doesn't add much.
> ---
>   src/qemu/qemu_command.c | 8 --------
>   1 file changed, 8 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 8cb0865..2872e47 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7672,14 +7672,6 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
>           break;
>
>       case VIR_DOMAIN_LOADER_TYPE_PFLASH:
> -        /* UEFI is supported only for x86_64 currently */
> -        if (def->os.arch != VIR_ARCH_X86_64) {

I think it would be better to deliberately allow arches we want. As far 
as I understand the cmd line generated to use UEFI can vary depending on 
the guest architecture, is that right Laszlo? If it is so, ACK to the 
opposite patch :)

> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                           _("pflash is not supported for %s guest architecture"),
> -                           virArchToString(def->os.arch));
> -            goto cleanup;
> -        }
> -
>           if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
>               virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                              _("this QEMU binary doesn't support -drive"));
>

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Laszlo Ersek Oct. 22, 2014, 5:31 p.m. | #2
On 10/15/14 15:55, Michal Privoznik wrote:
> On 14.10.2014 09:42, Cole Robinson wrote:
>> It's supported on aarch64 and armv7l as well, so just drop the
>> restriction
>> entirely since it doesn't add much.
>> ---
>>   src/qemu/qemu_command.c | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 8cb0865..2872e47 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -7672,14 +7672,6 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr
>> cmd,
>>           break;
>>
>>       case VIR_DOMAIN_LOADER_TYPE_PFLASH:
>> -        /* UEFI is supported only for x86_64 currently */
>> -        if (def->os.arch != VIR_ARCH_X86_64) {
> 
> I think it would be better to deliberately allow arches we want. As far
> as I understand the cmd line generated to use UEFI can vary depending on
> the guest architecture, is that right Laszlo? If it is so, ACK to the
> opposite patch :)
> 
>> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> -                           _("pflash is not supported for %s guest
>> architecture"),
>> -                           virArchToString(def->os.arch));
>> -            goto cleanup;
>> -        }
>> -
>>           if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
>>               virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>                              _("this QEMU binary doesn't support
>> -drive"));
>>
> 
> Michal

Sorry about the late response.

I wasn't CC'd on either this patch or the "opposite" one. But, I think I
agree with Michal -- enumerating architectures that are allowed to use
UEFI seems safer at this point than a whole-sale enablement (and also
safer than enumerating the architectures that are *not* allowed to use
UEFI).

So, default to "off", list the arches one by one that are allowed to use
UEFI, and (if necessary) customize the command line / pflash switches
for the given arch.

Thanks
Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patch

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8cb0865..2872e47 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7672,14 +7672,6 @@  qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
         break;
 
     case VIR_DOMAIN_LOADER_TYPE_PFLASH:
-        /* UEFI is supported only for x86_64 currently */
-        if (def->os.arch != VIR_ARCH_X86_64) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("pflash is not supported for %s guest architecture"),
-                           virArchToString(def->os.arch));
-            goto cleanup;
-        }
-
         if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("this QEMU binary doesn't support -drive"));