Re: [PATCH v4 2/3] qemu: Implement extended loader and nvram

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

 



On 22.08.2014 18:48, Eric Blake wrote:
On 08/21/2014 02:50 AM, Michal Privoznik wrote:
QEMU now supports UEFI with the following command line:

   -drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \
   -drive file=/usr/share/OVMF/OVMF_VARS.fd,if=pflash,format=raw,unit=1 \

where the first line reflects <loader> and the second one <nvram>.
Moreover, these two lines obsoletes the -bios argument.

s/obsoletes/obsolete/


Note that UEFI is unusable without ACPI. This is handled properly now.
Among with this extension, the variable file is expected to be
writable and hence we need security drivers to label it.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---

+    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 achitecture"),

s/achitecture/architecture/


+
+        if (loader->readonly) {
+            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("this qemu doesn't support passing "
+                                 "readonly attribute"));
+                goto cleanup;
+            }
+
+            virBufferAsprintf(&buf, ",readonly=%s",
+                              virTristateSwitchTypeToString(loader->readonly));
+        }
+
+        virCommandAddArg(cmd, "-drive");
+        virCommandAddArgBuffer(cmd, &buf);
+
+        if (loader->nvram) {
+            virBufferFreeAndReset(&buf);
+            virBufferAsprintf(&buf,
+                              "file=%s,if=pflash,format=raw,unit=%d",
+                              loader->nvram, unit);

Hmm.  Here, it looks like readonly='on' is supported ONLY for
type='pflash', and not for type='rom'.  In that case, I'd write the .rng
of patch 1 as (rough draft):

<element name='loader'>
   <choice>
     <group> <!-- bios, default loader type -->
       <optional>
         <attribute name='type'>
           <value>rom</value>
         </attribute>
       </optional>
     </group>
     <group> <!-- pflash, for OVMF use -->
       <attribute name='type'>
         <value>pflash</value>
       </attribute>
       <optional>
         <attribute name='readonly'...>
       </optional>
       <optional>
         nvram stuff...
       </optional>
     </group>
   </choice>
   <ref name='absFileName'>
</element>

While this can be correct I think having wider XML schema then the code is just okay. Keeping the schema readable wins over tightening all the narrow cases for me.

Michal

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]