| Message ID | 0f41bb2d5732fbdb44a4eb900343a04df260e785.1413272536.git.crobinso@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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"));